When renaming an imported symbol, rename only in current file (#28406)

* When renaming an imported symbol, rename only in current file

* Improve re-export references

* Remember to use 'range' in for loop

* Uncomment tests

Author: Andy

src/compiler/checker.ts

@@ -242,17 +242,7 @@ namespace ts {
                 const parsed = getParseTreeNode(node, isFunctionLike);
                 return parsed ? isImplementationOfOverload(parsed) : undefined;
             },
-            getImmediateAliasedSymbol: symbol => {
-                Debug.assert((symbol.flags & SymbolFlags.Alias) !== 0, "Should only get Alias here.");
-                const links = getSymbolLinks(symbol);
-                if (!links.immediateTarget) {
-                    const node = getDeclarationOfAliasSymbol(symbol);
-                    if (!node) return Debug.fail();
-                    links.immediateTarget = getTargetOfAliasDeclaration(node, /*dontRecursivelyResolve*/ true);
-                }
-
-                return links.immediateTarget;
-            },
+            getImmediateAliasedSymbol,
             getAliasedSymbol: resolveAlias,
             getEmitResolver,
             getExportsOfModule: getExportsOfModuleAsArray,
@@ -17828,6 +17818,18 @@ namespace ts {
             return createIndexInfo(unionType, /*isReadonly*/ false);
         }
 
+        function getImmediateAliasedSymbol(symbol: Symbol): Symbol | undefined {
+            Debug.assert((symbol.flags & SymbolFlags.Alias) !== 0, "Should only get Alias here.");
+            const links = getSymbolLinks(symbol);
+            if (!links.immediateTarget) {
+                const node = getDeclarationOfAliasSymbol(symbol);
+                if (!node) return Debug.fail();
+                links.immediateTarget = getTargetOfAliasDeclaration(node, /*dontRecursivelyResolve*/ true);
+            }
+
+            return links.immediateTarget;
+        }
+
         function checkObjectLiteral(node: ObjectLiteralExpression, checkMode?: CheckMode): Type {
             const inDestructuringPattern = isAssignmentTarget(node);
             // Grammar checking
@@ -28218,7 +28220,10 @@ namespace ts {
 
             if (isDeclarationNameOrImportPropertyName(node)) {
                 // This is a declaration, call getSymbolOfNode
-                return getSymbolOfNode(parent);
+                const parentSymbol = getSymbolOfNode(parent)!;
+                return isImportOrExportSpecifier(node.parent) && node.parent.propertyName === node
+                    ? getImmediateAliasedSymbol(parentSymbol)
+                    : parentSymbol;
             }
             else if (isLiteralComputedPropertyDeclarationName(node)) {
                 return getSymbolOfNode(parent.parent);

src/compiler/utilities.ts

@@ -6111,6 +6111,10 @@ namespace ts {
             || kind === SyntaxKind.TemplateTail;
     }
 
+    export function isImportOrExportSpecifier(node: Node): node is ImportSpecifier | ExportSpecifier {
+        return isImportSpecifier(node) || isExportSpecifier(node);
+    }
+
     export function isStringTextContainingNode(node: Node): node is StringLiteral | TemplateLiteralToken {
         return node.kind === SyntaxKind.StringLiteral || isTemplateLiteralKind(node.kind);
     }

src/services/findAllReferences.ts

@@ -89,10 +89,10 @@ namespace ts.FindAllReferences {
         program: Program, cancellationToken: CancellationToken, sourceFiles: ReadonlyArray<SourceFile>, node: Node, position: number, options: Options | undefined,
         convertEntry: ToReferenceOrRenameEntry<T>,
     ): T[] | undefined {
-        return map(flattenEntries(Core.getReferencedSymbolsForNode(position, node, program, sourceFiles, cancellationToken, options)), entry => convertEntry(entry, node));
+        return map(flattenEntries(Core.getReferencedSymbolsForNode(position, node, program, sourceFiles, cancellationToken, options)), entry => convertEntry(entry, node, program.getTypeChecker()));
     }
 
-    export type ToReferenceOrRenameEntry<T> = (entry: Entry, originalNode: Node) => T;
+    export type ToReferenceOrRenameEntry<T> = (entry: Entry, originalNode: Node, checker: TypeChecker) => T;
 
     export function getReferenceEntriesForNode(
         position: number,
@@ -157,8 +157,8 @@ namespace ts.FindAllReferences {
         return { displayParts, kind: symbolKind };
     }
 
-    export function toRenameLocation(entry: Entry, originalNode: Node): RenameLocation {
-        return { ...entryToDocumentSpan(entry), ...getPrefixAndSuffixText(entry, originalNode) };
+    export function toRenameLocation(entry: Entry, originalNode: Node, checker: TypeChecker): RenameLocation {
+        return { ...entryToDocumentSpan(entry), ...getPrefixAndSuffixText(entry, originalNode, checker) };
     }
 
     export function toReferenceEntry(entry: Entry): ReferenceEntry {
@@ -188,25 +188,28 @@ namespace ts.FindAllReferences {
         }
     }
 
-    function getPrefixAndSuffixText(entry: Entry, originalNode: Node): { readonly prefixText?: string, readonly suffixText?: string } {
+    interface PrefixAndSuffix { readonly prefixText?: string; readonly suffixText?: string; }
+    function getPrefixAndSuffixText(entry: Entry, originalNode: Node, checker: TypeChecker): PrefixAndSuffix {
         if (entry.kind !== EntryKind.Span && isIdentifier(originalNode)) {
             const { node, kind } = entry;
             const name = originalNode.text;
             const isShorthandAssignment = isShorthandPropertyAssignment(node.parent);
             if (isShorthandAssignment || isObjectBindingElementWithoutPropertyName(node.parent)) {
-                if (kind === EntryKind.SearchedLocalFoundProperty) {
-                    return { prefixText: name + ": " };
-                }
-                else if (kind === EntryKind.SearchedPropertyFoundLocal) {
-                    return { suffixText: ": " + name };
-                }
-                else {
-                    return isShorthandAssignment
-                        // In `const o = { x }; o.x`, symbolAtLocation at `x` in `{ x }` is the property symbol.
-                        ? { suffixText: ": " + name }
-                        // For a binding element `const { x } = o;`, symbolAtLocation at `x` is the property symbol.
-                        : { prefixText: name + ": " };
-                }
+                const prefixColon: PrefixAndSuffix = { prefixText: name + ": " };
+                const suffixColon: PrefixAndSuffix = { suffixText: ": " + name };
+                return kind === EntryKind.SearchedLocalFoundProperty ? prefixColon
+                    : kind === EntryKind.SearchedPropertyFoundLocal ? suffixColon
+                    // In `const o = { x }; o.x`, symbolAtLocation at `x` in `{ x }` is the property symbol.
+                    // For a binding element `const { x } = o;`, symbolAtLocation at `x` is the property symbol.
+                    : isShorthandAssignment ? suffixColon : prefixColon;
+            }
+            else if (isImportSpecifier(entry.node.parent) && !entry.node.parent.propertyName) {
+                // If the original symbol was using this alias, just rename the alias.
+                const originalSymbol = isExportSpecifier(originalNode.parent) ? checker.getExportSpecifierLocalTargetSymbol(originalNode.parent) : checker.getSymbolAtLocation(originalNode);
+                return contains(originalSymbol!.declarations, entry.node.parent) ? { prefixText: name + " as " } : emptyOptions;
+            }
+            else if (isExportSpecifier(entry.node.parent) && !entry.node.parent.propertyName) {
+                return originalNode === entry.node ? { prefixText: name + " as " } : { suffixText: " as " + name };
             }
         }
 
@@ -480,16 +483,21 @@ namespace ts.FindAllReferences.Core {
     }
 
     /** Core find-all-references algorithm for a normal symbol. */
-    function getReferencedSymbolsForSymbol(symbol: Symbol, node: Node | undefined, sourceFiles: ReadonlyArray<SourceFile>, sourceFilesSet: ReadonlyMap<true>, checker: TypeChecker, cancellationToken: CancellationToken, options: Options): SymbolAndEntries[] {
-        symbol = node && skipPastExportOrImportSpecifierOrUnion(symbol, node, checker) || symbol;
+    function getReferencedSymbolsForSymbol(originalSymbol: Symbol, node: Node | undefined, sourceFiles: ReadonlyArray<SourceFile>, sourceFilesSet: ReadonlyMap<true>, checker: TypeChecker, cancellationToken: CancellationToken, options: Options): SymbolAndEntries[] {
+        const symbol = node && skipPastExportOrImportSpecifierOrUnion(originalSymbol, node, checker, !!options.isForRename) || originalSymbol;
 
         // Compute the meaning from the location and the symbol it references
         const searchMeaning = node ? getIntersectingMeaningFromDeclarations(node, symbol) : SemanticMeaning.All;
 
         const result: SymbolAndEntries[] = [];
         const state = new State(sourceFiles, sourceFilesSet, node ? getSpecialSearchKind(node) : SpecialSearchKind.None, checker, cancellationToken, searchMeaning, options, result);
 
-        if (node && node.kind === SyntaxKind.DefaultKeyword) {
+        const exportSpecifier = !options.isForRename ? undefined : find(symbol.declarations, isExportSpecifier);
+        if (exportSpecifier) {
+            // When renaming at an export specifier, rename the export and not the thing being exported.
+            getReferencesAtExportSpecifier(exportSpecifier.name, symbol, exportSpecifier, state.createSearch(node, originalSymbol, /*comingFrom*/ undefined), state, /*addReferencesHere*/ true, /*alwaysGetReferences*/ true);
+        }
+        else if (node && node.kind === SyntaxKind.DefaultKeyword) {
             addReference(node, symbol, state);
             searchForImportsOfExport(node, symbol, { exportingModuleSymbol: Debug.assertDefined(symbol.parent, "Expected export symbol to have a parent"), exportKind: ExportKind.Default }, state);
         }
@@ -530,16 +538,11 @@ namespace ts.FindAllReferences.Core {
     }
 
     /** Handle a few special cases relating to export/import specifiers. */
-    function skipPastExportOrImportSpecifierOrUnion(symbol: Symbol, node: Node, checker: TypeChecker): Symbol | undefined {
+    function skipPastExportOrImportSpecifierOrUnion(symbol: Symbol, node: Node, checker: TypeChecker, isForRename: boolean): Symbol | undefined {
         const { parent } = node;
-        if (isExportSpecifier(parent)) {
+        if (isExportSpecifier(parent) && !isForRename) {
             return getLocalSymbolForExportSpecifier(node as Identifier, symbol, parent, checker);
         }
-        if (isImportSpecifier(parent) && parent.propertyName === node) {
-            // We're at `foo` in `import { foo as bar }`. Probably intended to find all refs on the original, not just on the import.
-            return checker.getImmediateAliasedSymbol(symbol)!;
-        }
-
         // If the symbol is declared as part of a declaration like `{ type: "a" } | { type: "b" }`, use the property on the union type to get more references.
         return firstDefined(symbol.declarations, decl => {
             if (!decl.parent) {
@@ -741,7 +744,8 @@ namespace ts.FindAllReferences.Core {
         }
         for (const indirectUser of indirectUsers) {
             for (const node of getPossibleSymbolReferenceNodes(indirectUser, isDefaultExport ? "default" : exportName)) {
-                if (isIdentifier(node) && checker.getSymbolAtLocation(node) === exportSymbol) {
+                // Import specifiers should be handled by importSearches
+                if (isIdentifier(node) && !isImportOrExportSpecifier(node.parent) && checker.getSymbolAtLocation(node) === exportSymbol) {
                     cb(node);
                 }
             }
@@ -754,7 +758,7 @@ namespace ts.FindAllReferences.Core {
         // Don't rename an import type `import("./module-name")` when renaming `name` in `export = name;`
         if (!isIdentifier(singleRef)) return false;
         // At `default` in `import { default as x }` or `export { default as x }`, do add a reference, but do not rename.
-        return !((isExportSpecifier(singleRef.parent) || isImportSpecifier(singleRef.parent)) && singleRef.escapedText === InternalSymbolName.Default);
+        return !(isImportOrExportSpecifier(singleRef.parent) && singleRef.escapedText === InternalSymbolName.Default);
     }
 
     // Go to the symbol we imported from and find references for it.
@@ -1058,17 +1062,25 @@ namespace ts.FindAllReferences.Core {
         getImportOrExportReferences(referenceLocation, referenceSymbol, search, state);
     }
 
-    function getReferencesAtExportSpecifier(referenceLocation: Identifier, referenceSymbol: Symbol, exportSpecifier: ExportSpecifier, search: Search, state: State, addReferencesHere: boolean): void {
+    function getReferencesAtExportSpecifier(
+        referenceLocation: Identifier,
+        referenceSymbol: Symbol,
+        exportSpecifier: ExportSpecifier,
+        search: Search,
+        state: State,
+        addReferencesHere: boolean,
+        alwaysGetReferences?: boolean,
+    ): void {
         const { parent, propertyName, name } = exportSpecifier;
         const exportDeclaration = parent.parent;
         const localSymbol = getLocalSymbolForExportSpecifier(referenceLocation, referenceSymbol, exportSpecifier, state.checker);
-        if (!search.includes(localSymbol)) {
+        if (!alwaysGetReferences && !search.includes(localSymbol)) {
             return;
         }
 
         if (!propertyName) {
             // Don't rename at `export { default } from "m";`. (but do continue to search for imports of the re-export)
-            if (!(state.options.isForRename && name.escapedText === InternalSymbolName.Default)) {
+            if (!(state.options.isForRename && (name.escapedText === InternalSymbolName.Default))) {
                 addRef();
             }
         }
@@ -1080,7 +1092,7 @@ namespace ts.FindAllReferences.Core {
             }
 
             if (addReferencesHere && !state.options.isForRename && state.markSeenReExportRHS(name)) {
-                addReference(name, referenceSymbol, state);
+                addReference(name, Debug.assertDefined(exportSpecifier.symbol), state);
             }
         }
         else {
@@ -1090,15 +1102,15 @@ namespace ts.FindAllReferences.Core {
         }
 
         // For `export { foo as bar }`, rename `foo`, but not `bar`.
-        if (!(referenceLocation === propertyName && state.options.isForRename)) {
+        if (!state.options.isForRename || alwaysGetReferences) {
             const exportKind = referenceLocation.originalKeywordKind === SyntaxKind.DefaultKeyword ? ExportKind.Default : ExportKind.Named;
-            const exportInfo = getExportInfo(referenceSymbol, exportKind, state.checker);
-            if (!exportInfo) return Debug.fail();
-            searchForImportsOfExport(referenceLocation, referenceSymbol, exportInfo, state);
+            const exportSymbol = Debug.assertDefined(exportSpecifier.symbol);
+            const exportInfo = Debug.assertDefined(getExportInfo(exportSymbol, exportKind, state.checker));
+            searchForImportsOfExport(referenceLocation, exportSymbol, exportInfo, state);
         }
 
         // At `export { x } from "foo"`, also search for the imported symbol `"foo".x`.
-        if (search.comingFrom !== ImportExport.Export && exportDeclaration.moduleSpecifier && !propertyName) {
+        if (search.comingFrom !== ImportExport.Export && exportDeclaration.moduleSpecifier && !propertyName && !state.options.isForRename) {
             const imported = state.checker.getExportSpecifierLocalTargetSymbol(exportSpecifier);
             if (imported) searchForImportedSymbol(imported, state);
         }
@@ -1133,12 +1145,11 @@ namespace ts.FindAllReferences.Core {
         const { symbol } = importOrExport;
 
         if (importOrExport.kind === ImportExport.Import) {
-            if (!state.options.isForRename || importOrExport.isNamedImport) {
+            if (!state.options.isForRename) {
                 searchForImportedSymbol(symbol, state);
             }
         }
         else {
-            // We don't check for `state.isForRename`, even for default exports, because importers that previously matched the export name should be updated to continue matching.
             searchForImportsOfExport(referenceLocation, symbol, importOrExport.exportInfo, state);
         }
     }

src/services/importTracker.ts

@@ -424,7 +424,6 @@ namespace ts.FindAllReferences {
     export interface ImportedSymbol {
         kind: ImportExport.Import;
         symbol: Symbol;
-        isNamedImport: boolean;
     }
     export interface ExportedSymbol {
         kind: ImportExport.Export;
@@ -467,7 +466,7 @@ namespace ts.FindAllReferences {
                         }
 
                         const lhsSymbol = checker.getSymbolAtLocation(exportNode.name)!;
-                        return { kind: ImportExport.Import, symbol: lhsSymbol, isNamedImport: false };
+                        return { kind: ImportExport.Import, symbol: lhsSymbol };
                     }
                     else {
                         return exportInfo(symbol, getExportKindForDeclaration(exportNode));
@@ -542,7 +541,7 @@ namespace ts.FindAllReferences {
             // (All imports returned from this function will be ignored anyway if we are in rename and this is a not a named export.)
             const importedName = symbolEscapedNameNoDefault(importedSymbol);
             if (importedName === undefined || importedName === InternalSymbolName.Default || importedName === symbol.escapedName) {
-                return { kind: ImportExport.Import, symbol: importedSymbol, ...isImport };
+                return { kind: ImportExport.Import, symbol: importedSymbol };
             }
         }
 
@@ -588,22 +587,20 @@ namespace ts.FindAllReferences {
         }
     }
 
-    function isNodeImport(node: Node): { isNamedImport: boolean } | undefined {
+    function isNodeImport(node: Node): boolean {
         const { parent } = node;
         switch (parent.kind) {
             case SyntaxKind.ImportEqualsDeclaration:
-                return (parent as ImportEqualsDeclaration).name === node && isExternalModuleImportEquals(parent as ImportEqualsDeclaration)
-                    ? { isNamedImport: false }
-                    : undefined;
+                return (parent as ImportEqualsDeclaration).name === node && isExternalModuleImportEquals(parent as ImportEqualsDeclaration);
             case SyntaxKind.ImportSpecifier:
                 // For a rename import `{ foo as bar }`, don't search for the imported symbol. Just find local uses of `bar`.
-                return (parent as ImportSpecifier).propertyName ? undefined : { isNamedImport: true };
+                return !(parent as ImportSpecifier).propertyName;
             case SyntaxKind.ImportClause:
             case SyntaxKind.NamespaceImport:
                 Debug.assert((parent as ImportClause | NamespaceImport).name === node);
-                return { isNamedImport: false };
+                return true;
             default:
-                return undefined;
+                return false;
         }
     }
 

src/services/utilities.ts

@@ -1628,9 +1628,7 @@ namespace ts {
     }
 
     export function isImportOrExportSpecifierName(location: Node): location is Identifier {
-        return !!location.parent &&
-            (location.parent.kind === SyntaxKind.ImportSpecifier || location.parent.kind === SyntaxKind.ExportSpecifier) &&
-            (<ImportOrExportSpecifier>location.parent).propertyName === location;
+        return !!location.parent && isImportOrExportSpecifier(location.parent) && location.parent.propertyName === location;
     }
 
     /**