From ef802a82512e6723b14fa12943c8fb2dcc0ab4b1 Mon Sep 17 00:00:00 2001 From: Roman Petrov Date: Fri, 11 Mar 2022 15:09:25 +0300 Subject: [PATCH] feat: add prefer-immediate-return rule --- CHANGELOG.md | 1 + .../lint_analyzer/rules/rules_factory.dart | 3 + .../prefer_immediate_return_rule.dart | 53 +++++++++++++ .../prefer_immediate_return/visitor.dart | 50 ++++++++++++ .../examples/example.dart | 43 +++++++++++ .../prefer_immediate_return_rule_test.dart | 76 +++++++++++++++++++ .../rules/common/prefer-immediate-return.md | 41 ++++++++++ website/docs/rules/overview.md | 4 + 8 files changed, 271 insertions(+) create mode 100644 lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_immediate_return/prefer_immediate_return_rule.dart create mode 100644 lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_immediate_return/visitor.dart create mode 100644 test/src/analyzers/lint_analyzer/rules/rules_list/prefer_immediate_return/examples/example.dart create mode 100644 test/src/analyzers/lint_analyzer/rules/rules_list/prefer_immediate_return/prefer_immediate_return_rule_test.dart create mode 100644 website/docs/rules/common/prefer-immediate-return.md diff --git a/CHANGELOG.md b/CHANGELOG.md index f10eb0df40..b4adda652b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * **Breaking Change:** cli arguments `--fatal-unused` and `--fatal-warnings` activate by default. * chore: restrict `analyzer` version to `>=3.0.0 <3.4.0`. * chore: restrict `analyzer_plugin` version to `>=0.9.0 <0.10.0`. +* feat: add [prefer-immediate-return](https://dartcodemetrics.dev/docs/rules/common/prefer-immediate-return) rule ## 4.12.0 diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart b/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart index 5eb6a2695d..dfb6810c76 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart @@ -39,6 +39,7 @@ import 'rules_list/prefer_correct_identifier_length/prefer_correct_identifier_le import 'rules_list/prefer_correct_type_name/prefer_correct_type_name_rule.dart'; import 'rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule.dart'; import 'rules_list/prefer_first/prefer_first_rule.dart'; +import 'rules_list/prefer_immediate_return/prefer_immediate_return_rule.dart'; import 'rules_list/prefer_intl_name/prefer_intl_name_rule.dart'; import 'rules_list/prefer_last/prefer_last_rule.dart'; import 'rules_list/prefer_match_file_name/prefer_match_file_name_rule.dart'; @@ -112,6 +113,8 @@ final _implementedRules = )>{ PreferExtractingCallbacksRule.ruleId: (config) => PreferExtractingCallbacksRule(config), PreferFirstRule.ruleId: (config) => PreferFirstRule(config), + PreferImmediateReturnRule.ruleId: (config) => + PreferImmediateReturnRule(config), PreferIntlNameRule.ruleId: (config) => PreferIntlNameRule(config), PreferLastRule.ruleId: (config) => PreferLastRule(config), PreferMatchFileNameRule.ruleId: (config) => PreferMatchFileNameRule(config), diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_immediate_return/prefer_immediate_return_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_immediate_return/prefer_immediate_return_rule.dart new file mode 100644 index 0000000000..bbed2e0fbc --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_immediate_return/prefer_immediate_return_rule.dart @@ -0,0 +1,53 @@ +// ignore_for_file: public_member_api_docs + +import 'package:analyzer/dart/ast/ast.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/replacement.dart'; +import '../../../models/severity.dart'; +import '../../models/common_rule.dart'; +import '../../rule_utils.dart'; + +part 'visitor.dart'; + +class PreferImmediateReturnRule extends CommonRule { + static const ruleId = 'prefer-immediate-return'; + static const _warningMessage = + 'Prefer returning the result immediately instead of declaring an intermediate variable right before the return statement.'; + static const _replaceComment = 'Replace with immediate return.'; + + PreferImmediateReturnRule([Map config = const {}]) + : super( + id: ruleId, + severity: readSeverity(config, Severity.style), + excludes: readExcludes(config), + ); + + @override + Iterable check(InternalResolvedUnitResult source) { + final visitor = _Visitor(); + source.unit.visitChildren(visitor); + + return visitor.issues + .map( + (issue) => createIssue( + rule: this, + location: nodeLocation( + node: issue.returnStatement, + source: source, + withCommentOrMetadata: true, + ), + message: _warningMessage, + replacement: Replacement( + comment: _replaceComment, + replacement: 'return ${issue.variableDeclarationInitializer};', + ), + ), + ) + .toList(growable: false); + } +} diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_immediate_return/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_immediate_return/visitor.dart new file mode 100644 index 0000000000..c55172e4be --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_immediate_return/visitor.dart @@ -0,0 +1,50 @@ +part of 'prefer_immediate_return_rule.dart'; + +class _Visitor extends RecursiveAstVisitor { + final _issues = <_IssueDetails>[]; + + Iterable<_IssueDetails> get issues => _issues; + + @override + void visitBlockFunctionBody(BlockFunctionBody node) { + super.visitBlockFunctionBody(node); + + if (node.block.statements.length < 2) { + return; + } + + final variableDeclarationStatement = + node.block.statements[node.block.statements.length - 2]; + final returnStatement = node.block.statements.last; + if (variableDeclarationStatement is! VariableDeclarationStatement || + returnStatement is! ReturnStatement) { + return; + } + + final returnIdentifier = returnStatement.expression; + if (returnIdentifier is! Identifier) { + return; + } + + final lastDeclaredVariable = + variableDeclarationStatement.variables.variables.last; + if (returnIdentifier.name != lastDeclaredVariable.name.name) { + return; + } + + _issues.add(_IssueDetails( + lastDeclaredVariable.initializer, + returnStatement, + )); + } +} + +class _IssueDetails { + const _IssueDetails( + this.variableDeclarationInitializer, + this.returnStatement, + ); + + final Expression? variableDeclarationInitializer; + final ReturnStatement returnStatement; +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_immediate_return/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_immediate_return/examples/example.dart new file mode 100644 index 0000000000..44735e3884 --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_immediate_return/examples/example.dart @@ -0,0 +1,43 @@ +int calculateSum(int a, int b) { + final sum = a + b; + + return sum; // LINT +} + +int calculateSum(int a, int b) { + final delta = a - b, sum = a + b; + + return sum; // LINT +} + +final calculateSum = (int a, int b) { + final delta = a - b, sum = a + b; + + return sum; // LINT +}; + +class Geometry { + static void calculateRectangleArea(int width, int height) { + final result = width * height; + + return result; // LINT + } +} + +void returnNull() { + final String? x; + + return x; // LINT +} + +int calculateSomething(int a, int b) { + final x = a * b; + + return x * x; // OK +} + +int calculateSum(int a, int b) { + final sum = a + b, delta = a - b; + + return sum; // OK, "sum" variable not immediately preceding return statement +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_immediate_return/prefer_immediate_return_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_immediate_return/prefer_immediate_return_rule_test.dart new file mode 100644 index 0000000000..c3bd1f750b --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_immediate_return/prefer_immediate_return_rule_test.dart @@ -0,0 +1,76 @@ +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_immediate_return/prefer_immediate_return_rule.dart'; +import 'package:test/test.dart'; + +import '../../../../../helpers/rule_test_helper.dart'; + +const _examplePath = 'prefer_immediate_return/examples/example.dart'; + +void main() { + group( + 'PreferImmediateReturnRule', + () { + test('initialization', () async { + final unit = await RuleTestHelper.resolveFromFile(_examplePath); + final issues = PreferImmediateReturnRule().check(unit); + + RuleTestHelper.verifyInitialization( + issues: issues, + ruleId: 'prefer-immediate-return', + severity: Severity.style, + ); + }); + + test('reports about found issues', () async { + final unit = await RuleTestHelper.resolveFromFile(_examplePath); + final issues = PreferImmediateReturnRule().check(unit); + + RuleTestHelper.verifyIssues( + issues: issues, + startLines: [ + 4, + 10, + 16, + 23, + 30, + ], + startColumns: [ + 3, + 3, + 3, + 5, + 3, + ], + messages: [ + 'Prefer returning the result immediately instead of declaring an intermediate variable right before the return statement.', + 'Prefer returning the result immediately instead of declaring an intermediate variable right before the return statement.', + 'Prefer returning the result immediately instead of declaring an intermediate variable right before the return statement.', + 'Prefer returning the result immediately instead of declaring an intermediate variable right before the return statement.', + 'Prefer returning the result immediately instead of declaring an intermediate variable right before the return statement.', + ], + replacementComments: [ + 'Replace with immediate return.', + 'Replace with immediate return.', + 'Replace with immediate return.', + 'Replace with immediate return.', + 'Replace with immediate return.', + ], + replacements: [ + 'return a + b;', + 'return a + b;', + 'return a + b;', + 'return width * height;', + 'return null;', + ], + locationTexts: [ + 'return sum;', + 'return sum;', + 'return sum;', + 'return result;', + 'return x;', + ], + ); + }); + }, + ); +} diff --git a/website/docs/rules/common/prefer-immediate-return.md b/website/docs/rules/common/prefer-immediate-return.md new file mode 100644 index 0000000000..cbc45fceda --- /dev/null +++ b/website/docs/rules/common/prefer-immediate-return.md @@ -0,0 +1,41 @@ +# Prefer immediate return + +![Has auto-fix](https://img.shields.io/badge/-has%20auto--fix-success) + +## Rule id {#rule-id} + +prefer-immediate-return + +## Severity {#severity} + +Style + +## Description {#description} + +Declaring a local variable only to immediately return it might be considered a bad practice. The name of a function or a class method with its return type should give enough information about what should be returned. + +### Example {#example} + +Bad: + +```dart +void calculateSum(int a, int b) { + final sum = a + b; + return sum; // LINT +} + +void calculateArea(int width, int height) { + final result = width * height; + return result; // LINT +} +``` + +Good: + +```dart +void calculateSum(int a, int b) { + return a + b; +} + +void calculateArea(int width, int height) => width * height; +``` diff --git a/website/docs/rules/overview.md b/website/docs/rules/overview.md index 79464deb14..3f94349684 100644 --- a/website/docs/rules/overview.md +++ b/website/docs/rules/overview.md @@ -135,6 +135,10 @@ Rules configuration is [described here](../getting-started/configuration#configu Use `first` to gets the first element. +- [prefer-immediate-return](./common/prefer-immediate-return.md)   ![Has auto-fix](https://img.shields.io/badge/-has%20auto--fix-success) + + Warns when a method or a function returns a variable declared right before the return statement. + - [prefer-last](./common/prefer-last.md)   ![Has auto-fix](https://img.shields.io/badge/-has%20auto--fix-success) Use `last` to gets the last element.