Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -4289,5 +4289,13 @@
"Convert '{0}' to mapped object type": {
"category": "Message",
"code": 95055
},
"Convert namespace import to named imports": {
"category": "Message",
"code": 95056
},
"Convert named imports to namespace import": {
"category": "Message",
"code": 95057
}
}
8 changes: 4 additions & 4 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,11 +223,11 @@ namespace ts {
}

/**
* Returns a value indicating whether a name is unique globally or within the current file
* Returns a value indicating whether a name is unique globally or within the current file.
* Note: This does not consider whether a name appears as a free identifier or not. For that, use `forEachFreeIdentifier`.
*/
export function isFileLevelUniqueName(currentSourceFile: SourceFile, name: string, hasGlobalName?: PrintHandlers["hasGlobalName"]): boolean {
return !(hasGlobalName && hasGlobalName(name))
&& !currentSourceFile.identifiers.has(name);
export function isFileLevelUniqueName(sourceFile: SourceFile, name: string, hasGlobalName?: PrintHandlers["hasGlobalName"]): boolean {
return !(hasGlobalName && hasGlobalName(name)) && !sourceFile.identifiers.has(name);
}

// Returns true if this node is missing from the actual source code. A 'missing' node is different
Expand Down
1 change: 1 addition & 0 deletions src/harness/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@
"../services/codefixes/useDefaultImport.ts",
"../services/codefixes/fixAddModuleReferTypeMissingTypeof.ts",
"../services/codefixes/convertToMappedObjectType.ts",
"../services/refactors/convertImport.ts",
"../services/refactors/extractSymbol.ts",
"../services/refactors/generateGetAccessorAndSetAccessor.ts",
"../services/refactors/moveToNewFile.ts",
Expand Down
1 change: 1 addition & 0 deletions src/server/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@
"../services/codefixes/useDefaultImport.ts",
"../services/codefixes/fixAddModuleReferTypeMissingTypeof.ts",
"../services/codefixes/convertToMappedObjectType.ts",
"../services/refactors/convertImport.ts",
"../services/refactors/extractSymbol.ts",
"../services/refactors/generateGetAccessorAndSetAccessor.ts",
"../services/refactors/moveToNewFile.ts",
Expand Down
1 change: 1 addition & 0 deletions src/server/tsconfig.library.json
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@
"../services/codefixes/useDefaultImport.ts",
"../services/codefixes/fixAddModuleReferTypeMissingTypeof.ts",
"../services/codefixes/convertToMappedObjectType.ts",
"../services/refactors/convertImport.ts",
"../services/refactors/extractSymbol.ts",
"../services/refactors/generateGetAccessorAndSetAccessor.ts",
"../services/refactors/moveToNewFile.ts",
Expand Down
19 changes: 1 addition & 18 deletions src/services/codefixes/convertToEs6Module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -437,27 +437,10 @@ namespace ts.codefix {
type FreeIdentifiers = ReadonlyMap<ReadonlyArray<Identifier>>;
function collectFreeIdentifiers(file: SourceFile): FreeIdentifiers {
const map = createMultiMap<Identifier>();
file.forEachChild(function recur(node) {
if (isIdentifier(node) && isFreeIdentifier(node)) {
map.add(node.text, node);
}
node.forEachChild(recur);
});
forEachFreeIdentifier(file, id => map.add(id.text, id));
return map;
}

function isFreeIdentifier(node: Identifier): boolean {
const { parent } = node;
switch (parent.kind) {
case SyntaxKind.PropertyAccessExpression:
return (parent as PropertyAccessExpression).name !== node;
case SyntaxKind.BindingElement:
return (parent as BindingElement).propertyName !== node;
default:
return true;
}
}

// Node helpers

function functionExpressionToDeclaration(name: string | undefined, additionalModifiers: ReadonlyArray<Modifier>, fn: FunctionExpression | ArrowFunction | MethodDeclaration): FunctionDeclaration {
Expand Down
23 changes: 15 additions & 8 deletions src/services/findAllReferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -712,16 +712,23 @@ namespace ts.FindAllReferences.Core {
}

/** Used as a quick check for whether a symbol is used at all in a file (besides its definition). */
export function isSymbolReferencedInFile(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile) {
export function isSymbolReferencedInFile(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile): boolean {
return eachSymbolReferenceInFile(definition, checker, sourceFile, () => true) || false;
}

export function eachSymbolReferenceInFile<T>(definition: Identifier, checker: TypeChecker, sourceFile: SourceFile, cb: (token: Identifier) => T): T | undefined {
const symbol = checker.getSymbolAtLocation(definition);
if (!symbol) return true; // Be lenient with invalid code.
return getPossibleSymbolReferenceNodes(sourceFile, symbol.name).some(token => {
if (!isIdentifier(token) || token === definition || token.escapedText !== definition.escapedText) return false;
const referenceSymbol = checker.getSymbolAtLocation(token)!;
return referenceSymbol === symbol
if (!symbol) return undefined;
for (const token of getPossibleSymbolReferenceNodes(sourceFile, symbol.name)) {
if (!isIdentifier(token) || token === definition || token.escapedText !== definition.escapedText) continue;
const referenceSymbol: Symbol = checker.getSymbolAtLocation(token)!; // See GH#19955 for why the type annotation is necessary
if (referenceSymbol === symbol
|| checker.getShorthandAssignmentValueSymbol(token.parent) === symbol
|| isExportSpecifier(token.parent) && getLocalSymbolForExportSpecifier(token, referenceSymbol, token.parent, checker) === symbol;
});
|| isExportSpecifier(token.parent) && getLocalSymbolForExportSpecifier(token, referenceSymbol, token.parent, checker) === symbol) {
const res = cb(token);
if (res) return res;
}
}
}

function getPossibleSymbolReferenceNodes(sourceFile: SourceFile, symbolName: string, container: Node = sourceFile): ReadonlyArray<Node> {
Expand Down
104 changes: 104 additions & 0 deletions src/services/refactors/convertImport.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/* @internal */
namespace ts.refactor.generateGetAccessorAndSetAccessor {
const refactorName = "Convert import";
const actionNameNamespaceToNamed = "Convert namespace import to named imports";
const actionNameNamedToNamespace = "Convert named imports to namespace import";
registerRefactor(refactorName, {
getAvailableActions(context): ApplicableRefactorInfo[] | undefined {
const i = getImportToConvert(context);
if (!i) return undefined;
const description = i.kind === SyntaxKind.NamespaceImport ? Diagnostics.Convert_namespace_import_to_named_imports.message : Diagnostics.Convert_named_imports_to_namespace_import.message;
const actionName = i.kind === SyntaxKind.NamespaceImport ? actionNameNamespaceToNamed : actionNameNamedToNamespace;
return [{ name: refactorName, description, actions: [{ name: actionName, description }] }];
},
getEditsForAction(context, actionName): RefactorEditInfo {
Debug.assert(actionName === actionNameNamespaceToNamed || actionName === actionNameNamedToNamespace);
const edits = textChanges.ChangeTracker.with(context, t => doChange(context.file, context.program, t, Debug.assertDefined(getImportToConvert(context))));
return { edits, renameFilename: undefined, renameLocation: undefined };
}
});

// Can convert imports of the form `import * as m from "m";` or `import d, { x, y } from "m";`.
function getImportToConvert(context: RefactorContext): NamedImportBindings | undefined {
const { file } = context;
const span = getRefactorContextSpan(context);
const token = getTokenAtPosition(file, span.start, /*includeJsDocComment*/ false);
const importDecl = getParentNodeInSpan(token, file, span);
if (!importDecl || !isImportDeclaration(importDecl)) return undefined;
const { importClause } = importDecl;
return importClause && importClause.namedBindings;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we verify that the namedBindings is within the span..

e.g. selecting d in import d, * as ns from "./mod" should not trigger any action..

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a test. If just d is selected, getParentNodeInSpan will return just d and not the entire import declaration.

}

function doChange(sourceFile: SourceFile, program: Program, changes: textChanges.ChangeTracker, toConvert: NamedImportBindings): void {
const usedIdentifiers = createMap<true>();
forEachFreeIdentifier(sourceFile, id => usedIdentifiers.set(id.text, true));
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like isFileLevelUniqueName isn't suitable for this purpose since sourceFile.identifiers includes the text of every Identifier node in the source file, including the names in property accesses. So if we access import * as m from "m"; m.x then that would make us convert it to import { x as _ x } from "m"; unnecessarily.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

D'oh.. did not think of this one.. i guess we need a walk after all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one other option is to call resolveName for the property name on every reference for the namespace import, if it resolves to anything then you know you need to alias it.

const checker = program.getTypeChecker();

if (toConvert.kind === SyntaxKind.NamespaceImport) {
doChangeNamespaceToNamed(sourceFile, checker, changes, toConvert, usedIdentifiers, getAllowSyntheticDefaultImports(program.getCompilerOptions()));
}
else {
doChangeNamedToNamespace(sourceFile, checker, changes, toConvert, usedIdentifiers);
}
}

function doChangeNamespaceToNamed(sourceFile: SourceFile, checker: TypeChecker, changes: textChanges.ChangeTracker, toConvert: NamespaceImport, usedIdentifiers: ReadonlyMap<true>, allowSyntheticDefaultImports: boolean): void {
// We may need to change `mod.x` to `_x` to avoid a name conflict.
const exportNameToImportName = createMap<string>();
let usedAsNamespaceOrDefault = false;

FindAllReferences.Core.eachSymbolReferenceInFile(toConvert.name, checker, sourceFile, id => {
if (!isPropertyAccessExpression(id.parent)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if it is used in a call expression.. import * as n from "mod"; n();?
if allowSyntheticDefaultImport or esModuleInterop are on, then you can change it to import {default as n} from "mod"; n();, if not, we need to keep the namespace import around.. do not think there is another way to handle that.

usedAsNamespaceOrDefault = true;
}
else {
const parent = cast(id.parent, isPropertyAccessExpression);
const exportName = parent.name.text;
let name = exportNameToImportName.get(exportName);
if (name === undefined) {
exportNameToImportName.set(exportName, name = generateName(exportName, usedIdentifiers));
}
Debug.assert(parent.expression === id);
changes.replaceNode(sourceFile, parent, createIdentifier(name));
}
});

const elements: ImportSpecifier[] = [];
exportNameToImportName.forEach((name, propertyName) => {
elements.push(createImportSpecifier(name === propertyName ? undefined : createIdentifier(propertyName), createIdentifier(name)));
});
const makeImportDeclaration = (defaultImportName: Identifier | undefined) =>
createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined,
createImportClause(defaultImportName, elements.length ? createNamedImports(elements) : undefined),
toConvert.parent.parent.moduleSpecifier);

if (usedAsNamespaceOrDefault && !allowSyntheticDefaultImports) {
changes.insertNodeAfter(sourceFile, toConvert.parent.parent, makeImportDeclaration(/*defaultImportName*/ undefined));
}
else {
changes.replaceNode(sourceFile, toConvert.parent.parent, makeImportDeclaration(usedAsNamespaceOrDefault ? createIdentifier(toConvert.name.text) : undefined));
}
}

function doChangeNamedToNamespace(sourceFile: SourceFile, checker: TypeChecker, changes: textChanges.ChangeTracker, toConvert: NamedImports, usedIdentifiers: ReadonlyMap<true>): void {
const { moduleSpecifier } = toConvert.parent.parent;
// We know the user is using at least ScriptTarget.ES6, and moduleSpecifierToValidIdentifier only cares if we're using ES5+, so just set ScriptTarget.ESNext
const namespaceImportName = generateName(moduleSpecifier && isStringLiteral(moduleSpecifier) ? codefix.moduleSpecifierToValidIdentifier(moduleSpecifier.text, ScriptTarget.ESNext) : "module", usedIdentifiers);

changes.replaceNode(sourceFile, toConvert, createNamespaceImport(createIdentifier(namespaceImportName)));

for (const element of toConvert.elements) {
const propertyName = (element.propertyName || element.name).text;
FindAllReferences.Core.eachSymbolReferenceInFile(element.name, checker, sourceFile, id => {
changes.replaceNode(sourceFile, id, createPropertyAccess(createIdentifier(namespaceImportName), propertyName));
});
}
}

function generateName(name: string, usedIdentifiers: ReadonlyMap<true>): string {
while (usedIdentifiers.has(name)) {
name = `_${name}`;
}
return name;
}
}
17 changes: 0 additions & 17 deletions src/services/refactors/extractSymbol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1744,23 +1744,6 @@ namespace ts.refactor.extractSymbol {
}
}

function getParentNodeInSpan(node: Node | undefined, file: SourceFile, span: TextSpan): Node | undefined {
if (!node) return undefined;

while (node.parent) {
if (isSourceFile(node.parent) || !spanContainsNode(span, node.parent, file)) {
return node;
}

node = node.parent;
}
}

function spanContainsNode(span: TextSpan, node: Node, file: SourceFile): boolean {
return textSpanContainsPosition(span, node.getStart(file)) &&
node.getEnd() <= textSpanEnd(span);
}

/**
* Computes whether or not a node represents an expression in a position where it could
* be extracted.
Expand Down
1 change: 1 addition & 0 deletions src/services/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@
"codefixes/useDefaultImport.ts",
"codefixes/fixAddModuleReferTypeMissingTypeof.ts",
"codefixes/convertToMappedObjectType.ts",
"refactors/convertImport.ts",
"refactors/extractSymbol.ts",
"refactors/generateGetAccessorAndSetAccessor.ts",
"refactors/moveToNewFile.ts",
Expand Down
40 changes: 40 additions & 0 deletions src/services/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1309,6 +1309,46 @@ namespace ts {
return forEachEntry(this.map, pred) || false;
}
}

export function getParentNodeInSpan(node: Node | undefined, file: SourceFile, span: TextSpan): Node | undefined {
if (!node) return undefined;

while (node.parent) {
if (isSourceFile(node.parent) || !spanContainsNode(span, node.parent, file)) {
return node;
}

node = node.parent;
}
}

function spanContainsNode(span: TextSpan, node: Node, file: SourceFile): boolean {
return textSpanContainsPosition(span, node.getStart(file)) &&
node.getEnd() <= textSpanEnd(span);
}

/**
* A free identifier is an identifier that can be accessed through name lookup as a local variable.
* In the expression `x.y`, `x` is a free identifier, but `y` is not.
*/
export function forEachFreeIdentifier(node: Node, cb: (id: Identifier) => void): void {
if (isIdentifier(node) && isFreeIdentifier(node)) cb(node);
node.forEachChild(child => forEachFreeIdentifier(child, cb));
}

function isFreeIdentifier(node: Identifier): boolean {
const { parent } = node;
switch (parent.kind) {
case SyntaxKind.PropertyAccessExpression:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am not sure i understand what this does.. and why not the name?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment.

return (parent as PropertyAccessExpression).name !== node;
case SyntaxKind.BindingElement:
return (parent as BindingElement).propertyName !== node;
case SyntaxKind.ImportSpecifier:
return (parent as ImportSpecifier).propertyName !== node;
default:
return true;
}
}
}

// Display-part writer helpers
Expand Down
20 changes: 20 additions & 0 deletions tests/cases/fourslash/refactorConvertImport_namedToNamespace.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/// <reference path='fourslash.ts' />

/////*a*/import { x, y as z, T } from "m";/*b*/
////const m = 0;
////x;
////z;
////const n: T = 0;

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 _m from "m";
const m = 0;
_m.x;
_m.y;
const n: _m.T = 0;`,
});
18 changes: 18 additions & 0 deletions tests/cases/fourslash/refactorConvertImport_namespaceToNamed.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path='fourslash.ts' />

/////*a*/import * as m from "m";/*b*/
////const a = 0;
////m.a;
////m.b;

goTo.select("a", "b");
edit.applyRefactor({
refactorName: "Convert import",
actionName: "Convert namespace import to named imports",
actionDescription: "Convert namespace import to named imports",
newContent:
`import { a as _a, b } from "m";
const a = 0;
_a;
b;`,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/// <reference path='fourslash.ts' />

/////*a*/import * as m from "m";/*b*/
////m.a;
////m;

goTo.select("a", "b");
edit.applyRefactor({
refactorName: "Convert import",
actionName: "Convert namespace import to named imports",
actionDescription: "Convert namespace import to named imports",
newContent:
`import * as m from "m";
import { a } from "m";
a;
m;`,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/// <reference path='fourslash.ts' />

////import /*a*/d/*b*/, * as n from "m";

goTo.select("a", "b");
verify.not.refactorAvailable("Convert import");
16 changes: 16 additions & 0 deletions tests/cases/fourslash/refactorConvertImport_useDefault.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/// <reference path='fourslash.ts' />

// @allowSyntheticDefaultImports: true

/////*a*/import * as m from "m";/*b*/
////m();

goTo.select("a", "b");
edit.applyRefactor({
refactorName: "Convert import",
actionName: "Convert namespace import to named imports",
actionDescription: "Convert namespace import to named imports",
newContent:
`import m from "m";
m();`,
});