Skip to content

Commit cf6c24c

Browse files
author
Andy Hanson
committed
Respond to minor PR comments
1 parent aa1c860 commit cf6c24c

4 files changed

Lines changed: 48 additions & 36 deletions

File tree

‎src/services/findAllReferences.ts‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -520,17 +520,18 @@ namespace ts.FindAllReferences.Core {
520520
}
521521

522522
if (indirectUsers.length) {
523-
const indirectSearch = (() => {
524-
switch (exportInfo.exportKind) {
525-
case ExportKind.Named:
526-
return state.createSearch(exportLocation, exportSymbol, ImportExport.Export);
527-
case ExportKind.Default:
528-
// Search for a property access to '.default'. This can't be renamed.
529-
return state.isForRename ? undefined : state.createSearch(exportLocation, exportSymbol, ImportExport.Export, { text: "default" });
530-
case ExportKind.ExportEquals:
531-
return undefined;
532-
}
533-
})();
523+
let indirectSearch: Search | undefined;
524+
switch (exportInfo.exportKind) {
525+
case ExportKind.Named:
526+
indirectSearch = state.createSearch(exportLocation, exportSymbol, ImportExport.Export);
527+
break;
528+
case ExportKind.Default:
529+
// Search for a property access to '.default'. This can't be renamed.
530+
indirectSearch = state.isForRename ? undefined : state.createSearch(exportLocation, exportSymbol, ImportExport.Export, { text: "default" });
531+
break;
532+
case ExportKind.ExportEquals:
533+
break;
534+
}
534535
if (indirectSearch) {
535536
for (const indirectUser of indirectUsers) {
536537
state.cancellationToken.throwIfCancellationRequested();
@@ -549,15 +550,11 @@ namespace ts.FindAllReferences.Core {
549550

550551
/** Search for all occurences of an identifier in a source file (and filter out the ones that match). */
551552
function searchForName(sourceFile: SourceFile, search: Search, state: State): void {
552-
if (sourceFileHasName(sourceFile, search.escapedText)) {
553+
if (getNameTable(sourceFile).get(search.escapedText) !== undefined) {
553554
getReferencesInSourceFile(sourceFile, search, state);
554555
}
555556
}
556557

557-
function sourceFileHasName(sourceFile: SourceFile, escapedName: string): boolean {
558-
return getNameTable(sourceFile).get(escapedName) !== undefined;
559-
}
560-
561558
function getPropertySymbolOfDestructuringAssignment(location: Node, checker: TypeChecker): Symbol | undefined {
562559
return isArrayLiteralOrObjectLiteralDestructuringPattern(location.parent.parent) &&
563560
checker.getPropertySymbolOfDestructuringAssignment(<Identifier>location);
@@ -613,7 +610,9 @@ namespace ts.FindAllReferences.Core {
613610
return undefined;
614611
}
615612

616-
// The only symbols with parents *not* globally acessible are exports of external modules, and only then if they do not have `export as namespace`.
613+
// If the symbol has a parent, it's globally visible.
614+
// Unless that parent is an external module, then we should only search in the module (and recurse on the export later).
615+
// But if the parent is a module that has `export as namespace`, then the symbol *is* globally visible.
617616
if (parent && !((parent.flags & SymbolFlags.Module) && isExternalModuleSymbol(parent) && !parent.globalExports)) {
618617
return undefined;
619618
}

‎src/services/importTracker.ts‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ namespace ts.FindAllReferences {
3636
type ImporterOrCallExpression = Importer | CallExpression;
3737

3838
/** Returns import statements that directly reference the exporting module, and a list of files that may access the module through a namespace. */
39-
function getImportersForExport(sourceFiles: SourceFile[], allDirectImports: ImporterOrCallExpression[][], { exportingModuleSymbol, exportKind }: ExportInfo, checker: TypeChecker): { directImports: Importer[], indirectUsers: SourceFile[] } {
39+
function getImportersForExport(sourceFiles: SourceFile[], allDirectImports: Map<ImporterOrCallExpression[]>, { exportingModuleSymbol, exportKind }: ExportInfo, checker: TypeChecker): { directImports: Importer[], indirectUsers: SourceFile[] } {
4040
const markSeenDirectImport = nodeSeenTracker<ImporterOrCallExpression>();
4141
const markSeenIndirectUser = nodeSeenTracker<SourceFileLike>();
4242
const directImports: Importer[] = [];
@@ -148,7 +148,7 @@ namespace ts.FindAllReferences {
148148
}
149149

150150
function getDirectImports(moduleSymbol: Symbol): ImporterOrCallExpression[] | undefined {
151-
return allDirectImports[getSymbolId(moduleSymbol)];
151+
return allDirectImports.get(getSymbolId(moduleSymbol).toString());
152152
}
153153
}
154154

@@ -173,7 +173,7 @@ namespace ts.FindAllReferences {
173173

174174
function handleImport(decl: Importer): void {
175175
if (decl.kind === SyntaxKind.ImportEqualsDeclaration) {
176-
if (isProperImportEquals(decl)) {
176+
if (isExternalModuleImportEquals(decl)) {
177177
handleNamespaceImportLike(decl.name);
178178
}
179179
return;
@@ -274,17 +274,17 @@ namespace ts.FindAllReferences {
274274
}
275275

276276
/** Returns a map from a module symbol Id to all import statements that directly reference the module. */
277-
function getDirectImportsMap(sourceFiles: SourceFile[], checker: TypeChecker): ImporterOrCallExpression[][] {
278-
const map: ImporterOrCallExpression[][] = [];
277+
function getDirectImportsMap(sourceFiles: SourceFile[], checker: TypeChecker): Map<ImporterOrCallExpression[]> {
278+
const map = createMap<ImporterOrCallExpression[]>();
279279

280280
for (const sourceFile of sourceFiles) {
281281
forEachImport(sourceFile, (importDecl, moduleSpecifier) => {
282282
const moduleSymbol = checker.getSymbolAtLocation(moduleSpecifier);
283283
if (moduleSymbol) {
284-
const id = getSymbolId(moduleSymbol);
285-
let imports = map[id];
284+
const id = getSymbolId(moduleSymbol).toString();
285+
let imports = map.get(id);
286286
if (!imports) {
287-
imports = map[id] = [];
287+
map.set(id, imports = []);
288288
}
289289
imports.push(importDecl);
290290
}
@@ -371,8 +371,7 @@ namespace ts.FindAllReferences {
371371
* @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.
372372
*/
373373
export function getImportOrExportSymbol(node: Node, symbol: Symbol, checker: TypeChecker, comingFromExport: boolean): ImportedSymbol | ExportedSymbol | undefined {
374-
const ex = getExport();
375-
return ex || comingFromExport ? ex : getImport();
374+
return comingFromExport ? getExport() : getExport() || getImport();
376375

377376
function getExport(): ExportedSymbol | ImportedSymbol | undefined {
378377
const { parent } = node;
@@ -459,7 +458,9 @@ namespace ts.FindAllReferences {
459458
const { parent } = node;
460459
switch (parent.kind) {
461460
case SyntaxKind.ImportEqualsDeclaration:
462-
return (parent as ImportEqualsDeclaration).name === node ? { isNamedImport: false } : undefined;
461+
return (parent as ImportEqualsDeclaration).name === node && isExternalModuleImportEquals(parent as ImportEqualsDeclaration)
462+
? { isNamedImport: false }
463+
: undefined;
463464
case SyntaxKind.ImportSpecifier:
464465
// For a rename import `{ foo as bar }`, don't search for the imported symbol. Just find local uses of `bar`.
465466
return (parent as ImportSpecifier).propertyName ? undefined : { isNamedImport: true };
@@ -522,7 +523,7 @@ namespace ts.FindAllReferences {
522523
return node.kind === SyntaxKind.ModuleDeclaration && (node as ModuleDeclaration).name.kind === SyntaxKind.StringLiteral;
523524
}
524525

525-
function isProperImportEquals({ moduleReference }: ImportEqualsDeclaration): boolean {
526+
function isExternalModuleImportEquals({ moduleReference }: ImportEqualsDeclaration): boolean {
526527
return moduleReference.kind === SyntaxKind.ExternalModuleReference && moduleReference.expression.kind === SyntaxKind.StringLiteral
527528
}
528529
}

‎src/services/rename.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@ namespace ts.Rename {
3131
return getRenameInfoError(Diagnostics.You_cannot_rename_elements_that_are_defined_in_the_standard_TypeScript_library);
3232
}
3333

34+
// Cannot rename `default` as in `import { default as foo } from "./someModule";
35+
if (node.kind === SyntaxKind.Identifier &&
36+
(node as Identifier).originalKeywordKind === SyntaxKind.DefaultKeyword &&
37+
symbol.parent.flags & ts.SymbolFlags.Module) {
38+
return undefined;
39+
}
40+
3441
const displayName = stripQuotes(getDeclaredName(typeChecker, symbol, node));
3542
const kind = SymbolDisplay.getSymbolKind(typeChecker, symbol, node);
3643
return kind ? getRenameInfoSuccess(displayName, typeChecker.getFullyQualifiedName(symbol), kind, SymbolDisplay.getSymbolModifiers(symbol), node, sourceFile) : undefined;
@@ -82,11 +89,8 @@ namespace ts.Rename {
8289
}
8390

8491
function nodeIsEligibleForRename(node: Node): boolean {
85-
if (node.kind === SyntaxKind.Identifier) {
86-
// Cannot rename `default` as in `import { default as foo } from "./someModule";
87-
return (node as Identifier).originalKeywordKind !== SyntaxKind.DefaultKeyword;
88-
}
89-
return node.kind === SyntaxKind.StringLiteral ||
92+
return node.kind === ts.SyntaxKind.Identifier ||
93+
node.kind === SyntaxKind.StringLiteral ||
9094
isLiteralNameOfPropertyDeclarationOrIndexAccess(node) ||
9195
isThis(node);
9296
}

‎tests/cases/fourslash/findAllRefsDefaultImportThroughNamespace.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,20 @@
99
// @Filename: /c.ts
1010
////import { a } from "./b";
1111
////a.[|default|]();
12+
////
13+
////declare const x: { [|{| "isWriteAccess": true, "isDefinition": true |}default|]: number };
14+
////x.[|default|];
1215

13-
verify.singleReferenceGroup("function f(): void");
16+
const [r0, r1, r2, r3] = test.ranges();
1417

15-
const [r0, r1] = test.ranges();
18+
verify.singleReferenceGroup("function f(): void", [r0, r1]);
19+
verify.singleReferenceGroup("(property) default: number", [r2, r3]);
1620

1721
verify.rangesAreRenameLocations([r0]);
1822

23+
// Can't rename a default import.
1924
goTo.rangeStart(r1);
2025
verify.renameInfoFailed();
26+
27+
// Can rename a default property.
28+
verify.rangesAreRenameLocations([r2, r3]);

0 commit comments

Comments
 (0)