Fix readonly occurrences highlighting (#32583)

* Fix readonly occurrences highlighting

* Rename function

* Rename again

* Apply suggestions from code review

Remove unused function

src/services/documentHighlights.ts

@@ -193,7 +193,7 @@ namespace ts.DocumentHighlights {
 
     function getNodesToSearchForModifier(declaration: Node, modifierFlag: ModifierFlags): ReadonlyArray<Node> | undefined {
         // Types of node whose children might have modifiers.
-        const container = declaration.parent as ModuleBlock | SourceFile | Block | CaseClause | DefaultClause | ConstructorDeclaration | MethodDeclaration | FunctionDeclaration | ClassLikeDeclaration;
+        const container = declaration.parent as ModuleBlock | SourceFile | Block | CaseClause | DefaultClause | ConstructorDeclaration | MethodDeclaration | FunctionDeclaration | ObjectTypeDeclaration;
         switch (container.kind) {
             case SyntaxKind.ModuleBlock:
             case SyntaxKind.SourceFile:
@@ -213,11 +213,13 @@ namespace ts.DocumentHighlights {
                 return [...container.parameters, ...(isClassLike(container.parent) ? container.parent.members : [])];
             case SyntaxKind.ClassDeclaration:
             case SyntaxKind.ClassExpression:
+            case SyntaxKind.InterfaceDeclaration:
+            case SyntaxKind.TypeLiteral:
                 const nodes = container.members;
 
                 // If we're an accessibility modifier, we're in an instance member and should search
                 // the constructor's parameter list for instance members as well.
-                if (modifierFlag & ModifierFlags.AccessibilityModifier) {
+                if (modifierFlag & (ModifierFlags.AccessibilityModifier | ModifierFlags.Readonly)) {
                     const constructor = find(container.members, isConstructorDeclaration);
                     if (constructor) {
                         return [...nodes, ...constructor.parameters];

src/services/findAllReferences.ts

@@ -709,10 +709,28 @@ namespace ts.FindAllReferences.Core {
         return references.length ? [{ definition: { type: DefinitionKind.Symbol, symbol }, references }] : emptyArray;
     }
 
+    /** As in a `readonly prop: any` or `constructor(readonly prop: any)`, not a `readonly any[]`. */
+    function isReadonlyTypeOperator(node: Node): boolean {
+        return node.kind === SyntaxKind.ReadonlyKeyword
+            && isTypeOperatorNode(node.parent)
+            && node.parent.operator === SyntaxKind.ReadonlyKeyword;
+    }
+
     /** getReferencedSymbols for special node kinds. */
     function getReferencedSymbolsSpecial(node: Node, sourceFiles: ReadonlyArray<SourceFile>, cancellationToken: CancellationToken): SymbolAndEntries[] | undefined {
         if (isTypeKeyword(node.kind)) {
-            return getAllReferencesForKeyword(sourceFiles, node.kind, cancellationToken);
+            // A modifier readonly (like on a property declaration) is not special;
+            // a readonly type keyword (like `readonly string[]`) is.
+            if (node.kind === SyntaxKind.ReadonlyKeyword && !isReadonlyTypeOperator(node)) {
+                return undefined;
+            }
+            // Likewise, when we *are* looking for a special keyword, make sure we
+            // *don’t* include readonly member modifiers.
+            return getAllReferencesForKeyword(
+                sourceFiles,
+                node.kind,
+                cancellationToken,
+                node.kind === SyntaxKind.ReadonlyKeyword ? isReadonlyTypeOperator : undefined);
         }
 
         // Labels
@@ -1235,11 +1253,14 @@ namespace ts.FindAllReferences.Core {
         }
     }
 
-    function getAllReferencesForKeyword(sourceFiles: ReadonlyArray<SourceFile>, keywordKind: SyntaxKind, cancellationToken: CancellationToken): SymbolAndEntries[] | undefined {
+    function getAllReferencesForKeyword(sourceFiles: ReadonlyArray<SourceFile>, keywordKind: SyntaxKind, cancellationToken: CancellationToken, filter?: (node: Node) => boolean): SymbolAndEntries[] | undefined {
         const references = flatMap(sourceFiles, sourceFile => {
             cancellationToken.throwIfCancellationRequested();
-            return mapDefined(getPossibleSymbolReferenceNodes(sourceFile, tokenToString(keywordKind)!, sourceFile), referenceLocation =>
-                referenceLocation.kind === keywordKind ? nodeEntry(referenceLocation) : undefined);
+            return mapDefined(getPossibleSymbolReferenceNodes(sourceFile, tokenToString(keywordKind)!, sourceFile), referenceLocation => {
+                if (referenceLocation.kind === keywordKind && (!filter || filter(referenceLocation))) {
+                    return nodeEntry(referenceLocation);
+                }
+            });
         });
         return references.length ? [{ definition: { type: DefinitionKind.Keyword, node: references[0].node }, references }] : undefined;
     }

tests/cases/fourslash/documentHighlightsInvalidModifierLocations.ts

@@ -4,7 +4,12 @@
 ////    m([|readonly|] p) {}
 ////}
 ////function f([|readonly|] p) {}
+////
+////class D {
+////    m([|public|] p) {}
+////}
+////function g([|public|] p) {}
 
 for (const r of test.ranges()) {
-    verify.documentHighlightsOf(r, test.ranges());
+    verify.documentHighlightsOf(r, [r]);
 }

tests/cases/fourslash/getOccurrencesReadonly1.ts

@@ -0,0 +1,7 @@
+/// <reference path="fourslash.ts" />
+
+////interface I {
+////  [|readonly|] prop: string;
+////}
+
+verify.rangesAreOccurrences(false);

tests/cases/fourslash/getOccurrencesReadonly2.ts

@@ -0,0 +1,7 @@
+/// <reference path="fourslash.ts" />
+
+////type T = {
+////  [|readonly|] prop: string;
+////}
+
+verify.rangesAreOccurrences(false);

tests/cases/fourslash/getOccurrencesReadonly3.ts

@@ -0,0 +1,14 @@
+/// <reference path="fourslash.ts" />
+
+////class C {
+////  [|readonly|] prop: /**/readonly string[] = [];
+////  constructor([|readonly|] prop2: string) {
+////    class D {
+////      readonly prop: string = "";  
+////    }
+////  }
+////}
+
+verify.rangesAreOccurrences(false);
+goTo.marker();
+verify.occurrencesAtPositionCount(1);