Skip to content

Commit 4ea6cb4

Browse files
velvitoff4akloon
andauthored
Add a rule prefer_guard_clause for reversing nested if statements (#154)
* reverse_if_to_avoid_nesting wip * change name to prefer_guard_close and improve tests * prefer_guard_clause wip * fix test name for prefer_guard_clause * changed to detecting nested if statements * fix pr comments * adapt to two sequential if statements * remove (.length <= 2) limit * fix alphabetic ordering of imports * refactor and fix prefer_early_return_visitor.dart to handle nested if statements correctly * refactor prefer_early_return --------- Co-authored-by: alexiuk.genius <alexiuk.genius@gmail.com>
1 parent 8dc2cd5 commit 4ea6cb4

9 files changed

Lines changed: 400 additions & 2 deletions

lib/analysis_options.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ custom_lint:
7878
- newline_before_return
7979
- no_empty_block
8080
- no_equal_then_else
81+
- prefer_early_return
8182

8283
- no_magic_number:
8384
allowed_in_widget_params: true

lib/solid_lints.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import 'package:solid_lints/src/lints/no_equal_then_else/no_equal_then_else_rule
2323
import 'package:solid_lints/src/lints/no_magic_number/no_magic_number_rule.dart';
2424
import 'package:solid_lints/src/lints/number_of_parameters/number_of_parameters_metric.dart';
2525
import 'package:solid_lints/src/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart';
26+
import 'package:solid_lints/src/lints/prefer_early_return/prefer_early_return_rule.dart';
2627
import 'package:solid_lints/src/lints/prefer_first/prefer_first_rule.dart';
2728
import 'package:solid_lints/src/lints/prefer_last/prefer_last_rule.dart';
2829
import 'package:solid_lints/src/lints/prefer_match_file_name/prefer_match_file_name_rule.dart';
@@ -62,6 +63,7 @@ class _SolidLints extends PluginBase {
6263
PreferMatchFileNameRule.createRule(configs),
6364
ProperSuperCallsRule.createRule(configs),
6465
AvoidDebugPrint.createRule(configs),
66+
PreferEarlyReturnRule.createRule(configs),
6567
AvoidFinalWithGetterRule.createRule(configs),
6668
];
6769

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import 'package:analyzer/error/listener.dart';
2+
import 'package:custom_lint_builder/custom_lint_builder.dart';
3+
import 'package:solid_lints/src/lints/prefer_early_return/visitors/prefer_early_return_visitor.dart';
4+
import 'package:solid_lints/src/models/rule_config.dart';
5+
import 'package:solid_lints/src/models/solid_lint_rule.dart';
6+
7+
/// A rule which highlights `if` statements that span the entire body,
8+
/// and suggests replacing them with a reversed boolean check
9+
/// with an early return.
10+
///
11+
/// ### Example
12+
///
13+
/// #### BAD:
14+
///
15+
/// ```dart
16+
/// void func() {
17+
/// if (a) { //LINT
18+
/// if (b) { //LINT
19+
/// c;
20+
/// }
21+
/// }
22+
/// }
23+
/// ```
24+
///
25+
/// #### GOOD:
26+
///
27+
/// ```dart
28+
/// void func() {
29+
/// if (!a) return;
30+
/// if (!b) return;
31+
/// c;
32+
/// }
33+
/// ```
34+
class PreferEarlyReturnRule extends SolidLintRule {
35+
/// The [LintCode] of this lint rule that represents the error if
36+
/// 'if' statements should be reversed
37+
static const String lintName = 'prefer_early_return';
38+
39+
PreferEarlyReturnRule._(super.config);
40+
41+
/// Creates a new instance of [PreferEarlyReturnRule]
42+
/// based on the lint configuration.
43+
factory PreferEarlyReturnRule.createRule(CustomLintConfigs configs) {
44+
final rule = RuleConfig(
45+
configs: configs,
46+
name: lintName,
47+
problemMessage: (_) => "Use reverse if to reduce nesting",
48+
);
49+
50+
return PreferEarlyReturnRule._(rule);
51+
}
52+
53+
@override
54+
void run(
55+
CustomLintResolver resolver,
56+
ErrorReporter reporter,
57+
CustomLintContext context,
58+
) {
59+
context.registry.addBlockFunctionBody((node) {
60+
final visitor = PreferEarlyReturnVisitor();
61+
node.accept(visitor);
62+
63+
for (final element in visitor.nodes) {
64+
reporter.reportErrorForNode(code, element);
65+
}
66+
});
67+
}
68+
}
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import 'package:analyzer/dart/ast/ast.dart';
2+
import 'package:analyzer/dart/ast/visitor.dart';
3+
import 'package:solid_lints/src/lints/prefer_early_return/visitors/return_statement_visitor.dart';
4+
5+
/// The AST visitor that will collect all unnecessary if statements
6+
class PreferEarlyReturnVisitor extends RecursiveAstVisitor<void> {
7+
final _nodes = <AstNode>[];
8+
9+
/// All unnecessary if statements and conditional expressions.
10+
Iterable<AstNode> get nodes => _nodes;
11+
12+
@override
13+
void visitBlockFunctionBody(BlockFunctionBody node) {
14+
super.visitBlockFunctionBody(node);
15+
16+
if (node.block.statements.isEmpty) return;
17+
18+
final (ifStatements, nextStatement) = _getStartIfStatements(node);
19+
if (ifStatements.isEmpty) return;
20+
21+
// limit visitor to only work with functions
22+
// that don't have a return statement or the return statement is empty
23+
final nextStatementIsEmptyReturn =
24+
nextStatement is ReturnStatement && nextStatement.expression == null;
25+
final nextStatementIsNull = nextStatement == null;
26+
27+
if (!nextStatementIsEmptyReturn && !nextStatementIsNull) return;
28+
29+
_handleIfStatement(ifStatements.last);
30+
}
31+
32+
void _handleIfStatement(IfStatement node) {
33+
if (_isElseIfStatement(node)) return;
34+
if (_hasElseStatement(node)) return;
35+
if (_hasReturnStatement(node)) return;
36+
37+
_nodes.add(node);
38+
}
39+
40+
// returns a list of if statements at the start of the function
41+
// and the next statement after it
42+
// examples:
43+
// [if, if, if, return] -> ([if, if, if], return)
44+
// [if, if, if, _doSomething, return] -> ([if, if, if], _doSomething)
45+
// [if, if, if] -> ([if, if, if], null)
46+
(List<IfStatement>, Statement?) _getStartIfStatements(
47+
BlockFunctionBody body,
48+
) {
49+
final List<IfStatement> ifStatements = [];
50+
for (final statement in body.block.statements) {
51+
if (statement is IfStatement) {
52+
ifStatements.add(statement);
53+
} else {
54+
return (ifStatements, statement);
55+
}
56+
}
57+
return (ifStatements, null);
58+
}
59+
60+
bool _hasElseStatement(IfStatement node) {
61+
return node.elseStatement != null;
62+
}
63+
64+
bool _isElseIfStatement(IfStatement node) {
65+
return node.elseStatement != null && node.elseStatement is IfStatement;
66+
}
67+
68+
bool _hasReturnStatement(Statement node) {
69+
final visitor = ReturnStatementVisitor();
70+
node.accept(visitor);
71+
return visitor.nodes.isNotEmpty;
72+
}
73+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import 'package:analyzer/dart/ast/ast.dart';
2+
import 'package:analyzer/dart/ast/visitor.dart';
3+
4+
/// The AST visitor that will collect every Return statement
5+
class ReturnStatementVisitor extends RecursiveAstVisitor<void> {
6+
final _nodes = <ReturnStatement>[];
7+
8+
/// All unnecessary return statements
9+
Iterable<ReturnStatement> get nodes => _nodes;
10+
11+
@override
12+
void visitReturnStatement(ReturnStatement node) {
13+
super.visitReturnStatement(node);
14+
_nodes.add(node);
15+
}
16+
}

lint_test/analysis_options.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ custom_lint:
2525
- no_empty_block
2626
- no_equal_then_else
2727
- avoid_debug_print
28+
- prefer_early_return
2829
- member_ordering:
2930
alphabetize: true
3031
order:

lint_test/cyclomatic_complexity_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// ignore_for_file: literal_only_boolean_expressions
1+
// ignore_for_file: literal_only_boolean_expressions, prefer_early_return
22
// ignore_for_file: no_empty_block, prefer_match_file_name
33

44
/// Check complexity fail

lint_test/no_empty_block_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// ignore_for_file: prefer_const_declarations, prefer_match_file_name
1+
// ignore_for_file: prefer_const_declarations, prefer_match_file_name, prefer_early_return
22
// ignore_for_file: unused_local_variable
33
// ignore_for_file: cyclomatic_complexity
44
// ignore_for_file: avoid_unused_parameters

0 commit comments

Comments
 (0)