diff --git a/config/sets/phpunit-code-quality.php b/config/sets/phpunit-code-quality.php index c45a7da9..1446f105 100644 --- a/config/sets/phpunit-code-quality.php +++ b/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 diff --git a/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/Fixture/add_return_false.php.inc b/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/Fixture/add_return_false.php.inc new file mode 100644 index 00000000..01f7c6db --- /dev/null +++ b/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/Fixture/add_return_false.php.inc @@ -0,0 +1,60 @@ +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; + } + }); + } +} + +?> +----- +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; + }); + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/Fixture/skip_already_has_return.php.inc b/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/Fixture/skip_already_has_return.php.inc new file mode 100644 index 00000000..d99dcdee --- /dev/null +++ b/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/Fixture/skip_already_has_return.php.inc @@ -0,0 +1,22 @@ +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; + }); + } +} diff --git a/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/Fixture/skip_no_return.php.inc b/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/Fixture/skip_no_return.php.inc new file mode 100644 index 00000000..32da18c6 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/Fixture/skip_no_return.php.inc @@ -0,0 +1,18 @@ +exactly(1); + + $eventMock = $this->createMock(\stdClass::class); + $eventMock->expects($matcher)->method('run')->willReturnCallback(function (...$parameters) use ($matcher) { + $this->assertSame(['emails'], $parameters[0]); + }); + } +} diff --git a/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/Fixture/skip_non_bool_return.php.inc b/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/Fixture/skip_non_bool_return.php.inc new file mode 100644 index 00000000..844d31d3 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/Fixture/skip_non_bool_return.php.inc @@ -0,0 +1,23 @@ +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; + } + }); + } +} diff --git a/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/WillReturnCallbackFallbackToReturnFalseRectorTest.php b/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/WillReturnCallbackFallbackToReturnFalseRectorTest.php new file mode 100644 index 00000000..81ef32c0 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/WillReturnCallbackFallbackToReturnFalseRectorTest.php @@ -0,0 +1,28 @@ +doTestFile($filePath); + } + + public static function provideData(): Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/config/configured_rule.php b/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/config/configured_rule.php new file mode 100644 index 00000000..3e13fdde --- /dev/null +++ b/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector/config/configured_rule.php @@ -0,0 +1,10 @@ +rule(WillReturnCallbackFallbackToReturnFalseRector::class); +}; diff --git a/rules/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector.php b/rules/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector.php new file mode 100644 index 00000000..4b42df0d --- /dev/null +++ b/rules/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToReturnFalseRector.php @@ -0,0 +1,161 @@ +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> + */ + 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; + } +}