From 34ea1b78559a65cd7af22e84d4494508d3626534 Mon Sep 17 00:00:00 2001 From: zhengbli Date: Wed, 5 Oct 2016 14:02:46 -0700 Subject: [PATCH 1/3] Turn on skipLibCheck for js-only inferred project and external project --- src/compiler/commandLineParser.ts | 2 +- .../convertCompilerOptionsFromJson.ts | 8 +- .../unittests/tsserverProjectSystem.ts | 84 +++++++++++++++++++ src/server/project.ts | 56 +++++++++++-- src/server/session.ts | 4 +- 5 files changed, 144 insertions(+), 10 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 648447ac26ac4..72dd42b3997e5 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -975,7 +975,7 @@ namespace ts { basePath: string, errors: Diagnostic[], configFileName?: string): CompilerOptions { const options: CompilerOptions = getBaseFileName(configFileName) === "jsconfig.json" - ? { allowJs: true, maxNodeModuleJsDepth: 2, allowSyntheticDefaultImports: true } + ? { allowJs: true, maxNodeModuleJsDepth: 2, allowSyntheticDefaultImports: true, skipLibCheck: true } : {}; convertOptionsFromJson(optionDeclarations, jsonOptions, basePath, options, Diagnostics.Unknown_compiler_option_0, errors); return options; diff --git a/src/harness/unittests/convertCompilerOptionsFromJson.ts b/src/harness/unittests/convertCompilerOptionsFromJson.ts index ba25409a9b4af..33cad7130c76c 100644 --- a/src/harness/unittests/convertCompilerOptionsFromJson.ts +++ b/src/harness/unittests/convertCompilerOptionsFromJson.ts @@ -405,6 +405,7 @@ namespace ts { allowJs: true, maxNodeModuleJsDepth: 2, allowSyntheticDefaultImports: true, + skipLibCheck: true, module: ModuleKind.CommonJS, target: ScriptTarget.ES5, noImplicitAny: false, @@ -433,6 +434,7 @@ namespace ts { allowJs: false, maxNodeModuleJsDepth: 2, allowSyntheticDefaultImports: true, + skipLibCheck: true, module: ModuleKind.CommonJS, target: ScriptTarget.ES5, noImplicitAny: false, @@ -456,7 +458,8 @@ namespace ts { { allowJs: true, maxNodeModuleJsDepth: 2, - allowSyntheticDefaultImports: true + allowSyntheticDefaultImports: true, + skipLibCheck: true }, errors: [{ file: undefined, @@ -477,7 +480,8 @@ namespace ts { { allowJs: true, maxNodeModuleJsDepth: 2, - allowSyntheticDefaultImports: true + allowSyntheticDefaultImports: true, + skipLibCheck: true }, errors: [] } diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 5b934cc4b0bd7..23fbf69af37ff 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -2063,4 +2063,88 @@ namespace ts.projectSystem { projectService.inferredProjects[0].getLanguageService().getProgram(); }); }); + + describe("skipLibCheck", () => { + it("should be turned on for js-only inferred projects", () => { + const file1 = { + path: "/a/b/file1.js", + content: ` + /// + var x = 1;` + }; + const file2 = { + path: "/a/b/file2.d.ts", + content: ` + interface T { + name: string; + }; + interface T { + name: number; + };` + }; + const host = createServerHost([file1, file2]); + const projectService = createProjectService(host); + + projectService.openClientFile(file2.path); + checkNumberOfInferredProjects(projectService, 1); + let inferredProject = projectService.inferredProjects[0]; + assert.isNotTrue(inferredProject.getCompilerOptions().skipLibCheck); + + projectService.openClientFile(file1.path); + checkNumberOfInferredProjects(projectService, 1); + inferredProject = projectService.inferredProjects[0]; + assert.isTrue(inferredProject.getCompilerOptions().skipLibCheck); + + projectService.closeClientFile(file1.path); + checkNumberOfInferredProjects(projectService, 1); + inferredProject = projectService.inferredProjects[0]; + assert.isNotTrue(inferredProject.getCompilerOptions().skipLibCheck); + }); + + it("should be turned on for js-only external projects", () => { + const file1 = { + path: "/a/b/f1.js", + content: "let x =1;" + }; + const file2 = { + path: "/a/b/f2.d.ts", + content: "interface T {};" + }; + const externalProjectName = "externalproject"; + const host = createServerHost([file1, file2]); + const projectService = createProjectService(host); + projectService.openExternalProject({ + rootFiles: toExternalFiles([file1.path, file2.path]), + options: {}, + projectFileName: externalProjectName + }); + + checkNumberOfExternalProjects(projectService, 1); + const externalProject = projectService.externalProjects[0]; + assert.isTrue(externalProject.getCompilerOptions().skipLibCheck); + }); + + it("should not be turned on for ts external projects", () => { + const file1 = { + path: "/a/b/f1.ts", + content: "let x =1;" + }; + const file2 = { + path: "/a/b/f2.d.ts", + content: "interface T {};" + }; + const externalProjectName = "externalproject"; + const host = createServerHost([file1, file2]); + const projectService = createProjectService(host); + projectService.openExternalProject({ + rootFiles: toExternalFiles([file1.path, file2.path]), + options: {}, + projectFileName: externalProjectName + }); + + checkNumberOfExternalProjects(projectService, 1); + const externalProject = projectService.externalProjects[0]; + assert.isNotTrue(externalProject.getCompilerOptions().skipLibCheck); + }); + }); } \ No newline at end of file diff --git a/src/server/project.ts b/src/server/project.ts index b59efb03cfd54..d65b4c7914acc 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -20,16 +20,45 @@ namespace ts.server { } } - function isJsOrDtsFile(info: ScriptInfo) { - return info.scriptKind === ScriptKind.JS || info.scriptKind == ScriptKind.JSX || fileExtensionIs(info.fileName, ".d.ts"); + function countEachFileTypes(infos: ScriptInfo[]): { js: number, jsx: number, ts: number, tsx: number, dts: number } { + const result = { js: 0, jsx: 0, ts: 0, tsx: 0, dts: 0 }; + for (const info of infos) { + switch (info.scriptKind) { + case ScriptKind.JS: + result.js += 1; + break; + case ScriptKind.JSX: + result.jsx += 1; + break; + case ScriptKind.TS: + if (fileExtensionIs(info.fileName, ".d.ts")) { + result.dts += 1; + } + else { + result.ts += 1; + } + break; + case ScriptKind.TSX: + result.tsx += 1; + break; + } + } + return result; + } + + function hasOneOrMoreJsAndNoTsFiles(project: Project) { + const counts = countEachFileTypes(project.getScriptInfos()); + return counts.js > 0 && counts.ts === 0 && counts.tsx === 0; } export function allRootFilesAreJsOrDts(project: Project): boolean { - return project.getRootScriptInfos().every(isJsOrDtsFile); + const counts = countEachFileTypes(project.getRootScriptInfos()); + return counts.ts === 0 && counts.tsx === 0; } export function allFilesAreJsOrDts(project: Project): boolean { - return project.getScriptInfos().every(isJsOrDtsFile); + const counts = countEachFileTypes(project.getScriptInfos()); + return counts.ts === 0 && counts.tsx === 0; } export interface ProjectFilesWithTSDiagnostics extends protocol.ProjectFiles { @@ -71,11 +100,16 @@ namespace ts.server { public typesVersion = 0; - public isJsOnlyProject() { + public isNonTsProject() { this.updateGraph(); return allFilesAreJsOrDts(this); } + public isJsOnlyProject() { + this.updateGraph(); + return hasOneOrMoreJsAndNoTsFiles(this); + } + constructor( readonly projectKind: ProjectKind, readonly projectService: ProjectService, @@ -322,6 +356,7 @@ namespace ts.server { } if (hasChanges) { this.projectStructureVersion++; + this.updateCompilerOptions(); } return !hasChanges; } @@ -382,6 +417,17 @@ namespace ts.server { return this.getScriptInfoForNormalizedPath(toNormalizedPath(uncheckedFileName)); } + updateCompilerOptions() { + if (this.projectKind === ProjectKind.External || this.projectKind === ProjectKind.Inferred) { + if (this.isJsOnlyProject()) { + this.compilerOptions.skipLibCheck = true; + } + else { + this.compilerOptions.skipLibCheck = false; + } + } + } + filesToString() { if (!this.program) { return ""; diff --git a/src/server/session.ts b/src/server/session.ts index d076d5deb6d97..37a8ae061a853 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1127,7 +1127,7 @@ namespace ts.server { return combineProjectOutput( projects, project => { - const navItems = project.getLanguageService().getNavigateToItems(args.searchValue, args.maxResultCount, fileName, /*excludeDts*/ project.isJsOnlyProject()); + const navItems = project.getLanguageService().getNavigateToItems(args.searchValue, args.maxResultCount, fileName, /*excludeDts*/ project.isNonTsProject()); if (!navItems) { return []; } @@ -1165,7 +1165,7 @@ namespace ts.server { else { return combineProjectOutput( projects, - project => project.getLanguageService().getNavigateToItems(args.searchValue, args.maxResultCount, fileName, /*excludeDts*/ project.isJsOnlyProject()), + project => project.getLanguageService().getNavigateToItems(args.searchValue, args.maxResultCount, fileName, /*excludeDts*/ project.isNonTsProject()), /*comparer*/ undefined, navigateToItemIsEqualTo); } From 26ccba88967904e51378c6b9a6b0c796443405b6 Mon Sep 17 00:00:00 2001 From: zhengbli Date: Wed, 5 Oct 2016 15:58:46 -0700 Subject: [PATCH 2/3] avoid changing compiler options --- .../unittests/tsserverProjectSystem.ts | 1 + src/server/project.ts | 12 --------- src/server/session.ts | 25 +++++++++++++++---- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 23fbf69af37ff..14d8ecaacfc19 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -2084,6 +2084,7 @@ namespace ts.projectSystem { }; const host = createServerHost([file1, file2]); const projectService = createProjectService(host); + const session = createSession() projectService.openClientFile(file2.path); checkNumberOfInferredProjects(projectService, 1); diff --git a/src/server/project.ts b/src/server/project.ts index d65b4c7914acc..bee3e4f644338 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -356,7 +356,6 @@ namespace ts.server { } if (hasChanges) { this.projectStructureVersion++; - this.updateCompilerOptions(); } return !hasChanges; } @@ -417,17 +416,6 @@ namespace ts.server { return this.getScriptInfoForNormalizedPath(toNormalizedPath(uncheckedFileName)); } - updateCompilerOptions() { - if (this.projectKind === ProjectKind.External || this.projectKind === ProjectKind.Inferred) { - if (this.isJsOnlyProject()) { - this.compilerOptions.skipLibCheck = true; - } - else { - this.compilerOptions.skipLibCheck = false; - } - } - } - filesToString() { if (!this.program) { return ""; diff --git a/src/server/session.ts b/src/server/session.ts index 37a8ae061a853..693e1e5855396 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -14,6 +14,17 @@ namespace ts.server { return ((1e9 * seconds) + nanoseconds) / 1000000.0; } + function shouldSkipSematicCheck(project: Project) { + if (project.getCompilerOptions().skipLibCheck !== undefined) { + return false; + } + + if ((project.projectKind === ProjectKind.Inferred || project.projectKind === ProjectKind.External) && project.isJsOnlyProject()) { + return true; + } + return false; + } + interface FileStart { file: string; start: ILineInfo; @@ -252,12 +263,13 @@ namespace ts.server { private semanticCheck(file: NormalizedPath, project: Project) { try { - const diags = project.getLanguageService().getSemanticDiagnostics(file); - - if (diags) { - const bakedDiags = diags.map((diag) => formatDiag(file, project, diag)); - this.event({ file: file, diagnostics: bakedDiags }, "semanticDiag"); + let diags: Diagnostic[] = []; + if (!shouldSkipSematicCheck(project)) { + diags = project.getLanguageService().getSemanticDiagnostics(file); } + + const bakedDiags = diags.map((diag) => formatDiag(file, project, diag)); + this.event({ file: file, diagnostics: bakedDiags }, "semanticDiag"); } catch (err) { this.logError(err, "semantic check"); @@ -370,6 +382,9 @@ namespace ts.server { private getDiagnosticsWorker(args: protocol.FileRequestArgs, selector: (project: Project, file: string) => Diagnostic[], includeLinePosition: boolean) { const { project, file } = this.getFileAndProject(args); + if (shouldSkipSematicCheck(project)) { + return []; + } const scriptInfo = project.getScriptInfoForNormalizedPath(file); const diagnostics = selector(project, file); return includeLinePosition From 524dec97aaa5771446af64d07a3d42c651eb18fd Mon Sep 17 00:00:00 2001 From: zhengbli Date: Mon, 10 Oct 2016 18:20:20 -0700 Subject: [PATCH 3/3] Update tests --- .../unittests/tsserverProjectSystem.ts | 117 ++++++++---------- src/server/project.ts | 9 +- 2 files changed, 58 insertions(+), 68 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index a7df8cf5f54a9..ceedaba50ec0d 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -3,6 +3,8 @@ namespace ts.projectSystem { import TI = server.typingsInstaller; + import protocol = server.protocol; + import CommandNames = server.CommandNames; const safeList = { path: "/safeList.json", @@ -128,7 +130,7 @@ namespace ts.projectSystem { return combinePaths(getDirectoryPath(libFile.path), "tsc.js"); } - export function toExternalFile(fileName: string): server.protocol.ExternalFile { + export function toExternalFile(fileName: string): protocol.ExternalFile { return { fileName }; } @@ -527,7 +529,7 @@ namespace ts.projectSystem { } export function makeSessionRequest(command: string, args: T) { - const newRequest: server.protocol.Request = { + const newRequest: protocol.Request = { seq: 0, type: "request", command, @@ -538,7 +540,7 @@ namespace ts.projectSystem { export function openFilesForSession(files: FileOrFolder[], session: server.Session) { for (const file of files) { - const request = makeSessionRequest(server.CommandNames.Open, { file: file.path }); + const request = makeSessionRequest(CommandNames.Open, { file: file.path }); session.executeCommand(request); } } @@ -1751,7 +1753,7 @@ namespace ts.projectSystem { }); describe("navigate-to for javascript project", () => { - function containsNavToItem(items: server.protocol.NavtoItem[], itemName: string, itemKind: string) { + function containsNavToItem(items: protocol.NavtoItem[], itemName: string, itemKind: string) { return find(items, item => item.name === itemName && item.kind === itemKind) !== undefined; } @@ -1769,12 +1771,12 @@ namespace ts.projectSystem { openFilesForSession([file1], session); // Try to find some interface type defined in lib.d.ts - const libTypeNavToRequest = makeSessionRequest(server.CommandNames.Navto, { searchValue: "Document", file: file1.path, projectFileName: configFile.path }); - const items: server.protocol.NavtoItem[] = session.executeCommand(libTypeNavToRequest).response; + const libTypeNavToRequest = makeSessionRequest(CommandNames.Navto, { searchValue: "Document", file: file1.path, projectFileName: configFile.path }); + const items: protocol.NavtoItem[] = session.executeCommand(libTypeNavToRequest).response; assert.isFalse(containsNavToItem(items, "Document", "interface"), `Found lib.d.ts symbol in JavaScript project nav to request result.`); - const localFunctionNavToRequst = makeSessionRequest(server.CommandNames.Navto, { searchValue: "foo", file: file1.path, projectFileName: configFile.path }); - const items2: server.protocol.NavtoItem[] = session.executeCommand(localFunctionNavToRequst).response; + const localFunctionNavToRequst = makeSessionRequest(CommandNames.Navto, { searchValue: "foo", file: file1.path, projectFileName: configFile.path }); + const items2: protocol.NavtoItem[] = session.executeCommand(localFunctionNavToRequst).response; assert.isTrue(containsNavToItem(items2, "foo", "function"), `Cannot find function symbol "foo".`); }); }); @@ -2097,7 +2099,7 @@ namespace ts.projectSystem { const projectFileName = "externalProject"; const host = createServerHost([f]); const projectService = createProjectService(host); - // create a project + // create a project projectService.openExternalProject({ projectFileName, rootFiles: [toExternalFile(f.path)], options: {} }); projectService.checkNumberOfProjects({ externalProjects: 1 }); @@ -2198,69 +2200,60 @@ namespace ts.projectSystem { };` }; const host = createServerHost([file1, file2]); - const projectService = createProjectService(host); - const session = createSession() + const session = createSession(host); + openFilesForSession([file1, file2], session); - projectService.openClientFile(file2.path); - checkNumberOfInferredProjects(projectService, 1); - let inferredProject = projectService.inferredProjects[0]; - assert.isNotTrue(inferredProject.getCompilerOptions().skipLibCheck); + const file2GetErrRequest = makeSessionRequest( + CommandNames.SemanticDiagnosticsSync, + { file: file2.path } + ); + let errorResult = session.executeCommand(file2GetErrRequest).response; + assert.isTrue(errorResult.length === 0); - projectService.openClientFile(file1.path); - checkNumberOfInferredProjects(projectService, 1); - inferredProject = projectService.inferredProjects[0]; - assert.isTrue(inferredProject.getCompilerOptions().skipLibCheck); + const closeFileRequest = makeSessionRequest(CommandNames.Close, { file: file1.path }); + session.executeCommand(closeFileRequest); + errorResult = session.executeCommand(file2GetErrRequest).response; + assert.isTrue(errorResult.length !== 0); - projectService.closeClientFile(file1.path); - checkNumberOfInferredProjects(projectService, 1); - inferredProject = projectService.inferredProjects[0]; - assert.isNotTrue(inferredProject.getCompilerOptions().skipLibCheck); + openFilesForSession([file1], session); + errorResult = session.executeCommand(file2GetErrRequest).response; + assert.isTrue(errorResult.length === 0); }); it("should be turned on for js-only external projects", () => { - const file1 = { - path: "/a/b/f1.js", - content: "let x =1;" - }; - const file2 = { - path: "/a/b/f2.d.ts", - content: "interface T {};" - }; - const externalProjectName = "externalproject"; - const host = createServerHost([file1, file2]); - const projectService = createProjectService(host); - projectService.openExternalProject({ - rootFiles: toExternalFiles([file1.path, file2.path]), - options: {}, - projectFileName: externalProjectName - }); - - checkNumberOfExternalProjects(projectService, 1); - const externalProject = projectService.externalProjects[0]; - assert.isTrue(externalProject.getCompilerOptions().skipLibCheck); - }); - - it("should not be turned on for ts external projects", () => { - const file1 = { - path: "/a/b/f1.ts", + const jsFile = { + path: "/a/b/file1.js", content: "let x =1;" }; - const file2 = { - path: "/a/b/f2.d.ts", - content: "interface T {};" + const dTsFile = { + path: "/a/b/file2.d.ts", + content: ` + interface T { + name: string; + }; + interface T { + name: number; + };` }; - const externalProjectName = "externalproject"; - const host = createServerHost([file1, file2]); - const projectService = createProjectService(host); - projectService.openExternalProject({ - rootFiles: toExternalFiles([file1.path, file2.path]), - options: {}, - projectFileName: externalProjectName - }); + const host = createServerHost([jsFile, dTsFile]); + const session = createSession(host); - checkNumberOfExternalProjects(projectService, 1); - const externalProject = projectService.externalProjects[0]; - assert.isNotTrue(externalProject.getCompilerOptions().skipLibCheck); + const openExternalProjectRequest = makeSessionRequest( + CommandNames.OpenExternalProject, + { + projectFileName: "project1", + rootFiles: toExternalFiles([jsFile.path, dTsFile.path]), + options: {} + } + ); + session.executeCommand(openExternalProjectRequest); + + const dTsFileGetErrRequest = makeSessionRequest( + CommandNames.SemanticDiagnosticsSync, + { file: dTsFile.path } + ); + const errorResult = session.executeCommand(dTsFileGetErrRequest).response; + assert.isTrue(errorResult.length === 0); }); }); } \ No newline at end of file diff --git a/src/server/project.ts b/src/server/project.ts index bee3e4f644338..a355d75f94236 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -31,12 +31,9 @@ namespace ts.server { result.jsx += 1; break; case ScriptKind.TS: - if (fileExtensionIs(info.fileName, ".d.ts")) { - result.dts += 1; - } - else { - result.ts += 1; - } + fileExtensionIs(info.fileName, ".d.ts") + ? result.dts += 1 + : result.ts += 1; break; case ScriptKind.TSX: result.tsx += 1;