Skip to content

Only report isDefinition when FAR is triggered on a definition #48566

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.

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
Apr 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion src/harness/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,6 @@ namespace ts.server {
fileName: entry.file,
textSpan: this.decodeSpan(entry),
isWriteAccess: entry.isWriteAccess,
isDefinition: false
}));
}

Expand Down
7 changes: 5 additions & 2 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1158,9 +1158,12 @@ namespace ts.server.protocol {
isWriteAccess: boolean;

/**
* True if reference is a definition, false otherwise.
* Present only if the search was triggered from a declaration.
* True indicates that the references refers to the same symbol
* (i.e. has the same meaning) as the declaration that began the
* search.
*/
isDefinition: boolean;
isDefinition?: boolean;
}

/**
Expand Down
6 changes: 5 additions & 1 deletion src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ namespace ts.server {
logger.info(`Finding references to ${location.fileName} position ${location.pos} in project ${project.getProjectName()}`);
const projectOutputs = project.getLanguageService().findReferences(location.fileName, location.pos);
if (projectOutputs) {
const clearIsDefinition = projectOutputs[0].references[0].isDefinition === undefined;
for (const referencedSymbol of projectOutputs) {
const mappedDefinitionFile = getMappedLocation(project, documentSpanLocation(referencedSymbol.definition));
const definition: ReferencedSymbolDefinitionInfo = mappedDefinitionFile === undefined ?
Expand All @@ -374,6 +375,9 @@ namespace ts.server {
for (const ref of referencedSymbol.references) {
// If it's in a mapped file, that is added to the todo list by `getMappedLocation`.
if (!contains(symbolToAddTo.references, ref, documentSpansEqual) && !getMappedLocation(project, documentSpanLocation(ref))) {
if (clearIsDefinition) {
delete ref.isDefinition;
}
symbolToAddTo.references.push(ref);
}
}
Expand Down Expand Up @@ -3260,7 +3264,7 @@ namespace ts.server {
return text;
}

function referenceEntryToReferencesResponseItem(projectService: ProjectService, { fileName, textSpan, contextSpan, isWriteAccess, isDefinition }: ReferenceEntry): protocol.ReferencesResponseItem {
function referenceEntryToReferencesResponseItem(projectService: ProjectService, { fileName, textSpan, contextSpan, isWriteAccess, isDefinition }: ReferencedSymbolEntry): protocol.ReferencesResponseItem {
const scriptInfo = Debug.checkDefined(projectService.getScriptInfo(fileName));
const span = toProtocolTextSpanWithContext(textSpan, contextSpan, scriptInfo);
const lineSpan = scriptInfo.lineToTextSpan(span.start.line - 1);
Expand Down
27 changes: 22 additions & 5 deletions src/services/findAllReferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,15 +209,24 @@ namespace ts.FindAllReferences {
const options = { use: FindReferencesUse.References };
const referencedSymbols = Core.getReferencedSymbolsForNode(position, node, program, sourceFiles, cancellationToken, options);
const checker = program.getTypeChecker();
const symbol = checker.getSymbolAtLocation(getAdjustedReferenceLocation(Core.getAdjustedNode(node, options)));
// Unless the starting node is a declaration (vs e.g. JSDoc), don't attempt to compute isDefinition
const adjustedNode = Core.getAdjustedNode(node, options);
const symbol = isDefinitionForReference(adjustedNode) ? checker.getSymbolAtLocation(adjustedNode) : undefined;
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(r => toReferenceEntry(r, symbol))
references: references.map(r => toReferencedSymbolEntry(r, symbol))
});
}

function isDefinitionForReference(node: Node): boolean {
return node.kind === SyntaxKind.DefaultKeyword
|| !!getDeclarationFromName(node)
|| isLiteralComputedPropertyDeclarationName(node)
|| (node.kind === SyntaxKind.ConstructorKeyword && isConstructorDeclaration(node.parent));
}

export function getImplementationsAtPosition(program: Program, cancellationToken: CancellationToken, sourceFiles: readonly SourceFile[], sourceFile: SourceFile, position: number): ImplementationLocation[] | undefined {
const node = getTouchingPropertyName(sourceFile, position);
let referenceEntries: Entry[] | undefined;
Expand Down Expand Up @@ -389,16 +398,24 @@ namespace ts.FindAllReferences {
return { ...entryToDocumentSpan(entry), ...(providePrefixAndSuffixText && getPrefixAndSuffixText(entry, originalNode, checker)) };
}

export function toReferenceEntry(entry: Entry, symbol: Symbol | undefined): ReferenceEntry {
function toReferencedSymbolEntry(entry: Entry, symbol: Symbol | undefined): ReferencedSymbolEntry {
const referenceEntry = toReferenceEntry(entry);
if (!symbol) return referenceEntry;
return {
...referenceEntry,
isDefinition: entry.kind !== EntryKind.Span && isDeclarationOfSymbol(entry.node, symbol)
};
}

export function toReferenceEntry(entry: Entry): ReferenceEntry {
const documentSpan = entryToDocumentSpan(entry);
if (entry.kind === EntryKind.Span) {
return { ...documentSpan, isWriteAccess: false, isDefinition: false };
return { ...documentSpan, isWriteAccess: false };
}
const { kind, node } = entry;
return {
...documentSpan,
isWriteAccess: isWriteAccessForReference(node),
isDefinition: isDeclarationOfSymbol(node, symbol),
isInString: kind === EntryKind.StringLiteral ? true : undefined,
};
}
Expand Down
6 changes: 2 additions & 4 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1798,7 +1798,6 @@ namespace ts {
fileName: entry.fileName,
textSpan: highlightSpan.textSpan,
isWriteAccess: highlightSpan.kind === HighlightSpanKind.writtenReference,
isDefinition: false,
...highlightSpan.isInString && { isInString: true },
...highlightSpan.contextSpan && { contextSpan: highlightSpan.contextSpan }
}))
Expand Down Expand Up @@ -1838,7 +1837,7 @@ namespace ts {

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

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

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

function getNavigateToItems(searchValue: string, maxResultCount?: number, fileName?: string, excludeDtsFiles = false): NavigateToItem[] {
Expand Down
2 changes: 1 addition & 1 deletion src/services/shims.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ namespace ts {

/**
* Returns a JSON-encoded value of the type:
* { fileName: string; highlights: { start: number; length: number, isDefinition: boolean }[] }[]
* { fileName: string; highlights: { start: number; length: number }[] }[]
*
* @param fileToSearch A JSON encoded string[] containing the file names that should be
* considered when searching.
Expand Down
7 changes: 5 additions & 2 deletions src/services/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -886,7 +886,6 @@ namespace ts {

export interface ReferenceEntry extends DocumentSpan {
isWriteAccess: boolean;
isDefinition: boolean;
isInString?: true;
}

Expand Down Expand Up @@ -1046,7 +1045,11 @@ namespace ts {

export interface ReferencedSymbol {
definition: ReferencedSymbolDefinitionInfo;
references: ReferenceEntry[];
references: ReferencedSymbolEntry[];
}

export interface ReferencedSymbolEntry extends ReferenceEntry {
isDefinition?: boolean;
}

export enum SymbolDisplayPartKind {
Expand Down
38 changes: 21 additions & 17 deletions src/testRunner/unittests/tsserver/declarationFileMaps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,20 @@ namespace ts.projectSystem {
}

interface MakeReferenceEntry extends DocumentSpanFromSubstring {
isDefinition: boolean;
isDefinition?: boolean;
isWriteAccess?: boolean;
}
function makeReferenceEntry({ isDefinition, ...rest }: MakeReferenceEntry): ReferenceEntry {
return {
function makeReferencedSymbolEntry({ isDefinition, isWriteAccess, ...rest }: MakeReferenceEntry): ReferencedSymbolEntry {
const result = {
...documentSpanFromSubstring(rest),
isDefinition,
isWriteAccess: isDefinition,
isWriteAccess: !!isWriteAccess,
isInString: undefined,
};
if (isDefinition === undefined) {
delete result.isDefinition;
}
return result;
}

function checkDeclarationFiles(file: File, session: TestSession, expectedFiles: readonly File[]): void {
Expand Down Expand Up @@ -373,17 +378,18 @@ namespace ts.projectSystem {
]);
});

const referenceATs = (aTs: File): protocol.ReferencesResponseItem => makeReferenceItem({
const referenceATs = (aTs: File, isDefinition: true | undefined): protocol.ReferencesResponseItem => makeReferenceItem({
file: aTs,
isDefinition: true,
isDefinition,
isWriteAccess: true,
text: "fnA",
contextText: "export function fnA() {}",
lineText: "export function fnA() {}"
});
const referencesUserTs = (userTs: File): readonly protocol.ReferencesResponseItem[] => [
const referencesUserTs = (userTs: File, isDefinition: false | undefined): readonly protocol.ReferencesResponseItem[] => [
makeReferenceItem({
file: userTs,
isDefinition: false,
isDefinition,
text: "fnA",
lineText: "export function fnUser() { a.fnA(); b.fnB(); a.instanceA; }"
}),
Expand All @@ -394,7 +400,7 @@ namespace ts.projectSystem {

const response = executeSessionRequest<protocol.ReferencesRequest, protocol.ReferencesResponse>(session, protocol.CommandTypes.References, protocolFileLocationFromSubstring(userTs, "fnA()"));
assert.deepEqual<protocol.ReferencesResponseBody | undefined>(response, {
refs: [...referencesUserTs(userTs), referenceATs(aTs)],
refs: [...referencesUserTs(userTs, /*isDefinition*/ undefined), referenceATs(aTs, /*isDefinition*/ true)], // Presently inconsistent across projects
symbolName: "fnA",
symbolStartOffset: protocolLocationFromSubstring(userTs.content, "fnA()").offset,
symbolDisplayString: "function fnA(): void",
Expand All @@ -408,7 +414,7 @@ namespace ts.projectSystem {
openFilesForSession([aTs], session); // If it's not opened, the reference isn't found.
const response = executeSessionRequest<protocol.ReferencesRequest, protocol.ReferencesResponse>(session, protocol.CommandTypes.References, protocolFileLocationFromSubstring(aTs, "fnA"));
assert.deepEqual<protocol.ReferencesResponseBody | undefined>(response, {
refs: [referenceATs(aTs), ...referencesUserTs(userTs)],
refs: [referenceATs(aTs, /*isDefinition*/ true), ...referencesUserTs(userTs, /*isDefinition*/ false)],
symbolName: "fnA",
symbolStartOffset: protocolLocationFromSubstring(aTs.content, "fnA").offset,
symbolDisplayString: "function fnA(): void",
Expand Down Expand Up @@ -448,8 +454,8 @@ namespace ts.projectSystem {
],
},
references: [
makeReferenceEntry({ file: userTs, /*isDefinition*/ isDefinition: false, text: "fnA" }),
makeReferenceEntry({ file: aTs, /*isDefinition*/ isDefinition: true, text: "fnA", contextText: "export function fnA() {}" }),
makeReferencedSymbolEntry({ file: userTs, text: "fnA" }),
makeReferencedSymbolEntry({ file: aTs, text: "fnA", isDefinition: true, isWriteAccess: true, contextText: "export function fnA() {}" }),
],
},
]);
Expand Down Expand Up @@ -502,16 +508,15 @@ namespace ts.projectSystem {
name: "function f(): void",
},
references: [
makeReferenceEntry({
makeReferencedSymbolEntry({
file: aTs,
text: "f",
options: { index: 1 },
contextText: "function f() {}",
isDefinition: true
isWriteAccess: true,
}),
{
fileName: bTs.path,
isDefinition: false,
isInString: undefined,
isWriteAccess: false,
textSpan: { start: 0, length: 1 },
Expand All @@ -529,14 +534,13 @@ namespace ts.projectSystem {
refs: [
makeReferenceItem({
file: bDts,
isDefinition: true,
isWriteAccess: true,
text: "fnB",
contextText: "export declare function fnB(): void;",
lineText: "export declare function fnB(): void;"
}),
makeReferenceItem({
file: userTs,
isDefinition: false,
text: "fnB",
lineText: "export function fnUser() { a.fnA(); b.fnB(); a.instanceA; }"
}),
Expand Down
8 changes: 4 additions & 4 deletions src/testRunner/unittests/tsserver/getFileReferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ namespace ts.projectSystem {

const expectResponse: protocol.FileReferencesResponseBody = {
refs: [
makeReferenceItem({ file: bTs, text: "./a", lineText: importA, contextText: importA, isDefinition: false, isWriteAccess: false }),
makeReferenceItem({ file: cTs, text: "./a", lineText: importCurlyFromA, contextText: importCurlyFromA, isDefinition: false, isWriteAccess: false }),
makeReferenceItem({ file: dTs, text: "/project/a", lineText: importAFromA, contextText: importAFromA, isDefinition: false, isWriteAccess: false }),
makeReferenceItem({ file: dTs, text: "./a", lineText: typeofImportA, contextText: typeofImportA, isDefinition: false, isWriteAccess: false }),
makeReferenceItem({ file: bTs, text: "./a", lineText: importA, contextText: importA, isWriteAccess: false }),
makeReferenceItem({ file: cTs, text: "./a", lineText: importCurlyFromA, contextText: importCurlyFromA, isWriteAccess: false }),
makeReferenceItem({ file: dTs, text: "/project/a", lineText: importAFromA, contextText: importAFromA, isWriteAccess: false }),
makeReferenceItem({ file: dTs, text: "./a", lineText: typeofImportA, contextText: typeofImportA, isWriteAccess: false }),
],
symbolName: `"${aTs.path}"`,
};
Expand Down
4 changes: 2 additions & 2 deletions src/testRunner/unittests/tsserver/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,7 @@ namespace ts.projectSystem {
}

export interface MakeReferenceItem extends DocumentSpanFromSubstring {
isDefinition: boolean;
isDefinition?: boolean;
isWriteAccess?: boolean;
lineText: string;
}
Expand All @@ -722,7 +722,7 @@ namespace ts.projectSystem {
return {
...protocolFileSpanWithContextFromSubstring(rest),
isDefinition,
isWriteAccess: isWriteAccess === undefined ? isDefinition : isWriteAccess,
isWriteAccess: isWriteAccess === undefined ? !!isDefinition : isWriteAccess,
lineText,
};
}
Expand Down
13 changes: 9 additions & 4 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6206,7 +6206,6 @@ declare namespace ts {
}
interface ReferenceEntry extends DocumentSpan {
isWriteAccess: boolean;
isDefinition: boolean;
isInString?: true;
}
interface ImplementationLocation extends DocumentSpan {
Expand Down Expand Up @@ -6320,7 +6319,10 @@ declare namespace ts {
}
interface ReferencedSymbol {
definition: ReferencedSymbolDefinitionInfo;
references: ReferenceEntry[];
references: ReferencedSymbolEntry[];
}
interface ReferencedSymbolEntry extends ReferenceEntry {
isDefinition?: boolean;
}
enum SymbolDisplayPartKind {
aliasName = 0,
Expand Down Expand Up @@ -7852,9 +7854,12 @@ declare namespace ts.server.protocol {
*/
isWriteAccess: boolean;
/**
* True if reference is a definition, false otherwise.
* Present only if the search was triggered from a declaration.
* True indicates that the references refers to the same symbol
* (i.e. has the same meaning) as the declaration that began the
* search.
*/
isDefinition: boolean;
isDefinition?: boolean;
}
/**
* The body of a "references" response message.
Expand Down
6 changes: 4 additions & 2 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6206,7 +6206,6 @@ declare namespace ts {
}
interface ReferenceEntry extends DocumentSpan {
isWriteAccess: boolean;
isDefinition: boolean;
isInString?: true;
}
interface ImplementationLocation extends DocumentSpan {
Expand Down Expand Up @@ -6320,7 +6319,10 @@ declare namespace ts {
}
interface ReferencedSymbol {
definition: ReferencedSymbolDefinitionInfo;
references: ReferenceEntry[];
references: ReferencedSymbolEntry[];
}
interface ReferencedSymbolEntry extends ReferenceEntry {
isDefinition?: boolean;
}
enum SymbolDisplayPartKind {
aliasName = 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@
"length": 1
},
"fileName": "/b/b.ts",
"isWriteAccess": false,
"isDefinition": false
"isWriteAccess": false
}
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,17 +142,15 @@ undefined
"start": 25,
"length": 23
},
"isWriteAccess": true,
"isDefinition": true
"isWriteAccess": true
},
{
"textSpan": {
"start": 21,
"length": 1
},
"fileName": "/b.ts",
"isWriteAccess": false,
"isDefinition": false
"isWriteAccess": false
}
]
}
Expand Down
Loading