Do not write files directly from builder when noEmitOnError is true (#34832)

* Add tests for noEmitOnError

* Do not write files directly from builder when noEmitOnError is true
Fixes #34823

* make linter happy

* Instead of generating output in memory, check errors before doing the emit in case of noEmitOnError

src/compiler/builder.ts

@@ -882,36 +882,36 @@ namespace ts {
         oldProgram = undefined;
         oldState = undefined;
 
-        const result = createRedirectedBuilderProgram(state, configFileParsingDiagnostics);
-        result.getState = () => state;
-        result.backupState = () => {
+        const builderProgram = createRedirectedBuilderProgram(state, configFileParsingDiagnostics);
+        builderProgram.getState = () => state;
+        builderProgram.backupState = () => {
             Debug.assert(backupState === undefined);
             backupState = cloneBuilderProgramState(state);
         };
-        result.restoreState = () => {
+        builderProgram.restoreState = () => {
             state = Debug.assertDefined(backupState);
             backupState = undefined;
         };
-        result.getAllDependencies = sourceFile => BuilderState.getAllDependencies(state, Debug.assertDefined(state.program), sourceFile);
-        result.getSemanticDiagnostics = getSemanticDiagnostics;
-        result.emit = emit;
-        result.releaseProgram = () => {
+        builderProgram.getAllDependencies = sourceFile => BuilderState.getAllDependencies(state, Debug.assertDefined(state.program), sourceFile);
+        builderProgram.getSemanticDiagnostics = getSemanticDiagnostics;
+        builderProgram.emit = emit;
+        builderProgram.releaseProgram = () => {
             releaseCache(state);
             backupState = undefined;
         };
 
         if (kind === BuilderProgramKind.SemanticDiagnosticsBuilderProgram) {
-            (result as SemanticDiagnosticsBuilderProgram).getSemanticDiagnosticsOfNextAffectedFile = getSemanticDiagnosticsOfNextAffectedFile;
+            (builderProgram as SemanticDiagnosticsBuilderProgram).getSemanticDiagnosticsOfNextAffectedFile = getSemanticDiagnosticsOfNextAffectedFile;
         }
         else if (kind === BuilderProgramKind.EmitAndSemanticDiagnosticsBuilderProgram) {
-            (result as EmitAndSemanticDiagnosticsBuilderProgram).getSemanticDiagnosticsOfNextAffectedFile = getSemanticDiagnosticsOfNextAffectedFile;
-            (result as EmitAndSemanticDiagnosticsBuilderProgram).emitNextAffectedFile = emitNextAffectedFile;
+            (builderProgram as EmitAndSemanticDiagnosticsBuilderProgram).getSemanticDiagnosticsOfNextAffectedFile = getSemanticDiagnosticsOfNextAffectedFile;
+            (builderProgram as EmitAndSemanticDiagnosticsBuilderProgram).emitNextAffectedFile = emitNextAffectedFile;
         }
         else {
             notImplemented();
         }
 
-        return result;
+        return builderProgram;
 
         /**
          * Emits the next affected file's emit result (EmitResult and sourceFiles emitted) or returns undefined if iteration is complete
@@ -987,6 +987,8 @@ namespace ts {
         function emit(targetSourceFile?: SourceFile, writeFile?: WriteFileCallback, cancellationToken?: CancellationToken, emitOnlyDtsFiles?: boolean, customTransformers?: CustomTransformers): EmitResult {
             if (kind === BuilderProgramKind.EmitAndSemanticDiagnosticsBuilderProgram) {
                 assertSourceFileOkWithoutNextAffectedCall(state, targetSourceFile);
+                const result = handleNoEmitOptions(builderProgram, targetSourceFile, cancellationToken);
+                if (result) return result;
                 if (!targetSourceFile) {
                     // Emit and report any errors we ran into.
                     let sourceMaps: SourceMapEmitResult[] = [];

src/compiler/program.ts

@@ -1570,37 +1570,9 @@ namespace ts {
         }
 
         function emitWorker(program: Program, sourceFile: SourceFile | undefined, writeFileCallback: WriteFileCallback | undefined, cancellationToken: CancellationToken | undefined, emitOnlyDtsFiles?: boolean, customTransformers?: CustomTransformers, forceDtsEmit?: boolean): EmitResult {
-            let declarationDiagnostics: readonly Diagnostic[] = [];
-
             if (!forceDtsEmit) {
-                if (options.noEmit) {
-                    return { diagnostics: declarationDiagnostics, sourceMaps: undefined, emittedFiles: undefined, emitSkipped: true };
-                }
-
-                // If the noEmitOnError flag is set, then check if we have any errors so far.  If so,
-                // immediately bail out.  Note that we pass 'undefined' for 'sourceFile' so that we
-                // get any preEmit diagnostics, not just the ones
-                if (options.noEmitOnError) {
-                    const diagnostics = [
-                        ...program.getOptionsDiagnostics(cancellationToken),
-                        ...program.getSyntacticDiagnostics(sourceFile, cancellationToken),
-                        ...program.getGlobalDiagnostics(cancellationToken),
-                        ...program.getSemanticDiagnostics(sourceFile, cancellationToken)
-                    ];
-
-                    if (diagnostics.length === 0 && getEmitDeclarations(program.getCompilerOptions())) {
-                        declarationDiagnostics = program.getDeclarationDiagnostics(/*sourceFile*/ undefined, cancellationToken);
-                    }
-
-                    if (diagnostics.length > 0 || declarationDiagnostics.length > 0) {
-                        return {
-                            diagnostics: concatenate(diagnostics, declarationDiagnostics),
-                            sourceMaps: undefined,
-                            emittedFiles: undefined,
-                            emitSkipped: true
-                        };
-                    }
-                }
+                const result = handleNoEmitOptions(program, sourceFile, cancellationToken);
+                if (result) return result;
             }
 
             // Create the emit resolver outside of the "emitTime" tracking code below.  That way
@@ -3442,6 +3414,33 @@ namespace ts {
         }
     }
 
+    /*@internal*/
+    export function handleNoEmitOptions(program: ProgramToEmitFilesAndReportErrors, sourceFile: SourceFile | undefined, cancellationToken: CancellationToken | undefined): EmitResult | undefined {
+        const options = program.getCompilerOptions();
+        if (options.noEmit) {
+            return { diagnostics: emptyArray, sourceMaps: undefined, emittedFiles: undefined, emitSkipped: true };
+        }
+
+        // If the noEmitOnError flag is set, then check if we have any errors so far.  If so,
+        // immediately bail out.  Note that we pass 'undefined' for 'sourceFile' so that we
+        // get any preEmit diagnostics, not just the ones
+        if (!options.noEmitOnError) return undefined;
+        let diagnostics: readonly Diagnostic[] = [
+            ...program.getOptionsDiagnostics(cancellationToken),
+            ...program.getSyntacticDiagnostics(sourceFile, cancellationToken),
+            ...program.getGlobalDiagnostics(cancellationToken),
+            ...program.getSemanticDiagnostics(sourceFile, cancellationToken)
+        ];
+
+        if (diagnostics.length === 0 && getEmitDeclarations(program.getCompilerOptions())) {
+            diagnostics = program.getDeclarationDiagnostics(/*sourceFile*/ undefined, cancellationToken);
+        }
+
+        return diagnostics.length > 0 ?
+            { diagnostics, sourceMaps: undefined, emittedFiles: undefined, emitSkipped: true } :
+            undefined;
+    }
+
     /*@internal*/
     interface CompilerHostLike {
         useCaseSensitiveFileNames(): boolean;

src/compiler/watch.ts

@@ -124,6 +124,7 @@ namespace ts {
         getOptionsDiagnostics(cancellationToken?: CancellationToken): readonly Diagnostic[];
         getGlobalDiagnostics(cancellationToken?: CancellationToken): readonly Diagnostic[];
         getSemanticDiagnostics(sourceFile?: SourceFile, cancellationToken?: CancellationToken): readonly Diagnostic[];
+        getDeclarationDiagnostics(sourceFile?: SourceFile, cancellationToken?: CancellationToken): readonly DiagnosticWithLocation[];
         getConfigFileParsingDiagnostics(): readonly Diagnostic[];
         emit(targetSourceFile?: SourceFile, writeFile?: WriteFileCallback, cancellationToken?: CancellationToken, emitOnlyDtsFiles?: boolean, customTransformers?: CustomTransformers): EmitResult;
     }
@@ -152,20 +153,20 @@ namespace ts {
         const isListFilesOnly = !!program.getCompilerOptions().listFilesOnly;
 
         // First get and report any syntactic errors.
-        const diagnostics = program.getConfigFileParsingDiagnostics().slice();
-        const configFileParsingDiagnosticsLength = diagnostics.length;
-        addRange(diagnostics, program.getSyntacticDiagnostics(/*sourceFile*/ undefined, cancellationToken));
+        const allDiagnostics = program.getConfigFileParsingDiagnostics().slice();
+        const configFileParsingDiagnosticsLength = allDiagnostics.length;
+        addRange(allDiagnostics, program.getSyntacticDiagnostics(/*sourceFile*/ undefined, cancellationToken));
 
         // If we didn't have any syntactic errors, then also try getting the global and
         // semantic errors.
-        if (diagnostics.length === configFileParsingDiagnosticsLength) {
-            addRange(diagnostics, program.getOptionsDiagnostics(cancellationToken));
+        if (allDiagnostics.length === configFileParsingDiagnosticsLength) {
+            addRange(allDiagnostics, program.getOptionsDiagnostics(cancellationToken));
 
             if (!isListFilesOnly) {
-                addRange(diagnostics, program.getGlobalDiagnostics(cancellationToken));
+                addRange(allDiagnostics, program.getGlobalDiagnostics(cancellationToken));
 
-                if (diagnostics.length === configFileParsingDiagnosticsLength) {
-                    addRange(diagnostics, program.getSemanticDiagnostics(/*sourceFile*/ undefined, cancellationToken));
+                if (allDiagnostics.length === configFileParsingDiagnosticsLength) {
+                    addRange(allDiagnostics, program.getSemanticDiagnostics(/*sourceFile*/ undefined, cancellationToken));
                 }
             }
         }
@@ -175,9 +176,10 @@ namespace ts {
             ? { emitSkipped: true, diagnostics: emptyArray }
             : program.emit(/*targetSourceFile*/ undefined, writeFile, cancellationToken, emitOnlyDtsFiles, customTransformers);
         const { emittedFiles, diagnostics: emitDiagnostics } = emitResult;
-        addRange(diagnostics, emitDiagnostics);
+        addRange(allDiagnostics, emitDiagnostics);
 
-        sortAndDeduplicateDiagnostics(diagnostics).forEach(reportDiagnostic);
+        const diagnostics = sortAndDeduplicateDiagnostics(allDiagnostics);
+        diagnostics.forEach(reportDiagnostic);
         if (writeFileName) {
             const currentDir = program.getCurrentDirectory();
             forEach(emittedFiles, file => {

src/testRunner/tsconfig.json

@@ -118,6 +118,7 @@
         "unittests/tsbuild/lateBoundSymbol.ts",
         "unittests/tsbuild/missingExtendedFile.ts",
         "unittests/tsbuild/moduleSpecifiers.ts",
+        "unittests/tsbuild/noEmitOnError.ts",
         "unittests/tsbuild/outFile.ts",
         "unittests/tsbuild/referencesWithRootDirInParent.ts",
         "unittests/tsbuild/resolveJsonModule.ts",

src/testRunner/unittests/tsbuild/exitCodeOnBogusFile.ts

@@ -4,7 +4,7 @@ namespace ts {
         verifyTsc({
             scenario: "exitCodeOnBogusFile",
             subScenario: `test exit code`,
-            fs: () => loadProjectFromFiles({}, symbolLibContent),
+            fs: () => loadProjectFromFiles({}),
             commandLineArgs: ["-b", "bogus.json"]
         });
     });

src/testRunner/unittests/tsbuild/noEmitOnError.ts

@@ -0,0 +1,17 @@
+namespace ts {
+    describe("unittests:: tsbuild - with noEmitOnError", () => {
+        let projFs: vfs.FileSystem;
+        before(() => {
+            projFs = loadProjectFromDisk("tests/projects/noEmitOnError");
+        });
+        after(() => {
+            projFs = undefined!;
+        });
+        verifyTsc({
+            scenario: "noEmitOnError",
+            subScenario: "has empty files diagnostic when files is empty and no references are provided",
+            fs: () => projFs,
+            commandLineArgs: ["--b", "/src/tsconfig.json"],
+        });
+    });
+}

src/testRunner/unittests/tsbuild/watchMode.ts

@@ -1389,4 +1389,45 @@ ${coreFiles[1].content}`);
             );
         }
     });
+
+    describe("unittests:: tsbuild:: watchMode:: with noEmitOnError", () => {
+        it("does not emit any files on error", () => {
+            const projectLocation = `${projectsLocation}/noEmitOnError`;
+            const host = createTsBuildWatchSystem([
+                ...["tsconfig.json", "shared/types/db.ts", "src/main.ts", "src/other.ts"]
+                    .map(f => getFileFromProject("noEmitOnError", f)),
+                { path: libFile.path, content: libContent }
+            ], { currentDirectory: projectLocation });
+            createSolutionBuilderWithWatch(host, ["tsconfig.json"], { verbose: true, watch: true });
+            checkOutputErrorsInitial(host, [
+                `src/main.ts(4,1): error TS1005: ',' expected.\n`,
+            ], /*disableConsoleClears*/ undefined, [
+                `Projects in this build: \r\n    * tsconfig.json\n\n`,
+                `Project 'tsconfig.json' is out of date because output file 'dev-build/shared/types/db.js' does not exist\n\n`,
+                `Building project '/user/username/projects/noEmitOnError/tsconfig.json'...\n\n`,
+            ]);
+            assert.equal(host.writtenFiles.size, 0, `Expected not to write any files: ${arrayFrom(host.writtenFiles.keys())}`);
+
+            // Make changes
+            host.writeFile(`${projectLocation}/src/main.ts`, `import { A } from "../shared/types/db";
+const a = {
+    lastName: 'sdsd'
+};`);
+            host.writtenFiles.clear();
+            host.checkTimeoutQueueLengthAndRun(1); // build project
+            host.checkTimeoutQueueLength(0);
+            checkOutputErrorsIncremental(host, emptyArray, /*disableConsoleClears*/ undefined, /*logsBeforeWatchDiagnostics*/ undefined, [
+                `Project 'tsconfig.json' is out of date because output file 'dev-build/shared/types/db.js' does not exist\n\n`,
+                `Building project '/user/username/projects/noEmitOnError/tsconfig.json'...\n\n`,
+            ]);
+            assert.equal(host.writtenFiles.size, 3, `Expected to write 3 files: Actual:: ${arrayFrom(host.writtenFiles.keys())}`);
+            for (const f of [
+                `${projectLocation}/dev-build/shared/types/db.js`,
+                `${projectLocation}/dev-build/src/main.js`,
+                `${projectLocation}/dev-build/src/other.js`,
+            ]) {
+                assert.isTrue(host.writtenFiles.has(f.toLowerCase()), `Expected to write file: ${f}:: Actual:: ${arrayFrom(host.writtenFiles.keys())}`);
+            }
+        });
+    });
 }

src/testRunner/unittests/tsc/incremental.ts

@@ -72,5 +72,21 @@ namespace ts {
             commandLineArgs: ["--p", "src/project"],
             incrementalScenarios: [noChangeRun]
         });
+
+        verifyTscIncrementalEdits({
+            scenario: "incremental",
+            subScenario: "with noEmitOnError",
+            fs: () => loadProjectFromDisk("tests/projects/noEmitOnError"),
+            commandLineArgs: ["--incremental", "-p", "src"],
+            incrementalScenarios: [
+                {
+                    buildKind: BuildKind.IncrementalDtsUnchanged,
+                    modifyFs: fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
+const a = {
+    lastName: 'sdsd'
+};`, "utf-8")
+                }
+            ]
+        });
     });
 }

src/testRunner/unittests/tscWatch/emitAndErrorUpdates.ts

@@ -392,5 +392,47 @@ export class Data2 {
                 verifyTransitiveExports(lib2Data, lib2Data2);
             });
         });
+
+        it("with noEmitOnError", () => {
+            const projectLocation = `${TestFSWithWatch.tsbuildProjectsLocation}/noEmitOnError`;
+            const allFiles = ["tsconfig.json", "shared/types/db.ts", "src/main.ts", "src/other.ts"]
+                .map(f => TestFSWithWatch.getTsBuildProjectFile("noEmitOnError", f));
+            const host = TestFSWithWatch.changeToHostTrackingWrittenFiles(
+                createWatchedSystem(
+                    [...allFiles, { path: libFile.path, content: libContent }],
+                    { currentDirectory: projectLocation }
+                )
+            );
+            const watch = createWatchOfConfigFile("tsconfig.json", host);
+            const mainFile = allFiles.find(f => f.path === `${projectLocation}/src/main.ts`)!;
+            checkOutputErrorsInitial(host, [
+                getDiagnosticOfFileFromProgram(
+                    watch(),
+                    mainFile.path,
+                    mainFile.content.lastIndexOf(";"),
+                    1,
+                    Diagnostics._0_expected,
+                    ","
+                )
+            ]);
+            assert.equal(host.writtenFiles.size, 0, `Expected not to write any files: ${arrayFrom(host.writtenFiles.keys())}`);
+
+            // Make changes
+            host.writeFile(mainFile.path, `import { A } from "../shared/types/db";
+const a = {
+    lastName: 'sdsd'
+};`);
+            host.writtenFiles.clear();
+            host.checkTimeoutQueueLengthAndRun(1); // build project
+            checkOutputErrorsIncremental(host, emptyArray);
+            assert.equal(host.writtenFiles.size, 3, `Expected to write 3 files: Actual:: ${arrayFrom(host.writtenFiles.keys())}`);
+            for (const f of [
+                `${projectLocation}/dev-build/shared/types/db.js`,
+                `${projectLocation}/dev-build/src/main.js`,
+                `${projectLocation}/dev-build/src/other.js`,
+            ]) {
+                assert.isTrue(host.writtenFiles.has(f.toLowerCase()), `Expected to write file: ${f}:: Actual:: ${arrayFrom(host.writtenFiles.keys())}`);
+            }
+        });
     });
 }

tests/baselines/reference/tsbuild/noEmitOnError/initial-build/has-empty-files-diagnostic-when-files-is-empty-and-no-references-are-provided.js

@@ -0,0 +1,6 @@
+//// [/lib/initial-buildOutput.txt]
+/lib/tsc --b /src/tsconfig.json
+src/src/main.ts(4,1): error TS1005: ',' expected.
+exitCode:: ExitStatus.DiagnosticsPresent_OutputsSkipped
+
+

tests/baselines/reference/tsc/incremental/incremental-declaration-doesnt-change/with-noEmitOnError.js

@@ -0,0 +1,72 @@
+//// [/lib/incremental-declaration-doesnt-changeOutput.txt]
+/lib/tsc --incremental -p src
+exitCode:: ExitStatus.Success
+
+
+//// [/src/dev-build/shared/types/db.js]
+"use strict";
+exports.__esModule = true;
+
+
+//// [/src/dev-build/src/main.js]
+"use strict";
+exports.__esModule = true;
+var a = {
+    lastName: 'sdsd'
+};
+
+
+//// [/src/dev-build/src/other.js]
+console.log("hi");
+
+
+//// [/src/dev-build/tsconfig.tsbuildinfo]
+{
+  "program": {
+    "fileInfos": {
+      "../../lib/lib.d.ts": {
+        "version": "3858781397-/// <reference no-default-lib=\"true\"/>\ninterface Boolean {}\ninterface Function {}\ninterface CallableFunction {}\ninterface NewableFunction {}\ninterface IArguments {}\ninterface Number { toExponential: any; }\ninterface Object {}\ninterface RegExp {}\ninterface String { charAt: any; }\ninterface Array<T> { length: number; [n: number]: T; }\ninterface ReadonlyArray<T> {}\ndeclare const console: { log(msg: any): void; };",
+        "signature": "3858781397-/// <reference no-default-lib=\"true\"/>\ninterface Boolean {}\ninterface Function {}\ninterface CallableFunction {}\ninterface NewableFunction {}\ninterface IArguments {}\ninterface Number { toExponential: any; }\ninterface Object {}\ninterface RegExp {}\ninterface String { charAt: any; }\ninterface Array<T> { length: number; [n: number]: T; }\ninterface ReadonlyArray<T> {}\ndeclare const console: { log(msg: any): void; };"
+      },
+      "../shared/types/db.ts": {
+        "version": "-9621097780-export interface A {\r\n    name: string;\r\n}",
+        "signature": "-6245214333-export interface A {\r\n    name: string;\r\n}\r\n"
+      },
+      "../src/main.ts": {
+        "version": "-2574605496-import { A } from \"../shared/types/db\";\nconst a = {\n    lastName: 'sdsd'\n};",
+        "signature": "-4882119183-export {};\r\n"
+      },
+      "../src/other.ts": {
+        "version": "7719445449-console.log(\"hi\");",
+        "signature": "5381-"
+      }
+    },
+    "options": {
+      "outDir": "./",
+      "noEmitOnError": true,
+      "incremental": true,
+      "project": "..",
+      "configFilePath": "../tsconfig.json"
+    },
+    "referencedMap": {
+      "../src/main.ts": [
+        "../shared/types/db.ts"
+      ]
+    },
+    "exportedModulesMap": {},
+    "semanticDiagnosticsPerFile": [
+      "../../lib/lib.d.ts",
+      "../shared/types/db.ts",
+      "../src/main.ts",
+      "../src/other.ts"
+    ]
+  },
+  "version": "FakeTSVersion"
+}
+
+//// [/src/src/main.ts]
+import { A } from "../shared/types/db";
+const a = {
+    lastName: 'sdsd'
+};
+

tests/baselines/reference/tsc/incremental/initial-build/with-noEmitOnError.js

@@ -0,0 +1,6 @@
+//// [/lib/initial-buildOutput.txt]
+/lib/tsc --incremental -p src
+src/src/main.ts(4,1): error TS1005: ',' expected.
+exitCode:: ExitStatus.DiagnosticsPresent_OutputsSkipped
+
+

tests/projects/noEmitOnError/shared/types/db.ts

@@ -0,0 +1,3 @@
+export interface A {
+    name: string;
+}

tests/projects/noEmitOnError/src/main.ts

@@ -0,0 +1,4 @@
+import { A } from "../shared/types/db";
+const a = {
+    lastName: 'sdsd'
+;

tests/projects/noEmitOnError/src/other.ts

@@ -0,0 +1 @@
+console.log("hi");

tests/projects/noEmitOnError/tsconfig.json

@@ -0,0 +1,6 @@
+{
+    "compilerOptions": {
+        "outDir": "./dev-build",
+        "noEmitOnError": true
+    }
+}