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

* Don't report isDefinition unless the starting node is a declaration

* Drop isDefinition everywhere it isn't specifically needed

* Fix tsserver tests

* Update shim comment

* Update baselines

* Add tests for isDefinition

* Update doc comment

* Clear isDefinition from all references if the first one lacks it

Author: amcasey

src/harness/client.ts

@@ -583,7 +583,6 @@ namespace ts.server {
                 fileName: entry.file,
                 textSpan: this.decodeSpan(entry),
                 isWriteAccess: entry.isWriteAccess,
-                isDefinition: false
             }));
         }
 

src/server/protocol.ts

@@ -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;
     }
 
     /**

src/server/session.ts

@@ -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 ?
@@ -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);
                             }
                         }
@@ -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);

src/services/findAllReferences.ts

@@ -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;
@@ -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,
         };
     }

src/services/services.ts

@@ -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 }
                 }))
@@ -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 {
@@ -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[] {

src/services/shims.ts

@@ -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.

src/services/types.ts

@@ -886,7 +886,6 @@ namespace ts {
 
     export interface ReferenceEntry extends DocumentSpan {
         isWriteAccess: boolean;
-        isDefinition: boolean;
         isInString?: true;
     }
 
@@ -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 {

src/testRunner/unittests/tsserver/declarationFileMaps.ts

@@ -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 {
@@ -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; }"
             }),
@@ -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",
@@ -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",
@@ -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() {}" }),
                     ],
                 },
             ]);
@@ -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 },
@@ -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; }"
                     }),

src/testRunner/unittests/tsserver/getFileReferences.ts

@@ -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}"`,
             };

src/testRunner/unittests/tsserver/helpers.ts

@@ -713,7 +713,7 @@ namespace ts.projectSystem {
     }
 
     export interface MakeReferenceItem extends DocumentSpanFromSubstring {
-        isDefinition: boolean;
+        isDefinition?: boolean;
         isWriteAccess?: boolean;
         lineText: string;
     }
@@ -722,7 +722,7 @@ namespace ts.projectSystem {
         return {
             ...protocolFileSpanWithContextFromSubstring(rest),
             isDefinition,
-            isWriteAccess: isWriteAccess === undefined ? isDefinition : isWriteAccess,
+            isWriteAccess: isWriteAccess === undefined ? !!isDefinition : isWriteAccess,
             lineText,
         };
     }