From 8f02dbae46e5d17e4353d03a2801509304ede815 Mon Sep 17 00:00:00 2001 From: Islam-Shaaban-Ibrahim Date: Thu, 16 Apr 2026 15:31:11 +0200 Subject: [PATCH] migrate prefer early return rule with tests --- .../prefer_early_return_rule.dart | 59 ++-- .../visitors/prefer_early_return_visitor.dart | 90 ++---- lint_test/prefer_early_return_test.dart | 299 ------------------ test/prefer_early_return_rule_test.dart | 91 ++++++ 4 files changed, 143 insertions(+), 396 deletions(-) delete mode 100644 lint_test/prefer_early_return_test.dart create mode 100644 test/prefer_early_return_rule_test.dart diff --git a/lib/src/lints/prefer_early_return/prefer_early_return_rule.dart b/lib/src/lints/prefer_early_return/prefer_early_return_rule.dart index e4b54b18..812841f8 100644 --- a/lib/src/lints/prefer_early_return/prefer_early_return_rule.dart +++ b/lib/src/lints/prefer_early_return/prefer_early_return_rule.dart @@ -1,8 +1,8 @@ -import 'package:analyzer/error/listener.dart'; -import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.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 @@ -31,38 +31,37 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// c; /// } /// ``` -class PreferEarlyReturnRule extends SolidLintRule { - /// This lint rule represents the error if - /// 'if' statements should be reversed +class PreferEarlyReturnRule extends AnalysisRule { + /// Lint name static const String lintName = 'prefer_early_return'; - PreferEarlyReturnRule._(super.config); + /// Lint code + static const LintCode _code = LintCode( + lintName, + "Use reverse if to reduce nesting", + ); - /// 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", - ); + /// Creates an instance of [PreferEarlyReturnRule] + PreferEarlyReturnRule() + : super( + name: lintName, + description: 'Use reverse if to reduce nesting', + ); - return PreferEarlyReturnRule._(rule); - } + @override + LintCode get diagnosticCode => _code; @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, ) { - context.registry.addBlockFunctionBody((node) { - final visitor = PreferEarlyReturnVisitor(); - node.accept(visitor); - - for (final element in visitor.nodes) { - reporter.atNode(element, code); - } - }); + registry.addBlockFunctionBody( + this, + PreferEarlyReturnVisitor( + rule: this, + context: context, + ), + ); } } diff --git a/lib/src/lints/prefer_early_return/visitors/prefer_early_return_visitor.dart b/lib/src/lints/prefer_early_return/visitors/prefer_early_return_visitor.dart index 127de6e8..12e984ba 100644 --- a/lib/src/lints/prefer_early_return/visitors/prefer_early_return_visitor.dart +++ b/lib/src/lints/prefer_early_return/visitors/prefer_early_return_visitor.dart @@ -1,81 +1,37 @@ +import 'package:analyzer/analysis_rule/rule_context.dart'; 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'; -import 'package:solid_lints/src/lints/prefer_early_return/visitors/throw_expression_visitor.dart'; +import 'package:solid_lints/src/lints/prefer_early_return/prefer_early_return_rule.dart'; -/// The AST visitor that will collect all unnecessary if statements +/// Visitor for [PreferEarlyReturnRule]. class PreferEarlyReturnVisitor extends RecursiveAstVisitor { - final _nodes = []; + /// The rule associated with the visitor. + final PreferEarlyReturnRule rule; - /// All unnecessary if statements and conditional expressions. - Iterable get nodes => _nodes; + /// The context associated with the visitor. + final RuleContext context; - @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; - if (_hasThrowExpression(node)) return; + /// Constructor for [PreferEarlyReturnVisitor]. + PreferEarlyReturnVisitor({ + required this.rule, + required this.context, + }); - _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, Statement?) _getStartIfStatements( - BlockFunctionBody body, - ) { - final List ifStatements = []; - for (final statement in body.block.statements) { - if (statement is IfStatement) { - ifStatements.add(statement); - } else { - return (ifStatements, statement); - } + @override + void visitIfStatement(IfStatement node) { + if (_shouldReport(node)) { + context.currentUnit?.diagnosticReporter.atNode( + node, + rule.diagnosticCode, + ); } - return (ifStatements, null); - } - bool _hasElseStatement(IfStatement node) { - return node.elseStatement != null; + super.visitIfStatement(node); } - 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; - } + bool _shouldReport(IfStatement node) { + final parent = node.parent; - bool _hasThrowExpression(Statement node) { - final visitor = ThrowExpressionVisitor(); - node.accept(visitor); - return visitor.nodes.isNotEmpty; + return parent is Block && parent.statements.length == 1; } } diff --git a/lint_test/prefer_early_return_test.dart b/lint_test/prefer_early_return_test.dart deleted file mode 100644 index d9108186..00000000 --- a/lint_test/prefer_early_return_test.dart +++ /dev/null @@ -1,299 +0,0 @@ -// ignore_for_file: dead_code, cyclomatic_complexity, no_equal_then_else, prefer_match_file_name - -// ignore: no_empty_block -void _doSomething() {} - -void oneIf() { - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } -} - -int oneIfWithReturnValue() { - //no lint - if (true) { - _doSomething(); - } - - return 1; -} - -void oneIfWithReturn() { - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } - - return; -} - -void nestedIf2() { - //expect_lint: prefer_early_return - if (true) { - if (true) { - _doSomething(); - } - } -} - -int nestedIf2WithReturnValue() { - //no lint - if (true) { - if (true) { - _doSomething(); - } - } - - return 1; -} - -void nestedIf3() { - //expect_lint: prefer_early_return - if (true) { - if (true) { - if (true) { - _doSomething(); - } - } - } -} - -void oneNestedIf2WithReturn() { - //expect_lint: prefer_early_return - 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: prefer_early_return - if (true) { - if (true) { - _doSomething(); - } else { - _doSomething(); - } - } -} - -void threeIf() { - //expect_lint: prefer_early_return - if (true) { - if (true) { - if (true) { - _doSomething(); - } - } - } -} - -void threeIfElse1() { - //no lint - if (true) { - if (true) { - if (true) { - _doSomething(); - } - } - } else { - _doSomething(); - } -} - -void threeIfElse2() { - //expect_lint: prefer_early_return - if (true) { - if (true) { - if (true) { - _doSomething(); - } - } else { - _doSomething(); - } - } -} - -void threeIfElse3() { - //expect_lint: prefer_early_return - if (true) { - if (true) { - if (true) { - _doSomething(); - } else { - _doSomething(); - } - } - } -} - -void twoSeqentialIf() { - if (false) return; - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } -} - -void twoSeqentialIfReturn() { - //no lint - if (false) return; - if (true) return; - - return; -} - -void twoSeqentialIfReturn2() { - //no lint - if (false) return; - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } - - return; -} - -void twoSeqentialIfSomething() { - //no lint - if (false) return; - //no lint - if (true) { - _doSomething(); - } - - _doSomething(); -} - -void twoSeqentialIfSomething2() { - //no lint - if (true) { - _doSomething(); - } - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } -} - -void threeSeqentialIfReturn() { - //no lint - if (false) return; - if (true) return; - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } - - return; -} - -void threeSeqentialIfReturn2() { - //no lint - if (false) return; - //no lint - if (true) { - _doSomething(); - } - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } -} - -void oneIfWithThrowWithReturn() { - //no lint - if (true) { - throw ''; - } - - return; -} - -void oneIfElseWithThrowReturn() { - //no lint - if (true) { - _doSomething(); - } else { - throw ''; - } -} - -void twoSeqentialIfWithThrow() { - if (false) throw ''; - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } -} - -void twoSeqentialIfWithThrowReturn2() { - //no lint - if (false) throw ''; - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } - - return; -} - -void threeSeqentialIfWithThrowReturn() { - //no lint - if (false) throw ''; - if (true) throw ''; - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } - - return; -} - -void threeSeqentialIfWithThrowReturn2() { - //no lint - if (false) throw ''; - //no lint - if (true) { - _doSomething(); - } - //expect_lint: prefer_early_return - if (true) { - _doSomething(); - } -} diff --git a/test/prefer_early_return_rule_test.dart b/test/prefer_early_return_rule_test.dart new file mode 100644 index 00000000..46bb94ca --- /dev/null +++ b/test/prefer_early_return_rule_test.dart @@ -0,0 +1,91 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/prefer_early_return/prefer_early_return_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(PreferEarlyReturnRuleTest); + }); +} + +@reflectiveTest +class PreferEarlyReturnRuleTest extends AnalysisRuleTest { + @override + void setUp() { + rule = PreferEarlyReturnRule(); + super.setUp(); + } + + @override + String get analysisRule => PreferEarlyReturnRule.lintName; + + void test_reports_if_as_only_statement_in_function() async { + await assertDiagnostics( + r''' +void test(bool a) { + if (a) { + print('hello'); + } +} +''', + [lint(22, 32)], + ); + } + + void test_reports_nested_if_as_only_statement() async { + await assertDiagnostics( + r''' +void test(bool a, bool b) { + if (a) { + if (b) { + print('nested'); + } + } +} +''', + [ + lint(30, 54), + lint(43, 37), + ], + ); + } + + void test_does_not_report_if_with_following_statement() async { + await assertNoDiagnostics( + r''' +void test(bool a) { + if (a) { + print('hello'); + } + + print('after'); +} +''', + ); + } + + void test_does_not_report_when_multiple_statements_exist() async { + await assertNoDiagnostics( + r''' +void test(bool a) { + print('before'); + + if (a) { + print('hello'); + } +} +''', + ); + } + + void test_does_not_report_regular_inline_if() async { + await assertNoDiagnostics( + r''' +void test(bool a) { + if (a) print('hello'); + print('done'); +} +''', + ); + } +}