diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index 99ccfb8c98286..e27f63eb29cda 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -255,16 +255,8 @@ namespace ts.moduleSpecifiers { return firstDefined(imports, ({ text }) => pathIsRelative(text) ? hasJSFileExtension(text) : undefined) || false; } - function numberOfDirectorySeparators(str: string) { - const match = str.match(/\//g); - return match ? match.length : 0; - } - function comparePathsByRedirectAndNumberOfDirectorySeparators(a: ModulePath, b: ModulePath) { - return compareBooleans(b.isRedirect, a.isRedirect) || compareValues( - numberOfDirectorySeparators(a.path), - numberOfDirectorySeparators(b.path) - ); + return compareBooleans(b.isRedirect, a.isRedirect) || compareNumberOfDirectorySeparators(a.path, b.path); } function getNearestAncestorDirectoryWithPackageJson(host: ModuleSpecifierResolutionHost, fileName: string) { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 8d9871545ffc8..8c3ff87cbbfad 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -6586,6 +6586,18 @@ namespace ts { return false; } + function numberOfDirectorySeparators(str: string) { + const match = str.match(/\//g); + return match ? match.length : 0; + } + + export function compareNumberOfDirectorySeparators(path1: string, path2: string) { + return compareValues( + numberOfDirectorySeparators(path1), + numberOfDirectorySeparators(path2) + ); + } + /** * Extension boundaries by priority. Lower numbers indicate higher priorities, and are * aligned to the offset of the highest priority extension in the diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index e47c7f79b53cf..628c26d294e5b 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -215,7 +215,7 @@ namespace ts.codefix { : getAllReExportingModules(sourceFile, exportedSymbol, moduleSymbol, symbolName, host, program, /*useAutoImportProvider*/ true); const useRequire = shouldUseRequire(sourceFile, program); const preferTypeOnlyImport = compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error && !isSourceFileJS(sourceFile) && isValidTypeOnlyAliasUseSite(getTokenAtPosition(sourceFile, position)); - const moduleSpecifier = first(getNewImportInfos(program, sourceFile, position, preferTypeOnlyImport, useRequire, exportInfos, host, preferences)).moduleSpecifier; + const moduleSpecifier = getBestFix(getNewImportInfos(program, sourceFile, position, preferTypeOnlyImport, useRequire, exportInfos, host, preferences), sourceFile, program, host).moduleSpecifier; const fix = getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, symbolName, program, position, preferTypeOnlyImport, useRequire, host, preferences); return { moduleSpecifier, codeAction: codeFixActionToCodeAction(codeActionForFix({ host, formatContext, preferences }, sourceFile, symbolName, fix, getQuotePreference(sourceFile, preferences))) }; } @@ -223,7 +223,7 @@ namespace ts.codefix { function getImportFixForSymbol(sourceFile: SourceFile, exportInfos: readonly SymbolExportInfo[], moduleSymbol: Symbol, symbolName: string, program: Program, position: number | undefined, preferTypeOnlyImport: boolean, useRequire: boolean, host: LanguageServiceHost, preferences: UserPreferences) { Debug.assert(exportInfos.some(info => info.moduleSymbol === moduleSymbol), "Some exportInfo should match the specified moduleSymbol"); // We sort the best codefixes first, so taking `first` is best. - return first(getFixForImport(exportInfos, symbolName, position, preferTypeOnlyImport, useRequire, program, sourceFile, host, preferences)); + return getBestFix(getFixForImport(exportInfos, symbolName, position, preferTypeOnlyImport, useRequire, program, sourceFile, host, preferences), sourceFile, program, host); } function codeFixActionToCodeAction({ description, changes, commands }: CodeFixAction): CodeAction { @@ -424,28 +424,13 @@ namespace ts.codefix { ): readonly (FixAddNewImport | FixUseImportType)[] { const isJs = isSourceFileJS(sourceFile); const compilerOptions = program.getCompilerOptions(); - const { allowsImportingSpecifier } = createAutoImportFilter(sourceFile, program, host); - - const choicesForEachExportingModule = flatMap(moduleSymbols, ({ moduleSymbol, importKind, exportedSymbolIsTypeOnly }) => + return flatMap(moduleSymbols, ({ moduleSymbol, importKind, exportedSymbolIsTypeOnly }) => moduleSpecifiers.getModuleSpecifiers(moduleSymbol, program.getTypeChecker(), compilerOptions, sourceFile, createModuleSpecifierResolutionHost(program, host), preferences) .map((moduleSpecifier): FixAddNewImport | FixUseImportType => // `position` should only be undefined at a missing jsx namespace, in which case we shouldn't be looking for pure types. exportedSymbolIsTypeOnly && isJs ? { kind: ImportFixKind.ImportType, moduleSpecifier, position: Debug.checkDefined(position, "position should be defined") } : { kind: ImportFixKind.AddNew, moduleSpecifier, importKind, useRequire, typeOnly: preferTypeOnlyImport })); - - // Sort by presence in package.json, then shortest paths first - return sort(choicesForEachExportingModule, (a, b) => { - const allowsImportingA = allowsImportingSpecifier(a.moduleSpecifier); - const allowsImportingB = allowsImportingSpecifier(b.moduleSpecifier); - if (allowsImportingA && !allowsImportingB) { - return -1; - } - if (allowsImportingB && !allowsImportingA) { - return 1; - } - return a.moduleSpecifier.length - b.moduleSpecifier.length; - }); } function getFixesForAddImport( @@ -479,7 +464,31 @@ namespace ts.codefix { const info = errorCode === Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code ? getFixesInfoForUMDImport(context, symbolToken) : isIdentifier(symbolToken) ? getFixesInfoForNonUMDImport(context, symbolToken, useAutoImportProvider) : undefined; - return info && { ...info, fixes: sort(info.fixes, (a, b) => a.kind - b.kind) }; + return info && { ...info, fixes: sortFixes(info.fixes, context.sourceFile, context.program, context.host) }; + } + + function sortFixes(fixes: readonly ImportFix[], sourceFile: SourceFile, program: Program, host: LanguageServiceHost): readonly ImportFix[] { + const { allowsImportingSpecifier } = createAutoImportFilter(sourceFile, program, host); + return sort(fixes, (a, b) => compareValues(a.kind, b.kind) || compareModuleSpecifiers(a, b, allowsImportingSpecifier)); + } + + function getBestFix(fixes: readonly T[], sourceFile: SourceFile, program: Program, host: LanguageServiceHost): T { + // These will always be placed first if available, and are better than other kinds + if (fixes[0].kind === ImportFixKind.UseNamespace || fixes[0].kind === ImportFixKind.AddToExisting) { + return fixes[0]; + } + const { allowsImportingSpecifier } = createAutoImportFilter(sourceFile, program, host); + return fixes.reduce((best, fix) => + compareModuleSpecifiers(fix, best, allowsImportingSpecifier) === Comparison.LessThan ? fix : best + ); + } + + function compareModuleSpecifiers(a: ImportFix, b: ImportFix, allowsImportingSpecifier: (specifier: string) => boolean): Comparison { + if (a.kind !== ImportFixKind.UseNamespace && b.kind !== ImportFixKind.UseNamespace) { + return compareBooleans(allowsImportingSpecifier(a.moduleSpecifier), allowsImportingSpecifier(b.moduleSpecifier)) + || compareNumberOfDirectorySeparators(a.moduleSpecifier, b.moduleSpecifier); + } + return Comparison.EqualTo; } function getFixesInfoForUMDImport({ sourceFile, program, host, preferences }: CodeFixContextBase, token: Node): FixesInfo | undefined { diff --git a/tests/cases/fourslash/importNameCodeFix_sortByDistance.ts b/tests/cases/fourslash/importNameCodeFix_sortByDistance.ts new file mode 100644 index 0000000000000..c25cf8c4d4fee --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFix_sortByDistance.ts @@ -0,0 +1,26 @@ +/// + +// @module: commonjs + +// @Filename: /src/admin/utils/db/db.ts +//// export const db = {}; + +// @Filename: /src/admin/utils/db/index.ts +//// export * from "./db"; + +// @Filename: /src/client/helpers/db.ts +//// export const db = {}; + +// @Filename: /src/client/db.ts +//// export const db = {}; + +// @Filename: /src/client/foo.ts +//// db/**/ + +goTo.marker(""); +verify.importFixAtPosition([ + `import { db } from "./db";\n\ndb`, + `import { db } from "./helpers/db";\n\ndb`, + `import { db } from "../admin/utils/db";\n\ndb`, + `import { db } from "../admin/utils/db/db";\n\ndb` +]);