linter: public_member_api_docs: dont report internal top-level variable

Fixes #60617

This includes a bit of clean-up of the other cases where we are concerned with
`@internal`.

Change-Id: I506725c2280ab90fd5e1014eb8478497de8a0480
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/427340
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>

Author: srawlins

pkg/linter/lib/src/extensions.dart

@@ -77,6 +77,14 @@ extension AstNodeExtension on AstNode {
   }
 
   bool get isInternal {
+    var self = this;
+    if (self is VariableDeclaration) {
+      var element = self.declaredFragment?.element;
+      if (element is TopLevelVariableElement) {
+        return element.metadata2.hasInternal;
+      }
+    }
+
     var parent = thisOrAncestorOfType<CompilationUnitMember>();
     if (parent == null) return false;
 

pkg/linter/lib/src/rules/public_member_api_docs.dart

@@ -58,6 +58,8 @@ class _Visitor extends SimpleAstVisitor<void> {
   _Visitor(this.rule, this.context);
 
   bool check(Declaration node) {
+    if (node.isInternal) return false;
+
     if (node.documentationComment == null && !isOverridingMember(node)) {
       var errorNode = getNodeToAnnotate(node);
       rule.reportAtOffset(errorNode.offset, errorNode.length);
@@ -67,8 +69,6 @@ class _Visitor extends SimpleAstVisitor<void> {
   }
 
   void checkMethods(List<ClassMember> members) {
-    // Check methods
-
     var getters = <String, MethodDeclaration>{};
     var setters = <MethodDeclaration>[];
 
@@ -117,8 +117,7 @@ class _Visitor extends SimpleAstVisitor<void> {
 
   @override
   void visitClassDeclaration(ClassDeclaration node) {
-    var element = node.declaredFragment?.element;
-    if (element == null || element.metadata2.hasInternal) return;
+    if (node.declaredFragment?.element == null) return;
     _visitMembers(node, node.name, node.members);
   }
 
@@ -134,8 +133,6 @@ class _Visitor extends SimpleAstVisitor<void> {
     var getters = <String, FunctionDeclaration>{};
     var setters = <FunctionDeclaration>[];
 
-    // Check functions.
-
     // Non-getters/setters.
     var functions = <FunctionDeclaration>[];
 
@@ -172,11 +169,7 @@ class _Visitor extends SimpleAstVisitor<void> {
     }
 
     // Check remaining functions.
-    for (var function in functions) {
-      if (!function.isEffectivelyPrivate) {
-        check(function);
-      }
-    }
+    functions.forEach(check);
 
     super.visitCompilationUnit(node);
   }
@@ -193,9 +186,6 @@ class _Visitor extends SimpleAstVisitor<void> {
 
   @override
   void visitEnumConstantDeclaration(EnumConstantDeclaration node) {
-    // TODO(pq): update this to be called from the parent (like with visitMembers)
-    if (node.isInternal) return;
-
     if (!node.inPrivateMember && !node.name.isPrivate) {
       check(node);
     }
@@ -204,7 +194,6 @@ class _Visitor extends SimpleAstVisitor<void> {
   @override
   void visitEnumDeclaration(EnumDeclaration node) {
     if (node.name.isPrivate) return;
-    if (node.isInternal) return;
 
     check(node);
     checkMethods(node.members);
@@ -221,8 +210,7 @@ class _Visitor extends SimpleAstVisitor<void> {
 
   @override
   void visitExtensionTypeDeclaration(ExtensionTypeDeclaration node) {
-    var element = node.declaredFragment?.element;
-    if (element == null || element.metadata2.hasInternal) return;
+    if (node.declaredFragment?.element == null) return;
     _visitMembers(node, node.name, node.members);
   }
 
@@ -256,7 +244,6 @@ class _Visitor extends SimpleAstVisitor<void> {
 
   @override
   void visitMixinDeclaration(MixinDeclaration node) {
-    if (node.isInternal) return;
     _visitMembers(node, node.name, node.members);
   }
 
@@ -271,6 +258,7 @@ class _Visitor extends SimpleAstVisitor<void> {
 
   void _visitMembers(Declaration node, Token name, List<ClassMember> members) {
     if (name.isPrivate) return;
+    if (node.isInternal) return;
 
     check(node);
     checkMethods(members);

pkg/linter/test/rules/public_member_api_docs_test.dart

@@ -346,6 +346,16 @@ mixin M {
     );
   }
 
+  /// https://github.com/dart-lang/linter/issues/5030
+  test_internalTopLevelVariable() async {
+    await assertNoDiagnostics(r'''
+import 'package:meta/meta.dart';
+
+@internal
+var f = 1;
+''');
+  }
+
   test_mixin_method() async {
     await assertDiagnostics(
       r'''
@@ -393,7 +403,7 @@ final class A extends S {}
 ''',
       [
         lint(13, 1),
-        // No lint on `S()` declaration
+        // No lint on `S()` constructor declaration.
         lint(39, 1),
       ],
     );