Skip to content

Exclude legacy safelist files in external projects #19542

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Nov 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,13 @@ namespace ts {
}

// The global Map object. This may not be available, so we must test for it.
declare const Map: { new<T>(): Map<T> } | undefined;
declare const Map: { new <T>(): Map<T> } | undefined;
// Internet Explorer's Map doesn't support iteration, so don't use it.
// tslint:disable-next-line no-in-operator variable-name
const MapCtr = typeof Map !== "undefined" && "entries" in Map.prototype ? Map : shimMap();

// Keep the class inside a function so it doesn't get compiled if it's not used.
function shimMap(): { new<T>(): Map<T> } {
function shimMap(): { new <T>(): Map<T> } {

class MapIterator<T, U extends (string | T | [string, T])> {
private data: MapLike<T>;
Expand All @@ -97,7 +97,7 @@ namespace ts {
}
}

return class<T> implements Map<T> {
return class <T> implements Map<T> {
private data = createDictionaryObject<T>();
public size = 0;

Expand Down Expand Up @@ -2635,6 +2635,17 @@ namespace ts {
return <T>(removeFileExtension(path) + newExtension);
}

/**
* Takes a string like "jquery-min.4.2.3" and returns "jquery"
*/
export function removeMinAndVersionNumbers(fileName: string) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need this function in compiler? Can it be part of services\utilities instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This functions needs to be used in the typings installer and in tsserver; I couldn't find another file that was shared by them. services\utilities isn't involved in the typings installer and adding it introduced more errors because it needs references to e.g. Project

// Match a "." or "-" followed by a version number or 'min' at the end of the name
const trailingMinOrVersion = /[.-]((min)|(\d+(\.\d+)*))$/;

// The "min" or version may both be present, in either order, so try applying the above twice.
return fileName.replace(trailingMinOrVersion, "").replace(trailingMinOrVersion, "");
}

export interface ObjectAllocator {
getNodeConstructor(): new (kind: SyntaxKind, pos?: number, end?: number) => Node;
getTokenConstructor(): new <TKind extends SyntaxKind>(kind: TKind, pos?: number, end?: number) => Token<TKind>;
Expand Down Expand Up @@ -2835,7 +2846,7 @@ namespace ts {
return findBestPatternMatch(patterns, _ => _, candidate);
}

export function patternText({prefix, suffix}: Pattern): string {
export function patternText({ prefix, suffix }: Pattern): string {
return `${prefix}*${suffix}`;
}

Expand Down Expand Up @@ -2865,7 +2876,7 @@ namespace ts {
return matchedValue;
}

function isPatternMatch({prefix, suffix}: Pattern, candidate: string) {
function isPatternMatch({ prefix, suffix }: Pattern, candidate: string) {
return candidate.length >= prefix.length + suffix.length &&
startsWith(candidate, prefix) &&
endsWith(candidate, suffix);
Expand Down
36 changes: 36 additions & 0 deletions src/harness/unittests/tsserverProjectSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1539,6 +1539,42 @@ namespace ts.projectSystem {
}
});

it("removes version numbers correctly", () => {
const testData: [string, string][] = [
["jquery-max", "jquery-max"],
["jquery.min", "jquery"],
["jquery-min.4.2.3", "jquery"],
["jquery.min.4.2.1", "jquery"],
["minimum", "minimum"],
["min", "min"],
["min.3.2", "min"],
["jquery", "jquery"]
];
for (const t of testData) {
assert.equal(removeMinAndVersionNumbers(t[0]), t[1], t[0]);
}
});

it("ignores files excluded by a legacy safe type list", () => {
const file1 = {
path: "/a/b/bliss.js",
content: "let x = 5"
};
const file2 = {
path: "/a/b/foo.js",
content: ""
};
const host = createServerHost([file1, file2, customTypesMap]);
const projectService = createProjectService(host);
try {
projectService.openExternalProject({ projectFileName: "project", options: {}, rootFiles: toExternalFiles([file1.path, file2.path]), typeAcquisition: { enable: true } });
const proj = projectService.externalProjects[0];
assert.deepEqual(proj.getFileNames(), [file2.path]);
} finally {
projectService.resetSafeList();
}
});

it("open file become a part of configured project if it is referenced from root file", () => {
const file1 = {
path: "/a/b/f1.ts",
Expand Down
5 changes: 3 additions & 2 deletions src/harness/unittests/typingsInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1057,11 +1057,12 @@ namespace ts.projectSystem {
const host = createServerHost([app, jquery, chroma]);
const logger = trackingLogger();
const result = JsTyping.discoverTypings(host, logger.log, [app.path, jquery.path, chroma.path], getDirectoryPath(<Path>app.path), safeList, emptyMap, { enable: true }, emptyArray);
assert.deepEqual(logger.finish(), [
const finish = logger.finish();
assert.deepEqual(finish, [
'Inferred typings from file names: ["jquery","chroma-js"]',
"Inferred typings from unresolved imports: []",
'Result: {"cachedTypingPaths":[],"newTypingNames":["jquery","chroma-js"],"filesToWatch":["/a/b/bower_components","/a/b/node_modules"]}',
]);
], finish.join("\r\n"));
assert.deepEqual(result.newTypingNames, ["jquery", "chroma-js"]);
});

Expand Down
35 changes: 28 additions & 7 deletions src/server/editorServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ namespace ts.server {

export interface TypesMapFile {
typesMap: SafeList;
simpleMap: string[];
simpleMap: { [libName: string]: string };
}

/**
Expand Down Expand Up @@ -380,6 +380,7 @@ namespace ts.server {

private readonly hostConfiguration: HostConfiguration;
private safelist: SafeList = defaultTypeSafeList;
private legacySafelist: { [key: string]: string } = {};

private changedFiles: ScriptInfo[];
private pendingProjectUpdates = createMap<Project>();
Expand Down Expand Up @@ -432,9 +433,12 @@ namespace ts.server {
this.toCanonicalFileName = createGetCanonicalFileName(this.host.useCaseSensitiveFileNames);
this.throttledOperations = new ThrottledOperations(this.host, this.logger);

if (opts.typesMapLocation) {
if (this.typesMapLocation) {
this.loadTypesMap();
}
else {
this.logger.info("No types map provided; using the default");
}

this.typingsInstaller.attach(this);

Expand Down Expand Up @@ -524,10 +528,12 @@ namespace ts.server {
}
// raw is now fixed and ready
this.safelist = raw.typesMap;
this.legacySafelist = raw.simpleMap;
}
catch (e) {
this.logger.info(`Error loading types map: ${e}`);
this.safelist = defaultTypeSafeList;
this.legacySafelist = {};
}
}

Expand Down Expand Up @@ -1418,7 +1424,7 @@ namespace ts.server {
}
}

private createExternalProject(projectFileName: string, files: protocol.ExternalFile[], options: protocol.ExternalProjectCompilerOptions, typeAcquisition: TypeAcquisition) {
private createExternalProject(projectFileName: string, files: protocol.ExternalFile[], options: protocol.ExternalProjectCompilerOptions, typeAcquisition: TypeAcquisition, excludedFiles: NormalizedPath[]) {
const compilerOptions = convertCompilerOptions(options);
const project = new ExternalProject(
projectFileName,
Expand All @@ -1427,6 +1433,7 @@ namespace ts.server {
compilerOptions,
/*languageServiceEnabled*/ !this.exceededTotalSizeLimitForNonTsFiles(projectFileName, compilerOptions, files, externalFilePropertyReader),
options.compileOnSave === undefined ? true : options.compileOnSave);
project.excludedFiles = excludedFiles;

this.addFilesToNonInferredProjectAndUpdateGraph(project, files, externalFilePropertyReader, typeAcquisition);
this.externalProjects.push(project);
Expand Down Expand Up @@ -2197,7 +2204,7 @@ namespace ts.server {
const rule = this.safelist[name];
for (const root of normalizedNames) {
if (rule.match.test(root)) {
this.logger.info(`Excluding files based on rule ${name}`);
this.logger.info(`Excluding files based on rule ${name} matching file '${root}'`);

// If the file matches, collect its types packages and exclude rules
if (rule.types) {
Expand Down Expand Up @@ -2256,7 +2263,22 @@ namespace ts.server {
excludedFiles.push(normalizedNames[i]);
}
else {
filesToKeep.push(proj.rootFiles[i]);
let exclude = false;
if (typeAcquisition && (typeAcquisition.enable || typeAcquisition.enableAutoDiscovery)) {
const baseName = getBaseFileName(normalizedNames[i].toLowerCase());
if (fileExtensionIs(baseName, "js")) {
const inferredTypingName = removeFileExtension(baseName);
const cleanedTypingName = removeMinAndVersionNumbers(inferredTypingName);
if (this.legacySafelist[cleanedTypingName]) {
this.logger.info(`Excluded '${normalizedNames[i]}' because it matched ${cleanedTypingName} from the legacy safelist`);
excludedFiles.push(normalizedNames[i]);
exclude = true;
}
}
}
if (!exclude) {
filesToKeep.push(proj.rootFiles[i]);
}
}
}
proj.rootFiles = filesToKeep;
Expand Down Expand Up @@ -2364,8 +2386,7 @@ namespace ts.server {
else {
// no config files - remove the item from the collection
this.externalProjectToConfiguredProjectMap.delete(proj.projectFileName);
const newProj = this.createExternalProject(proj.projectFileName, rootFiles, proj.options, proj.typeAcquisition);
newProj.excludedFiles = excludedFiles;
this.createExternalProject(proj.projectFileName, rootFiles, proj.options, proj.typeAcquisition, excludedFiles);
}
if (!suppressRefreshOfInferredProjects) {
this.ensureProjectStructuresUptoDate(/*refreshInferredProjects*/ true);
Expand Down
2 changes: 1 addition & 1 deletion src/services/jsTyping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ namespace ts.JsTyping {
if (!hasJavaScriptFileExtension(j)) return undefined;

const inferredTypingName = removeFileExtension(getBaseFileName(j.toLowerCase()));
const cleanedTypingName = inferredTypingName.replace(/((?:\.|-)min(?=\.|$))|((?:-|\.)\d+)/g, "");
const cleanedTypingName = removeMinAndVersionNumbers(inferredTypingName);
return safeList.get(cleanedTypingName);
});
if (fromFileNames.length) {
Expand Down
7 changes: 5 additions & 2 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7429,7 +7429,9 @@ declare namespace ts.server {
}
interface TypesMapFile {
typesMap: SafeList;
simpleMap: string[];
simpleMap: {
[libName: string]: string;
};
}
function convertFormatOptions(protocolOptions: protocol.FormatCodeSettings): FormatCodeSettings;
function convertCompilerOptions(protocolOptions: protocol.ExternalProjectCompilerOptions): CompilerOptions & protocol.CompileOnSaveMixin;
Expand Down Expand Up @@ -7510,6 +7512,7 @@ declare namespace ts.server {
private readonly throttledOperations;
private readonly hostConfiguration;
private safelist;
private legacySafelist;
private changedFiles;
private pendingProjectUpdates;
private pendingInferredProjectUpdate;
Expand Down Expand Up @@ -7618,7 +7621,7 @@ declare namespace ts.server {
private findExternalProjectByProjectName(projectFileName);
private convertConfigFileContentToProjectOptions(configFilename, cachedDirectoryStructureHost);
private exceededTotalSizeLimitForNonTsFiles<T>(name, options, fileNames, propertyReader);
private createExternalProject(projectFileName, files, options, typeAcquisition);
private createExternalProject(projectFileName, files, options, typeAcquisition, excludedFiles);
private sendProjectTelemetry(projectKey, project, projectOptions?);
private addFilesToNonInferredProjectAndUpdateGraph<T>(project, files, propertyReader, typeAcquisition);
private createConfiguredProject(configFileName);
Expand Down