Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -904,7 +904,7 @@ namespace ts {

function computeCommonSourceDirectory(sourceFiles: SourceFile[]): string {
let commonPathComponents: string[];
forEach(files, sourceFile => {
const failed = forEach(files, sourceFile => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting...I wonder why this was forEach before. It seems like a labelled for would have been more obvious.

Regardless, it's convenient because it makes your change smaller.

// Each file contributes into common source file path
if (isDeclarationFile(sourceFile)) {
return;
Expand All @@ -920,10 +920,10 @@ namespace ts {
}

for (let i = 0, n = Math.min(commonPathComponents.length, sourcePathComponents.length); i < n; i++) {
if (commonPathComponents[i] !== sourcePathComponents[i]) {
if (getCanonicalFileName(commonPathComponents[i]) !== getCanonicalFileName(sourcePathComponents[i])) {
if (i === 0) {
programDiagnostics.add(createCompilerDiagnostic(Diagnostics.Cannot_find_the_common_subdirectory_path_for_the_input_files));
return;
// Failed to find any common path component
return true;
}

// New common path found that is 0 -> i-1
Expand All @@ -938,6 +938,11 @@ namespace ts {
}
});

// A common path can not be found when paths span multiple drives on windows, for example
if (failed) {
return "";
}

if (!commonPathComponents) { // Can happen when all input files are .d.ts files
return currentDirectory;
}
Expand Down Expand Up @@ -1059,6 +1064,10 @@ namespace ts {
else {
// Compute the commonSourceDirectory from the input files
commonSourceDirectory = computeCommonSourceDirectory(files);
// If we failed to find a good common directory, but outDir is specified and at least one of our files is on a windows drive/URL/other resource, add a failure
if (options.outDir && commonSourceDirectory === "" && forEach(files, file => getRootLength(file.fileName) > 1)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isRootedDiskPath ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aye, that works.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scratch that, no - we're saying > 1 because we want to exclude paths which start with /, which isRootedDiskPath would include.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since if the root is a filesystem rooted at /, we can just use / as the common dir.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but apparently we cannot do this since commonSourceDirectory is "". Just trying to understand in what cases using isRootedDiskPath will yield incorrect results.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct - in effect, we want to allow "" as a valid common source directory provided either all paths are relative or all paths are rooted to a unix-style drive (with a root of /), otherwise "" is the return value when there is no available common path and we should error in its presence.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discussed offline

programDiagnostics.add(createCompilerDiagnostic(Diagnostics.Cannot_find_the_common_subdirectory_path_for_the_input_files));
}
}

if (commonSourceDirectory && commonSourceDirectory[commonSourceDirectory.length - 1] !== directorySeparator) {
Expand Down
30 changes: 12 additions & 18 deletions src/harness/compilerRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,18 @@ class CompilerBaselineRunner extends RunnerBase {
this.basePath += "/" + this.testSuiteName;
}

private makeUnitName(name: string, root: string) {
return ts.isRootedDiskPath(name) ? name : ts.combinePaths(root, name);
};

public checkTestCodeOutput(fileName: string) {
describe("compiler tests for " + fileName, () => {
// Mocha holds onto the closure environment of the describe callback even after the test is done.
// Everything declared here should be cleared out in the "after" callback.
let justName: string;
let content: string;
let testCaseContent: { settings: Harness.TestCaseParser.CompilerSettings; testUnitData: Harness.TestCaseParser.TestUnitData[]; };

let units: Harness.TestCaseParser.TestUnitData[];
let harnessSettings: Harness.TestCaseParser.CompilerSettings;

let lastUnit: Harness.TestCaseParser.TestUnitData;
let rootDir: string;
let harnessSettings: Harness.TestCaseParser.CompilerSettings;

let result: Harness.Compiler.CompilerResult;
let options: ts.CompilerOptions;
Expand All @@ -63,28 +62,28 @@ class CompilerBaselineRunner extends RunnerBase {

before(() => {
justName = fileName.replace(/^.*[\\\/]/, ""); // strips the fileName from the path.
content = Harness.IO.readFile(fileName);
testCaseContent = Harness.TestCaseParser.makeUnitsFromTest(content, fileName);
units = testCaseContent.testUnitData;
const content = Harness.IO.readFile(fileName);
const testCaseContent = Harness.TestCaseParser.makeUnitsFromTest(content, fileName);
const units = testCaseContent.testUnitData;
harnessSettings = testCaseContent.settings;
lastUnit = units[units.length - 1];
rootDir = lastUnit.originalFilePath.indexOf("conformance") === -1 ? "tests/cases/compiler/" : lastUnit.originalFilePath.substring(0, lastUnit.originalFilePath.lastIndexOf("/")) + "/";
const rootDir = lastUnit.originalFilePath.indexOf("conformance") === -1 ? "tests/cases/compiler/" : lastUnit.originalFilePath.substring(0, lastUnit.originalFilePath.lastIndexOf("/")) + "/";
// We need to assemble the list of input files for the compiler and other related files on the 'filesystem' (ie in a multi-file test)
// If the last file in a test uses require or a triple slash reference we'll assume all other files will be brought in via references,
// otherwise, assume all files are just meant to be in the same compilation session without explicit references to one another.
toBeCompiled = [];
otherFiles = [];
if (/require\(/.test(lastUnit.content) || /reference\spath/.test(lastUnit.content)) {
toBeCompiled.push({ unitName: ts.combinePaths(rootDir, lastUnit.name), content: lastUnit.content });
toBeCompiled.push({ unitName: this.makeUnitName(lastUnit.name, rootDir), content: lastUnit.content });
units.forEach(unit => {
if (unit.name !== lastUnit.name) {
otherFiles.push({ unitName: ts.combinePaths(rootDir, unit.name), content: unit.content });
otherFiles.push({ unitName: this.makeUnitName(unit.name, rootDir), content: unit.content });
}
});
}
else {
toBeCompiled = units.map(unit => {
return { unitName: ts.combinePaths(rootDir, unit.name), content: unit.content };
return { unitName: this.makeUnitName(unit.name, rootDir), content: unit.content };
});
}

Expand All @@ -99,12 +98,7 @@ class CompilerBaselineRunner extends RunnerBase {
// Mocha holds onto the closure environment of the describe callback even after the test is done.
// Therefore we have to clean out large objects after the test is done.
justName = undefined;
content = undefined;
testCaseContent = undefined;
units = undefined;
harnessSettings = undefined;
lastUnit = undefined;
rootDir = undefined;
result = undefined;
options = undefined;
toBeCompiled = undefined;
Expand Down
13 changes: 13 additions & 0 deletions tests/baselines/reference/commonSourceDir1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//// [tests/cases/compiler/commonSourceDir1.ts] ////

//// [bar.ts]
var x: number;

//// [baz.ts]
var y: number;


//// [bar.js]
var x;
//// [baz.js]
var y;
8 changes: 8 additions & 0 deletions tests/baselines/reference/commonSourceDir1.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
=== A:/foo/bar.ts ===
var x: number;
>x : Symbol(x, Decl(bar.ts, 0, 3))

=== A:/foo/baz.ts ===
var y: number;
>y : Symbol(y, Decl(baz.ts, 0, 3))

8 changes: 8 additions & 0 deletions tests/baselines/reference/commonSourceDir1.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
=== A:/foo/bar.ts ===
var x: number;
>x : number

=== A:/foo/baz.ts ===
var y: number;
>y : number

9 changes: 9 additions & 0 deletions tests/baselines/reference/commonSourceDir2.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error TS5009: Cannot find the common subdirectory path for the input files.


!!! error TS5009: Cannot find the common subdirectory path for the input files.
==== A:/foo/bar.ts (0 errors) ====
var x: number;

==== B:/foo/baz.ts (0 errors) ====
var y: number;
12 changes: 12 additions & 0 deletions tests/baselines/reference/commonSourceDir2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//// [tests/cases/compiler/commonSourceDir2.ts] ////

//// [bar.ts]
var x: number;

//// [baz.ts]
var y: number;

//// [bar.js]
var x;
//// [baz.js]
var y;
12 changes: 12 additions & 0 deletions tests/baselines/reference/commonSourceDir3.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//// [tests/cases/compiler/commonSourceDir3.ts] ////

//// [bar.ts]
var x: number;

//// [baz.ts]
var y: number;

//// [bar.js]
var x;
//// [baz.js]
var y;
8 changes: 8 additions & 0 deletions tests/baselines/reference/commonSourceDir3.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
=== A:/foo/bar.ts ===
var x: number;
>x : Symbol(x, Decl(bar.ts, 0, 3))

=== a:/foo/baz.ts ===
var y: number;
>y : Symbol(y, Decl(baz.ts, 0, 3))

8 changes: 8 additions & 0 deletions tests/baselines/reference/commonSourceDir3.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
=== A:/foo/bar.ts ===
var x: number;
>x : number

=== a:/foo/baz.ts ===
var y: number;
>y : number

9 changes: 9 additions & 0 deletions tests/baselines/reference/commonSourceDir4.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error TS5009: Cannot find the common subdirectory path for the input files.


!!! error TS5009: Cannot find the common subdirectory path for the input files.
==== A:/foo/bar.ts (0 errors) ====
var x: number;

==== a:/foo/baz.ts (0 errors) ====
var y: number;
12 changes: 12 additions & 0 deletions tests/baselines/reference/commonSourceDir4.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//// [tests/cases/compiler/commonSourceDir4.ts] ////

//// [bar.ts]
var x: number;

//// [baz.ts]
var y: number;

//// [bar.js]
var x;
//// [baz.js]
var y;
6 changes: 6 additions & 0 deletions tests/cases/compiler/commonSourceDir1.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// @outDir: A:/
// @Filename: A:/foo/bar.ts
var x: number;

// @Filename: A:/foo/baz.ts
var y: number;
6 changes: 6 additions & 0 deletions tests/cases/compiler/commonSourceDir2.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// @outDir: A:/
// @Filename: A:/foo/bar.ts
var x: number;

// @Filename: B:/foo/baz.ts
var y: number;
7 changes: 7 additions & 0 deletions tests/cases/compiler/commonSourceDir3.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// @useCaseSensitiveFileNames: false
// @outDir: A:/
// @Filename: A:/foo/bar.ts
var x: number;

// @Filename: a:/foo/baz.ts
var y: number;
7 changes: 7 additions & 0 deletions tests/cases/compiler/commonSourceDir4.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// @useCaseSensitiveFileNames: true
// @outDir: A:/
// @Filename: A:/foo/bar.ts
var x: number;

// @Filename: a:/foo/baz.ts
var y: number;