Use originalFileName (fileName of input project reference file) to resolve module/typereferences/reference paths in it instead of output decl file path

sheetalkamat · sheetalkamat · commit d51b8d940c59 · 2018-09-17T19:00:30.000-07:00
This also ensures that originalFileName, resolvedPath are set correctly even when we are reusing program structure Fixes microsoft#26036
diff --git a/src/compiler/builderState.ts b/src/compiler/builderState.ts
@@ -88,7 +88,7 @@ namespace ts.BuilderState {
     function getReferencedFileFromImportedModuleSymbol(symbol: Symbol) {
         if (symbol.declarations && symbol.declarations[0]) {
             const declarationSourceFile = getSourceFileOfNode(symbol.declarations[0]);
-            return declarationSourceFile && (declarationSourceFile.resolvedPath || declarationSourceFile.path);
+            return declarationSourceFile && declarationSourceFile.resolvedPath;
         }
     }
 
diff --git a/src/compiler/program.ts b/src/compiler/program.ts
@@ -485,11 +485,11 @@ namespace ts {
 
         function sourceFileNotUptoDate(sourceFile: SourceFile) {
             return !sourceFileVersionUptoDate(sourceFile) ||
-                hasInvalidatedResolution(sourceFile.resolvedPath || sourceFile.path);
+                hasInvalidatedResolution(sourceFile.resolvedPath);
         }
 
         function sourceFileVersionUptoDate(sourceFile: SourceFile) {
-            return sourceFile.version === getSourceVersion(sourceFile.resolvedPath || sourceFile.path);
+            return sourceFile.version === getSourceVersion(sourceFile.resolvedPath);
         }
 
         function projectReferenceUptoDate(oldRef: ProjectReference, newRef: ProjectReference, index: number) {
@@ -1079,7 +1079,7 @@ namespace ts {
 
             for (const oldSourceFile of oldSourceFiles) {
                 let newSourceFile = host.getSourceFileByPath
-                    ? host.getSourceFileByPath(oldSourceFile.fileName, oldSourceFile.resolvedPath || oldSourceFile.path, options.target!, /*onError*/ undefined, shouldCreateNewSourceFile)
+                    ? host.getSourceFileByPath(oldSourceFile.fileName, oldSourceFile.resolvedPath, options.target!, /*onError*/ undefined, shouldCreateNewSourceFile)
                     : host.getSourceFile(oldSourceFile.fileName, options.target!, /*onError*/ undefined, shouldCreateNewSourceFile); // TODO: GH#18217
 
                 if (!newSourceFile) {
@@ -1110,7 +1110,11 @@ namespace ts {
                     fileChanged = newSourceFile !== oldSourceFile;
                 }
 
+                // Since the project references havent changed, its right to set originalFileName and resolvedPath here
                 newSourceFile.path = oldSourceFile.path;
+                newSourceFile.originalFileName = oldSourceFile.originalFileName;
+                newSourceFile.resolvedPath = oldSourceFile.resolvedPath;
+                newSourceFile.fileName = oldSourceFile.fileName;
                 filePaths.push(newSourceFile.path);
 
                 const packageName = oldProgram.sourceFileToPackageName.get(oldSourceFile.path);
@@ -1187,7 +1191,7 @@ namespace ts {
             modifiedFilePaths = modifiedSourceFiles.map(f => f.newFile.path);
             // try to verify results of module resolution
             for (const { oldFile: oldSourceFile, newFile: newSourceFile } of modifiedSourceFiles) {
-                const newSourceFilePath = getNormalizedAbsolutePath(newSourceFile.fileName, currentDirectory);
+                const newSourceFilePath = getNormalizedAbsolutePath(newSourceFile.originalFileName, currentDirectory);
                 if (resolveModuleNamesWorker) {
                     const moduleNames = getModuleNames(newSourceFile);
                     const oldProgramState: OldProgramState = { program: oldProgram, oldSourceFile, modifiedFilePaths };
@@ -2040,6 +2044,7 @@ namespace ts {
 
         // Get source file from normalized fileName
         function findSourceFile(fileName: string, path: Path, isDefaultLib: boolean, ignoreNoDefaultLib: boolean, refFile: SourceFile, refPos: number, refEnd: number, packageId: PackageId | undefined): SourceFile | undefined {
+            const originalFileName = fileName;
             if (filesByName.has(path)) {
                 const file = filesByName.get(path);
                 // try to check if we've already seen this file but with a different casing in path
@@ -2136,6 +2141,7 @@ namespace ts {
                 sourceFilesFoundSearchingNodeModules.set(path, currentNodeModulesDepth > 0);
                 file.path = path;
                 file.resolvedPath = toPath(fileName);
+                file.originalFileName = originalFileName;
 
                 if (host.useCaseSensitiveFileNames()) {
                     const pathLowerCase = path.toLowerCase();
@@ -2191,7 +2197,7 @@ namespace ts {
 
         function processReferencedFiles(file: SourceFile, isDefaultLib: boolean) {
             forEach(file.referencedFiles, ref => {
-                const referencedFileName = resolveTripleslashReference(ref.fileName, file.fileName);
+                const referencedFileName = resolveTripleslashReference(ref.fileName, file.originalFileName);
                 processSourceFile(referencedFileName, isDefaultLib, /*ignoreNoDefaultLib*/ false, /*packageId*/ undefined, file, ref.pos, ref.end);
             });
         }
@@ -2203,7 +2209,7 @@ namespace ts {
                 return;
             }
 
-            const resolutions = resolveTypeReferenceDirectiveNamesWorker(typeDirectives, file.fileName);
+            const resolutions = resolveTypeReferenceDirectiveNamesWorker(typeDirectives, file.originalFileName);
 
             for (let i = 0; i < typeDirectives.length; i++) {
                 const ref = file.typeReferenceDirectives[i];
@@ -2299,7 +2305,7 @@ namespace ts {
                 // Because global augmentation doesn't have string literal name, we can check for global augmentation as such.
                 const moduleNames = getModuleNames(file);
                 const oldProgramState: OldProgramState = { program: oldProgram, oldSourceFile: oldProgram && oldProgram.getSourceFile(file.fileName), modifiedFilePaths };
-                const resolutions = resolveModuleNamesReusingOldState(moduleNames, getNormalizedAbsolutePath(file.fileName, currentDirectory), file, oldProgramState);
+                const resolutions = resolveModuleNamesReusingOldState(moduleNames, getNormalizedAbsolutePath(file.originalFileName, currentDirectory), file, oldProgramState);
                 Debug.assert(resolutions.length === moduleNames.length);
                 for (let i = 0; i < moduleNames.length; i++) {
                     const resolution = resolutions[i];
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -2557,6 +2557,12 @@ namespace ts {
          * path = input file's path
          */
         /* @internal */ resolvedPath: Path;
+        /** Original file name that can be different from fileName,
+         * when file is included through project reference is mapped to its output instead of source
+         * in that case originalFileName = name of input file
+         * fileName = output file's name
+         */
+        /* @internal */ originalFileName: string;
 
         /**
          * If two source files are for the same version of the same package, one will redirect to the other.
diff --git a/src/server/project.ts b/src/server/project.ts
@@ -653,7 +653,7 @@ namespace ts.server {
                 return this.rootFiles;
             }
             return map(this.program.getSourceFiles(), sourceFile => {
-                const scriptInfo = this.projectService.getScriptInfoForPath(sourceFile.resolvedPath || sourceFile.path);
+                const scriptInfo = this.projectService.getScriptInfoForPath(sourceFile.resolvedPath);
                 Debug.assert(!!scriptInfo, "getScriptInfo", () => `scriptInfo for a file '${sourceFile.fileName}' Path: '${sourceFile.path}' / '${sourceFile.resolvedPath}' is missing.`);
                 return scriptInfo!;
             });
diff --git a/src/services/services.ts b/src/services/services.ts
@@ -541,6 +541,7 @@ namespace ts {
         public fileName: string;
         public path: Path;
         public resolvedPath: Path;
+        public originalFileName: string;
         public text: string;
         public scriptSnapshot: IScriptSnapshot;
         public lineMap: ReadonlyArray<number>;
diff --git a/src/testRunner/unittests/tsbuild.ts b/src/testRunner/unittests/tsbuild.ts
@@ -23,6 +23,42 @@ namespace ts {
                     assert(fs.existsSync(output), `Expect file ${output} to exist`);
                 }
             });
+
+            it("builds correctly when outDir is specified", () => {
+                const fs = projFs.shadow();
+                fs.writeFileSync("/src/logic/tsconfig.json", JSON.stringify({
+                    compilerOptions: { composite: true, declaration: true, sourceMap: true, outDir: "outDir" },
+                    references: [{ path: "../core" }]
+                }));
+
+                const host = new fakes.SolutionBuilderHost(fs);
+                const builder = createSolutionBuilder(host, ["/src/tests"], {});
+                builder.buildAllProjects();
+                host.assertDiagnosticMessages(/*empty*/);
+                const expectedOutputs = allExpectedOutputs.map(f => f.replace("/logic/", "/logic/outDir/"));
+                // Check for outputs. Not an exhaustive list
+                for (const output of expectedOutputs) {
+                    assert(fs.existsSync(output), `Expect file ${output} to exist`);
+                }
+            });
+
+            it("builds correctly when declarationDir is specified", () => {
+                const fs = projFs.shadow();
+                fs.writeFileSync("/src/logic/tsconfig.json", JSON.stringify({
+                    compilerOptions: { composite: true, declaration: true, sourceMap: true, declarationDir: "out/decls" },
+                    references: [{ path: "../core" }]
+                }));
+
+                const host = new fakes.SolutionBuilderHost(fs);
+                const builder = createSolutionBuilder(host, ["/src/tests"], {});
+                builder.buildAllProjects();
+                host.assertDiagnosticMessages(/*empty*/);
+                const expectedOutputs = allExpectedOutputs.map(f => f.replace("/logic/index.d.ts", "/logic/out/decls/index.d.ts"));
+                // Check for outputs. Not an exhaustive list
+                for (const output of expectedOutputs) {
+                    assert(fs.existsSync(output), `Expect file ${output} to exist`);
+                }
+            });
         });
 
         describe("tsbuild - dry builds", () => {
diff --git a/src/testRunner/unittests/tsbuildWatchMode.ts b/src/testRunner/unittests/tsbuildWatchMode.ts
@@ -486,11 +486,7 @@ export function gfoo() {
                     solutionBuilder.buildInvalidatedProject();
 
                     host.checkTimeoutQueueLengthAndRun(1);
-                    checkOutputErrorsIncremental(host, [
-                        // TODO: #26036
-                        // The error is reported in d.ts file because it isnt resolved from ts file path, but is resolved from .d.ts file
-                        "sample1/logic/decls/index.d.ts(2,22): error TS2307: Cannot find module '../core/anotherModule'.\n"
-                    ]);
+                    checkOutputErrorsIncremental(host, emptyArray);
                     checkProgramActualFiles(watch().getProgram(), [tests[1].path, libFile.path, coreIndexDts, coreAnotherModuleDts, projectFilePath(SubProject.logic, "decls/index.d.ts")]);
                 });
             });

Original file line number	Diff line number	Diff line change
`@@ -88,7 +88,7 @@ namespace ts.BuilderState {`
`88`	`88`	`function getReferencedFileFromImportedModuleSymbol(symbol: Symbol) {`
`89`	`89`	`if (symbol.declarations && symbol.declarations[0]) {`
`90`	`90`	`const declarationSourceFile = getSourceFileOfNode(symbol.declarations[0]);`
`91`		`- return declarationSourceFile && (declarationSourceFile.resolvedPath \|\| declarationSourceFile.path);`
	`91`	`+ return declarationSourceFile && declarationSourceFile.resolvedPath;`
`92`	`92`	`}`
`93`	`93`	`}`
`94`	`94`
Original file line number	Diff line number	Diff line change
`@@ -653,7 +653,7 @@ namespace ts.server {`
`653`	`653`	`return this.rootFiles;`
`654`	`654`	`}`
`655`	`655`	`return map(this.program.getSourceFiles(), sourceFile => {`
`656`		`- const scriptInfo = this.projectService.getScriptInfoForPath(sourceFile.resolvedPath \|\| sourceFile.path);`
	`656`	`+ const scriptInfo = this.projectService.getScriptInfoForPath(sourceFile.resolvedPath);`
`657`	`657`	Debug.assert(!!scriptInfo, "getScriptInfo", () => `scriptInfo for a file '${sourceFile.fileName}' Path: '${sourceFile.path}' / '${sourceFile.resolvedPath}' is missing.`);
`658`	`658`	`return scriptInfo!;`
`659`	`659`	`});`