Leave files from node_modules out when calculating getCommonSourceDirectory

Author: Andy Hanson

src/compiler/program.ts

@@ -430,13 +430,14 @@ namespace ts {
         return program;
 
         function getCommonSourceDirectory() {
-            if (typeof commonSourceDirectory === "undefined") {
-                if (options.rootDir && checkSourceFilesBelongToPath(files, options.rootDir)) {
+            if (commonSourceDirectory === undefined) {
+                const emittedFiles = filterSourceFilesInDirectory(files, isSourceFileFromExternalLibrary);
+                if (options.rootDir && checkSourceFilesBelongToPath(emittedFiles, options.rootDir)) {
                     // If a rootDir is specified and is valid use it as the commonSourceDirectory
                     commonSourceDirectory = getNormalizedAbsolutePath(options.rootDir, currentDirectory);
                 }
                 else {
-                    commonSourceDirectory = computeCommonSourceDirectory(files);
+                    commonSourceDirectory = computeCommonSourceDirectory(emittedFiles);
                 }
                 if (commonSourceDirectory && commonSourceDirectory[commonSourceDirectory.length - 1] !== directorySeparator) {
                     // Make sure directory path ends with directory separator so this string can directly
@@ -448,6 +449,10 @@ namespace ts {
             return commonSourceDirectory;
         }
 
+        function isSourceFileFromExternalLibrary(file: SourceFile): boolean {
+            return !!sourceFilesFoundSearchingNodeModules[file.path];
+        }
+
         function getClassifiableNames() {
             if (!classifiableNames) {
                 // Initialize a checker so that all our files are bound.
@@ -592,7 +597,7 @@ namespace ts {
                 getSourceFile: program.getSourceFile,
                 getSourceFileByPath: program.getSourceFileByPath,
                 getSourceFiles: program.getSourceFiles,
-                isSourceFileFromExternalLibrary: (file: SourceFile) => !!sourceFilesFoundSearchingNodeModules[file.path],
+                isSourceFileFromExternalLibrary,
                 writeFile: writeFileCallback || (
                     (fileName, data, writeByteOrderMark, onError, sourceFiles) => host.writeFile(fileName, data, writeByteOrderMark, onError, sourceFiles)),
                 isEmitBlocked,

src/compiler/utilities.ts

@@ -2534,16 +2534,29 @@ namespace ts {
         }
         else {
             const sourceFiles = targetSourceFile === undefined ? host.getSourceFiles() : [targetSourceFile];
-            return filter(sourceFiles, isNonDeclarationFile);
+            return filterSourceFilesInDirectory(sourceFiles, file => host.isSourceFileFromExternalLibrary(file));
         }
     }
 
+    /** Don't call this for `--outFile`, just for `--outDir` or plain emit. */
+    export function filterSourceFilesInDirectory(sourceFiles: SourceFile[], isSourceFileFromExternalLibrary: (file: SourceFile) => boolean): SourceFile[] {
+        return filter(sourceFiles, file => shouldEmitInDirectory(file, isSourceFileFromExternalLibrary));
+    }
+
     function isNonDeclarationFile(sourceFile: SourceFile) {
         return !isDeclarationFile(sourceFile);
     }
 
+    /**
+     * Whether a file should be emitted in a non-`--outFile` case.
+     * Don't emit if source file is a declaration file, or was located under node_modules
+     */
+    function shouldEmitInDirectory(sourceFile: SourceFile, isSourceFileFromExternalLibrary: (file: SourceFile) => boolean): boolean {
+        return isNonDeclarationFile(sourceFile) && !isSourceFileFromExternalLibrary(sourceFile);
+    }
+
     function isBundleEmitNonExternalModule(sourceFile: SourceFile) {
-        return !isDeclarationFile(sourceFile) && !isExternalModule(sourceFile);
+        return isNonDeclarationFile(sourceFile) && !isExternalModule(sourceFile);
     }
 
     /**
@@ -2631,8 +2644,7 @@ namespace ts {
         else {
             const sourceFiles = targetSourceFile === undefined ? host.getSourceFiles() : [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)) {
+                if (shouldEmitInDirectory(sourceFile, file => host.isSourceFileFromExternalLibrary(file))) {
                     onSingleFileEmit(host, sourceFile);
                 }
             }

src/harness/harness.ts

@@ -470,19 +470,6 @@ namespace Utils {
     }
 }
 
-namespace Harness.Path {
-    export function getFileName(fullPath: string) {
-        return fullPath.replace(/^.*[\\\/]/, "");
-    }
-
-    export function filePath(fullPath: string) {
-        fullPath = ts.normalizeSlashes(fullPath);
-        const components = fullPath.split("/");
-        const path: string[] = components.slice(0, components.length - 1);
-        return path.join("/") + "/";
-    }
-}
-
 namespace Harness {
     export interface IO {
         newLine(): string;
@@ -1090,7 +1077,9 @@ namespace Harness {
             { name: "suppressOutputPathCheck", type: "boolean" },
             { name: "noImplicitReferences", type: "boolean" },
             { name: "currentDirectory", type: "string" },
-            { name: "symlink", type: "string" }
+            { name: "symlink", type: "string" },
+            // Emitted js baseline will print full paths for every output file
+            { name: "fullEmitPaths", type: "boolean" }
         ];
 
         let optionsIndex: ts.Map<ts.CommandLineOption>;
@@ -1588,7 +1577,7 @@ namespace Harness {
 
                     let sourceMapCode = "";
                     for (let i = 0; i < result.sourceMaps.length; i++) {
-                        sourceMapCode += "//// [" + Harness.Path.getFileName(result.sourceMaps[i].fileName) + "]\r\n";
+                        sourceMapCode += "//// [" + ts.getBaseFileName(result.sourceMaps[i].fileName) + "]\r\n";
                         sourceMapCode += getByteOrderMarkText(result.sourceMaps[i]);
                         sourceMapCode += result.sourceMaps[i].code;
                     }
@@ -1611,21 +1600,22 @@ namespace Harness {
                     tsCode += "//// [" + header + "] ////\r\n\r\n";
                 }
                 for (let i = 0; i < tsSources.length; i++) {
-                    tsCode += "//// [" + Harness.Path.getFileName(tsSources[i].unitName) + "]\r\n";
+                    tsCode += "//// [" + ts.getBaseFileName(tsSources[i].unitName) + "]\r\n";
                     tsCode += tsSources[i].content + (i < (tsSources.length - 1) ? "\r\n" : "");
                 }
 
                 let jsCode = "";
-                for (let i = 0; i < result.files.length; i++) {
-                    jsCode += "//// [" + Harness.Path.getFileName(result.files[i].fileName) + "]\r\n";
-                    jsCode += getByteOrderMarkText(result.files[i]);
-                    jsCode += result.files[i].code;
+                for (const file of result.files) {
+                    const fileName = harnessSettings["fullEmitPaths"] ? file.fileName : ts.getBaseFileName(file.fileName);
+                    jsCode += "//// [" + fileName + "]\r\n";
+                    jsCode += getByteOrderMarkText(file);
+                    jsCode += file.code;
                 }
 
                 if (result.declFilesCode.length > 0) {
                     jsCode += "\r\n\r\n";
                     for (let i = 0; i < result.declFilesCode.length; i++) {
-                        jsCode += "//// [" + Harness.Path.getFileName(result.declFilesCode[i].fileName) + "]\r\n";
+                        jsCode += "//// [" + ts.getBaseFileName(result.declFilesCode[i].fileName) + "]\r\n";
                         jsCode += getByteOrderMarkText(result.declFilesCode[i]);
                         jsCode += result.declFilesCode[i].code;
                     }
@@ -1848,7 +1838,7 @@ namespace Harness {
             }
 
             // normalize the fileName for the single file case
-            currentFileName = testUnitData.length > 0 || currentFileName ? currentFileName : Path.getFileName(fileName);
+            currentFileName = testUnitData.length > 0 || currentFileName ? currentFileName : ts.getBaseFileName(fileName);
 
             // EOF, push whatever remains
             const newTestFile2 = {
@@ -2012,7 +2002,7 @@ namespace Harness {
 
     export function isDefaultLibraryFile(filePath: string): boolean {
         // We need to make sure that the filePath is prefixed with "lib." not just containing "lib." and end with ".d.ts"
-        const fileName = Path.getFileName(filePath);
+        const fileName = ts.getBaseFileName(filePath);
         return ts.startsWith(fileName, "lib.") && ts.endsWith(fileName, ".d.ts");
     }
 

src/harness/runnerbase.ts

@@ -32,7 +32,7 @@ abstract class RunnerBase {
     /** Replaces instances of full paths with fileNames only */
     static removeFullPaths(path: string) {
         // If its a full path (starts with "C:" or "/") replace with just the filename
-        let fixedPath = /^(\w:|\/)/.test(path) ? Harness.Path.getFileName(path) : path;
+        let fixedPath = /^(\w:|\/)/.test(path) ? ts.getBaseFileName(path) : path;
 
         // when running in the browser the 'full path' is the host name, shows up in error baselines
         const localHost = /http:\/localhost:\d+/g;

tests/baselines/reference/commonSourceDirectory.js

@@ -0,0 +1,16 @@
+//// [tests/cases/compiler/commonSourceDirectory.ts] ////
+
+//// [index.ts]
+// Test that importing a file from `node_modules` does not affect calculation of the common source directory.
+
+export const x = 0;
+
+//// [index.ts]
+import { x } from "foo";
+x + 1;
+
+
+//// [/app/bin/index.js]
+"use strict";
+var foo_1 = require("foo");
+foo_1.x + 1;

tests/baselines/reference/commonSourceDirectory.symbols

@@ -0,0 +1,13 @@
+=== /app/index.ts ===
+import { x } from "foo";
+>x : Symbol(x, Decl(index.ts, 0, 8))
+
+x + 1;
+>x : Symbol(x, Decl(index.ts, 0, 8))
+
+=== /node_modules/foo/index.ts ===
+// Test that importing a file from `node_modules` does not affect calculation of the common source directory.
+
+export const x = 0;
+>x : Symbol(x, Decl(index.ts, 2, 12))
+

tests/baselines/reference/commonSourceDirectory.types

@@ -0,0 +1,16 @@
+=== /app/index.ts ===
+import { x } from "foo";
+>x : 0
+
+x + 1;
+>x + 1 : number
+>x : 0
+>1 : 1
+
+=== /node_modules/foo/index.ts ===
+// Test that importing a file from `node_modules` does not affect calculation of the common source directory.
+
+export const x = 0;
+>x : 0
+>0 : 0
+

tests/cases/compiler/commonSourceDirectory.ts

@@ -0,0 +1,18 @@
+// Test that importing a file from `node_modules` does not affect calculation of the common source directory.
+// @noImplicitReferences: true
+// @moduleResolution: node
+// @fullEmitPaths: true
+
+// @filename: /node_modules/foo/index.ts
+export const x = 0;
+
+// @filename: /app/index.ts
+import { x } from "foo";
+x + 1;
+
+// @filename: /app/tsconfig.json
+{
+    "compilerOptions": {
+        "outDir": "bin"
+    }
+}