From 84386f9d3b4ac8309ab5f3ce936613f14cad9106 Mon Sep 17 00:00:00 2001 From: zhengbli Date: Sat, 20 Aug 2016 20:46:56 -0700 Subject: [PATCH 1/3] Tolerate certain errors in tsconfig.json --- src/server/editorServices.ts | 50 +++++++++++++++++++++--------------- src/server/session.ts | 20 ++++++++++----- 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index db9bdd5043bf6..0d84fa54a4a01 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -603,8 +603,11 @@ namespace ts.server { return projects.length > 1 ? deduplicate(result, areEqual) : result; } + export type ProjectServiceEvent = + { eventName: "context", data: { project: Project, fileName: string } } | { eventName: "configFileDiag", data: { triggerFile?: string, configFileName: string, diagnostics: Diagnostic[] } } + export interface ProjectServiceEventHandler { - (eventName: string, project: Project, fileName: string): void; + (event: ProjectServiceEvent): void; } export interface HostConfiguration { @@ -748,10 +751,13 @@ namespace ts.server { return ts.normalizePath(name); } - watchedProjectConfigFileChanged(project: Project) { + watchedProjectConfigFileChanged(project: Project): void { this.log("Config file changed: " + project.projectFilename); - this.updateConfiguredProject(project); + const configFileErrors = this.updateConfiguredProject(project); this.updateProjectStructure(); + if (configFileErrors && configFileErrors.length > 0) { + this.eventHandler({ eventName: "configFileDiag", data: { triggerFile: project.projectFilename, configFileName: project.projectFilename, diagnostics: configFileErrors } }); + } } log(msg: string, type = "Err") { @@ -828,13 +834,13 @@ namespace ts.server { for (let j = 0, flen = this.openFileRoots.length; j < flen; j++) { const openFile = this.openFileRoots[j]; if (this.eventHandler) { - this.eventHandler("context", openFile.defaultProject, openFile.fileName); + this.eventHandler({ eventName: "context", data: { project: openFile.defaultProject, fileName: openFile.fileName } }); } } for (let j = 0, flen = this.openFilesReferenced.length; j < flen; j++) { const openFile = this.openFilesReferenced[j]; if (this.eventHandler) { - this.eventHandler("context", openFile.defaultProject, openFile.fileName); + this.eventHandler({ eventName: "context", data: { project: openFile.defaultProject, fileName: openFile.fileName } }); } } } @@ -1234,11 +1240,12 @@ namespace ts.server { else { this.updateConfiguredProject(project); } + return { configFileName }; } else { this.log("No config files found."); } - return configFileName ? { configFileName } : {}; + return {}; } /** @@ -1328,7 +1335,7 @@ namespace ts.server { return undefined; } - configFileToProjectOptions(configFilename: string): { succeeded: boolean, projectOptions?: ProjectOptions, errors?: Diagnostic[] } { + configFileToProjectOptions(configFilename: string): { succeeded: boolean, projectOptions?: ProjectOptions, errors: Diagnostic[] } { configFilename = ts.normalizePath(configFilename); // file references will be relative to dirPath (or absolute) const dirPath = ts.getDirectoryPath(configFilename); @@ -1341,20 +1348,19 @@ namespace ts.server { const parsedCommandLine = ts.parseJsonConfigFileContent(rawConfig.config, this.host, dirPath, /*existingOptions*/ {}, configFilename); Debug.assert(!!parsedCommandLine.fileNames); - if (parsedCommandLine.errors && (parsedCommandLine.errors.length > 0)) { - return { succeeded: false, errors: parsedCommandLine.errors }; - } - else if (parsedCommandLine.fileNames.length === 0) { + if (parsedCommandLine.fileNames.length === 0) { const error = createCompilerDiagnostic(Diagnostics.The_config_file_0_found_doesn_t_contain_any_source_files, configFilename); - return { succeeded: false, errors: [error] }; + return { succeeded: false, errors: concatenate(parsedCommandLine.errors, [error]) }; } else { + // if the project has some files, we can continue with the parsed options and tolerate + // errors in the parsedCommandLine const projectOptions: ProjectOptions = { files: parsedCommandLine.fileNames, wildcardDirectories: parsedCommandLine.wildcardDirectories, compilerOptions: parsedCommandLine.options, }; - return { succeeded: true, projectOptions }; + return { succeeded: true, projectOptions, errors: parsedCommandLine.errors }; } } } @@ -1377,10 +1383,11 @@ namespace ts.server { return false; } - openConfigFile(configFilename: string, clientFileName?: string): { success: boolean, project?: Project, errors?: Diagnostic[] } { - const { succeeded, projectOptions, errors } = this.configFileToProjectOptions(configFilename); + openConfigFile(configFilename: string, clientFileName?: string): { success: boolean, project?: Project, errors: Diagnostic[] } { + const { succeeded, projectOptions, errors: errorsFromConfigFile } = this.configFileToProjectOptions(configFilename); + // Note: even if "succeeded"" is true, "errors" may still exist, as they are just tolerated if (!succeeded) { - return { success: false, errors }; + return { success: false, errors: errorsFromConfigFile }; } else { if (!projectOptions.compilerOptions.disableSizeLimit && projectOptions.compilerOptions.allowJs) { @@ -1392,7 +1399,7 @@ namespace ts.server { project.projectFileWatcher = this.host.watchFile( toPath(configFilename, configFilename, createGetCanonicalFileName(sys.useCaseSensitiveFileNames)), _ => this.watchedProjectConfigFileChanged(project)); - return { success: true, project }; + return { success: true, project, errors: errorsFromConfigFile }; } } @@ -1433,7 +1440,7 @@ namespace ts.server { return watchers; }, >{}); - return { success: true, project: project, errors }; + return { success: true, project: project, errors: concatenate(errors, errorsFromConfigFile) }; } } @@ -1451,7 +1458,7 @@ namespace ts.server { if (projectOptions.compilerOptions && !projectOptions.compilerOptions.disableSizeLimit && this.exceedTotalNonTsFileSizeLimit(projectOptions.files)) { project.setProjectOptions(projectOptions); if (project.languageServiceDiabled) { - return; + return errors; } project.disableLanguageService(); @@ -1459,7 +1466,7 @@ namespace ts.server { project.directoryWatcher.close(); project.directoryWatcher = undefined; } - return; + return errors; } if (project.languageServiceDiabled) { @@ -1478,7 +1485,7 @@ namespace ts.server { } } project.finishGraph(); - return; + return errors; } // if the project is too large, the root files might not have been all loaded if the total @@ -1524,6 +1531,7 @@ namespace ts.server { project.setProjectOptions(projectOptions); project.finishGraph(); } + return errors; } } diff --git a/src/server/session.ts b/src/server/session.ts index 9383a54bd48f7..00468330ffbcb 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -153,16 +153,22 @@ namespace ts.server { private logger: Logger ) { this.projectService = - new ProjectService(host, logger, (eventName, project, fileName) => { - this.handleEvent(eventName, project, fileName); + new ProjectService(host, logger, event => { + this.handleEvent(event); }); } - private handleEvent(eventName: string, project: Project, fileName: string) { - if (eventName == "context") { - this.projectService.log("got context event, updating diagnostics for" + fileName, "Info"); - this.updateErrorCheck([{ fileName, project }], this.changeSeq, - (n) => n === this.changeSeq, 100); + private handleEvent(event: ProjectServiceEvent) { + switch (event.eventName) { + case "context": + const { project, fileName } = event.data; + this.projectService.log("got context event, updating diagnostics for" + fileName, "Info"); + this.updateErrorCheck([{ fileName, project }], this.changeSeq, + (n) => n === this.changeSeq, 100); + break; + case "configFileDiag": + const { triggerFile, configFileName, diagnostics } = event.data; + this.configFileDiagnosticEvent(triggerFile, configFileName, diagnostics); } } From b2074172e1ef7b6f389d398e737268c16dc2cd93 Mon Sep 17 00:00:00 2001 From: zhengbli Date: Sat, 20 Aug 2016 20:47:30 -0700 Subject: [PATCH 2/3] Add test for configFile error tolerance --- src/harness/unittests/tsserverProjectSystem.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 49f033b9defcb..d4ed84191b41f 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -623,5 +623,23 @@ namespace ts { checkNumberOfConfiguredProjects(projectService, 1); checkNumberOfInferredProjects(projectService, 0); }); + + it("should tolerate config file errors and still try to build a project", () => { + const configFile: FileOrFolder = { + path: "/a/b/tsconfig.json", + content: `{ + "compilerOptions": { + "target": "es6", + "allowAnything": true + }, + "someOtherProperty": {} + }` + }; + const host = new TestServerHost(/*useCaseSensitiveFileNames*/ false, getExecutingFilePathFromLibFile(libFile), "/", [commonFile1, commonFile2, libFile, configFile]); + const projectService = new server.ProjectService(host, nullLogger); + projectService.openClientFile(commonFile1.path); + checkNumberOfConfiguredProjects(projectService, 1); + checkConfiguredProjectRootFiles(projectService.configuredProjects[0], [commonFile1.path, commonFile2.path]); + }); }); } \ No newline at end of file From a8ab52fd573dd28e1ee8a19f6271e6e98b35dd64 Mon Sep 17 00:00:00 2001 From: zhengbli Date: Sat, 20 Aug 2016 20:47:35 -0700 Subject: [PATCH 3/3] Use TS parser to tolerate more errors in tsconfig.json --- src/harness/unittests/tsconfigParsing.ts | 20 +++ src/server/editorServices.ts | 165 ++++++++++++----------- src/services/utilities.ts | 20 +++ 3 files changed, 128 insertions(+), 77 deletions(-) diff --git a/src/harness/unittests/tsconfigParsing.ts b/src/harness/unittests/tsconfigParsing.ts index 557379dff3b7d..2c9bcdb84233e 100644 --- a/src/harness/unittests/tsconfigParsing.ts +++ b/src/harness/unittests/tsconfigParsing.ts @@ -181,5 +181,25 @@ namespace ts { ["/d.ts", "/folder/e.ts"] ); }); + + it("parse and re-emit tsconfig.json file with diagnostics", () => { + const content = `{ + "compilerOptions": { + "allowJs": true + "outDir": "bin" + } + "files": ["file1.ts"] + }`; + const { configJsonObject, diagnostics } = parseAndReEmitConfigJSONFile(content); + const expectedResult = { + compilerOptions: { + allowJs: true, + outDir: "bin" + }, + files: ["file1.ts"] + }; + assert.isTrue(diagnostics.length === 2); + assert.equal(JSON.stringify(configJsonObject), JSON.stringify(expectedResult)); + }); }); } diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 0d84fa54a4a01..255ad8547ea48 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -702,7 +702,8 @@ namespace ts.server { } handleProjectFileListChanges(project: Project) { - const { projectOptions } = this.configFileToProjectOptions(project.projectFilename); + const { projectOptions, errors } = this.configFileToProjectOptions(project.projectFilename); + this.reportConfigFileDiagnostics(project.projectFilename, errors); const newRootFiles = projectOptions.files.map((f => this.getCanonicalFileName(f))); const currentRootFiles = project.getRootFiles().map((f => this.getCanonicalFileName(f))); @@ -720,18 +721,32 @@ namespace ts.server { } } + reportConfigFileDiagnostics(configFileName: string, diagnostics: Diagnostic[], triggerFile?: string) { + if (diagnostics && diagnostics.length > 0) { + this.eventHandler({ + eventName: "configFileDiag", + data: { configFileName, diagnostics, triggerFile } + }); + } + } + /** * This is the callback function when a watched directory has an added tsconfig file. */ directoryWatchedForTsconfigChanged(fileName: string) { - if (ts.getBaseFileName(fileName) != "tsconfig.json") { + if (ts.getBaseFileName(fileName) !== "tsconfig.json") { this.log(fileName + " is not tsconfig.json"); return; } this.log("Detected newly added tsconfig file: " + fileName); - const { projectOptions } = this.configFileToProjectOptions(fileName); + const { projectOptions, errors } = this.configFileToProjectOptions(fileName); + this.reportConfigFileDiagnostics(fileName, errors); + + if (!projectOptions) { + return; + } const rootFilesInTsconfig = projectOptions.files.map(f => this.getCanonicalFileName(f)); const openFileRoots = this.openFileRoots.map(s => this.getCanonicalFileName(s.fileName)); @@ -1224,7 +1239,7 @@ namespace ts.server { const project = this.findConfiguredProjectByConfigFile(configFileName); if (!project) { const configResult = this.openConfigFile(configFileName, fileName); - if (!configResult.success) { + if (!configResult.project) { return { configFileName, configFileErrors: configResult.errors }; } else { @@ -1335,33 +1350,31 @@ namespace ts.server { return undefined; } - configFileToProjectOptions(configFilename: string): { succeeded: boolean, projectOptions?: ProjectOptions, errors: Diagnostic[] } { + configFileToProjectOptions(configFilename: string): { projectOptions?: ProjectOptions, errors: Diagnostic[] } { configFilename = ts.normalizePath(configFilename); + let errors: Diagnostic[] = []; // file references will be relative to dirPath (or absolute) const dirPath = ts.getDirectoryPath(configFilename); const contents = this.host.readFile(configFilename); - const rawConfig: { config?: ProjectOptions; error?: Diagnostic; } = ts.parseConfigFileTextToJson(configFilename, contents); - if (rawConfig.error) { - return { succeeded: false, errors: [rawConfig.error] }; + const { configJsonObject, diagnostics } = ts.parseAndReEmitConfigJSONFile(contents); + errors = concatenate(errors, diagnostics); + const parsedCommandLine = ts.parseJsonConfigFileContent(configJsonObject, this.host, dirPath, /*existingOptions*/ {}, configFilename); + errors = concatenate(errors, parsedCommandLine.errors); + Debug.assert(!!parsedCommandLine.fileNames); + + if (parsedCommandLine.fileNames.length === 0) { + errors.push(createCompilerDiagnostic(Diagnostics.The_config_file_0_found_doesn_t_contain_any_source_files, configFilename)); + return { errors }; } else { - const parsedCommandLine = ts.parseJsonConfigFileContent(rawConfig.config, this.host, dirPath, /*existingOptions*/ {}, configFilename); - Debug.assert(!!parsedCommandLine.fileNames); - - if (parsedCommandLine.fileNames.length === 0) { - const error = createCompilerDiagnostic(Diagnostics.The_config_file_0_found_doesn_t_contain_any_source_files, configFilename); - return { succeeded: false, errors: concatenate(parsedCommandLine.errors, [error]) }; - } - else { - // if the project has some files, we can continue with the parsed options and tolerate - // errors in the parsedCommandLine - const projectOptions: ProjectOptions = { - files: parsedCommandLine.fileNames, - wildcardDirectories: parsedCommandLine.wildcardDirectories, - compilerOptions: parsedCommandLine.options, - }; - return { succeeded: true, projectOptions, errors: parsedCommandLine.errors }; - } + // if the project has some files, we can continue with the parsed options and tolerate + // errors in the parsedCommandLine + const projectOptions: ProjectOptions = { + files: parsedCommandLine.fileNames, + wildcardDirectories: parsedCommandLine.wildcardDirectories, + compilerOptions: parsedCommandLine.options, + }; + return { projectOptions, errors }; } } @@ -1383,65 +1396,63 @@ namespace ts.server { return false; } - openConfigFile(configFilename: string, clientFileName?: string): { success: boolean, project?: Project, errors: Diagnostic[] } { - const { succeeded, projectOptions, errors: errorsFromConfigFile } = this.configFileToProjectOptions(configFilename); - // Note: even if "succeeded"" is true, "errors" may still exist, as they are just tolerated - if (!succeeded) { - return { success: false, errors: errorsFromConfigFile }; + openConfigFile(configFilename: string, clientFileName?: string): { project?: Project, errors: Diagnostic[] } { + const parseConfigFileResult = this.configFileToProjectOptions(configFilename); + let errors = parseConfigFileResult.errors; + if (!parseConfigFileResult.projectOptions) { + return { errors }; } - else { - if (!projectOptions.compilerOptions.disableSizeLimit && projectOptions.compilerOptions.allowJs) { - if (this.exceedTotalNonTsFileSizeLimit(projectOptions.files)) { - const project = this.createProject(configFilename, projectOptions, /*languageServiceDisabled*/ true); - - // for configured projects with languageService disabled, we only watch its config file, - // do not care about the directory changes in the folder. - project.projectFileWatcher = this.host.watchFile( - toPath(configFilename, configFilename, createGetCanonicalFileName(sys.useCaseSensitiveFileNames)), - _ => this.watchedProjectConfigFileChanged(project)); - return { success: true, project, errors: errorsFromConfigFile }; - } + const projectOptions = parseConfigFileResult.projectOptions; + if (!projectOptions.compilerOptions.disableSizeLimit && projectOptions.compilerOptions.allowJs) { + if (this.exceedTotalNonTsFileSizeLimit(projectOptions.files)) { + const project = this.createProject(configFilename, projectOptions, /*languageServiceDisabled*/ true); + + // for configured projects with languageService disabled, we only watch its config file, + // do not care about the directory changes in the folder. + project.projectFileWatcher = this.host.watchFile( + toPath(configFilename, configFilename, createGetCanonicalFileName(sys.useCaseSensitiveFileNames)), + _ => this.watchedProjectConfigFileChanged(project)); + return { project, errors }; } + } - const project = this.createProject(configFilename, projectOptions); - let errors: Diagnostic[]; - for (const rootFilename of projectOptions.files) { - if (this.host.fileExists(rootFilename)) { - const info = this.openFile(rootFilename, /*openedByClient*/ clientFileName == rootFilename); - project.addRoot(info); - } - else { - (errors || (errors = [])).push(createCompilerDiagnostic(Diagnostics.File_0_not_found, rootFilename)); - } + const project = this.createProject(configFilename, projectOptions); + for (const rootFilename of projectOptions.files) { + if (this.host.fileExists(rootFilename)) { + const info = this.openFile(rootFilename, /*openedByClient*/ clientFileName == rootFilename); + project.addRoot(info); + } + else { + (errors || (errors = [])).push(createCompilerDiagnostic(Diagnostics.File_0_not_found, rootFilename)); } - project.finishGraph(); - project.projectFileWatcher = this.host.watchFile(configFilename, _ => this.watchedProjectConfigFileChanged(project)); + } + project.finishGraph(); + project.projectFileWatcher = this.host.watchFile(configFilename, _ => this.watchedProjectConfigFileChanged(project)); - const configDirectoryPath = ts.getDirectoryPath(configFilename); + const configDirectoryPath = ts.getDirectoryPath(configFilename); - this.log("Add recursive watcher for: " + configDirectoryPath); - project.directoryWatcher = this.host.watchDirectory( - configDirectoryPath, - path => this.directoryWatchedForSourceFilesChanged(project, path), - /*recursive*/ true - ); + this.log("Add recursive watcher for: " + configDirectoryPath); + project.directoryWatcher = this.host.watchDirectory( + configDirectoryPath, + path => this.directoryWatchedForSourceFilesChanged(project, path), + /*recursive*/ true + ); - project.directoriesWatchedForWildcards = reduceProperties(createMap(projectOptions.wildcardDirectories), (watchers, flag, directory) => { - if (comparePaths(configDirectoryPath, directory, ".", !this.host.useCaseSensitiveFileNames) !== Comparison.EqualTo) { - const recursive = (flag & WatchDirectoryFlags.Recursive) !== 0; - this.log(`Add ${ recursive ? "recursive " : ""}watcher for: ${directory}`); - watchers[directory] = this.host.watchDirectory( - directory, - path => this.directoryWatchedForSourceFilesChanged(project, path), - recursive - ); - } + project.directoriesWatchedForWildcards = reduceProperties(createMap(projectOptions.wildcardDirectories), (watchers, flag, directory) => { + if (comparePaths(configDirectoryPath, directory, ".", !this.host.useCaseSensitiveFileNames) !== Comparison.EqualTo) { + const recursive = (flag & WatchDirectoryFlags.Recursive) !== 0; + this.log(`Add ${ recursive ? "recursive " : ""}watcher for: ${directory}`); + watchers[directory] = this.host.watchDirectory( + directory, + path => this.directoryWatchedForSourceFilesChanged(project, path), + recursive + ); + } - return watchers; - }, >{}); + return watchers; + }, >{}); - return { success: true, project: project, errors: concatenate(errors, errorsFromConfigFile) }; - } + return { project: project, errors }; } updateConfiguredProject(project: Project): Diagnostic[] { @@ -1450,8 +1461,8 @@ namespace ts.server { this.removeProject(project); } else { - const { succeeded, projectOptions, errors } = this.configFileToProjectOptions(project.projectFilename); - if (!succeeded) { + const { projectOptions, errors } = this.configFileToProjectOptions(project.projectFilename); + if (!projectOptions) { return errors; } else { diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 7f41cdbb3a42e..858f889bc6c91 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -925,4 +925,24 @@ namespace ts { } return ensureScriptKind(fileName, scriptKind); } + + export function parseAndReEmitConfigJSONFile(content: string) { + const options: TranspileOptions = { + fileName: "config.js", + compilerOptions: { + target: ScriptTarget.ES6 + }, + reportDiagnostics: true + }; + const { outputText, diagnostics } = ts.transpileModule("(" + content + ")", options); + // Becasue the content was wrapped in "()", the start position of diagnostics needs to be subtract by 1 + // also, the emitted result will have "(" in the beginning and ");" in the end. We need to strip these + // as well + const trimmedOutput = outputText.trim(); + const configJsonObject = JSON.parse(trimmedOutput.substring(1, trimmedOutput.length - 2)); + for (const diagnostic of diagnostics) { + diagnostic.start = diagnostic.start - 1; + } + return { configJsonObject, diagnostics }; + } } \ No newline at end of file