Skip to content

Commit df9e262

Browse files
author
Andy
authored
Fix find-all-refs for import("mod").Foo where Foo is a typedef (#23881)
* Fix find-all-refs for import("mod").Foo where Foo is a typedef * Add Debug.assertNever message * Ensure SemanticMeaning is matched
1 parent 5133c70 commit df9e262

7 files changed

+85
-14
lines changed

Diff for: src/services/findAllReferences.ts

+7-4
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,8 @@ namespace ts.FindAllReferences.Core {
559559
const addRef = state.referenceAdder(exportSymbol);
560560
for (const singleRef of singleReferences) {
561561
// At `default` in `import { default as x }` or `export { default as x }`, do add a reference, but do not rename.
562-
if (!(state.options.isForRename && (isExportSpecifier(singleRef.parent) || isImportSpecifier(singleRef.parent)) && singleRef.escapedText === InternalSymbolName.Default)) {
562+
if (hasMatchingMeaning(singleRef, state) &&
563+
!(state.options.isForRename && (isExportSpecifier(singleRef.parent) || isImportSpecifier(singleRef.parent)) && singleRef.escapedText === InternalSymbolName.Default)) {
563564
addRef(singleRef);
564565
}
565566
}
@@ -822,6 +823,10 @@ namespace ts.FindAllReferences.Core {
822823
}
823824
}
824825

826+
function hasMatchingMeaning(referenceLocation: Node, state: State): boolean {
827+
return !!(getMeaningFromLocation(referenceLocation) & state.searchMeaning);
828+
}
829+
825830
function getReferencesAtLocation(sourceFile: SourceFile, position: number, search: Search, state: State, addReferencesHere: boolean): void {
826831
const referenceLocation = getTouchingPropertyName(sourceFile, position, /*includeJsDocComment*/ true);
827832

@@ -840,9 +845,7 @@ namespace ts.FindAllReferences.Core {
840845
return;
841846
}
842847

843-
if (!(getMeaningFromLocation(referenceLocation) & state.searchMeaning)) {
844-
return;
845-
}
848+
if (!hasMatchingMeaning(referenceLocation, state)) return;
846849

847850
const referenceSymbol = state.checker.getSymbolAtLocation(referenceLocation);
848851
if (!referenceSymbol) {

Diff for: src/services/importTracker.ts

+26-10
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,9 @@ namespace ts.FindAllReferences {
9999
}
100100
break;
101101

102+
case SyntaxKind.Identifier: // for 'const x = require("y");
103+
break; // TODO: GH#23879
104+
102105
case SyntaxKind.ImportEqualsDeclaration:
103106
handleNamespaceImport(direct, direct.name, hasModifier(direct, ModifierFlags.Export));
104107
break;
@@ -130,6 +133,19 @@ namespace ts.FindAllReferences {
130133
directImports.push(direct);
131134
}
132135
break;
136+
137+
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+
}
145+
break;
146+
147+
default:
148+
Debug.assertNever(direct, `Unexpected import kind: ${Debug.showSyntaxKind(direct)}`);
133149
}
134150
}
135151
}
@@ -216,6 +232,9 @@ namespace ts.FindAllReferences {
216232
}
217233

218234
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);
237+
}
219238
return;
220239
}
221240

@@ -313,16 +332,10 @@ namespace ts.FindAllReferences {
313332
const namespaceImportSymbol = checker.getSymbolAtLocation(name);
314333

315334
return forEachPossibleImportOrExportStatement(sourceFileLike, statement => {
316-
if (statement.kind !== SyntaxKind.ExportDeclaration) return;
317-
318-
const { exportClause, moduleSpecifier } = statement as ExportDeclaration;
319-
if (moduleSpecifier || !exportClause) return;
320-
321-
for (const element of exportClause.elements) {
322-
if (checker.getExportSpecifierLocalTargetSymbol(element) === namespaceImportSymbol) {
323-
return true;
324-
}
325-
}
335+
if (!isExportDeclaration(statement)) return;
336+
const { exportClause, moduleSpecifier } = statement;
337+
return !moduleSpecifier && exportClause &&
338+
exportClause.elements.some(element => checker.getExportSpecifierLocalTargetSymbol(element) === namespaceImportSymbol);
326339
});
327340
}
328341

@@ -485,6 +498,9 @@ namespace ts.FindAllReferences {
485498
else if (isBinaryExpression(parent.parent)) {
486499
return getSpecialPropertyExport(parent.parent, /*useLhsSymbol*/ true);
487500
}
501+
else if (isJSDocTypedefTag(parent)) {
502+
return exportInfo(symbol, ExportKind.Named);
503+
}
488504
}
489505

490506
function getExportAssignmentExport(ex: ExportAssignment): ExportedSymbol {

Diff for: src/services/utilities.ts

+2
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,8 @@ namespace ts {
180180
switch (node.parent.kind) {
181181
case SyntaxKind.TypeReference:
182182
return true;
183+
case SyntaxKind.ImportType:
184+
return !(node.parent as ImportTypeNode).isTypeOf;
183185
case SyntaxKind.ExpressionWithTypeArguments:
184186
return !isExpressionWithTypeArgumentsInClassExtendsClause(<ExpressionWithTypeArguments>node.parent);
185187
}

Diff for: tests/cases/fourslash/findAllRefsImportType.ts

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @allowJs: true
4+
5+
// @Filename: /a.js
6+
////module.exports = 0;
7+
////export type [|{| "isWriteAccess": true, "isDefinition": true |}N|] = number;
8+
9+
// @Filename: /b.js
10+
////type T = import("./a").[|N|];
11+
12+
verify.rangesReferenceEachOther();
+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////type [|{| "isWriteAccess": true, "isDefinition": true |}T|] = number;
4+
////export = [|T|];
5+
6+
////const x: [|import("./b")|] = 0;
7+
8+
// TODO: GH#23879 should just verify.rangesReferenceEachOther();
9+
const [r0, r1, r2] = test.ranges();
10+
verify.referenceGroups([r0, r1], [{ definition: "type T = number", ranges: [r0, r1] }]);
11+
verify.referenceGroups(r2, undefined);
+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @allowJs: true
4+
5+
// @Filename: /a.js
6+
////module.exports = 0;
7+
/////** @typedef {number} [|{| "isWriteAccess": true, "isDefinition": true |}Foo|] */
8+
////const dummy = 0;
9+
10+
// @Filename: /b.js
11+
/////** @type {import('./a').[|Foo|]} */
12+
////const x = 0;
13+
14+
verify.rangesReferenceEachOther();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @Filename: /a.ts
4+
////export type [|{| "isWriteAccess": true, "isDefinition": true |}T|] = 0;
5+
////export const [|{| "isWriteAccess": true, "isDefinition": true |}T|] = 0;
6+
7+
// @Filename: /b.ts
8+
////const x: import("./a").[|T|] = 0;
9+
////const x: typeof import("./a").[|T|] = 0;
10+
11+
const [r0, r1, r2, r3] = test.ranges();
12+
verify.rangesReferenceEachOther([r0, r2]);
13+
verify.rangesReferenceEachOther([r1, r3]);

0 commit comments

Comments
 (0)