From da63c2c57977840cf0b349c84ab139a92e6f4787 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Fri, 27 Oct 2017 16:24:12 -0700 Subject: [PATCH 1/8] Exclude legacy safelist files in external projects --- src/compiler/core.ts | 7 ++++ src/server/editorServices.ts | 33 +++++++++++++++---- src/services/jsTyping.ts | 2 +- .../reference/api/tsserverlibrary.d.ts | 7 ++-- 4 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 12b8dd2f87e..c9b644b22ce 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -2413,6 +2413,13 @@ namespace ts { return (removeFileExtension(path) + newExtension); } + /** + * Takes a string like "jquery-min.4.2.3" and returns "jquery" + */ + export function removeMinAndVersionNumbers(fileName: string) { + return fileName.replace(/((?:\.|-)min(?=\.|$))|((?:-|\.)\d+)/g, ""); + } + export interface ObjectAllocator { getNodeConstructor(): new (kind: SyntaxKind, pos?: number, end?: number) => Node; getTokenConstructor(): new (kind: TKind, pos?: number, end?: number) => Token; diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index b7617e1fa17..eb2ae0f3cd9 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -110,7 +110,7 @@ namespace ts.server { export interface TypesMapFile { typesMap: SafeList; - simpleMap: string[]; + simpleMap: { [libName: string]: string }; } /** @@ -374,6 +374,7 @@ namespace ts.server { private readonly hostConfiguration: HostConfiguration; private safelist: SafeList = defaultTypeSafeList; + private legacySafelist: { [key: string]: string } = {}; private changedFiles: ScriptInfo[]; private pendingProjectUpdates = createMap(); @@ -426,9 +427,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); @@ -518,10 +522,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 = {}; } } @@ -1393,7 +1399,7 @@ namespace ts.server { return false; } - 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, @@ -1402,6 +1408,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); @@ -2204,7 +2211,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]}'`); + excludedFiles.push(normalizedNames[i]); + exclude = true; + } + } + } + if (!exclude) { + filesToKeep.push(proj.rootFiles[i]); + } } } proj.rootFiles = filesToKeep; @@ -2312,8 +2334,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); diff --git a/src/services/jsTyping.ts b/src/services/jsTyping.ts index 572858dd2fd..e31201b951f 100644 --- a/src/services/jsTyping.ts +++ b/src/services/jsTyping.ts @@ -180,7 +180,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) { diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 5d439e840b0..44c8095b9fb 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -7391,7 +7391,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; @@ -7468,6 +7470,7 @@ declare namespace ts.server { private readonly throttledOperations; private readonly hostConfiguration; private safelist; + private legacySafelist; private changedFiles; private pendingProjectUpdates; private pendingInferredProjectUpdate; @@ -7576,7 +7579,7 @@ declare namespace ts.server { private findExternalProjectByProjectName(projectFileName); private convertConfigFileContentToProjectOptions(configFilename, cachedDirectoryStructureHost); private exceededTotalSizeLimitForNonTsFiles(name, options, fileNames, propertyReader); - private createExternalProject(projectFileName, files, options, typeAcquisition); + private createExternalProject(projectFileName, files, options, typeAcquisition, excludedFiles); private sendProjectTelemetry(projectKey, project, projectOptions?); private addFilesToNonInferredProjectAndUpdateGraph(project, files, propertyReader, typeAcquisition); private createConfiguredProject(configFileName); From 5830bb9b19f400994f77bdb1df0b5aff888dbdc3 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Fri, 27 Oct 2017 17:07:04 -0700 Subject: [PATCH 2/8] Improved logging --- src/server/editorServices.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index eb2ae0f3cd9..0bd02f32d95 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -2152,7 +2152,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) { @@ -2218,7 +2218,7 @@ namespace ts.server { const inferredTypingName = removeFileExtension(baseName); const cleanedTypingName = removeMinAndVersionNumbers(inferredTypingName); if (this.legacySafelist[cleanedTypingName]) { - this.logger.info(`Excluded '${normalizedNames[i]}'`); + this.logger.info(`Excluded '${normalizedNames[i]}' because it matched ${cleanedTypingName} from the legacy safelist`); excludedFiles.push(normalizedNames[i]); exclude = true; } From 5395d0ddb872acb83c2cca3ec31240572082db05 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Mon, 30 Oct 2017 10:44:51 -0700 Subject: [PATCH 3/8] Add test --- .../unittests/tsserverProjectSystem.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index bc8b476ceb9..fb2bbec6741 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -1496,6 +1496,26 @@ namespace ts.projectSystem { } }); + 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", From 5dc02ef5ccf267c0f4a07f4571d0b8d603a9eefa Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Tue, 31 Oct 2017 18:38:00 -0700 Subject: [PATCH 4/8] Use a different RegEx --- src/compiler/core.ts | 8 ++++++- .../unittests/tsserverProjectSystem.ts | 22 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index c9b644b22ce..626480035b1 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -2417,7 +2417,13 @@ namespace ts { * Takes a string like "jquery-min.4.2.3" and returns "jquery" */ export function removeMinAndVersionNumbers(fileName: string) { - return fileName.replace(/((?:\.|-)min(?=\.|$))|((?:-|\.)\d+)/g, ""); + const match = /((\w|(-(?!min)))+)(\.|-)?.*/.exec(fileName); + if (match) { + return match[1]; + } + else { + return fileName; + } } export interface ObjectAllocator { diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index fb2bbec6741..c72a27e173e 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -1496,6 +1496,28 @@ 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.4.2-test.js", "jquery"], + ["jquery.min.4.2.1", "jquery"], + ["jquery.7.min.js", "jquery"], + ["jquery.7.min-beta", "jquery"], + ["minimum", "minimum"], + ["min", "min"], + ["min.3.2", "min"], + ["jquery", "jquery"] + ]; + const suffixes = [".js", ".jsx", ""]; + for (const t of testData) { + for (const suf of suffixes) { + assert.equal(removeMinAndVersionNumbers(t[0] + suf), t[1]); + } + } + }); + it("ignores files excluded by a legacy safe type list", () => { const file1 = { path: "/a/b/bliss.js", From ddd8c95c63766d2eb321404213bb7a999193aef8 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Thu, 9 Nov 2017 12:30:29 -0800 Subject: [PATCH 5/8] Remove testcases we don't like --- src/harness/unittests/tsserverProjectSystem.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index c72a27e173e..5b6c36d071f 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -1501,20 +1501,17 @@ namespace ts.projectSystem { ["jquery-max", "jquery-max"], ["jquery.min", "jquery"], ["jquery-min.4.2.3", "jquery"], - ["jquery.4.2-test.js", "jquery"], + // ["jquery.4.2-test.js", "jquery"], ["jquery.min.4.2.1", "jquery"], - ["jquery.7.min.js", "jquery"], - ["jquery.7.min-beta", "jquery"], + // ["jquery.7.min.js", "jquery"], + // ["jquery.7.min-beta", "jquery"], ["minimum", "minimum"], ["min", "min"], ["min.3.2", "min"], ["jquery", "jquery"] ]; - const suffixes = [".js", ".jsx", ""]; for (const t of testData) { - for (const suf of suffixes) { - assert.equal(removeMinAndVersionNumbers(t[0] + suf), t[1]); - } + assert.equal(removeMinAndVersionNumbers(t[0]), t[1], t[0]); } }); From 19cc42782b1abe17fdd3911505f8afe36689d053 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Thu, 9 Nov 2017 12:30:36 -0800 Subject: [PATCH 6/8] Format + new regex --- src/compiler/core.ts | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 626480035b1..657f362067b 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -74,13 +74,13 @@ namespace ts { } // The global Map object. This may not be available, so we must test for it. - declare const Map: { new(): Map } | undefined; + declare const Map: { new (): Map } | undefined; // Internet Explorer's Map doesn't support iteration, so don't use it. // tslint:disable-next-line:no-in-operator 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(): Map } { + function shimMap(): { new (): Map } { class MapIterator { private data: MapLike; @@ -103,7 +103,7 @@ namespace ts { } } - return class implements Map { + return class implements Map { private data = createDictionaryObject(); public size = 0; @@ -166,8 +166,8 @@ namespace ts { } export const enum Comparison { - LessThan = -1, - EqualTo = 0, + LessThan = -1, + EqualTo = 0, GreaterThan = 1 } @@ -2417,13 +2417,11 @@ namespace ts { * Takes a string like "jquery-min.4.2.3" and returns "jquery" */ export function removeMinAndVersionNumbers(fileName: string) { - const match = /((\w|(-(?!min)))+)(\.|-)?.*/.exec(fileName); - if (match) { - return match[1]; - } - else { - return fileName; - } + // 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 { @@ -2627,7 +2625,7 @@ namespace ts { return findBestPatternMatch(patterns, _ => _, candidate); } - export function patternText({prefix, suffix}: Pattern): string { + export function patternText({ prefix, suffix }: Pattern): string { return `${prefix}*${suffix}`; } @@ -2657,7 +2655,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); From 0e105ad8de2b64a262b978b78ebca6c465a45bbf Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Thu, 9 Nov 2017 12:30:44 -0800 Subject: [PATCH 7/8] Log more usefully when this test fails --- src/harness/unittests/typingsInstaller.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/harness/unittests/typingsInstaller.ts b/src/harness/unittests/typingsInstaller.ts index fb1a7a26a7a..65d904940b3 100644 --- a/src/harness/unittests/typingsInstaller.ts +++ b/src/harness/unittests/typingsInstaller.ts @@ -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(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"]); }); From 0d5dec9a9870637cb4602cd7eb05683d97d0dcda Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Thu, 9 Nov 2017 13:55:20 -0800 Subject: [PATCH 8/8] Remove commented tests --- src/harness/unittests/tsserverProjectSystem.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 018934e9d7d..ea573726eb8 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -1544,10 +1544,7 @@ namespace ts.projectSystem { ["jquery-max", "jquery-max"], ["jquery.min", "jquery"], ["jquery-min.4.2.3", "jquery"], - // ["jquery.4.2-test.js", "jquery"], ["jquery.min.4.2.1", "jquery"], - // ["jquery.7.min.js", "jquery"], - // ["jquery.7.min-beta", "jquery"], ["minimum", "minimum"], ["min", "min"], ["min.3.2", "min"],