Populate if emit was skipped correctly as part of emit result

sheetalkamat · sheetalkamat · commit ff933be5ff25 · 2015-10-28T13:07:50.000-07:00
diff --git a/src/compiler/declarationEmitter.ts b/src/compiler/declarationEmitter.ts
@@ -1608,12 +1608,14 @@ namespace ts {
     /* @internal */
     export function writeDeclarationFile(declarationFilePath: string, sourceFile: SourceFile, host: EmitHost, resolver: EmitResolver, diagnostics: Diagnostic[]) {
         let emitDeclarationResult = emitDeclarations(host, resolver, diagnostics, declarationFilePath, sourceFile);
-        if (!emitDeclarationResult.reportedDeclarationError && !host.isDeclarationEmitBlocked(declarationFilePath, sourceFile)) {
+        let emitSkipped = emitDeclarationResult.reportedDeclarationError || host.isDeclarationEmitBlocked(declarationFilePath, sourceFile);
+        if (!emitSkipped) {
             let declarationOutput = emitDeclarationResult.referencePathsOutput
                 + getDeclarationOutput(emitDeclarationResult.synchronousDeclarationOutput, emitDeclarationResult.moduleElementDeclarationEmitInfo);
             let compilerOptions = host.getCompilerOptions();
             writeFile(host, diagnostics, declarationFilePath, declarationOutput, compilerOptions.emitBOM);
         }
+        return emitSkipped;
 
         function getDeclarationOutput(synchronousDeclarationOutput: string, moduleElementDeclarationEmitInfo: ModuleElementDeclarationEmitInfo[]) {
             let appliedSyncOutputPos = 0;
diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts
@@ -323,35 +323,36 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
         let modulekind = compilerOptions.module ? compilerOptions.module : languageVersion === ScriptTarget.ES6 ? ModuleKind.ES6 : ModuleKind.None;
         let sourceMapDataList: SourceMapData[] = compilerOptions.sourceMap || compilerOptions.inlineSourceMap ? [] : undefined;
         let diagnostics: Diagnostic[] = [];
+        let emitSkipped = false;
         let newLine = host.getNewLine();
 
         if (targetSourceFile === undefined) {
             forEach(host.getSourceFiles(), sourceFile => {
                 if (shouldEmitToOwnFile(sourceFile, compilerOptions)) {
-                    emitFile(getEmitFileNames(sourceFile, host), sourceFile);
+                    emitSkipped = emitFile(getEmitFileNames(sourceFile, host), sourceFile) || emitSkipped;
                 }
             });
 
             if (compilerOptions.outFile || compilerOptions.out) {
-                emitFile(getBundledEmitFileNames(compilerOptions));
+                emitSkipped = emitFile(getBundledEmitFileNames(compilerOptions)) || emitSkipped;
             }
         }
         else {
             // targetSourceFile is specified (e.g calling emitter from language service or calling getSemanticDiagnostic from language service)
             if (shouldEmitToOwnFile(targetSourceFile, compilerOptions)) {
-                emitFile(getEmitFileNames(targetSourceFile, host), targetSourceFile);
+                emitSkipped = emitFile(getEmitFileNames(targetSourceFile, host), targetSourceFile) || emitSkipped;
             }
             else if (!isDeclarationFile(targetSourceFile) &&
                 (compilerOptions.outFile || compilerOptions.out)) {
-                emitFile(getBundledEmitFileNames(compilerOptions));
+                emitSkipped = emitFile(getBundledEmitFileNames(compilerOptions)) || emitSkipped;
             }
         }
 
         // Sort and make the unique list of diagnostics
         diagnostics = sortAndDeduplicateDiagnostics(diagnostics);
 
         return {
-            emitSkipped: false,
+            emitSkipped,
             diagnostics,
             sourceMaps: sourceMapDataList
         };
@@ -7597,13 +7598,16 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, Promi
 
         function emitFile({ jsFilePath, sourceMapFilePath, declarationFilePath}: { jsFilePath: string, sourceMapFilePath: string, declarationFilePath: string }, sourceFile?: SourceFile) {
             // Make sure not to write js File and source map file if any of them cannot be written
-            if (!host.isEmitBlocked(jsFilePath) && (!sourceMapFilePath || !host.isEmitBlocked(sourceMapFilePath))) {
+            let emitSkipped = host.isEmitBlocked(jsFilePath) || (sourceMapFilePath && host.isEmitBlocked(sourceMapFilePath));
+            if (!emitSkipped) {
                 emitJavaScript(jsFilePath, sourceMapFilePath, sourceFile);
             }
 
             if (compilerOptions.declaration) {
-                writeDeclarationFile(declarationFilePath, sourceFile, host, resolver, diagnostics);
+                emitSkipped = writeDeclarationFile(declarationFilePath, sourceFile, host, resolver, diagnostics) || emitSkipped;
             }
+
+            return emitSkipped;
         }
     }
 }
diff --git a/src/compiler/program.ts b/src/compiler/program.ts
@@ -293,7 +293,7 @@ namespace ts {
                           program.getSemanticDiagnostics(sourceFile, cancellationToken));
 
         if (program.getCompilerOptions().declaration) {
-            diagnostics.concat(program.getDeclarationDiagnostics(sourceFile, cancellationToken));
+            diagnostics = diagnostics.concat(program.getDeclarationDiagnostics(sourceFile, cancellationToken));
         }
 
         return sortAndDeduplicateDiagnostics(diagnostics);
diff --git a/src/harness/harness.ts b/src/harness/harness.ts
@@ -1121,7 +1121,7 @@ namespace Harness {
 
                 let emitResult = program.emit();
 
-                let errors = ts.getPreEmitDiagnostics(program).concat(emitResult.diagnostics);
+                let errors = ts.getPreEmitDiagnostics(program);
                 this.lastErrors = errors;
 
                 let result = new CompilerResult(fileOutputs, errors, program, Harness.IO.getCurrentDirectory(), emitResult.sourceMaps);
@@ -1666,7 +1666,7 @@ namespace Harness {
             let encoded_actual =  Utils.encodeString(actual);
             if (expected != encoded_actual) {
                 // Overwrite & issue error
-                let errMsg = "The baseline file " + relativeFileName + " has changed.\nExpected:\n" + expected + "\nActual:\n" + encoded_actual;
+                let errMsg = "The baseline file " + relativeFileName + " has changed.";
                 throw new Error(errMsg);
             }
         }
diff --git a/tests/baselines/reference/compileOnSaveWorksWhenEmitBlockingErrorOnOtherFile.baseline b/tests/baselines/reference/compileOnSaveWorksWhenEmitBlockingErrorOnOtherFile.baseline
@@ -0,0 +1,8 @@
+EmitSkipped: true
+Diagnostics:
+  Cannot write file 'tests/cases/fourslash/b.js' which is one of the input files.
+
+EmitSkipped: false
+FileName : tests/cases/fourslash/a.js
+function foo2() { return 30; } // no error - should emit a.js
+
diff --git a/tests/baselines/reference/getEmitOutputWithEmitterErrors.baseline b/tests/baselines/reference/getEmitOutputWithEmitterErrors.baseline
@@ -1,4 +1,6 @@
-EmitSkipped: false
+EmitSkipped: true
+Diagnostics:
+  Exported variable 'foo' has or is using private name 'C'.
 FileName : tests/cases/fourslash/inputFile.js
 var M;
 (function (M) {
diff --git a/tests/baselines/reference/getEmitOutputWithEmitterErrors2.baseline b/tests/baselines/reference/getEmitOutputWithEmitterErrors2.baseline
@@ -1,4 +1,6 @@
-EmitSkipped: false
+EmitSkipped: true
+Diagnostics:
+  Exported variable 'foo' has or is using private name 'C'.
 FileName : tests/cases/fourslash/inputFile.js
 define(["require", "exports"], function (require, exports) {
     var C = (function () {
diff --git a/tests/baselines/reference/getEmitOutputWithSemanticErrors2.baseline b/tests/baselines/reference/getEmitOutputWithSemanticErrors2.baseline
@@ -1,4 +1,6 @@
-EmitSkipped: false
+EmitSkipped: true
+Diagnostics:
+  Type 'string' is not assignable to type 'number'.
 FileName : tests/cases/fourslash/inputFile.js
 var x = "hello world";
 
diff --git a/tests/baselines/reference/getEmitOutputWithSemanticErrorsForMultipleFiles2.baseline b/tests/baselines/reference/getEmitOutputWithSemanticErrorsForMultipleFiles2.baseline
@@ -1,4 +1,6 @@
-EmitSkipped: false
+EmitSkipped: true
+Diagnostics:
+  Type 'string' is not assignable to type 'boolean'.
 FileName : out.js
 // File to emit, does not contain semantic errors, but --out is passed
 // expected to not generate declarations because of the semantic errors in the other file
diff --git a/tests/cases/fourslash/compileOnSaveWorksWhenEmitBlockingErrorOnOtherFile.ts b/tests/cases/fourslash/compileOnSaveWorksWhenEmitBlockingErrorOnOtherFile.ts
@@ -0,0 +1,13 @@
+/// <reference path="fourslash.ts" />
+
+// @BaselineFile: compileOnSaveWorksWhenEmitBlockingErrorOnOtherFile.baseline
+// @jsExtensions: js
+// @Filename: b.js
+// @emitThisFile: true
+////function foo() { } // This has error because js file cannot be overwritten - emitSkipped should be true
+
+// @Filename: a.ts
+// @emitThisFile: true
+////function foo2() { return 30; } // no error - should emit a.js
+
+verify.baselineGetEmitOutput();

Original file line number	Diff line number	Diff line change
`@@ -323,35 +323,36 @@ var __awaiter = (this && this.__awaiter) \|\| function (thisArg, _arguments, Promi`
`323`	`323`	`let modulekind = compilerOptions.module ? compilerOptions.module : languageVersion === ScriptTarget.ES6 ? ModuleKind.ES6 : ModuleKind.None;`
`324`	`324`	`let sourceMapDataList: SourceMapData[] = compilerOptions.sourceMap \|\| compilerOptions.inlineSourceMap ? [] : undefined;`
`325`	`325`	`let diagnostics: Diagnostic[] = [];`
	`326`	`+ let emitSkipped = false;`
`326`	`327`	`let newLine = host.getNewLine();`
`327`	`328`
`328`	`329`	`if (targetSourceFile === undefined) {`
`329`	`330`	`forEach(host.getSourceFiles(), sourceFile => {`
`330`	`331`	`if (shouldEmitToOwnFile(sourceFile, compilerOptions)) {`
`331`		`- emitFile(getEmitFileNames(sourceFile, host), sourceFile);`
	`332`	`+ emitSkipped = emitFile(getEmitFileNames(sourceFile, host), sourceFile) \|\| emitSkipped;`
`332`	`333`	`}`
`333`	`334`	`});`
`334`	`335`
`335`	`336`	`if (compilerOptions.outFile \|\| compilerOptions.out) {`
`336`		`- emitFile(getBundledEmitFileNames(compilerOptions));`
	`337`	`+ emitSkipped = emitFile(getBundledEmitFileNames(compilerOptions)) \|\| emitSkipped;`
`337`	`338`	`}`
`338`	`339`	`}`
`339`	`340`	`else {`
`340`	`341`	`// targetSourceFile is specified (e.g calling emitter from language service or calling getSemanticDiagnostic from language service)`
`341`	`342`	`if (shouldEmitToOwnFile(targetSourceFile, compilerOptions)) {`
`342`		`- emitFile(getEmitFileNames(targetSourceFile, host), targetSourceFile);`
	`343`	`+ emitSkipped = emitFile(getEmitFileNames(targetSourceFile, host), targetSourceFile) \|\| emitSkipped;`
`343`	`344`	`}`
`344`	`345`	`else if (!isDeclarationFile(targetSourceFile) &&`
`345`	`346`	`(compilerOptions.outFile \|\| compilerOptions.out)) {`
`346`		`- emitFile(getBundledEmitFileNames(compilerOptions));`
	`347`	`+ emitSkipped = emitFile(getBundledEmitFileNames(compilerOptions)) \|\| emitSkipped;`
`347`	`348`	`}`
`348`	`349`	`}`
`349`	`350`
`350`	`351`	`// Sort and make the unique list of diagnostics`
`351`	`352`	`diagnostics = sortAndDeduplicateDiagnostics(diagnostics);`
`352`	`353`
`353`	`354`	`return {`
`354`		`- emitSkipped: false,`
	`355`	`+ emitSkipped,`
`355`	`356`	`diagnostics,`
`356`	`357`	`sourceMaps: sourceMapDataList`
`357`	`358`	`};`
`@@ -7597,13 +7598,16 @@ var __awaiter = (this && this.__awaiter) \|\| function (thisArg, _arguments, Promi`
`7597`	`7598`
`7598`	`7599`	`function emitFile({ jsFilePath, sourceMapFilePath, declarationFilePath}: { jsFilePath: string, sourceMapFilePath: string, declarationFilePath: string }, sourceFile?: SourceFile) {`
`7599`	`7600`	`// Make sure not to write js File and source map file if any of them cannot be written`
`7600`		`- if (!host.isEmitBlocked(jsFilePath) && (!sourceMapFilePath \|\| !host.isEmitBlocked(sourceMapFilePath))) {`
	`7601`	`+ let emitSkipped = host.isEmitBlocked(jsFilePath) \|\| (sourceMapFilePath && host.isEmitBlocked(sourceMapFilePath));`
	`7602`	`+ if (!emitSkipped) {`
`7601`	`7603`	`emitJavaScript(jsFilePath, sourceMapFilePath, sourceFile);`
`7602`	`7604`	`}`
`7603`	`7605`
`7604`	`7606`	`if (compilerOptions.declaration) {`
`7605`		`- writeDeclarationFile(declarationFilePath, sourceFile, host, resolver, diagnostics);`
	`7607`	`+ emitSkipped = writeDeclarationFile(declarationFilePath, sourceFile, host, resolver, diagnostics) \|\| emitSkipped;`
`7606`	`7608`	`}`
	`7609`	`+`
	`7610`	`+ return emitSkipped;`
`7607`	`7611`	`}`
`7608`	`7612`	`}`
`7609`	`7613`	`}`
Original file line number	Diff line number	Diff line change
`@@ -293,7 +293,7 @@ namespace ts {`
`293`	`293`	`program.getSemanticDiagnostics(sourceFile, cancellationToken));`
`294`	`294`
`295`	`295`	`if (program.getCompilerOptions().declaration) {`
`296`		`- diagnostics.concat(program.getDeclarationDiagnostics(sourceFile, cancellationToken));`
	`296`	`+ diagnostics = diagnostics.concat(program.getDeclarationDiagnostics(sourceFile, cancellationToken));`
`297`	`297`	`}`
`298`	`298`
`299`	`299`	`return sortAndDeduplicateDiagnostics(diagnostics);`
Original file line number	Diff line number	Diff line change
`@@ -1121,7 +1121,7 @@ namespace Harness {`
`1121`	`1121`
`1122`	`1122`	`let emitResult = program.emit();`
`1123`	`1123`
`1124`		`- let errors = ts.getPreEmitDiagnostics(program).concat(emitResult.diagnostics);`
	`1124`	`+ let errors = ts.getPreEmitDiagnostics(program);`
`1125`	`1125`	`this.lastErrors = errors;`
`1126`	`1126`
`1127`	`1127`	`let result = new CompilerResult(fileOutputs, errors, program, Harness.IO.getCurrentDirectory(), emitResult.sourceMaps);`
`@@ -1666,7 +1666,7 @@ namespace Harness {`
`1666`	`1666`	`let encoded_actual = Utils.encodeString(actual);`
`1667`	`1667`	`if (expected != encoded_actual) {`
`1668`	`1668`	`// Overwrite & issue error`
`1669`		`- let errMsg = "The baseline file " + relativeFileName + " has changed.\nExpected:\n" + expected + "\nActual:\n" + encoded_actual;`
	`1669`	`+ let errMsg = "The baseline file " + relativeFileName + " has changed.";`
`1670`	`1670`	`throw new Error(errMsg);`
`1671`	`1671`	`}`
`1672`	`1672`	`}`