Port needed changes from #24469

Author: Andy Hanson

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<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> {

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)

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),

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;
+    }
 }

tests/cases/fourslash/extract-method-uniqueName.ts

@@ -1,6 +1,6 @@
 /// <reference path='fourslash.ts' />
 
-////// 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() {

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";

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;
     }
 }`,

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;
     }
 }`,

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;
     }
 }`,

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;
     }
 }`,