diff --git a/config/sets/phpunit-code-quality.php b/config/sets/phpunit-code-quality.php index f5789b3e..c45a7da9 100644 --- a/config/sets/phpunit-code-quality.php +++ b/config/sets/phpunit-code-quality.php @@ -89,9 +89,6 @@ NarrowSingleWillReturnCallbackRector::class, SingleWithConsecutiveToWithRector::class, - // enable once better tested - // WillReturnCallbackFallbackToThrowRector::class, - // type declarations TypeWillReturnCallableArrowFunctionRector::class, VoidMethodWithCallbackToWillReturnCallbackRector::class, diff --git a/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector/Fixture/skip_fallback_return.php.inc b/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector/Fixture/skip_fallback_return.php.inc deleted file mode 100644 index 2427fb71..00000000 --- a/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector/Fixture/skip_fallback_return.php.inc +++ /dev/null @@ -1,28 +0,0 @@ -exactly(2); - - $someServiceMock = $this->createMock(SomeMockedClass::class); - $someServiceMock->expects($matcher) - ->method('run') - ->willReturnCallback(function () use ($matcher) { - if ($matcher->numberOfInvocations() === 1) { - return 'first'; - } - - if ($matcher->numberOfInvocations() === 2) { - return 'second'; - } - - return 'third'; - }); - } -} diff --git a/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector/Fixture/skip_no_matcher_branch.php.inc b/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector/Fixture/skip_no_matcher_branch.php.inc deleted file mode 100644 index b0f9a312..00000000 --- a/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector/Fixture/skip_no_matcher_branch.php.inc +++ /dev/null @@ -1,20 +0,0 @@ -exactly(1); - - $someServiceMock = $this->createMock(SomeMockedClass::class); - $someServiceMock->expects($matcher) - ->method('run') - ->willReturnCallback(function () use ($matcher) { - return 'value'; - }); - } -} diff --git a/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector/Fixture/void_fallback.php.inc b/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector/Fixture/void_fallback.php.inc deleted file mode 100644 index e3093500..00000000 --- a/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector/Fixture/void_fallback.php.inc +++ /dev/null @@ -1,50 +0,0 @@ -exactly(1); - - $someServiceMock = $this->createMock(SomeMockedClass::class); - $someServiceMock->expects($matcher) - ->method('run') - ->willReturnCallback(function () use ($matcher) { - if ($matcher->numberOfInvocations() === 1) { - return 'first'; - } - }); - } -} - -?> ------ -exactly(1); - - $someServiceMock = $this->createMock(SomeMockedClass::class); - $someServiceMock->expects($matcher) - ->method('run') - ->willReturnCallback(function () use ($matcher) { - if ($matcher->numberOfInvocations() === 1) { - return 'first'; - } - throw new \PHPUnit\Framework\Exception(sprintf('Method should not be called for the %dth time', $matcher->numberOfInvocations())); - }); - } -} - -?> diff --git a/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector/WillReturnCallbackFallbackToThrowRectorTest.php b/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector/WillReturnCallbackFallbackToThrowRectorTest.php deleted file mode 100644 index 57724913..00000000 --- a/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector/WillReturnCallbackFallbackToThrowRectorTest.php +++ /dev/null @@ -1,28 +0,0 @@ -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/WillReturnCallbackFallbackToThrowRector/config/configured_rule.php b/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector/config/configured_rule.php deleted file mode 100644 index e34176a5..00000000 --- a/rules-tests/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector/config/configured_rule.php +++ /dev/null @@ -1,10 +0,0 @@ -rule(WillReturnCallbackFallbackToThrowRector::class); -}; diff --git a/rules/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector.php b/rules/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector.php index 2987f8f3..e6fd08e8 100644 --- a/rules/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector.php +++ b/rules/CodeQuality/Rector/MethodCall/WillReturnCallbackFallbackToThrowRector.php @@ -5,35 +5,18 @@ namespace Rector\PHPUnit\CodeQuality\Rector\MethodCall; use PhpParser\Node; -use PhpParser\Node\Arg; -use PhpParser\Node\ClosureUse; -use PhpParser\Node\Expr\Closure; use PhpParser\Node\Expr\MethodCall; -use PhpParser\Node\Expr\New_; -use PhpParser\Node\Expr\Throw_; -use PhpParser\Node\Expr\Variable; -use PhpParser\Node\Name\FullyQualified; -use PhpParser\Node\Scalar\String_; -use PhpParser\Node\Stmt\Expression; -use PhpParser\Node\Stmt\If_; -use Rector\PHPUnit\Enum\ConsecutiveVariable; -use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer; -use Rector\PHPUnit\NodeFactory\MatcherInvocationCountMethodCallNodeFactory; +use Rector\Configuration\Deprecation\Contract\DeprecatedInterface; +use Rector\Exception\ShouldNotHappenException; use Rector\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; /** - * @see \Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\WillReturnCallbackFallbackToThrowRector\WillReturnCallbackFallbackToThrowRectorTest + * @deprecated This rule is deprecated as the case is very rare, hardly automatable and it is not part of any set. Handle it in a custom way or keep the code untouched. */ -final class WillReturnCallbackFallbackToThrowRector extends AbstractRector +final class WillReturnCallbackFallbackToThrowRector extends AbstractRector implements DeprecatedInterface { - public function __construct( - private readonly TestsNodeAnalyzer $testsNodeAnalyzer, - private readonly MatcherInvocationCountMethodCallNodeFactory $matcherInvocationCountMethodCallNodeFactory, - ) { - } - public function getRuleDefinition(): RuleDefinition { return new RuleDefinition( @@ -99,93 +82,9 @@ public function getNodeTypes(): array */ 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 (! $this->usesMatcher($closure)) { - return null; - } - - // the closure must branch on the matcher invocation count first - if (! $this->hasConsecutiveIf($closure)) { - return null; - } - - $lastStmt = $closure->stmts[array_key_last($closure->stmts)] ?? null; - - // the last statement must be an if branch; an explicit fallback "return ;" is left untouched - if (! $lastStmt instanceof If_) { - return null; - } - - $closure->stmts[] = $this->createThrow(); - - return $node; - } - - private function usesMatcher(Closure $closure): bool - { - return array_any( - $closure->uses, - fn (ClosureUse $use): bool => $this->isName($use->var, ConsecutiveVariable::MATCHER) - ); - } - - private function hasConsecutiveIf(Closure $closure): bool - { - foreach ($closure->stmts as $stmt) { - if (! $stmt instanceof If_) { - continue; - } - - $hasMatcherCall = false; - $this->traverseNodesWithCallable($stmt->cond, function (Node $node) use (&$hasMatcherCall): null { - if ($node instanceof MethodCall - && $node->var instanceof Variable - && $this->isName($node->var, ConsecutiveVariable::MATCHER) - ) { - $hasMatcherCall = true; - } - - return null; - }); - - if ($hasMatcherCall) { - return true; - } - } - - return false; - } - - private function createThrow(): Expression - { - $sprintfFuncCall = $this->nodeFactory->createFuncCall('sprintf', [ - new String_('Method should not be called for the %dth time'), - $this->matcherInvocationCountMethodCallNodeFactory->create(), - ]); - - $new = new New_(new FullyQualified('PHPUnit\Framework\Exception'), [new Arg($sprintfFuncCall)]); - - return new Expression(new Throw_($new)); + throw new ShouldNotHappenException(sprintf( + '"%s" is deprecated and should not be used anymore. Remove it from your config files.', + self::class, + )); } }