diff --git a/config/sets/composer-based.php b/config/sets/composer-based.php index 0140d5f6..357bcdc3 100644 --- a/config/sets/composer-based.php +++ b/config/sets/composer-based.php @@ -12,6 +12,7 @@ use Rector\PHPUnit\AnnotationsToAttributes\Rector\ClassMethod\DataProviderAnnotationToAttributeRector; use Rector\PHPUnit\AnnotationsToAttributes\Rector\ClassMethod\DependsAnnotationWithValueToAttributeRector; use Rector\PHPUnit\AnnotationsToAttributes\Rector\ClassMethod\TestWithAnnotationToAttributeRector; +use Rector\PHPUnit\CodeQuality\Rector\Class_\AddIntersectionParamToMockObjectParamRector; use Rector\PHPUnit\CodeQuality\Rector\Class_\AddIntersectionVarToMockObjectPropertyRector; use Rector\PHPUnit\CodeQuality\Rector\Class_\AddStubIntersectionVarToStubPropertyRector; use Rector\PHPUnit\CodeQuality\Rector\ClassMethod\BareCreateMockAssignToDirectUseRector; @@ -57,6 +58,7 @@ // mocks back over stubs, where a mock object is required MockObjectArgCreateStubToCreateMockRector::class, ExpectsParamToMockObjectRector::class, + AddIntersectionParamToMockObjectParamRector::class, // deprecated in PHPUnit 11.5 AssertContainsOnlyMethodCallRector::class, diff --git a/config/sets/phpunit-mock-to-stub.php b/config/sets/phpunit-mock-to-stub.php index 985a9ebe..15d1170b 100644 --- a/config/sets/phpunit-mock-to-stub.php +++ b/config/sets/phpunit-mock-to-stub.php @@ -3,6 +3,7 @@ declare(strict_types=1); use Rector\Config\RectorConfig; +use Rector\PHPUnit\CodeQuality\Rector\Class_\AddIntersectionParamToMockObjectParamRector; use Rector\PHPUnit\CodeQuality\Rector\Class_\AddIntersectionVarToMockObjectPropertyRector; use Rector\PHPUnit\CodeQuality\Rector\Class_\AddStubIntersectionVarToStubPropertyRector; use Rector\PHPUnit\CodeQuality\Rector\ClassMethod\BareCreateMockAssignToDirectUseRector; @@ -27,5 +28,6 @@ // mocks back over stubs, where mock object is required MockObjectArgCreateStubToCreateMockRector::class, + AddIntersectionParamToMockObjectParamRector::class, ]); }; diff --git a/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/AddIntersectionParamToMockObjectParamRectorTest.php b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/AddIntersectionParamToMockObjectParamRectorTest.php new file mode 100644 index 00000000..e9d920fe --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/AddIntersectionParamToMockObjectParamRectorTest.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_/AddIntersectionParamToMockObjectParamRector/Fixture/direct_create_mock_arg.php.inc b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/direct_create_mock_arg.php.inc new file mode 100644 index 00000000..b802090a --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/direct_create_mock_arg.php.inc @@ -0,0 +1,48 @@ +prepareServiceMock($this->createMock(\stdClass::class)); + } + + private function prepareServiceMock(MockObject $someService): void + { + $someService->expects($this->once()) + ->method('getId'); + } +} + +?> +----- +prepareServiceMock($this->createMock(\stdClass::class)); + } + + /** + * @param \stdClass&\PHPUnit\Framework\MockObject\MockObject $someService + */ + private function prepareServiceMock(MockObject $someService): void + { + $someService->expects($this->once()) + ->method('getId'); + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/fixture.php.inc b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/fixture.php.inc new file mode 100644 index 00000000..ccb03597 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/fixture.php.inc @@ -0,0 +1,50 @@ +createMock(\stdClass::class); + $this->prepareServiceMock($someService); + } + + private function prepareServiceMock(MockObject $someService): void + { + $someService->expects($this->once()) + ->method('getId'); + } +} + +?> +----- +createMock(\stdClass::class); + $this->prepareServiceMock($someService); + } + + /** + * @param \stdClass&\PHPUnit\Framework\MockObject\MockObject $someService + */ + private function prepareServiceMock(MockObject $someService): void + { + $someService->expects($this->once()) + ->method('getId'); + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/second_param_only.php.inc b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/second_param_only.php.inc new file mode 100644 index 00000000..f2035885 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/second_param_only.php.inc @@ -0,0 +1,50 @@ +getEventListener(new \DateTime('now'), $builder); + } + + private function getEventListener(\DateTime $dateTime, \PHPUnit\Framework\MockObject\MockObject $builder): void + { + $builder->expects($this->once()) + ->method('add'); + } +} + +?> +----- +getEventListener(new \DateTime('now'), $builder); + } + + /** + * @param \stdClass&\PHPUnit\Framework\MockObject\MockObject $builder + */ + private function getEventListener(\DateTime $dateTime, \PHPUnit\Framework\MockObject\MockObject $builder): void + { + $builder->expects($this->once()) + ->method('add'); + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_create_stub.php.inc b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_create_stub.php.inc new file mode 100644 index 00000000..00fe6a3f --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_create_stub.php.inc @@ -0,0 +1,21 @@ +createStub(\stdClass::class); + $this->prepareServiceMock($someService); + } + + private function prepareServiceMock(MockObject $someService): void + { + $someService->expects($this->once()) + ->method('getId'); + } +} diff --git a/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_different_mocked_classes.php.inc b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_different_mocked_classes.php.inc new file mode 100644 index 00000000..120ddb0f --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_different_mocked_classes.php.inc @@ -0,0 +1,27 @@ +createMock(\stdClass::class); + $this->prepareServiceMock($someService); + } + + public function testSecond(): void + { + $someService = $this->createMock(\DateTime::class); + $this->prepareServiceMock($someService); + } + + private function prepareServiceMock(MockObject $someService): void + { + $someService->expects($this->once()) + ->method('getId'); + } +} diff --git a/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_existing_param_doc.php.inc b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_existing_param_doc.php.inc new file mode 100644 index 00000000..d21b5520 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_existing_param_doc.php.inc @@ -0,0 +1,24 @@ +createMock(\stdClass::class); + $this->prepareServiceMock($someService); + } + + /** + * @param MockObject $someService + */ + private function prepareServiceMock(MockObject $someService): void + { + $someService->expects($this->once()) + ->method('getId'); + } +} diff --git a/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_no_call_site.php.inc b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_no_call_site.php.inc new file mode 100644 index 00000000..aa1b8aa0 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_no_call_site.php.inc @@ -0,0 +1,15 @@ +expects($this->once()) + ->method('getId'); + } +} diff --git a/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_non_test_class.php.inc b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_non_test_class.php.inc new file mode 100644 index 00000000..fb5f9c55 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_non_test_class.php.inc @@ -0,0 +1,20 @@ +createMock(\stdClass::class); + $this->prepareServiceMock($someService); + } + + private function prepareServiceMock(MockObject $someService): void + { + $someService->expects($this->once()) + ->method('getId'); + } +} diff --git a/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_public_method.php.inc b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_public_method.php.inc new file mode 100644 index 00000000..4703ffa0 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_public_method.php.inc @@ -0,0 +1,21 @@ +createMock(\stdClass::class); + $this->prepareServiceMock($someService); + } + + public function prepareServiceMock(MockObject $someService): void + { + $someService->expects($this->once()) + ->method('getId'); + } +} diff --git a/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_reassigned_variable.php.inc b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_reassigned_variable.php.inc new file mode 100644 index 00000000..073b57f2 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/Fixture/skip_reassigned_variable.php.inc @@ -0,0 +1,23 @@ +createMock(\stdClass::class); + $someService = $this->createMock(\DateTime::class); + + $this->prepareServiceMock($someService); + } + + private function prepareServiceMock(MockObject $someService): void + { + $someService->expects($this->once()) + ->method('getId'); + } +} diff --git a/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/config/configured_rule.php b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/config/configured_rule.php new file mode 100644 index 00000000..6901cfeb --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector/config/configured_rule.php @@ -0,0 +1,9 @@ +withRules([AddIntersectionParamToMockObjectParamRector::class]); diff --git a/rules/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector.php b/rules/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector.php new file mode 100644 index 00000000..f56f382e --- /dev/null +++ b/rules/CodeQuality/Rector/Class_/AddIntersectionParamToMockObjectParamRector.php @@ -0,0 +1,378 @@ +=11.0'); + } + + public function getRuleDefinition(): RuleDefinition + { + return new RuleDefinition( + 'Add a MockObject intersection @param docblock with the mocked class, based on the mock passed in the private method call', + [ + new CodeSample( + <<<'CODE_SAMPLE' +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; + +final class SomeTest extends TestCase +{ + public function test(): void + { + $someService = $this->createMock(SomeService::class); + $this->prepareServiceMock($someService); + } + + private function prepareServiceMock(MockObject $someService): void + { + $someService->expects($this->once()) + ->method('getId'); + } +} +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; + +final class SomeTest extends TestCase +{ + public function test(): void + { + $someService = $this->createMock(SomeService::class); + $this->prepareServiceMock($someService); + } + + /** + * @param \SomeService&\PHPUnit\Framework\MockObject\MockObject $someService + */ + private function prepareServiceMock(MockObject $someService): void + { + $someService->expects($this->once()) + ->method('getId'); + } +} +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; + } + + $mockObjectParamClassMethods = $this->collectMockObjectParamClassMethods($node); + if ($mockObjectParamClassMethods === []) { + return null; + } + + $mockedClassesByMethodName = $this->resolveMockedClassesFromCallSites( + $node, + array_keys($mockObjectParamClassMethods) + ); + + $hasChanged = false; + + foreach ($mockObjectParamClassMethods as $methodName => $classMethod) { + $mockedClassesByPosition = $mockedClassesByMethodName[$methodName] ?? []; + + foreach ($classMethod->params as $position => $param) { + if (! $this->isBareMockObjectParam($param)) { + continue; + } + + $mockedClass = $this->resolveSingleMockedClass($mockedClassesByPosition[$position] ?? []); + if ($mockedClass === null) { + continue; + } + + $paramName = $this->getName($param->var); + if ($paramName === null) { + continue; + } + + $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($classMethod); + + // do not contradict an existing docblock type + if ($phpDocInfo->getParamTagValueByName($paramName) instanceof ParamTagValueNode) { + continue; + } + + $intersectionTypeNode = new BracketsAwareIntersectionTypeNode([ + new IdentifierTypeNode('\\' . $mockedClass), + new IdentifierTypeNode('\\' . PHPUnitClassName::MOCK_OBJECT), + ]); + + $this->phpDocTypeChanger->changeParamTypeNode( + $classMethod, + $phpDocInfo, + $param, + $paramName, + $intersectionTypeNode + ); + + $hasChanged = true; + } + } + + if (! $hasChanged) { + return null; + } + + return $node; + } + + /** + * @return array + */ + private function collectMockObjectParamClassMethods(Class_ $class): array + { + $mockObjectParamClassMethods = []; + + foreach ($class->getMethods() as $classMethod) { + // only private methods, as those can only be called from the very same class + if (! $classMethod->isPrivate()) { + continue; + } + + if ($classMethod->stmts === null) { + continue; + } + + foreach ($classMethod->params as $param) { + if (! $this->isBareMockObjectParam($param)) { + continue; + } + + $mockObjectParamClassMethods[$this->getName($classMethod)] = $classMethod; + break; + } + } + + return $mockObjectParamClassMethods; + } + + /** + * @param string[] $methodNames + * @return array>> + */ + private function resolveMockedClassesFromCallSites(Class_ $class, array $methodNames): array + { + $mockedClassesByMethodName = []; + + foreach ($class->getMethods() as $classMethod) { + if ($classMethod->stmts === null) { + continue; + } + + $mockedClassesByVariableName = $this->resolveMockedClassesByVariableName($classMethod); + + /** @var MethodCall[] $methodCalls */ + $methodCalls = $this->betterNodeFinder->findInstancesOfScoped( + (array) $classMethod->stmts, + MethodCall::class + ); + + foreach ($methodCalls as $methodCall) { + if (! $this->isName($methodCall->var, 'this')) { + continue; + } + + $calledMethodName = $this->getName($methodCall->name); + if (! in_array($calledMethodName, $methodNames, true)) { + continue; + } + + foreach ($methodCall->getArgs() as $position => $arg) { + $mockedClassesByMethodName[$calledMethodName][$position][] = $this->resolveArgMockedClass( + $arg, + $mockedClassesByVariableName + ); + } + } + } + + return $mockedClassesByMethodName; + } + + /** + * @param array $mockedClassesByVariableName + */ + private function resolveArgMockedClass(Arg $arg, array $mockedClassesByVariableName): ?string + { + // named args make the position unreliable + if ($arg->name instanceof Identifier) { + return null; + } + + if ($arg->unpack) { + return null; + } + + if ($arg->value instanceof Variable) { + $variableName = $this->getName($arg->value); + if ($variableName !== null) { + return $mockedClassesByVariableName[$variableName] ?? null; + } + + return null; + } + + return $this->resolveCreateMockClass($arg->value); + } + + /** + * Variables assigned exactly once from a createMock() call + * + * @return array + */ + private function resolveMockedClassesByVariableName(ClassMethod $classMethod): array + { + $mockedClassesByVariableName = []; + + /** @var Assign[] $assigns */ + $assigns = $this->betterNodeFinder->findInstancesOfScoped((array) $classMethod->stmts, Assign::class); + + foreach ($assigns as $assign) { + if (! $assign->var instanceof Variable) { + continue; + } + + $variableName = $this->getName($assign->var); + if ($variableName === null) { + continue; + } + + // re-assigned variable, the type is not reliable + if (array_key_exists($variableName, $mockedClassesByVariableName)) { + $mockedClassesByVariableName[$variableName] = null; + continue; + } + + $mockedClassesByVariableName[$variableName] = $this->resolveCreateMockClass($assign->expr); + } + + return $mockedClassesByVariableName; + } + + private function resolveCreateMockClass(Expr $expr): ?string + { + // both $this->createMock() and self::createMock() + if (! $expr instanceof MethodCall && ! $expr instanceof StaticCall) { + return null; + } + + if (! $this->isName($expr->name, 'createMock')) { + return null; + } + + $firstArg = $expr->getArgs()[0] ?? null; + if (! $firstArg instanceof Arg) { + return null; + } + + if (! $firstArg->value instanceof ClassConstFetch) { + return null; + } + + $className = $this->getName($firstArg->value->class); + if (! is_string($className)) { + return null; + } + + return $className; + } + + /** + * @param array $mockedClasses + */ + private function resolveSingleMockedClass(array $mockedClasses): ?string + { + if ($mockedClasses === []) { + return null; + } + + // every call site must pass a mock of the very same class + $uniqueMockedClasses = array_unique($mockedClasses, SORT_REGULAR); + if (count($uniqueMockedClasses) !== 1) { + return null; + } + + return array_pop($uniqueMockedClasses); + } + + private function isBareMockObjectParam(Param $param): bool + { + if ($param->variadic) { + return false; + } + + if (! $param->var instanceof Variable) { + return false; + } + + if (! $param->type instanceof Name) { + return false; + } + + return $this->isName($param->type, PHPUnitClassName::MOCK_OBJECT); + } +}