Skip to content

Commit

Permalink
add support for autoimports/moveToFile to generate aliased named impo…
Browse files Browse the repository at this point in the history
…rts (#59885)
  • Loading branch information
iisaduan authored Sep 10, 2024
1 parent 0d7763e commit 89e004f
Show file tree
Hide file tree
Showing 5 changed files with 197 additions and 16 deletions.
48 changes: 32 additions & 16 deletions src/services/codefixes/importFixes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ export function createImportAdder(sourceFile: SourceFile | FutureSourceFile, pro
interface AddToExistingState {
readonly importClauseOrBindingPattern: ImportClause | ObjectBindingPattern;
defaultImport: Import | undefined;
readonly namedImports: Map<string, AddAsTypeOnly>;
readonly namedImports: Map<string, { addAsTypeOnly: AddAsTypeOnly; propertyName?: string | undefined; }>;
}

function createImportAdderWorker(sourceFile: SourceFile | FutureSourceFile, program: Program, useAutoImportProvider: boolean, preferences: UserPreferences, host: LanguageServiceHost, cancellationToken: CancellationToken | undefined): ImportAdder {
Expand Down Expand Up @@ -290,15 +290,28 @@ 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)
&& (fix.kind === ImportFixKind.AddNew || fix.kind === ImportFixKind.AddToExisting)
&& 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 });
}
}
Expand Down Expand Up @@ -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");
Expand All @@ -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");

Expand All @@ -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");
Expand Down Expand Up @@ -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,
);
Expand All @@ -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,
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
Expand All @@ -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),
)
),
Expand Down Expand Up @@ -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<string, AddAsTypeOnly>;
readonly namedImports?: Map<string, [AddAsTypeOnly, /*propertyName*/ string?]>;
readonly namespaceLikeImport?: {
readonly importKind: ImportKind.CommonJS | ImportKind.Namespace;
readonly name: string;
Expand Down Expand Up @@ -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),
)
),
Expand Down Expand Up @@ -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));
}
Expand Down
43 changes: 43 additions & 0 deletions tests/cases/fourslash/moveToFile_alias.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@

/// <reference path='fourslash.ts' />


// @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" },
});
43 changes: 43 additions & 0 deletions tests/cases/fourslash/moveToFile_alias2.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@

/// <reference path='fourslash.ts' />


// @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" },
});
42 changes: 42 additions & 0 deletions tests/cases/fourslash/moveToFile_alias3.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@

/// <reference path='fourslash.ts' />


// @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" },
});
37 changes: 37 additions & 0 deletions tests/cases/fourslash/moveToNewFile_alias.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@

/// <reference path='fourslash.ts' />


// @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();
}
}
`
}
});

0 comments on commit 89e004f

Please sign in to comment.