[CodeQuality] Add WillReturnCallbackFallbackToReturnFalseRector

config/sets/phpunit-code-quality.php

@@ -56,6 +56,7 @@
 use Rector\PHPUnit\CodeQuality\Rector\MethodCall\StringCastAssertStringContainsStringRector;
 use Rector\PHPUnit\CodeQuality\Rector\MethodCall\UseSpecificWillMethodRector;
 use Rector\PHPUnit\CodeQuality\Rector\MethodCall\UseSpecificWithMethodRector;
+use Rector\PHPUnit\CodeQuality\Rector\MethodCall\WillReturnCallbackFallbackToReturnFalseRector;
 use Rector\PHPUnit\CodeQuality\Rector\MethodCall\WithCallbackIdenticalToStandaloneAssertsRector;
 use Rector\PHPUnit\CodeQuality\Rector\StmtsAwareInterface\DeclareStrictTypesTestsRector;
 use Rector\PHPUnit\PHPUnit120\Rector\CallLike\CreateStubOverCreateMockArgRector;
@@ -87,6 +88,7 @@
         // narrow with consecutive
         NarrowIdenticalWithConsecutiveRector::class,
         NarrowSingleWillReturnCallbackRector::class,
+        WillReturnCallbackFallbackToReturnFalseRector::class,
         SingleWithConsecutiveToWithRector::class,
 
         // type declarations

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

@@ -0,0 +1,60 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\WillReturnCallbackFallbackToReturnFalseRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class AddReturnFalse extends TestCase
+{
+    public function test()
+    {
+        $matcher = $this->exactly(2);
+
+        $eventMock = $this->createMock(\stdClass::class);
+        $eventMock->expects($matcher)->method('checkContext')->willReturnCallback(function (...$parameters) use ($matcher) {
+            if (1 === $matcher->numberOfInvocations()) {
+                $this->assertSame(['email.stats', 'emails'], $parameters[0]);
+
+                return true;
+            }
+            if (2 === $matcher->numberOfInvocations()) {
+                $this->assertSame('emails', $parameters[0]);
+
+                return false;
+            }
+        });
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\WillReturnCallbackFallbackToReturnFalseRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class AddReturnFalse extends TestCase
+{
+    public function test()
+    {
+        $matcher = $this->exactly(2);
+
+        $eventMock = $this->createMock(\stdClass::class);
+        $eventMock->expects($matcher)->method('checkContext')->willReturnCallback(function (...$parameters) use ($matcher) {
+            if (1 === $matcher->numberOfInvocations()) {
+                $this->assertSame(['email.stats', 'emails'], $parameters[0]);
+
+                return true;
+            }
+            if (2 === $matcher->numberOfInvocations()) {
+                $this->assertSame('emails', $parameters[0]);
+
+                return false;
+            }
+            return false;
+        });
+    }
+}
+
+?>

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

@@ -0,0 +1,22 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\WillReturnCallbackFallbackToReturnFalseRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class SkipAlreadyHasReturn extends TestCase
+{
+    public function test()
+    {
+        $matcher = $this->exactly(2);
+
+        $eventMock = $this->createMock(\stdClass::class);
+        $eventMock->expects($matcher)->method('checkContext')->willReturnCallback(function (...$parameters) use ($matcher) {
+            if (1 === $matcher->numberOfInvocations()) {
+                return true;
+            }
+
+            return false;
+        });
+    }
+}

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

@@ -0,0 +1,18 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\WillReturnCallbackFallbackToReturnFalseRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class SkipNoReturn extends TestCase
+{
+    public function test()
+    {
+        $matcher = $this->exactly(1);
+
+        $eventMock = $this->createMock(\stdClass::class);
+        $eventMock->expects($matcher)->method('run')->willReturnCallback(function (...$parameters) use ($matcher) {
+            $this->assertSame(['emails'], $parameters[0]);
+        });
+    }
+}

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

@@ -0,0 +1,23 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\WillReturnCallbackFallbackToReturnFalseRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class SkipNonBoolReturn extends TestCase
+{
+    public function test()
+    {
+        $matcher = $this->exactly(2);
+
+        $eventMock = $this->createMock(\stdClass::class);
+        $eventMock->expects($matcher)->method('run')->willReturnCallback(function (...$parameters) use ($matcher) {
+            if (1 === $matcher->numberOfInvocations()) {
+                return 1;
+            }
+            if (2 === $matcher->numberOfInvocations()) {
+                return false;
+            }
+        });
+    }
+}

rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/WillReturnCallbackFallbackToReturnFalseRectorTest.php

@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\WillReturnCallbackFallbackToReturnFalseRector;
+
+use Iterator;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class WillReturnCallbackFallbackToReturnFalseRectorTest 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/WillReturnCallbackFallbackToReturnFalseRector/config/configured_rule.php

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

rules/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector.php

@@ -0,0 +1,161 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\PHPUnit\CodeQuality\Rector\MethodCall;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\Closure;
+use PhpParser\Node\Expr\ConstFetch;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Stmt\Return_;
+use Rector\PhpParser\Node\BetterNodeFinder;
+use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer;
+use Rector\Rector\AbstractRector;
+use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
+use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
+
+/**
+ * @see \Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\WillReturnCallbackFallbackToReturnFalseRector\WillReturnCallbackFallbackToReturnFalseRectorTest
+ */
+final class WillReturnCallbackFallbackToReturnFalseRector extends AbstractRector
+{
+    public function __construct(
+        private readonly TestsNodeAnalyzer $testsNodeAnalyzer,
+        private readonly BetterNodeFinder $betterNodeFinder,
+    ) {
+    }
+
+    public function getRuleDefinition(): RuleDefinition
+    {
+        return new RuleDefinition(
+            'Add a "return false" fallback to a willReturnCallback() closure that only returns booleans, but can fall through without a return',
+            [
+                new CodeSample(
+                    <<<'CODE_SAMPLE'
+use PHPUnit\Framework\TestCase;
+
+final class SomeTest extends TestCase
+{
+    public function test()
+    {
+        $matcher = $this->exactly(2);
+
+        $this->eventMock->expects($matcher)
+            ->method('checkContext')
+            ->willReturnCallback(function (...$parameters) use ($matcher) {
+                if ($matcher->numberOfInvocations() === 1) {
+                    return true;
+                }
+                if ($matcher->numberOfInvocations() === 2) {
+                    return false;
+                }
+            });
+    }
+}
+CODE_SAMPLE
+                    ,
+                    <<<'CODE_SAMPLE'
+use PHPUnit\Framework\TestCase;
+
+final class SomeTest extends TestCase
+{
+    public function test()
+    {
+        $matcher = $this->exactly(2);
+
+        $this->eventMock->expects($matcher)
+            ->method('checkContext')
+            ->willReturnCallback(function (...$parameters) use ($matcher) {
+                if ($matcher->numberOfInvocations() === 1) {
+                    return true;
+                }
+                if ($matcher->numberOfInvocations() === 2) {
+                    return false;
+                }
+
+                return false;
+            });
+    }
+}
+CODE_SAMPLE
+                ),
+            ]
+        );
+    }
+
+    /**
+     * @return array<class-string<Node>>
+     */
+    public function getNodeTypes(): array
+    {
+        return [MethodCall::class];
+    }
+
+    /**
+     * @param MethodCall $node
+     */
+    public function refactor(Node $node): ?MethodCall
+    {
+        if (! $this->testsNodeAnalyzer->isInTestClass($node)) {
+            return null;
+        }
+
+        if (! $this->isName($node->name, 'willReturnCallback')) {
+            return null;
+        }
+
+        if ($node->isFirstClassCallable()) {
+            return null;
+        }
+
+        if (count($node->getArgs()) !== 1) {
+            return null;
+        }
+
+        $closure = $node->getArgs()[0]
+            ->value;
+        if (! $closure instanceof Closure) {
+            return null;
+        }
+
+        if ($closure->stmts === []) {
+            return null;
+        }
+
+        // already ends with an explicit return, no fallback needed
+        $lastStmt = $closure->stmts[count($closure->stmts) - 1];
+        if ($lastStmt instanceof Return_) {
+            return null;
+        }
+
+        if (! $this->hasOnlyBoolReturns($closure)) {
+            return null;
+        }
+
+        $closure->stmts[] = new Return_($this->nodeFactory->createFalse());
+
+        return $node;
+    }
+
+    private function hasOnlyBoolReturns(Closure $closure): bool
+    {
+        /** @var Return_[] $returns */
+        $returns = $this->betterNodeFinder->findInstancesOfScoped($closure->stmts, [Return_::class]);
+        if ($returns === []) {
+            return false;
+        }
+
+        foreach ($returns as $return) {
+            if (! $return->expr instanceof ConstFetch) {
+                return false;
+            }
+
+            if (! $this->isNames($return->expr->name, ['true', 'false'])) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+}