Use more nodelike paths for import types when possible (microsoft#24610)

weswigham · web-flow · commit d9b93903c035 · 2018-06-05T12:54:36.000-07:00
* Use more nodelike paths for import types when possible

* move functionality from services into compiler, fix with propert file/directory conflict handling

* mark suspect cast
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -4084,7 +4084,8 @@ namespace ts {
                         // ambient module, just use declaration/symbol name (fallthrough)
                     }
                     else {
-                        return `"${getResolvedExternalModuleName(context!.tracker.moduleResolverHost!, file, getSourceFileOfNode(getOriginalNode(context!.enclosingDeclaration)))}"`;
+                        const contextFile = getSourceFileOfNode(getOriginalNode(context!.enclosingDeclaration))!;
+                        return `"${file.moduleName || moduleSpecifiers.getModuleSpecifier(compilerOptions, contextFile, contextFile.path, file.path, context!.tracker.moduleResolverHost!)}"`;
                     }
                 }
                 const declaration = symbol.declarations[0];
diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts
@@ -132,10 +132,6 @@ namespace ts {
         }
     }
 
-    export interface GetEffectiveTypeRootsHost {
-        directoryExists?(directoryName: string): boolean;
-        getCurrentDirectory?(): string;
-    }
     export function getEffectiveTypeRoots(options: CompilerOptions, host: GetEffectiveTypeRootsHost): string[] | undefined {
         if (options.typeRoots) {
             return options.typeRoots;
diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts
@@ -1,10 +1,13 @@
 // Used by importFixes to synthesize import module specifiers.
 /* @internal */
 namespace ts.moduleSpecifiers {
+    export interface ModuleSpecifierPreferences {
+        importModuleSpecifierPreference?: "relative" | "non-relative";
+    }
+
     // Note: fromSourceFile is just for usesJsExtensionOnImports
-    export function getModuleSpecifier(program: Program, fromSourceFile: SourceFile, fromSourceFileName: string, toFileName: string, host: LanguageServiceHost, preferences: UserPreferences) {
-        const info = getInfo(program.getCompilerOptions(), fromSourceFile, fromSourceFileName, host);
-        const compilerOptions = program.getCompilerOptions();
+    export function getModuleSpecifier(compilerOptions: CompilerOptions, fromSourceFile: SourceFile, fromSourceFileName: string, toFileName: string, host: ModuleSpecifierResolutionHost, preferences: ModuleSpecifierPreferences = {}) {
+        const info = getInfo(compilerOptions, fromSourceFile, fromSourceFileName, host);
         return getGlobalModuleSpecifier(toFileName, info, host, compilerOptions) ||
             first(getLocalModuleSpecifiers(toFileName, info, compilerOptions, preferences));
     }
@@ -14,15 +17,15 @@ namespace ts.moduleSpecifiers {
         moduleSymbol: Symbol,
         program: Program,
         importingSourceFile: SourceFile,
-        host: LanguageServiceHost,
-        preferences: UserPreferences,
+        host: ModuleSpecifierResolutionHost,
+        preferences: ModuleSpecifierPreferences,
     ): ReadonlyArray<ReadonlyArray<string>> {
         const ambient = tryGetModuleNameFromAmbientModule(moduleSymbol);
         if (ambient) return [[ambient]];
 
         const compilerOptions = program.getCompilerOptions();
         const info = getInfo(compilerOptions, importingSourceFile, importingSourceFile.fileName, host);
-        const modulePaths = getAllModulePaths(program, moduleSymbol.valueDeclaration.getSourceFile());
+        const modulePaths = getAllModulePaths(program, getSourceFileOfNode(moduleSymbol.valueDeclaration));
 
         const global = mapDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions));
         return global.length ? global.map(g => [g]) : modulePaths.map(moduleFileName =>
@@ -36,18 +39,18 @@ namespace ts.moduleSpecifiers {
         readonly sourceDirectory: string;
     }
     // importingSourceFileName is separate because getEditsForFileRename may need to specify an updated path
-    function getInfo(compilerOptions: CompilerOptions, importingSourceFile: SourceFile, importingSourceFileName: string, host: LanguageServiceHost): Info {
+    function getInfo(compilerOptions: CompilerOptions, importingSourceFile: SourceFile, importingSourceFileName: string, host: ModuleSpecifierResolutionHost): Info {
         const moduleResolutionKind = getEmitModuleResolutionKind(compilerOptions);
         const addJsExtension = usesJsExtensionOnImports(importingSourceFile);
-        const getCanonicalFileName = hostGetCanonicalFileName(host);
+        const getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames ? host.useCaseSensitiveFileNames() : true);
         const sourceDirectory = getDirectoryPath(importingSourceFileName);
         return { moduleResolutionKind, addJsExtension, getCanonicalFileName, sourceDirectory };
     }
 
     function getGlobalModuleSpecifier(
         moduleFileName: string,
         { addJsExtension, getCanonicalFileName, sourceDirectory }: Info,
-        host: LanguageServiceHost,
+        host: ModuleSpecifierResolutionHost,
         compilerOptions: CompilerOptions,
     ) {
         return tryGetModuleNameFromTypeRoots(compilerOptions, host, getCanonicalFileName, moduleFileName, addJsExtension)
@@ -59,7 +62,7 @@ namespace ts.moduleSpecifiers {
         moduleFileName: string,
         { moduleResolutionKind, addJsExtension, getCanonicalFileName, sourceDirectory }: Info,
         compilerOptions: CompilerOptions,
-        preferences: UserPreferences,
+        preferences: ModuleSpecifierPreferences,
     ) {
         const { baseUrl, paths } = compilerOptions;
 
@@ -210,7 +213,7 @@ namespace ts.moduleSpecifiers {
     function tryGetModuleNameAsNodeModule(
         options: CompilerOptions,
         moduleFileName: string,
-        host: LanguageServiceHost,
+        host: ModuleSpecifierResolutionHost,
         getCanonicalFileName: (file: string) => string,
         sourceDirectory: string,
     ): string | undefined {
@@ -256,14 +259,26 @@ namespace ts.moduleSpecifiers {
             const fullModulePathWithoutExtension = removeFileExtension(path);
 
             // If the file is /index, it can be imported by its directory name
-            if (getCanonicalFileName(fullModulePathWithoutExtension.substring(parts.fileNameIndex)) === "/index") {
+            // IFF there is not _also_ a file by the same name
+            if (getCanonicalFileName(fullModulePathWithoutExtension.substring(parts.fileNameIndex)) === "/index" && !tryGetAnyFileFromPath(host, fullModulePathWithoutExtension.substring(0, parts.fileNameIndex))) {
                 return fullModulePathWithoutExtension.substring(0, parts.fileNameIndex);
             }
 
             return fullModulePathWithoutExtension;
         }
     }
 
+    function tryGetAnyFileFromPath(host: ModuleSpecifierResolutionHost, path: string) {
+        // We check all js, `node` and `json` extensions in addition to TS, since node module resolution would also choose those over the directory
+        const extensions = getSupportedExtensions({ allowJs: true }, [{ extension: "node", isMixedContent: false }, { extension: "json", isMixedContent: false, scriptKind: ScriptKind.JSON }]);
+        for (const e of extensions) {
+            const fullPath = path + e;
+            if (host.fileExists!(fullPath)) { // TODO: GH#18217
+                return fullPath;
+            }
+        }
+    }
+
     interface NodeModulePathParts {
         readonly topLevelNodeModulesIndex: number;
         readonly topLevelPackageNameIndex: number;
diff --git a/src/compiler/program.ts b/src/compiler/program.ts
@@ -1207,6 +1207,9 @@ namespace ts {
                 writeFile: writeFileCallback || (
                     (fileName, data, writeByteOrderMark, onError, sourceFiles) => host.writeFile(fileName, data, writeByteOrderMark, onError, sourceFiles)),
                 isEmitBlocked,
+                readFile: f => host.readFile(f),
+                fileExists: f => host.fileExists(f),
+                ...(host.directoryExists ? { directoryExists: f => host.directoryExists!(f) } : {}),
             };
         }
 
diff --git a/src/compiler/tsconfig.json b/src/compiler/tsconfig.json
@@ -44,6 +44,7 @@
         "builderState.ts",
         "builder.ts",
         "resolutionCache.ts",
+        "moduleSpecifiers.ts",
         "watch.ts",
         "commandLineParser.ts",
         "tsc.ts"
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -5026,8 +5026,9 @@ namespace ts {
     }
 
     /* @internal */
-    export interface EmitHost extends ScriptReferenceHost {
+    export interface EmitHost extends ScriptReferenceHost, ModuleSpecifierResolutionHost {
         getSourceFiles(): ReadonlyArray<SourceFile>;
+        getCurrentDirectory(): string;
 
         /* @internal */
         isSourceFileFromExternalLibrary(file: SourceFile): boolean;
@@ -5289,11 +5290,15 @@ namespace ts {
         isAtStartOfLine(): boolean;
     }
 
-    /* @internal */
-    export interface ModuleNameResolverHost {
-        getCanonicalFileName(f: string): string;
-        getCommonSourceDirectory(): string;
-        getCurrentDirectory(): string;
+    export interface GetEffectiveTypeRootsHost {
+        directoryExists?(directoryName: string): boolean;
+        getCurrentDirectory?(): string;
+    }
+    /** @internal */
+    export interface ModuleSpecifierResolutionHost extends GetEffectiveTypeRootsHost {
+        useCaseSensitiveFileNames?(): boolean;
+        fileExists?(path: string): boolean;
+        readFile?(path: string): string | undefined;
     }
 
     /** @deprecated See comment on SymbolWriter */
@@ -5307,7 +5312,7 @@ namespace ts {
         reportPrivateInBaseOfClassExpression?(propertyName: string): void;
         reportInaccessibleUniqueSymbolError?(): void;
         /* @internal */
-        moduleResolverHost?: ModuleNameResolverHost;
+        moduleResolverHost?: ModuleSpecifierResolutionHost;
         /* @internal */
         trackReferencedAmbientModule?(decl: ModuleDeclaration): void;
     }
diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -2934,11 +2934,11 @@ namespace ts {
         };
     }
 
-    export function getResolvedExternalModuleName(host: ModuleNameResolverHost, file: SourceFile, referenceFile?: SourceFile): string {
+    export function getResolvedExternalModuleName(host: EmitHost, file: SourceFile, referenceFile?: SourceFile): string {
         return file.moduleName || getExternalModuleNameFromPath(host, file.fileName, referenceFile && referenceFile.fileName);
     }
 
-    export function getExternalModuleNameFromDeclaration(host: ModuleNameResolverHost, resolver: EmitResolver, declaration: ImportEqualsDeclaration | ImportDeclaration | ExportDeclaration | ModuleDeclaration | ImportTypeNode): string | undefined {
+    export function getExternalModuleNameFromDeclaration(host: EmitHost, resolver: EmitResolver, declaration: ImportEqualsDeclaration | ImportDeclaration | ExportDeclaration | ModuleDeclaration | ImportTypeNode): string | undefined {
         const file = resolver.getExternalModuleFileFromDeclaration(declaration);
         if (!file || file.isDeclarationFile) {
             return undefined;
@@ -2949,7 +2949,7 @@ namespace ts {
     /**
      * Resolves a local path to a path which is absolute to the base of the emit
      */
-    export function getExternalModuleNameFromPath(host: ModuleNameResolverHost, fileName: string, referencePath?: string): string {
+    export function getExternalModuleNameFromPath(host: EmitHost, fileName: string, referencePath?: string): string {
         const getCanonicalFileName = (f: string) => host.getCanonicalFileName(f);
         const dir = toPath(referencePath ? getDirectoryPath(referencePath) : host.getCommonSourceDirectory(), host.getCurrentDirectory(), getCanonicalFileName);
         const filePath = getNormalizedAbsolutePath(fileName, host.getCurrentDirectory());
diff --git a/src/harness/tsconfig.json b/src/harness/tsconfig.json
@@ -51,6 +51,7 @@
         "../compiler/builderState.ts",
         "../compiler/builder.ts",
         "../compiler/resolutionCache.ts",
+        "../compiler/moduleSpecifiers.ts",
         "../compiler/watch.ts",
         "../compiler/commandLineParser.ts",
 
@@ -115,7 +116,6 @@
         "../services/codefixes/inferFromUsage.ts",
         "../services/codefixes/fixInvalidImportSyntax.ts",
         "../services/codefixes/fixStrictClassInitialization.ts",
-        "../services/codefixes/moduleSpecifiers.ts",
         "../services/codefixes/requireInTs.ts",
         "../services/codefixes/useDefaultImport.ts",
         "../services/codefixes/fixAddModuleReferTypeMissingTypeof.ts",
diff --git a/src/server/tsconfig.json b/src/server/tsconfig.json
@@ -47,6 +47,7 @@
         "../compiler/builderState.ts",
         "../compiler/builder.ts",
         "../compiler/resolutionCache.ts",
+        "../compiler/moduleSpecifiers.ts",
         "../compiler/watch.ts",
         "../compiler/commandLineParser.ts",
 
@@ -111,7 +112,6 @@
         "../services/codefixes/inferFromUsage.ts",
         "../services/codefixes/fixInvalidImportSyntax.ts",
         "../services/codefixes/fixStrictClassInitialization.ts",
-        "../services/codefixes/moduleSpecifiers.ts",
         "../services/codefixes/requireInTs.ts",
         "../services/codefixes/useDefaultImport.ts",
         "../services/codefixes/fixAddModuleReferTypeMissingTypeof.ts",
diff --git a/src/server/tsconfig.library.json b/src/server/tsconfig.library.json
@@ -53,6 +53,7 @@
         "../compiler/builderState.ts",
         "../compiler/builder.ts",
         "../compiler/resolutionCache.ts",
+        "../compiler/moduleSpecifiers.ts",
         "../compiler/watch.ts",
         "../compiler/commandLineParser.ts",
 
@@ -117,7 +118,6 @@
         "../services/codefixes/inferFromUsage.ts",
         "../services/codefixes/fixInvalidImportSyntax.ts",
         "../services/codefixes/fixStrictClassInitialization.ts",
-        "../services/codefixes/moduleSpecifiers.ts",
         "../services/codefixes/requireInTs.ts",
         "../services/codefixes/useDefaultImport.ts",
         "../services/codefixes/fixAddModuleReferTypeMissingTypeof.ts",
diff --git a/src/services/getEditsForFileRename.ts b/src/services/getEditsForFileRename.ts
@@ -123,7 +123,7 @@ namespace ts {
                         // TODO:GH#18217
                         ? getSourceFileToImportFromResolved(resolveModuleName(importLiteral.text, oldImportFromPath, program.getCompilerOptions(), host as ModuleResolutionHost), oldToNew, program)
                         : getSourceFileToImport(importLiteral, sourceFile, program, host, oldToNew);
-                    return toImport === undefined ? undefined : moduleSpecifiers.getModuleSpecifier(program, sourceFile, newImportFromPath, toImport, host, preferences);
+                    return toImport === undefined ? undefined : moduleSpecifiers.getModuleSpecifier(program.getCompilerOptions(), sourceFile, newImportFromPath, toImport, host, preferences);
                 });
         }
     }
diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json
@@ -44,6 +44,7 @@
         "../compiler/builderState.ts",
         "../compiler/builder.ts",
         "../compiler/resolutionCache.ts",
+        "../compiler/moduleSpecifiers.ts",
         "../compiler/watch.ts",
         "../compiler/commandLineParser.ts",
 
@@ -108,7 +109,6 @@
         "codefixes/inferFromUsage.ts",
         "codefixes/fixInvalidImportSyntax.ts",
         "codefixes/fixStrictClassInitialization.ts",
-        "codefixes/moduleSpecifiers.ts",
         "codefixes/requireInTs.ts",
         "codefixes/useDefaultImport.ts",
         "codefixes/fixAddModuleReferTypeMissingTypeof.ts",
diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts
@@ -2881,6 +2881,10 @@ declare namespace ts {
         omitTrailingSemicolon?: boolean;
         noEmitHelpers?: boolean;
     }
+    interface GetEffectiveTypeRootsHost {
+        directoryExists?(directoryName: string): boolean;
+        getCurrentDirectory?(): string;
+    }
     /** @deprecated See comment on SymbolWriter */
     interface SymbolTracker {
         trackSymbol?(symbol: Symbol, enclosingDeclaration?: Node, meaning?: SymbolFlags): void;
@@ -3472,10 +3476,6 @@ declare namespace ts {
     function updateSourceFile(sourceFile: SourceFile, newText: string, textChangeRange: TextChangeRange, aggressiveChecks?: boolean): SourceFile;
 }
 declare namespace ts {
-    interface GetEffectiveTypeRootsHost {
-        directoryExists?(directoryName: string): boolean;
-        getCurrentDirectory?(): string;
-    }
     function getEffectiveTypeRoots(options: CompilerOptions, host: GetEffectiveTypeRootsHost): string[] | undefined;
     /**
      * @param {string | undefined} containingFile - file that contains type reference directive, can be undefined if containing file is unknown.
diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts
@@ -2881,6 +2881,10 @@ declare namespace ts {
         omitTrailingSemicolon?: boolean;
         noEmitHelpers?: boolean;
     }
+    interface GetEffectiveTypeRootsHost {
+        directoryExists?(directoryName: string): boolean;
+        getCurrentDirectory?(): string;
+    }
     /** @deprecated See comment on SymbolWriter */
     interface SymbolTracker {
         trackSymbol?(symbol: Symbol, enclosingDeclaration?: Node, meaning?: SymbolFlags): void;
@@ -3472,10 +3476,6 @@ declare namespace ts {
     function updateSourceFile(sourceFile: SourceFile, newText: string, textChangeRange: TextChangeRange, aggressiveChecks?: boolean): SourceFile;
 }
 declare namespace ts {
-    interface GetEffectiveTypeRootsHost {
-        directoryExists?(directoryName: string): boolean;
-        getCurrentDirectory?(): string;
-    }
     function getEffectiveTypeRoots(options: CompilerOptions, host: GetEffectiveTypeRootsHost): string[] | undefined;
     /**
      * @param {string | undefined} containingFile - file that contains type reference directive, can be undefined if containing file is unknown.
diff --git a/tests/baselines/reference/declarationEmitCommonJsModuleReferencedType.js b/tests/baselines/reference/declarationEmitCommonJsModuleReferencedType.js
@@ -0,0 +1,38 @@
+//// [tests/cases/compiler/declarationEmitCommonJsModuleReferencedType.ts] ////
+
+//// [index.d.ts]
+export interface NestedProps {}
+//// [index.d.ts]
+export interface OtherIndexProps {}
+//// [other.d.ts]
+export interface OtherProps {}
+//// [index.d.ts]
+import { OtherProps } from "./other";
+import { OtherIndexProps } from "./other/index";
+import { NestedProps } from "nested";
+export interface SomeProps {}
+
+export function foo(): [SomeProps, OtherProps, OtherIndexProps, NestedProps];
+//// [index.d.ts]
+export interface RootProps {}
+
+export function bar(): RootProps;
+//// [entry.ts]
+import { foo } from "foo";
+import { bar } from "root";
+export const x = foo();
+export const y = bar();
+
+
+//// [entry.js]
+"use strict";
+exports.__esModule = true;
+var foo_1 = require("foo");
+var root_1 = require("root");
+exports.x = foo_1.foo();
+exports.y = root_1.bar();
+
+
+//// [entry.d.ts]
+export declare const x: [import("foo").SomeProps, import("foo/other").OtherProps, import("foo/other/index").OtherIndexProps, import("foo/node_modules/nested").NestedProps];
+export declare const y: import("root").RootProps;
diff --git a/tests/baselines/reference/declarationEmitCommonJsModuleReferencedType.symbols b/tests/baselines/reference/declarationEmitCommonJsModuleReferencedType.symbols
diff --git a/tests/baselines/reference/declarationEmitCommonJsModuleReferencedType.types b/tests/baselines/reference/declarationEmitCommonJsModuleReferencedType.types
diff --git a/tests/baselines/reference/importShouldNotBeElidedInDeclarationEmit.js b/tests/baselines/reference/importShouldNotBeElidedInDeclarationEmit.js
diff --git a/tests/cases/compiler/declarationEmitCommonJsModuleReferencedType.ts b/tests/cases/compiler/declarationEmitCommonJsModuleReferencedType.ts

Original file line number	Diff line number	Diff line change
`@@ -4084,7 +4084,8 @@ namespace ts {`
`4084`	`4084`	`// ambient module, just use declaration/symbol name (fallthrough)`
`4085`	`4085`	`}`
`4086`	`4086`	`else {`
`4087`		- return `"${getResolvedExternalModuleName(context!.tracker.moduleResolverHost!, file, getSourceFileOfNode(getOriginalNode(context!.enclosingDeclaration)))}"`;
	`4087`	`+ const contextFile = getSourceFileOfNode(getOriginalNode(context!.enclosingDeclaration))!;`
	`4088`	+ return `"${file.moduleName \|\| moduleSpecifiers.getModuleSpecifier(compilerOptions, contextFile, contextFile.path, file.path, context!.tracker.moduleResolverHost!)}"`;
`4088`	`4089`	`}`
`4089`	`4090`	`}`
`4090`	`4091`	`const declaration = symbol.declarations[0];`
Original file line number	Diff line number	Diff line change
`@@ -132,10 +132,6 @@ namespace ts {`
`132`	`132`	`}`
`133`	`133`	`}`
`134`	`134`
`135`		`- export interface GetEffectiveTypeRootsHost {`
`136`		`- directoryExists?(directoryName: string): boolean;`
`137`		`- getCurrentDirectory?(): string;`
`138`		`- }`
`139`	`135`	`export function getEffectiveTypeRoots(options: CompilerOptions, host: GetEffectiveTypeRootsHost): string[] \| undefined {`
`140`	`136`	`if (options.typeRoots) {`
`141`	`137`	`return options.typeRoots;`
Original file line number	Diff line number	Diff line change
`@@ -1207,6 +1207,9 @@ namespace ts {`
`1207`	`1207`	`writeFile: writeFileCallback \|\| (`
`1208`	`1208`	`(fileName, data, writeByteOrderMark, onError, sourceFiles) => host.writeFile(fileName, data, writeByteOrderMark, onError, sourceFiles)),`
`1209`	`1209`	`isEmitBlocked,`
	`1210`	`+ readFile: f => host.readFile(f),`
	`1211`	`+ fileExists: f => host.fileExists(f),`
	`1212`	`+ ...(host.directoryExists ? { directoryExists: f => host.directoryExists!(f) } : {}),`
`1210`	`1213`	`};`
`1211`	`1214`	`}`
`1212`	`1215`