Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a rule prefer_guard_clause for reversing nested if statements #154

Merged
merged 12 commits into from
Apr 22, 2024
1 change: 1 addition & 0 deletions lib/analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ custom_lint:
- newline_before_return
- no_empty_block
- no_equal_then_else
- prefer_early_return

- no_magic_number:
allowed_in_widget_params: true
Expand Down
2 changes: 2 additions & 0 deletions lib/solid_lints.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import 'package:solid_lints/src/lints/no_equal_then_else/no_equal_then_else_rule
import 'package:solid_lints/src/lints/no_magic_number/no_magic_number_rule.dart';
import 'package:solid_lints/src/lints/number_of_parameters/number_of_parameters_metric.dart';
import 'package:solid_lints/src/lints/prefer_conditional_expressions/prefer_conditional_expressions_rule.dart';
import 'package:solid_lints/src/lints/prefer_early_return/prefer_early_return_rule.dart';
import 'package:solid_lints/src/lints/prefer_first/prefer_first_rule.dart';
import 'package:solid_lints/src/lints/prefer_last/prefer_last_rule.dart';
import 'package:solid_lints/src/lints/prefer_match_file_name/prefer_match_file_name_rule.dart';
Expand Down Expand Up @@ -62,6 +63,7 @@ class _SolidLints extends PluginBase {
PreferMatchFileNameRule.createRule(configs),
ProperSuperCallsRule.createRule(configs),
AvoidDebugPrint.createRule(configs),
PreferEarlyReturnRule.createRule(configs),
AvoidFinalWithGetterRule.createRule(configs),
];

Expand Down
68 changes: 68 additions & 0 deletions lib/src/lints/prefer_early_return/prefer_early_return_rule.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import 'package:analyzer/error/listener.dart';
import 'package:custom_lint_builder/custom_lint_builder.dart';
import 'package:solid_lints/src/lints/prefer_early_return/visitors/prefer_early_return_visitor.dart';
import 'package:solid_lints/src/models/rule_config.dart';
import 'package:solid_lints/src/models/solid_lint_rule.dart';

/// A rule which highlights `if` statements that span the entire body,
/// and suggests replacing them with a reversed boolean check
/// with an early return.
///
/// ### Example
///
/// #### BAD:
///
/// ```dart
/// void func() {
/// if (a) { //LINT
/// if (b) { //LINT
/// c;
/// }
/// }
/// }
/// ```
///
/// #### GOOD:
///
/// ```dart
/// void func() {
/// if (!a) return;
/// if (!b) return;
/// c;
/// }
/// ```
class PreferEarlyReturnRule extends SolidLintRule {
/// The [LintCode] of this lint rule that represents the error if
/// 'if' statements should be reversed
static const String lintName = 'prefer_early_return';

PreferEarlyReturnRule._(super.config);

/// Creates a new instance of [PreferEarlyReturnRule]
/// based on the lint configuration.
factory PreferEarlyReturnRule.createRule(CustomLintConfigs configs) {
final rule = RuleConfig(
configs: configs,
name: lintName,
problemMessage: (_) => "Use reverse if to reduce nesting",
);

return PreferEarlyReturnRule._(rule);
}

@override
void run(
CustomLintResolver resolver,
ErrorReporter reporter,
CustomLintContext context,
) {
context.registry.addBlockFunctionBody((node) {
final visitor = PreferEarlyReturnVisitor();
node.accept(visitor);

for (final element in visitor.nodes) {
reporter.reportErrorForNode(code, element);
}
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:solid_lints/src/lints/prefer_early_return/visitors/return_statement_visitor.dart';

/// The AST visitor that will collect all unnecessary if statements
class PreferEarlyReturnVisitor extends RecursiveAstVisitor<void> {
final _nodes = <AstNode>[];

/// All unnecessary if statements and conditional expressions.
Iterable<AstNode> get nodes => _nodes;

@override
void visitBlockFunctionBody(BlockFunctionBody node) {
super.visitBlockFunctionBody(node);

if (node.block.statements.isEmpty) return;

final (ifStatements, nextStatement) = _getStartIfStatements(node);
if (ifStatements.isEmpty) return;

// limit visitor to only work with functions
// that don't have a return statement or the return statement is empty
final nextStatementIsEmptyReturn =
nextStatement is ReturnStatement && nextStatement.expression == null;
final nextStatementIsNull = nextStatement == null;

if (!nextStatementIsEmptyReturn && !nextStatementIsNull) return;

_handleIfStatement(ifStatements.last);
}

void _handleIfStatement(IfStatement node) {
if (_isElseIfStatement(node)) return;
if (_hasElseStatement(node)) return;
if (_hasReturnStatement(node)) return;

_nodes.add(node);
}

// returns a list of if statements at the start of the function
// and the next statement after it
// examples:
// [if, if, if, return] -> ([if, if, if], return)
// [if, if, if, _doSomething, return] -> ([if, if, if], _doSomething)
// [if, if, if] -> ([if, if, if], null)
(List<IfStatement>, Statement?) _getStartIfStatements(
BlockFunctionBody body,
) {
final List<IfStatement> ifStatements = [];
for (final statement in body.block.statements) {
if (statement is IfStatement) {
ifStatements.add(statement);
} else {
return (ifStatements, statement);
}
}
return (ifStatements, null);
}

bool _hasElseStatement(IfStatement node) {
return node.elseStatement != null;
}

bool _isElseIfStatement(IfStatement node) {
return node.elseStatement != null && node.elseStatement is IfStatement;
}

bool _hasReturnStatement(Statement node) {
final visitor = ReturnStatementVisitor();
node.accept(visitor);
return visitor.nodes.isNotEmpty;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';

/// The AST visitor that will collect every Return statement
class ReturnStatementVisitor extends RecursiveAstVisitor<void> {
final _nodes = <ReturnStatement>[];

/// All unnecessary return statements
Iterable<ReturnStatement> get nodes => _nodes;

@override
void visitReturnStatement(ReturnStatement node) {
super.visitReturnStatement(node);
_nodes.add(node);
}
}
1 change: 1 addition & 0 deletions lint_test/analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ custom_lint:
- no_empty_block
- no_equal_then_else
- avoid_debug_print
- prefer_early_return
- member_ordering:
alphabetize: true
order:
Expand Down
2 changes: 1 addition & 1 deletion lint_test/cyclomatic_complexity_test.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// ignore_for_file: literal_only_boolean_expressions
// ignore_for_file: literal_only_boolean_expressions, prefer_early_return
// ignore_for_file: no_empty_block, prefer_match_file_name

/// Check complexity fail
Expand Down
2 changes: 1 addition & 1 deletion lint_test/no_empty_block_test.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// ignore_for_file: prefer_const_declarations, prefer_match_file_name
// ignore_for_file: prefer_const_declarations, prefer_match_file_name, prefer_early_return
// ignore_for_file: unused_local_variable
// ignore_for_file: cyclomatic_complexity
// ignore_for_file: avoid_unused_parameters
Expand Down
Loading
Loading