From a6523eff978f6b4edcea95525e1819f8cda7e2bf Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Thu, 27 Aug 2026 17:29:43 +0300 Subject: [PATCH 1/3] feat: enhance prefer_early_return rule to support loops and configurable parameters --- lib/analysis_options.yaml | 5 +- lib/main.dart | 2 +- .../prefer_early_return_parameters.dart | 39 ++ .../prefer_early_return_rule.dart | 66 ++- .../visitors/early_return_exit_visitor.dart | 74 ++++ .../visitors/prefer_early_return_visitor.dart | 114 ++++-- .../visitors/return_statement_visitor.dart | 16 - .../visitors/throw_expression_visitor.dart | 16 - .../prefer_early_return_rule_test.dart | 382 +++++++++++++++++- 9 files changed, 607 insertions(+), 107 deletions(-) create mode 100644 lib/src/lints/prefer_early_return/models/prefer_early_return_parameters.dart create mode 100644 lib/src/lints/prefer_early_return/visitors/early_return_exit_visitor.dart delete mode 100644 lib/src/lints/prefer_early_return/visitors/return_statement_visitor.dart delete mode 100644 lib/src/lints/prefer_early_return/visitors/throw_expression_visitor.dart diff --git a/lib/analysis_options.yaml b/lib/analysis_options.yaml index a20f30f7..167f9fcb 100644 --- a/lib/analysis_options.yaml +++ b/lib/analysis_options.yaml @@ -93,8 +93,9 @@ solid_lints: newline_before_return: true no_empty_block: true no_equal_then_else: true - # Disabled by default for now. Will be considered for future inclusion. - prefer_early_return: false + prefer_early_return: + maximum_statements: 1 + ignore_if_case: true no_magic_number: allowed_in_widget_params: true diff --git a/lib/main.dart b/lib/main.dart index 1274efa2..dd35a7dc 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -85,7 +85,7 @@ class SolidLintsPlugin extends Plugin { NoMagicNumberRule(analysisOptionsLoader: analysisLoader), NumberOfParametersRule(analysisOptionsLoader: analysisLoader), PreferConditionalExpressionsRule(analysisOptionsLoader: analysisLoader), - PreferEarlyReturnRule(), + PreferEarlyReturnRule(analysisOptionsLoader: analysisLoader), PreferFirstRule(), PreferLastRule(), PreferMatchFileNameRule(analysisOptionsLoader: analysisLoader), diff --git a/lib/src/lints/prefer_early_return/models/prefer_early_return_parameters.dart b/lib/src/lints/prefer_early_return/models/prefer_early_return_parameters.dart new file mode 100644 index 00000000..bd1d3c23 --- /dev/null +++ b/lib/src/lints/prefer_early_return/models/prefer_early_return_parameters.dart @@ -0,0 +1,39 @@ +/// A data model class that represents the "prefer early return" +/// input parameters. +class PreferEarlyReturnParameters { + static const _maximumStatementsConfig = 'maximum_statements'; + static const _ignoreIfCaseConfig = 'ignore_if_case'; + + static const _defaultMaximumStatements = 1; + static const _defaultIgnoreIfCase = true; + + /// The maximum number of statements allowed inside an `if` block before + /// triggering the lint. If the number of statements does not exceed this + /// threshold, the analysis is skipped. + final int maximumStatements; + + /// Whether to ignore `if-case` pattern matching statements. + final bool ignoreIfCase; + + /// Constructor for [PreferEarlyReturnParameters] model. + const PreferEarlyReturnParameters({ + required this.maximumStatements, + required this.ignoreIfCase, + }); + + /// Empty [PreferEarlyReturnParameters] model with default values. + factory PreferEarlyReturnParameters.empty() => + const PreferEarlyReturnParameters( + maximumStatements: _defaultMaximumStatements, + ignoreIfCase: _defaultIgnoreIfCase, + ); + + /// Method for creating [PreferEarlyReturnParameters] from json data. + factory PreferEarlyReturnParameters.fromJson( + Map json, + ) => PreferEarlyReturnParameters( + maximumStatements: + json[_maximumStatementsConfig] as int? ?? _defaultMaximumStatements, + ignoreIfCase: json[_ignoreIfCaseConfig] as bool? ?? _defaultIgnoreIfCase, + ); +} 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 91f26aec..a6d28c29 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,12 +1,23 @@ -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/models/prefer_early_return_parameters.dart'; import 'package:solid_lints/src/lints/prefer_early_return/visitors/prefer_early_return_visitor.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. +/// A rule which highlights `if` statements that span the entire body of a +/// function or loop, and suggests replacing them with a reversed boolean check +/// with an early return or continue. +/// +/// ### Example config: +/// +/// ```yaml +/// solid_lints: +/// diagnostics: +/// prefer_early_return: +/// maximum_statements: 1 +/// ignore_if_case: true +/// ``` /// /// ### Example /// @@ -15,8 +26,16 @@ import 'package:solid_lints/src/lints/prefer_early_return/visitors/prefer_early_ /// ```dart /// void func() { /// if (a) { //LINT -/// if (b) { //LINT -/// c; +/// c; +/// d; +/// } +/// } +/// +/// void loop() { +/// for (final item in items) { +/// if (item.isValid) { //LINT +/// process(item); +/// save(item); /// } /// } /// } @@ -27,26 +46,36 @@ import 'package:solid_lints/src/lints/prefer_early_return/visitors/prefer_early_ /// ```dart /// void func() { /// if (!a) return; -/// if (!b) return; /// c; +/// d; +/// } +/// +/// void loop() { +/// for (final item in items) { +/// if (!item.isValid) continue; +/// process(item); +/// save(item); +/// } /// } /// ``` -class PreferEarlyReturnRule extends AnalysisRule { +class PreferEarlyReturnRule extends SolidLintRule { /// Lint name static const String lintName = 'prefer_early_return'; /// Lint code static const LintCode _code = LintCode( lintName, - "Use reverse if to reduce nesting", + 'Use reverse if to reduce nesting', ); /// Creates an instance of [PreferEarlyReturnRule] - PreferEarlyReturnRule() - : super( - name: lintName, - description: 'Use reverse if to reduce nesting', - ); + PreferEarlyReturnRule({ + required super.analysisOptionsLoader, + }) : super.withParameters( + name: lintName, + description: 'Use reverse if to reduce nesting', + parametersParser: PreferEarlyReturnParameters.fromJson, + ); @override LintCode get diagnosticCode => _code; @@ -56,11 +85,20 @@ class PreferEarlyReturnRule extends AnalysisRule { RuleVisitorRegistry registry, RuleContext context, ) { + super.registerNodeProcessors(registry, context); + + final parameters = + getParametersForContext(context) ?? PreferEarlyReturnParameters.empty(); + final visitor = PreferEarlyReturnVisitor( rule: this, context: context, + parameters: parameters, ); registry.addBlockFunctionBody(this, visitor); + registry.addForStatement(this, visitor); + registry.addWhileStatement(this, visitor); + registry.addDoStatement(this, visitor); } } diff --git a/lib/src/lints/prefer_early_return/visitors/early_return_exit_visitor.dart b/lib/src/lints/prefer_early_return/visitors/early_return_exit_visitor.dart new file mode 100644 index 00000000..f08b28cc --- /dev/null +++ b/lib/src/lints/prefer_early_return/visitors/early_return_exit_visitor.dart @@ -0,0 +1,74 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; + +/// AST visitor that checks if a statement contains an early exit +/// (return, throw, or loop break/continue) respecting scope boundaries. +class EarlyReturnExitVisitor extends RecursiveAstVisitor { + final bool _isLoop; + bool _hasExit = false; + int _nestedLoopDepth = 0; + int _nestedSwitchDepth = 0; + + EarlyReturnExitVisitor._(this._isLoop); + + /// Checks whether [node] contains any early exit statement. + static bool hasExitIn(Statement node, {required bool isLoop}) { + final visitor = EarlyReturnExitVisitor._(isLoop); + node.accept(visitor); + return visitor._hasExit; + } + + @override + void visitReturnStatement(ReturnStatement node) => _hasExit = true; + + @override + void visitThrowExpression(ThrowExpression node) => _hasExit = true; + + @override + void visitBreakStatement(BreakStatement node) { + if (_isLoop && _nestedLoopDepth == 0 && _nestedSwitchDepth == 0) { + _hasExit = true; + } + } + + @override + void visitContinueStatement(ContinueStatement node) { + if (_isLoop && _nestedLoopDepth == 0) { + _hasExit = true; + } + } + + @override + void visitForStatement(ForStatement node) => + _withNestedLoop(() => super.visitForStatement(node)); + + @override + void visitWhileStatement(WhileStatement node) => + _withNestedLoop(() => super.visitWhileStatement(node)); + + @override + void visitDoStatement(DoStatement node) => + _withNestedLoop(() => super.visitDoStatement(node)); + + @override + void visitSwitchStatement(SwitchStatement node) => + _withNestedSwitch(() => super.visitSwitchStatement(node)); + + @override + void visitFunctionExpression(FunctionExpression node) {} + + @override + void visitFunctionDeclaration(FunctionDeclaration node) {} + + void _withNestedLoop(void Function() visit) { + _nestedLoopDepth++; + visit(); + _nestedLoopDepth--; + } + + void _withNestedSwitch(void Function() visit) { + _nestedSwitchDepth++; + visit(); + _nestedSwitchDepth--; + } +} 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 eb30616c..0415d685 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,9 +1,9 @@ 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/models/prefer_early_return_parameters.dart'; import 'package:solid_lints/src/lints/prefer_early_return/prefer_early_return_rule.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/visitors/early_return_exit_visitor.dart'; /// Visitor for [PreferEarlyReturnRule]. class PreferEarlyReturnVisitor extends RecursiveAstVisitor { @@ -13,35 +13,73 @@ class PreferEarlyReturnVisitor extends RecursiveAstVisitor { /// The context associated with the visitor. final RuleContext context; + /// The parameters associated with the rule. + final PreferEarlyReturnParameters parameters; + /// Constructor for [PreferEarlyReturnVisitor]. PreferEarlyReturnVisitor({ required this.rule, required this.context, + required this.parameters, }); @override void visitBlockFunctionBody(BlockFunctionBody node) { super.visitBlockFunctionBody(node); - if (node.block.statements.isEmpty) return; + _checkStatements(node.block.statements, isLoop: false); + } - final (ifStatements, nextStatement) = _getIfStatementsAndNextStatement( - node, - ); - if (ifStatements.isEmpty) return; + @override + void visitForStatement(ForStatement node) { + super.visitForStatement(node); + + _checkLoopBody(node.body); + } + + @override + void visitWhileStatement(WhileStatement node) { + super.visitWhileStatement(node); + + _checkLoopBody(node.body); + } - // 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; + @override + void visitDoStatement(DoStatement node) { + super.visitDoStatement(node); + + _checkLoopBody(node.body); + } + + void _checkLoopBody(Statement body) { + final List statements = switch (body) { + Block(:final statements) => statements, + IfStatement() => [body], + _ => const [], + }; + + if (statements.isEmpty) return; - if (!nextStatementIsEmptyReturn && !nextStatementIsNull) return; + _checkStatements(statements, isLoop: true); + } - final lastIf = ifStatements.last; + void _checkStatements( + List statements, { + required bool isLoop, + }) { + final (leading, lastIf) = switch (statements) { + [...final leading, IfStatement lastIf] => (leading, lastIf), + [...final leading, IfStatement lastIf, ContinueStatement(label: null)] + when isLoop => + (leading, lastIf), + [...final leading, IfStatement lastIf, ReturnStatement(expression: null)] + when !isLoop => + (leading, lastIf), + _ => (null, null), + }; - if (lastIf case IfStatement(elseStatement: Statement())) return; - if (_hasReturnStatement(lastIf) || _hasThrowExpression(lastIf)) return; + if (lastIf == null || !leading!.every((s) => s is IfStatement)) return; + if (!_shouldReport(lastIf, isLoop: isLoop)) return; context.currentUnit?.diagnosticReporter.atNode( lastIf, @@ -49,36 +87,24 @@ class PreferEarlyReturnVisitor extends RecursiveAstVisitor { ); } - // 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?) _getIfStatementsAndNextStatement( - BlockFunctionBody body, - ) { - final List ifStatements = []; - for (final statement in body.block.statements) { - if (statement is IfStatement) { - ifStatements.add(statement); - } else { - return (ifStatements, statement); - } - } + bool _shouldReport(IfStatement ifStatement, {required bool isLoop}) { + final IfStatement(:thenStatement, :elseStatement, :caseClause) = + ifStatement; - return (ifStatements, null); - } + if (elseStatement != null || + (parameters.ignoreIfCase && caseClause != null)) { + return false; + } - bool _hasReturnStatement(Statement node) { - final visitor = ReturnStatementVisitor(); - node.accept(visitor); - return visitor.nodes.isNotEmpty; - } + final statementsCount = switch (thenStatement) { + Block(:final statements) => statements.length, + _ => 1, + }; - bool _hasThrowExpression(Statement node) { - final visitor = ThrowExpressionVisitor(); - node.accept(visitor); - return visitor.nodes.isNotEmpty; + return statementsCount > parameters.maximumStatements && + !EarlyReturnExitVisitor.hasExitIn( + thenStatement, + isLoop: isLoop, + ); } } diff --git a/lib/src/lints/prefer_early_return/visitors/return_statement_visitor.dart b/lib/src/lints/prefer_early_return/visitors/return_statement_visitor.dart deleted file mode 100644 index e1b09cc5..00000000 --- a/lib/src/lints/prefer_early_return/visitors/return_statement_visitor.dart +++ /dev/null @@ -1,16 +0,0 @@ -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 { - final _nodes = []; - - /// All unnecessary return statements - Iterable get nodes => _nodes; - - @override - void visitReturnStatement(ReturnStatement node) { - super.visitReturnStatement(node); - _nodes.add(node); - } -} diff --git a/lib/src/lints/prefer_early_return/visitors/throw_expression_visitor.dart b/lib/src/lints/prefer_early_return/visitors/throw_expression_visitor.dart deleted file mode 100644 index ceff7bd2..00000000 --- a/lib/src/lints/prefer_early_return/visitors/throw_expression_visitor.dart +++ /dev/null @@ -1,16 +0,0 @@ -import 'package:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/dart/ast/visitor.dart'; - -/// The AST visitor that will collect every Return statement -class ThrowExpressionVisitor extends RecursiveAstVisitor { - final _nodes = []; - - /// All unnecessary return statements - Iterable get nodes => _nodes; - - @override - void visitThrowExpression(ThrowExpression node) { - super.visitThrowExpression(node); - _nodes.add(node); - } -} diff --git a/test/src/lints/prefer_early_return/prefer_early_return_rule_test.dart b/test/src/lints/prefer_early_return/prefer_early_return_rule_test.dart index d6281b5e..8ab9b663 100644 --- a/test/src/lints/prefer_early_return/prefer_early_return_rule_test.dart +++ b/test/src/lints/prefer_early_return/prefer_early_return_rule_test.dart @@ -1,4 +1,6 @@ import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:analyzer_testing/utilities/utilities.dart'; +import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; import 'package:solid_lints/src/lints/prefer_early_return/prefer_early_return_rule.dart'; import 'package:test_reflective_loader/test_reflective_loader.dart'; @@ -15,15 +17,33 @@ class PreferEarlyReturnRuleTest extends AnalysisRuleTest with AutoTestLintOffsets { @override void setUp() { - rule = PreferEarlyReturnRule(); + rule = PreferEarlyReturnRule( + analysisOptionsLoader: AnalysisOptionsLoader( + resourceProvider: resourceProvider, + ), + ); super.setUp(); } + void _configureRule({int? maximumStatements, bool? ignoreIfCase}) { + newAnalysisOptionsYamlFile( + testPackageRootPath, + '''${analysisOptionsContent(rules: [rule.name])} +plugins: + solid_lints: + diagnostics: + prefer_early_return: +${maximumStatements != null ? ' maximum_statements: $maximumStatements\n' : ''}''' + '''${ignoreIfCase != null ? ' ignore_if_case: $ignoreIfCase\n' : ''}''', + ); + } + Future test_reports_if_as_only_statement_in_function() async { await assertAutoDiagnostics(''' void test(bool a) { ${expectLint('''if (a) { print('hello'); + print('world'); }''')} } '''); @@ -34,6 +54,7 @@ void test(bool a) { void test(bool a) { ${expectLint('''if (a) { print('hello'); + print('world'); }''')} return; @@ -46,6 +67,7 @@ void test(bool a) { int test(bool a) { if (a) { print('hello'); + print('world'); } return 1; @@ -60,6 +82,7 @@ void test(bool a, bool b) { if (b) { print('nested'); } + print('done'); }''')} } '''); @@ -72,6 +95,7 @@ int test(bool a, bool b) { if (b) { print('nested'); } + print('done'); } return 1; @@ -84,10 +108,11 @@ int test(bool a, bool b) { void test(bool a, bool b, bool c) { ${expectLint('''if (a) { if (b) { - if (c){ + if (c) { print('nested'); } } + print('done'); }''')} } '''); @@ -98,10 +123,11 @@ void test(bool a, bool b, bool c) { void test(bool a, bool b, bool c) { ${expectLint('''if (a) { if (b) { - if (c){ + if (c) { print('nested'); } } + print('done'); }''')} return; } @@ -113,8 +139,10 @@ void test(bool a, bool b, bool c) { void test(bool a) { if (a) { print('hello'); + print('world'); } else { print('hello'); + print('world'); } } '''); @@ -125,6 +153,7 @@ void test(bool a) { void test(bool a) { if (a) { print('hello'); + print('world'); } else { return; } @@ -136,11 +165,13 @@ void test(bool a) { await assertNoDiagnostics(r''' void test(bool a, bool b) { if (a) { - if(b){ + if (b) { print('hello'); + print('world'); } } else { print('hello'); + print('world'); } } '''); @@ -150,13 +181,15 @@ void test(bool a, bool b) { await assertAutoDiagnostics(''' void test(bool a, bool b) { ${expectLint('''if (a) { - if(b){ + if (b) { print('hello'); - } - else { + print('world'); + } else { print('hello'); + print('world'); } - }''')} + print('done'); + }''')} } '''); } @@ -168,8 +201,10 @@ void threeIf(bool a, bool b, bool c) { if (b) { if (c) { print('hello'); + print('world'); } } + print('done'); }''')} }'''); } @@ -179,12 +214,14 @@ void threeIf(bool a, bool b, bool c) { void test(bool a, bool b, bool c) { if (a) { if (b) { - if (c){ + if (c) { print('nested'); + print('done'); } } - } else{ + } else { print('hello'); + print('world'); } } '''); @@ -197,10 +234,13 @@ void test(bool a, bool b, bool c) { if (b) { if (c) { print('nested'); + print('done'); } } else { print('nested'); + print('done'); } + print('done'); }''')} } '''); @@ -213,11 +253,13 @@ void test(bool a, bool b, bool c) { if (b) { if (c) { print('nested'); - } - else { + print('done'); + } else { print('nested'); + print('done'); } - } + } + print('done'); }''')} } '''); @@ -228,7 +270,8 @@ void test(bool a, bool b, bool c) { void test(bool a, bool b) { if (a) return; ${expectLint('''if (b) { - print('gello'); + print('hello'); + print('world'); }''')} } '''); @@ -251,6 +294,7 @@ void test(bool a, bool b) { if (a) return; ${expectLint('''if (b) { print('hello'); + print('world'); }''')} return; @@ -265,6 +309,7 @@ void test(bool a, bool b) { if (a) return; if (b) { print('hello'); + print('world'); } print('after'); @@ -277,9 +322,11 @@ void test(bool a, bool b) { void test(bool a, bool b) { if (a) { print('hello'); + print('world'); } ${expectLint('''if (b) { print('hello'); + print('world'); }''')} } '''); @@ -292,6 +339,7 @@ void test(bool a, bool b, bool c) { if (b) return; ${expectLint('''if (c) { print('hello'); + print('world'); }''')} return; @@ -305,9 +353,11 @@ void test(bool a, bool b, bool c) { if (a) return; if (b) { print('hello'); + print('world'); } ${expectLint('''if (c) { print('hello'); + print('world'); }''')} } '''); @@ -330,6 +380,7 @@ void test(bool a) { void test(bool a) { if (a) { print('hello'); + print('world'); } else { throw ''; } @@ -343,6 +394,7 @@ void test(bool a, bool b) { if (a) throw ''; ${expectLint('''if (b) { print('hello'); + print('world'); }''')} } '''); @@ -354,6 +406,7 @@ void test(bool a, bool b) { if (a) throw ''; ${expectLint('''if (b) { print('hello'); + print('world'); }''')} return; @@ -368,6 +421,7 @@ void test(bool a, bool b, bool c) { if (b) throw ''; ${expectLint('''if (c) { print('hello'); + print('world'); }''')} return; @@ -381,9 +435,309 @@ void test(bool a, bool b, bool c) { if (a) throw ''; if (b) { print('hello'); + print('world'); } ${expectLint('''if (c) { print('hello'); + print('world'); + }''')} +} +'''); + } + + // --- Tests for maximum_statements parameter --- + + Future test_does_not_report_single_statement_by_default() async { + await assertNoDiagnostics(r''' +void test(bool a) { + if (a) { + print('hello'); + } +} +'''); + } + + Future + test_reports_single_statement_when_maximum_statements_zero() async { + _configureRule(maximumStatements: 0); + await assertAutoDiagnostics(''' +void test(bool a) { + ${expectLint('''if (a) { + print('hello'); + }''')} +} +'''); + } + + Future + test_does_not_report_two_statements_when_maximum_statements_two() async { + _configureRule(maximumStatements: 2); + await assertNoDiagnostics(r''' +void test(bool a) { + if (a) { + print('one'); + print('two'); + } +} +'''); + } + + Future + test_reports_three_statements_when_maximum_statements_two() async { + _configureRule(maximumStatements: 2); + await assertAutoDiagnostics(''' +void test(bool a) { + ${expectLint('''if (a) { + print('one'); + print('two'); + print('three'); + }''')} +} +'''); + } + + // --- Tests for ignore_if_case parameter --- + + Future test_does_not_report_if_case_by_default() async { + await assertNoDiagnostics(r''' +void test(Object? value) { + if (value case String s) { + print(s); + print('done'); + } +} +'''); + } + + Future test_reports_if_case_when_ignore_if_case_false() async { + _configureRule(ignoreIfCase: false); + await assertAutoDiagnostics(''' +void test(Object? value) { + ${expectLint('''if (value case String s) { + print(s); + print('done'); + }''')} +} +'''); + } + + // --- Tests for loop statements --- + + Future test_reports_for_in_loop() async { + await assertAutoDiagnostics(''' +void test(List items) { + for (final item in items) { + ${expectLint('''if (item.isNotEmpty) { + print(item); + print('done'); + }''')} + } +} +'''); + } + + Future test_reports_traditional_for_loop() async { + await assertAutoDiagnostics(''' +void test(List items) { + for (var i = 0; i < items.length; i++) { + ${expectLint('''if (items[i].isNotEmpty) { + print(items[i]); + print('done'); + }''')} + } +} +'''); + } + + Future test_reports_while_loop() async { + await assertAutoDiagnostics(''' +void test(bool condition, bool ready) { + while (condition) { + ${expectLint('''if (ready) { + print('ready'); + print('go'); + }''')} + } +} +'''); + } + + Future test_reports_do_while_loop() async { + await assertAutoDiagnostics(''' +void test(bool condition, bool ready) { + do { + ${expectLint('''if (ready) { + print('ready'); + print('go'); + }''')} + } while (condition); +} +'''); + } + + Future test_reports_loop_with_trailing_continue() async { + await assertAutoDiagnostics(''' +void test(List items) { + for (final item in items) { + ${expectLint('''if (item.isNotEmpty) { + print(item); + print('done'); + }''')} + continue; + } +} +'''); + } + + Future test_does_not_report_loop_with_statement_after_if() async { + await assertNoDiagnostics(r''' +void test(List items) { + for (final item in items) { + if (item.isNotEmpty) { + print(item); + print('done'); + } + print('after'); + } +} +'''); + } + + Future test_does_not_report_loop_with_if_else() async { + await assertNoDiagnostics(r''' +void test(List items) { + for (final item in items) { + if (item.isNotEmpty) { + print(item); + print('done'); + } else { + print('empty'); + print('done'); + } + } +} +'''); + } + + Future test_does_not_report_loop_with_continue_in_if() async { + await assertNoDiagnostics(r''' +void test(List items) { + for (final item in items) { + if (item.isNotEmpty) { + print(item); + continue; + } + } +} +'''); + } + + Future test_does_not_report_loop_with_break_in_if() async { + await assertNoDiagnostics(r''' +void test(List items) { + for (final item in items) { + if (item.isNotEmpty) { + print(item); + break; + } + } +} +'''); + } + + Future test_reports_if_with_closure_containing_return() async { + await assertAutoDiagnostics(''' +void test(bool a, List items) { + ${expectLint('''if (a) { + final doubled = items.map((x) { + return x * 2; + }); + print(doubled); + }''')} +} +'''); + } + + Future test_reports_loop_with_nested_loop_containing_break() async { + await assertAutoDiagnostics(''' +void test(List items, List numbers) { + for (final item in items) { + ${expectLint('''if (item.isNotEmpty) { + for (final n in numbers) { + if (n > 0) break; + } + print(item); + }''')} + } +} +'''); + } + + Future test_reports_loop_with_switch_containing_break() async { + await assertAutoDiagnostics(''' +void test(List items) { + for (final item in items) { + ${expectLint('''if (item > 0) { + switch (item) { + case 1: + break; + } + print(item); + }''')} + } +} +'''); + } + + Future + test_does_not_report_loop_with_switch_containing_continue() async { + await assertNoDiagnostics(r''' +void test(List items) { + for (final item in items) { + if (item > 0) { + switch (item) { + case 1: + continue; + } + print(item); + } + } +} +'''); + } + + Future test_reports_loop_with_single_if_body() async { + await assertAutoDiagnostics(''' +void test(List items) { + for (final item in items) + ${expectLint('''if (item.isNotEmpty) { + print(item); + print('done'); + }''')} +} +'''); + } + + Future test_reports_async_function() async { + await assertAutoDiagnostics(''' +Future test(bool a) async { + ${expectLint('''if (a) { + print('hello'); + print('world'); + }''')} +} +'''); + } + + Future test_reports_if_with_local_function_containing_return() async { + await assertAutoDiagnostics(''' +void test(bool a) { + ${expectLint('''if (a) { + void helper() { + return; + } + helper(); + print('done'); }''')} } '''); From f97648cc310b07445b4fffd4b663a8acf3cee987 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Fri, 28 Aug 2026 17:29:41 +0300 Subject: [PATCH 2/3] refactor: switch to SimpleAstVisitor and remove redundant super calls in PreferEarlyReturnVisitor while using cascade notation for visitor registration --- .../prefer_early_return/prefer_early_return_rule.dart | 9 +++++---- .../visitors/prefer_early_return_visitor.dart | 10 +--------- 2 files changed, 6 insertions(+), 13 deletions(-) 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 a6d28c29..350ab890 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 @@ -96,9 +96,10 @@ class PreferEarlyReturnRule extends SolidLintRule { parameters: parameters, ); - registry.addBlockFunctionBody(this, visitor); - registry.addForStatement(this, visitor); - registry.addWhileStatement(this, visitor); - registry.addDoStatement(this, visitor); + registry + ..addBlockFunctionBody(this, visitor) + ..addForStatement(this, visitor) + ..addWhileStatement(this, visitor) + ..addDoStatement(this, visitor); } } 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 0415d685..11d9748d 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 @@ -6,7 +6,7 @@ import 'package:solid_lints/src/lints/prefer_early_return/prefer_early_return_ru import 'package:solid_lints/src/lints/prefer_early_return/visitors/early_return_exit_visitor.dart'; /// Visitor for [PreferEarlyReturnRule]. -class PreferEarlyReturnVisitor extends RecursiveAstVisitor { +class PreferEarlyReturnVisitor extends SimpleAstVisitor { /// The rule associated with the visitor. final PreferEarlyReturnRule rule; @@ -25,29 +25,21 @@ class PreferEarlyReturnVisitor extends RecursiveAstVisitor { @override void visitBlockFunctionBody(BlockFunctionBody node) { - super.visitBlockFunctionBody(node); - _checkStatements(node.block.statements, isLoop: false); } @override void visitForStatement(ForStatement node) { - super.visitForStatement(node); - _checkLoopBody(node.body); } @override void visitWhileStatement(WhileStatement node) { - super.visitWhileStatement(node); - _checkLoopBody(node.body); } @override void visitDoStatement(DoStatement node) { - super.visitDoStatement(node); - _checkLoopBody(node.body); } From aab4c157a78f8e05cd201434abc1b4170b814cfc Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Fri, 28 Aug 2026 18:12:51 +0300 Subject: [PATCH 3/3] refactor: improve loop exit detection in prefer_early_return by checking control flow statement targets against root scope --- .../visitors/early_return_exit_visitor.dart | 59 ++++------ .../visitors/prefer_early_return_visitor.dart | 32 ++---- .../prefer_early_return_rule_test.dart | 107 ++++++++++++++++++ 3 files changed, 139 insertions(+), 59 deletions(-) diff --git a/lib/src/lints/prefer_early_return/visitors/early_return_exit_visitor.dart b/lib/src/lints/prefer_early_return/visitors/early_return_exit_visitor.dart index f08b28cc..9fd5ebb7 100644 --- a/lib/src/lints/prefer_early_return/visitors/early_return_exit_visitor.dart +++ b/lib/src/lints/prefer_early_return/visitors/early_return_exit_visitor.dart @@ -4,16 +4,14 @@ import 'package:analyzer/dart/ast/visitor.dart'; /// AST visitor that checks if a statement contains an early exit /// (return, throw, or loop break/continue) respecting scope boundaries. class EarlyReturnExitVisitor extends RecursiveAstVisitor { - final bool _isLoop; + final Statement _root; bool _hasExit = false; - int _nestedLoopDepth = 0; - int _nestedSwitchDepth = 0; - EarlyReturnExitVisitor._(this._isLoop); + EarlyReturnExitVisitor._(this._root); /// Checks whether [node] contains any early exit statement. - static bool hasExitIn(Statement node, {required bool isLoop}) { - final visitor = EarlyReturnExitVisitor._(isLoop); + static bool hasExitIn(Statement node) { + final visitor = EarlyReturnExitVisitor._(node); node.accept(visitor); return visitor._hasExit; } @@ -25,34 +23,14 @@ class EarlyReturnExitVisitor extends RecursiveAstVisitor { void visitThrowExpression(ThrowExpression node) => _hasExit = true; @override - void visitBreakStatement(BreakStatement node) { - if (_isLoop && _nestedLoopDepth == 0 && _nestedSwitchDepth == 0) { - _hasExit = true; - } - } - - @override - void visitContinueStatement(ContinueStatement node) { - if (_isLoop && _nestedLoopDepth == 0) { - _hasExit = true; - } - } + void visitRethrowExpression(RethrowExpression node) => _hasExit = true; @override - void visitForStatement(ForStatement node) => - _withNestedLoop(() => super.visitForStatement(node)); + void visitBreakStatement(BreakStatement node) => _checkExit(node.target); @override - void visitWhileStatement(WhileStatement node) => - _withNestedLoop(() => super.visitWhileStatement(node)); - - @override - void visitDoStatement(DoStatement node) => - _withNestedLoop(() => super.visitDoStatement(node)); - - @override - void visitSwitchStatement(SwitchStatement node) => - _withNestedSwitch(() => super.visitSwitchStatement(node)); + void visitContinueStatement(ContinueStatement node) => + _checkExit(node.target); @override void visitFunctionExpression(FunctionExpression node) {} @@ -60,15 +38,20 @@ class EarlyReturnExitVisitor extends RecursiveAstVisitor { @override void visitFunctionDeclaration(FunctionDeclaration node) {} - void _withNestedLoop(void Function() visit) { - _nestedLoopDepth++; - visit(); - _nestedLoopDepth--; + bool _isDescendantOfRoot(AstNode target) { + AstNode? current = target; + while (current != null) { + if (identical(current, _root)) { + return true; + } + current = current.parent; + } + return false; } - void _withNestedSwitch(void Function() visit) { - _nestedSwitchDepth++; - visit(); - _nestedSwitchDepth--; + void _checkExit(AstNode? target) { + if (target case final target? when !_isDescendantOfRoot(target)) { + _hasExit = true; + } } } 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 11d9748d..e855fad4 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 @@ -24,24 +24,17 @@ class PreferEarlyReturnVisitor extends SimpleAstVisitor { }); @override - void visitBlockFunctionBody(BlockFunctionBody node) { - _checkStatements(node.block.statements, isLoop: false); - } + void visitBlockFunctionBody(BlockFunctionBody node) => + _checkStatements(node.block.statements, isLoop: false); @override - void visitForStatement(ForStatement node) { - _checkLoopBody(node.body); - } + void visitForStatement(ForStatement node) => _checkLoopBody(node.body); @override - void visitWhileStatement(WhileStatement node) { - _checkLoopBody(node.body); - } + void visitWhileStatement(WhileStatement node) => _checkLoopBody(node.body); @override - void visitDoStatement(DoStatement node) { - _checkLoopBody(node.body); - } + void visitDoStatement(DoStatement node) => _checkLoopBody(node.body); void _checkLoopBody(Statement body) { final List statements = switch (body) { @@ -50,9 +43,9 @@ class PreferEarlyReturnVisitor extends SimpleAstVisitor { _ => const [], }; - if (statements.isEmpty) return; - - _checkStatements(statements, isLoop: true); + if (statements.isNotEmpty) { + _checkStatements(statements, isLoop: true); + } } void _checkStatements( @@ -71,7 +64,7 @@ class PreferEarlyReturnVisitor extends SimpleAstVisitor { }; if (lastIf == null || !leading!.every((s) => s is IfStatement)) return; - if (!_shouldReport(lastIf, isLoop: isLoop)) return; + if (!_shouldReport(lastIf)) return; context.currentUnit?.diagnosticReporter.atNode( lastIf, @@ -79,7 +72,7 @@ class PreferEarlyReturnVisitor extends SimpleAstVisitor { ); } - bool _shouldReport(IfStatement ifStatement, {required bool isLoop}) { + bool _shouldReport(IfStatement ifStatement) { final IfStatement(:thenStatement, :elseStatement, :caseClause) = ifStatement; @@ -94,9 +87,6 @@ class PreferEarlyReturnVisitor extends SimpleAstVisitor { }; return statementsCount > parameters.maximumStatements && - !EarlyReturnExitVisitor.hasExitIn( - thenStatement, - isLoop: isLoop, - ); + !EarlyReturnExitVisitor.hasExitIn(thenStatement); } } diff --git a/test/src/lints/prefer_early_return/prefer_early_return_rule_test.dart b/test/src/lints/prefer_early_return/prefer_early_return_rule_test.dart index 8ab9b663..a5e64861 100644 --- a/test/src/lints/prefer_early_return/prefer_early_return_rule_test.dart +++ b/test/src/lints/prefer_early_return/prefer_early_return_rule_test.dart @@ -375,6 +375,22 @@ void test(bool a) { '''); } + Future test_does_not_report_if_rethrow_with_return() async { + await assertNoDiagnostics(r''' +void test(bool a) { + try { + print('hello'); + } catch (_) { + if (a) { + rethrow; + } + + return; + } +} +'''); + } + Future test_does_not_report_if_else_throw() async { await assertNoDiagnostics(r''' void test(bool a) { @@ -645,6 +661,23 @@ void test(List items) { '''); } + Future test_does_not_report_loop_with_rethrow_in_if() async { + await assertNoDiagnostics(r''' +void test(List items) { + for (final item in items) { + try { + print('hello'); + } catch (_) { + if (item.isNotEmpty) { + print(item); + rethrow; + } + } + } +} +'''); + } + Future test_reports_if_with_closure_containing_return() async { await assertAutoDiagnostics(''' void test(bool a, List items) { @@ -740,6 +773,80 @@ void test(bool a) { print('done'); }''')} } +'''); + } + + // --- Labeled break/continue edge cases --- + + Future + test_reports_loop_with_switch_containing_continue_to_case() async { + await assertAutoDiagnostics(''' +void test(List items) { + for (final item in items) { + ${expectLint('''if (item > 0) { + switch (item) { + case 1: + continue target; + target: + case 2: + print('two'); + } + print(item); + }''')} + } +} +'''); + } + + Future + test_does_not_report_loop_with_nested_loop_containing_continue_outer() async { + await assertNoDiagnostics(r''' +void test(List items, List numbers) { + outer: + for (final item in items) { + if (item.isNotEmpty) { + for (final n in numbers) { + continue outer; + } + print(item); + } + } +} +'''); + } + + Future + test_does_not_report_loop_with_nested_loop_containing_break_outer() async { + await assertNoDiagnostics(r''' +void test(List items, List numbers) { + outer: + for (final item in items) { + if (item.isNotEmpty) { + for (final n in numbers) { + break outer; + } + print(item); + } + } +} +'''); + } + + Future + test_does_not_report_loop_with_switch_containing_break_outer() async { + await assertNoDiagnostics(r''' +void test(List items) { + outer: + for (final item in items) { + if (item > 0) { + switch (item) { + case 1: + break outer; + } + print(item); + } + } +} '''); } }