Skip to content

Commit 4d2983a

Browse files
TypeScript Botandrewbranch
TypeScript Bot
andauthored
Cherry-pick PR #49442 into release-4.7 (#49447)
Component commits: d8f251b Avoid repeating codefix work when resolving auto-import specifiers for completions Co-authored-by: Andrew Branch <[email protected]>
1 parent e5050bb commit 4d2983a

File tree

2 files changed

+78
-44
lines changed

2 files changed

+78
-44
lines changed

Diff for: src/services/codefixes/importFixes.ts

+70-36
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ namespace ts.codefix {
4040
},
4141
});
4242

43+
/**
44+
* Computes multiple import additions to a file and writes them to a ChangeTracker.
45+
*/
4346
export interface ImportAdder {
4447
hasFixes(): boolean;
4548
addImportFromDiagnostic: (diagnostic: DiagnosticWithLocation, context: CodeFixContextBase) => void;
@@ -235,6 +238,47 @@ namespace ts.codefix {
235238
}
236239
}
237240

241+
/**
242+
* Computes module specifiers for multiple import additions to a file.
243+
*/
244+
export interface ImportSpecifierResolver {
245+
getModuleSpecifierForBestExportInfo(
246+
exportInfo: readonly SymbolExportInfo[],
247+
symbolName: string,
248+
position: number,
249+
isValidTypeOnlyUseSite: boolean,
250+
fromCacheOnly?: boolean
251+
): { exportInfo?: SymbolExportInfo, moduleSpecifier: string, computedWithoutCacheCount: number } | undefined;
252+
}
253+
254+
export function createImportSpecifierResolver(importingFile: SourceFile, program: Program, host: LanguageServiceHost, preferences: UserPreferences): ImportSpecifierResolver {
255+
const packageJsonImportFilter = createPackageJsonImportFilter(importingFile, preferences, host);
256+
const importMap = createExistingImportMap(program.getTypeChecker(), importingFile, program.getCompilerOptions());
257+
return { getModuleSpecifierForBestExportInfo };
258+
259+
function getModuleSpecifierForBestExportInfo(
260+
exportInfo: readonly SymbolExportInfo[],
261+
symbolName: string,
262+
position: number,
263+
isValidTypeOnlyUseSite: boolean,
264+
fromCacheOnly?: boolean,
265+
): { exportInfo?: SymbolExportInfo, moduleSpecifier: string, computedWithoutCacheCount: number } | undefined {
266+
const { fixes, computedWithoutCacheCount } = getImportFixes(
267+
exportInfo,
268+
{ symbolName, position },
269+
isValidTypeOnlyUseSite,
270+
/*useRequire*/ false,
271+
program,
272+
importingFile,
273+
host,
274+
preferences,
275+
importMap,
276+
fromCacheOnly);
277+
const result = getBestFix(fixes, importingFile, program, packageJsonImportFilter, host);
278+
return result && { ...result, computedWithoutCacheCount };
279+
}
280+
}
281+
238282
// Sorted with the preferred fix coming first.
239283
const enum ImportFixKind { UseNamespace, JsdocTypeImport, AddToExisting, AddNew, PromoteTypeOnly }
240284
// These should not be combined as bitflags, but are given powers of 2 values to
@@ -394,32 +438,6 @@ namespace ts.codefix {
394438
}
395439
}
396440

397-
export function getModuleSpecifierForBestExportInfo(
398-
exportInfo: readonly SymbolExportInfo[],
399-
symbolName: string,
400-
position: number,
401-
isValidTypeOnlyUseSite: boolean,
402-
importingFile: SourceFile,
403-
program: Program,
404-
host: LanguageServiceHost,
405-
preferences: UserPreferences,
406-
packageJsonImportFilter?: PackageJsonImportFilter,
407-
fromCacheOnly?: boolean,
408-
): { exportInfo?: SymbolExportInfo, moduleSpecifier: string, computedWithoutCacheCount: number } | undefined {
409-
const { fixes, computedWithoutCacheCount } = getImportFixes(
410-
exportInfo,
411-
{ symbolName, position },
412-
isValidTypeOnlyUseSite,
413-
/*useRequire*/ false,
414-
program,
415-
importingFile,
416-
host,
417-
preferences,
418-
fromCacheOnly);
419-
const result = getBestFix(fixes, importingFile, program, packageJsonImportFilter || createPackageJsonImportFilter(importingFile, preferences, host), host);
420-
return result && { ...result, computedWithoutCacheCount };
421-
}
422-
423441
function getImportFixes(
424442
exportInfos: readonly SymbolExportInfo[],
425443
useNamespaceInfo: {
@@ -433,10 +451,11 @@ namespace ts.codefix {
433451
sourceFile: SourceFile,
434452
host: LanguageServiceHost,
435453
preferences: UserPreferences,
454+
importMap = createExistingImportMap(program.getTypeChecker(), sourceFile, program.getCompilerOptions()),
436455
fromCacheOnly?: boolean,
437456
): { computedWithoutCacheCount: number, fixes: readonly ImportFixWithModuleSpecifier[] } {
438457
const checker = program.getTypeChecker();
439-
const existingImports = flatMap(exportInfos, info => getExistingImportDeclarations(info, checker, sourceFile, program.getCompilerOptions()));
458+
const existingImports = flatMap(exportInfos, importMap.getImportsForExportInfo);
440459
const useNamespace = useNamespaceInfo && tryUseExistingNamespaceImport(existingImports, useNamespaceInfo.symbolName, useNamespaceInfo.position, checker);
441460
const addToExisting = tryAddToExistingImport(existingImports, isValidTypeOnlyUseSite, checker, program.getCompilerOptions());
442461
if (addToExisting) {
@@ -587,19 +606,34 @@ namespace ts.codefix {
587606
});
588607
}
589608

590-
function getExistingImportDeclarations({ moduleSymbol, exportKind, targetFlags, symbol }: SymbolExportInfo, checker: TypeChecker, importingFile: SourceFile, compilerOptions: CompilerOptions): readonly FixAddToExistingImportInfo[] {
591-
// Can't use an es6 import for a type in JS.
592-
if (!(targetFlags & SymbolFlags.Value) && isSourceFileJS(importingFile)) return emptyArray;
593-
const importKind = getImportKind(importingFile, exportKind, compilerOptions);
594-
return mapDefined(importingFile.imports, (moduleSpecifier): FixAddToExistingImportInfo | undefined => {
609+
function createExistingImportMap(checker: TypeChecker, importingFile: SourceFile, compilerOptions: CompilerOptions) {
610+
let importMap: MultiMap<SymbolId, AnyImportOrRequire> | undefined;
611+
for (const moduleSpecifier of importingFile.imports) {
595612
const i = importFromModuleSpecifier(moduleSpecifier);
596613
if (isVariableDeclarationInitializedToRequire(i.parent)) {
597-
return checker.resolveExternalModuleName(moduleSpecifier) === moduleSymbol ? { declaration: i.parent, importKind, symbol, targetFlags } : undefined;
614+
const moduleSymbol = checker.resolveExternalModuleName(moduleSpecifier);
615+
if (moduleSymbol) {
616+
(importMap ||= createMultiMap()).add(getSymbolId(moduleSymbol), i.parent);
617+
}
598618
}
599-
if (i.kind === SyntaxKind.ImportDeclaration || i.kind === SyntaxKind.ImportEqualsDeclaration) {
600-
return checker.getSymbolAtLocation(moduleSpecifier) === moduleSymbol ? { declaration: i, importKind, symbol, targetFlags } : undefined;
619+
else if (i.kind === SyntaxKind.ImportDeclaration || i.kind === SyntaxKind.ImportEqualsDeclaration) {
620+
const moduleSymbol = checker.getSymbolAtLocation(moduleSpecifier);
621+
if (moduleSymbol) {
622+
(importMap ||= createMultiMap()).add(getSymbolId(moduleSymbol), i);
623+
}
601624
}
602-
});
625+
}
626+
627+
return {
628+
getImportsForExportInfo: ({ moduleSymbol, exportKind, targetFlags, symbol }: SymbolExportInfo): readonly FixAddToExistingImportInfo[] => {
629+
// Can't use an es6 import for a type in JS.
630+
if (!(targetFlags & SymbolFlags.Value) && isSourceFileJS(importingFile)) return emptyArray;
631+
const matchingDeclarations = importMap?.get(getSymbolId(moduleSymbol));
632+
if (!matchingDeclarations) return emptyArray;
633+
const importKind = getImportKind(importingFile, exportKind, compilerOptions);
634+
return matchingDeclarations.map(declaration => ({ declaration, importKind, symbol, targetFlags }));
635+
}
636+
};
603637
}
604638

605639
function shouldUseRequire(sourceFile: SourceFile, program: Program): boolean {

Diff for: src/services/completions.ts

+8-8
Original file line numberDiff line numberDiff line change
@@ -184,16 +184,15 @@ namespace ts.Completions {
184184
function resolvingModuleSpecifiers<TReturn>(
185185
logPrefix: string,
186186
host: LanguageServiceHost,
187+
resolver: codefix.ImportSpecifierResolver,
187188
program: Program,
188-
sourceFile: SourceFile,
189189
position: number,
190190
preferences: UserPreferences,
191191
isForImportStatementCompletion: boolean,
192192
isValidTypeOnlyUseSite: boolean,
193193
cb: (context: ModuleSpecifierResolutioContext) => TReturn,
194194
): TReturn {
195195
const start = timestamp();
196-
const packageJsonImportFilter = createPackageJsonImportFilter(sourceFile, preferences, host);
197196
// Under `--moduleResolution nodenext`, we have to resolve module specifiers up front, because
198197
// package.json exports can mean we *can't* resolve a module specifier (that doesn't include a
199198
// relative path into node_modules), and we want to filter those completions out entirely.
@@ -221,7 +220,7 @@ namespace ts.Completions {
221220

222221
function tryResolve(exportInfo: readonly SymbolExportInfo[], symbolName: string, isFromAmbientModule: boolean): ModuleSpecifierResolutionResult {
223222
if (isFromAmbientModule) {
224-
const result = codefix.getModuleSpecifierForBestExportInfo(exportInfo, symbolName, position, isValidTypeOnlyUseSite, sourceFile, program, host, preferences);
223+
const result = resolver.getModuleSpecifierForBestExportInfo(exportInfo, symbolName, position, isValidTypeOnlyUseSite);
225224
if (result) {
226225
ambientCount++;
227226
}
@@ -230,7 +229,7 @@ namespace ts.Completions {
230229
const shouldResolveModuleSpecifier = needsFullResolution || preferences.allowIncompleteCompletions && resolvedCount < moduleSpecifierResolutionLimit;
231230
const shouldGetModuleSpecifierFromCache = !shouldResolveModuleSpecifier && preferences.allowIncompleteCompletions && cacheAttemptCount < moduleSpecifierResolutionCacheAttemptLimit;
232231
const result = (shouldResolveModuleSpecifier || shouldGetModuleSpecifierFromCache)
233-
? codefix.getModuleSpecifierForBestExportInfo(exportInfo, symbolName, position, isValidTypeOnlyUseSite, sourceFile, program, host, preferences, packageJsonImportFilter, shouldGetModuleSpecifierFromCache)
232+
? resolver.getModuleSpecifierForBestExportInfo(exportInfo, symbolName, position, isValidTypeOnlyUseSite, shouldGetModuleSpecifierFromCache)
234233
: undefined;
235234

236235
if (!shouldResolveModuleSpecifier && !shouldGetModuleSpecifierFromCache || shouldGetModuleSpecifierFromCache && !result) {
@@ -371,8 +370,8 @@ namespace ts.Completions {
371370
const newEntries = resolvingModuleSpecifiers(
372371
"continuePreviousIncompleteResponse",
373372
host,
373+
codefix.createImportSpecifierResolver(file, program, host, preferences),
374374
program,
375-
file,
376375
location.getStart(),
377376
preferences,
378377
/*isForImportStatementCompletion*/ false,
@@ -2206,6 +2205,7 @@ namespace ts.Completions {
22062205
let hasUnresolvedAutoImports = false;
22072206
// This also gets mutated in nested-functions after the return
22082207
let symbols: Symbol[] = [];
2208+
let importSpecifierResolver: codefix.ImportSpecifierResolver | undefined;
22092209
const symbolToOriginInfoMap: SymbolOriginInfoMap = [];
22102210
const symbolToSortTextMap: SymbolSortTextMap = [];
22112211
const seenPropertySymbols = new Map<SymbolId, true>();
@@ -2448,14 +2448,14 @@ namespace ts.Completions {
24482448
}
24492449
else {
24502450
const fileName = isExternalModuleNameRelative(stripQuotes(moduleSymbol.name)) ? getSourceFileOfModule(moduleSymbol)?.fileName : undefined;
2451-
const { moduleSpecifier } = codefix.getModuleSpecifierForBestExportInfo([{
2451+
const { moduleSpecifier } = (importSpecifierResolver ||= codefix.createImportSpecifierResolver(sourceFile, program, host, preferences)).getModuleSpecifierForBestExportInfo([{
24522452
exportKind: ExportKind.Named,
24532453
moduleFileName: fileName,
24542454
isFromPackageJson: false,
24552455
moduleSymbol,
24562456
symbol: firstAccessibleSymbol,
24572457
targetFlags: skipAlias(firstAccessibleSymbol, typeChecker).flags,
2458-
}], firstAccessibleSymbol.name, position, isValidTypeOnlyAliasUseSite(location), sourceFile, program, host, preferences) || {};
2458+
}], firstAccessibleSymbol.name, position, isValidTypeOnlyAliasUseSite(location)) || {};
24592459

24602460
if (moduleSpecifier) {
24612461
const origin: SymbolOriginInfoResolvedExport = {
@@ -2733,8 +2733,8 @@ namespace ts.Completions {
27332733
resolvingModuleSpecifiers(
27342734
"collectAutoImports",
27352735
host,
2736+
importSpecifierResolver ||= codefix.createImportSpecifierResolver(sourceFile, program, host, preferences),
27362737
program,
2737-
sourceFile,
27382738
position,
27392739
preferences,
27402740
!!importCompletionNode,

0 commit comments

Comments
 (0)