Allow rich response for compile on save (#37462)

sheetalkamat · web-flow · commit 7e07a2b5d129 · 2020-03-19T14:03:21.000-07:00
Fixes #30739
diff --git a/src/compiler/builderState.ts b/src/compiler/builderState.ts
@@ -3,8 +3,8 @@ namespace ts {
     export function getFileEmitOutput(program: Program, sourceFile: SourceFile, emitOnlyDtsFiles: boolean,
         cancellationToken?: CancellationToken, customTransformers?: CustomTransformers, forceDtsEmit?: boolean): EmitOutput {
         const outputFiles: OutputFile[] = [];
-        const emitResult = program.emit(sourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, customTransformers, forceDtsEmit);
-        return { outputFiles, emitSkipped: emitResult.emitSkipped, exportedModulesFromDeclarationEmit: emitResult.exportedModulesFromDeclarationEmit };
+        const { emitSkipped, diagnostics, exportedModulesFromDeclarationEmit } = program.emit(sourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, customTransformers, forceDtsEmit);
+        return { outputFiles, emitSkipped, diagnostics, exportedModulesFromDeclarationEmit };
 
         function writeFile(fileName: string, text: string, writeByteOrderMark: boolean) {
             outputFiles.push({ name: fileName, writeByteOrderMark, text });
diff --git a/src/compiler/builderStatePublic.ts b/src/compiler/builderStatePublic.ts
@@ -2,6 +2,7 @@ namespace ts {
     export interface EmitOutput {
         outputFiles: OutputFile[];
         emitSkipped: boolean;
+        /* @internal */ diagnostics: readonly Diagnostic[];
         /* @internal */ exportedModulesFromDeclarationEmit?: ExportedModulesFromDeclarationEmit;
     }
 
@@ -10,4 +11,4 @@ namespace ts {
         writeByteOrderMark: boolean;
         text: string;
     }
-}
+}
diff --git a/src/harness/client.ts b/src/harness/client.ts
@@ -358,7 +358,7 @@ namespace ts.server {
         getEmitOutput(file: string): EmitOutput {
             const request = this.processRequest<protocol.EmitOutputRequest>(protocol.CommandTypes.EmitOutput, { file });
             const response = this.processResponse<protocol.EmitOutputResponse>(request);
-            return response.body;
+            return response.body as EmitOutput;
         }
 
         getSyntacticDiagnostics(file: string): DiagnosticWithLocation[] {
diff --git a/src/server/project.ts b/src/server/project.ts
@@ -118,6 +118,12 @@ namespace ts.server {
         return (watch as GeneratedFileWatcher).generatedFilePath !== undefined;
     }
 
+    /*@internal*/
+    export interface EmitResult {
+        emitSkipped: boolean;
+        diagnostics: readonly Diagnostic[];
+    }
+
     export abstract class Project implements LanguageServiceHost, ModuleResolutionHost {
         private rootFiles: ScriptInfo[] = [];
         private rootFilesMap = createMap<ProjectRootFile>();
@@ -587,19 +593,19 @@ namespace ts.server {
         /**
          * Returns true if emit was conducted
          */
-        emitFile(scriptInfo: ScriptInfo, writeFile: (path: string, data: string, writeByteOrderMark?: boolean) => void): boolean {
+        emitFile(scriptInfo: ScriptInfo, writeFile: (path: string, data: string, writeByteOrderMark?: boolean) => void): EmitResult {
             if (!this.languageServiceEnabled || !this.shouldEmitFile(scriptInfo)) {
-                return false;
+                return { emitSkipped: true, diagnostics: emptyArray };
             }
-            const { emitSkipped, outputFiles } = this.getLanguageService(/*ensureSynchronized*/ false).getEmitOutput(scriptInfo.fileName);
+            const { emitSkipped, diagnostics, outputFiles } = this.getLanguageService().getEmitOutput(scriptInfo.fileName);
             if (!emitSkipped) {
                 for (const outputFile of outputFiles) {
                     const outputFileAbsoluteFileName = getNormalizedAbsolutePath(outputFile.name, this.currentDirectory);
                     writeFile(outputFileAbsoluteFileName, outputFile.text, outputFile.writeByteOrderMark);
                 }
             }
 
-            return !emitSkipped;
+            return { emitSkipped, diagnostics };
         }
 
         enableLanguageService() {
diff --git a/src/server/protocol.ts b/src/server/protocol.ts
@@ -857,10 +857,25 @@ namespace ts.server.protocol {
     }
 
     /** @internal */
-    export interface EmitOutputRequest extends FileRequest {}
+    export interface EmitOutputRequest extends FileRequest {
+        command: CommandTypes.EmitOutput;
+        arguments: EmitOutputRequestArgs;
+    }
+    /** @internal */
+    export interface EmitOutputRequestArgs extends FileRequestArgs {
+        includeLinePosition?: boolean;
+        /** if true - return response as object with emitSkipped and diagnostics */
+        richResponse?: boolean;
+    }
     /** @internal */
     export interface EmitOutputResponse extends Response {
-        readonly body: EmitOutput;
+        readonly body: EmitOutput | ts.EmitOutput;
+    }
+    /** @internal */
+    export interface EmitOutput {
+        outputFiles: OutputFile[];
+        emitSkipped: boolean;
+        diagnostics: Diagnostic[] | DiagnosticWithLinePosition[];
     }
 
     /**
@@ -1808,6 +1823,18 @@ namespace ts.server.protocol {
          * if true - then file should be recompiled even if it does not have any changes.
          */
         forced?: boolean;
+        includeLinePosition?: boolean;
+        /** if true - return response as object with emitSkipped and diagnostics */
+        richResponse?: boolean;
+    }
+
+    export interface CompileOnSaveEmitFileResponse extends Response {
+        body: boolean | EmitResult;
+    }
+
+    export interface EmitResult {
+        emitSkipped: boolean;
+        diagnostics: Diagnostic[] | DiagnosticWithLinePosition[];
     }
 
     /**
diff --git a/src/server/session.ts b/src/server/session.ts
@@ -85,9 +85,9 @@ namespace ts.server {
         return { line: lineAndCharacter.line + 1, offset: lineAndCharacter.character + 1 };
     }
 
-    function formatConfigFileDiag(diag: Diagnostic, includeFileName: true): protocol.DiagnosticWithFileName;
-    function formatConfigFileDiag(diag: Diagnostic, includeFileName: false): protocol.Diagnostic;
-    function formatConfigFileDiag(diag: Diagnostic, includeFileName: boolean): protocol.Diagnostic | protocol.DiagnosticWithFileName {
+    function formatDiagnosticToProtocol(diag: Diagnostic, includeFileName: true): protocol.DiagnosticWithFileName;
+    function formatDiagnosticToProtocol(diag: Diagnostic, includeFileName: false): protocol.Diagnostic;
+    function formatDiagnosticToProtocol(diag: Diagnostic, includeFileName: boolean): protocol.Diagnostic | protocol.DiagnosticWithFileName {
         const start = (diag.file && convertToLocation(getLineAndCharacterOfPosition(diag.file, diag.start!)))!; // TODO: GH#18217
         const end = (diag.file && convertToLocation(getLineAndCharacterOfPosition(diag.file, diag.start! + diag.length!)))!; // TODO: GH#18217
         const text = flattenDiagnosticMessageText(diag.messageText, "\n");
@@ -699,7 +699,7 @@ namespace ts.server {
                     break;
                 case ConfigFileDiagEvent:
                     const { triggerFile, configFileName: configFile, diagnostics } = event.data;
-                    const bakedDiags = map(diagnostics, diagnostic => formatConfigFileDiag(diagnostic, /*includeFileName*/ true));
+                    const bakedDiags = map(diagnostics, diagnostic => formatDiagnosticToProtocol(diagnostic, /*includeFileName*/ true));
                     this.event<protocol.ConfigFileDiagnosticEventBody>({
                         triggerFile,
                         configFile,
@@ -998,7 +998,7 @@ namespace ts.server {
                 this.convertToDiagnosticsWithLinePositionFromDiagnosticFile(diagnosticsForConfigFile) :
                 map(
                     diagnosticsForConfigFile,
-                    diagnostic => formatConfigFileDiag(diagnostic, /*includeFileName*/ false)
+                    diagnostic => formatDiagnosticToProtocol(diagnostic, /*includeFileName*/ false)
                 );
         }
 
@@ -1009,8 +1009,10 @@ namespace ts.server {
                 length: d.length!, // TODO: GH#18217
                 category: diagnosticCategoryName(d),
                 code: d.code,
+                source: d.source,
                 startLocation: (d.file && convertToLocation(getLineAndCharacterOfPosition(d.file, d.start!)))!, // TODO: GH#18217
                 endLocation: (d.file && convertToLocation(getLineAndCharacterOfPosition(d.file, d.start! + d.length!)))!, // TODO: GH#18217
+                reportsUnnecessary: d.reportsUnnecessary,
                 relatedInformation: map(d.relatedInformation, formatRelatedInformation)
             }));
         }
@@ -1108,11 +1110,20 @@ namespace ts.server {
             };
         }
 
-        private getEmitOutput(args: protocol.FileRequestArgs): EmitOutput {
+        private getEmitOutput(args: protocol.EmitOutputRequestArgs): EmitOutput | protocol.EmitOutput {
             const { file, project } = this.getFileAndProject(args);
-            return project.shouldEmitFile(project.getScriptInfo(file)) ?
-                project.getLanguageService().getEmitOutput(file) :
-                { emitSkipped: true, outputFiles: [] };
+            if (!project.shouldEmitFile(project.getScriptInfo(file))) {
+                return { emitSkipped: true, outputFiles: [], diagnostics: [] };
+            }
+            const result = project.getLanguageService().getEmitOutput(file);
+            return args.richResponse ?
+                {
+                    ...result,
+                    diagnostics: args.includeLinePosition ?
+                        this.convertToDiagnosticsWithLinePositionFromDiagnosticFile(result.diagnostics) :
+                        result.diagnostics.map(d => formatDiagnosticToProtocol(d, /*includeFileName*/ true))
+                } :
+                result;
         }
 
         private mapDefinitionInfo(definitions: readonly DefinitionInfo[], project: Project): readonly protocol.FileSpanWithContext[] {
@@ -1708,16 +1719,24 @@ namespace ts.server {
             );
         }
 
-        private emitFile(args: protocol.CompileOnSaveEmitFileRequestArgs) {
+        private emitFile(args: protocol.CompileOnSaveEmitFileRequestArgs): boolean | protocol.EmitResult | EmitResult {
             const { file, project } = this.getFileAndProject(args);
             if (!project) {
                 Errors.ThrowNoProject();
             }
             if (!project.languageServiceEnabled) {
-                return false;
+                return args.richResponse ? { emitSkipped: true, diagnostics: [] } : false;
             }
             const scriptInfo = project.getScriptInfo(file)!;
-            return project.emitFile(scriptInfo, (path, data, writeByteOrderMark) => this.host.writeFile(path, data, writeByteOrderMark));
+            const { emitSkipped, diagnostics } = project.emitFile(scriptInfo, (path, data, writeByteOrderMark) => this.host.writeFile(path, data, writeByteOrderMark));
+            return args.richResponse ?
+                {
+                    emitSkipped,
+                    diagnostics: args.includeLinePosition ?
+                        this.convertToDiagnosticsWithLinePositionFromDiagnosticFile(diagnostics) :
+                        diagnostics.map(d => formatDiagnosticToProtocol(d, /*includeFileName*/ true))
+                } :
+                !emitSkipped;
         }
 
         private getSignatureHelpItems(args: protocol.SignatureHelpRequestArgs, simplifiedResult: boolean): protocol.SignatureHelpItems | SignatureHelpItems | undefined {
diff --git a/src/services/shims.ts b/src/services/shims.ts
@@ -1051,7 +1051,10 @@ namespace ts {
         public getEmitOutput(fileName: string): string {
             return this.forwardJSONCall(
                 `getEmitOutput('${fileName}')`,
-                () => this.languageService.getEmitOutput(fileName)
+                () => {
+                    const { diagnostics, ...rest } = this.languageService.getEmitOutput(fileName);
+                    return { ...rest, diagnostics: this.realizeDiagnostics(diagnostics) };
+                }
             );
         }
 
diff --git a/src/testRunner/unittests/services/languageService.ts b/src/testRunner/unittests/services/languageService.ts
@@ -58,6 +58,7 @@ export function Component(x: Config): any;`
                 ),
                 {
                     emitSkipped: true,
+                    diagnostics: emptyArray,
                     outputFiles: emptyArray,
                     exportedModulesFromDeclarationEmit: undefined
                 }
@@ -71,6 +72,7 @@ export function Component(x: Config): any;`
                 ),
                 {
                     emitSkipped: false,
+                    diagnostics: emptyArray,
                     outputFiles: [{
                         name: "foo.d.ts",
                         text: "export {};\r\n",
diff --git a/src/testRunner/unittests/tsserver/compileOnSave.ts b/src/testRunner/unittests/tsserver/compileOnSave.ts
@@ -799,6 +799,87 @@ namespace ts.projectSystem {
                 assert.isTrue(stringContains(content, str), `Expected "${content}" to have "${str}"`);
             }
         });
+
+        describe("compile on save emit with and without richResponse", () => {
+            it("without rich Response", () => {
+                verify(/*richRepsonse*/ undefined);
+            });
+            it("with rich Response set to false", () => {
+                verify(/*richRepsonse*/ false);
+            });
+            it("with rich Repsonse", () => {
+                verify(/*richRepsonse*/ true);
+            });
+
+            function verify(richResponse: boolean | undefined) {
+                const config: File = {
+                    path: `${tscWatch.projectRoot}/tsconfig.json`,
+                    content: JSON.stringify({
+                        compileOnSave: true,
+                        compilerOptions: {
+                            outDir: "test",
+                            noEmitOnError: true,
+                            declaration: true,
+                        },
+                        exclude: ["node_modules"]
+                    })
+                };
+                const file1: File = {
+                    path: `${tscWatch.projectRoot}/file1.ts`,
+                    content: "const x = 1;"
+                };
+                const file2: File = {
+                    path: `${tscWatch.projectRoot}/file2.ts`,
+                    content: "const y = 2;"
+                };
+                const host = createServerHost([file1, file2, config, libFile]);
+                const session = createSession(host);
+                openFilesForSession([file1], session);
+
+                const affectedFileResponse = session.executeCommandSeq<protocol.CompileOnSaveAffectedFileListRequest>({
+                    command: protocol.CommandTypes.CompileOnSaveAffectedFileList,
+                    arguments: { file: file1.path }
+                }).response as protocol.CompileOnSaveAffectedFileListSingleProject[];
+                assert.deepEqual(affectedFileResponse, [
+                    { fileNames: [file1.path, file2.path], projectFileName: config.path, projectUsesOutFile: false }
+                ]);
+                const file1SaveResponse = session.executeCommandSeq<protocol.CompileOnSaveEmitFileRequest>({
+                    command: protocol.CommandTypes.CompileOnSaveEmitFile,
+                    arguments: { file: file1.path, richResponse }
+                }).response;
+                if (richResponse) {
+                    assert.deepEqual(file1SaveResponse, { emitSkipped: false, diagnostics: emptyArray });
+                }
+                else {
+                    assert.isTrue(file1SaveResponse);
+                }
+                assert.strictEqual(host.readFile(`${tscWatch.projectRoot}/test/file1.d.ts`), "declare const x = 1;\n");
+                const file2SaveResponse = session.executeCommandSeq<protocol.CompileOnSaveEmitFileRequest>({
+                    command: protocol.CommandTypes.CompileOnSaveEmitFile,
+                    arguments: { file: file2.path, richResponse }
+                }).response;
+                if (richResponse) {
+                    assert.deepEqual(file2SaveResponse, {
+                        emitSkipped: true,
+                        diagnostics: [{
+                            start: undefined,
+                            end: undefined,
+                            fileName: undefined,
+                            text: formatStringFromArgs(Diagnostics.Cannot_write_file_0_because_it_would_overwrite_input_file.message, [`${tscWatch.projectRoot}/test/file1.d.ts`]),
+                            code: Diagnostics.Cannot_write_file_0_because_it_would_overwrite_input_file.code,
+                            category: diagnosticCategoryName(Diagnostics.Cannot_write_file_0_because_it_would_overwrite_input_file),
+                            reportsUnnecessary: undefined,
+                            relatedInformation: undefined,
+                            source: undefined
+                        }]
+                    });
+                }
+                else {
+                    assert.isFalse(file2SaveResponse);
+                }
+                assert.isFalse(host.fileExists(`${tscWatch.projectRoot}/test/file2.d.ts`));
+            }
+        });
     });
 
     describe("unittests:: tsserver:: compileOnSave:: CompileOnSaveAffectedFileListRequest with and without projectFileName in request", () => {
diff --git a/src/testRunner/unittests/tsserver/projectReferenceCompileOnSave.ts b/src/testRunner/unittests/tsserver/projectReferenceCompileOnSave.ts
@@ -252,7 +252,8 @@ ${appendJs}`
                     text: content,
                     writeByteOrderMark: false
                 })),
-                emitSkipped: false
+                emitSkipped: false,
+                diagnostics: emptyArray
             };
         }
 
@@ -270,7 +271,8 @@ ${appendJs}`
         function noEmitOutput(): EmitOutput {
             return {
                 emitSkipped: true,
-                outputFiles: []
+                outputFiles: [],
+                diagnostics: emptyArray
             };
         }
 
diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts
@@ -7495,6 +7495,16 @@ declare namespace ts.server.protocol {
          * if true - then file should be recompiled even if it does not have any changes.
          */
         forced?: boolean;
+        includeLinePosition?: boolean;
+        /** if true - return response as object with emitSkipped and diagnostics */
+        richResponse?: boolean;
+    }
+    interface CompileOnSaveEmitFileResponse extends Response {
+        body: boolean | EmitResult;
+    }
+    interface EmitResult {
+        emitSkipped: boolean;
+        diagnostics: Diagnostic[] | DiagnosticWithLinePosition[];
     }
     /**
      * Quickinfo request; value of command field is
@@ -8909,7 +8919,7 @@ declare namespace ts.server {
         /**
          * Returns true if emit was conducted
          */
-        emitFile(scriptInfo: ScriptInfo, writeFile: (path: string, data: string, writeByteOrderMark?: boolean) => void): boolean;
+        emitFile(scriptInfo: ScriptInfo, writeFile: (path: string, data: string, writeByteOrderMark?: boolean) => void): EmitResult;
         enableLanguageService(): void;
         disableLanguageService(lastFileExceededProgramSize?: string): void;
         getProjectName(): string;

Original file line number	Diff line number	Diff line change
`@@ -2,6 +2,7 @@ namespace ts {`
`2`	`2`	`export interface EmitOutput {`
`3`	`3`	`outputFiles: OutputFile[];`
`4`	`4`	`emitSkipped: boolean;`
	`5`	`+ /* @internal */ diagnostics: readonly Diagnostic[];`
`5`	`6`	`/* @internal */ exportedModulesFromDeclarationEmit?: ExportedModulesFromDeclarationEmit;`
`6`	`7`	`}`
`7`	`8`
`@@ -10,4 +11,4 @@ namespace ts {`
`10`	`11`	`writeByteOrderMark: boolean;`
`11`	`12`	`text: string;`
`12`	`13`	`}`
`13`		`-}`
	`14`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -358,7 +358,7 @@ namespace ts.server {`
`358`	`358`	`getEmitOutput(file: string): EmitOutput {`
`359`	`359`	`const request = this.processRequest<protocol.EmitOutputRequest>(protocol.CommandTypes.EmitOutput, { file });`
`360`	`360`	`const response = this.processResponse<protocol.EmitOutputResponse>(request);`
`361`		`- return response.body;`
	`361`	`+ return response.body as EmitOutput;`
`362`	`362`	`}`
`363`	`363`
`364`	`364`	`getSyntacticDiagnostics(file: string): DiagnosticWithLocation[] {`
Original file line number	Diff line number	Diff line change
@@ -58,6 +58,7 @@ export function Component(x: Config): any;`
`58`	`58`	`),`
`59`	`59`	`{`
`60`	`60`	`emitSkipped: true,`
	`61`	`+ diagnostics: emptyArray,`
`61`	`62`	`outputFiles: emptyArray,`
`62`	`63`	`exportedModulesFromDeclarationEmit: undefined`
`63`	`64`	`}`
@@ -71,6 +72,7 @@ export function Component(x: Config): any;`
`71`	`72`	`),`
`72`	`73`	`{`
`73`	`74`	`emitSkipped: false,`
	`75`	`+ diagnostics: emptyArray,`
`74`	`76`	`outputFiles: [{`
`75`	`77`	`name: "foo.d.ts",`
`76`	`78`	`text: "export {};\r\n",`