Remove NEXT_NODE dependency in ChangeAndIfToEarlyReturnRector, narrow to only closed scope function likes

packages/NodeNestingScope/ContextAnalyzer.php

@@ -5,18 +5,12 @@
 namespace Rector\NodeNestingScope;
 
 use PhpParser\Node;
-use PhpParser\Node\Expr;
-use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\FunctionLike;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\If_;
-use PhpParser\Node\Stmt\Return_;
 use PhpParser\Node\Stmt\Switch_;
-use PHPStan\Type\ObjectType;
 use Rector\Core\PhpParser\Node\BetterNodeFinder;
 use Rector\NodeNestingScope\ValueObject\ControlStructure;
-use Rector\NodeTypeResolver\Node\AttributeKey;
-use Rector\NodeTypeResolver\NodeTypeResolver;
 
 final class ContextAnalyzer
 {
@@ -28,7 +22,6 @@ final class ContextAnalyzer
 
     public function __construct(
         private readonly BetterNodeFinder $betterNodeFinder,
-        private readonly NodeTypeResolver $nodeTypeResolver,
     ) {
     }
 
@@ -71,33 +64,4 @@ public function isInIf(Node $node): bool
 
         return $previousNode instanceof If_;
     }
-
-    public function hasAssignWithIndirectReturn(Node $node, If_ $if): bool
-    {
-        foreach (ControlStructure::LOOP_NODES as $loopNode) {
-            $loopObjectType = new ObjectType($loopNode);
-            $parentType = $this->nodeTypeResolver->getType($node);
-
-            $superType = $parentType->isSuperTypeOf($loopObjectType);
-            if (! $superType->yes()) {
-                continue;
-            }
-
-            $nextNode = $node->getAttribute(AttributeKey::NEXT_NODE);
-            if ($nextNode instanceof Node) {
-                if ($nextNode instanceof Return_ && ! $nextNode->expr instanceof Expr) {
-                    continue;
-                }
-
-                $hasAssign = (bool) $this->betterNodeFinder->findFirstInstanceOf($if->stmts, Assign::class);
-                if (! $hasAssign) {
-                    continue;
-                }
-
-                return true;
-            }
-        }
-
-        return false;
-    }
 }

phpstan.neon

@@ -766,3 +766,4 @@ parameters:
         - '#Anonymous variable in a `\$stmtsAware\->stmts\[\$key\]\->\.\.\.\(\)` method call can lead to false dead methods\. Make sure the variable type is known#'
 
         - '#Cognitive complexity for "Rector\\TypeDeclaration\\Rector\\ClassMethod\\BoolReturnTypeFromStrictScalarReturnsRector\:\:hasOnlyBoolScalarReturnExprs\(\)" is 11, keep it under 10#'
+        - '#Access to an undefined property PhpParser\\Node\\Stmt\\ClassLike\|PhpParser\\Node\\Stmt\\Declare_\|Rector\\Core\\Contract\\PhpParser\\Node\\StmtsAwareInterface\:\:\$stmts#'

rules-tests/EarlyReturn/Rector/If_/ChangeAndIfToEarlyReturnRector/Fixture/assign_in_loop_with_continue.php.inc

@@ -0,0 +1,59 @@
+<?php
+
+namespace Rector\Tests\EarlyReturn\Rector\If_\ChangeAndIfToEarlyReturnRector\Fixture;
+
+final class AssignInLoopWithContinue
+{
+    public function updateArtworkStatus(): Release
+    {
+        // Assume we start completed
+        $status = Release::ARTWORK_STATUS_COMPLETE;
+        $artwork = new Artwork($this);
+        foreach ($artwork->specs as $key => $pdf) {
+            if ($this->shouldHaveArtwork($key) && !$this->hasArtwork($key)) {
+                $status = Release::ARTWORK_STATUS_INCOMPLETE;
+            }
+        }
+
+        if ($this->artwork_status != $status) {
+            $this->artwork_status = $status;
+            $this->artwork_status_message = null;
+        }
+
+        return $this;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\EarlyReturn\Rector\If_\ChangeAndIfToEarlyReturnRector\Fixture;
+
+final class AssignInLoopWithContinue
+{
+    public function updateArtworkStatus(): Release
+    {
+        // Assume we start completed
+        $status = Release::ARTWORK_STATUS_COMPLETE;
+        $artwork = new Artwork($this);
+        foreach ($artwork->specs as $key => $pdf) {
+            if (!$this->shouldHaveArtwork($key)) {
+                continue;
+            }
+            if ($this->hasArtwork($key)) {
+                continue;
+            }
+            $status = Release::ARTWORK_STATUS_INCOMPLETE;
+        }
+
+        if ($this->artwork_status != $status) {
+            $this->artwork_status = $status;
+            $this->artwork_status_message = null;
+        }
+
+        return $this;
+    }
+}
+
+?>

rules/EarlyReturn/Rector/If_/ChangeAndIfToEarlyReturnRector.php

@@ -7,15 +7,18 @@
 use PhpParser\Node;
 use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
+use PhpParser\Node\Expr\Closure;
 use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\Break_;
+use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Continue_;
-use PhpParser\Node\Stmt\Else_;
-use PhpParser\Node\Stmt\ElseIf_;
+use PhpParser\Node\Stmt\Foreach_;
+use PhpParser\Node\Stmt\Function_;
 use PhpParser\Node\Stmt\If_;
+use PhpParser\Node\Stmt\Namespace_;
 use PhpParser\Node\Stmt\Return_;
-use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
 use Rector\Core\NodeManipulator\IfManipulator;
+use Rector\Core\PhpParser\Node\CustomNode\FileWithoutNamespace;
 use Rector\Core\Rector\AbstractRector;
 use Rector\EarlyReturn\NodeAnalyzer\IfAndAnalyzer;
 use Rector\EarlyReturn\NodeAnalyzer\SimpleScalarAnalyzer;
@@ -86,11 +89,18 @@ public function canDrive(Car $car)
      */
     public function getNodeTypes(): array
     {
-        return [StmtsAwareInterface::class];
+        return [
+            ClassMethod::class,
+            Function_::class,
+            Foreach_::class,
+            Closure::class,
+            FileWithoutNamespace::class,
+            Namespace_::class,
+        ];
     }
 
     /**
-     * @param StmtsAwareInterface $node
+     * @param Stmt\ClassMethod|Stmt\Function_|Stmt\Foreach_|Expr\Closure|FileWithoutNamespace|Stmt\Namespace_ $node
      */
     public function refactor(Node $node): ?Node
     {
@@ -110,7 +120,7 @@ public function refactor(Node $node): ?Node
 
             $nextStmt = $stmts[$key + 1] ?? null;
 
-            if ($this->shouldSkip($node, $stmt, $nextStmt)) {
+            if ($this->shouldSkip($stmt, $nextStmt)) {
                 $newStmts[] = $stmt;
                 continue;
             }
@@ -215,7 +225,7 @@ private function processReplaceIfs(
         return array_merge($result, [$ifNextReturnClone]);
     }
 
-    private function shouldSkip(StmtsAwareInterface $stmtsAware, If_ $if, ?Stmt $nexStmt): bool
+    private function shouldSkip(If_ $if, ?Stmt $nexStmt): bool
     {
         if (! $this->ifManipulator->isIfWithOnlyOneStmt($if)) {
             return true;
@@ -229,14 +239,6 @@ private function shouldSkip(StmtsAwareInterface $stmtsAware, If_ $if, ?Stmt $nex
             return true;
         }
 
-        if ($this->isParentIfReturnsVoidOrParentIfHasNextNode($stmtsAware)) {
-            return true;
-        }
-
-        if ($this->isNestedIfInLoop($if, $stmtsAware)) {
-            return true;
-        }
-
         // is simple return? skip it
         $onlyStmt = $if->stmts[0];
         if ($onlyStmt instanceof Return_ && $onlyStmt->expr instanceof Expr && $this->simpleScalarAnalyzer->isSimpleScalar(
@@ -252,31 +254,6 @@ private function shouldSkip(StmtsAwareInterface $stmtsAware, If_ $if, ?Stmt $nex
         return ! $this->isLastIfOrBeforeLastReturn($if, $nexStmt);
     }
 
-    private function isParentIfReturnsVoidOrParentIfHasNextNode(StmtsAwareInterface $stmtsAware): bool
-    {
-        if (! $stmtsAware instanceof If_) {
-            $parent = $stmtsAware->getAttribute(AttributeKey::PARENT_NODE);
-            if ($parent instanceof If_) {
-                $node = $parent->getAttribute(AttributeKey::NEXT_NODE);
-                return ! $node instanceof Return_;
-            }
-
-            return false;
-        }
-
-        $nextNode = $stmtsAware->getAttribute(AttributeKey::NEXT_NODE);
-        return $nextNode instanceof Node;
-    }
-
-    private function isNestedIfInLoop(If_ $if, StmtsAwareInterface $stmtsAware): bool
-    {
-        if (! $this->contextAnalyzer->isInLoop($if)) {
-            return false;
-        }
-
-        return $stmtsAware instanceof If_ || $stmtsAware instanceof Else_ || $stmtsAware instanceof ElseIf_;
-    }
-
     private function isLastIfOrBeforeLastReturn(If_ $if, ?Stmt $nextStmt): bool
     {
         if ($nextStmt instanceof Node) {
@@ -292,6 +269,6 @@ private function isLastIfOrBeforeLastReturn(If_ $if, ?Stmt $nextStmt): bool
             return $this->isLastIfOrBeforeLastReturn($parentNode, $nextStmt);
         }
 
-        return ! $this->contextAnalyzer->hasAssignWithIndirectReturn($parentNode, $if);
+        return true;
     }
 }

rules/Php80/NodeResolver/SwitchExprsResolver.php

@@ -36,9 +36,15 @@ public function resolve(Switch_ $switch): array
                 return [];
             }
 
-            if ($case->stmts === [] && $case->cond instanceof Expr) {
-                $collectionEmptyCasesCond[$key] = $case->cond;
+            if ($case->stmts !== []) {
+                continue;
             }
+
+            if (! $case->cond instanceof Expr) {
+                continue;
+            }
+
+            $collectionEmptyCasesCond[$key] = $case->cond;
         }
 
         foreach ($newSwitch->cases as $key => $case) {