Skip to content

Commit fd17f77

Browse files
author
Andy
authored
Improvements to find-all-references for import types (#23998)
* Improvements to find-all-references for import types * Add JS test
1 parent 3f39916 commit fd17f77

16 files changed

+203
-79
lines changed

src/compiler/types.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3252,12 +3252,17 @@ namespace ts {
32523252
/* @internal */
32533253
export type AnyImportOrReExport = AnyImportSyntax | ExportDeclaration;
32543254

3255+
/* @internal */
3256+
export interface ValidImportTypeNode extends ImportTypeNode {
3257+
argument: LiteralTypeNode & { literal: StringLiteral };
3258+
}
3259+
32553260
/* @internal */
32563261
export type AnyValidImportOrReExport =
32573262
| (ImportDeclaration | ExportDeclaration) & { moduleSpecifier: StringLiteral }
32583263
| ImportEqualsDeclaration & { moduleReference: ExternalModuleReference & { expression: StringLiteral } }
32593264
| RequireOrImportCall
3260-
| ImportTypeNode & { argument: LiteralType };
3265+
| ValidImportTypeNode;
32613266

32623267
/* @internal */
32633268
export type RequireOrImportCall = CallExpression & { arguments: [StringLiteralLike] };

src/compiler/utilities.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1728,6 +1728,10 @@ namespace ts {
17281728
}
17291729

17301730
export function importFromModuleSpecifier(node: StringLiteralLike): AnyValidImportOrReExport {
1731+
return tryGetImportFromModuleSpecifier(node) || Debug.fail(Debug.showSyntaxKind(node.parent));
1732+
}
1733+
1734+
export function tryGetImportFromModuleSpecifier(node: StringLiteralLike): AnyValidImportOrReExport | undefined {
17311735
switch (node.parent.kind) {
17321736
case SyntaxKind.ImportDeclaration:
17331737
case SyntaxKind.ExportDeclaration:
@@ -1737,9 +1741,10 @@ namespace ts {
17371741
case SyntaxKind.CallExpression:
17381742
return node.parent as AnyValidImportOrReExport;
17391743
case SyntaxKind.LiteralType:
1740-
return cast(node.parent.parent, isImportTypeNode) as ImportTypeNode & { argument: LiteralType };
1744+
Debug.assert(isStringLiteral(node));
1745+
return tryCast(node.parent.parent, isImportTypeNode) as ValidImportTypeNode | undefined;
17411746
default:
1742-
return Debug.fail(Debug.showSyntaxKind(node.parent));
1747+
return undefined;
17431748
}
17441749
}
17451750

src/services/findAllReferences.ts

Lines changed: 45 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ namespace ts.FindAllReferences.Core {
236236
export function getReferencedSymbolsForNode(position: number, node: Node, program: Program, sourceFiles: ReadonlyArray<SourceFile>, cancellationToken: CancellationToken, options: Options = {}, sourceFilesSet: ReadonlyMap<true> = arrayToSet(sourceFiles, f => f.fileName)): SymbolAndEntries[] | undefined {
237237
if (isSourceFile(node)) {
238238
const reference = GoToDefinition.getReferenceAtPosition(node, position, program);
239-
return reference && getReferencedSymbolsForModule(program, program.getTypeChecker().getMergedSymbol(reference.file.symbol), sourceFiles, sourceFilesSet);
239+
return reference && getReferencedSymbolsForModule(program, program.getTypeChecker().getMergedSymbol(reference.file.symbol), /*excludeImportTypeOfExportEquals*/ false, sourceFiles, sourceFilesSet);
240240
}
241241

242242
if (!options.implementations) {
@@ -247,45 +247,45 @@ namespace ts.FindAllReferences.Core {
247247
}
248248

249249
const checker = program.getTypeChecker();
250-
const symbol = checker.getSymbolAtLocation(node);
250+
let symbol = checker.getSymbolAtLocation(node);
251251

252252
// Could not find a symbol e.g. unknown identifier
253253
if (!symbol) {
254254
// String literal might be a property (and thus have a symbol), so do this here rather than in getReferencedSymbolsSpecial.
255255
return !options.implementations && isStringLiteral(node) ? getReferencesForStringLiteral(node, sourceFiles, cancellationToken) : undefined;
256256
}
257257

258-
if (symbol.flags & SymbolFlags.Module && isModuleReferenceLocation(node)) {
259-
return getReferencedSymbolsForModule(program, symbol, sourceFiles, sourceFilesSet);
258+
let moduleReferences: SymbolAndEntries[] = emptyArray;
259+
const moduleSourceFile = isModuleSymbol(symbol);
260+
if (moduleSourceFile) {
261+
const exportEquals = symbol.exports.get(InternalSymbolName.ExportEquals);
262+
// If !!exportEquals, we're about to add references to `import("mod")` anyway, so don't double-count them.
263+
moduleReferences = getReferencedSymbolsForModule(program, symbol, !!exportEquals, sourceFiles, sourceFilesSet);
264+
if (!exportEquals || !sourceFilesSet.has(moduleSourceFile.fileName)) return moduleReferences;
265+
// Continue to get references to 'export ='.
266+
symbol = skipAlias(exportEquals, checker);
267+
node = undefined;
260268
}
261-
262-
return getReferencedSymbolsForSymbol(symbol, node, sourceFiles, sourceFilesSet, checker, cancellationToken, options);
269+
return concatenate(moduleReferences, getReferencedSymbolsForSymbol(symbol, node, sourceFiles, sourceFilesSet, checker, cancellationToken, options));
263270
}
264271

265-
function isModuleReferenceLocation(node: Node): boolean {
266-
if (!isStringLiteralLike(node)) {
267-
return false;
268-
}
269-
switch (node.parent.kind) {
270-
case SyntaxKind.ModuleDeclaration:
271-
case SyntaxKind.ExternalModuleReference:
272-
case SyntaxKind.ImportDeclaration:
273-
case SyntaxKind.ExportDeclaration:
274-
return true;
275-
case SyntaxKind.LiteralType:
276-
return isImportTypeNode(node.parent.parent);
277-
case SyntaxKind.CallExpression:
278-
return isRequireCall(node.parent as CallExpression, /*checkArgumentIsStringLiteralLike*/ false) || isImportCall(node.parent as CallExpression);
279-
default:
280-
return false;
281-
}
272+
function isModuleSymbol(symbol: Symbol): SourceFile | undefined {
273+
return symbol.flags & SymbolFlags.Module && find(symbol.declarations, isSourceFile);
282274
}
283275

284-
function getReferencedSymbolsForModule(program: Program, symbol: Symbol, sourceFiles: ReadonlyArray<SourceFile>, sourceFilesSet: ReadonlyMap<true>): SymbolAndEntries[] {
276+
function getReferencedSymbolsForModule(program: Program, symbol: Symbol, excludeImportTypeOfExportEquals: boolean, sourceFiles: ReadonlyArray<SourceFile>, sourceFilesSet: ReadonlyMap<true>): SymbolAndEntries[] {
285277
Debug.assert(!!symbol.valueDeclaration);
286278

287-
const references = findModuleReferences(program, sourceFiles, symbol).map<Entry>(reference => {
279+
const references = mapDefined<ModuleReference, Entry>(findModuleReferences(program, sourceFiles, symbol), reference => {
288280
if (reference.kind === "import") {
281+
const parent = reference.literal.parent;
282+
if (isLiteralTypeNode(parent)) {
283+
const importType = cast(parent.parent, isImportTypeNode);
284+
if (excludeImportTypeOfExportEquals && !importType.qualifier) {
285+
return undefined;
286+
}
287+
}
288+
// import("foo") with no qualifier will reference the `export =` of the module, which may be referenced anyway.
289289
return { type: "node", node: reference.literal };
290290
}
291291
else {
@@ -308,11 +308,12 @@ namespace ts.FindAllReferences.Core {
308308
}
309309
break;
310310
default:
311+
// This may be merged with something.
311312
Debug.fail("Expected a module symbol to be declared by a SourceFile or ModuleDeclaration.");
312313
}
313314
}
314315

315-
return [{ definition: { type: "symbol", symbol }, references }];
316+
return references.length ? [{ definition: { type: "symbol", symbol }, references }] : emptyArray;
316317
}
317318

318319
/** getReferencedSymbols for special node kinds. */
@@ -345,21 +346,21 @@ namespace ts.FindAllReferences.Core {
345346
}
346347

347348
/** Core find-all-references algorithm for a normal symbol. */
348-
function getReferencedSymbolsForSymbol(symbol: Symbol, node: Node, sourceFiles: ReadonlyArray<SourceFile>, sourceFilesSet: ReadonlyMap<true>, checker: TypeChecker, cancellationToken: CancellationToken, options: Options): SymbolAndEntries[] {
349-
symbol = skipPastExportOrImportSpecifierOrUnion(symbol, node, checker) || symbol;
349+
function getReferencedSymbolsForSymbol(symbol: Symbol, node: Node | undefined, sourceFiles: ReadonlyArray<SourceFile>, sourceFilesSet: ReadonlyMap<true>, checker: TypeChecker, cancellationToken: CancellationToken, options: Options): SymbolAndEntries[] {
350+
symbol = node && skipPastExportOrImportSpecifierOrUnion(symbol, node, checker) || symbol;
350351

351352
// Compute the meaning from the location and the symbol it references
352-
const searchMeaning = getIntersectingMeaningFromDeclarations(node, symbol);
353+
const searchMeaning = node ? getIntersectingMeaningFromDeclarations(node, symbol) : SemanticMeaning.All;
353354

354355
const result: SymbolAndEntries[] = [];
355-
const state = new State(sourceFiles, sourceFilesSet, getSpecialSearchKind(node), checker, cancellationToken, searchMeaning, options, result);
356+
const state = new State(sourceFiles, sourceFilesSet, node ? getSpecialSearchKind(node) : SpecialSearchKind.None, checker, cancellationToken, searchMeaning, options, result);
356357

357-
if (node.kind === SyntaxKind.DefaultKeyword) {
358+
if (node && node.kind === SyntaxKind.DefaultKeyword) {
358359
addReference(node, symbol, state);
359360
searchForImportsOfExport(node, symbol, { exportingModuleSymbol: Debug.assertDefined(symbol.parent, "Expected export symbol to have a parent"), exportKind: ExportKind.Default }, state);
360361
}
361362
else {
362-
const search = state.createSearch(node, symbol, /*comingFrom*/ undefined, { allSearchSymbols: populateSearchSymbolSet(symbol, node, checker, options.implementations) });
363+
const search = state.createSearch(node, symbol, /*comingFrom*/ undefined, { allSearchSymbols: node ? populateSearchSymbolSet(symbol, node, checker, options.implementations) : [symbol] });
363364

364365
// Try to get the smallest valid scope that we can limit our search to;
365366
// otherwise we'll need to search globally (i.e. include each file).
@@ -499,7 +500,7 @@ namespace ts.FindAllReferences.Core {
499500
}
500501

501502
/** @param allSearchSymbols set of additinal symbols for use by `includes`. */
502-
createSearch(location: Node, symbol: Symbol, comingFrom: ImportExport | undefined, searchOptions: { text?: string, allSearchSymbols?: Symbol[] } = {}): Search {
503+
createSearch(location: Node | undefined, symbol: Symbol, comingFrom: ImportExport | undefined, searchOptions: { text?: string, allSearchSymbols?: Symbol[] } = {}): Search {
503504
// Note: if this is an external module symbol, the name doesn't include quotes.
504505
// Note: getLocalSymbolForExportDefault handles `export default class C {}`, but not `export default C` or `export { C as default }`.
505506
// The other two forms seem to be handled downstream (e.g. in `skipPastExportOrImportSpecifier`), so special-casing the first form
@@ -509,7 +510,7 @@ namespace ts.FindAllReferences.Core {
509510
allSearchSymbols = [symbol],
510511
} = searchOptions;
511512
const escapedText = escapeLeadingUnderscores(text);
512-
const parents = this.options.implementations && getParentSymbolsOfPropertyAccess(location, symbol, this.checker);
513+
const parents = this.options.implementations && location && getParentSymbolsOfPropertyAccess(location, symbol, this.checker);
513514
return { symbol, comingFrom, text, escapedText, parents, allSearchSymbols, includes: sym => contains(allSearchSymbols, sym) };
514515
}
515516

@@ -559,11 +560,7 @@ namespace ts.FindAllReferences.Core {
559560
if (singleReferences.length) {
560561
const addRef = state.referenceAdder(exportSymbol);
561562
for (const singleRef of singleReferences) {
562-
// At `default` in `import { default as x }` or `export { default as x }`, do add a reference, but do not rename.
563-
if (hasMatchingMeaning(singleRef, state) &&
564-
!(state.options.isForRename && (isExportSpecifier(singleRef.parent) || isImportSpecifier(singleRef.parent)) && singleRef.escapedText === InternalSymbolName.Default)) {
565-
addRef(singleRef);
566-
}
563+
if (shouldAddSingleReference(singleRef, state)) addRef(singleRef);
567564
}
568565
}
569566

@@ -593,6 +590,15 @@ namespace ts.FindAllReferences.Core {
593590
}
594591
}
595592

593+
function shouldAddSingleReference(singleRef: Identifier | StringLiteral, state: State): boolean {
594+
if (!hasMatchingMeaning(singleRef, state)) return false;
595+
if (!state.options.isForRename) return true;
596+
// Don't rename an import type `import("./module-name")` when renaming `name` in `export = name;`
597+
if (!isIdentifier(singleRef)) return false;
598+
// At `default` in `import { default as x }` or `export { default as x }`, do add a reference, but do not rename.
599+
return !((isExportSpecifier(singleRef.parent) || isImportSpecifier(singleRef.parent)) && singleRef.escapedText === InternalSymbolName.Default);
600+
}
601+
596602
// Go to the symbol we imported from and find references for it.
597603
function searchForImportedSymbol(symbol: Symbol, state: State): void {
598604
for (const declaration of symbol.declarations) {

src/services/importTracker.ts

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33
namespace ts.FindAllReferences {
44
export interface ImportsResult {
55
/** For every import of the symbol, the location and local symbol for the import. */
6-
importSearches: [Identifier, Symbol][];
6+
importSearches: ReadonlyArray<[Identifier, Symbol]>;
77
/** For rename imports/exports `{ foo as bar }`, `foo` is not a local, so it may be added as a reference immediately without further searching. */
8-
singleReferences: Identifier[];
8+
singleReferences: ReadonlyArray<Identifier | StringLiteral>;
99
/** List of source files that may (or may not) use the symbol via a namespace. (For UMD modules this is every file.) */
1010
indirectUsers: ReadonlyArray<SourceFile>;
1111
}
@@ -33,7 +33,7 @@ namespace ts.FindAllReferences {
3333
interface AmbientModuleDeclaration extends ModuleDeclaration { body?: ModuleBlock; }
3434
type SourceFileLike = SourceFile | AmbientModuleDeclaration;
3535
// Identifier for the case of `const x = require("y")`.
36-
type Importer = AnyImportOrReExport | ImportTypeNode | Identifier;
36+
type Importer = AnyImportOrReExport | ValidImportTypeNode | Identifier;
3737
type ImporterOrCallExpression = Importer | CallExpression;
3838

3939
/** Returns import statements that directly reference the exporting module, and a list of files that may access the module through a namespace. */
@@ -135,13 +135,7 @@ namespace ts.FindAllReferences {
135135
break;
136136

137137
case SyntaxKind.ImportType:
138-
if (direct.qualifier) {
139-
// `import("foo").x` named import
140-
directImports.push(direct);
141-
}
142-
else {
143-
// TODO: GH#23879
144-
}
138+
directImports.push(direct);
145139
break;
146140

147141
default:
@@ -205,7 +199,7 @@ namespace ts.FindAllReferences {
205199
*/
206200
function getSearchesFromDirectImports(directImports: Importer[], exportSymbol: Symbol, exportKind: ExportKind, checker: TypeChecker, isForRename: boolean): Pick<ImportsResult, "importSearches" | "singleReferences"> {
207201
const importSearches: [Identifier, Symbol][] = [];
208-
const singleReferences: Identifier[] = [];
202+
const singleReferences: (Identifier | StringLiteral)[] = [];
209203
function addSearch(location: Identifier, symbol: Symbol): void {
210204
importSearches.push([location, symbol]);
211205
}
@@ -232,8 +226,13 @@ namespace ts.FindAllReferences {
232226
}
233227

234228
if (decl.kind === SyntaxKind.ImportType) {
235-
if (decl.qualifier) { // TODO: GH#23879
236-
singleReferences.push(decl.qualifier.kind === SyntaxKind.Identifier ? decl.qualifier : decl.qualifier.right);
229+
if (decl.qualifier) {
230+
if (isIdentifier(decl.qualifier) && decl.qualifier.escapedText === symbolName(exportSymbol)) {
231+
singleReferences.push(decl.qualifier);
232+
}
233+
}
234+
else if (exportKind === ExportKind.ExportEquals) {
235+
singleReferences.push(decl.argument.literal);
237236
}
238237
return;
239238
}

src/services/rename.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ namespace ts.Rename {
3636
return undefined;
3737
}
3838

39+
// Can't rename a module name.
40+
if (isStringLiteralLike(node) && tryGetImportFromModuleSpecifier(node)) return undefined;
41+
3942
const kind = SymbolDisplay.getSymbolKind(typeChecker, symbol, node);
4043
const specifierName = (isImportOrExportSpecifierName(node) || isStringOrNumericLiteral(node) && node.parent.kind === SyntaxKind.ComputedPropertyName)
4144
? stripQuotes(getTextOfIdentifierOrLiteral(node))

src/services/utilities.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,10 @@ namespace ts {
107107
Debug.assert(isJSDocTemplateTag(node.parent.parent)); // Else would be handled by isDeclarationName
108108
return SemanticMeaning.Type;
109109
}
110+
else if (isLiteralTypeNode(node.parent)) {
111+
// This might be T["name"], which is actually referencing a property and not a type. So allow both meanings.
112+
return SemanticMeaning.Type | SemanticMeaning.Value;
113+
}
110114
else {
111115
return SemanticMeaning.Value;
112116
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @Filename: /a.ts
4+
////type [|{| "isWriteAccess": true, "isDefinition": true |}T|] = number;
5+
////export = [|T|];
6+
7+
// @Filename: /b.ts
8+
////import [|{| "isWriteAccess": true, "isDefinition": true |}T|] = require("[|./a|]");
9+
10+
const [r0, r1, r2, r3] = test.ranges();
11+
const mod = { definition: 'module "/a"', ranges: [r3] };
12+
const a = { definition: "type T = number", ranges: [r0, r1] };
13+
const b = { definition: '(alias) type T = number\nimport T = require("./a")', ranges: [r2] };
14+
verify.referenceGroups([r0, r1], [a, b]);
15+
verify.referenceGroups(r2, [b, a]);
16+
verify.referenceGroups(r3, [mod, a, b]);

tests/cases/fourslash/findAllRefsForModuleGlobal.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,4 @@
99
////declare module "[|{| "isWriteAccess": true, "isDefinition": true |}foo|]" {}
1010

1111
verify.noErrors();
12-
13-
const ranges = test.ranges();
14-
const [r0, r1, r2] = ranges;
1512
verify.singleReferenceGroup('module "/node_modules/foo/index"');

tests/cases/fourslash/findAllRefsImportTypeOfModule.ts

Lines changed: 0 additions & 13 deletions
This file was deleted.

0 commit comments

Comments
 (0)