From 2eca0af91b0e4d28c4cb7fbe6a6a6c3da72d8ae4 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Wed, 2 Nov 2016 07:05:54 -0700 Subject: [PATCH] Leave files from `node_modules` out when calculating `getCommonSourceDirectory` --- src/compiler/program.ts | 13 ++++--- src/compiler/utilities.ts | 20 ++++++++--- src/harness/harness.ts | 36 +++++++------------ src/harness/runnerbase.ts | 2 +- .../reference/commonSourceDirectory.js | 16 +++++++++ .../reference/commonSourceDirectory.symbols | 13 +++++++ .../reference/commonSourceDirectory.types | 16 +++++++++ tests/cases/compiler/commonSourceDirectory.ts | 18 ++++++++++ 8 files changed, 102 insertions(+), 32 deletions(-) create mode 100644 tests/baselines/reference/commonSourceDirectory.js create mode 100644 tests/baselines/reference/commonSourceDirectory.symbols create mode 100644 tests/baselines/reference/commonSourceDirectory.types create mode 100644 tests/cases/compiler/commonSourceDirectory.ts diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 82d274e0487..35c6818fea8 100644 --- a/src/compiler/program.ts +++ b/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. @@ -722,7 +727,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, diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 299f91be662..a2fe7a2d3c6 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -2556,16 +2556,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); } /** @@ -2653,8 +2666,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); } } diff --git a/src/harness/harness.ts b/src/harness/harness.ts index a6e1ffad7f7..57ee46299b5 100644 --- a/src/harness/harness.ts +++ b/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; @@ -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"); } diff --git a/src/harness/runnerbase.ts b/src/harness/runnerbase.ts index 346382b7a57..d50604803ed 100644 --- a/src/harness/runnerbase.ts +++ b/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; diff --git a/tests/baselines/reference/commonSourceDirectory.js b/tests/baselines/reference/commonSourceDirectory.js new file mode 100644 index 00000000000..3babba98337 --- /dev/null +++ b/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; diff --git a/tests/baselines/reference/commonSourceDirectory.symbols b/tests/baselines/reference/commonSourceDirectory.symbols new file mode 100644 index 00000000000..88ded4c8c87 --- /dev/null +++ b/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)) + diff --git a/tests/baselines/reference/commonSourceDirectory.types b/tests/baselines/reference/commonSourceDirectory.types new file mode 100644 index 00000000000..a0c66b86ae0 --- /dev/null +++ b/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 + diff --git a/tests/cases/compiler/commonSourceDirectory.ts b/tests/cases/compiler/commonSourceDirectory.ts new file mode 100644 index 00000000000..6583da67c6a --- /dev/null +++ b/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" + } +}