Skip to content

Fix the relative path reference resolution #1131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Nov 20, 2014
4 changes: 4 additions & 0 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,10 @@ module ts {
return normalizedPathComponents(path, rootLength);
}

export function getNormalizedAbsolutePath(filename: string, currentDirectory: string) {
return getNormalizedPathFromPathComponents(getNormalizedPathComponents(filename, currentDirectory));
}

export function getNormalizedPathFromPathComponents(pathComponents: string[]) {
if (pathComponents && pathComponents.length) {
return pathComponents[0] + pathComponents.slice(1).join(directorySeparator);
Expand Down
11 changes: 3 additions & 8 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ module ts {
var newLine = program.getCompilerHost().getNewLine();

function getSourceFilePathInNewDir(newDirPath: string, sourceFile: SourceFile) {
var sourceFilePath = getNormalizedPathFromPathComponents(getNormalizedPathComponents(sourceFile.filename, compilerHost.getCurrentDirectory()));
var sourceFilePath = getNormalizedAbsolutePath(sourceFile.filename, compilerHost.getCurrentDirectory());
sourceFilePath = sourceFilePath.replace(program.getCommonSourceDirectory(), "");
return combinePaths(newDirPath, sourceFilePath);
}
Expand Down Expand Up @@ -3414,11 +3414,6 @@ module ts {
}
}

function tryResolveScriptReference(sourceFile: SourceFile, reference: FileReference) {
var referenceFileName = normalizePath(combinePaths(getDirectoryPath(sourceFile.filename), reference.filename));
return program.getSourceFile(referenceFileName);
}

// Contains the reference paths that needs to go in the declaration file.
// Collecting this separately because reference paths need to be first thing in the declaration file
// and we could be collecting these paths from multiple files into single one with --out option
Expand All @@ -3445,7 +3440,7 @@ module ts {
if (!compilerOptions.noResolve) {
var addedGlobalFileReference = false;
forEach(root.referencedFiles, fileReference => {
var referencedFile = tryResolveScriptReference(root, fileReference);
var referencedFile = tryResolveScriptReference(program, root, fileReference);

// All the references that are not going to be part of same file
if (referencedFile && ((referencedFile.flags & NodeFlags.DeclarationFile) || // This is a declare file reference
Expand All @@ -3470,7 +3465,7 @@ module ts {
// Check what references need to be added
if (!compilerOptions.noResolve) {
forEach(sourceFile.referencedFiles, fileReference => {
var referencedFile = tryResolveScriptReference(sourceFile, fileReference);
var referencedFile = tryResolveScriptReference(program, sourceFile, fileReference);

// If the reference file is a declaration file or an external module, emit that reference
if (referencedFile && (isExternalModuleOrDeclarationFile(referencedFile) &&
Expand Down
36 changes: 31 additions & 5 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,14 @@ module ts {
return false;
}

export function tryResolveScriptReference(program: Program, sourceFile: SourceFile, reference: FileReference) {
if (!program.getCompilerOptions().noResolve) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not understand why noResolve is of any consequence here. we will come here because we decided to resolve references, correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we just need to compute an absolute path for every file for comparison purposes regardless of --noresolve, just do not make it the main name of the file so that error messages still use the name the user inputed.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mhegazy don't we say if --noResolve is given do not resolve file names on the disk but use whatever is given in to the compiler, so why should we be resolving to full name in case of --noResolve

Copy link
Contributor

Choose a reason for hiding this comment

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

--noResolve=true means do not resolve /// references. the path resolution is more of a compiler implementation detail.

var referenceFileName = isRootedDiskPath(reference.filename) ? reference.filename : combinePaths(getDirectoryPath(sourceFile.filename), reference.filename);
referenceFileName = getNormalizedAbsolutePath(referenceFileName, program.getCompilerHost().getCurrentDirectory());
return program.getSourceFile(referenceFileName);
}
}

export function getAncestor(node: Node, kind: SyntaxKind): Node {
switch (kind) {
// special-cases that can be come first
Expand Down Expand Up @@ -4416,20 +4424,26 @@ module ts {
var canonicalName = host.getCanonicalFileName(filename);
if (hasProperty(filesByName, canonicalName)) {
// We've already looked for this file, use cached result
var file = filesByName[canonicalName];
if (file && host.useCaseSensitiveFileNames() && canonicalName !== file.filename) {
errors.push(createFileDiagnostic(refFile, refStart, refLength,
Diagnostics.Filename_0_differs_from_already_included_filename_1_only_in_casing, filename, file.filename));
}
return getSourceFileFromCache(filename, canonicalName, /*useAbsolutePath*/ false);
}
else {
var normalizedAbsolutePath = getNormalizedAbsolutePath(filename, host.getCurrentDirectory());
var canonicalAbsolutePath = host.getCanonicalFileName(normalizedAbsolutePath);
if (hasProperty(filesByName, canonicalAbsolutePath)) {
return getSourceFileFromCache(normalizedAbsolutePath, canonicalAbsolutePath, /*useAbsolutePath*/ true);
}

// We haven't looked for this file, do so now and cache result
var file = filesByName[canonicalName] = host.getSourceFile(filename, options.target, hostErrorMessage => {
errors.push(createFileDiagnostic(refFile, refStart, refLength,
Diagnostics.Cannot_read_file_0_Colon_1, filename, hostErrorMessage));
});
if (file) {
seenNoDefaultLib = seenNoDefaultLib || file.hasNoDefaultLib;

// Set the source file for normalized absolute path
filesByName[canonicalAbsolutePath] = file;

if (!options.noResolve) {
var basePath = getDirectoryPath(filename);
processReferencedFiles(file, basePath);
Expand All @@ -4447,6 +4461,18 @@ module ts {
}
}
return file;

function getSourceFileFromCache(filename: string, canonicalName: string, useAbsolutePath: boolean): SourceFile {
var file = filesByName[canonicalName];
if (file && host.useCaseSensitiveFileNames()) {
var sourceFileName = useAbsolutePath ? getNormalizedAbsolutePath(file.filename, host.getCurrentDirectory()) : file.filename;
if (canonicalName !== sourceFileName) {
errors.push(createFileDiagnostic(refFile, refStart, refLength,
Diagnostics.Filename_0_differs_from_already_included_filename_1_only_in_casing, filename, sourceFileName));
}
}
return file;
}
}

function processReferencedFiles(file: SourceFile, basePath: string) {
Expand Down
2 changes: 1 addition & 1 deletion src/harness/harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,7 @@ module Harness {
var sourceFileName: string;
if (ts.isExternalModule(sourceFile) || !options.out) {
if (options.outDir) {
var sourceFilePath = ts.getNormalizedPathFromPathComponents(ts.getNormalizedPathComponents(sourceFile.filename, result.currentDirectoryForProgram));
var sourceFilePath = ts.getNormalizedAbsolutePath(sourceFile.filename, result.currentDirectoryForProgram);
sourceFilePath = sourceFilePath.replace(result.program.getCommonSourceDirectory(), "");
sourceFileName = ts.combinePaths(options.outDir, sourceFilePath);
}
Expand Down
42 changes: 34 additions & 8 deletions src/harness/projectsRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ interface ProjectRunnerTestCase {
baselineCheck?: boolean; // Verify the baselines of output files, if this is false, we will write to output to the disk but there is no verification of baselines
runTest?: boolean; // Run the resulting test
bug?: string; // If there is any bug associated with this test case
noResolve?: boolean;
}

interface ProjectRunnerTestCaseResolutionInfo extends ProjectRunnerTestCase {
Expand Down Expand Up @@ -162,7 +163,8 @@ class ProjectRunner extends RunnerBase {
outDir: testCase.outDir,
mapRoot: testCase.resolveMapRoot && testCase.mapRoot ? sys.resolvePath(testCase.mapRoot) : testCase.mapRoot,
sourceRoot: testCase.resolveSourceRoot && testCase.sourceRoot ? sys.resolvePath(testCase.sourceRoot) : testCase.sourceRoot,
module: moduleKind
module: moduleKind,
noResolve: testCase.noResolve
};
}

Expand Down Expand Up @@ -272,16 +274,40 @@ class ProjectRunner extends RunnerBase {
}

function compileCompileDTsFiles(compilerResult: BatchCompileProjectTestCaseResult) {
var inputDtsSourceFiles = ts.map(ts.filter(compilerResult.program.getSourceFiles(),
sourceFile => Harness.Compiler.isDTS(sourceFile.filename)),
sourceFile => {
return { emittedFileName: sourceFile.filename, code: sourceFile.text };
});
var allInputFiles: { emittedFileName: string; code: string; }[] = [];
var compilerOptions = compilerResult.program.getCompilerOptions();
var compilerHost = compilerResult.program.getCompilerHost();
ts.forEach(compilerResult.program.getSourceFiles(), sourceFile => {
if (Harness.Compiler.isDTS(sourceFile.filename)) {
allInputFiles.unshift({ emittedFileName: sourceFile.filename, code: sourceFile.text });
}
else if (ts.shouldEmitToOwnFile(sourceFile, compilerResult.program.getCompilerOptions())) {
if (compilerOptions.outDir) {
var sourceFilePath = ts.getNormalizedAbsolutePath(sourceFile.filename, compilerHost.getCurrentDirectory());
sourceFilePath = sourceFilePath.replace(compilerResult.program.getCommonSourceDirectory(), "");
var emitOutputFilePathWithoutExtension = ts.removeFileExtension(ts.combinePaths(compilerOptions.outDir, sourceFilePath));
}
else {
var emitOutputFilePathWithoutExtension = ts.removeFileExtension(sourceFile.filename);
}

var outputDtsFileName = emitOutputFilePathWithoutExtension + ".d.ts";
allInputFiles.unshift(findOutpuDtsFile(outputDtsFileName));
}
else {
var outputDtsFileName = ts.removeFileExtension(compilerOptions.out) + ".d.ts";
var outputDtsFile = findOutpuDtsFile(outputDtsFileName);
if (!ts.contains(allInputFiles, outputDtsFile)) {
allInputFiles.unshift(outputDtsFile);
}
}
});

var ouputDtsFiles = ts.filter(compilerResult.outputFiles, ouputFile => Harness.Compiler.isDTS(ouputFile.emittedFileName));
var allInputFiles = inputDtsSourceFiles.concat(ouputDtsFiles);
return compileProjectFiles(compilerResult.moduleKind,getInputFiles, getSourceFileText, writeFile);

function findOutpuDtsFile(fileName: string) {
return ts.forEach(compilerResult.outputFiles, outputFile => outputFile.emittedFileName === fileName ? outputFile : undefined);
}
function getInputFiles() {
return ts.map(allInputFiles, outputFile => outputFile.emittedFileName);
}
Expand Down
7 changes: 3 additions & 4 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3326,11 +3326,10 @@ module ts {
/// Triple slash reference comments
var comment = forEach(sourceFile.referencedFiles, r => (r.pos <= position && position < r.end) ? r : undefined);
if (comment) {
var targetFilename = isRootedDiskPath(comment.filename) ? comment.filename : combinePaths(getDirectoryPath(filename), comment.filename);
targetFilename = normalizePath(targetFilename);
if (program.getSourceFile(targetFilename)) {
var referenceFile = tryResolveScriptReference(program, sourceFile, comment);
if (referenceFile) {
return [{
fileName: targetFilename,
fileName: referenceFile.filename,
textSpan: TextSpan.fromBounds(0, 0),
kind: ScriptElementKind.scriptElement,
name: comment.filename,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/// <reference path="../src/ts/foo/foo.ts" />
// This is bar.ts
var bar = (function () {
function bar() {
}
return bar;
})();
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
/// <reference path="../src/ts/foo/foo.d.ts" />
declare class bar {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
/// <reference path="../../../bar/bar.d.ts" />
declare class foo {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/// <reference path="../../../bar/bar.ts" />
var foo = (function () {
function foo() {
}
return foo;
})();
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"scenario": "referenceResolution1_FromFooFolder",
"projectRoot": "tests/cases/projects/ReferenceResolution/src/ts/foo",
"inputFiles": [
"foo.ts"
],
"declaration": true,
"baselineCheck": true,
"resolvedInputFiles": [
"lib.d.ts",
"../../../bar/bar.ts",
"foo.ts"
],
"emittedFiles": [
"../../../bar/bar.js",
"../../../bar/bar.d.ts",
"foo.js",
"foo.d.ts"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/// <reference path="../src/ts/foo/foo.ts" />
// This is bar.ts
var bar = (function () {
function bar() {
}
return bar;
})();
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
/// <reference path="../src/ts/foo/foo.d.ts" />
declare class bar {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
/// <reference path="../../../bar/bar.d.ts" />
declare class foo {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/// <reference path="../../../bar/bar.ts" />
var foo = (function () {
function foo() {
}
return foo;
})();
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"scenario": "referenceResolution1_FromFooFolder",
"projectRoot": "tests/cases/projects/ReferenceResolution/src/ts/foo",
"inputFiles": [
"foo.ts"
],
"declaration": true,
"baselineCheck": true,
"resolvedInputFiles": [
"lib.d.ts",
"../../../bar/bar.ts",
"foo.ts"
],
"emittedFiles": [
"../../../bar/bar.js",
"../../../bar/bar.d.ts",
"foo.js",
"foo.d.ts"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
/// <reference path="../src/ts/foo/foo.d.ts" />
declare class bar {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/// <reference path="../src/ts/foo/foo.ts" />
// This is bar.ts
var bar = (function () {
function bar() {
}
return bar;
})();
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"scenario": "referenceResolution1_FromRootDirectory",
"projectRoot": "tests/cases/projects/ReferenceResolution",
"inputFiles": [
"src/ts/foo/foo.ts"
],
"declaration": true,
"baselineCheck": true,
"resolvedInputFiles": [
"lib.d.ts",
"bar/bar.ts",
"src/ts/foo/foo.ts"
],
"emittedFiles": [
"bar/bar.js",
"bar/bar.d.ts",
"src/ts/foo/foo.js",
"src/ts/foo/foo.d.ts"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
/// <reference path="../../../bar/bar.d.ts" />
declare class foo {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/// <reference path="../../../bar/bar.ts" />
var foo = (function () {
function foo() {
}
return foo;
})();
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
/// <reference path="../src/ts/foo/foo.d.ts" />
declare class bar {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/// <reference path="../src/ts/foo/foo.ts" />
// This is bar.ts
var bar = (function () {
function bar() {
}
return bar;
})();
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"scenario": "referenceResolution1_FromRootDirectory",
"projectRoot": "tests/cases/projects/ReferenceResolution",
"inputFiles": [
"src/ts/foo/foo.ts"
],
"declaration": true,
"baselineCheck": true,
"resolvedInputFiles": [
"lib.d.ts",
"bar/bar.ts",
"src/ts/foo/foo.ts"
],
"emittedFiles": [
"bar/bar.js",
"bar/bar.d.ts",
"src/ts/foo/foo.js",
"src/ts/foo/foo.d.ts"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
/// <reference path="../../../bar/bar.d.ts" />
declare class foo {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/// <reference path="../../../bar/bar.ts" />
var foo = (function () {
function foo() {
}
return foo;
})();
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/// <reference path="../src/ts/foo/foo.ts" />
// This is bar.ts
var bar = (function () {
function bar() {
}
return bar;
})();
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
declare class bar {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
declare class foo {
}
Loading