diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 8b0e3cfacd3..d9c33f9aa3a 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -394,6 +394,14 @@ namespace ts { return result; } + export function mapIterator(iter: Iterator, mapFn: (x: T) => U): Iterator { + return { next }; + function next(): { value: U, done: false } | { value: never, done: true } { + const iterRes = iter.next(); + return iterRes.done ? iterRes : { value: mapFn(iterRes.value), done: false }; + } + } + // Maps from T to T and avoids allocation if all elements map to themselves export function sameMap(array: T[], f: (x: T, i: number) => T): T[]; export function sameMap(array: ReadonlyArray, f: (x: T, i: number) => T): ReadonlyArray; @@ -917,6 +925,36 @@ namespace ts { return array.slice().sort(comparer); } + export function best(iter: Iterator, isBetter: (a: T, b: T) => boolean): T | undefined { + const x = iter.next(); + if (x.done) { + return undefined; + } + let best = x.value; + while (true) { + const { value, done } = iter.next(); + if (done) { + return best; + } + if (isBetter(value, best)) { + best = value; + } + } + } + + export function arrayIterator(array: ReadonlyArray): Iterator { + let i = 0; + return { next: () => { + if (i === array.length) { + return { value: undefined as never, done: true }; + } + else { + i++; + return { value: array[i - 1], done: false }; + } + }}; + } + /** * Stable sort of an array. Elements equal to each other maintain their relative position in the array. */ @@ -1122,6 +1160,10 @@ namespace ts { return result; } + export function toArray(value: T | ReadonlyArray): ReadonlyArray { + return isArray(value) ? value : [value]; + } + /** * Calls `callback` for each entry in the map, returning the first truthy result. * Use `map.forEach` instead for normal iteration. diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index cd67d10330c..ec56a08656c 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -184,8 +184,10 @@ namespace ts.codefix { Equals } - export function getCodeActionForImport(moduleSymbol: Symbol, context: ImportCodeFixOptions): ImportCodeAction[] { - const declarations = getImportDeclarations(moduleSymbol, context.checker, context.sourceFile, context.cachedImportDeclarations); + export function getCodeActionForImport(moduleSymbols: Symbol | ReadonlyArray, context: ImportCodeFixOptions): ImportCodeAction[] { + moduleSymbols = toArray(moduleSymbols); + const declarations = flatMap(moduleSymbols, moduleSymbol => + getImportDeclarations(moduleSymbol, context.checker, context.sourceFile, context.cachedImportDeclarations)); const actions: ImportCodeAction[] = []; if (context.symbolToken) { // It is possible that multiple import statements with the same specifier exist in the file. @@ -207,7 +209,7 @@ namespace ts.codefix { } } } - actions.push(getCodeActionForAddImport(moduleSymbol, context, declarations)); + actions.push(getCodeActionForAddImport(moduleSymbols, context, declarations)); return actions; } @@ -313,16 +315,19 @@ namespace ts.codefix { } } - export function getModuleSpecifierForNewImport(sourceFile: SourceFile, moduleSymbol: Symbol, options: CompilerOptions, getCanonicalFileName: (file: string) => string, host: LanguageServiceHost): string | undefined { - const moduleFileName = moduleSymbol.valueDeclaration.getSourceFile().fileName; - const sourceDirectory = getDirectoryPath(sourceFile.fileName); + export function getModuleSpecifierForNewImport(sourceFile: SourceFile, moduleSymbols: ReadonlyArray, options: CompilerOptions, getCanonicalFileName: (file: string) => string, host: LanguageServiceHost): string | undefined { + const choices = mapIterator(arrayIterator(moduleSymbols), moduleSymbol => { + const moduleFileName = moduleSymbol.valueDeclaration.getSourceFile().fileName; + const sourceDirectory = getDirectoryPath(sourceFile.fileName); - return tryGetModuleNameFromAmbientModule(moduleSymbol) || - tryGetModuleNameFromTypeRoots(options, host, getCanonicalFileName, moduleFileName) || - tryGetModuleNameAsNodeModule(options, moduleFileName, host, getCanonicalFileName, sourceDirectory) || - tryGetModuleNameFromBaseUrl(options, moduleFileName, getCanonicalFileName) || - options.rootDirs && tryGetModuleNameFromRootDirs(options.rootDirs, moduleFileName, sourceDirectory, getCanonicalFileName) || - removeExtensionAndIndexPostFix(getRelativePath(moduleFileName, sourceDirectory, getCanonicalFileName), options); + return tryGetModuleNameFromAmbientModule(moduleSymbol) || + tryGetModuleNameFromTypeRoots(options, host, getCanonicalFileName, moduleFileName) || + tryGetModuleNameAsNodeModule(options, moduleFileName, host, getCanonicalFileName, sourceDirectory) || + tryGetModuleNameFromBaseUrl(options, moduleFileName, getCanonicalFileName) || + options.rootDirs && tryGetModuleNameFromRootDirs(options.rootDirs, moduleFileName, sourceDirectory, getCanonicalFileName) || + removeExtensionAndIndexPostFix(getRelativePath(moduleFileName, sourceDirectory, getCanonicalFileName), options); + }); + return best(choices, (a, b) => a.length < b.length); } function tryGetModuleNameFromAmbientModule(moduleSymbol: Symbol): string | undefined { @@ -543,7 +548,7 @@ namespace ts.codefix { } function getCodeActionForAddImport( - moduleSymbol: Symbol, + moduleSymbols: ReadonlyArray, ctx: ImportCodeFixOptions, declarations: ReadonlyArray): ImportCodeAction { const fromExistingImport = firstDefined(declarations, declaration => { @@ -565,7 +570,7 @@ namespace ts.codefix { } const moduleSpecifier = firstDefined(declarations, moduleSpecifierFromAnyImport) - || getModuleSpecifierForNewImport(ctx.sourceFile, moduleSymbol, ctx.compilerOptions, ctx.getCanonicalFileName, ctx.host); + || getModuleSpecifierForNewImport(ctx.sourceFile, moduleSymbols, ctx.compilerOptions, ctx.getCanonicalFileName, ctx.host); return getCodeActionForNewImport(ctx, moduleSpecifier); } @@ -659,24 +664,33 @@ namespace ts.codefix { symbolName = symbol.name; } else { - Debug.fail("Either the symbol or the JSX namespace should be a UMD global if we got here"); + throw Debug.fail("Either the symbol or the JSX namespace should be a UMD global if we got here"); } - const allowSyntheticDefaultImports = getAllowSyntheticDefaultImports(compilerOptions); - + return getCodeActionForImport(symbol, { ...context, symbolName, kind: getUmdImportKind(compilerOptions) }); + } + function getUmdImportKind(compilerOptions: CompilerOptions) { // Import a synthetic `default` if enabled. - if (allowSyntheticDefaultImports) { - return getCodeActionForImport(symbol, { ...context, symbolName, kind: ImportKind.Default }); + if (getAllowSyntheticDefaultImports(compilerOptions)) { + return ImportKind.Default; } - const moduleKind = getEmitModuleKind(compilerOptions); // When a synthetic `default` is unavailable, use `import..require` if the module kind supports it. - if (moduleKind === ModuleKind.AMD || moduleKind === ModuleKind.CommonJS || moduleKind === ModuleKind.UMD) { - return getCodeActionForImport(symbol, { ...context, symbolName, kind: ImportKind.Equals }); + const moduleKind = getEmitModuleKind(compilerOptions); + switch (moduleKind) { + case ModuleKind.AMD: + case ModuleKind.CommonJS: + case ModuleKind.UMD: + return ImportKind.Equals; + case ModuleKind.System: + case ModuleKind.ES2015: + case ModuleKind.ESNext: + case ModuleKind.None: + // Fall back to the `import * as ns` style import. + return ImportKind.Namespace; + default: + throw Debug.assertNever(moduleKind); } - - // Fall back to the `import * as ns` style import. - return getCodeActionForImport(symbol, { ...context, symbolName, kind: ImportKind.Namespace }); } function getActionsForNonUMDImport(context: ImportCodeFixContext, allSourceFiles: ReadonlyArray, cancellationToken: CancellationToken): ImportCodeAction[] { diff --git a/src/services/completions.ts b/src/services/completions.ts index f2771c0a7e1..6bbdfacac4c 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -443,7 +443,7 @@ namespace ts.Completions { } case "symbol": { const { symbol, location, symbolToOriginInfoMap } = symbolCompletion; - const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(symbolToOriginInfoMap, symbol, typeChecker, host, compilerOptions, sourceFile, formatContext, getCanonicalFileName); + const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(symbolToOriginInfoMap, symbol, typeChecker, host, compilerOptions, sourceFile, formatContext, getCanonicalFileName, allSourceFiles); const kindModifiers = SymbolDisplay.getSymbolModifiers(symbol); const { displayParts, documentation, symbolKind, tags } = SymbolDisplay.getSymbolDisplayPartsDocumentationAndSymbolKind(typeChecker, symbol, sourceFile, location, location, SemanticMeaning.All); return { name, kindModifiers, kind: symbolKind, displayParts, documentation, tags, codeActions, source: sourceDisplay }; @@ -476,6 +476,7 @@ namespace ts.Completions { sourceFile: SourceFile, formatContext: formatting.FormatContext, getCanonicalFileName: GetCanonicalFileName, + allSourceFiles: ReadonlyArray, ): { codeActions: CodeAction[] | undefined, sourceDisplay: SymbolDisplayPart[] | undefined } { const symbolOriginInfo = symbolToOriginInfoMap[getSymbolId(symbol)]; if (!symbolOriginInfo) { @@ -483,9 +484,12 @@ namespace ts.Completions { } const { moduleSymbol, isDefaultExport } = symbolOriginInfo; + const exportedSymbol = skipAlias(symbol.exportSymbol || symbol, checker); + const moduleSymbols = getAllReExportingModules(exportedSymbol, checker, allSourceFiles); + Debug.assert(contains(moduleSymbols, moduleSymbol)); - const sourceDisplay = [textPart(codefix.getModuleSpecifierForNewImport(sourceFile, moduleSymbol, compilerOptions, getCanonicalFileName, host))]; - const codeActions = codefix.getCodeActionForImport(moduleSymbol, { + const sourceDisplay = [textPart(codefix.getModuleSpecifierForNewImport(sourceFile, moduleSymbols, compilerOptions, getCanonicalFileName, host))]; + const codeActions = codefix.getCodeActionForImport(moduleSymbols, { host, checker, newLineCharacter: host.getNewLine(), @@ -500,6 +504,18 @@ namespace ts.Completions { return { sourceDisplay, codeActions }; } + function getAllReExportingModules(exportedSymbol: Symbol, checker: TypeChecker, allSourceFiles: ReadonlyArray): ReadonlyArray { + const result: Symbol[] = []; + codefix.forEachExternalModule(checker, allSourceFiles, module => { + for (const exported of checker.getExportsOfModule(module)) { + if (skipAlias(exported, checker) === exportedSymbol) { + result.push(module); + } + } + }); + return result; + } + export function getCompletionEntrySymbol( typeChecker: TypeChecker, log: (message: string) => void, diff --git a/tests/cases/fourslash/completionsImport_ofAlias_preferShortPath.ts b/tests/cases/fourslash/completionsImport_ofAlias_preferShortPath.ts new file mode 100644 index 00000000000..a1eba5981db --- /dev/null +++ b/tests/cases/fourslash/completionsImport_ofAlias_preferShortPath.ts @@ -0,0 +1,31 @@ +/// + +// Test that the completion is for the shortest path, even if that's a re-export. +// Note that `source` in completionEntries will still be the original exporting path, but we use the re-export in completionDetails. + +// @moduleResolution: node +// @module: commonJs + +// @Filename: /foo/index.ts +////export { foo } from "./lib/foo"; + +// @Filename: /foo/lib/foo.ts +////export const foo = 0; + +// @Filename: /user.ts +////fo/**/ + +goTo.marker(""); +const options = { includeExternalModuleExports: true, sourceDisplay: "./foo" }; +verify.completionListContains({ name: "foo", source: "/foo/lib/foo" }, "const foo: 0", "", "const", /*spanIndex*/ undefined, /*hasAction*/ true, options); +verify.not.completionListContains({ name: "foo", source: "/foo/index" }, undefined, undefined, undefined, undefined, undefined, options); + +verify.applyCodeActionFromCompletion("", { + name: "foo", + source: "/foo/lib/foo", + description: `Import 'foo' from "./foo".`, + // TODO: GH#18445 + newFileContent: `import { foo } from "./foo";\r +\r +fo`, +});