Skip to content

Commit f0fe1b8

Browse files
sandersnsandersn
authored
Make isDefinition aware of declaring symbol (#45920)
* Make isDefinition aware of target symbol Initial code, haven't fixed any tests yet. * Update baselines This commit includes a regression for commonjs aliases: ```js // @filename: a.js function f() { } module.exports.f = f // @filename: b.js const { f } = require('./a') f/**/ ``` Now says that `f` in b.js has 1 reference -- the alias `module.exports.f = f`. This is not correct (or not exactly correct), but correctly fixing will involve re-creating the ad-hoc commonjs alias resolution code from the checker. I don't think it's worth it for an edge case like this. * update more unit tests * Fix symbol lookup for constructors * More baselines + two fixes 1. Fix `default` support. 2. Add a secondary declaration location for commonjs assignment declarations. * Update rest of baselines * Switch a few more tests over to baselines
1 parent 110b059 commit f0fe1b8

File tree

145 files changed

+3552
-705
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

145 files changed

+3552
-705
lines changed

‎src/services/findAllReferences.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -208,11 +208,12 @@ namespace ts.FindAllReferences {
208208
const node = getTouchingPropertyName(sourceFile, position);
209209
const referencedSymbols = Core.getReferencedSymbolsForNode(position, node, program, sourceFiles, cancellationToken, { use: FindReferencesUse.References });
210210
const checker = program.getTypeChecker();
211+
const symbol = checker.getSymbolAtLocation(node);
211212
return !referencedSymbols || !referencedSymbols.length ? undefined : mapDefined<SymbolAndEntries, ReferencedSymbol>(referencedSymbols, ({ definition, references }) =>
212213
// Only include referenced symbols that have a valid definition.
213214
definition && {
214215
definition: checker.runWithCancellationToken(cancellationToken, checker => definitionToReferencedSymbolDefinitionInfo(definition, checker, node)),
215-
references: references.map(toReferenceEntry)
216+
references: references.map(r => toReferenceEntry(r, symbol))
216217
});
217218
}
218219

@@ -387,7 +388,7 @@ namespace ts.FindAllReferences {
387388
return { ...entryToDocumentSpan(entry), ...(providePrefixAndSuffixText && getPrefixAndSuffixText(entry, originalNode, checker)) };
388389
}
389390

390-
export function toReferenceEntry(entry: Entry): ReferenceEntry {
391+
export function toReferenceEntry(entry: Entry, symbol: Symbol | undefined): ReferenceEntry {
391392
const documentSpan = entryToDocumentSpan(entry);
392393
if (entry.kind === EntryKind.Span) {
393394
return { ...documentSpan, isWriteAccess: false, isDefinition: false };
@@ -396,7 +397,7 @@ namespace ts.FindAllReferences {
396397
return {
397398
...documentSpan,
398399
isWriteAccess: isWriteAccessForReference(node),
399-
isDefinition: isDefinitionForReference(node),
400+
isDefinition: isDeclarationOfSymbol(node, symbol),
400401
isInString: kind === EntryKind.StringLiteral ? true : undefined,
401402
};
402403
}
@@ -544,11 +545,16 @@ namespace ts.FindAllReferences {
544545
return !!decl && declarationIsWriteAccess(decl) || node.kind === SyntaxKind.DefaultKeyword || isWriteAccess(node);
545546
}
546547

547-
function isDefinitionForReference(node: Node): boolean {
548-
return node.kind === SyntaxKind.DefaultKeyword
549-
|| !!getDeclarationFromName(node)
550-
|| isLiteralComputedPropertyDeclarationName(node)
551-
|| (node.kind === SyntaxKind.ConstructorKeyword && isConstructorDeclaration(node.parent));
548+
/** Whether a reference, `node`, is a definition of the `target` symbol */
549+
function isDeclarationOfSymbol(node: Node, target: Symbol | undefined): boolean {
550+
if (!target) return false;
551+
const source = getDeclarationFromName(node) ||
552+
(node.kind === SyntaxKind.DefaultKeyword ? node.parent
553+
: isLiteralComputedPropertyDeclarationName(node) ? node.parent.parent
554+
: node.kind === SyntaxKind.ConstructorKeyword && isConstructorDeclaration(node.parent) ? node.parent.parent
555+
: undefined);
556+
const commonjsSource = source && isBinaryExpression(source) ? source.left as unknown as Declaration : undefined;
557+
return !!(source && target.declarations?.some(d => d === source || d === commonjsSource));
552558
}
553559

554560
/**

‎src/services/services.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1763,7 +1763,7 @@ namespace ts {
17631763

17641764
function getReferencesAtPosition(fileName: string, position: number): ReferenceEntry[] | undefined {
17651765
synchronizeHostData();
1766-
return getReferencesWorker(getTouchingPropertyName(getValidSourceFile(fileName), position), position, { use: FindAllReferences.FindReferencesUse.References }, FindAllReferences.toReferenceEntry);
1766+
return getReferencesWorker(getTouchingPropertyName(getValidSourceFile(fileName), position), position, { use: FindAllReferences.FindReferencesUse.References }, (entry, node, checker) => FindAllReferences.toReferenceEntry(entry, checker.getSymbolAtLocation(node)));
17671767
}
17681768

17691769
function getReferencesWorker<T>(node: Node, position: number, options: FindAllReferences.Options, cb: FindAllReferences.ToReferenceOrRenameEntry<T>): T[] | undefined {
@@ -1784,7 +1784,8 @@ namespace ts {
17841784

17851785
function getFileReferences(fileName: string): ReferenceEntry[] {
17861786
synchronizeHostData();
1787-
return FindAllReferences.Core.getReferencesForFileName(fileName, program, program.getSourceFiles()).map(FindAllReferences.toReferenceEntry);
1787+
const moduleSymbol = program.getSourceFile(fileName)?.symbol;
1788+
return FindAllReferences.Core.getReferencesForFileName(fileName, program, program.getSourceFiles()).map(r => FindAllReferences.toReferenceEntry(r, moduleSymbol));
17881789
}
17891790

17901791
function getNavigateToItems(searchValue: string, maxResultCount?: number, fileName?: string, excludeDtsFiles = false): NavigateToItem[] {

‎src/testRunner/unittests/tsserver/getExportReferences.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ ${exportNestedObject}
3232
const referenceMainTs = (mainTs: File, text: string): protocol.ReferencesResponseItem =>
3333
makeReferenceItem({
3434
file: mainTs,
35-
isDefinition: true,
35+
isDefinition: false,
36+
isWriteAccess: true,
3637
lineText: mainTs.content,
3738
contextText: mainTs.content,
3839
text,
@@ -113,8 +114,11 @@ ${exportNestedObject}
113114
referenceMainTs(mainTs, "valueC"),
114115
referenceModTs(
115116
{ text: "valueC", lineText: exportObjectDestructured, contextText: "valueC: 0" },
116-
{ options: { index: 1 } },
117-
),
117+
{
118+
options: { index: 1 },
119+
isDefinition: false,
120+
isWriteAccess: true,
121+
}),
118122
],
119123
symbolDisplayString: "const valueC: number",
120124
symbolName: "valueC",
@@ -171,7 +175,11 @@ ${exportNestedObject}
171175
lineText: exportNestedObject,
172176
contextText: "valueF: 1",
173177
},
174-
{ options: { index: 1 } },
178+
{
179+
options: { index: 1 },
180+
isDefinition: false,
181+
isWriteAccess: true,
182+
},
175183
),
176184
],
177185
symbolDisplayString: "const valueF: number",

‎src/testRunner/unittests/tsserver/projectReferences.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,8 @@ function foo() {
186186
file: keyboardTestTs,
187187
text: searchStr,
188188
contextText: importStr,
189-
isDefinition: true,
189+
isDefinition: false,
190+
isWriteAccess: true,
190191
lineText: importStr
191192
}),
192193
makeReferenceItem({
@@ -200,7 +201,8 @@ function foo() {
200201
file: terminalTs,
201202
text: searchStr,
202203
contextText: importStr,
203-
isDefinition: true,
204+
isDefinition: false,
205+
isWriteAccess: true,
204206
lineText: importStr
205207
}),
206208
makeReferenceItem({

‎tests/baselines/reference/duplicatePackageServices.baseline.jsonc

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@
141141
"length": 49
142142
},
143143
"isWriteAccess": true,
144-
"isDefinition": true
144+
"isDefinition": false
145145
}
146146
]
147147
},
@@ -219,7 +219,7 @@
219219
"length": 18
220220
},
221221
"isWriteAccess": true,
222-
"isDefinition": true
222+
"isDefinition": false
223223
},
224224
{
225225
"textSpan": {
@@ -368,7 +368,7 @@
368368
"length": 18
369369
},
370370
"isWriteAccess": true,
371-
"isDefinition": true
371+
"isDefinition": false
372372
},
373373
{
374374
"textSpan": {
@@ -455,7 +455,7 @@
455455
"length": 18
456456
},
457457
"isWriteAccess": true,
458-
"isDefinition": true
458+
"isDefinition": false
459459
},
460460
{
461461
"textSpan": {
@@ -613,7 +613,7 @@
613613
"length": 49
614614
},
615615
"isWriteAccess": true,
616-
"isDefinition": true
616+
"isDefinition": false
617617
}
618618
]
619619
},
@@ -691,7 +691,7 @@
691691
"length": 18
692692
},
693693
"isWriteAccess": true,
694-
"isDefinition": true
694+
"isDefinition": false
695695
},
696696
{
697697
"textSpan": {

‎tests/baselines/reference/findAllReferencesDynamicImport3.baseline.jsonc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@
7878
"length": 7
7979
},
8080
"isWriteAccess": true,
81-
"isDefinition": true
81+
"isDefinition": false
8282
}
8383
]
8484
}
@@ -151,7 +151,7 @@
151151
"length": 39
152152
},
153153
"isWriteAccess": true,
154-
"isDefinition": true
154+
"isDefinition": false
155155
}
156156
]
157157
},

0 commit comments

Comments
 (0)