Skip to content
This repository has been archived by the owner on Jul 16, 2023. It is now read-only.

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

Merged
merged 5 commits into from Sep 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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).

Expand Down
2 changes: 2 additions & 0 deletions lib/src/analyzers/lint_analyzer/rules/rules_factory.dart
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
@@ -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,
));
}
}
@@ -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;
}
}
@@ -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.",
],
);
});
});
}
@@ -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);
}
}
39 changes: 39 additions & 0 deletions 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';
```
9 changes: 9 additions & 0 deletions website/docs/rules/index.mdx
Expand Up @@ -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"
Expand Down