From 232257313ed04f4007f70c2f7ca09e82c9e5a6c2 Mon Sep 17 00:00:00 2001 From: Dmitry Zhifarsky Date: Fri, 16 Sep 2022 14:00:16 +0400 Subject: [PATCH 1/2] feat: add static code diagnostic avoid-redundant-async --- CHANGELOG.md | 4 + .../lint_analyzer/rules/rules_factory.dart | 2 + .../avoid_redundant_async_rule.dart | 45 +++++++++++ .../avoid_redundant_async/visitor.dart | 74 +++++++++++++++++++ .../avoid_redundant_async_rule_test.dart | 43 +++++++++++ .../examples/example.dart | 25 +++++++ .../rules/common/avoid-redundant-async.mdx | 39 ++++++++++ website/docs/rules/index.mdx | 9 +++ 8 files changed, 241 insertions(+) create mode 100644 lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/avoid_redundant_async_rule.dart create mode 100644 lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/visitor.dart create mode 100644 test/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/avoid_redundant_async_rule_test.dart create mode 100644 test/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/examples/example.dart create mode 100644 website/docs/rules/common/avoid-redundant-async.mdx diff --git a/CHANGELOG.md b/CHANGELOG.md index 13b3abdee5..885a09fead 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +* feat: add static code diagnostic [`avoid-redundant-async`](https://dartcodemetrics.dev/docs/rules/common/avoid-redundant-async). + ## 4.18.3 * fix: fix regression in is! checks for [`avoid-unnecessary-type-assertions`](https://dartcodemetrics.dev/docs/rules/common/avoid-unnecessary-type-assertions). diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart b/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart index d284718e51..fab0db12d9 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart +++ b/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'; @@ -80,6 +81,7 @@ final _implementedRules = )>{ AvoidPassingAsyncWhenSyncExpectedRule.ruleId: AvoidPassingAsyncWhenSyncExpectedRule.new, AvoidPreserveWhitespaceFalseRule.ruleId: AvoidPreserveWhitespaceFalseRule.new, + AvoidRedundantAsyncRule.ruleId: AvoidRedundantAsyncRule.new, AvoidReturningWidgetsRule.ruleId: AvoidReturningWidgetsRule.new, AvoidShrinkWrapInListsRule.ruleId: AvoidShrinkWrapInListsRule.new, AvoidThrowInCatchBlockRule.ruleId: AvoidThrowInCatchBlockRule.new, diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/avoid_redundant_async_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/avoid_redundant_async_rule.dart new file mode 100644 index 0000000000..7b08cb0334 --- /dev/null +++ b/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 config = const {}]) + : super( + id: ruleId, + severity: readSeverity(config, Severity.warning), + excludes: readExcludes(config), + ); + + @override + Iterable 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, + )); + } +} diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/visitor.dart new file mode 100644 index 0000000000..c094b77217 --- /dev/null +++ b/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 { + final _declarations = []; + + Iterable 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 { + 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; + } +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/avoid_redundant_async_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/avoid_redundant_async_rule_test.dart new file mode 100644 index 0000000000..3b7567c0d4 --- /dev/null +++ b/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 fastestBranch(Future left, Future right) async {\n' + ' return Future.any([left, right]);\n' + '}', + "Future anotherAsyncMethod() async => Future.value('value');", + ], + messages: [ + "'async' keyword is redundant, consider removing it.", + "'async' keyword is redundant, consider removing it.", + ], + ); + }); + }); +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/examples/example.dart new file mode 100644 index 0000000000..a52f8b75f2 --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/examples/example.dart @@ -0,0 +1,25 @@ +Future usesAwait(Future later) async { + print(await later); +} + +Future asyncError() async { + throw 'Error!'; +} + +Future asyncValue() async => 'value'; + +// LINT +Future fastestBranch(Future left, Future right) async { + return Future.any([left, right]); +} + +class SomeClass { + void syncMethod() {} + + Future asyncMethod() async => 1; + + // LINT + Future anotherAsyncMethod() async => Future.value('value'); + + Future someAsyncMethod(Future later) => later; +} diff --git a/website/docs/rules/common/avoid-redundant-async.mdx b/website/docs/rules/common/avoid-redundant-async.mdx new file mode 100644 index 0000000000..38ade2103d --- /dev/null +++ b/website/docs/rules/common/avoid-redundant-async.mdx @@ -0,0 +1,39 @@ +import RuleDetails from '@site/src/components/RuleDetails'; + + + +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: + +- + +### Example + +**❌ Bad:** + +```dart +Future afterTwoThings(Future first, Future second) async { + return Future.wait([first, second]); +} +``` + +**✅ Good:** + +```dart +Future usesAwait(Future later) async { + print(await later); +} + +Future asyncError() async { + throw 'Error!'; +} + +Future asyncValue() async => 'value'; +``` diff --git a/website/docs/rules/index.mdx b/website/docs/rules/index.mdx index ab2bfb970f..f8a26a6ffd 100644 --- a/website/docs/rules/index.mdx +++ b/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. + + Checks for redundant async in a method or function body. + + Date: Mon, 19 Sep 2022 20:30:55 +0400 Subject: [PATCH 2/2] fix: correctly handle return void --- .../rules/rules_list/avoid_redundant_async/visitor.dart | 2 +- .../avoid_redundant_async/examples/example.dart | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/visitor.dart index c094b77217..9aa582df3e 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/visitor.dart @@ -53,7 +53,7 @@ class _AsyncVisitor extends RecursiveAstVisitor { final type = node.expression?.staticType; - if (type != null && !type.isDartAsyncFuture) { + if (type == null || !type.isDartAsyncFuture) { hasValidAsync = true; } } diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/examples/example.dart index a52f8b75f2..4029956092 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_redundant_async/examples/example.dart @@ -22,4 +22,12 @@ class SomeClass { Future anotherAsyncMethod() async => Future.value('value'); Future someAsyncMethod(Future later) => later; + + Future report(Iterable records) async { + if (records.isEmpty) { + return; + } + + print(records); + } }