Skip to content

Make isDefinition aware of declaring symbol #45920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Sep 22, 2021
Merged

Make isDefinition aware of declaring symbol #45920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a73b14c
Make isDefinition aware of target symbol
sandersn Sep 16, 2021
3c114e3
Update baselines
sandersn Sep 16, 2021
358d32a
update more unit tests
sandersn Sep 16, 2021
9f8f8e1
Merge branch 'main' into make-isDefinition-symbol-aware
sandersn Sep 21, 2021
2356127
Fix symbol lookup for constructors
sandersn Sep 22, 2021
bc82414
More baselines + two fixes
sandersn Sep 22, 2021
8d72080
Update rest of baselines
sandersn Sep 22, 2021
988fecc
Switch a few more tests over to baselines
sandersn Sep 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions src/services/findAllReferences.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,12 @@ namespace ts.FindAllReferences {
const node = getTouchingPropertyName(sourceFile, position);
const referencedSymbols = Core.getReferencedSymbolsForNode(position, node, program, sourceFiles, cancellationToken, { use: FindReferencesUse.References });
const checker = program.getTypeChecker();
const symbol = checker.getSymbolAtLocation(node);
return !referencedSymbols || !referencedSymbols.length ? undefined : mapDefined<SymbolAndEntries, ReferencedSymbol>(referencedSymbols, ({ definition, references }) =>
// Only include referenced symbols that have a valid definition.
definition && {
definition: checker.runWithCancellationToken(cancellationToken, checker => definitionToReferencedSymbolDefinitionInfo(definition, checker, node)),
references: references.map(toReferenceEntry)
references: references.map(r => toReferenceEntry(r, symbol))
});
}

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

export function toReferenceEntry(entry: Entry): ReferenceEntry {
export function toReferenceEntry(entry: Entry, symbol: Symbol | undefined): ReferenceEntry {
const documentSpan = entryToDocumentSpan(entry);
if (entry.kind === EntryKind.Span) {
return { ...documentSpan, isWriteAccess: false, isDefinition: false };
Expand All @@ -396,7 +397,7 @@ namespace ts.FindAllReferences {
return {
...documentSpan,
isWriteAccess: isWriteAccessForReference(node),
isDefinition: isDefinitionForReference(node),
isDefinition: isDeclarationOfSymbol(node, symbol),
isInString: kind === EntryKind.StringLiteral ? true : undefined,
};
}
Expand Down Expand Up @@ -544,11 +545,16 @@ namespace ts.FindAllReferences {
return !!decl && declarationIsWriteAccess(decl) || node.kind === SyntaxKind.DefaultKeyword || isWriteAccess(node);
}

function isDefinitionForReference(node: Node): boolean {
return node.kind === SyntaxKind.DefaultKeyword
|| !!getDeclarationFromName(node)
|| isLiteralComputedPropertyDeclarationName(node)
|| (node.kind === SyntaxKind.ConstructorKeyword && isConstructorDeclaration(node.parent));
/** Whether a reference, `node`, is a definition of the `target` symbol */
function isDeclarationOfSymbol(node: Node, target: Symbol | undefined): boolean {
if (!target) return false;
const source = getDeclarationFromName(node) ||
(node.kind === SyntaxKind.DefaultKeyword ? node.parent
: isLiteralComputedPropertyDeclarationName(node) ? node.parent.parent
: node.kind === SyntaxKind.ConstructorKeyword && isConstructorDeclaration(node.parent) ? node.parent.parent
: undefined);
const commonjsSource = source && isBinaryExpression(source) ? source.left as unknown as Declaration : undefined;
return !!(source && target.declarations?.some(d => d === source || d === commonjsSource));
}

/**
Expand Down
5 changes: 3 additions & 2 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1763,7 +1763,7 @@ namespace ts {

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

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

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

function getNavigateToItems(searchValue: string, maxResultCount?: number, fileName?: string, excludeDtsFiles = false): NavigateToItem[] {
Expand Down
16 changes: 12 additions & 4 deletions src/testRunner/unittests/tsserver/getExportReferences.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ ${exportNestedObject}
const referenceMainTs = (mainTs: File, text: string): protocol.ReferencesResponseItem =>
makeReferenceItem({
file: mainTs,
isDefinition: true,
isDefinition: false,
isWriteAccess: true,
lineText: mainTs.content,
contextText: mainTs.content,
text,
Expand Down Expand Up @@ -113,8 +114,11 @@ ${exportNestedObject}
referenceMainTs(mainTs, "valueC"),
referenceModTs(
{ text: "valueC", lineText: exportObjectDestructured, contextText: "valueC: 0" },
{ options: { index: 1 } },
),
{
options: { index: 1 },
isDefinition: false,
isWriteAccess: true,
}),
],
symbolDisplayString: "const valueC: number",
symbolName: "valueC",
Expand Down Expand Up @@ -171,7 +175,11 @@ ${exportNestedObject}
lineText: exportNestedObject,
contextText: "valueF: 1",
},
{ options: { index: 1 } },
{
options: { index: 1 },
isDefinition: false,
isWriteAccess: true,
},
),
],
symbolDisplayString: "const valueF: number",
Expand Down
6 changes: 4 additions & 2 deletions src/testRunner/unittests/tsserver/projectReferences.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,8 @@ function foo() {
file: keyboardTestTs,
text: searchStr,
contextText: importStr,
isDefinition: true,
isDefinition: false,
isWriteAccess: true,
lineText: importStr
}),
makeReferenceItem({
Expand All @@ -200,7 +201,8 @@ function foo() {
file: terminalTs,
text: searchStr,
contextText: importStr,
isDefinition: true,
isDefinition: false,
isWriteAccess: true,
lineText: importStr
}),
makeReferenceItem({
Expand Down
12 changes: 6 additions & 6 deletions tests/baselines/reference/duplicatePackageServices.baseline.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@
"length": 49
},
"isWriteAccess": true,
"isDefinition": true
"isDefinition": false
}
]
},
Expand Down Expand Up @@ -219,7 +219,7 @@
"length": 18
},
"isWriteAccess": true,
"isDefinition": true
"isDefinition": false
},
{
"textSpan": {
Expand Down Expand Up @@ -368,7 +368,7 @@
"length": 18
},
"isWriteAccess": true,
"isDefinition": true
"isDefinition": false
},
{
"textSpan": {
Expand Down Expand Up @@ -455,7 +455,7 @@
"length": 18
},
"isWriteAccess": true,
"isDefinition": true
"isDefinition": false
},
{
"textSpan": {
Expand Down Expand Up @@ -613,7 +613,7 @@
"length": 49
},
"isWriteAccess": true,
"isDefinition": true
"isDefinition": false
}
]
},
Expand Down Expand Up @@ -691,7 +691,7 @@
"length": 18
},
"isWriteAccess": true,
"isDefinition": true
"isDefinition": false
},
{
"textSpan": {
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/findAllReferencesDynamicImport3.baseline.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
"length": 7
},
"isWriteAccess": true,
"isDefinition": true
"isDefinition": false
}
]
}
Expand Down Expand Up @@ -151,7 +151,7 @@
"length": 39
},
"isWriteAccess": true,
"isDefinition": true
"isDefinition": false
}
]
},
Expand Down
Loading