When reporting unused elements, do not count a CommentReference as usage

Fixes #37116

Change-Id: I9ac563036a367a5e5da4fbba8ab521f174062570
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/127223
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

Author: srawlins

pkg/analyzer/lib/src/error/unused_local_elements_verifier.dart

@@ -88,6 +88,9 @@ class GatherUsedLocalElementsVisitor extends RecursiveAstVisitor {
     if (node.inDeclarationContext()) {
       return;
     }
+    if (_inCommentReference(node)) {
+      return;
+    }
     Element element = node.staticElement;
     // Store un-parameterized members.
     if (element is ExecutableMember) {
@@ -131,18 +134,18 @@ class GatherUsedLocalElementsVisitor extends RecursiveAstVisitor {
     if (element == null) {
       return;
     }
-    // check if a local element
+    // Check if [element] is a local element.
     if (!identical(element.library, _enclosingLibrary)) {
       return;
     }
-    // ignore references to an element from itself
+    // Ignore references to an element from itself.
     if (identical(element, _enclosingClass)) {
       return;
     }
     if (identical(element, _enclosingExec)) {
       return;
     }
-    // ignore places where the element is not actually used
+    // Ignore places where the element is not actually used.
     if (node.parent is TypeName) {
       if (element is ClassElement) {
         AstNode parent2 = node.parent.parent;
@@ -161,13 +164,24 @@ class GatherUsedLocalElementsVisitor extends RecursiveAstVisitor {
     usedElements.addElement(element);
   }
 
+  /// Returns whether [identifier] is found in a [CommentReference].
+  static bool _inCommentReference(SimpleIdentifier identifier) {
+    var parent = identifier.parent;
+    return parent is CommentReference || parent?.parent is CommentReference;
+  }
+
+  /// Returns whether the value of [node] is _only_ being read at this position.
+  ///
+  /// Returns `false` if [node] is not a read access, or if [node] is a combined
+  /// read/write access.
   static bool _isReadIdentifier(SimpleIdentifier node) {
-    // not reading at all
+    // Not reading at all.
     if (!node.inGetterContext()) {
       return false;
     }
-    // check if useless reading
+    // Check if useless reading.
     AstNode parent = node.parent;
+
     if (parent.parent is ExpressionStatement) {
       if (parent is PrefixExpression || parent is PostfixExpression) {
         // v++;

pkg/analyzer/test/src/diagnostics/unused_element_test.dart

@@ -264,6 +264,15 @@ main() {
     ]);
   }
 
+  test_functionTop_notUsed_referenceInComment() async {
+    await assertErrorsInCode(r'''
+/// [_f] is a great function.
+_f(int p) => 7;
+''', [
+      error(HintCode.UNUSED_ELEMENT, 30, 2),
+    ]);
+  }
+
   test_functionTypeAlias_isUsed_isExpression() async {
     await assertNoErrorsInCode(r'''
 typedef _F(a, b);
@@ -461,20 +470,6 @@ class A {
     ]);
   }
 
-  test_method_isNotUsed_hasSameNameAsUsed() async {
-    await assertErrorsInCode(r'''
-class A {
-  void _m1() {}
-}
-class B {
-  void public() => _m1();
-  void _m1() {}
-}
-''', [
-      error(HintCode.UNUSED_ELEMENT, 17, 3),
-    ]);
-  }
-
   test_method_isUsed_hasReference_implicitThis() async {
     await assertNoErrorsInCode(r'''
 class A {
@@ -645,6 +640,20 @@ main() {
 ''');
   }
 
+  test_method_notUsed_hasSameNameAsUsed() async {
+    await assertErrorsInCode(r'''
+class A {
+  void _m1() {}
+}
+class B {
+  void public() => _m1();
+  void _m1() {}
+}
+''', [
+      error(HintCode.UNUSED_ELEMENT, 17, 3),
+    ]);
+  }
+
   test_method_notUsed_noReference() async {
     await assertErrorsInCode(r'''
 class A {
@@ -667,6 +676,29 @@ class A {
     ]);
   }
 
+  test_method_notUsed_referenceInComment() async {
+    await assertErrorsInCode(r'''
+/// [A] has a method, [_f].
+class A {
+  int _f(int p) => 7;
+}
+''', [
+      error(HintCode.UNUSED_ELEMENT, 44, 2),
+    ]);
+  }
+
+  test_method_notUsed_referenceInComment_outsideEnclosingClass() async {
+    await assertErrorsInCode(r'''
+class A {
+  int _f(int p) => 7;
+}
+/// This is similar to [A._f].
+int g() => 7;
+''', [
+      error(HintCode.UNUSED_ELEMENT, 16, 2),
+    ]);
+  }
+
   test_setter_isUsed_invocation_implicitThis() async {
     await assertNoErrorsInCode(r'''
 class A {
@@ -753,4 +785,13 @@ main() {
       error(HintCode.UNUSED_ELEMENT, 4, 2),
     ]);
   }
+
+  test_topLevelVariable_notUsed_referenceInComment() async {
+    await assertErrorsInCode(r'''
+/// [_a] is a great variable.
+int _a = 7;
+''', [
+      error(HintCode.UNUSED_ELEMENT, 34, 2),
+    ]);
+  }
 }

pkg/analyzer/test/src/diagnostics/unused_field_test.dart

@@ -278,6 +278,17 @@ class A {
     ]);
   }
 
+  test_notUsed_referenceInComment() async {
+    await assertErrorsInCode(r'''
+/// [A._f] is great.
+class A {
+  int _f;
+}
+''', [
+      error(HintCode.UNUSED_FIELD, 37, 2),
+    ]);
+  }
+
   test_notUsed_simpleAssignment() async {
     await assertErrorsInCode(r'''
 class A {