[linter] Deprecate package_api_docs lint

The lint hasn't been functional for a long time. Rather than try to fix it and start introducing a lot of warnings on packages that include it, let's deprecate it and prepare for removal. Keeping it around in its current state results in the analyzer not meeting the expectations of developers that include the rule, potentially even causing misconceptions that an element isn't public when it is. A replacement, with a new name, can be introduced in the future if there is the desire and time to implement it. Bug: dart-lang/linter#5107 Change-Id: Ied113e4c98be2253cb938277cfd6ea1c96bcac4a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/389585 Reviewed-by: Phil Quitslund <[email protected]> Auto-Submit: Parker Lougheed <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
dart-lang · Oct 16, 2024 · e032779 · e032779
1 parent 1e624b8
commit e032779
Show file tree

Hide file tree

Showing 9 changed files with 17 additions and 152 deletions.
diff --git a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml
@@ -2207,8 +2207,6 @@ LintCode.only_throw_errors:
   status: noFix
 LintCode.overridden_fields:
   status: noFix
-LintCode.package_api_docs:
-  status: noFix
 LintCode.package_names:
   status: noFix
   notes: |-

diff --git a/pkg/linter/CHANGELOG.md b/pkg/linter/CHANGELOG.md
@@ -1,4 +1,8 @@
-# 3.6.0-wip
+# 3.7.0-wip
+
+- deprecated lint: `package_api_docs`
+
+# 3.6.0
 
 - new lint: `use_truncating_division`
 - new _(experimental)_ lint: `omit_obvious_local_variable_types`

diff --git a/pkg/linter/analyzer_use_new_elements.txt b/pkg/linter/analyzer_use_new_elements.txt
@@ -99,7 +99,6 @@ lib/src/rules/null_closures.dart
 lib/src/rules/omit_obvious_local_variable_types.dart
 lib/src/rules/one_member_abstracts.dart
 lib/src/rules/only_throw_errors.dart
-lib/src/rules/package_api_docs.dart
 lib/src/rules/package_prefixed_library_names.dart
 lib/src/rules/prefer_adjacent_string_concatenation.dart
 lib/src/rules/prefer_asserts_with_message.dart
@@ -324,7 +323,6 @@ test/rules/omit_obvious_local_variable_types_test.dart
 test/rules/one_member_abstracts_test.dart
 test/rules/only_throw_errors_test.dart
 test/rules/overridden_fields_test.dart
-test/rules/package_api_docs_test.dart
 test/rules/package_names_test.dart
 test/rules/package_prefixed_library_names_test.dart
 test/rules/parameter_assignments_test.dart

diff --git a/pkg/linter/example/all.yaml b/pkg/linter/example/all.yaml
@@ -114,7 +114,6 @@ linter:
     - one_member_abstracts
     - only_throw_errors
     - overridden_fields
-    - package_api_docs
     - package_names
     - package_prefixed_library_names
     - parameter_assignments

diff --git a/pkg/linter/lib/src/rules/package_api_docs.dart b/pkg/linter/lib/src/rules/package_api_docs.dart
@@ -2,8 +2,7 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-import 'package:analyzer/dart/ast/ast.dart';
-import 'package:analyzer/dart/ast/visitor.dart';
+import 'package:pub_semver/pub_semver.dart';
 
 import '../analyzer.dart';
 
@@ -14,76 +13,9 @@ class PackageApiDocs extends LintRule {
       : super(
           name: LintNames.package_api_docs,
           description: _desc,
+          state: State.deprecated(since: Version(3, 7, 0)),
         );
 
   @override
-  LintCode get lintCode => LinterLintCode.package_api_docs;
-
-  @override
-  void registerNodeProcessors(
-      NodeLintRegistry registry, LinterContext context) {
-    var visitor = _Visitor(this);
-    registry.addConstructorDeclaration(this, visitor);
-    registry.addFieldDeclaration(this, visitor);
-    registry.addMethodDeclaration(this, visitor);
-    registry.addClassDeclaration(this, visitor);
-    registry.addEnumDeclaration(this, visitor);
-    registry.addFunctionDeclaration(this, visitor);
-    registry.addTopLevelVariableDeclaration(this, visitor);
-    registry.addClassTypeAlias(this, visitor);
-    registry.addFunctionTypeAlias(this, visitor);
-  }
-}
-
-class _Visitor extends GeneralizingAstVisitor<void> {
-  final PackageApiDocs rule;
-
-  _Visitor(this.rule);
-
-  // ignore: prefer_expression_function_bodies
-  void check(Declaration node) {
-    // See: https://github.com/dart-lang/linter/issues/3395
-    // (`DartProject` removal).
-    return;
-
-    // // If no project info is set, bail early.
-    // // https://github.com/dart-lang/linter/issues/154
-    // var currentProject = rule.project;
-    // if (currentProject == null) {
-    //   return;
-    // }
-    //
-    // var declaredElement = node.declaredElement;
-    // if (declaredElement != null && currentProject.isApi(declaredElement)) {
-    //   if (node.documentationComment == null) {
-    //     rule.reportLint(getNodeToAnnotate(node));
-    //   }
-    // }
-  }
-
-  ///  classMember ::=
-  ///    [ConstructorDeclaration]
-  ///  | [FieldDeclaration]
-  ///  | [MethodDeclaration]
-  @override
-  void visitClassMember(ClassMember node) {
-    check(node);
-  }
-
-  ///  compilationUnitMember ::=
-  ///    [ClassDeclaration]
-  ///  | [EnumDeclaration]
-  ///  | [FunctionDeclaration]
-  ///  | [TopLevelVariableDeclaration]
-  ///  | [ClassTypeAlias]
-  ///  | [FunctionTypeAlias]
-  @override
-  void visitCompilationUnitMember(CompilationUnitMember node) {
-    check(node);
-  }
-
-  @override
-  void visitNode(AstNode node) {
-    // Don't visit children
-  }
+  LintCode get lintCode => LinterLintCode.removed_lint;
 }
diff --git a/pkg/linter/messages.yaml b/pkg/linter/messages.yaml
@@ -7140,6 +7140,10 @@ LintCode:
     categories: [effectiveDart, publicInterface]
     hasPublishedDocs: false
     deprecatedDetails: |-
+      **NOTE:** This lint is deprecated because it is has not
+      been fully functional since at least Dart 2.0.
+      Remove all inclusions of this lint from your analysis options.
+
       **DO** provide doc comments for all public APIs.
 
       As described in the [pub package layout doc](https://dart.dev/tools/pub/package-layout#implementation-files),

diff --git a/pkg/linter/test/rules/all.dart b/pkg/linter/test/rules/all.dart
@@ -165,7 +165,6 @@ import 'omit_obvious_local_variable_types_test.dart'
 import 'one_member_abstracts_test.dart' as one_member_abstracts;
 import 'only_throw_errors_test.dart' as only_throw_errors;
 import 'overridden_fields_test.dart' as overridden_fields;
-import 'package_api_docs_test.dart' as package_api_docs;
 import 'package_names_test.dart' as package_names;
 import 'package_prefixed_library_names_test.dart'
     as package_prefixed_library_names;
@@ -434,7 +433,6 @@ void main() {
   one_member_abstracts.main();
   only_throw_errors.main();
   overridden_fields.main();
-  package_api_docs.main();
   package_names.main();
   package_prefixed_library_names.main();
   parameter_assignments.main();

diff --git a/pkg/linter/test/rules/package_api_docs_test.dart b/pkg/linter/test/rules/package_api_docs_test.dart
diff --git a/pkg/linter/tool/machine/rules.json b/pkg/linter/tool/machine/rules.json
@@ -373,11 +373,11 @@
     "categories": [
       "style"
     ],
-    "state": "stable",
+    "state": "removed",
     "incompatible": [],
     "sets": [],
-    "fixStatus": "hasFix",
-    "details": "**DON'T** check for `null` in custom `==` operators.\n\nAs `null` is a special value, no instance of any class (other than `Null`) can\nbe equivalent to it.  Thus, it is redundant to check whether the other instance\nis `null`.\n\n**BAD:**\n```dart\nclass Person {\n  final String? name;\n\n  @override\n  operator ==(Object? other) =>\n      other != null && other is Person && name == other.name;\n}\n```\n\n**GOOD:**\n```dart\nclass Person {\n  final String? name;\n\n  @override\n  operator ==(Object? other) => other is Person && name == other.name;\n}\n```",
+    "fixStatus": "noFix",
+    "details": "**DON'T** check for `null` in custom `==` operators.\n\nAs `null` is a special value, no instance of any class (other than `Null`) can\nbe equivalent to it.  Thus, it is redundant to check whether the other instance\nis `null`.\n\n**BAD:**\n```dart\nclass Person {\n  final String? name;\n\n  @override\n  operator ==(Object? other) =>\n      other != null && other is Person && name == other.name;\n}\n```\n\n**GOOD:**\n```dart\nclass Person {\n  final String? name;\n\n  @override\n  operator ==(Object? other) => other is Person && name == other.name;\n}\n```\n\nThis rule has been removed.",
     "sinceDartSdk": "2.0"
   },
   {
@@ -1613,11 +1613,11 @@
       "effectiveDart",
       "publicInterface"
     ],
-    "state": "stable",
+    "state": "deprecated",
     "incompatible": [],
     "sets": [],
     "fixStatus": "noFix",
-    "details": "**DO** provide doc comments for all public APIs.\n\nAs described in the [pub package layout doc](https://dart.dev/tools/pub/package-layout#implementation-files),\npublic APIs consist in everything in your package's `lib` folder, minus\nimplementation files in `lib/src`, adding elements explicitly exported with an\n`export` directive.\n\nFor example, given `lib/foo.dart`:\n```dart\nexport 'src/bar.dart' show Bar;\nexport 'src/baz.dart';\n\nclass Foo { }\n\nclass _Foo { }\n```\nits API includes:\n\n* `Foo` (but not `_Foo`)\n* `Bar` (exported) and\n* all *public* elements in `src/baz.dart`\n\nAll public API members should be documented with `///` doc-style comments.\n\n**BAD:**\n```dart\nclass Bar {\n  void bar();\n}\n```\n\n**GOOD:**\n```dart\n/// A Foo.\nabstract class Foo {\n  /// Start foo-ing.\n  void start() => _start();\n\n  _start();\n}\n```\n\nAdvice for writing good doc comments can be found in the\n[Doc Writing Guidelines](https://dart.dev/effective-dart/documentation).",
+    "details": "**NOTE:** This lint is deprecated because it is has not\nbeen fully functional since at least Dart 2.0.\nRemove all inclusions of this lint from your analysis options.\n\n**DO** provide doc comments for all public APIs.\n\nAs described in the [pub package layout doc](https://dart.dev/tools/pub/package-layout#implementation-files),\npublic APIs consist in everything in your package's `lib` folder, minus\nimplementation files in `lib/src`, adding elements explicitly exported with an\n`export` directive.\n\nFor example, given `lib/foo.dart`:\n```dart\nexport 'src/bar.dart' show Bar;\nexport 'src/baz.dart';\n\nclass Foo { }\n\nclass _Foo { }\n```\nits API includes:\n\n* `Foo` (but not `_Foo`)\n* `Bar` (exported) and\n* all *public* elements in `src/baz.dart`\n\nAll public API members should be documented with `///` doc-style comments.\n\n**BAD:**\n```dart\nclass Bar {\n  void bar();\n}\n```\n\n**GOOD:**\n```dart\n/// A Foo.\nabstract class Foo {\n  /// Start foo-ing.\n  void start() => _start();\n\n  _start();\n}\n```\n\nAdvice for writing good doc comments can be found in the\n[Doc Writing Guidelines](https://dart.dev/effective-dart/documentation).",
     "sinceDartSdk": "2.0"
   },
   {