Skip to content

Commit e9092f3

Browse files
andrewbranchandrewbranch
authored
Proposal: simplify auto import descriptions (microsoft#47631)
* Simplify import fix descriptions * Update tests * Fix new test
1 parent 208cb2d commit e9092f3

File tree

68 files changed

+113
-108
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

68 files changed

+113
-108
lines changed

‎src/compiler/diagnosticMessages.json

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6354,18 +6354,14 @@
63546354
"category": "Message",
63556355
"code": 90012
63566356
},
6357-
"Import '{0}' from module \"{1}\"": {
6357+
"Import '{0}' from \"{1}\"": {
63586358
"category": "Message",
63596359
"code": 90013
63606360
},
63616361
"Change '{0}' to '{1}'": {
63626362
"category": "Message",
63636363
"code": 90014
63646364
},
6365-
"Add '{0}' to existing import declaration from \"{1}\"": {
6366-
"category": "Message",
6367-
"code": 90015
6368-
},
63696365
"Declare property '{0}'": {
63706366
"category": "Message",
63716367
"code": 90016
@@ -6430,14 +6426,6 @@
64306426
"category": "Message",
64316427
"code": 90031
64326428
},
6433-
"Import default '{0}' from module \"{1}\"": {
6434-
"category": "Message",
6435-
"code": 90032
6436-
},
6437-
"Add default import '{0}' to existing import declaration from \"{1}\"": {
6438-
"category": "Message",
6439-
"code": 90033
6440-
},
64416429
"Add parameter name": {
64426430
"category": "Message",
64436431
"code": 90034
@@ -6482,6 +6470,14 @@
64826470
"category": "Message",
64836471
"code": 90056
64846472
},
6473+
"Add import from \"{0}\"": {
6474+
"category": "Message",
6475+
"code": 90057
6476+
},
6477+
"Update import from \"{0}\"": {
6478+
"category": "Message",
6479+
"code": 90058
6480+
},
64856481

64866482
"Convert function to an ES2015 class": {
64876483
"category": "Message",

‎src/services/codefixes/importFixes.ts

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,16 @@ namespace ts.codefix {
2020
const { errorCode, preferences, sourceFile, span, program } = context;
2121
const info = getFixesInfo(context, errorCode, span.start, /*useAutoImportProvider*/ true);
2222
if (!info) return undefined;
23-
const { fixes, symbolName } = info;
23+
const { fixes, symbolName, errorIdentifierText } = info;
2424
const quotePreference = getQuotePreference(sourceFile, preferences);
25-
return fixes.map(fix => codeActionForFix(context, sourceFile, symbolName, fix, quotePreference, program.getCompilerOptions()));
25+
return fixes.map(fix => codeActionForFix(
26+
context,
27+
sourceFile,
28+
symbolName,
29+
fix,
30+
/*includeSymbolNameInDescription*/ symbolName !== errorIdentifierText,
31+
quotePreference,
32+
program.getCompilerOptions()));
2633
},
2734
fixIds: [importFixId],
2835
getAllCodeActions: context => {
@@ -78,7 +85,7 @@ namespace ts.codefix {
7885
const useRequire = shouldUseRequire(sourceFile, program);
7986
const fix = getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, symbolName, program, /*position*/ undefined, !!isValidTypeOnlyUseSite, useRequire, host, preferences);
8087
if (fix) {
81-
addImport({ fixes: [fix], symbolName });
88+
addImport({ fixes: [fix], symbolName, errorIdentifierText: undefined });
8289
}
8390
}
8491

@@ -308,6 +315,7 @@ namespace ts.codefix {
308315
sourceFile,
309316
symbolName,
310317
fix,
318+
/*includeSymbolNameInDescription*/ false,
311319
getQuotePreference(sourceFile, preferences), compilerOptions))
312320
};
313321
}
@@ -316,7 +324,8 @@ namespace ts.codefix {
316324
const compilerOptions = program.getCompilerOptions();
317325
const symbolName = getSymbolName(sourceFile, program.getTypeChecker(), symbolToken, compilerOptions);
318326
const fix = getTypeOnlyPromotionFix(sourceFile, symbolToken, symbolName, program);
319-
return fix && codeFixActionToCodeAction(codeActionForFix({ host, formatContext, preferences }, sourceFile, symbolName, fix, QuotePreference.Double, compilerOptions));
327+
const includeSymbolNameInDescription = symbolName !== symbolToken.text;
328+
return fix && codeFixActionToCodeAction(codeActionForFix({ host, formatContext, preferences }, sourceFile, symbolName, fix, includeSymbolNameInDescription, QuotePreference.Double, compilerOptions));
320329
}
321330

322331
function getImportFixForSymbol(sourceFile: SourceFile, exportInfos: readonly SymbolExportInfo[], moduleSymbol: Symbol, symbolName: string, program: Program, position: number | undefined, isValidTypeOnlyUseSite: boolean, useRequire: boolean, host: LanguageServiceHost, preferences: UserPreferences) {
@@ -666,7 +675,7 @@ namespace ts.codefix {
666675
}
667676
}
668677

669-
interface FixesInfo { readonly fixes: readonly ImportFix[]; readonly symbolName: string; }
678+
interface FixesInfo { readonly fixes: readonly ImportFix[]; readonly symbolName: string; readonly errorIdentifierText: string | undefined; }
670679
function getFixesInfo(context: CodeFixContextBase, errorCode: number, pos: number, useAutoImportProvider: boolean): FixesInfo | undefined {
671680
const symbolToken = getTokenAtPosition(context.sourceFile, pos);
672681
let info;
@@ -679,7 +688,7 @@ namespace ts.codefix {
679688
else if (errorCode === Diagnostics._0_cannot_be_used_as_a_value_because_it_was_imported_using_import_type.code) {
680689
const symbolName = getSymbolName(context.sourceFile, context.program.getTypeChecker(), symbolToken, context.program.getCompilerOptions());
681690
const fix = getTypeOnlyPromotionFix(context.sourceFile, symbolToken, symbolName, context.program);
682-
return fix && { fixes: [fix], symbolName };
691+
return fix && { fixes: [fix], symbolName, errorIdentifierText: symbolToken.text };
683692
}
684693
else {
685694
info = getFixesInfoForNonUMDImport(context, symbolToken, useAutoImportProvider);
@@ -736,7 +745,7 @@ namespace ts.codefix {
736745
const exportInfos: readonly SymbolExportInfo[] = [{ symbol: umdSymbol, moduleSymbol: symbol, moduleFileName: undefined, exportKind: ExportKind.UMD, targetFlags: symbol.flags, isFromPackageJson: false }];
737746
const useRequire = shouldUseRequire(sourceFile, program);
738747
const fixes = getImportFixes(exportInfos, symbolName, isIdentifier(token) ? token.getStart(sourceFile) : undefined, /*isValidTypeOnlyUseSite*/ false, useRequire, program, sourceFile, host, preferences);
739-
return { fixes, symbolName };
748+
return { fixes, symbolName, errorIdentifierText: tryCast(token, isIdentifier)?.text };
740749
}
741750
function getUmdSymbol(token: Node, checker: TypeChecker): Symbol | undefined {
742751
// try the identifier to see if it is the umd symbol
@@ -808,7 +817,7 @@ namespace ts.codefix {
808817
const exportInfos = getExportInfos(symbolName, isJSXTagName(symbolToken), getMeaningFromLocation(symbolToken), cancellationToken, sourceFile, program, useAutoImportProvider, host, preferences);
809818
const fixes = arrayFrom(flatMapIterator(exportInfos.entries(), ([_, exportInfos]) =>
810819
getImportFixes(exportInfos, symbolName, symbolToken.getStart(sourceFile), isValidTypeOnlyUseSite, useRequire, program, sourceFile, host, preferences)));
811-
return { fixes, symbolName };
820+
return { fixes, symbolName, errorIdentifierText: symbolToken.text };
812821
}
813822

814823
function getTypeOnlyPromotionFix(sourceFile: SourceFile, symbolToken: Identifier, symbolName: string, program: Program): FixPromoteTypeOnlyImport | undefined {
@@ -920,14 +929,14 @@ namespace ts.codefix {
920929
return allowSyntheticDefaults ? ImportKind.Default : ImportKind.CommonJS;
921930
}
922931

923-
function codeActionForFix(context: textChanges.TextChangesContext, sourceFile: SourceFile, symbolName: string, fix: ImportFix, quotePreference: QuotePreference, compilerOptions: CompilerOptions): CodeFixAction {
932+
function codeActionForFix(context: textChanges.TextChangesContext, sourceFile: SourceFile, symbolName: string, fix: ImportFix, includeSymbolNameInDescription: boolean, quotePreference: QuotePreference, compilerOptions: CompilerOptions): CodeFixAction {
924933
let diag!: DiagnosticAndArguments;
925934
const changes = textChanges.ChangeTracker.with(context, tracker => {
926-
diag = codeActionForFixWorker(tracker, sourceFile, symbolName, fix, quotePreference, compilerOptions);
935+
diag = codeActionForFixWorker(tracker, sourceFile, symbolName, fix, includeSymbolNameInDescription, quotePreference, compilerOptions);
927936
});
928937
return createCodeFixAction(importFixName, changes, diag, importFixId, Diagnostics.Add_all_missing_imports);
929938
}
930-
function codeActionForFixWorker(changes: textChanges.ChangeTracker, sourceFile: SourceFile, symbolName: string, fix: ImportFix, quotePreference: QuotePreference, compilerOptions: CompilerOptions): DiagnosticAndArguments {
939+
function codeActionForFixWorker(changes: textChanges.ChangeTracker, sourceFile: SourceFile, symbolName: string, fix: ImportFix, includeSymbolNameInDescription: boolean, quotePreference: QuotePreference, compilerOptions: CompilerOptions): DiagnosticAndArguments {
931940
switch (fix.kind) {
932941
case ImportFixKind.UseNamespace:
933942
addNamespaceQualifier(changes, sourceFile, fix);
@@ -945,11 +954,9 @@ namespace ts.codefix {
945954
importKind === ImportKind.Named ? [{ name: symbolName, addAsTypeOnly }] : emptyArray,
946955
compilerOptions);
947956
const moduleSpecifierWithoutQuotes = stripQuotes(moduleSpecifier);
948-
return [
949-
importKind === ImportKind.Default ? Diagnostics.Add_default_import_0_to_existing_import_declaration_from_1 : Diagnostics.Add_0_to_existing_import_declaration_from_1,
950-
symbolName,
951-
moduleSpecifierWithoutQuotes
952-
]; // you too!
957+
return includeSymbolNameInDescription
958+
? [Diagnostics.Import_0_from_1, symbolName, moduleSpecifierWithoutQuotes]
959+
: [Diagnostics.Update_import_from_0, moduleSpecifierWithoutQuotes];
953960
}
954961
case ImportFixKind.AddNew: {
955962
const { importKind, moduleSpecifier, addAsTypeOnly, useRequire } = fix;
@@ -958,7 +965,9 @@ namespace ts.codefix {
958965
const namedImports: Import[] | undefined = importKind === ImportKind.Named ? [{ name: symbolName, addAsTypeOnly }] : undefined;
959966
const namespaceLikeImport = importKind === ImportKind.Namespace || importKind === ImportKind.CommonJS ? { importKind, name: symbolName, addAsTypeOnly } : undefined;
960967
insertImports(changes, sourceFile, getDeclarations(moduleSpecifier, quotePreference, defaultImport, namedImports, namespaceLikeImport), /*blankLineBetween*/ true);
961-
return [importKind === ImportKind.Default ? Diagnostics.Import_default_0_from_module_1 : Diagnostics.Import_0_from_module_1, symbolName, moduleSpecifier];
968+
return includeSymbolNameInDescription
969+
? [Diagnostics.Import_0_from_1, symbolName, moduleSpecifier]
970+
: [Diagnostics.Add_import_from_0, moduleSpecifier];
962971
}
963972
case ImportFixKind.PromoteTypeOnly: {
964973
const { typeOnlyAliasDeclaration } = fix;

‎src/testRunner/unittests/tsserver/completions.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ namespace ts.projectSystem {
8585
{
8686
codeActions: [
8787
{
88-
description: `Import 'foo' from module "./a"`,
88+
description: `Add import from "./a"`,
8989
changes: [
9090
{
9191
fileName: "/b.ts",
@@ -118,7 +118,7 @@ namespace ts.projectSystem {
118118
{
119119
codeActions: [
120120
{
121-
description: `Import 'foo' from module "./a"`,
121+
description: `Add import from "./a"`,
122122
changes: [
123123
{
124124
fileName: "/b.ts",

‎src/testRunner/unittests/tsserver/duplicatePackages.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ namespace ts.projectSystem {
3333
});
3434
assert.deepEqual<readonly protocol.CodeFixAction[] | undefined>(response, [
3535
{
36-
description: `Import 'foo' from module "foo"`,
36+
description: `Add import from "foo"`,
3737
fixName: "import",
3838
changes: [{
3939
fileName: user.path,

‎tests/baselines/reference/tsserver/projectReferences/auto-import-with-referenced-project-when-built-with-disableSourceOfProjectReferenceRedirect.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,4 +98,4 @@ response:{"responseRequired":false}
9898
request:{"command":"getCodeFixes","arguments":{"file":"/user/username/projects/myproject/app/src/program/index.ts","startLine":1,"startOffset":1,"endLine":1,"endOffset":4,"errorCodes":[2304]},"seq":1,"type":"request"}
9999
DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache
100100
Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache
101-
response:{"response":[{"fixName":"import","description":"Import 'foo' from module \"shared\"","changes":[{"fileName":"/user/username/projects/myproject/app/src/program/index.ts","textChanges":[{"start":{"line":1,"offset":1},"end":{"line":1,"offset":1},"newText":"import { foo } from \"shared\";\n\n"}]}]}],"responseRequired":true}
101+
response:{"response":[{"fixName":"import","description":"Add import from \"shared\"","changes":[{"fileName":"/user/username/projects/myproject/app/src/program/index.ts","textChanges":[{"start":{"line":1,"offset":1},"end":{"line":1,"offset":1},"newText":"import { foo } from \"shared\";\n\n"}]}]}],"responseRequired":true}

‎tests/baselines/reference/tsserver/projectReferences/auto-import-with-referenced-project-when-built.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,4 @@ response:{"responseRequired":false}
9797
request:{"command":"getCodeFixes","arguments":{"file":"/user/username/projects/myproject/app/src/program/index.ts","startLine":1,"startOffset":1,"endLine":1,"endOffset":4,"errorCodes":[2304]},"seq":1,"type":"request"}
9898
DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache
9999
Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache
100-
response:{"response":[{"fixName":"import","description":"Import 'foo' from module \"shared\"","changes":[{"fileName":"/user/username/projects/myproject/app/src/program/index.ts","textChanges":[{"start":{"line":1,"offset":1},"end":{"line":1,"offset":1},"newText":"import { foo } from \"shared\";\n\n"}]}]}],"responseRequired":true}
100+
response:{"response":[{"fixName":"import","description":"Add import from \"shared\"","changes":[{"fileName":"/user/username/projects/myproject/app/src/program/index.ts","textChanges":[{"start":{"line":1,"offset":1},"end":{"line":1,"offset":1},"newText":"import { foo } from \"shared\";\n\n"}]}]}],"responseRequired":true}

‎tests/baselines/reference/tsserver/projectReferences/auto-import-with-referenced-project.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,4 @@ response:{"responseRequired":false}
9797
request:{"command":"getCodeFixes","arguments":{"file":"/user/username/projects/myproject/app/src/program/index.ts","startLine":1,"startOffset":1,"endLine":1,"endOffset":4,"errorCodes":[2304]},"seq":1,"type":"request"}
9898
DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache
9999
Elapsed:: *ms DirectoryWatcher:: Added:: WatchInfo: /user/username/projects/myproject/node_modules 1 undefined WatchType: node_modules for closed script infos and package.jsons affecting module specifier cache
100-
response:{"response":[{"fixName":"import","description":"Import 'foo' from module \"shared\"","changes":[{"fileName":"/user/username/projects/myproject/app/src/program/index.ts","textChanges":[{"start":{"line":1,"offset":1},"end":{"line":1,"offset":1},"newText":"import { foo } from \"shared\";\n\n"}]}]}],"responseRequired":true}
100+
response:{"response":[{"fixName":"import","description":"Add import from \"shared\"","changes":[{"fileName":"/user/username/projects/myproject/app/src/program/index.ts","textChanges":[{"start":{"line":1,"offset":1},"end":{"line":1,"offset":1},"newText":"import { foo } from \"shared\";\n\n"}]}]}],"responseRequired":true}

‎tests/cases/fourslash/codeFixForgottenThisPropertyAccess01.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
goTo.file("/b.ts");
1616
verify.codeFixAvailable([
17-
{ description: `Import 'foo' from module "./a"` },
17+
{ description: `Add import from "./a"` },
1818
{ description: "Change spelling to 'fooo'" },
1919
{ description: "Add 'this.' to unresolved variable" },
2020
]);

‎tests/cases/fourslash/codeFixSpellingVsImport.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@
1111

1212
goTo.file("/b.ts");
1313
verify.codeFixAvailable([
14-
{ description: `Import 'foo' from module "./a"` },
14+
{ description: `Add import from "./a"` },
1515
{ description: "Change spelling to 'foof'" },
1616
]);

‎tests/cases/fourslash/completionsImportFromJSXTag.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
verify.applyCodeActionFromCompletion("", {
2323
name: "Box",
2424
source: "/Box",
25-
description: `Import 'Box' from module "./Box"`,
25+
description: `Add import from "./Box"`,
2626
newFileContent: `import { Box } from "./Box";
2727
2828
export function App() {

0 commit comments

Comments
 (0)