From 89e004f632323a276b67649e118e78f39a7dc429 Mon Sep 17 00:00:00 2001 From: Isabel Duan Date: Tue, 10 Sep 2024 15:38:37 -0700 Subject: [PATCH] add support for autoimports/moveToFile to generate aliased named imports (#59885) --- src/services/codefixes/importFixes.ts | 48 +++++++++++++------- tests/cases/fourslash/moveToFile_alias.ts | 43 ++++++++++++++++++ tests/cases/fourslash/moveToFile_alias2.ts | 43 ++++++++++++++++++ tests/cases/fourslash/moveToFile_alias3.ts | 42 +++++++++++++++++ tests/cases/fourslash/moveToNewFile_alias.ts | 37 +++++++++++++++ 5 files changed, 197 insertions(+), 16 deletions(-) create mode 100644 tests/cases/fourslash/moveToFile_alias.ts create mode 100644 tests/cases/fourslash/moveToFile_alias2.ts create mode 100644 tests/cases/fourslash/moveToFile_alias3.ts create mode 100644 tests/cases/fourslash/moveToNewFile_alias.ts diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 063ed6946eee4..ad7f7c63022ff 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -241,7 +241,7 @@ export function createImportAdder(sourceFile: SourceFile | FutureSourceFile, pro interface AddToExistingState { readonly importClauseOrBindingPattern: ImportClause | ObjectBindingPattern; defaultImport: Import | undefined; - readonly namedImports: Map; + readonly namedImports: Map; } function createImportAdderWorker(sourceFile: SourceFile | FutureSourceFile, program: Program, useAutoImportProvider: boolean, preferences: UserPreferences, host: LanguageServiceHost, cancellationToken: CancellationToken | undefined): ImportAdder { @@ -290,6 +290,8 @@ function createImportAdderWorker(sourceFile: SourceFile | FutureSourceFile, prog let fix = getImportFixForSymbol(sourceFile, exportInfo, program, /*position*/ undefined, !!isValidTypeOnlyUseSite, useRequire, host, preferences); if (fix) { const localName = tryCast(referenceImport?.name, isIdentifier)?.text ?? symbolName; + let addAsTypeOnly: AddAsTypeOnly | undefined; + let propertyName: string | undefined; if ( referenceImport && isTypeOnlyImportDeclaration(referenceImport) @@ -297,8 +299,19 @@ function createImportAdderWorker(sourceFile: SourceFile | FutureSourceFile, prog && fix.addAsTypeOnly === AddAsTypeOnly.Allowed ) { // Copy the type-only status from the reference import - fix = { ...fix, addAsTypeOnly: AddAsTypeOnly.Required }; + addAsTypeOnly = AddAsTypeOnly.Required; } + + if (exportedSymbol.name !== localName) { + // checks if the symbol was aliased at the referenced import + propertyName = exportedSymbol.name; + } + + fix = { + ...fix, + ...(addAsTypeOnly === undefined ? {} : { addAsTypeOnly }), + ...(propertyName === undefined ? {} : { propertyName }), + }; addImport({ fix, symbolName: localName ?? symbolName, errorIdentifierText: undefined }); } } @@ -375,14 +388,14 @@ function createImportAdderWorker(sourceFile: SourceFile | FutureSourceFile, prog importType.push(fix); break; case ImportFixKind.AddToExisting: { - const { importClauseOrBindingPattern, importKind, addAsTypeOnly } = fix; + const { importClauseOrBindingPattern, importKind, addAsTypeOnly, propertyName } = fix; let entry = addToExisting.get(importClauseOrBindingPattern); if (!entry) { addToExisting.set(importClauseOrBindingPattern, entry = { importClauseOrBindingPattern, defaultImport: undefined, namedImports: new Map() }); } if (importKind === ImportKind.Named) { - const prevValue = entry?.namedImports.get(symbolName); - entry.namedImports.set(symbolName, reduceAddAsTypeOnlyValues(prevValue, addAsTypeOnly)); + const prevTypeOnly = entry?.namedImports.get(symbolName)?.addAsTypeOnly; + entry.namedImports.set(symbolName, { addAsTypeOnly: reduceAddAsTypeOnlyValues(prevTypeOnly, addAsTypeOnly), propertyName }); } else { Debug.assert(entry.defaultImport === undefined || entry.defaultImport.name === symbolName, "(Add to Existing) Default import should be missing or match symbolName"); @@ -394,7 +407,7 @@ function createImportAdderWorker(sourceFile: SourceFile | FutureSourceFile, prog break; } case ImportFixKind.AddNew: { - const { moduleSpecifier, importKind, useRequire, addAsTypeOnly } = fix; + const { moduleSpecifier, importKind, useRequire, addAsTypeOnly, propertyName } = fix; const entry = getNewImportEntry(moduleSpecifier, importKind, useRequire, addAsTypeOnly); Debug.assert(entry.useRequire === useRequire, "(Add new) Tried to add an `import` and a `require` for the same module"); @@ -405,12 +418,12 @@ function createImportAdderWorker(sourceFile: SourceFile | FutureSourceFile, prog break; case ImportKind.Named: const prevValue = (entry.namedImports ||= new Map()).get(symbolName); - entry.namedImports.set(symbolName, reduceAddAsTypeOnlyValues(prevValue, addAsTypeOnly)); + entry.namedImports.set(symbolName, [reduceAddAsTypeOnlyValues(prevValue, addAsTypeOnly), propertyName]); break; case ImportKind.CommonJS: if (compilerOptions.verbatimModuleSyntax) { const prevValue = (entry.namedImports ||= new Map()).get(symbolName); - entry.namedImports.set(symbolName, reduceAddAsTypeOnlyValues(prevValue, addAsTypeOnly)); + entry.namedImports.set(symbolName, [reduceAddAsTypeOnlyValues(prevValue, addAsTypeOnly), propertyName]); } else { Debug.assert(entry.namespaceLikeImport === undefined || entry.namespaceLikeImport.name === symbolName, "Namespacelike import shoudl be missing or match symbolName"); @@ -582,7 +595,7 @@ function createImportAdderWorker(sourceFile: SourceFile | FutureSourceFile, prog sourceFile as SourceFile, importClauseOrBindingPattern, defaultImport, - arrayFrom(namedImports.entries(), ([name, addAsTypeOnly]) => ({ addAsTypeOnly, name })), + arrayFrom(namedImports.entries(), ([name, { addAsTypeOnly, propertyName }]) => ({ addAsTypeOnly, propertyName, name })), importSpecifiersToRemoveWhileAdding, preferences, ); @@ -596,7 +609,7 @@ function createImportAdderWorker(sourceFile: SourceFile | FutureSourceFile, prog moduleSpecifier, quotePreference, defaultImport, - namedImports && arrayFrom(namedImports.entries(), ([name, addAsTypeOnly]) => ({ addAsTypeOnly, name })), + namedImports && arrayFrom(namedImports.entries(), ([name, [addAsTypeOnly, propertyName]]) => ({ addAsTypeOnly, propertyName, name })), namespaceLikeImport, compilerOptions, preferences, @@ -772,11 +785,13 @@ interface FixAddToExistingImport extends ImportFixBase { readonly importClauseOrBindingPattern: ImportClause | ObjectBindingPattern; readonly importKind: ImportKind.Default | ImportKind.Named; readonly addAsTypeOnly: AddAsTypeOnly; + readonly propertyName?: string; } interface FixAddNewImport extends ImportFixBase { readonly kind: ImportFixKind.AddNew; readonly importKind: ImportKind; readonly addAsTypeOnly: AddAsTypeOnly; + readonly propertyName?: string; readonly useRequire: boolean; readonly qualification?: Qualification; } @@ -1794,7 +1809,7 @@ function doAddExistingFix( factory.createObjectBindingPattern([ ...clause.elements.filter(e => !removeExistingImportSpecifiers.has(e)), ...defaultImport ? [factory.createBindingElement(/*dotDotDotToken*/ undefined, /*propertyName*/ "default", defaultImport.name)] : emptyArray, - ...namedImports.map(i => factory.createBindingElement(/*dotDotDotToken*/ undefined, /*propertyName*/ undefined, i.name)), + ...namedImports.map(i => factory.createBindingElement(/*dotDotDotToken*/ undefined, i.propertyName, i.name)), ]), ); return; @@ -1803,7 +1818,7 @@ function doAddExistingFix( addElementToBindingPattern(clause, defaultImport.name, "default"); } for (const specifier of namedImports) { - addElementToBindingPattern(clause, specifier.name, /*propertyName*/ undefined); + addElementToBindingPattern(clause, specifier.name, specifier.propertyName); } return; } @@ -1823,7 +1838,7 @@ function doAddExistingFix( namedImports.map(namedImport => factory.createImportSpecifier( (!clause.isTypeOnly || promoteFromTypeOnly) && shouldUseTypeOnly(namedImport, preferences), - /*propertyName*/ undefined, + namedImport.propertyName === undefined ? undefined : factory.createIdentifier(namedImport.propertyName), factory.createIdentifier(namedImport.name), ) ), @@ -1919,11 +1934,12 @@ function getImportTypePrefix(moduleSpecifier: string, quotePreference: QuotePref interface Import { readonly name: string; readonly addAsTypeOnly: AddAsTypeOnly; + readonly propertyName?: string; // Use when needing to generate an `ImportSpecifier with a `propertyName`; the name preceding "as" keyword (undefined when "as" is absent) } interface ImportsCollection { readonly defaultImport?: Import; - readonly namedImports?: Map; + readonly namedImports?: Map; readonly namespaceLikeImport?: { readonly importKind: ImportKind.CommonJS | ImportKind.Namespace; readonly name: string; @@ -1964,7 +1980,7 @@ function getNewImports( namedImports?.map(namedImport => factory.createImportSpecifier( !topLevelTypeOnly && shouldUseTypeOnly(namedImport, preferences), - /*propertyName*/ undefined, + namedImport.propertyName === undefined ? undefined : factory.createIdentifier(namedImport.propertyName), factory.createIdentifier(namedImport.name), ) ), @@ -2003,7 +2019,7 @@ function getNewRequires(moduleSpecifier: string, quotePreference: QuotePreferenc let statements: RequireVariableStatement | readonly RequireVariableStatement[] | undefined; // const { default: foo, bar, etc } = require('./mod'); if (defaultImport || namedImports?.length) { - const bindingElements = namedImports?.map(({ name }) => factory.createBindingElement(/*dotDotDotToken*/ undefined, /*propertyName*/ undefined, name)) || []; + const bindingElements = namedImports?.map(({ name, propertyName }) => factory.createBindingElement(/*dotDotDotToken*/ undefined, propertyName, name)) || []; if (defaultImport) { bindingElements.unshift(factory.createBindingElement(/*dotDotDotToken*/ undefined, "default", defaultImport.name)); } diff --git a/tests/cases/fourslash/moveToFile_alias.ts b/tests/cases/fourslash/moveToFile_alias.ts new file mode 100644 index 0000000000000..ee02d08e0fe62 --- /dev/null +++ b/tests/cases/fourslash/moveToFile_alias.ts @@ -0,0 +1,43 @@ + +/// + + +// @filename: /producer.ts +//// export function doit() {} + +// @filename: /test.ts +//// import { doit as doit2 } from "./producer"; +//// +//// class Another {} +//// +//// [|class Consumer { +//// constructor() { +//// doit2(); +//// } +//// }|] + +// @filename: /consumer.ts +//// + + +verify.moveToFile({ + newFileContents: { + "/test.ts": +` +class Another {} + +`, + + "/consumer.ts": +`import { doit as doit2 } from "./producer"; + + +class Consumer { + constructor() { + doit2(); + } +} +` + }, + interactiveRefactorArguments: { targetFile: "/consumer.ts" }, +}); diff --git a/tests/cases/fourslash/moveToFile_alias2.ts b/tests/cases/fourslash/moveToFile_alias2.ts new file mode 100644 index 0000000000000..ff81c6198b03e --- /dev/null +++ b/tests/cases/fourslash/moveToFile_alias2.ts @@ -0,0 +1,43 @@ + +/// + + +// @filename: /producer.ts +//// export function doit() {}; +//// export const x = 1; + +// @filename: /test.ts +//// import { doit as doit2 } from "./producer"; +//// +//// class Another {} +//// +//// [|class Consumer { +//// constructor() { +//// doit2(); +//// } +//// }|] + +// @filename: /consumer.ts +//// import { x } from "./producer"; +//// x; + +verify.moveToFile({ + newFileContents: { + "/test.ts": +` +class Another {} + +`, + + "/consumer.ts": +`import { doit as doit2, x } from "./producer"; +x; +class Consumer { + constructor() { + doit2(); + } +} +` + }, + interactiveRefactorArguments: { targetFile: "/consumer.ts" }, +}); diff --git a/tests/cases/fourslash/moveToFile_alias3.ts b/tests/cases/fourslash/moveToFile_alias3.ts new file mode 100644 index 0000000000000..46fe048a9dfdd --- /dev/null +++ b/tests/cases/fourslash/moveToFile_alias3.ts @@ -0,0 +1,42 @@ + +/// + + +// @filename: /producer.ts +//// export function doit() {}; + +// @filename: /test.ts +//// import { doit as doit2 } from "./producer"; +//// +//// class Another {} +//// +//// [|class Consumer { +//// constructor() { +//// doit2(); +//// } +//// }|] + +// @filename: /consumer.ts +//// import { doit } from "./producer"; // existing import does not change when alias imported +//// doit(); + +verify.moveToFile({ + newFileContents: { + "/test.ts": +` +class Another {} + +`, + + "/consumer.ts": +`import { doit } from "./producer"; // existing import does not change when alias imported +doit(); +class Consumer { + constructor() { + doit2(); + } +} +` + }, + interactiveRefactorArguments: { targetFile: "/consumer.ts" }, +}); diff --git a/tests/cases/fourslash/moveToNewFile_alias.ts b/tests/cases/fourslash/moveToNewFile_alias.ts new file mode 100644 index 0000000000000..6a6d536a37c67 --- /dev/null +++ b/tests/cases/fourslash/moveToNewFile_alias.ts @@ -0,0 +1,37 @@ + +/// + + +// @filename: /producer.ts +//// export function doit() {} + +// @filename: /test.ts +//// import { doit as doit2 } from "./producer"; +//// +//// class Another {} +//// +//// [|class Consumer { +//// constructor() { +//// doit2(); +//// } +//// }|] + +verify.moveToNewFile({ + newFileContents: { + "/test.ts": +` +class Another {} + +`, + + "/Consumer.ts": +`import { doit as doit2 } from "./producer"; + +class Consumer { + constructor() { + doit2(); + } +} +` + } +});