diff --git a/src/services/refactors/convertImport.ts b/src/services/refactors/convertImport.ts index e84cb4ef668c7..848d37c7a5945 100644 --- a/src/services/refactors/convertImport.ts +++ b/src/services/refactors/convertImport.ts @@ -138,13 +138,36 @@ namespace ts.refactor { const importDecl = toConvert.parent.parent; const { moduleSpecifier } = importDecl; + const toConvertSymbols: Set = new Set(); + toConvert.elements.forEach(namedImport => { + const symbol = checker.getSymbolAtLocation(namedImport.name); + if (symbol) { + toConvertSymbols.add(symbol); + } + }); const preferredName = moduleSpecifier && isStringLiteral(moduleSpecifier) ? codefix.moduleSpecifierToValidIdentifier(moduleSpecifier.text, ScriptTarget.ESNext) : "module"; - const namespaceNameConflicts = toConvert.elements.some(element => - FindAllReferences.Core.eachSymbolReferenceInFile(element.name, checker, sourceFile, id => - !!checker.resolveName(preferredName, id, SymbolFlags.All, /*excludeGlobals*/ true)) || false); + function hasNamespaceNameConflict(namedImport: ImportSpecifier): boolean { + // We need to check if the preferred namespace name (`preferredName`) we'd like to use in the refactored code will present a name conflict. + // A name conflict means that, in a scope where we would like to use the preferred namespace name, there already exists a symbol with that name in that scope. + // We are going to use the namespace name in the scopes the named imports being refactored are referenced, + // so we look for conflicts by looking at every reference to those named imports. + return !!FindAllReferences.Core.eachSymbolReferenceInFile(namedImport.name, checker, sourceFile, id => { + const symbol = checker.resolveName(preferredName, id, SymbolFlags.All, /*excludeGlobals*/ true); + if (symbol) { // There already is a symbol with the same name as the preferred namespace name. + if (toConvertSymbols.has(symbol)) { // `preferredName` resolves to a symbol for one of the named import references we are going to transform into namespace import references... + return isExportSpecifier(id.parent); // ...but if this reference is an export specifier, it will not be transformed, so it is a conflict; otherwise, it will be renamed and is not a conflict. + } + return true; // `preferredName` resolves to any other symbol, which will be present in the refactored code and so poses a name conflict. + } + return false; // There is no symbol with the same name as the preferred namespace name, so no conflict. + }); + } + const namespaceNameConflicts = toConvert.elements.some(hasNamespaceNameConflict); const namespaceImportName = namespaceNameConflicts ? getUniqueName(preferredName, sourceFile) : preferredName; - const neededNamedImports: ImportSpecifier[] = []; + // Imports that need to be kept as named imports in the refactored code, to avoid changing the semantics. + // More specifically, those are named imports that appear in named exports in the original code, e.g. `a` in `import { a } from "m"; export { a }`. + const neededNamedImports: Set = new Set(); for (const element of toConvert.elements) { const propertyName = (element.propertyName || element.name).text; @@ -153,10 +176,8 @@ namespace ts.refactor { if (isShorthandPropertyAssignment(id.parent)) { changes.replaceNode(sourceFile, id.parent, factory.createPropertyAssignment(id.text, access)); } - else if (isExportSpecifier(id.parent) && !id.parent.propertyName) { - if (!neededNamedImports.some(n => n.name === element.name)) { - neededNamedImports.push(factory.createImportSpecifier(element.propertyName && factory.createIdentifier(element.propertyName.text), factory.createIdentifier(element.name.text))); - } + else if (isExportSpecifier(id.parent)) { + neededNamedImports.add(element); } else { changes.replaceNode(sourceFile, id, access); @@ -165,8 +186,10 @@ namespace ts.refactor { } changes.replaceNode(sourceFile, toConvert, factory.createNamespaceImport(factory.createIdentifier(namespaceImportName))); - if (neededNamedImports.length) { - changes.insertNodeAfter(sourceFile, toConvert.parent.parent, updateImport(importDecl, /*defaultImportName*/ undefined, neededNamedImports)); + if (neededNamedImports.size) { + const newNamedImports: ImportSpecifier[] = arrayFrom(neededNamedImports.values()).map(element => + factory.createImportSpecifier(element.propertyName && factory.createIdentifier(element.propertyName.text), factory.createIdentifier(element.name.text))); + changes.insertNodeAfter(sourceFile, toConvert.parent.parent, updateImport(importDecl, /*defaultImportName*/ undefined, newNamedImports)); } } diff --git a/tests/cases/fourslash/refactorConvertImport_namedToNamespace1.ts b/tests/cases/fourslash/refactorConvertImport_namedToNamespace1.ts new file mode 100644 index 0000000000000..4fb00e80e8d21 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertImport_namedToNamespace1.ts @@ -0,0 +1,19 @@ +/// + +/////*a*/import { a, b, foo } from "./foo";/*b*/ +////a; +////b; +////foo; + + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert import", + actionName: "Convert named imports to namespace import", + actionDescription: "Convert named imports to namespace import", + newContent: +`import * as foo from "./foo"; +foo.a; +foo.b; +foo.foo;`, +}); diff --git a/tests/cases/fourslash/refactorConvertImport_namedToNamespace2.ts b/tests/cases/fourslash/refactorConvertImport_namedToNamespace2.ts new file mode 100644 index 0000000000000..7ef5adbbd07ea --- /dev/null +++ b/tests/cases/fourslash/refactorConvertImport_namedToNamespace2.ts @@ -0,0 +1,31 @@ +/// + +////import foo, /*a*/{ a, b, c as d }/*b*/ from "./foo"; +////a; +////b; +////d; +////foo; +////export default a; +////export { b }; +////export { d }; +////export { foo }; + + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert import", + actionName: "Convert named imports to namespace import", + actionDescription: "Convert named imports to namespace import", + triggerReason: "invoked", + newContent: +`import foo, * as foo_1 from "./foo"; +import { b, c as d } from "./foo"; +foo_1.a; +foo_1.b; +foo_1.c; +foo; +export default foo_1.a; +export { b }; +export { d }; +export { foo };`, +}); diff --git a/tests/cases/fourslash/refactorConvertImport_namedToNamespace3.ts b/tests/cases/fourslash/refactorConvertImport_namedToNamespace3.ts new file mode 100644 index 0000000000000..4ce38adc2bd04 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertImport_namedToNamespace3.ts @@ -0,0 +1,30 @@ +/// + +/////*a*/import { a, b } from "./foo"/*b*/; +////a; +////b; +////function newScope() { +//// const foo = "foo"; +//// foo; +//// return a; +////} +////newScope(); + + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert import", + actionName: "Convert named imports to namespace import", + actionDescription: "Convert named imports to namespace import", + triggerReason: "invoked", + newContent: +`import * as foo_1 from "./foo"; +foo_1.a; +foo_1.b; +function newScope() { + const foo = "foo"; + foo; + return foo_1.a; +} +newScope();`, +}); diff --git a/tests/cases/fourslash/refactorConvertImport_namedToNamespace4.ts b/tests/cases/fourslash/refactorConvertImport_namedToNamespace4.ts new file mode 100644 index 0000000000000..bce4240fa092e --- /dev/null +++ b/tests/cases/fourslash/refactorConvertImport_namedToNamespace4.ts @@ -0,0 +1,30 @@ +/// + +/////*a*/import { a, b, c as d } from "./foo"/*b*/; +////a; +////b; +////d; +////export default a; +////export { b }; +////export { d }; +////export { d as e }; +////export { b as f }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert import", + actionName: "Convert named imports to namespace import", + actionDescription: "Convert named imports to namespace import", + triggerReason: "invoked", + newContent: +`import * as foo from "./foo"; +import { b, c as d } from "./foo"; +foo.a; +foo.b; +foo.c; +export default foo.a; +export { b }; +export { d }; +export { d as e }; +export { b as f };`, +}); diff --git a/tests/cases/fourslash/refactorConvertImport_namedToNamespace5.ts b/tests/cases/fourslash/refactorConvertImport_namedToNamespace5.ts new file mode 100644 index 0000000000000..5471be75c9ca7 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertImport_namedToNamespace5.ts @@ -0,0 +1,24 @@ +/// + +/////*a*/import { a, b, foo } from "./foo";/*b*/ +////a; +////b; +////foo; +////export { foo }; +////export { foo as bar }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert import", + actionName: "Convert named imports to namespace import", + actionDescription: "Convert named imports to namespace import", + triggerReason: "invoked", + newContent: +`import * as foo_1 from "./foo"; +import { foo } from "./foo"; +foo_1.a; +foo_1.b; +foo_1.foo; +export { foo }; +export { foo as bar };`, +});