diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index 5f4d653ca3dba..11084f11de24e 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -710,16 +710,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(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 { diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index 32a39c8f9ad86..1e2783b159a00 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -714,7 +714,7 @@ namespace ts.refactor.extractSymbol { // Make a unique name for the extracted function const file = scope.getSourceFile(); - const functionNameText = getUniqueName(isClassLike(scope) ? "newMethod" : "newFunction", file.text); + const functionNameText = getUniqueName(isClassLike(scope) ? "newMethod" : "newFunction", file); const isJS = isInJavaScriptFile(scope); const functionName = createIdentifier(functionNameText); @@ -1001,7 +1001,7 @@ namespace ts.refactor.extractSymbol { // Make a unique name for the extracted variable const file = scope.getSourceFile(); - const localNameText = getUniqueName(isClassLike(scope) ? "newProperty" : "newLocal", file.text); + const localNameText = getUniqueName(isClassLike(scope) ? "newProperty" : "newLocal", file); const isJS = isInJavaScriptFile(scope); const variableType = isJS || !checker.isContextSensitive(node) diff --git a/src/services/refactors/generateGetAccessorAndSetAccessor.ts b/src/services/refactors/generateGetAccessorAndSetAccessor.ts index 92d87b1fcd0e4..b4be490562420 100644 --- a/src/services/refactors/generateGetAccessorAndSetAccessor.ts +++ b/src/services/refactors/generateGetAccessorAndSetAccessor.ts @@ -129,8 +129,8 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { const name = declaration.name.text; const startWithUnderscore = startsWithUnderscore(name); - const fieldName = createPropertyName(startWithUnderscore ? name : getUniqueName(`_${name}`, file.text), declaration.name); - const accessorName = createPropertyName(startWithUnderscore ? getUniqueName(name.substring(1), file.text) : name, declaration.name); + const fieldName = createPropertyName(startWithUnderscore ? name : getUniqueName(`_${name}`, file), declaration.name); + const accessorName = createPropertyName(startWithUnderscore ? getUniqueName(name.substring(1), file) : name, declaration.name); return { isStatic: hasStaticModifier(declaration), isReadonly: hasReadonlyModifier(declaration), diff --git a/src/services/refactors/moveToNewFile.ts b/src/services/refactors/moveToNewFile.ts index 56b7009287619..ed51769c22cc1 100644 --- a/src/services/refactors/moveToNewFile.ts +++ b/src/services/refactors/moveToNewFile.ts @@ -153,6 +153,8 @@ namespace ts.refactor { if (sourceFile === oldFile) continue; for (const statement of sourceFile.statements) { forEachImportInStatement(statement, importNode => { + if (checker.getSymbolAtLocation(moduleSpecifierFromImport(importNode)) !== oldFile.symbol) return; + const shouldMove = (name: Identifier): boolean => { const symbol = isBindingElement(name.parent) ? getPropertySymbolFromBindingElement(checker, name.parent as BindingElement & { name: Identifier }) @@ -163,11 +165,76 @@ namespace ts.refactor { const newModuleSpecifier = combinePaths(getDirectoryPath(moduleSpecifierFromImport(importNode).text), newModuleName); const newImportDeclaration = filterImport(importNode, createLiteral(newModuleSpecifier), shouldMove); if (newImportDeclaration) changes.insertNodeAfter(sourceFile, statement, newImportDeclaration); + + const ns = getNamespaceLikeImport(importNode); + if (ns) updateNamespaceLikeImport(changes, sourceFile, checker, movedSymbols, newModuleName, newModuleSpecifier, ns, importNode); }); } } } + function getNamespaceLikeImport(node: SupportedImport): Identifier | undefined { + switch (node.kind) { + case SyntaxKind.ImportDeclaration: + return node.importClause && node.importClause.namedBindings && node.importClause.namedBindings.kind === SyntaxKind.NamespaceImport ? + node.importClause.namedBindings.name : undefined; + case SyntaxKind.ImportEqualsDeclaration: + return node.name; + case SyntaxKind.VariableDeclaration: + return tryCast(node.name, isIdentifier); + default: + return Debug.assertNever(node); + } + } + + function updateNamespaceLikeImport( + changes: textChanges.ChangeTracker, + sourceFile: SourceFile, + checker: TypeChecker, + movedSymbols: ReadonlySymbolSet, + newModuleName: string, + newModuleSpecifier: string, + oldImportId: Identifier, + oldImportNode: SupportedImport, + ): void { + const preferredNewNamespaceName = codefix.moduleSpecifierToValidIdentifier(newModuleName, ScriptTarget.ESNext); + let needUniqueName = false; + const toChange: Identifier[] = []; + FindAllReferences.Core.eachSymbolReferenceInFile(oldImportId, checker, sourceFile, ref => { + if (!isPropertyAccessExpression(ref.parent)) return; + needUniqueName = needUniqueName || !!checker.resolveName(preferredNewNamespaceName, ref, SymbolFlags.All, /*excludeGlobals*/ true); + if (movedSymbols.has(checker.getSymbolAtLocation(ref.parent.name)!)) { + toChange.push(ref); + } + }); + + if (toChange.length) { + const newNamespaceName = needUniqueName ? getUniqueName(preferredNewNamespaceName, sourceFile) : preferredNewNamespaceName; + for (const ref of toChange) { + changes.replaceNode(sourceFile, ref, createIdentifier(newNamespaceName)); + } + changes.insertNodeAfter(sourceFile, oldImportNode, updateNamespaceLikeImportNode(oldImportNode, newModuleName, newModuleSpecifier)); + } + } + + function updateNamespaceLikeImportNode(node: SupportedImport, newNamespaceName: string, newModuleSpecifier: string): Node { + const newNamespaceId = createIdentifier(newNamespaceName); + const newModuleString = createLiteral(newModuleSpecifier); + switch (node.kind) { + case SyntaxKind.ImportDeclaration: + return createImportDeclaration( + /*decorators*/ undefined, /*modifiers*/ undefined, + createImportClause(/*name*/ undefined, createNamespaceImport(newNamespaceId)), + newModuleString); + case SyntaxKind.ImportEqualsDeclaration: + return createImportEqualsDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, newNamespaceId, createExternalModuleReference(newModuleString)); + case SyntaxKind.VariableDeclaration: + return createVariableDeclaration(newNamespaceId, /*type*/ undefined, createRequireCall(newModuleString)); + default: + return Debug.assertNever(node); + } + } + function moduleSpecifierFromImport(i: SupportedImport): StringLiteralLike { return (i.kind === SyntaxKind.ImportDeclaration ? i.moduleSpecifier : i.kind === SyntaxKind.ImportEqualsDeclaration ? i.moduleReference.expression diff --git a/src/services/utilities.ts b/src/services/utilities.ts index a0b98c1d8164f..24e20c076870f 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1596,9 +1596,9 @@ namespace ts { } /* @internal */ - export function getUniqueName(baseName: string, fileText: string): string { + export function getUniqueName(baseName: string, sourceFile: SourceFile): string { let nameText = baseName; - for (let i = 1; stringContains(fileText, nameText); i++) { + for (let i = 1; !isFileLevelUniqueName(sourceFile, nameText); i++) { nameText = `${baseName}_${i}`; } return nameText; @@ -1617,7 +1617,7 @@ namespace ts { Debug.assert(fileName === renameFilename); for (const change of textChanges) { const { span, newText } = change; - const index = newText.indexOf(name); + const index = indexInTextChange(newText, name); if (index !== -1) { lastPos = span.start + delta + index; @@ -1635,4 +1635,13 @@ namespace ts { Debug.assert(lastPos >= 0); return lastPos; } + + function indexInTextChange(change: string, name: string): number { + if (startsWith(change, name)) return 0; + // Add a " " to avoid references inside words + let idx = change.indexOf(" " + name); + if (idx === -1) idx = change.indexOf("." + name); + if (idx === -1) idx = change.indexOf('"' + name); + return idx === -1 ? -1 : idx + 1; + } } diff --git a/tests/cases/fourslash/extract-method-uniqueName.ts b/tests/cases/fourslash/extract-method-uniqueName.ts index ae62f4ba7e7ac..218cad66f6867 100644 --- a/tests/cases/fourslash/extract-method-uniqueName.ts +++ b/tests/cases/fourslash/extract-method-uniqueName.ts @@ -1,6 +1,6 @@ /// -////// newFunction +////const newFunction = 0; /////*start*/1 + 1/*end*/; goTo.select('start', 'end') @@ -9,7 +9,7 @@ edit.applyRefactor({ actionName: "function_scope_0", actionDescription: "Extract to function in global scope", newContent: -`// newFunction +`const newFunction = 0; /*RENAME*/newFunction_1(); function newFunction_1() { diff --git a/tests/cases/fourslash/moveToNewFile_namespaceImport.ts b/tests/cases/fourslash/moveToNewFile_namespaceImport.ts new file mode 100644 index 0000000000000..152d1fcaeb518 --- /dev/null +++ b/tests/cases/fourslash/moveToNewFile_namespaceImport.ts @@ -0,0 +1,49 @@ +/// + +// @allowJs: true + +// @Filename: /a.ts +////[|export const x = 0;|] +////export const y = 0; + +// @Filename: /b.ts +////import * as a from "./a"; +////a.x; +////a.y; + +// @Filename: /c.ts +////import a = require("./a"); +////a.x; +////a.y; + +// @Filename: /d.js +////const a = require("./a"); +////a.x; +////a.y; + +verify.moveToNewFile({ + newFileContents: { + "/a.ts": +`export const y = 0;`, + + "/x.ts": +`export const x = 0;`, + + "/b.ts": +`import * as a from "./a"; +import * as x from "./x"; +x.x; +a.y;`, + + "/c.ts": +`import a = require("./a"); +import x = require("./x"); +x.x; +a.y;`, + + "/d.js": +`const a = require("./a"), x = require("./x"); +x.x; +a.y;`, + }, +}); diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess23.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess23.ts index 32f4bfe009dd1..6c262044a99b3 100644 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess23.ts +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess23.ts @@ -29,10 +29,10 @@ edit.applyRefactor({ actionDescription: "Generate 'get' and 'set' accessors", newContent: `class A { private _a: number = 1; - public get /*RENAME*/a_2(): number { + public get /*RENAME*/a_1(): number { return this._a; } - public set a_2(value: number) { + public set a_1(value: number) { this._a = value; } private _a_1: string = "foo"; diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess4.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess4.ts index c2d9a7eea7e02..ed01e35964feb 100644 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess4.ts +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess4.ts @@ -11,10 +11,10 @@ edit.applyRefactor({ actionDescription: "Generate 'get' and 'set' accessors", newContent: `class A { private _a: string; - public get /*RENAME*/a_1(): string { + public get /*RENAME*/a(): string { return this._a; } - public set a_1(value: string) { + public set a(value: string) { this._a = value; } }`, diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess5.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess5.ts index 9d2526d8a7d79..f68726495fecf 100644 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess5.ts +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess5.ts @@ -11,10 +11,10 @@ edit.applyRefactor({ actionDescription: "Generate 'get' and 'set' accessors", newContent: `class A { private _a: string; - public get /*RENAME*/a_1(): string { + public get /*RENAME*/a(): string { return this._a; } - public set a_1(value: string) { + public set a(value: string) { this._a = value; } }`, diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess6.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess6.ts index b3899c0647a5e..d9bbd08e4d01a 100644 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess6.ts +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess6.ts @@ -11,10 +11,10 @@ edit.applyRefactor({ actionDescription: "Generate 'get' and 'set' accessors", newContent: `class A { private _a: string; - public get /*RENAME*/a_1(): string { + public get /*RENAME*/a(): string { return this._a; } - public set a_1(value: string) { + public set a(value: string) { this._a = value; } }`, diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess7.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess7.ts index 114cfd39d8757..ebac8142dc1e2 100644 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess7.ts +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess7.ts @@ -11,10 +11,10 @@ edit.applyRefactor({ actionDescription: "Generate 'get' and 'set' accessors", newContent: `class A { private _a: string; - protected get /*RENAME*/a_1(): string { + protected get /*RENAME*/a(): string { return this._a; } - protected set a_1(value: string) { + protected set a(value: string) { this._a = value; } }`,