[CodeQuality] Add CallbackSingleAssertToSimplerRector to simplify single-assert callback matchers in with()

config/sets/phpunit-code-quality.php

@@ -43,6 +43,7 @@
 use Rector\PHPUnit\CodeQuality\Rector\MethodCall\AssertSameBoolNullToSpecificMethodRector;
 use Rector\PHPUnit\CodeQuality\Rector\MethodCall\AssertSameTrueFalseToAssertTrueFalseRector;
 use Rector\PHPUnit\CodeQuality\Rector\MethodCall\AssertTrueFalseToSpecificMethodRector;
+use Rector\PHPUnit\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector;
 use Rector\PHPUnit\CodeQuality\Rector\MethodCall\FlipAssertRector;
 use Rector\PHPUnit\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector;
 use Rector\PHPUnit\CodeQuality\Rector\MethodCall\MergeWithCallableAndWillReturnRector;
@@ -137,6 +138,7 @@
          */
         RemoveExpectAnyFromMockRector::class,
         SingleMockPropertyTypeRector::class,
+        CallbackSingleAssertToSimplerRector::class,
         SimplerWithIsInstanceOfRector::class,
         DirectInstanceOverMockArgRector::class,
 

rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/Fixture/return_throw_and_null.php.inc

@@ -0,0 +1,53 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\RemoveReturnFromVoidMethodMockCallbackRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+use Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\RemoveReturnFromVoidMethodMockCallbackRector\Source\SomeEntityManager;
+
+final class ReturnThrowAndNull extends TestCase
+{
+    public function test($matcher): void
+    {
+        $this->createMock(SomeEntityManager::class)
+            ->method('persist')
+            ->willReturnCallback(function ($entity) use ($matcher) {
+                if (1 === $matcher->numberOfInvocations()) {
+                    return null;
+                }
+
+                if (2 === $matcher->numberOfInvocations()) {
+                    return throw new \RuntimeException('boom');
+                }
+            });
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\RemoveReturnFromVoidMethodMockCallbackRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+use Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\RemoveReturnFromVoidMethodMockCallbackRector\Source\SomeEntityManager;
+
+final class ReturnThrowAndNull extends TestCase
+{
+    public function test($matcher): void
+    {
+        $this->createMock(SomeEntityManager::class)
+            ->method('persist')
+            ->willReturnCallback(function ($entity) use ($matcher): void {
+                if (1 === $matcher->numberOfInvocations()) {
+                    return;
+                }
+
+                if (2 === $matcher->numberOfInvocations()) {
+                    throw new \RuntimeException('boom');
+                }
+            });
+    }
+}
+
+?>

rules-tests/CodeQuality/Rector/MethodCall/CallbackSingleAssertToSimplerRector/CallbackSingleAssertToSimplerRectorTest.php

@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector;
+
+use Iterator;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class CallbackSingleAssertToSimplerRectorTest extends AbstractRectorTestCase
+{
+    #[DataProvider('provideData')]
+    public function test(string $filePath): void
+    {
+        $this->doTestFile($filePath);
+    }
+
+    public static function provideData(): Iterator
+    {
+        return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
+    }
+
+    public function provideConfigFilePath(): string
+    {
+        return __DIR__ . '/config/configured_rule.php';
+    }
+}

rules-tests/CodeQuality/Rector/MethodCall/CallbackSingleAssertToSimplerRector/Fixture/multiple_with_args.php.inc

@@ -0,0 +1,47 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class MultipleWithArgs extends TestCase
+{
+    public function test()
+    {
+        $builder = $this->getMockBuilder('AnyType')->getMock();
+
+        $builder->expects($this->exactly(2))
+            ->method('add')
+            ->with($this->callback(function ($type): bool {
+                $this->assertSame(TextType::class, $type);
+
+                return true;
+            }), $this->callback(function ($options): bool {
+                $this->assertSame([], $options);
+
+                return true;
+            }));
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class MultipleWithArgs extends TestCase
+{
+    public function test()
+    {
+        $builder = $this->getMockBuilder('AnyType')->getMock();
+
+        $builder->expects($this->exactly(2))
+            ->method('add')
+            ->with($this->equalTo(TextType::class), $this->equalTo([]));
+    }
+}
+
+?>

rules-tests/CodeQuality/Rector/MethodCall/CallbackSingleAssertToSimplerRector/Fixture/skip_different_assert.php.inc

@@ -0,0 +1,21 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class SkipDifferentAssert extends TestCase
+{
+    public function test()
+    {
+        $builder = $this->getMockBuilder('AnyType')->getMock();
+
+        $builder->expects($this->exactly(2))
+            ->method('add')
+            ->with($this->callback(function ($type): bool {
+                $this->assertInstanceOf(TextType::class, $type);
+
+                return true;
+            }));
+    }
+}

rules-tests/CodeQuality/Rector/MethodCall/CallbackSingleAssertToSimplerRector/Fixture/skip_multiple_stmts.php.inc

@@ -0,0 +1,22 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class SkipMultipleStmts extends TestCase
+{
+    public function test()
+    {
+        $builder = $this->getMockBuilder('AnyType')->getMock();
+
+        $builder->expects($this->exactly(2))
+            ->method('add')
+            ->with($this->callback(function ($type): bool {
+                $this->assertSame(TextType::class, $type);
+                $this->assertNotNull($type);
+
+                return true;
+            }));
+    }
+}

rules-tests/CodeQuality/Rector/MethodCall/CallbackSingleAssertToSimplerRector/Fixture/some_file.php.inc

@@ -0,0 +1,43 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class SomeFile extends TestCase
+{
+    public function test()
+    {
+        $builder = $this->getMockBuilder('AnyType')->getMock();
+
+        $builder->expects($this->exactly(2))
+            ->method('add')
+            ->with($this->callback(function ($type): bool {
+                $this->assertSame(TextType::class, $type);
+
+                return true;
+            }));
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class SomeFile extends TestCase
+{
+    public function test()
+    {
+        $builder = $this->getMockBuilder('AnyType')->getMock();
+
+        $builder->expects($this->exactly(2))
+            ->method('add')
+            ->with($this->equalTo(TextType::class));
+    }
+}
+
+?>

rules-tests/CodeQuality/Rector/MethodCall/CallbackSingleAssertToSimplerRector/config/configured_rule.php

@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\Config\RectorConfig;
+use Rector\PHPUnit\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector;
+
+return static function (RectorConfig $rectorConfig): void {
+    $rectorConfig->rule(CallbackSingleAssertToSimplerRector::class);
+};

rules-tests/CodeQuality/Rector/MethodCall/SimplerWithIsInstanceOfRector/Fixture/multiple_with_args.php.inc

@@ -0,0 +1,45 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\SimplerWithIsInstanceOfRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class MultipleWithArgs extends TestCase
+{
+    public function test()
+    {
+        $someMock = $this->getMockBuilder('AnyType')->getMock();
+
+        $someMock->expects($this->any())
+            ->method('dispatch')
+            ->with($this->callback(function ($event): bool {
+                $this->assertInstanceOf(\stdClass::class, $event);
+                return true;
+            }), $this->callback(function ($name): bool {
+                $this->assertInstanceOf(\Stringable::class, $name);
+                return true;
+            }));
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\SimplerWithIsInstanceOfRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class MultipleWithArgs extends TestCase
+{
+    public function test()
+    {
+        $someMock = $this->getMockBuilder('AnyType')->getMock();
+
+        $someMock->expects($this->any())
+            ->method('dispatch')
+            ->with($this->isInstanceOf(\stdClass::class), $this->isInstanceOf(\Stringable::class));
+    }
+}
+
+?>

rules/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector.php

@@ -11,11 +11,13 @@
 use PhpParser\Node\Expr\ConstFetch;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Expr\PropertyFetch;
+use PhpParser\Node\Expr\Throw_;
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Identifier;
 use PhpParser\Node\Scalar;
 use PhpParser\Node\Scalar\String_;
 use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\Function_;
 use PhpParser\Node\Stmt\Return_;
 use PhpParser\NodeFinder;
@@ -275,21 +277,39 @@ private function refactorClosureToVoid(Closure $closure): bool
             return false;
         }
 
-        // only strip side-effect-free values, to not lose behavior
+        // only strip side-effect-free values or a "return throw", to not lose behavior
         foreach ($valueReturns as $valueReturn) {
             $returnedExpr = $valueReturn->expr;
             if (! $returnedExpr instanceof Expr) {
                 continue;
             }
 
+            // "return throw new X" becomes a bare "throw new X" statement below
+            if ($returnedExpr instanceof Throw_) {
+                continue;
+            }
+
             if (! $this->isPureValue($returnedExpr)) {
                 return false;
             }
         }
 
-        foreach ($valueReturns as $valueReturn) {
-            $valueReturn->expr = null;
-        }
+        $this->traverseNodesWithCallable($closure->stmts, static function (Node $subNode): ?Expression {
+            if (! $subNode instanceof Return_) {
+                return null;
+            }
+
+            // "return throw new X;" is itself invalid in a void function, unwrap to "throw new X;"
+            if ($subNode->expr instanceof Throw_) {
+                return new Expression($subNode->expr);
+            }
+
+            if ($subNode->expr instanceof Expr) {
+                $subNode->expr = null;
+            }
+
+            return null;
+        });
 
         // drop a now-empty trailing "return;"
         $lastStmt = $closure->stmts[array_key_last($closure->stmts)] ?? null;