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 @@ -77,6 +77,7 @@ custom_lint:
- newline_before_return
- no_empty_block
- no_equal_then_else
- reverse_if_to_reduce_nesting

- 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 @@ -26,6 +26,7 @@ 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';
import 'package:solid_lints/src/lints/proper_super_calls/proper_super_calls_rule.dart';
import 'package:solid_lints/src/lints/reverse_if_to_reduce_nesting/reverse_if_to_reduce_nesting_rule.dart';
import 'package:solid_lints/src/models/solid_lint_rule.dart';

/// Creates a plugin for our custom linter
Expand Down Expand Up @@ -61,6 +62,7 @@ class _SolidLints extends PluginBase {
PreferMatchFileNameRule.createRule(configs),
ProperSuperCallsRule.createRule(configs),
AvoidDebugPrint.createRule(configs),
ReverseIfToReduceNestingRule.createRule(configs),
];

// Return only enabled rules
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import 'package:analyzer/error/listener.dart';
import 'package:custom_lint_builder/custom_lint_builder.dart';
import 'package:solid_lints/src/lints/reverse_if_to_reduce_nesting/visitors/reverse_if_to_reduce_nesting_visitor.dart';
import 'package:solid_lints/src/models/rule_config.dart';
import 'package:solid_lints/src/models/solid_lint_rule.dart';

/// A `reverse_if_to_reduce_nesting` rule which promotes using guard clauses
/// over nesting `if` statements
velvitoff marked this conversation as resolved.
Show resolved Hide resolved
///
/// ### Example
///
/// #### BAD:
///
/// ```dart
/// void func() {
/// if(a){ //LINT
/// if(b){ //LINT
/// c;
/// }
/// }
/// }
velvitoff marked this conversation as resolved.
Show resolved Hide resolved
/// ```
///
/// #### GOOD:
///
/// ```dart
/// void func() {
/// if(!a) return;
/// if(!b) return;
/// c;
/// }
/// ```
class ReverseIfToReduceNestingRule extends SolidLintRule {
/// The [LintCode] of this lint rule that represents the error if
/// 'if' statements should be reversed
static const String lintName = 'reverse_if_to_reduce_nesting';
velvitoff marked this conversation as resolved.
Show resolved Hide resolved

ReverseIfToReduceNestingRule._(super.config);

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

return ReverseIfToReduceNestingRule._(rule);
}

@override
void run(
CustomLintResolver resolver,
ErrorReporter reporter,
CustomLintContext context,
) {
context.registry.addBlockFunctionBody((node) {
final visitor = ReverseIfToReduceNestingVisitor();
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,16 @@
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';

/// The AST visitor that will collect every If statement
class IfStatementVisitor extends RecursiveAstVisitor<void> {
final _nodes = <IfStatement>[];

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

@override
void visitIfStatement(IfStatement node) {
super.visitIfStatement(node);
_nodes.add(node);
}
}
velvitoff marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:solid_lints/src/lints/reverse_if_to_reduce_nesting/visitors/if_statement_visitor.dart';

/// The AST visitor that will collect all unnecessary if statements
class ReverseIfToReduceNestingVisitor 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 first = node.block.statements.first;

switch (node.block.statements.length) {
case 1:
if (first is IfStatement) _handleIfStatement(first);
return;
case 2:
final last = node.block.statements.last;
if (last is ReturnStatement && first is IfStatement) {
_handleIfStatement(first);
}
case _:
return;
}
}

void _handleIfStatement(IfStatement node) {
if (_isElseIfStatement(node)) return;
if (_hasElseStatement(node)) return;
if (!_containsIfStatement(node.thenStatement)) return;
yurii-prykhodko-solid marked this conversation as resolved.
Show resolved Hide resolved

_nodes.add(node);
}
}

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

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

bool _containsIfStatement(Statement node) {
final visitor = IfStatementVisitor();
node.accept(visitor);
return visitor.nodes.isNotEmpty;
}
1 change: 1 addition & 0 deletions lint_test/analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ custom_lint:
- no_empty_block
- no_equal_then_else
- avoid_debug_print
- reverse_if_to_reduce_nesting
- 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, reverse_if_to_reduce_nesting
// ignore_for_file: no_empty_block, prefer_match_file_name

/// Check complexity fail
Expand Down
143 changes: 143 additions & 0 deletions lint_test/reverse_if_to_reduce_nesting_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
// ignore_for_file: dead_code, cyclomatic_complexity, no_equal_then_else, prefer_match_file_name

// ignore: no_empty_block
void _doSomething() {}

void oneIf() {
//no lint
if (true) {
_doSomething();
}
}

void oneIfWithReturn() {
//no lint
if (true) {
_doSomething();
}

return;
}

void nestedIf2() {
//expect_lint: reverse_if_to_reduce_nesting
if (true) {
if (true) {
_doSomething();
}
}
}

void nestedIf3() {
//expect_lint: reverse_if_to_reduce_nesting
if (true) {
if (true) {
if (true) {
_doSomething();
}
}
}
}

void oneNestedIf2WithReturn() {
//expect_lint: reverse_if_to_reduce_nesting
if (true) {
if (true) {
if (true) {
_doSomething();
}
}
}

return;
}

void oneIfElse() {
//no lint
if (true) {
_doSomething();
} else {
_doSomething();
}
}

void oneIfElseReturn() {
//no lint
if (true) {
_doSomething();
} else {
return;
}
}

void twoIfElse() {
//no lint
if (true) {
if (true) {
_doSomething();
}
} else {
_doSomething();
}
}

void twoIfElseInner() {
//expect_lint: reverse_if_to_reduce_nesting
if (true) {
if (true) {
_doSomething();
} else {
_doSomething();
}
}
}

void threeIf() {
//expect_lint: reverse_if_to_reduce_nesting
if (true) {
if (true) {
if (true) {
_doSomething();
}
}
}
}

void threeIfElse1() {
//no lint
if (true) {
if (true) {
if (true) {
_doSomething();
}
}
} else {
_doSomething();
}
}

void threeIfElse2() {
//expect_lint: reverse_if_to_reduce_nesting
if (true) {
if (true) {
if (true) {
_doSomething();
}
} else {
_doSomething();
}
}
}

void threeIfElse3() {
//expect_lint: reverse_if_to_reduce_nesting
if (true) {
if (true) {
if (true) {
_doSomething();
} else {
_doSomething();
}
}
}
}
Loading