feat: add static code diagnostic avoid-redundant-async (#1002)

* feat: add static code diagnostic avoid-redundant-async

* fix: correctly handle return void

Co-authored-by: Dmitry Krutskikh <[email protected]>

Author: incendial

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+* feat: add static code diagnostic [`avoid-redundant-async`](https://dartcodemetrics.dev/docs/rules/common/avoid-redundant-async).
 * feat: add static code diagnostic [`prefer-correct-test-file-name`](https://dartcodemetrics.dev/docs/rules/common/prefer-correct-test-file-name).
 * feat: add static code diagnostic [`prefer-iterable-of`](https://dartcodemetrics.dev/docs/rules/common/prefer-iterable-of).
 

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

@@ -15,6 +15,7 @@ import 'rules_list/avoid_non_ascii_symbols/avoid_non_ascii_symbols_rule.dart';
 import 'rules_list/avoid_non_null_assertion/avoid_non_null_assertion_rule.dart';
 import 'rules_list/avoid_passing_async_when_sync_expected/avoid_passing_async_when_sync_expected_rule.dart';
 import 'rules_list/avoid_preserve_whitespace_false/avoid_preserve_whitespace_false_rule.dart';
+import 'rules_list/avoid_redundant_async/avoid_redundant_async_rule.dart';
 import 'rules_list/avoid_returning_widgets/avoid_returning_widgets_rule.dart';
 import 'rules_list/avoid_shrink_wrap_in_lists/avoid_shrink_wrap_in_lists_rule.dart';
 import 'rules_list/avoid_throw_in_catch_block/avoid_throw_in_catch_block_rule.dart';
@@ -82,6 +83,7 @@ final _implementedRules = <String, Rule Function(Map<String, Object>)>{
   AvoidPassingAsyncWhenSyncExpectedRule.ruleId:
       AvoidPassingAsyncWhenSyncExpectedRule.new,
   AvoidPreserveWhitespaceFalseRule.ruleId: AvoidPreserveWhitespaceFalseRule.new,
+  AvoidRedundantAsyncRule.ruleId: AvoidRedundantAsyncRule.new,
   AvoidReturningWidgetsRule.ruleId: AvoidReturningWidgetsRule.new,
   AvoidShrinkWrapInListsRule.ruleId: AvoidShrinkWrapInListsRule.new,
   AvoidThrowInCatchBlockRule.ruleId: AvoidThrowInCatchBlockRule.new,

lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/avoid_redundant_async_rule.dart

@@ -0,0 +1,45 @@
+// 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 AvoidRedundantAsyncRule extends CommonRule {
+  static const String ruleId = 'avoid-redundant-async';
+
+  static const _warningMessage =
+      "'async' keyword is redundant, consider removing it.";
+
+  AvoidRedundantAsyncRule([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.declarations.map((declaration) => createIssue(
+          rule: this,
+          location: nodeLocation(
+            node: declaration,
+            source: source,
+          ),
+          message: _warningMessage,
+        ));
+  }
+}

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

@@ -0,0 +1,74 @@
+part of 'avoid_redundant_async_rule.dart';
+
+class _Visitor extends RecursiveAstVisitor<void> {
+  final _declarations = <Declaration>[];
+
+  Iterable<Declaration> get declarations => _declarations;
+
+  @override
+  void visitMethodDeclaration(MethodDeclaration node) {
+    super.visitMethodDeclaration(node);
+
+    if (_hasRedundantAsync(node.body)) {
+      _declarations.add(node);
+    }
+  }
+
+  @override
+  void visitFunctionDeclaration(FunctionDeclaration node) {
+    super.visitFunctionDeclaration(node);
+
+    if (_hasRedundantAsync(node.functionExpression.body)) {
+      _declarations.add(node);
+    }
+  }
+
+  bool _hasRedundantAsync(FunctionBody body) {
+    final hasAsyncKeyword = body.keyword?.type == Keyword.ASYNC;
+    if (!hasAsyncKeyword) {
+      return false;
+    }
+
+    if (body is ExpressionFunctionBody) {
+      final type = body.expression.staticType;
+
+      if (type != null && !type.isDartAsyncFuture) {
+        return false;
+      }
+    }
+
+    final asyncVisitor = _AsyncVisitor();
+    body.parent?.visitChildren(asyncVisitor);
+
+    return !asyncVisitor.hasValidAsync;
+  }
+}
+
+class _AsyncVisitor extends RecursiveAstVisitor<void> {
+  bool hasValidAsync = false;
+
+  @override
+  void visitReturnStatement(ReturnStatement node) {
+    super.visitReturnStatement(node);
+
+    final type = node.expression?.staticType;
+
+    if (type == null || !type.isDartAsyncFuture) {
+      hasValidAsync = true;
+    }
+  }
+
+  @override
+  void visitThrowExpression(ThrowExpression node) {
+    super.visitThrowExpression(node);
+
+    hasValidAsync = true;
+  }
+
+  @override
+  void visitAwaitExpression(AwaitExpression node) {
+    super.visitAwaitExpression(node);
+
+    hasValidAsync = true;
+  }
+}

test/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/avoid_redundant_async_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_redundant_async/avoid_redundant_async_rule.dart';
+import 'package:test/test.dart';
+
+import '../../../../../helpers/rule_test_helper.dart';
+
+const _examplePath = 'avoid_redundant_async/examples/example.dart';
+
+void main() {
+  group('AvoidRedundantAsyncRule', () {
+    test('initialization', () async {
+      final unit = await RuleTestHelper.resolveFromFile(_examplePath);
+      final issues = AvoidRedundantAsyncRule().check(unit);
+
+      RuleTestHelper.verifyInitialization(
+        issues: issues,
+        ruleId: 'avoid-redundant-async',
+        severity: Severity.warning,
+      );
+    });
+
+    test('reports about found issues with the default config', () async {
+      final unit = await RuleTestHelper.resolveFromFile(_examplePath);
+      final issues = AvoidRedundantAsyncRule().check(unit);
+
+      RuleTestHelper.verifyIssues(
+        issues: issues,
+        startLines: [12, 22],
+        startColumns: [1, 3],
+        locationTexts: [
+          'Future<int> fastestBranch(Future<int> left, Future<int> right) async {\n'
+              '  return Future.any([left, right]);\n'
+              '}',
+          "Future<String> anotherAsyncMethod() async => Future.value('value');",
+        ],
+        messages: [
+          "'async' keyword is redundant, consider removing it.",
+          "'async' keyword is redundant, consider removing it.",
+        ],
+      );
+    });
+  });
+}

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

@@ -0,0 +1,33 @@
+Future<void> usesAwait(Future<String> later) async {
+  print(await later);
+}
+
+Future<void> asyncError() async {
+  throw 'Error!';
+}
+
+Future<void> asyncValue() async => 'value';
+
+// LINT
+Future<int> fastestBranch(Future<int> left, Future<int> right) async {
+  return Future.any([left, right]);
+}
+
+class SomeClass {
+  void syncMethod() {}
+
+  Future<int> asyncMethod() async => 1;
+
+  // LINT
+  Future<String> anotherAsyncMethod() async => Future.value('value');
+
+  Future<String> someAsyncMethod(Future<String> later) => later;
+
+  Future<void> report(Iterable<String> records) async {
+    if (records.isEmpty) {
+      return;
+    }
+
+    print(records);
+  }
+}

website/docs/rules/common/avoid-redundant-async.mdx

@@ -0,0 +1,39 @@
+import RuleDetails from '@site/src/components/RuleDetails';
+
+<RuleDetails version="4.19.0" severity="warning" />
+
+Checks for redundant `async` in a method or function body.
+
+Cases where `async` is useful include:
+
+- The function body has `await`.
+- An error is returnted asynchronously. `async` and then `throw` is shorter than return `Future.error(...)`.
+- A value is returned and it will be impliclty wrapped in a future. `async` is shorter than `Future.value(...)`.
+
+Additional resoureces:
+
+- <https://dart.dev/guides/language/effective-dart/usage#dont-use-async-when-it-has-no-useful-effect>
+
+### Example
+
+**❌ Bad:**
+
+```dart
+Future<void> afterTwoThings(Future<void> first, Future<void> second) async {
+  return Future.wait([first, second]);
+}
+```
+
+**✅ Good:**
+
+```dart
+Future<void> usesAwait(Future<String> later) async {
+  print(await later);
+}
+
+Future<void> asyncError() async {
+  throw 'Error!';
+}
+
+Future<void> asyncValue() async => 'value';
+```

website/docs/rules/index.mdx

@@ -132,6 +132,15 @@ Rules are grouped by category to help you understand their purpose. Each rule ha
   function is expected.
 </RuleEntry>
 
+<RuleEntry
+  name="avoid-redundant-async"
+  type="common"
+  severity="warning"
+  version="4.19.0"
+>
+  Checks for redundant <code>async</code> in a method or function body.
+</RuleEntry>
+
 <RuleEntry
   name="avoid-throw-in-catch-block"
   type="common"