[CodeQuality] Skip never-returning fall-through in WillReturnCallbackFallbackToReturnFalseRector (#801)

Author: TomasVotruba

rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/Fixture/skip_fail_fallthrough.php.inc

@@ -0,0 +1,23 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\WillReturnCallbackFallbackToReturnFalseRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class SkipFailFallthrough extends TestCase
+{
+    public function test()
+    {
+        $eventMock = $this->createMock(\stdClass::class);
+        $eventMock->method('has')->willReturnCallback(function (string $id): bool {
+            if ('service' === $id) {
+                return true;
+            }
+            if ('other' === $id) {
+                return false;
+            }
+
+            self::fail('Unknown ID requested: ' . $id);
+        });
+    }
+}

rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/Fixture/skip_throw_fallthrough.php.inc

@@ -0,0 +1,23 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\WillReturnCallbackFallbackToReturnFalseRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class SkipThrowFallthrough extends TestCase
+{
+    public function test()
+    {
+        $eventMock = $this->createMock(\stdClass::class);
+        $eventMock->method('has')->willReturnCallback(function (string $id): bool {
+            if ('service' === $id) {
+                return true;
+            }
+            if ('other' === $id) {
+                return false;
+            }
+
+            throw new \Exception('Unknown ID requested: ' . $id);
+        });
+    }
+}

rules/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector.php

@@ -5,11 +5,15 @@
 namespace Rector\PHPUnit\CodeQuality\Rector\MethodCall;
 
 use PhpParser\Node;
+use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\Closure;
 use PhpParser\Node\Expr\ConstFetch;
 use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\Throw_;
+use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\Nop;
 use PhpParser\Node\Stmt\Return_;
+use PHPStan\Type\NeverType;
 use Rector\PhpParser\Node\BetterNodeFinder;
 use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer;
 use Rector\Rector\AbstractRector;
@@ -132,6 +136,11 @@ public function refactor(Node $node): ?MethodCall
             return null;
         }
 
+        // closure cannot fall through: last statement throws or calls a never-returning method
+        if ($lastStmt instanceof Expression && $this->isNeverReturningExpression($lastStmt->expr)) {
+            return null;
+        }
+
         if (! $this->hasOnlyBoolReturns($closure)) {
             return null;
         }
@@ -161,4 +170,14 @@ private function hasOnlyBoolReturns(Closure $closure): bool
 
         return true;
     }
+
+    private function isNeverReturningExpression(Expr $expr): bool
+    {
+        // cheap structural check first, before reflection-based type resolution
+        if ($expr instanceof Throw_) {
+            return true;
+        }
+
+        return $this->getType($expr) instanceof NeverType;
+    }
 }