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 4f420de0..582378df 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 @@ -32,23 +32,20 @@ import 'package:solid_lints/src/lints/prefer_early_return/visitors/prefer_early_ /// } /// ``` class PreferEarlyReturnRule extends AnalysisRule { - /// The name of the lint + /// Lint name static const String lintName = 'prefer_early_return'; - /// The message shown when the lint is triggered - static const String lintMessage = 'Use reverse if to reduce nesting'; - /// Lint code static const LintCode _code = LintCode( lintName, - lintMessage, + "Use reverse if to reduce nesting", ); - /// Creates a new instance of [PreferEarlyReturnRule] + /// Creates an instance of [PreferEarlyReturnRule] PreferEarlyReturnRule() : super( name: lintName, - description: lintMessage, + description: 'Use reverse if to reduce nesting', ); @override @@ -59,7 +56,11 @@ class PreferEarlyReturnRule extends AnalysisRule { RuleVisitorRegistry registry, RuleContext context, ) { - final visitor = PreferEarlyReturnVisitor(this); + final visitor = PreferEarlyReturnVisitor( + rule: this, + context: context, + ); + registry.addBlockFunctionBody(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 a8342ef5..eb30616c 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,3 +1,4 @@ +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/prefer_early_return_rule.dart'; @@ -6,11 +7,17 @@ import 'package:solid_lints/src/lints/prefer_early_return/visitors/throw_express /// Visitor for [PreferEarlyReturnRule]. class PreferEarlyReturnVisitor extends RecursiveAstVisitor { - /// The rule associated with this visitor. + /// The rule associated with the visitor. final PreferEarlyReturnRule rule; - /// Creates an instance of [PreferEarlyReturnVisitor]. - PreferEarlyReturnVisitor(this.rule); + /// The context associated with the visitor. + final RuleContext context; + + /// Constructor for [PreferEarlyReturnVisitor]. + PreferEarlyReturnVisitor({ + required this.rule, + required this.context, + }); @override void visitBlockFunctionBody(BlockFunctionBody node) { @@ -18,7 +25,9 @@ class PreferEarlyReturnVisitor extends RecursiveAstVisitor { if (node.block.statements.isEmpty) return; - final (ifStatements, nextStatement) = _getStartIfStatements(node); + final (ifStatements, nextStatement) = _getIfStatementsAndNextStatement( + node, + ); if (ifStatements.isEmpty) return; // limit visitor to only work with functions @@ -29,25 +38,24 @@ class PreferEarlyReturnVisitor extends RecursiveAstVisitor { if (!nextStatementIsEmptyReturn && !nextStatementIsNull) return; - _handleIfStatement(ifStatements.last); - } + final lastIf = ifStatements.last; - void _handleIfStatement(IfStatement node) { - if (_isElseIfStatement(node)) return; - if (_hasElseStatement(node)) return; - if (_hasReturnStatement(node)) return; - if (_hasThrowExpression(node)) return; + if (lastIf case IfStatement(elseStatement: Statement())) return; + if (_hasReturnStatement(lastIf) || _hasThrowExpression(lastIf)) return; - rule.reportAtNode(node); + context.currentUnit?.diagnosticReporter.atNode( + lastIf, + rule.diagnosticCode, + ); } -// 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( + // 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 = []; @@ -58,15 +66,8 @@ class PreferEarlyReturnVisitor extends RecursiveAstVisitor { 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; + return (ifStatements, null); } bool _hasReturnStatement(Statement node) { 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/pubspec.yaml b/pubspec.yaml index f85b1309..801124c4 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -17,9 +17,6 @@ dependencies: glob: ^2.1.3 path: ^1.9.1 yaml: ^3.1.3 - # These packages are required for pana analysis to run correctly - flutter: - sdk: flutter test: ^1.25.14 dev_dependencies: diff --git a/test/prefer_early_return_rule_test.dart b/test/prefer_early_return_rule_test.dart new file mode 100644 index 00000000..895b91cd --- /dev/null +++ b/test/prefer_early_return_rule_test.dart @@ -0,0 +1,425 @@ +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; + + Future test_reports_on_one_if() async { + await assertDiagnostics( + r''' +void oneIf(bool a) { + if (a) { + print('s'); + } +}''', + [lint(23, 28)], + ); + } + + Future test_doesn_not_report_on_one_if_with_return_value() async { + await assertNoDiagnostics(r''' +int oneIfWithReturnValue(bool a) { + if (a) { + print('s'); + } + + return 1; +}'''); + } + + Future test_reports_on_one_if_with_return() async { + await assertDiagnostics( + r''' +void oneIfWithReturn(bool a) { + if (a) { + print('s'); + } + + return; +}''', + [lint(33, 28)], + ); + } + + Future test_reports_on_nested_if2() async { + await assertDiagnostics( + r''' +void nestedIf2(bool a, bool b) { + if (a) { + if (b) { + print('s'); + } + } +}''', + [lint(35, 49)], + ); + } + + Future test_doesn_not_report_on_nested_if2_with_return_value() async { + await assertNoDiagnostics(r''' +int nestedIf2WithReturnValue(bool a, bool b) { + if (a) { + if (b) { + print('s'); + } + } + + return 1; +}'''); + } + + Future test_reports_on_nested_if3() async { + await assertDiagnostics( + r''' +void nestedIf3(bool a, bool b, bool c) { + if (a) { + if (b) { + if (c) { + print('s'); + } + } + } +}''', + [lint(43, 74)], + ); + } + + Future test_reports_on_one_nested_if2_with_return() async { + await assertDiagnostics( + r''' +void oneNestedIf2WithReturn(bool a, bool b, bool c) { + if (a) { + if (b) { + if (c) { + print('s'); + } + } + } + + return; +}''', + [lint(56, 74)], + ); + } + + Future test_doesn_not_report_on_one_if_else() async { + await assertNoDiagnostics(r''' +void oneIfElse(bool a) { + if (a) { + print('s'); + } else { + print('s'); + } +}'''); + } + + Future test_doesn_not_report_on_one_if_else_return() async { + await assertNoDiagnostics(r''' +void oneIfElseReturn(bool a) { + if (a) { + print('s'); + } else { + return; + } +}'''); + } + + Future test_doesn_not_report_on_two_if_else() async { + await assertNoDiagnostics(r''' +void twoIfElse(bool a, bool b) { + if (a) { + if (b) { + print('s'); + } + } else { + print('s'); + } +}'''); + } + + Future test_reports_on_two_if_else_inner() async { + await assertDiagnostics( + r''' +void twoIfElseInner(bool a, bool b) { + if (a) { + if (b) { + print('s'); + } else { + print('s'); + } + } +}''', + [lint(40, 80)], + ); + } + + Future test_reports_on_three_if() async { + await assertDiagnostics( + r''' +void threeIf(bool a, bool b, bool c) { + if (a) { + if (b) { + if (c) { + print('s'); + } + } + } +}''', + [lint(41, 74)], + ); + } + + Future test_doesn_not_report_on_three_if_else1() async { + await assertNoDiagnostics(r''' +void threeIfElse1(bool a, bool b, bool c) { + if (a) { + if (b) { + if (c) { + print('s'); + } + } + } else { + print('s'); + } +}'''); + } + + Future test_reports_on_three_if_else2() async { + await assertDiagnostics( + r''' +void threeIfElse2(bool a, bool b, bool c) { + if (a) { + if (b) { + if (c) { + print('s'); + } + } else { + print('s'); + } + } +}''', + [lint(46, 105)], + ); + } + + Future test_reports_on_three_if_else3() async { + await assertDiagnostics( + r''' +void threeIfElse3(bool a, bool b, bool c) { + if (a) { + if (b) { + if (c) { + print('s'); + } else { + print('s'); + } + } + } +}''', + [lint(46, 109)], + ); + } + + Future test_reports_on_two_seqential_if() async { + await assertDiagnostics( + r''' +void twoSeqentialIf(bool a, bool b) { + if (a) return; + if (b) { + print('s'); + } +}''', + [lint(57, 28)], + ); + } + + Future test_doesn_not_report_on_two_seqential_if_return() async { + await assertNoDiagnostics(r''' +void twoSeqentialIfReturn(bool a, bool b) { + if (a) return; + if (b) return; + + return; +}'''); + } + + Future test_reports_on_two_seqential_if_return2() async { + await assertDiagnostics( + r''' +void twoSeqentialIfReturn2(bool a, bool b) { + //no lint + if (a) return; + if (b) { + print('s'); + } + + return; +}''', + [lint(76, 28)], + ); + } + + Future test_doesn_not_report_on_two_seqential_if_something() async { + await assertNoDiagnostics(r''' +void twoSeqentialIfSomething(bool a, bool b) { + if (a) return; + if (b) { + print('s'); + } + + print('s'); +}'''); + } + + Future test_reports_on_two_seqential_if_something2() async { + await assertDiagnostics( + r''' +void twoSeqentialIfSomething2(bool a, bool b) { + //no lint + if (a) { + print('s'); + } + if (b) { + print('s'); + } +}''', + [lint(93, 28)], + ); + } + + Future test_reports_on_three_seqential_if_return() async { + await assertDiagnostics( + r''' +void threeSeqentialIfReturn(bool a, bool b, bool c) { + //no lint + if (a) return; + if (b) return; + if (c) { + print('s'); + } + + return; +}''', + [lint(102, 28)], + ); + } + + Future test_reports_on_three_seqential_if_return2() async { + await assertDiagnostics( + r''' +void threeSeqentialIfReturn2(bool a, bool b, bool c) { + //no lint + if (a) return; + //no lint + if (b) { + print('s'); + } + if (c) { + print('s'); + } +}''', + [lint(129, 28)], + ); + } + + Future test_doesn_not_report_on_one_if_with_throw_with_return() async { + await assertNoDiagnostics(r''' +void oneIfWithThrowWithReturn(bool a) { + if (a) { + throw ''; + } + + return; +}'''); + } + + Future test_doesn_not_report_on_one_if_else_with_throw_return() async { + await assertNoDiagnostics(r''' +void oneIfElseWithThrowReturn(bool a) { + if (a) { + print('s'); + } else { + throw ''; + } +}'''); + } + + Future test_reports_on_two_seqential_if_with_throw() async { + await assertDiagnostics( + r''' +void twoSeqentialIfWithThrow(bool a, bool b) { + if (a) throw ''; + if (b) { + print('s'); + } +}''', + [lint(68, 28)], + ); + } + + Future test_reports_on_two_seqential_if_with_throw_return2() async { + await assertDiagnostics( + r''' +void twoSeqentialIfWithThrowReturn2(bool a, bool b) { + //no lint + if (a) throw ''; + if (b) { + print('s'); + } + + return; +}''', + [lint(87, 28)], + ); + } + + Future test_reports_on_three_seqential_if_with_throw_return() async { + await assertDiagnostics( + r''' +void threeSeqentialIfWithThrowReturn(bool a, bool b, bool c) { + //no lint + if (a) throw ''; + if (b) throw ''; + if (c) { + print('s'); + } + + return; +}''', + [lint(115, 28)], + ); + } + + Future test_reports_on_three_seqential_if_with_throw_return2() async { + await assertDiagnostics( + r''' +void threeSeqentialIfWithThrowReturn2(bool a, bool b, bool c) { + //no lint + if (a) throw ''; + //no lint + if (b) { + print('s'); + } + if (c) { + print('s'); + } +}''', + [lint(140, 28)], + ); + } +}