Respond to minor PR comments

Author: Andy Hanson

src/services/findAllReferences.ts

@@ -520,17 +520,18 @@ namespace ts.FindAllReferences.Core {
         }
 
         if (indirectUsers.length) {
-            const indirectSearch = (() => {
-                switch (exportInfo.exportKind) {
-                    case ExportKind.Named:
-                        return state.createSearch(exportLocation, exportSymbol, ImportExport.Export);
-                    case ExportKind.Default:
-                        // Search for a property access to '.default'. This can't be renamed.
-                        return state.isForRename ? undefined : state.createSearch(exportLocation, exportSymbol, ImportExport.Export, { text: "default" });
-                    case ExportKind.ExportEquals:
-                        return undefined;
-                }
-            })();
+            let indirectSearch: Search | undefined;
+            switch (exportInfo.exportKind) {
+                case ExportKind.Named:
+                    indirectSearch = state.createSearch(exportLocation, exportSymbol, ImportExport.Export);
+                    break;
+                case ExportKind.Default:
+                    // Search for a property access to '.default'. This can't be renamed.
+                    indirectSearch = state.isForRename ? undefined : state.createSearch(exportLocation, exportSymbol, ImportExport.Export, { text: "default" });
+                    break;
+                case ExportKind.ExportEquals:
+                    break;
+            }
             if (indirectSearch) {
                 for (const indirectUser of indirectUsers) {
                     state.cancellationToken.throwIfCancellationRequested();
@@ -549,15 +550,11 @@ namespace ts.FindAllReferences.Core {
 
     /** Search for all occurences of an identifier in a source file (and filter out the ones that match). */
     function searchForName(sourceFile: SourceFile, search: Search, state: State): void {
-        if (sourceFileHasName(sourceFile, search.escapedText)) {
+        if (getNameTable(sourceFile).get(search.escapedText) !== undefined) {
             getReferencesInSourceFile(sourceFile, search, state);
         }
     }
 
-    function sourceFileHasName(sourceFile: SourceFile, escapedName: string): boolean {
-        return getNameTable(sourceFile).get(escapedName) !== undefined;
-    }
-
     function getPropertySymbolOfDestructuringAssignment(location: Node, checker: TypeChecker): Symbol | undefined {
         return isArrayLiteralOrObjectLiteralDestructuringPattern(location.parent.parent) &&
             checker.getPropertySymbolOfDestructuringAssignment(<Identifier>location);
@@ -613,7 +610,9 @@ namespace ts.FindAllReferences.Core {
             return undefined;
         }
 
-        // The only symbols with parents *not* globally acessible are exports of external modules, and only then if they do not have `export as namespace`.
+        // If the symbol has a parent, it's globally visible.
+        // Unless that parent is an external module, then we should only search in the module (and recurse on the export later).
+        // But if the parent is a module that has `export as namespace`, then the symbol *is* globally visible.
         if (parent && !((parent.flags & SymbolFlags.Module) && isExternalModuleSymbol(parent) && !parent.globalExports)) {
             return undefined;
         }

src/services/importTracker.ts

@@ -36,7 +36,7 @@ namespace ts.FindAllReferences {
     type ImporterOrCallExpression = Importer | CallExpression;
 
     /** Returns import statements that directly reference the exporting module, and a list of files that may access the module through a namespace. */
-    function getImportersForExport(sourceFiles: SourceFile[], allDirectImports: ImporterOrCallExpression[][], { exportingModuleSymbol, exportKind }: ExportInfo, checker: TypeChecker): { directImports: Importer[], indirectUsers: SourceFile[] } {
+    function getImportersForExport(sourceFiles: SourceFile[], allDirectImports: Map<ImporterOrCallExpression[]>, { exportingModuleSymbol, exportKind }: ExportInfo, checker: TypeChecker): { directImports: Importer[], indirectUsers: SourceFile[] } {
         const markSeenDirectImport = nodeSeenTracker<ImporterOrCallExpression>();
         const markSeenIndirectUser = nodeSeenTracker<SourceFileLike>();
         const directImports: Importer[] = [];
@@ -148,7 +148,7 @@ namespace ts.FindAllReferences {
         }
 
         function getDirectImports(moduleSymbol: Symbol): ImporterOrCallExpression[] | undefined {
-            return allDirectImports[getSymbolId(moduleSymbol)];
+            return allDirectImports.get(getSymbolId(moduleSymbol).toString());
         }
     }
 
@@ -173,7 +173,7 @@ namespace ts.FindAllReferences {
 
         function handleImport(decl: Importer): void {
             if (decl.kind === SyntaxKind.ImportEqualsDeclaration) {
-                if (isProperImportEquals(decl)) {
+                if (isExternalModuleImportEquals(decl)) {
                     handleNamespaceImportLike(decl.name);
                 }
                 return;
@@ -274,17 +274,17 @@ namespace ts.FindAllReferences {
     }
 
     /** Returns a map from a module symbol Id to all import statements that directly reference the module. */
-    function getDirectImportsMap(sourceFiles: SourceFile[], checker: TypeChecker): ImporterOrCallExpression[][] {
-        const map: ImporterOrCallExpression[][] = [];
+    function getDirectImportsMap(sourceFiles: SourceFile[], checker: TypeChecker): Map<ImporterOrCallExpression[]> {
+        const map = createMap<ImporterOrCallExpression[]>();
 
         for (const sourceFile of sourceFiles) {
             forEachImport(sourceFile, (importDecl, moduleSpecifier) => {
                 const moduleSymbol = checker.getSymbolAtLocation(moduleSpecifier);
                 if (moduleSymbol) {
-                    const id = getSymbolId(moduleSymbol);
-                    let imports = map[id];
+                    const id = getSymbolId(moduleSymbol).toString();
+                    let imports = map.get(id);
                     if (!imports) {
-                        imports = map[id] = [];
+                        map.set(id, imports = []);
                     }
                     imports.push(importDecl);
                 }
@@ -371,8 +371,7 @@ namespace ts.FindAllReferences {
      * @param comingFromExport If we are doing a search for all exports, don't bother looking backwards for the imported symbol, since that's the reason we're here.
      */
     export function getImportOrExportSymbol(node: Node, symbol: Symbol, checker: TypeChecker, comingFromExport: boolean): ImportedSymbol | ExportedSymbol | undefined {
-        const ex = getExport();
-        return ex || comingFromExport ? ex : getImport();
+        return comingFromExport ? getExport() : getExport() || getImport();
 
         function getExport(): ExportedSymbol | ImportedSymbol | undefined {
             const { parent } = node;
@@ -459,7 +458,9 @@ namespace ts.FindAllReferences {
         const { parent } = node;
         switch (parent.kind) {
             case SyntaxKind.ImportEqualsDeclaration:
-                return (parent as ImportEqualsDeclaration).name === node ? { isNamedImport: false } : undefined;
+                return (parent as ImportEqualsDeclaration).name === node && isExternalModuleImportEquals(parent as ImportEqualsDeclaration)
+                    ? { isNamedImport: false }
+                    : undefined;
             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 };
@@ -522,7 +523,7 @@ namespace ts.FindAllReferences {
         return node.kind === SyntaxKind.ModuleDeclaration && (node as ModuleDeclaration).name.kind === SyntaxKind.StringLiteral;
     }
 
-    function isProperImportEquals({ moduleReference }: ImportEqualsDeclaration): boolean {
+    function isExternalModuleImportEquals({ moduleReference }: ImportEqualsDeclaration): boolean {
         return moduleReference.kind === SyntaxKind.ExternalModuleReference && moduleReference.expression.kind === SyntaxKind.StringLiteral
     }
 }

src/services/rename.ts

@@ -31,6 +31,13 @@ namespace ts.Rename {
                     return getRenameInfoError(Diagnostics.You_cannot_rename_elements_that_are_defined_in_the_standard_TypeScript_library);
                 }
 
+                // Cannot rename `default` as in `import { default as foo } from "./someModule";
+                if (node.kind === SyntaxKind.Identifier &&
+                        (node as Identifier).originalKeywordKind === SyntaxKind.DefaultKeyword &&
+                        symbol.parent.flags & ts.SymbolFlags.Module) {
+                    return undefined;
+                }
+
                 const displayName = stripQuotes(getDeclaredName(typeChecker, symbol, node));
                 const kind = SymbolDisplay.getSymbolKind(typeChecker, symbol, node);
                 return kind ? getRenameInfoSuccess(displayName, typeChecker.getFullyQualifiedName(symbol), kind, SymbolDisplay.getSymbolModifiers(symbol), node, sourceFile) : undefined;
@@ -82,11 +89,8 @@ namespace ts.Rename {
     }
 
     function nodeIsEligibleForRename(node: Node): boolean {
-        if (node.kind === SyntaxKind.Identifier) {
-            // Cannot rename `default` as in `import { default as foo } from "./someModule";
-            return (node as Identifier).originalKeywordKind !== SyntaxKind.DefaultKeyword;
-        }
-        return node.kind === SyntaxKind.StringLiteral ||
+        return node.kind === ts.SyntaxKind.Identifier ||
+            node.kind === SyntaxKind.StringLiteral ||
             isLiteralNameOfPropertyDeclarationOrIndexAccess(node) ||
             isThis(node);
     }

tests/cases/fourslash/findAllRefsDefaultImportThroughNamespace.ts

@@ -9,12 +9,20 @@
 // @Filename: /c.ts
 ////import { a } from "./b";
 ////a.[|default|]();
+////
+////declare const x: { [|{| "isWriteAccess": true, "isDefinition": true |}default|]: number };
+////x.[|default|];
 
-verify.singleReferenceGroup("function f(): void");
+const [r0, r1, r2, r3] = test.ranges();
 
-const [r0, r1] = test.ranges();
+verify.singleReferenceGroup("function f(): void", [r0, r1]);
+verify.singleReferenceGroup("(property) default: number", [r2, r3]);
 
 verify.rangesAreRenameLocations([r0]);
 
+// Can't rename a default import.
 goTo.rangeStart(r1);
 verify.renameInfoFailed();
+
+// Can rename a default property.
+verify.rangesAreRenameLocations([r2, r3]);