diff --git a/config/sets/phpunit-code-quality.php b/config/sets/phpunit-code-quality.php index 78cd18ce..2c5407af 100644 --- a/config/sets/phpunit-code-quality.php +++ b/config/sets/phpunit-code-quality.php @@ -12,6 +12,7 @@ use Rector\PHPUnit\CodeQuality\Rector\Class_\NarrowUnusedSetUpDefinedPropertyRector; use Rector\PHPUnit\CodeQuality\Rector\Class_\PreferPHPUnitThisCallRector; use Rector\PHPUnit\CodeQuality\Rector\Class_\RemoveNeverUsedMockPropertyRector; +use Rector\PHPUnit\CodeQuality\Rector\Class_\RemoveReturnFromVoidMethodMockCallbackRector; use Rector\PHPUnit\CodeQuality\Rector\Class_\SingleMockPropertyTypeRector; use Rector\PHPUnit\CodeQuality\Rector\Class_\TestWithToDataProviderRector; use Rector\PHPUnit\CodeQuality\Rector\Class_\TypeWillReturnCallableArrowFunctionRector; @@ -92,6 +93,7 @@ // type declarations TypeWillReturnCallableArrowFunctionRector::class, + RemoveReturnFromVoidMethodMockCallbackRector::class, StringCastAssertStringContainsStringRector::class, AddParamTypeFromDependsRector::class, AddReturnTypeToDependedRector::class, diff --git a/rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/Fixture/skip_already_void.php.inc b/rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/Fixture/skip_already_void.php.inc new file mode 100644 index 00000000..d7ed8c45 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/Fixture/skip_already_void.php.inc @@ -0,0 +1,18 @@ +createMock(SomeEntityManager::class) + ->method('persist') + ->willReturnCallback(function ($entity): void { + echo $entity; + }); + } +} diff --git a/rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/Fixture/skip_non_void_method.php.inc b/rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/Fixture/skip_non_void_method.php.inc new file mode 100644 index 00000000..99b0562f --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/Fixture/skip_non_void_method.php.inc @@ -0,0 +1,18 @@ +createMock(SomeEntityManager::class) + ->method('count') + ->willReturnCallback(function ($name) { + return 100; + }); + } +} diff --git a/rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/Fixture/will_return_callback_void.php.inc b/rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/Fixture/will_return_callback_void.php.inc new file mode 100644 index 00000000..fc89107b --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/Fixture/will_return_callback_void.php.inc @@ -0,0 +1,43 @@ +createMock(SomeEntityManager::class) + ->method('persist') + ->willReturnCallback(function ($entity) { + echo $entity; + + return true; + }); + } +} + +?> +----- +createMock(SomeEntityManager::class) + ->method('persist') + ->willReturnCallback(function ($entity): void { + echo $entity; + }); + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/Fixture/with_callback_void_setup.php.inc b/rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/Fixture/with_callback_void_setup.php.inc new file mode 100644 index 00000000..8cb75416 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/Fixture/with_callback_void_setup.php.inc @@ -0,0 +1,59 @@ +entityManager = $this->createMock(SomeEntityManager::class); + } + + public function test(): void + { + $this->entityManager + ->method('persist') + ->with($this->callback(function ($entity): bool { + $this->assertInstanceOf(\stdClass::class, $entity); + + return true; + })); + } +} + +?> +----- +entityManager = $this->createMock(SomeEntityManager::class); + } + + public function test(): void + { + $this->entityManager + ->method('persist') + ->with($this->callback(function ($entity): void { + $this->assertInstanceOf(\stdClass::class, $entity); + })); + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/RemoveReturnFromVoidMethodMockCallbackRectorTest.php b/rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/RemoveReturnFromVoidMethodMockCallbackRectorTest.php new file mode 100644 index 00000000..d884ab3a --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/RemoveReturnFromVoidMethodMockCallbackRectorTest.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/Class_/RemoveReturnFromVoidMethodMockCallbackRector/Source/SomeEntityManager.php b/rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/Source/SomeEntityManager.php new file mode 100644 index 00000000..0762ff09 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector/Source/SomeEntityManager.php @@ -0,0 +1,16 @@ +rule(RemoveReturnFromVoidMethodMockCallbackRector::class); +}; diff --git a/rules/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector.php b/rules/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector.php new file mode 100644 index 00000000..087929fc --- /dev/null +++ b/rules/CodeQuality/Rector/Class_/RemoveReturnFromVoidMethodMockCallbackRector.php @@ -0,0 +1,349 @@ +createMock(SomeClass::class) + ->method('run') + ->willReturnCallback(function ($arg) { + echo $arg; + + return true; + }); + } +} + +final class SomeClass +{ + public function run($arg): void + { + } +} +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +use PHPUnit\Framework\TestCase; + +final class SomeTest extends TestCase +{ + public function test() + { + $this->createMock(SomeClass::class) + ->method('run') + ->willReturnCallback(function ($arg): void { + echo $arg; + }); + } +} + +final class SomeClass +{ + public function run($arg): void + { + } +} +CODE_SAMPLE + ), + ] + ); + } + + /** + * @return array> + */ + public function getNodeTypes(): array + { + return [Class_::class]; + } + + /** + * @param Class_ $node + */ + public function refactor(Node $node): ?Class_ + { + if (! $this->testsNodeAnalyzer->isInTestClass($node)) { + return null; + } + + $currentClassReflection = $this->reflectionResolver->resolveClassReflection($node); + if (! $currentClassReflection instanceof ClassReflection) { + return null; + } + + $propertyNameToMockedTypes = $this->setUpAssignedMockTypesResolver->resolveFromClass($node); + + $hasChanged = false; + + $this->traverseNodesWithCallable($node->getMethods(), function (Node $subNode) use ( + &$hasChanged, + $propertyNameToMockedTypes, + $currentClassReflection + ): null { + if (! $subNode instanceof MethodCall || $subNode->isFirstClassCallable()) { + return null; + } + + $closure = $this->matchInnerClosure($subNode); + if (! $closure instanceof Closure) { + return null; + } + + if (! $this->isMockedVoidMethodCall($subNode, $propertyNameToMockedTypes, $currentClassReflection)) { + return null; + } + + if ($this->refactorClosureToVoid($closure)) { + $hasChanged = true; + } + + return null; + }); + + if (! $hasChanged) { + return null; + } + + return $node; + } + + private function matchInnerClosure(MethodCall $methodCall): ?Closure + { + if ($this->isName($methodCall->name, 'willReturnCallback')) { + $innerArg = $methodCall->getArgs()[0]; + if ($innerArg->value instanceof Closure) { + return $innerArg->value; + } + + return null; + } + + if ($this->isName($methodCall->name, 'with')) { + $withFirstArg = $methodCall->getArgs()[0]; + if (! $withFirstArg->value instanceof MethodCall) { + return null; + } + + $nestedMethodCall = $withFirstArg->value; + if (! $this->isName($nestedMethodCall->name, 'callback')) { + return null; + } + + $nestedArg = $nestedMethodCall->getArgs()[0]; + if ($nestedArg->value instanceof Closure) { + return $nestedArg->value; + } + } + + return null; + } + + /** + * @param array $propertyNameToMockedTypes + */ + private function isMockedVoidMethodCall( + MethodCall $methodCall, + array $propertyNameToMockedTypes, + ClassReflection $currentClassReflection + ): bool { + if (! $methodCall->var instanceof MethodCall) { + return false; + } + + $parentMethodCall = $methodCall->var; + if (! $this->isName($parentMethodCall->name, 'method')) { + return false; + } + + $methodNameExpr = $parentMethodCall->getArgs()[0] + ->value; + if (! $methodNameExpr instanceof String_) { + return false; + } + + $methodName = $methodNameExpr->value; + $callerType = $this->getType($parentMethodCall->var); + + if ($callerType instanceof ObjectType && in_array( + $callerType->getClassName(), + [PHPUnitClassName::INVOCATION_MOCKER, PHPUnitClassName::INVOCATION_STUBBER], + true + )) { + $parentMethodCall = $parentMethodCall->var; + + if ($parentMethodCall instanceof MethodCall) { + $callerType = $this->getType($parentMethodCall->var); + } + } + + $callerType = $this->fallbackMockedObjectInSetUp($callerType, $parentMethodCall, $propertyNameToMockedTypes); + if (! $callerType instanceof IntersectionType) { + return false; + } + + $paramTypesAndReturnType = $this->methodParametersAndReturnTypesResolver->resolveFromReflection( + $callerType, + $methodName, + $currentClassReflection + ); + + if (! $paramTypesAndReturnType instanceof ParamTypesAndReturnType) { + return false; + } + + return $paramTypesAndReturnType->getReturnType() instanceof VoidType; + } + + private function refactorClosureToVoid(Closure $closure): bool + { + $nodeFinder = new NodeFinder(); + + // nested function-likes have their own return scope, skip to stay safe + $nestedFunctionLikes = $nodeFinder->find( + $closure->stmts, + static fn (Node $node): bool => $node instanceof Closure || $node instanceof ArrowFunction || $node instanceof Function_ + ); + if ($nestedFunctionLikes !== []) { + return false; + } + + /** @var Return_[] $returns */ + $returns = $nodeFinder->findInstanceOf($closure->stmts, Return_::class); + + $valueReturns = array_filter($returns, static fn (Return_ $return): bool => $return->expr instanceof Expr); + + // nothing to drop, typing void is left to TypeWillReturnCallableArrowFunctionRector + if ($valueReturns === []) { + return false; + } + + // only strip side-effect-free values, to not lose behavior + foreach ($valueReturns as $valueReturn) { + $returnedExpr = $valueReturn->expr; + if (! $returnedExpr instanceof Expr) { + continue; + } + + if (! $this->isPureValue($returnedExpr)) { + return false; + } + } + + foreach ($valueReturns as $valueReturn) { + $valueReturn->expr = null; + } + + // drop a now-empty trailing "return;" + $lastStmt = $closure->stmts[array_key_last($closure->stmts)] ?? null; + if ($lastStmt instanceof Return_ && ! $lastStmt->expr instanceof Expr) { + array_pop($closure->stmts); + } + + $closure->returnType = new Identifier('void'); + + return true; + } + + private function isPureValue(Expr $expr): bool + { + return $expr instanceof Scalar || $expr instanceof ConstFetch || $expr instanceof Variable; + } + + /** + * @param array $propertyNameToMockedTypes + */ + private function fallbackMockedObjectInSetUp( + Type $callerType, + Expr $expr, + array $propertyNameToMockedTypes + ): Type { + if (! $callerType instanceof ObjectType && ! $callerType instanceof NeverType) { + return $callerType; + } + + if (! $expr instanceof MethodCall) { + return $callerType; + } + + if ($callerType instanceof ObjectType && $callerType->getClassName() !== ClassName::MOCK_OBJECT) { + return $callerType; + } + + // type is missing, because of "final" keyword on mocked class + // resolve from setUp assignment instead + if (! $expr->var instanceof PropertyFetch && ! $expr->var instanceof Variable) { + return $callerType; + } + + if ($expr->var instanceof Variable) { + $propertyOrVariableName = $this->getName($expr->var); + } else { + $propertyOrVariableName = $this->getName($expr->var->name); + } + + if (isset($propertyNameToMockedTypes[$propertyOrVariableName])) { + $mockedType = $propertyNameToMockedTypes[$propertyOrVariableName]; + return new IntersectionType([$callerType, new ObjectType($mockedType)]); + } + + return $callerType; + } +}