From 98bd31017d70e7fa683ba2f94d3c9f98f1414807 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 29 Nov 2016 12:42:17 -0800 Subject: [PATCH] Update the protocol to return file name in compiler diagnostics and project errors --- .../unittests/tsserverProjectSystem.ts | 65 ++++++++++++++ src/server/protocol.ts | 15 +++- src/server/session.ts | 86 ++++++++++++++++--- 3 files changed, 150 insertions(+), 16 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 5ed0d981511..a6cf0ac6cd8 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -3102,4 +3102,69 @@ namespace ts.projectSystem { }); }); + describe("Options Diagnostic locations reported correctly with changes in configFile contents", () => { + it("when options change", () => { + const file = { + path: "/a/b/app.ts", + content: "let x = 10" + }; + const configFileContentBeforeComment = `{`; + const configFileContentComment = ` + // comment`; + const configFileContentAfterComment = ` + "compilerOptions": { + "allowJs": true, + "declaration": true + } + }`; + const configFileContentWithComment = configFileContentBeforeComment + configFileContentComment + configFileContentAfterComment; + const configFileContentWithoutCommentLine = configFileContentBeforeComment + configFileContentAfterComment; + + const configFile = { + path: "/a/b/tsconfig.json", + content: configFileContentWithComment + }; + const host = createServerHost([file, libFile, configFile]); + const session = createSession(host); + openFilesForSession([file], session); + + const projectService = session.getProjectService(); + checkNumberOfProjects(projectService, { configuredProjects: 1 }); + const projectName = projectService.configuredProjects[0].getProjectName(); + + const diags = session.executeCommand({ + type: "request", + command: server.CommandNames.CompilerOptionsDiagnosticsFull, + seq: 2, + arguments: { projectFileName: projectName } + }).response; + assert.isTrue(diags.length === 2); + + configFile.content = configFileContentWithoutCommentLine; + host.reloadFS([file, configFile]); + host.triggerFileWatcherCallback(configFile.path); + + const diagsAfterEdit = session.executeCommand({ + type: "request", + command: server.CommandNames.CompilerOptionsDiagnosticsFull, + seq: 2, + arguments: { projectFileName: projectName } + }).response; + assert.isTrue(diagsAfterEdit.length === 2); + + verifyDiagnostic(diags[0], diagsAfterEdit[0]); + verifyDiagnostic(diags[1], diagsAfterEdit[1]); + + function verifyDiagnostic(beforeEditDiag: server.protocol.DiagnosticWithLinePositionAndFileName, afterEditDiag: server.protocol.DiagnosticWithLinePositionAndFileName) { + assert.equal(beforeEditDiag.message, afterEditDiag.message); + assert.equal(beforeEditDiag.code, afterEditDiag.code); + assert.equal(beforeEditDiag.category, afterEditDiag.category); + assert.equal(beforeEditDiag.startLocation.line, afterEditDiag.startLocation.line + 1); + assert.equal(beforeEditDiag.startLocation.offset, afterEditDiag.startLocation.offset); + assert.equal(beforeEditDiag.endLocation.line, afterEditDiag.endLocation.line + 1); + assert.equal(beforeEditDiag.endLocation.offset, afterEditDiag.endLocation.offset); + assert.equal(beforeEditDiag.fileName, afterEditDiag.fileName); + } + }); + }); } \ No newline at end of file diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 680b81dff99..284125396b0 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -358,6 +358,10 @@ namespace ts.server.protocol { code: number; } + export interface DiagnosticWithLinePositionAndFileName extends DiagnosticWithLinePosition { + fileName: string; + } + /** * Response message for "projectInfo" request */ @@ -960,7 +964,7 @@ namespace ts.server.protocol { /** * List of errors in project */ - projectErrors: DiagnosticWithLinePosition[]; + projectErrors: DiagnosticWithLinePositionAndFileName[]; } /** @@ -1786,6 +1790,13 @@ namespace ts.server.protocol { code?: number; } + export interface DiagnosticWithFileName extends Diagnostic { + /** + * Name of the file the diagnostic is in + */ + fileName: string; + } + export interface DiagnosticEventBody { /** * The file for which diagnostic information is reported. @@ -1820,7 +1831,7 @@ namespace ts.server.protocol { /** * An arry of diagnostic information items for the found config file. */ - diagnostics: Diagnostic[]; + diagnostics: DiagnosticWithFileName[]; } /** diff --git a/src/server/session.ts b/src/server/session.ts index 1fa3986f66f..e34b03a9c8c 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -60,11 +60,17 @@ namespace ts.server { }; } - function formatConfigFileDiag(diag: ts.Diagnostic): protocol.Diagnostic { + function convertToILineInfo(lineAndCharacter: LineAndCharacter): ILineInfo { + return { line: lineAndCharacter.line + 1, offset: lineAndCharacter.character + 1 }; + } + + function formatConfigFileDiag(diag: ts.Diagnostic): protocol.DiagnosticWithFileName { return { - start: undefined, - end: undefined, - text: ts.flattenDiagnosticMessageText(diag.messageText, "\n") + start: diag.file && convertToILineInfo(getLineAndCharacterOfPosition(diag.file, diag.start)), + end: diag.file && convertToILineInfo(getLineAndCharacterOfPosition(diag.file, diag.start + diag.length)), + text: ts.flattenDiagnosticMessageText(diag.messageText, "\n"), + code: diag.code, + fileName: diag.file && diag.file.fileName }; } @@ -193,6 +199,31 @@ namespace ts.server { return `Content-Length: ${1 + len}\r\n\r\n${json}${newLine}`; } + /** + * Should Remap project Files with ts diagnostics if + * - there are project errors + * - options contain configFile - so we can remove it from options before serializing + * @param p project files with ts diagnostics + */ + function shouldRemapProjectFilesWithTSDiagnostics(p: ProjectFilesWithTSDiagnostics) { + return (p.projectErrors && !!p.projectErrors.length) || + (p.info && !!p.info.options.configFile); + } + + /** + * Get the compiler options without configFile key + * @param options + */ + function getCompilerOptionsWithoutConfigFile(options: CompilerOptions) { + const result: CompilerOptions = {}; + for (const option in options) { + if (option !== "configFile") { + result[option] = options[option]; + } + } + return result; + } + export class Session implements EventSender { private readonly gcTimer: GcTimer; protected projectService: ProjectService; @@ -410,7 +441,20 @@ namespace ts.server { private getCompilerOptionsDiagnostics(args: protocol.CompilerOptionsDiagnosticsRequestArgs) { const project = this.getProject(args.projectFileName); - return this.convertToDiagnosticsWithLinePosition(project.getLanguageService().getCompilerOptionsDiagnostics(), /*scriptInfo*/ undefined); + return this.convertToCompilerOptionsDiagnosticsWithLinePosition(project.getLanguageService().getCompilerOptionsDiagnostics()); + } + + private convertToCompilerOptionsDiagnosticsWithLinePosition(diagnostics: Diagnostic[]) { + return diagnostics.map(d => { + message: flattenDiagnosticMessageText(d.messageText, this.host.newLine), + start: d.start, + length: d.length, + category: DiagnosticCategory[d.category].toLowerCase(), + code: d.code, + startLocation: d.file && convertToILineInfo(getLineAndCharacterOfPosition(d.file, d.start)), + endLocation: d.file && convertToILineInfo(getLineAndCharacterOfPosition(d.file, d.start + d.length)), + fileName: d.file && d.file.fileName + }); } private convertToDiagnosticsWithLinePosition(diagnostics: Diagnostic[], scriptInfo: ScriptInfo) { @@ -1408,19 +1452,33 @@ namespace ts.server { }, [CommandNames.SynchronizeProjectList]: (request: protocol.SynchronizeProjectListRequest) => { const result = this.projectService.synchronizeProjectList(request.arguments.knownProjects); - if (!result.some(p => p.projectErrors && p.projectErrors.length !== 0)) { + if (!result.some(shouldRemapProjectFilesWithTSDiagnostics)) { return this.requiredResponse(result); } const converted = map(result, p => { - if (!p.projectErrors || p.projectErrors.length === 0) { - return p; + if (shouldRemapProjectFilesWithTSDiagnostics(p)) { + const projectErrors = p.projectErrors && p.projectErrors.length ? + this.convertToCompilerOptionsDiagnosticsWithLinePosition(p.projectErrors) : + p.projectErrors; + + const info = p.info && !!p.info.options.configFile ? + { + projectName: p.info.projectName, + isInferred: p.info.isInferred, + version: p.info.version, + options: getCompilerOptionsWithoutConfigFile(p.info.options), + languageServiceDisabled: p.info.languageServiceDisabled + } : p.info; + + return { + info, + changes: p.changes, + files: p.files, + projectErrors + }; } - return { - info: p.info, - changes: p.changes, - files: p.files, - projectErrors: this.convertToDiagnosticsWithLinePosition(p.projectErrors, /*scriptInfo*/ undefined) - }; + + return p; }); return this.requiredResponse(converted); },