adapt to two sequential if statements

illia-romanenko · Apr 22, 2024 · Mar 27, 2024 · Mar 28, 2024 · Mar 28, 2024 · Mar 28, 2024
commit c8fec9fdccb0efd025f704837cf484218f8bf0ce
diff --git a/lib/src/lints/prefer_early_return/visitors/if_statement_visitor.dart b/lib/src/lints/prefer_early_return/visitors/if_statement_visitor.dart
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
@@ -1,6 +1,6 @@
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
-import 'package:solid_lints/src/lints/prefer_early_return/visitors/if_statement_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> {
@@ -12,34 +12,53 @@ class PreferEarlyReturnVisitor extends RecursiveAstVisitor<void> {
   @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;
-    }
+    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 statementis empty
+    final nextStatementIsEmptyReturn =
+        nextStatement is ReturnStatement && nextStatement.expression == null;
+    final nextStatementIsNull = nextStatement == null;
+
+    if (!(nextStatementIsEmptyReturn || nextStatementIsNull)) return;
+
+    // limit to only handling cases with two ifs
+    if (ifStatements.length > 2) return;
+
+    _handleIfStatement(ifStatements.last);
   }
 
   void _handleIfStatement(IfStatement node) {
     if (_isElseIfStatement(node)) return;
     if (_hasElseStatement(node)) return;
-    final containsNestedIfStatement = _containsIfStatement(node.thenStatement);
-    if (!containsNestedIfStatement) 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;
 }
@@ -48,8 +67,8 @@ bool _isElseIfStatement(IfStatement node) {
   return node.elseStatement != null && node.elseStatement is IfStatement;
 }
 
-bool _containsIfStatement(Statement node) {
-  final visitor = IfStatementVisitor();
+bool _hasReturnStatement(Statement node) {
+  final visitor = ReturnStatementVisitor();
   node.accept(visitor);
   return visitor.nodes.isNotEmpty;
 }
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
@@ -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);
+  }
+}
diff --git a/lint_test/prefer_early_return_test.dart b/lint_test/prefer_early_return_test.dart
@@ -4,14 +4,23 @@
 void _doSomething() {}
 
 void oneIf() {
+  //expect_lint: prefer_early_return
+  if (true) {
+    _doSomething();
+  }
+}
+
+int oneIfWithReturnValue() {
   //no lint
   if (true) {
     _doSomething();
   }
+
+  return 1;
 }
 
 void oneIfWithReturn() {
-  //no lint
+  //expect_lint: prefer_early_return
   if (true) {
     _doSomething();
   }
@@ -28,6 +37,17 @@ void nestedIf2() {
   }
 }
 
+int nestedIf2WithReturnValue() {
+  //no lint
+  if (true) {
+    if (true) {
+      _doSomething();
+    }
+  }
+
+  return 1;
+}
+
 void nestedIf3() {
   //expect_lint: prefer_early_return
   if (true) {
@@ -141,3 +161,52 @@ void threeIfElse3() {
     }
   }
 }
+
+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();
+  }
+}