feat: add avoid-cascade-after-if-null rule (#1039)

Author: incendial

.github/workflows/package_analyze.yaml

@@ -113,7 +113,7 @@ jobs:
           bash <(curl -s https://codecov.io/bash) -f coverage/coverage.lcov
 
       - name: Publish test results
-        uses: dorny/test-reporter@v1.5.0
+        uses: dorny/test-reporter@v1.6.0
         if: success() || failure() # run this step even if previous step failed
         with:
           name: Tests result

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased
+
+* feat: add static code diagnostic [`avoid-cascade-after-if-null`](https://dartcodemetrics.dev/docs/rules/common/avoid-cascade-after-if-null).
+
 ## 4.21.2
 
 * fix: correctly handle FutureOr functions for [`avoid-passing-async-when-sync-expected`](https://dartcodemetrics.dev/docs/rules/common/avoid-passing-async-when-sync-expected).

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

@@ -2,6 +2,7 @@ import 'models/rule.dart';
 import 'rules_list/always_remove_listener/always_remove_listener_rule.dart';
 import 'rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart';
 import 'rules_list/avoid_border_all/avoid_border_all_rule.dart';
+import 'rules_list/avoid_cascade_after_if_null/avoid_cascade_after_if_null_rule.dart';
 import 'rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart';
 import 'rules_list/avoid_duplicate_exports/avoid_duplicate_exports_rule.dart';
 import 'rules_list/avoid_dynamic/avoid_dynamic_rule.dart';
@@ -70,6 +71,7 @@ final _implementedRules = <String, Rule Function(Map<String, Object>)>{
   AlwaysRemoveListenerRule.ruleId: AlwaysRemoveListenerRule.new,
   AvoidBannedImportsRule.ruleId: AvoidBannedImportsRule.new,
   AvoidBorderAllRule.ruleId: AvoidBorderAllRule.new,
+  AvoidCascadeAfterIfNullRule.ruleId: AvoidCascadeAfterIfNullRule.new,
   AvoidCollectionMethodsWithUnrelatedTypesRule.ruleId:
       AvoidCollectionMethodsWithUnrelatedTypesRule.new,
   AvoidDuplicateExportsRule.ruleId: AvoidDuplicateExportsRule.new,

lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_cascade_after_if_null/avoid_cascade_after_if_null_rule.dart

@@ -0,0 +1,47 @@
+// ignore_for_file: public_member_api_docs
+
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/token.dart';
+import 'package:analyzer/dart/ast/visitor.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 AvoidCascadeAfterIfNullRule extends CommonRule {
+  static const String ruleId = 'avoid-cascade-after-if-null';
+
+  static const _warning =
+      'Avoid using cascade after ?? without precedence. It might lead to unexpected errors.';
+
+  AvoidCascadeAfterIfNullRule([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.unit.visitChildren(visitor);
+
+    return visitor.expressions
+        .map((expression) => createIssue(
+              rule: this,
+              location: nodeLocation(
+                node: expression,
+                source: source,
+              ),
+              message: _warning,
+            ))
+        .toList(growable: false);
+  }
+}

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

@@ -0,0 +1,21 @@
+part of 'avoid_cascade_after_if_null_rule.dart';
+
+class _Visitor extends RecursiveAstVisitor<void> {
+  final _expressions = <Expression>[];
+
+  Iterable<Expression> get expressions => _expressions;
+
+  @override
+  void visitBinaryExpression(BinaryExpression node) {
+    super.visitBinaryExpression(node);
+
+    if (node.operator.type != TokenType.QUESTION_QUESTION) {
+      return;
+    }
+
+    final parent = node.parent;
+    if (parent is CascadeExpression) {
+      _expressions.add(parent);
+    }
+  }
+}

test/src/analyzers/lint_analyzer/rules/rules_list/avoid_cascade_after_if_null/avoid_cascade_after_if_null_rule_test.dart

@@ -0,0 +1,43 @@
+import 'package:dart_code_metrics/src/analyzers/lint_analyzer/models/severity.dart';
+import 'package:dart_code_metrics/src/analyzers/lint_analyzer/rules/rules_list/avoid_cascade_after_if_null/avoid_cascade_after_if_null_rule.dart';
+import 'package:test/test.dart';
+
+import '../../../../../helpers/rule_test_helper.dart';
+
+const _examplePath = 'avoid_cascade_after_if_null/examples/example.dart';
+
+void main() {
+  group('AvoidCascadeAfterIfNullRule', () {
+    test('initialization', () async {
+      final unit = await RuleTestHelper.resolveFromFile(_examplePath);
+      final issues = AvoidCascadeAfterIfNullRule().check(unit);
+
+      RuleTestHelper.verifyInitialization(
+        issues: issues,
+        ruleId: 'avoid-cascade-after-if-null',
+        severity: Severity.warning,
+      );
+    });
+
+    test('reports about found issues', () async {
+      final unit = await RuleTestHelper.resolveFromFile(_examplePath);
+      final issues = AvoidCascadeAfterIfNullRule().check(unit);
+
+      RuleTestHelper.verifyIssues(
+        issues: issues,
+        startLines: [9, 16],
+        startColumns: [16, 15],
+        locationTexts: [
+          'cow ?? Cow()\n'
+              '          ..moo()',
+          'nullableCow ?? Cow()\n'
+              '    ..moo()',
+        ],
+        messages: [
+          'Avoid using cascade after ?? without precedence. It might lead to unexpected errors.',
+          'Avoid using cascade after ?? without precedence. It might lead to unexpected errors.',
+        ],
+      );
+    });
+  });
+}

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

@@ -0,0 +1,20 @@
+class Cow {
+  void moo() {}
+}
+
+class Ranch {
+  final Cow? _cow;
+
+  Ranch([Cow? cow])
+      : _cow = cow ?? Cow()
+          ..moo(); // LINT
+}
+
+void main() {
+  final Cow? nullableCow;
+
+  final cow = nullableCow ?? Cow()
+    ..moo(); // LINT
+  final cow = (nullableCow ?? Cow())..moo();
+  final cow = nullableCow ?? (Cow()..moo());
+}

website/docs/rules/common/avoid-cascade-after-if-null.mdx

@@ -0,0 +1,48 @@
+import RuleDetails from '@site/src/components/RuleDetails';
+
+<RuleDetails version="5.0.0" severity="warning" />
+
+Warns when a cascade expression is used after if null (??) binary expression
+without parentheses.
+
+Not adding parentheses might lead to unexpected results since cascade will be executed **after** if null expression.
+
+Additional resources:
+
+- <https://github.com/dart-lang/sdk/issues/45667>
+
+### Example {#example}
+
+**❌ Bad:**
+
+```dart
+class Cow {
+  void moo() {}
+}
+
+class Ranch {
+  final Cow? _cow;
+
+  Ranch([Cow? cow])
+      : _cow = cow ?? Cow()
+          ..moo(); // LINT
+}
+
+void main() {
+  final Cow? nullableCow;
+
+  final cow = nullableCow ?? Cow()
+    ..moo(); // LINT
+}
+```
+
+**✅ Good:**
+
+```dart
+void main() {
+  final Cow? nullableCow;
+
+  final cow = (nullableCow ?? Cow())..moo();
+  final cow = nullableCow ?? (Cow()..moo());
+}
+```

website/docs/rules/index.mdx

@@ -26,6 +26,16 @@ Rules are grouped by category to help you understand their purpose. Each rule ha
   Configure some imports that you want to ban.
 </RuleEntry>
 
+<RuleEntry
+  name="avoid-cascade-after-if-null"
+  type="common"
+  severity="warning"
+  version="5.0.0"
+>
+  Warns when a cascade expression is used after if null (??) binary expression
+  without parentheses.
+</RuleEntry>
+
 <RuleEntry
   name="avoid-collection-methods-with-unrelated-types"
   type="common"