Skip to content

Commit ee8337d

Browse files
author
Andy
authored
Minor cleanups in importFixes (microsoft#23995)
1 parent ac0657a commit ee8337d

File tree

2 files changed

+53
-64
lines changed

2 files changed

+53
-64
lines changed

‎src/compiler/core.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -965,7 +965,8 @@ namespace ts {
965965
export function append<T>(to: T[], value: T | undefined): T[];
966966
export function append<T>(to: T[] | undefined, value: T): T[];
967967
export function append<T>(to: T[] | undefined, value: T | undefined): T[] | undefined;
968-
export function append<T>(to: T[] | undefined, value: T | undefined): T[] | undefined {
968+
export function append<T>(to: Push<T>, value: T | undefined): void;
969+
export function append<T>(to: T[], value: T | undefined): T[] | undefined {
969970
if (value === undefined) return to;
970971
if (to === undefined) return [value];
971972
to.push(value);

‎src/services/codefixes/importFixes.ts

Lines changed: 51 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,14 @@ namespace ts.codefix {
1010
Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code,
1111
Diagnostics._0_only_refers_to_a_type_but_is_being_used_as_a_value_here.code,
1212
],
13-
getCodeActions: getImportCodeActions,
13+
getCodeActions: context => context.errorCode === Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code
14+
? getActionsForUMDImport(context)
15+
: getActionsForNonUMDImport(context),
1416
// TODO: GH#20315
1517
fixIds: [],
1618
getAllCodeActions: notImplemented,
1719
});
1820

19-
// Map from module Id to an array of import declarations in that module.
20-
type ImportDeclarationMap = ExistingImportInfo[][];
21-
2221
interface SymbolContext extends textChanges.TextChangesContext {
2322
sourceFile: SourceFile;
2423
symbolName: string;
@@ -30,7 +29,6 @@ namespace ts.codefix {
3029
checker: TypeChecker;
3130
compilerOptions: CompilerOptions;
3231
getCanonicalFileName: GetCanonicalFileName;
33-
cachedImportDeclarations?: ImportDeclarationMap;
3432
preferences: UserPreferences;
3533
}
3634

@@ -50,7 +48,6 @@ namespace ts.codefix {
5048
program,
5149
checker,
5250
compilerOptions: program.getCompilerOptions(),
53-
cachedImportDeclarations: [],
5451
getCanonicalFileName: createGetCanonicalFileName(hostUsesCaseSensitiveFileNames(context.host)),
5552
symbolName,
5653
symbolToken,
@@ -130,8 +127,19 @@ namespace ts.codefix {
130127
}
131128

132129
function getCodeActionsForImport_separateExistingAndNew(exportInfos: ReadonlyArray<SymbolExportInfo>, context: ImportCodeFixContext, useExisting: Push<CodeFixAction>, addNew: Push<CodeFixAction>): void {
133-
const existingImports = flatMap(exportInfos, info =>
134-
getImportDeclarations(info, context.checker, context.sourceFile, context.cachedImportDeclarations));
130+
const existingImports = flatMap(exportInfos, info => getExistingImportDeclarations(info, context.checker, context.sourceFile));
131+
132+
append(useExisting, tryUseExistingNamespaceImport(existingImports, context, context.symbolToken, context.checker));
133+
const addToExisting = tryAddToExistingImport(existingImports, context);
134+
135+
if (addToExisting) {
136+
useExisting.push(addToExisting);
137+
}
138+
else { // Don't bother providing an action to add a new import if we can add to an existing one.
139+
getCodeActionsForAddImport(exportInfos, context, existingImports, addNew);
140+
}
141+
}
142+
function tryUseExistingNamespaceImport(existingImports: ReadonlyArray<ExistingImportInfo>, context: SymbolContext, symbolToken: Node | undefined, checker: TypeChecker): CodeFixAction | undefined {
135143
// It is possible that multiple import statements with the same specifier exist in the file.
136144
// e.g.
137145
//
@@ -144,18 +152,26 @@ namespace ts.codefix {
144152
// 1. change "member3" to "ns.member3"
145153
// 2. add "member3" to the second import statement's import list
146154
// and it is up to the user to decide which one fits best.
147-
if (context.symbolToken && isIdentifier(context.symbolToken)) {
148-
for (const { declaration } of existingImports) {
149-
const namespace = getNamespaceImportName(declaration);
150-
if (namespace) {
151-
const moduleSymbol = context.checker.getAliasedSymbol(context.checker.getSymbolAtLocation(namespace)!);
152-
if (moduleSymbol && moduleSymbol.exports!.has(escapeLeadingUnderscores(context.symbolName))) {
153-
useExisting.push(getCodeActionForUseExistingNamespaceImport(namespace.text, context, context.symbolToken));
154-
}
155+
return !symbolToken || !isIdentifier(symbolToken) ? undefined : firstDefined(existingImports, ({ declaration }) => {
156+
const namespace = getNamespaceImportName(declaration);
157+
if (namespace) {
158+
const moduleSymbol = namespace && checker.getAliasedSymbol(checker.getSymbolAtLocation(namespace)!);
159+
if (moduleSymbol && moduleSymbol.exports!.has(escapeLeadingUnderscores(context.symbolName))) {
160+
return getCodeActionForUseExistingNamespaceImport(namespace.text, context, symbolToken);
155161
}
156162
}
157-
}
158-
getCodeActionsForAddImport(exportInfos, context, existingImports, useExisting, addNew);
163+
});
164+
}
165+
function tryAddToExistingImport(existingImports: ReadonlyArray<ExistingImportInfo>, context: SymbolContext): CodeFixAction | undefined {
166+
return firstDefined(existingImports, ({ declaration, importKind }) => {
167+
if (declaration.kind === SyntaxKind.ImportDeclaration && declaration.importClause) {
168+
const changes = tryUpdateExistingImport(context, declaration.importClause, importKind);
169+
if (changes) {
170+
const moduleSpecifierWithoutQuotes = stripQuotes(declaration.moduleSpecifier.getText());
171+
return createCodeAction(Diagnostics.Add_0_to_existing_import_declaration_from_1, [context.symbolName, moduleSpecifierWithoutQuotes], changes);
172+
}
173+
}
174+
});
159175
}
160176

161177
function getNamespaceImportName(declaration: AnyImportSyntax): Identifier | undefined {
@@ -168,18 +184,12 @@ namespace ts.codefix {
168184
}
169185
}
170186

171-
// TODO(anhans): This doesn't seem important to cache... just use an iterator instead of creating a new array?
172-
function getImportDeclarations({ moduleSymbol, importKind }: SymbolExportInfo, checker: TypeChecker, { imports }: SourceFile, cachedImportDeclarations: ImportDeclarationMap = []): ReadonlyArray<ExistingImportInfo> {
173-
const moduleSymbolId = getUniqueSymbolId(moduleSymbol, checker);
174-
let cached = cachedImportDeclarations[moduleSymbolId];
175-
if (!cached) {
176-
cached = cachedImportDeclarations[moduleSymbolId] = mapDefined<StringLiteralLike, ExistingImportInfo>(imports, moduleSpecifier => {
177-
const i = importFromModuleSpecifier(moduleSpecifier);
178-
return (i.kind === SyntaxKind.ImportDeclaration || i.kind === SyntaxKind.ImportEqualsDeclaration)
179-
&& checker.getSymbolAtLocation(moduleSpecifier) === moduleSymbol ? { declaration: i, importKind } : undefined;
180-
});
181-
}
182-
return cached;
187+
function getExistingImportDeclarations({ moduleSymbol, importKind }: SymbolExportInfo, checker: TypeChecker, { imports }: SourceFile): ReadonlyArray<ExistingImportInfo> {
188+
return mapDefined<StringLiteralLike, ExistingImportInfo>(imports, moduleSpecifier => {
189+
const i = importFromModuleSpecifier(moduleSpecifier);
190+
return (i.kind === SyntaxKind.ImportDeclaration || i.kind === SyntaxKind.ImportEqualsDeclaration)
191+
&& checker.getSymbolAtLocation(moduleSpecifier) === moduleSymbol ? { declaration: i, importKind } : undefined;
192+
});
183193
}
184194

185195
function getCodeActionForNewImport(context: SymbolContext & { preferences: UserPreferences }, { moduleSpecifier, importKind }: NewImportInfo): CodeFixAction {
@@ -258,23 +268,8 @@ namespace ts.codefix {
258268
exportInfos: ReadonlyArray<SymbolExportInfo>,
259269
ctx: ImportCodeFixContext,
260270
existingImports: ReadonlyArray<ExistingImportInfo>,
261-
useExisting: Push<CodeFixAction>,
262271
addNew: Push<CodeFixAction>,
263272
): void {
264-
const fromExistingImport = firstDefined(existingImports, ({ declaration, importKind }) => {
265-
if (declaration.kind === SyntaxKind.ImportDeclaration && declaration.importClause) {
266-
const changes = tryUpdateExistingImport(ctx, (isImportClause(declaration.importClause) && declaration.importClause || undefined)!, importKind); // TODO: GH#18217
267-
if (changes) {
268-
const moduleSpecifierWithoutQuotes = stripQuotes(declaration.moduleSpecifier.getText());
269-
return createCodeAction(Diagnostics.Add_0_to_existing_import_declaration_from_1, [ctx.symbolName, moduleSpecifierWithoutQuotes], changes);
270-
}
271-
}
272-
});
273-
if (fromExistingImport) {
274-
useExisting.push(fromExistingImport);
275-
return;
276-
}
277-
278273
const existingDeclaration = firstDefined(existingImports, newImportInfoFromExistingSpecifier);
279274
const newImportInfos = existingDeclaration
280275
? [existingDeclaration]
@@ -348,12 +343,6 @@ namespace ts.codefix {
348343
return createCodeAction(Diagnostics.Change_0_to_1, [symbolName, `${namespacePrefix}.${symbolName}`], changes);
349344
}
350345

351-
function getImportCodeActions(context: CodeFixContext): CodeFixAction[] | undefined {
352-
return context.errorCode === Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code
353-
? getActionsForUMDImport(context)
354-
: getActionsForNonUMDImport(context);
355-
}
356-
357346
function getActionsForUMDImport(context: CodeFixContext): CodeFixAction[] | undefined {
358347
const token = getTokenAtPosition(context.sourceFile, context.span.start, /*includeJsDocComment*/ false);
359348
const checker = context.program.getTypeChecker();
@@ -422,11 +411,18 @@ namespace ts.codefix {
422411
? checker.getJsxNamespace()
423412
: isIdentifier(symbolToken) ? symbolToken.text : undefined;
424413
if (!symbolName) return undefined;
425-
426414
// "default" is a keyword and not a legal identifier for the import, so we don't expect it here
427415
Debug.assert(symbolName !== "default");
428-
const currentTokenMeaning = getMeaningFromLocation(symbolToken);
429416

417+
const addToExistingDeclaration: CodeFixAction[] = [];
418+
const addNewDeclaration: CodeFixAction[] = [];
419+
getExportInfos(symbolName, getMeaningFromLocation(symbolToken), cancellationToken, sourceFile, checker, program).forEach(exportInfos => {
420+
getCodeActionsForImport_separateExistingAndNew(exportInfos, convertToImportCodeFixContext(context, symbolToken, symbolName), addToExistingDeclaration, addNewDeclaration);
421+
});
422+
return [...addToExistingDeclaration, ...addNewDeclaration];
423+
}
424+
425+
function getExportInfos(symbolName: string, currentTokenMeaning: SemanticMeaning, cancellationToken: CancellationToken, sourceFile: SourceFile, checker: TypeChecker, program: Program): ReadonlyMap<ReadonlyArray<SymbolExportInfo>> {
430426
// For each original symbol, keep all re-exports of that symbol together so we can call `getCodeActionsForImport` on the whole group at once.
431427
// Maps symbol id to info for modules providing that symbol (original export + re-exports).
432428
const originalSymbolToExportInfos = createMultiMap<SymbolExportInfo>();
@@ -464,20 +460,12 @@ namespace ts.codefix {
464460
}
465461
else if (isExportSpecifier(declaration)) {
466462
Debug.assert(declaration.name.escapedText === InternalSymbolName.Default);
467-
if (declaration.propertyName) {
468-
return declaration.propertyName.escapedText;
469-
}
463+
return declaration.propertyName && declaration.propertyName.escapedText;
470464
}
471465
});
472466
}
473467
});
474-
475-
const addToExistingDeclaration: CodeFixAction[] = [];
476-
const addNewDeclaration: CodeFixAction[] = [];
477-
originalSymbolToExportInfos.forEach(exportInfos => {
478-
getCodeActionsForImport_separateExistingAndNew(exportInfos, convertToImportCodeFixContext(context, symbolToken, symbolName), addToExistingDeclaration, addNewDeclaration);
479-
});
480-
return [...addToExistingDeclaration, ...addNewDeclaration];
468+
return originalSymbolToExportInfos;
481469
}
482470

483471
function checkSymbolHasMeaning({ declarations }: Symbol, meaning: SemanticMeaning): boolean {

0 commit comments

Comments
 (0)