Exclude js files in non-configured projects compile-on-save emitting (#12118)

zhengbli · web-flow · commit 4dc58dde38bd · 2016-11-10T14:12:24.000-08:00
* Exclude js files in non-configured projects CoS emitting

* remove unnecessary method
diff --git a/src/compiler/program.ts b/src/compiler/program.ts
@@ -1706,18 +1706,13 @@ namespace ts {
                     const emitFilePath = toPath(emitFileName, currentDirectory, getCanonicalFileName);
                     // Report error if the output overwrites input file
                     if (filesByName.contains(emitFilePath)) {
-                        if (options.noEmitOverwritenFiles && !options.out && !options.outDir && !options.outFile) {
-                            blockEmittingOfFile(emitFileName);
-                        }
-                        else {
-                            let chain: DiagnosticMessageChain;
-                            if (!options.configFilePath) {
-                                // The program is from either an inferred project or an external project
-                                chain = chainDiagnosticMessages(/*details*/ undefined, Diagnostics.Adding_a_tsconfig_json_file_will_help_organize_projects_that_contain_both_TypeScript_and_JavaScript_files_Learn_more_at_https_Colon_Slash_Slashaka_ms_Slashtsconfig);
-                            }
-                            chain = chainDiagnosticMessages(chain, Diagnostics.Cannot_write_file_0_because_it_would_overwrite_input_file, emitFileName);
-                            blockEmittingOfFile(emitFileName, createCompilerDiagnosticFromMessageChain(chain));
+                        let chain: DiagnosticMessageChain;
+                        if (!options.configFilePath) {
+                            // The program is from either an inferred project or an external project
+                            chain = chainDiagnosticMessages(/*details*/ undefined, Diagnostics.Adding_a_tsconfig_json_file_will_help_organize_projects_that_contain_both_TypeScript_and_JavaScript_files_Learn_more_at_https_Colon_Slash_Slashaka_ms_Slashtsconfig);
                         }
+                        chain = chainDiagnosticMessages(chain, Diagnostics.Cannot_write_file_0_because_it_would_overwrite_input_file, emitFileName);
+                        blockEmittingOfFile(emitFileName, createCompilerDiagnosticFromMessageChain(chain));
                     }
 
                     // Report error if multiple files write into same file
@@ -1732,11 +1727,9 @@ namespace ts {
             }
         }
 
-        function blockEmittingOfFile(emitFileName: string, diag?: Diagnostic) {
+        function blockEmittingOfFile(emitFileName: string, diag: Diagnostic) {
             hasEmitBlockingDiagnostics.set(toPath(emitFileName, currentDirectory, getCanonicalFileName), true);
-            if (diag) {
-                programDiagnostics.add(diag);
-            }
+            programDiagnostics.add(diag);
         }
     }
 
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -3075,7 +3075,7 @@ namespace ts {
         moduleResolution?: ModuleResolutionKind;
         newLine?: NewLineKind;
         noEmit?: boolean;
-        /*@internal*/noEmitOverwritenFiles?: boolean;
+        /*@internal*/noEmitForJsFiles?: boolean;
         noEmitHelpers?: boolean;
         noEmitOnError?: boolean;
         noErrorTruncation?: boolean;
diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -192,7 +192,7 @@ namespace ts {
     export function nodePosToString(node: Node): string {
         const file = getSourceFileOfNode(node);
         const loc = getLineAndCharacterOfPosition(file, node.pos);
-        return `${ file.fileName }(${ loc.line + 1 },${ loc.character + 1 })`;
+        return `${file.fileName}(${loc.line + 1},${loc.character + 1})`;
     }
 
     export function getStartPosOfNode(node: Node): number {
@@ -439,7 +439,7 @@ namespace ts {
     }
 
     export function isBlockScope(node: Node, parentNode: Node) {
-        switch (node.kind)  {
+        switch (node.kind) {
             case SyntaxKind.SourceFile:
             case SyntaxKind.CaseBlock:
             case SyntaxKind.CatchClause:
@@ -644,9 +644,9 @@ namespace ts {
 
     export function getJsDocCommentsFromText(node: Node, text: string) {
         const commentRanges = (node.kind === SyntaxKind.Parameter ||
-                               node.kind === SyntaxKind.TypeParameter ||
-                               node.kind === SyntaxKind.FunctionExpression ||
-                               node.kind === SyntaxKind.ArrowFunction) ?
+            node.kind === SyntaxKind.TypeParameter ||
+            node.kind === SyntaxKind.FunctionExpression ||
+            node.kind === SyntaxKind.ArrowFunction) ?
             concatenate(getTrailingCommentRanges(text, node.pos), getLeadingCommentRanges(text, node.pos)) :
             getLeadingCommentRangesOfNodeFromText(node, text);
         return filter(commentRanges, isJsDocComment);
@@ -911,7 +911,7 @@ namespace ts {
     export function isObjectLiteralOrClassExpressionMethod(node: Node): node is MethodDeclaration {
         return node.kind === SyntaxKind.MethodDeclaration &&
             (node.parent.kind === SyntaxKind.ObjectLiteralExpression ||
-            node.parent.kind === SyntaxKind.ClassExpression);
+                node.parent.kind === SyntaxKind.ClassExpression);
     }
 
     export function isIdentifierTypePredicate(predicate: TypePredicate): predicate is IdentifierTypePredicate {
@@ -1133,8 +1133,8 @@ namespace ts {
                 // if the parameter's parent has a body and its grandparent is a class declaration, this is a valid target;
                 return (<FunctionLikeDeclaration>node.parent).body !== undefined
                     && (node.parent.kind === SyntaxKind.Constructor
-                    || node.parent.kind === SyntaxKind.MethodDeclaration
-                    || node.parent.kind === SyntaxKind.SetAccessor)
+                        || node.parent.kind === SyntaxKind.MethodDeclaration
+                        || node.parent.kind === SyntaxKind.SetAccessor)
                     && node.parent.parent.kind === SyntaxKind.ClassDeclaration;
         }
 
@@ -1481,7 +1481,7 @@ namespace ts {
         }, tags => tags);
     }
 
-   function getJSDocs<T>(node: Node, checkParentVariableStatement: boolean, getDocs: (docs: JSDoc[]) => T[], getTags: (tags: JSDocTag[]) => T[]): T[] {
+    function getJSDocs<T>(node: Node, checkParentVariableStatement: boolean, getDocs: (docs: JSDoc[]) => T[], getTags: (tags: JSDocTag[]) => T[]): T[] {
         // TODO: Get rid of getJsDocComments and friends (note the lowercase 's' in Js)
         // TODO: A lot of this work should be cached, maybe. I guess it's only used in services right now...
         let result: T[] = undefined;
@@ -1502,8 +1502,8 @@ namespace ts {
 
             const variableStatementNode =
                 isInitializerOfVariableDeclarationInStatement ? node.parent.parent.parent :
-                isVariableOfVariableDeclarationStatement ? node.parent.parent :
-                undefined;
+                    isVariableOfVariableDeclarationStatement ? node.parent.parent :
+                        undefined;
             if (variableStatementNode) {
                 result = append(result, getJSDocs(variableStatementNode, checkParentVariableStatement, getDocs, getTags));
             }
@@ -1668,7 +1668,7 @@ namespace ts {
                     if ((<ShorthandPropertyAssignment>parent).name !== node) {
                         return AssignmentKind.None;
                     }
-                    // Fall through
+                // Fall through
                 case SyntaxKind.PropertyAssignment:
                     node = parent.parent;
                     break;
@@ -2570,14 +2570,18 @@ namespace ts {
         if (options.outFile || options.out) {
             const moduleKind = getEmitModuleKind(options);
             const moduleEmitEnabled = moduleKind === ModuleKind.AMD || moduleKind === ModuleKind.System;
-            const sourceFiles = host.getSourceFiles();
+            const sourceFiles = getAllEmittableSourceFiles();
             // Can emit only sources that are not declaration file and are either non module code or module with --module or --target es6 specified
             return filter(sourceFiles, moduleEmitEnabled ? isNonDeclarationFile : isBundleEmitNonExternalModule);
         }
         else {
-            const sourceFiles = targetSourceFile === undefined ? host.getSourceFiles() : [targetSourceFile];
+            const sourceFiles = targetSourceFile === undefined ? getAllEmittableSourceFiles() : [targetSourceFile];
             return filter(sourceFiles, isNonDeclarationFile);
         }
+
+        function getAllEmittableSourceFiles() {
+            return options.noEmitForJsFiles ? filter(host.getSourceFiles(), sourceFile => !isSourceFileJavaScript(sourceFile)) : host.getSourceFiles();
+        }
     }
 
     function isNonDeclarationFile(sourceFile: SourceFile) {
@@ -2609,7 +2613,7 @@ namespace ts {
         }
         else {
             for (const sourceFile of sourceFiles) {
-                 // Don't emit if source file is a declaration file, or was located under node_modules
+                // Don't emit if source file is a declaration file, or was located under node_modules
                 if (!isDeclarationFile(sourceFile) && !host.isSourceFileFromExternalLibrary(sourceFile)) {
                     onSingleFileEmit(host, sourceFile);
                 }
@@ -2671,7 +2675,7 @@ namespace ts {
             onBundledEmit(host);
         }
         else {
-            const sourceFiles = targetSourceFile === undefined ? host.getSourceFiles() : [targetSourceFile];
+            const sourceFiles = targetSourceFile === undefined ? getSourceFilesToEmit(host) : [targetSourceFile];
             for (const sourceFile of sourceFiles) {
                 // Don't emit if source file is a declaration file, or was located under node_modules
                 if (!isDeclarationFile(sourceFile) && !host.isSourceFileFromExternalLibrary(sourceFile)) {
@@ -2709,11 +2713,11 @@ namespace ts {
         function onBundledEmit(host: EmitHost) {
             // Can emit only sources that are not declaration file and are either non module code or module with
             // --module or --target es6 specified. Files included by searching under node_modules are also not emitted.
-            const bundledSources = filter(host.getSourceFiles(),
+            const bundledSources = filter(getSourceFilesToEmit(host),
                 sourceFile => !isDeclarationFile(sourceFile) &&
-                              !host.isSourceFileFromExternalLibrary(sourceFile) &&
-                              (!isExternalModule(sourceFile) ||
-                               !!getEmitModuleKind(options)));
+                    !host.isSourceFileFromExternalLibrary(sourceFile) &&
+                    (!isExternalModule(sourceFile) ||
+                        !!getEmitModuleKind(options)));
             if (bundledSources.length) {
                 const jsFilePath = options.outFile || options.out;
                 const emitFileNames: EmitFileNames = {
@@ -2899,7 +2903,7 @@ namespace ts {
         writeComment: (text: string, lineMap: number[], writer: EmitTextWriter, commentPos: number, commentEnd: number, newLine: string) => void,
         node: TextRange, newLine: string, removeComments: boolean) {
         let leadingComments: CommentRange[];
-        let currentDetachedCommentInfo: {nodePos: number, detachedCommentEndPos: number};
+        let currentDetachedCommentInfo: { nodePos: number, detachedCommentEndPos: number };
         if (removeComments) {
             // removeComments is true, only reserve pinned comment at the top of file
             // For example:
@@ -3246,10 +3250,10 @@ namespace ts {
 
     function stringifyValue(value: any): string {
         return typeof value === "string" ? `"${escapeString(value)}"`
-             : typeof value === "number" ? isFinite(value) ? String(value) : "null"
-             : typeof value === "boolean" ? value ? "true" : "false"
-             : typeof value === "object" && value ? isArray(value) ? cycleCheck(stringifyArray, value) : cycleCheck(stringifyObject, value)
-             : /*fallback*/ "null";
+            : typeof value === "number" ? isFinite(value) ? String(value) : "null"
+                : typeof value === "boolean" ? value ? "true" : "false"
+                    : typeof value === "object" && value ? isArray(value) ? cycleCheck(stringifyArray, value) : cycleCheck(stringifyObject, value)
+                        : /*fallback*/ "null";
     }
 
     function cycleCheck(cb: (value: any) => string, value: any) {
@@ -3274,7 +3278,7 @@ namespace ts {
 
     function stringifyProperty(memo: string, value: any, key: string) {
         return value === undefined || typeof value === "function" || key === "__cycle" ? memo
-             : (memo ? memo + "," : memo) + `"${escapeString(key)}":${stringifyValue(value)}`;
+            : (memo ? memo + "," : memo) + `"${escapeString(key)}":${stringifyValue(value)}`;
     }
 
     const base64Digits = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=";
@@ -3519,7 +3523,7 @@ namespace ts {
      * @param token The token.
      */
     export function createTokenRange(pos: number, token: SyntaxKind): TextRange {
-        return createRange(pos,  pos + tokenToString(token).length);
+        return createRange(pos, pos + tokenToString(token).length);
     }
 
     export function rangeIsOnSingleLine(range: TextRange, sourceFile: SourceFile) {
diff --git a/src/harness/unittests/compileOnSave.ts b/src/harness/unittests/compileOnSave.ts
@@ -492,5 +492,45 @@ namespace ts.projectSystem {
             assert.isTrue(host.fileExists(expectedEmittedFileName));
             assert.equal(host.readFile(expectedEmittedFileName), `"use strict";\r\nfunction Foo() { return 10; }\r\nexports.Foo = Foo;\r\n`);
         });
+
+        it("shoud not emit js files in external projects", () => {
+            const file1 = {
+                path: "/a/b/file1.ts",
+                content: "consonle.log('file1');"
+            };
+            // file2 has errors. The emitting should not be blocked.
+            const file2 = {
+                path: "/a/b/file2.js",
+                content: "console.log'file2');"
+            };
+            const file3 = {
+                path: "/a/b/file3.js",
+                content: "console.log('file3');"
+            };
+            const externalProjectName = "externalproject";
+            const host = createServerHost([file1, file2, file3, libFile]);
+            const session = createSession(host);
+            const projectService = session.getProjectService();
+
+            projectService.openExternalProject({
+                rootFiles: toExternalFiles([file1.path, file2.path]),
+                options: {
+                    allowJs: true,
+                    outFile: "dist.js",
+                    compileOnSave: true
+                },
+                projectFileName: externalProjectName
+            });
+
+            const emitRequest = makeSessionRequest<server.protocol.CompileOnSaveEmitFileRequestArgs>(CommandNames.CompileOnSaveEmitFile, { file: file1.path });
+            session.executeCommand(emitRequest);
+
+            const expectedOutFileName = "/a/b/dist.js";
+            assert.isTrue(host.fileExists(expectedOutFileName));
+            const outFileContent = host.readFile(expectedOutFileName);
+            assert.isTrue(outFileContent.indexOf(file1.content) !== -1);
+            assert.isTrue(outFileContent.indexOf(file2.content) === -1);
+            assert.isTrue(outFileContent.indexOf(file3.content) === -1);
+        });
     });
 }
diff --git a/src/server/project.ts b/src/server/project.ts
@@ -161,8 +161,8 @@ namespace ts.server {
                 this.compilerOptions.allowNonTsExtensions = true;
             }
 
-            if (this.projectKind === ProjectKind.Inferred) {
-                this.compilerOptions.noEmitOverwritenFiles = true;
+            if (this.projectKind === ProjectKind.Inferred || this.projectKind === ProjectKind.External) {
+                this.compilerOptions.noEmitForJsFiles = true;
             }
 
             if (languageServiceEnabled) {

Original file line number	Diff line number	Diff line change
`@@ -161,8 +161,8 @@ namespace ts.server {`
`161`	`161`	`this.compilerOptions.allowNonTsExtensions = true;`
`162`	`162`	`}`
`163`	`163`
`164`		`- if (this.projectKind === ProjectKind.Inferred) {`
`165`		`- this.compilerOptions.noEmitOverwritenFiles = true;`
	`164`	`+ if (this.projectKind === ProjectKind.Inferred \|\| this.projectKind === ProjectKind.External) {`
	`165`	`+ this.compilerOptions.noEmitForJsFiles = true;`
`166`	`166`	`}`
`167`	`167`
`168`	`168`	`if (languageServiceEnabled) {`