feat: add static code diagnostic prefer-commenting-analyzer-ignores (#776)

* feat: add static code diagnostic prefer-commenting-analyzer-ignores

* fix: add new rule to factory

* test: update suppression test

Co-authored-by: Dmitry Krutskikh <dmitry.krutskikh@gmail.com>

Author: incendial

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+* feat: add static code diagnostic [`prefer-commenting-analyzer-ignores`](https://dartcodemetrics.dev/docs/rules/common/prefer-commenting-analyzer-ignores).
 * fix: add check for supertypes for [`avoid-non-null-assertions`](https://dartcodemetrics.dev/docs/rules/common/avoid-non-null-assertion) rule.
 * fix: cover more cases in [`prefer-immediate-return`](https://dartcodemetrics.dev/docs/rules/common/prefer-immediate-return) rule
 * fix: support index expressions for [`no-magic-number`](https://dartcodemetrics.dev/docs/rules/common/no-magic-number) rule.

lib/src/analyzers/lint_analyzer/rules/rules_factory.dart

@@ -33,6 +33,7 @@ import 'rules_list/no_equal_then_else/no_equal_then_else_rule.dart';
 import 'rules_list/no_magic_number/no_magic_number_rule.dart';
 import 'rules_list/no_object_declaration/no_object_declaration_rule.dart';
 import 'rules_list/prefer_async_await/prefer_async_await_rule.dart';
+import 'rules_list/prefer_commenting_analyzer_ignores/prefer_commenting_analyzer_ignores.dart';
 import 'rules_list/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart';
 import 'rules_list/prefer_const_border_radius/prefer_const_border_radius_rule.dart';
 import 'rules_list/prefer_correct_identifier_length/prefer_correct_identifier_length_rule.dart';
@@ -102,6 +103,8 @@ final _implementedRules = <String, Rule Function(Map<String, Object>)>{
   NoMagicNumberRule.ruleId: (config) => NoMagicNumberRule(config),
   NoObjectDeclarationRule.ruleId: (config) => NoObjectDeclarationRule(config),
   PreferAsyncAwaitRule.ruleId: (config) => PreferAsyncAwaitRule(config),
+  PreferCommentingAnalyzerIgnores.ruleId: (config) =>
+      PreferCommentingAnalyzerIgnores(config),
   PreferConditionalExpressionsRule.ruleId: (config) =>
       PreferConditionalExpressionsRule(config),
   PreferConstBorderRadiusRule.ruleId: (config) =>

lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_commenting_analyzer_ignores/prefer_commenting_analyzer_ignores.dart

@@ -0,0 +1,41 @@
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/token.dart';
+import 'package:analyzer/dart/ast/visitor.dart';
+import 'package:analyzer/source/line_info.dart';
+
+import '../../../../../utils/node_utils.dart';
+import '../../../lint_utils.dart';
+import '../../../models/internal_resolved_unit_result.dart';
+import '../../../models/issue.dart';
+import '../../../models/severity.dart';
+import '../../models/common_rule.dart';
+import '../../rule_utils.dart';
+
+part 'visitor.dart';
+
+class PreferCommentingAnalyzerIgnores extends CommonRule {
+  static const String ruleId = 'prefer-commenting-analyzer-ignores';
+
+  static const _warning = 'Prefer commenting analyzer ignores.';
+
+  PreferCommentingAnalyzerIgnores([Map<String, Object> config = const {}])
+      : super(
+          id: ruleId,
+          severity: readSeverity(config, Severity.warning),
+          excludes: readExcludes(config),
+        );
+
+  @override
+  Iterable<Issue> check(InternalResolvedUnitResult source) {
+    final visitor = _Visitor(source.lineInfo)..checkComments(source.unit.root);
+
+    return visitor.comments.map((comment) => createIssue(
+          rule: this,
+          location: nodeLocation(
+            node: comment,
+            source: source,
+          ),
+          message: _warning,
+        ));
+  }
+}

lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_commenting_analyzer_ignores/visitor.dart

@@ -0,0 +1,59 @@
+part of 'prefer_commenting_analyzer_ignores.dart';
+
+class _Visitor extends RecursiveAstVisitor<void> {
+  final LineInfo _lineInfo;
+
+  final _comments = <Token>[];
+
+  Iterable<Token> get comments => _comments;
+
+  _Visitor(this._lineInfo);
+
+  void checkComments(AstNode node) {
+    Token? token = node.beginToken;
+    while (token != null) {
+      Token? commentToken = token.precedingComments;
+      while (commentToken != null) {
+        _visitCommentToken(commentToken);
+        commentToken = commentToken.next;
+      }
+
+      if (token == token.next) {
+        break;
+      }
+
+      token = token.next;
+    }
+  }
+
+  void _visitCommentToken(Token node) {
+    if (node.type == TokenType.SINGLE_LINE_COMMENT) {
+      final comment = node.toString();
+
+      final matches = _getIgnoreMatches(comment);
+      if (_hasNoDescription(matches, comment) && _hasNoPreviousComment(node)) {
+        _comments.add(node);
+      }
+    }
+  }
+
+  bool _hasNoDescription(Iterable<Match> matches, String comment) =>
+      matches.isNotEmpty &&
+      matches.first.groupCount > 0 &&
+      comment.trim().endsWith(matches.first.group(0)?.trim() ?? '');
+
+  bool _hasNoPreviousComment(Token node) {
+    final previous = node.previous;
+
+    return previous == null ||
+        (previous.type != TokenType.SINGLE_LINE_COMMENT ||
+            _lineInfo.getLocation(node.offset).lineNumber - 1 !=
+                _lineInfo.getLocation(previous.offset).lineNumber);
+  }
+
+  Iterable<Match> _getIgnoreMatches(String comment) {
+    final regExp = RegExp(r'ignore:[a-z_,-\s]*([a-z]*(-|_)[a-z]*)+');
+
+    return regExp.allMatches(comment);
+  }
+}

test/resources/suppression_example.dart

@@ -4,7 +4,8 @@ void main() {
   // ignore: rule_id4
   const a = 5; // ignore: RULE_ID5
 
-  // ignore:rule_id6 ,rule_id7
+  // ignore comment
+  // ignore:rule_id6 ,rule_id7, some comment about ignores
   const b = a + 5; // ignore: RULE_ID8, rule_id9, unused_local_variable
 }
 

test/src/analyzers/lint_analyzer/models/suppression_test.dart

@@ -19,14 +19,14 @@ void main() {
     expect(suppression.isSuppressed('rule_id3'), isTrue);
     expect(suppression.isSuppressed('rule_id4'), isFalse);
 
-    expect(suppression.isSuppressedAt('rule_id1', 5), isTrue);
-    expect(suppression.isSuppressedAt('rule_id2', 8), isTrue);
-    expect(suppression.isSuppressedAt('rule_id3', 2), isTrue);
+    expect(suppression.isSuppressedAt('rule_id1', null), isTrue);
+    expect(suppression.isSuppressedAt('rule_id2', null), isTrue);
+    expect(suppression.isSuppressedAt('rule_id3', null), isTrue);
     expect(suppression.isSuppressedAt('rule_id4', 5), isTrue);
     expect(suppression.isSuppressedAt('rule_id5', 5), isTrue);
-    expect(suppression.isSuppressedAt('rule_id6', 8), isTrue);
-    expect(suppression.isSuppressedAt('rule_id7', 8), isTrue);
-    expect(suppression.isSuppressedAt('rule_id8', 8), isTrue);
-    expect(suppression.isSuppressedAt('rule_id9', 8), isTrue);
+    expect(suppression.isSuppressedAt('rule_id6', 9), isTrue);
+    expect(suppression.isSuppressedAt('rule_id7', 9), isTrue);
+    expect(suppression.isSuppressedAt('rule_id8', 9), isTrue);
+    expect(suppression.isSuppressedAt('rule_id9', 9), isTrue);
   });
 }

test/src/analyzers/lint_analyzer/rules/rules_list/prefer_commenting_analyzer_ignores/examples/example.dart

@@ -0,0 +1,18 @@
+// ignore_for_file: some_rule
+void main() {
+  // ignore: deprecated_member_use
+  final map = Map(); // LINT
+
+  // ignore: deprecated_member_use, long-method
+  final set = Set(); // LINT
+
+  // Ignored for some reasons
+  // ignore: deprecated_member_use
+  final list = List();
+
+  // ignore: deprecated_member_use same line ignore
+  final queue = Queue();
+
+  // ignore: deprecated_member_use, long-method multiple same line ignore
+  final linkedList = LinkedList();
+}

test/src/analyzers/lint_analyzer/rules/rules_list/prefer_commenting_analyzer_ignores/prefer_commenting_analyzer_ignores_rule_test.dart

@@ -0,0 +1,41 @@
+import 'package:dart_code_metrics/src/analyzers/lint_analyzer/models/severity.dart';
+import 'package:dart_code_metrics/src/analyzers/lint_analyzer/rules/rules_list/prefer_commenting_analyzer_ignores/prefer_commenting_analyzer_ignores.dart';
+import 'package:test/test.dart';
+
+import '../../../../../helpers/rule_test_helper.dart';
+
+const _examplePath = 'prefer_commenting_analyzer_ignores/examples/example.dart';
+
+void main() {
+  group('PreferCommentingAnalyzerIgnores', () {
+    test('initialization', () async {
+      final unit = await RuleTestHelper.resolveFromFile(_examplePath);
+      final issues = PreferCommentingAnalyzerIgnores().check(unit);
+
+      RuleTestHelper.verifyInitialization(
+        issues: issues,
+        ruleId: 'prefer-commenting-analyzer-ignores',
+        severity: Severity.warning,
+      );
+    });
+
+    test('reports about found issues', () async {
+      final unit = await RuleTestHelper.resolveFromFile(_examplePath);
+      final issues = PreferCommentingAnalyzerIgnores().check(unit);
+
+      RuleTestHelper.verifyIssues(
+        issues: issues,
+        startLines: [3, 6],
+        startColumns: [3, 3],
+        locationTexts: [
+          '// ignore: deprecated_member_use',
+          '// ignore: deprecated_member_use, long-method',
+        ],
+        messages: [
+          'Prefer commenting analyzer ignores.',
+          'Prefer commenting analyzer ignores.',
+        ],
+      );
+    });
+  });
+}

website/docs/rules/common/prefer-commenting-analyzer-ignores.md

@@ -0,0 +1,41 @@
+# Prefer commenting analyzer ignores
+
+## Rule id
+
+prefer-commenting-analyzer-ignores
+
+## Severity {#severity}
+
+Warning
+
+## Description
+
+Warns when `// ignore:` comments are left without any additional description why this ignore is applied.
+
+This rule doesn't trigger on global `ignore_for_file:` comments.
+
+### Example
+
+Bad:
+
+```dart
+// ignore: deprecated_member_use
+final map = Map(); // LINT
+
+// ignore: deprecated_member_use, long-method
+final set = Set(); // LINT
+```
+
+Good:
+
+```dart
+// Ignored for some reasons
+// ignore: deprecated_member_use
+final list = List();
+
+// ignore: deprecated_member_use same line ignore
+final queue = Queue();
+
+// ignore: deprecated_member_use, long-method multiple same line ignore
+final linkedList = LinkedList();
+```

website/docs/rules/overview.md

@@ -119,6 +119,10 @@ Rules configuration is [described here](../getting-started/configuration#configu
 
     Recommends to use async/await syntax to handle Futures result instead of `.then()` invocation.
 
+- [prefer-commenting-analyzer-ignores](./common/prefer-commenting-analyzer-ignores.md)
+
+    Warns when `// ignore:` comments are left without any additional description why this ignore is applied.
+
 - [prefer-conditional-expressions](./common/prefer-conditional-expressions.md)   ![Has auto-fix](https://img.shields.io/badge/-has%20auto--fix-success)
 
     Recommends to use a conditional expression instead of assigning to the same thing or return statement in each branch of an if statement.