analyzer: Prefer to doc-comment getters; refactor tests

Change-Id: I4f1660bd535e659193bac1953af6cf4701f79151 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/224084 Reviewed-by: Konstantin Shcheglov <scheglov@google.com> Commit-Queue: Samuel Rawlins <srawlins@google.com>
2024-09-16 03:17:55 +00:00 · 2021-12-15 05:41:05 +00:00 · 2021-12-15 05:41:05 +00:00 · cb78badbac
parent 78b7fc5ac1
commit cb78badbac
2 changed files with 376 additions and 363 deletions
--- a/pkg/analyzer/lib/src/dart/resolver/comment_reference_resolver.dart
+++ b/pkg/analyzer/lib/src/dart/resolver/comment_reference_resolver.dart
@ -162,13 +162,7 @@ class CommentReferenceResolver {
        propertyErrorEntity: identifier,
        nameErrorEntity: identifier,
      );
-      if (identifier.parent is CommentReference) {
-        // TODO(srawlins): Why is the setter preferred? This seems very flawed
-        // as it will only use the setter for a [SimpleIdentifier] comment
-        // reference, and not a [PrefixedIdentifier] or a [PropertyAccess].
-        element = result.setter;
-      }
-      element ??= result.getter;
+      element = result.getter ?? result.setter;
    }
    return element;
  }
--- a/pkg/analyzer/test/src/dart/resolution/comment_test.dart
+++ b/pkg/analyzer/test/src/dart/resolution/comment_test.dart
@ -9,11 +9,215 @@ import 'context_collection_resolution.dart';

 main() {
  defineReflectiveSuite(() {
-    defineReflectiveTests(CommentDriverResolutionTest);
+    defineReflectiveTests(CommentDriverResolution_PrefixedIdentifierTest);
    defineReflectiveTests(CommentDriverResolution_PropertyAccessTest);
+    defineReflectiveTests(CommentDriverResolution_SimpleIdentifierTest);
  });
 }

+@reflectiveTest
+class CommentDriverResolution_PrefixedIdentifierTest
+    extends PubPackageResolutionTest {
+  test_class_constructor_named() async {
+    // TODO(srawlins): improve coverage regarding constructors, operators, the
+    // 'new' keyword, and members on an extension on a type variable
+    // (`extension <T> on T`).
+    await assertNoErrorsInCode('''
+class A {
+  A.named();
+}
+
+/// [A.named]
+void f() {}
+''');
+
+    assertElement(findNode.simple('A.named]'), findElement.class_('A'));
+    assertElement(findNode.simple('named]'), findElement.constructor('named'));
+  }
+
+  test_class_constructor_unnamedViaNew() async {
+    await assertNoErrorsInCode('''
+class A {
+  A();
+}
+
+/// [A.new]
+void f() {}
+''');
+
+    assertElement(findNode.simple('A.new'), findElement.class_('A'));
+    assertElement(findNode.simple('new]'), findElement.unnamedConstructor('A'));
+  }
+
+  test_class_instanceGetter() async {
+    await assertNoErrorsInCode('''
+class A {
+  int get foo => 0;
+}
+
+/// [A.foo]
+void f() {}
+''');
+
+    assertElement(findNode.simple('A.foo'), findElement.class_('A'));
+    assertElement(findNode.simple('foo]'), findElement.getter('foo'));
+  }
+
+  test_class_instanceMethod() async {
+    await assertNoErrorsInCode('''
+class A {
+  void foo() {}
+}
+
+/// [A.foo]
+void f() {}
+''');
+
+    assertElement(findNode.simple('A.foo'), findElement.class_('A'));
+    assertElement(findNode.simple('foo]'), findElement.method('foo'));
+  }
+
+  test_class_instanceSetter() async {
+    await assertNoErrorsInCode('''
+class A {
+  set foo(int _) {}
+}
+
+/// [A.foo]
+void f() {}
+''');
+
+    assertElement(findNode.simple('A.foo'), findElement.class_('A'));
+    assertElement(findNode.simple('foo]'), findElement.setter('foo'));
+  }
+
+  test_class_staticGetter() async {
+    await assertNoErrorsInCode('''
+class A {
+  static int get foo => 0;
+}
+
+/// [A.foo]
+void f() {}
+''');
+
+    assertElement(findNode.simple('A.foo'), findElement.class_('A'));
+    assertElement(findNode.simple('foo]'), findElement.getter('foo'));
+  }
+
+  test_class_staticMethod() async {
+    await assertNoErrorsInCode('''
+class A {
+  static void foo() {}
+}
+
+/// [A.foo]
+void f() {}
+''');
+
+    assertElement(findNode.simple('A.foo'), findElement.class_('A'));
+    assertElement(findNode.simple('foo]'), findElement.method('foo'));
+  }
+
+  test_class_staticSetter() async {
+    await assertNoErrorsInCode('''
+class A {
+  static set foo(int _) {}
+}
+
+/// [A.foo]
+void f() {}
+''');
+
+    assertElement(findNode.simple('A.foo'), findElement.class_('A'));
+    assertElement(findNode.simple('foo]'), findElement.setter('foo'));
+  }
+
+  test_extension_instanceGetter() async {
+    await assertNoErrorsInCode('''
+extension E on int {
+  int get foo => 0;
+}
+
+/// [E.foo]
+void f() {}
+''');
+
+    assertElement(findNode.simple('E.foo'), findElement.extension_('E'));
+    assertElement(findNode.simple('foo]'), findElement.getter('foo'));
+  }
+
+  test_extension_instanceMethod() async {
+    await assertNoErrorsInCode('''
+extension E on int {
+  void foo() {}
+}
+
+/// [E.foo]
+void f() {}
+''');
+
+    assertElement(findNode.simple('E.foo'), findElement.extension_('E'));
+    assertElement(findNode.simple('foo]'), findElement.method('foo'));
+  }
+
+  test_extension_instanceSetter() async {
+    await assertNoErrorsInCode('''
+extension E on int {
+  set foo(int _) {}
+}
+
+/// [E.foo]
+void f() {}
+''');
+
+    assertElement(findNode.simple('E.foo'), findElement.extension_('E'));
+    assertElement(findNode.simple('foo]'), findElement.setter('foo'));
+  }
+
+  test_extension_staticGetter() async {
+    await assertNoErrorsInCode('''
+extension E on int {
+  static int get foo => 0;
+}
+
+/// [E.foo]
+void f() {}
+''');
+
+    assertElement(findNode.simple('E.foo'), findElement.extension_('E'));
+    assertElement(findNode.simple('foo]'), findElement.getter('foo'));
+  }
+
+  test_extension_staticMethod() async {
+    await assertNoErrorsInCode('''
+extension E on int {
+  static void foo() {}
+}
+
+/// [E.foo]
+void f() {}
+''');
+
+    assertElement(findNode.simple('E.foo'), findElement.extension_('E'));
+    assertElement(findNode.simple('foo]'), findElement.method('foo'));
+  }
+
+  test_extension_staticSetter() async {
+    await assertNoErrorsInCode('''
+extension E on int {
+  static set foo(int _) {}
+}
+
+/// [E.foo]
+void f() {}
+''');
+
+    assertElement(findNode.simple('E.foo'), findElement.extension_('E'));
+    assertElement(findNode.simple('foo]'), findElement.setter('foo'));
+  }
+}
+
@reflectiveTest
 class CommentDriverResolution_PropertyAccessTest
    extends PubPackageResolutionTest {
@ -244,7 +448,173 @@ void f() {}
 }

@reflectiveTest
-class CommentDriverResolutionTest extends PubPackageResolutionTest {
+class CommentDriverResolution_SimpleIdentifierTest
+    extends PubPackageResolutionTest {
+  test_associatedSetterAndGetter() async {
+    await assertNoErrorsInCode('''
+int get foo => 0;
+
+set foo(int value) {}
+
+/// [foo]
+void f() {}
+''');
+
+    assertElement(findNode.simple('foo]'), findElement.topGet('foo'));
+  }
+
+  test_associatedSetterAndGetter_setterInScope() async {
+    await assertNoErrorsInCode('''
+extension E1 on int {
+  int get foo => 0;
+}
+
+/// [foo]
+extension E2 on int {
+  set foo(int value) {}
+}
+''');
+
+    assertElement(findNode.simple('foo]'), findElement.setter('foo'));
+  }
+
+  test_beforeClass() async {
+    await assertNoErrorsInCode(r'''
+/// [foo]
+class A {
+  foo() {}
+}
+''');
+
+    assertElement(
+      findNode.simple('foo]'),
+      findElement.method('foo'),
+    );
+  }
+
+  test_beforeConstructor() async {
+    await assertNoErrorsInCode(r'''
+class A {
+  /// [p]
+  A(int p);
+}''');
+
+    assertElement(
+      findNode.simple('p]'),
+      findElement.parameter('p'),
+    );
+  }
+
+  test_beforeEnum() async {
+    await assertNoErrorsInCode(r'''
+/// This is the [Samurai] kind.
+enum Samurai {
+  /// Use [int].
+  WITH_SWORD,
+  /// Like [WITH_SWORD], but only without one.
+  WITHOUT_SWORD
+}''');
+
+    assertElement(
+      findNode.simple('Samurai]'),
+      findElement.enum_('Samurai'),
+    );
+    assertElement(
+      findNode.simple('int]'),
+      intElement,
+    );
+    assertElement(
+      findNode.simple('WITH_SWORD]'),
+      findElement.getter('WITH_SWORD'),
+    );
+  }
+
+  test_beforeFunction_blockBody() async {
+    await assertNoErrorsInCode(r'''
+/// [p]
+foo(int p) {}
+''');
+
+    assertElement(
+      findNode.simple('p]'),
+      findElement.parameter('p'),
+    );
+  }
+
+  test_beforeFunction_expressionBody() async {
+    await assertNoErrorsInCode(r'''
+/// [p]
+foo(int p) => null;
+''');
+
+    assertElement(
+      findNode.simple('p]'),
+      findElement.parameter('p'),
+    );
+  }
+
+  test_beforeFunctionTypeAlias() async {
+    await assertNoErrorsInCode(r'''
+/// [p]
+typedef Foo(int p);
+''');
+
+    assertElement(
+      findNode.simple('p]'),
+      findElement.parameter('p'),
+    );
+  }
+
+  test_beforeGenericTypeAlias() async {
+    await assertNoErrorsInCode(r'''
+/// Can resolve [T], [S], and [p].
+typedef Foo<T> = Function<S>(int p);
+''');
+
+    assertElement(
+      findNode.simple('T]'),
+      findElement.typeParameter('T'),
+    );
+    assertElement(findNode.simple('S]'), findElement.typeParameter('S'));
+    assertElement(
+      findNode.simple('p]'),
+      findElement.parameter('p'),
+    );
+  }
+
+  test_beforeGetter() async {
+    await assertNoErrorsInCode(r'''
+/// [int]
+get g => null;
+''');
+
+    assertElement(findNode.simple('int]'), intElement);
+  }
+
+  test_beforeMethod() async {
+    await assertNoErrorsInCode(r'''
+abstract class A {
+  /// [p1]
+  ma(int p1);
+
+  /// [p2]
+  mb(int p2);
+
+  /// [p3] and [p4]
+  mc(int p3, p4());
+
+  /// [p5]
+  md(int p5, {int p6});
+}
+''');
+
+    assertElement(findNode.simple('p1]'), findElement.parameter('p1'));
+    assertElement(findNode.simple('p2]'), findElement.parameter('p2'));
+    assertElement(findNode.simple('p3]'), findElement.parameter('p3'));
+    assertElement(findNode.simple('p4]'), findElement.parameter('p4'));
+    assertElement(findNode.simple('p5]'), findElement.parameter('p5'));
+  }
+
  test_newKeyword() async {
    await assertErrorsInCode('''
 class A {
@ -273,358 +643,7 @@ main() {}
    );
  }

-  test_prefixedIdentifier_class_constructor_named() async {
-    // TODO(srawlins): Move PrefixedIdentifier tests into their own class, and
-    // improve coverage regarding getter/setter pairs, constructors, operators,
-    // and the 'new' keyword.
-    await assertNoErrorsInCode('''
-class A {
-  A.named();
-}
-
-/// [A.named]
-void f() {}
-''');
-
-    assertElement(findNode.simple('A.named]'), findElement.class_('A'));
-    assertElement(findNode.simple('named]'), findElement.constructor('named'));
-  }
-
-  test_prefixedIdentifier_class_constructor_unnamedViaNew() async {
-    await assertNoErrorsInCode('''
-class A {
-  A();
-}
-
-/// [A.new]
-void f() {}
-''');
-
-    assertElement(findNode.simple('A.new'), findElement.class_('A'));
-    assertElement(findNode.simple('new]'), findElement.unnamedConstructor('A'));
-  }
-
-  test_prefixedIdentifier_class_instanceGetter() async {
-    await assertNoErrorsInCode(r'''
-class A {
-  int get foo => 0;
-}
-
-/// [A.foo]
-void f() {}
-''');
-
-    assertElement(findNode.simple('A.foo'), findElement.class_('A'));
-    assertElement(findNode.simple('foo]'), findElement.getter('foo'));
-  }
-
-  test_prefixedIdentifier_class_instanceMethod() async {
-    await assertNoErrorsInCode(r'''
-class A {
-  void foo() {}
-}
-
-/// [A.foo]
-void f() {}
-''');
-
-    assertElement(findNode.simple('A.foo'), findElement.class_('A'));
-    assertElement(findNode.simple('foo]'), findElement.method('foo'));
-  }
-
-  test_prefixedIdentifier_class_instanceSetter() async {
-    await assertNoErrorsInCode(r'''
-class A {
-  set foo(int _) {}
-}
-
-/// [A.foo]
-void f() {}
-''');
-
-    assertElement(findNode.simple('A.foo'), findElement.class_('A'));
-    assertElement(findNode.simple('foo]'), findElement.setter('foo'));
-  }
-
-  test_prefixedIdentifier_class_staticGetter() async {
-    await assertNoErrorsInCode(r'''
-class A {
-  static int get foo => 0;
-}
-
-/// [A.foo]
-void f() {}
-''');
-
-    assertElement(findNode.simple('A.foo'), findElement.class_('A'));
-    assertElement(findNode.simple('foo]'), findElement.getter('foo'));
-  }
-
-  test_prefixedIdentifier_class_staticMethod() async {
-    await assertNoErrorsInCode(r'''
-class A {
-  static void foo() {}
-}
-
-/// [A.foo]
-void f() {}
-''');
-
-    assertElement(findNode.simple('A.foo'), findElement.class_('A'));
-    assertElement(findNode.simple('foo]'), findElement.method('foo'));
-  }
-
-  test_prefixedIdentifier_class_staticSetter() async {
-    await assertNoErrorsInCode(r'''
-class A {
-  static set foo(int _) {}
-}
-
-/// [A.foo]
-void f() {}
-''');
-
-    assertElement(findNode.simple('A.foo'), findElement.class_('A'));
-    assertElement(findNode.simple('foo]'), findElement.setter('foo'));
-  }
-
-  test_prefixedIdentifier_extension_instanceGetter() async {
-    await assertNoErrorsInCode(r'''
-extension E on int {
-  int get foo => 0;
-}
-
-/// [E.foo]
-void f() {}
-''');
-
-    assertElement(findNode.simple('E.foo'), findElement.extension_('E'));
-    assertElement(findNode.simple('foo]'), findElement.getter('foo'));
-  }
-
-  test_prefixedIdentifier_extension_instanceMethod() async {
-    await assertNoErrorsInCode(r'''
-extension E on int {
-  void foo() {}
-}
-
-/// [E.foo]
-void f() {}
-''');
-
-    assertElement(findNode.simple('E.foo'), findElement.extension_('E'));
-    assertElement(findNode.simple('foo]'), findElement.method('foo'));
-  }
-
-  test_prefixedIdentifier_extension_instanceSetter() async {
-    await assertNoErrorsInCode(r'''
-extension E on int {
-  set foo(int _) {}
-}
-
-/// [E.foo]
-void f() {}
-''');
-
-    assertElement(findNode.simple('E.foo'), findElement.extension_('E'));
-    assertElement(findNode.simple('foo]'), findElement.setter('foo'));
-  }
-
-  test_prefixedIdentifier_extension_staticGetter() async {
-    await assertNoErrorsInCode(r'''
-extension E on int {
-  static int get foo => 0;
-}
-
-/// [E.foo]
-void f() {}
-''');
-
-    assertElement(findNode.simple('E.foo'), findElement.extension_('E'));
-    assertElement(findNode.simple('foo]'), findElement.getter('foo'));
-  }
-
-  test_prefixedIdentifier_extension_staticMethod() async {
-    await assertNoErrorsInCode(r'''
-extension E on int {
-  static void foo() {}
-}
-
-/// [E.foo]
-void f() {}
-''');
-
-    assertElement(findNode.simple('E.foo'), findElement.extension_('E'));
-    assertElement(findNode.simple('foo]'), findElement.method('foo'));
-  }
-
-  test_prefixedIdentifier_extension_staticSetter() async {
-    await assertNoErrorsInCode(r'''
-extension E on int {
-  static set foo(int _) {}
-}
-
-/// [E.foo]
-void f() {}
-''');
-
-    assertElement(findNode.simple('E.foo'), findElement.extension_('E'));
-    assertElement(findNode.simple('foo]'), findElement.setter('foo'));
-  }
-
-  test_simpleIdentifier_beforeClass() async {
-    await assertNoErrorsInCode(r'''
-/// [foo]
-class A {
-  foo() {}
-}
-''');
-
-    assertElement(
-      findNode.simple('foo]'),
-      findElement.method('foo'),
-    );
-  }
-
-  test_simpleIdentifier_beforeConstructor() async {
-    await assertNoErrorsInCode(r'''
-class A {
-  /// [p]
-  A(int p);
-}''');
-
-    assertElement(
-      findNode.simple('p]'),
-      findElement.parameter('p'),
-    );
-  }
-
-  test_simpleIdentifier_beforeEnum() async {
-    await assertNoErrorsInCode(r'''
-/// This is the [Samurai] kind.
-enum Samurai {
-  /// Use [int].
-  WITH_SWORD,
-  /// Like [WITH_SWORD], but only without one.
-  WITHOUT_SWORD
-}''');
-
-    assertElement(
-      findNode.simple('Samurai]'),
-      findElement.enum_('Samurai'),
-    );
-    assertElement(
-      findNode.simple('int]'),
-      intElement,
-    );
-    assertElement(
-      findNode.simple('WITH_SWORD]'),
-      findElement.getter('WITH_SWORD'),
-    );
-  }
-
-  test_simpleIdentifier_beforeFunction_blockBody() async {
-    await assertNoErrorsInCode(r'''
-/// [p]
-foo(int p) {}
-''');
-
-    assertElement(
-      findNode.simple('p]'),
-      findElement.parameter('p'),
-    );
-  }
-
-  test_simpleIdentifier_beforeFunction_expressionBody() async {
-    await assertNoErrorsInCode(r'''
-/// [p]
-foo(int p) => null;
-''');
-
-    assertElement(
-      findNode.simple('p]'),
-      findElement.parameter('p'),
-    );
-  }
-
-  test_simpleIdentifier_beforeFunctionTypeAlias() async {
-    await assertNoErrorsInCode(r'''
-/// [p]
-typedef Foo(int p);
-''');
-
-    assertElement(
-      findNode.simple('p]'),
-      findElement.parameter('p'),
-    );
-  }
-
-  test_simpleIdentifier_beforeGenericTypeAlias() async {
-    await assertNoErrorsInCode(r'''
-/// Can resolve [T], [S], and [p].
-typedef Foo<T> = Function<S>(int p);
-''');
-
-    assertElement(
-      findNode.simple('T]'),
-      findElement.typeParameter('T'),
-    );
-    assertElement(findNode.simple('S]'), findElement.typeParameter('S'));
-    assertElement(
-      findNode.simple('p]'),
-      findElement.parameter('p'),
-    );
-  }
-
-  test_simpleIdentifier_beforeGetter() async {
-    await assertNoErrorsInCode(r'''
-/// [int]
-get g => null;
-''');
-
-    assertElement(findNode.simple('int]'), intElement);
-  }
-
-  test_simpleIdentifier_beforeMethod() async {
-    await assertNoErrorsInCode(r'''
-abstract class A {
-  /// [p1]
-  ma(int p1);
-
-  /// [p2]
-  mb(int p2);
-
-  /// [p3] and [p4]
-  mc(int p3, p4());
-
-  /// [p5]
-  md(int p5, {int p6});
-}
-''');
-
-    assertElement(findNode.simple('p1]'), findElement.parameter('p1'));
-    assertElement(findNode.simple('p2]'), findElement.parameter('p2'));
-    assertElement(findNode.simple('p3]'), findElement.parameter('p3'));
-    assertElement(findNode.simple('p4]'), findElement.parameter('p4'));
-    assertElement(findNode.simple('p5]'), findElement.parameter('p5'));
-  }
-
-  test_simpleIdentifier_extension_conflictingSetterAndGetter() async {
-    await assertNoErrorsInCode('''
-extension E1 on int {
-  int get foo => 0;
-}
-
-/// [foo]
-extension E2 on int {
-  set foo(int value) {}
-}
-''');
-
-    assertElement(findNode.simple('foo]'), findElement.setter('foo'));
-  }
-
-  test_simpleIdentifier_parameter_functionTyped() async {
+  test_parameter_functionTyped() async {
    await assertNoErrorsInCode(r'''
 /// [bar]
 foo(int bar()) {}
@ -636,7 +655,7 @@ foo(int bar()) {}
    );
  }

-  test_simpleIdentifier_setter() async {
+  test_setter() async {
    await assertNoErrorsInCode(r'''
 class A {
  /// [x] in A
@ -655,7 +674,7 @@ class B extends A {
    assertElement(findNode.simple('x] in B'), x);
  }

-  test_simpleIdentifier_unqualifiedReferenceToNonLocalStaticMember() async {
+  test_unqualifiedReferenceToNonLocalStaticMember() async {
    await assertNoErrorsInCode('''
 class A {
  static void foo() {}