[CodeQuality] Move Yield_ and YieldFrom check to SilentVoidResolver (#…

…5759)

* [CodeQuality] Move Yield_ and YieldFrom to SilentVoidResolver

* [ci-review] Rector Rectify

* clean up

* clean up

* clean up

* clean up

* clean up

* clean up

* clean up

* clean up

* performance: early check with silent void as not use find() usage from Node finder

---------

Co-authored-by: GitHub Action <actions@github.com>

rules-tests/CodeQuality/Rector/ClassMethod/ExplicitReturnNullRector/Fixture/skip_with_yield.php.inc

@@ -0,0 +1,15 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\ClassMethod\ExplicitReturnNullRector\Fixture;
+
+final class SkipWithYield
+{
+    public function run(int $number)
+    {
+        if ($number > 50) {
+            return 1;
+        }
+
+        yield 'foo';
+    }
+}

rules/CodeQuality/Rector/ClassMethod/ExplicitReturnNullRector.php

@@ -6,12 +6,9 @@
 
 use PhpParser\Node;
 use PhpParser\Node\Expr\ConstFetch;
-use PhpParser\Node\Expr\Yield_;
-use PhpParser\Node\Expr\YieldFrom;
 use PhpParser\Node\Name;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Return_;
-use PhpParser\Node\Stmt\Throw_;
 use PHPStan\Type\NullType;
 use PHPStan\Type\UnionType;
 use PHPStan\Type\VoidType;
@@ -105,7 +102,7 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
-        if ($this->containsYieldOrThrow($node)) {
+        if (! $this->silentVoidResolver->hasSilentVoid($node)) {
             return null;
         }
 
@@ -114,10 +111,6 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
-        if (! $this->silentVoidResolver->hasSilentVoid($node)) {
-            return null;
-        }
-
         $node->stmts[] = new Return_(new ConstFetch(new Name('null')));
 
         $this->transformDocUnionVoidToUnionNull($node);
@@ -157,14 +150,6 @@ private function transformDocUnionVoidToUnionNull(ClassMethod $classMethod): voi
         $this->phpDocTypeChanger->changeReturnType($classMethod, $phpDocInfo, $type);
     }
 
-    private function containsYieldOrThrow(ClassMethod $classMethod): bool
-    {
-        return (bool) $this->betterNodeFinder->findInstancesOf(
-            $classMethod,
-            [Yield_::class, Throw_::class, Node\Expr\Throw_::class, YieldFrom::class]
-        );
-    }
-
     private function hasReturnsWithValues(ClassMethod $classMethod): bool
     {
         /** @var Return_[] $returns */

rules/TypeDeclaration/TypeInferer/SilentVoidResolver.php

@@ -81,11 +81,11 @@ private function hasStmtsAlwaysReturnOrExit(array $stmts): bool
             }
 
             // has switch with always return
-            if ($stmt instanceof Switch_ && $this->isSwitchWithAlwaysReturn($stmt)) {
+            if ($stmt instanceof Switch_ && $this->isSwitchWithAlwaysReturnOrExit($stmt)) {
                 return true;
             }
 
-            if ($stmt instanceof TryCatch && $this->isTryCatchAlwaysReturn($stmt)) {
+            if ($stmt instanceof TryCatch && $this->isTryCatchAlwaysReturnOrExit($stmt)) {
                 return true;
             }
 
@@ -122,10 +122,14 @@ private function isIfReturn(Stmt|Expr $stmt): bool
 
     private function isStopped(Stmt|Expr $stmt): bool
     {
-        return $stmt instanceof Throw_ || $stmt instanceof Exit_ || $stmt instanceof Return_;
+        return $stmt instanceof Throw_
+            || $stmt instanceof Exit_
+            || $stmt instanceof Return_
+            || $stmt instanceof Yield_
+            || $stmt instanceof YieldFrom;
     }
 
-    private function isSwitchWithAlwaysReturn(Switch_ $switch): bool
+    private function isSwitchWithAlwaysReturnOrExit(Switch_ $switch): bool
     {
         $hasDefault = false;
 
@@ -140,13 +144,13 @@ private function isSwitchWithAlwaysReturn(Switch_ $switch): bool
             return false;
         }
 
-        $casesWithReturnCount = $this->resolveReturnCount($switch);
+        $casesWithReturnOrExitCount = $this->resolveReturnOrExitCount($switch);
 
-        // has same amount of returns as switches
-        return count($switch->cases) === $casesWithReturnCount;
+        // has same amount of first return or exit nodes as switches
+        return count($switch->cases) === $casesWithReturnOrExitCount;
     }
 
-    private function isTryCatchAlwaysReturn(TryCatch $tryCatch): bool
+    private function isTryCatchAlwaysReturnOrExit(TryCatch $tryCatch): bool
     {
         if (! $this->hasStmtsAlwaysReturnOrExit($tryCatch->stmts)) {
             return false;
@@ -163,18 +167,13 @@ private function isTryCatchAlwaysReturn(TryCatch $tryCatch): bool
         ));
     }
 
-    private function resolveReturnCount(Switch_ $switch): int
+    private function resolveReturnOrExitCount(Switch_ $switch): int
     {
         $casesWithReturnCount = 0;
 
         foreach ($switch->cases as $case) {
-            foreach ($case->stmts as $caseStmt) {
-                if (! $caseStmt instanceof Return_) {
-                    continue;
-                }
-
+            if ($this->hasStmtsAlwaysReturnOrExit($case->stmts)) {
                 ++$casesWithReturnCount;
-                break;
             }
         }