From a1257a3cc199b596222487254f20f63cd4e6f1a5 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 31 Jul 2026 23:39:59 +0200 Subject: [PATCH] trait data provdiers --- config/sets/phpunit-mock-to-stub.php | 4 + .../Fixture/in_test_trait.php.inc | 29 ++ .../skip_non_public_trait_method.php.inc | 13 + .../Fixture/skip_static_trait_method.php.inc | 13 + .../Fixture/fixture.php.inc | 51 ++++ .../intersection_docblock_param.php.inc | 57 ++++ .../Fixture/named_arg.php.inc | 49 ++++ .../Fixture/skip_plain_param_type.php.inc | 20 ++ .../Fixture/skip_stub_only_usage.php.inc | 17 ++ ...ectArgCreateStubToCreateMockRectorTest.php | 28 ++ .../Source/SomeUser.php | 13 + .../config/configured_rule.php | 10 + ...ataProviderAnnotationToAttributeRector.php | 12 - ...kObjectArgCreateStubToCreateMockRector.php | 249 ++++++++++++++++++ src/NodeAnalyzer/TestsNodeAnalyzer.php | 44 ++++ 15 files changed, 597 insertions(+), 12 deletions(-) create mode 100644 rules-tests/AnnotationsToAttributes/Rector/ClassMethod/DataProviderAnnotationToAttributeRector/Fixture/in_test_trait.php.inc create mode 100644 rules-tests/AnnotationsToAttributes/Rector/ClassMethod/DataProviderAnnotationToAttributeRector/Fixture/skip_non_public_trait_method.php.inc create mode 100644 rules-tests/AnnotationsToAttributes/Rector/ClassMethod/DataProviderAnnotationToAttributeRector/Fixture/skip_static_trait_method.php.inc create mode 100644 rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Fixture/fixture.php.inc create mode 100644 rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Fixture/intersection_docblock_param.php.inc create mode 100644 rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Fixture/named_arg.php.inc create mode 100644 rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Fixture/skip_plain_param_type.php.inc create mode 100644 rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Fixture/skip_stub_only_usage.php.inc create mode 100644 rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/MockObjectArgCreateStubToCreateMockRectorTest.php create mode 100644 rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Source/SomeUser.php create mode 100644 rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/config/configured_rule.php create mode 100644 rules/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector.php diff --git a/config/sets/phpunit-mock-to-stub.php b/config/sets/phpunit-mock-to-stub.php index a2fee2ea..985a9ebe 100644 --- a/config/sets/phpunit-mock-to-stub.php +++ b/config/sets/phpunit-mock-to-stub.php @@ -6,6 +6,7 @@ use Rector\PHPUnit\CodeQuality\Rector\Class_\AddIntersectionVarToMockObjectPropertyRector; use Rector\PHPUnit\CodeQuality\Rector\Class_\AddStubIntersectionVarToStubPropertyRector; use Rector\PHPUnit\CodeQuality\Rector\ClassMethod\BareCreateMockAssignToDirectUseRector; +use Rector\PHPUnit\PHPUnit110\Rector\ClassMethod\MockObjectArgCreateStubToCreateMockRector; use Rector\PHPUnit\PHPUnit120\Rector\CallLike\CreateStubInCoalesceArgRector; use Rector\PHPUnit\PHPUnit120\Rector\CallLike\CreateStubOverCreateMockArgRector; use Rector\PHPUnit\PHPUnit120\Rector\Class_\PropertyCreateMockToCreateStubRector; @@ -23,5 +24,8 @@ AddIntersectionVarToMockObjectPropertyRector::class, AddStubIntersectionVarToStubPropertyRector::class, BareCreateMockAssignToDirectUseRector::class, + + // mocks back over stubs, where mock object is required + MockObjectArgCreateStubToCreateMockRector::class, ]); }; diff --git a/rules-tests/AnnotationsToAttributes/Rector/ClassMethod/DataProviderAnnotationToAttributeRector/Fixture/in_test_trait.php.inc b/rules-tests/AnnotationsToAttributes/Rector/ClassMethod/DataProviderAnnotationToAttributeRector/Fixture/in_test_trait.php.inc new file mode 100644 index 00000000..80b3a76c --- /dev/null +++ b/rules-tests/AnnotationsToAttributes/Rector/ClassMethod/DataProviderAnnotationToAttributeRector/Fixture/in_test_trait.php.inc @@ -0,0 +1,29 @@ + +----- + diff --git a/rules-tests/AnnotationsToAttributes/Rector/ClassMethod/DataProviderAnnotationToAttributeRector/Fixture/skip_non_public_trait_method.php.inc b/rules-tests/AnnotationsToAttributes/Rector/ClassMethod/DataProviderAnnotationToAttributeRector/Fixture/skip_non_public_trait_method.php.inc new file mode 100644 index 00000000..907a7df0 --- /dev/null +++ b/rules-tests/AnnotationsToAttributes/Rector/ClassMethod/DataProviderAnnotationToAttributeRector/Fixture/skip_non_public_trait_method.php.inc @@ -0,0 +1,13 @@ +createStub(SomeUser::class); + $this->prepareUserMock($user); + } + + private function prepareUserMock(MockObject $user): void + { + $user->expects($this->once()) + ->method('getId') + ->willReturn(1); + } +} + +?> +----- +createMock(SomeUser::class); + $this->prepareUserMock($user); + } + + private function prepareUserMock(MockObject $user): void + { + $user->expects($this->once()) + ->method('getId') + ->willReturn(1); + } +} + +?> diff --git a/rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Fixture/intersection_docblock_param.php.inc b/rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Fixture/intersection_docblock_param.php.inc new file mode 100644 index 00000000..66a446b5 --- /dev/null +++ b/rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Fixture/intersection_docblock_param.php.inc @@ -0,0 +1,57 @@ +createStub(SomeUser::class); + $this->prepareCommonMocks('some_event', $user); + } + + /** + * @param MockObject&SomeUser $user + */ + private function prepareCommonMocks(string $event, MockObject $user): void + { + $user->expects($this->once()) + ->method('getId') + ->willReturn(1); + } +} + +?> +----- +createMock(SomeUser::class); + $this->prepareCommonMocks('some_event', $user); + } + + /** + * @param MockObject&SomeUser $user + */ + private function prepareCommonMocks(string $event, MockObject $user): void + { + $user->expects($this->once()) + ->method('getId') + ->willReturn(1); + } +} + +?> diff --git a/rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Fixture/named_arg.php.inc b/rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Fixture/named_arg.php.inc new file mode 100644 index 00000000..a2122b6a --- /dev/null +++ b/rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Fixture/named_arg.php.inc @@ -0,0 +1,49 @@ +createStub(SomeUser::class); + $this->prepareUserMock(user: $user); + } + + private function prepareUserMock(MockObject $user): void + { + $user->expects($this->once()) + ->method('getId'); + } +} + +?> +----- +createMock(SomeUser::class); + $this->prepareUserMock(user: $user); + } + + private function prepareUserMock(MockObject $user): void + { + $user->expects($this->once()) + ->method('getId'); + } +} + +?> diff --git a/rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Fixture/skip_plain_param_type.php.inc b/rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Fixture/skip_plain_param_type.php.inc new file mode 100644 index 00000000..25963e41 --- /dev/null +++ b/rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Fixture/skip_plain_param_type.php.inc @@ -0,0 +1,20 @@ +createStub(SomeUser::class); + $this->useUser($user); + } + + private function useUser(SomeUser $user): void + { + $user->getId(); + } +} diff --git a/rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Fixture/skip_stub_only_usage.php.inc b/rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Fixture/skip_stub_only_usage.php.inc new file mode 100644 index 00000000..ce44db1c --- /dev/null +++ b/rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Fixture/skip_stub_only_usage.php.inc @@ -0,0 +1,17 @@ +createStub(SomeUser::class); + $user->method('getId')->willReturn(1); + + $this->assertSame(1, $user->getId()); + } +} diff --git a/rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/MockObjectArgCreateStubToCreateMockRectorTest.php b/rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/MockObjectArgCreateStubToCreateMockRectorTest.php new file mode 100644 index 00000000..f74216af --- /dev/null +++ b/rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/MockObjectArgCreateStubToCreateMockRectorTest.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/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Source/SomeUser.php b/rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Source/SomeUser.php new file mode 100644 index 00000000..ec86aafd --- /dev/null +++ b/rules-tests/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector/Source/SomeUser.php @@ -0,0 +1,13 @@ +rule(MockObjectArgCreateStubToCreateMockRector::class); +}; diff --git a/rules/AnnotationsToAttributes/Rector/ClassMethod/DataProviderAnnotationToAttributeRector.php b/rules/AnnotationsToAttributes/Rector/ClassMethod/DataProviderAnnotationToAttributeRector.php index 735c4e58..9eeb7662 100644 --- a/rules/AnnotationsToAttributes/Rector/ClassMethod/DataProviderAnnotationToAttributeRector.php +++ b/rules/AnnotationsToAttributes/Rector/ClassMethod/DataProviderAnnotationToAttributeRector.php @@ -9,7 +9,6 @@ use PhpParser\Node\Stmt\ClassMethod; use PHPStan\PhpDocParser\Ast\PhpDoc\GenericTagValueNode; use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode; -use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\ReflectionProvider; use Rector\BetterPhpDocParser\PhpDoc\DoctrineAnnotationTagValueNode; use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo; @@ -20,7 +19,6 @@ use Rector\PHPUnit\Enum\PHPUnitAttribute; use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer; use Rector\Rector\AbstractRector; -use Rector\Reflection\ReflectionResolver; use Rector\ValueObject\PhpVersion; use Rector\VersionBonding\Contract\MinPhpVersionInterface; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; @@ -35,7 +33,6 @@ public function __construct( private readonly TestsNodeAnalyzer $testsNodeAnalyzer, private readonly PhpAttributeGroupFactory $phpAttributeGroupFactory, private readonly PhpDocTagRemover $phpDocTagRemover, - private readonly ReflectionResolver $reflectionResolver, private readonly DocBlockUpdater $docBlockUpdater, private readonly PhpDocInfoFactory $phpDocInfoFactory, private readonly ReflectionProvider $reflectionProvider @@ -119,15 +116,6 @@ public function refactor(Node $node): ?Node return null; } - $classReflection = $this->reflectionResolver->resolveClassReflection($node); - if (! $classReflection instanceof ClassReflection) { - return null; - } - - if (! $classReflection->isClass()) { - return null; - } - foreach ($desiredTagValueNodes as $desiredTagValueNode) { if (! $desiredTagValueNode->value instanceof GenericTagValueNode && ! $desiredTagValueNode->value instanceof DoctrineAnnotationTagValueNode) { continue; diff --git a/rules/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector.php b/rules/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector.php new file mode 100644 index 00000000..54a7b9ee --- /dev/null +++ b/rules/PHPUnit110/Rector/ClassMethod/MockObjectArgCreateStubToCreateMockRector.php @@ -0,0 +1,249 @@ +=11.0'); + } + + public function getRuleDefinition(): RuleDefinition + { + return new RuleDefinition( + 'Change createStub() to createMock(), when the created variable is passed to a method that requires MockObject type', + [ + new CodeSample( + <<<'CODE_SAMPLE' +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; + +final class SomeTest extends TestCase +{ + public function test() + { + $someMock = $this->createStub(SomeClass::class); + $this->prepareMock($someMock); + } + + private function prepareMock(MockObject $someMock): void + { + $someMock->expects($this->once()) + ->method('someMethod'); + } +} +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; + +final class SomeTest extends TestCase +{ + public function test() + { + $someMock = $this->createMock(SomeClass::class); + $this->prepareMock($someMock); + } + + private function prepareMock(MockObject $someMock): void + { + $someMock->expects($this->once()) + ->method('someMethod'); + } +} +CODE_SAMPLE + ), + ] + ); + } + + /** + * @return array> + */ + public function getNodeTypes(): array + { + return [ClassMethod::class]; + } + + /** + * @param ClassMethod $node + */ + public function refactor(Node $node): ?ClassMethod + { + if (! $this->testsNodeAnalyzer->isInTestClass($node)) { + return null; + } + + $createStubMethodCallsByVariableName = $this->collectCreateStubAssigns($node); + if ($createStubMethodCallsByVariableName === []) { + return null; + } + + $hasChanged = false; + + foreach ($createStubMethodCallsByVariableName as $variableName => $createStubMethodCall) { + if (! $this->isPassedAsMockObjectArg($node, (string) $variableName)) { + continue; + } + + $createStubMethodCall->name = new Identifier('createMock'); + $hasChanged = true; + } + + if ($hasChanged) { + return $node; + } + + return null; + } + + /** + * @return array + */ + private function collectCreateStubAssigns(ClassMethod $classMethod): array + { + $createStubMethodCallsByVariableName = []; + + foreach ((array) $classMethod->stmts as $stmt) { + if (! $stmt instanceof Expression) { + continue; + } + + if (! $stmt->expr instanceof Assign) { + continue; + } + + $assign = $stmt->expr; + if (! $assign->var instanceof Variable) { + continue; + } + + if (! $assign->expr instanceof MethodCall) { + continue; + } + + $methodCall = $assign->expr; + if (! $this->isName($methodCall->name, 'createStub')) { + continue; + } + + $variableName = $this->getName($assign->var); + if ($variableName === null) { + continue; + } + + $createStubMethodCallsByVariableName[$variableName] = $methodCall; + } + + return $createStubMethodCallsByVariableName; + } + + private function isPassedAsMockObjectArg(ClassMethod $classMethod, string $variableName): bool + { + /** @var array $callLikes */ + $callLikes = $this->betterNodeFinder->findInstancesOfScoped( + (array) $classMethod->stmts, + [MethodCall::class, StaticCall::class] + ); + + foreach ($callLikes as $callLike) { + if ($callLike->isFirstClassCallable()) { + continue; + } + + foreach ($callLike->getArgs() as $argIndex => $arg) { + if (! $arg->value instanceof Variable) { + continue; + } + + if (! $this->isName($arg->value, $variableName)) { + continue; + } + + if ($this->isMockObjectParam($callLike, $arg->name, $argIndex)) { + return true; + } + } + } + + return false; + } + + private function isMockObjectParam( + MethodCall|StaticCall $callLike, + ?Identifier $identifier, + int $argIndex + ): bool { + $methodReflection = $callLike instanceof MethodCall + ? $this->reflectionResolver->resolveMethodReflectionFromMethodCall($callLike) + : $this->reflectionResolver->resolveMethodReflectionFromStaticCall($callLike); + + if (! $methodReflection instanceof MethodReflection) { + return false; + } + + $mockObjectType = new ObjectType(PHPUnitClassName::MOCK_OBJECT); + + foreach ($methodReflection->getVariants() as $parametersAcceptor) { + $parameters = $parametersAcceptor->getParameters(); + + if ($identifier instanceof Identifier) { + foreach ($parameters as $parameter) { + if ($parameter->getName() !== $identifier->toString()) { + continue; + } + + if ($mockObjectType->isSuperTypeOf($parameter->getType())->yes()) { + return true; + } + } + + continue; + } + + if (! isset($parameters[$argIndex])) { + continue; + } + + if ($mockObjectType->isSuperTypeOf($parameters[$argIndex]->getType())->yes()) { + return true; + } + } + + return false; + } +} diff --git a/src/NodeAnalyzer/TestsNodeAnalyzer.php b/src/NodeAnalyzer/TestsNodeAnalyzer.php index b6cc8d8f..cf3d2790 100644 --- a/src/NodeAnalyzer/TestsNodeAnalyzer.php +++ b/src/NodeAnalyzer/TestsNodeAnalyzer.php @@ -35,6 +35,11 @@ public function isInTestClass(Node $node): bool return false; } + // traits have no parent, so the test case check below can never match them + if ($classReflection->isTrait()) { + return $this->isInTestTrait($classReflection, $node); + } + return array_any( PHPUnitClassName::TEST_CLASSES, fn (string $testCaseObjectClass): bool => $classReflection->is($testCaseObjectClass) @@ -83,6 +88,45 @@ public function isAssertMethodCallName(Node $node, string $name): bool return $this->nodeNameResolver->isName($node->name, $name); } + /** + * Test traits live next to the test cases that use them, so the namespace is the main hint. + * Only public non-static methods can be test methods, and a "test" prefixed one is a test + * method even outside a tests namespace. + */ + private function isInTestTrait(ClassReflection $classReflection, Node $node): bool + { + if (! $node instanceof ClassMethod) { + return $this->isInTestsNamespace($classReflection); + } + + if (! $node->isPublic()) { + return false; + } + + if ($node->isStatic()) { + return false; + } + + if ($this->isInTestsNamespace($classReflection)) { + return true; + } + + return str_starts_with($node->name->toString(), 'test'); + } + + private function isInTestsNamespace(ClassReflection $classReflection): bool + { + $nameParts = explode('\\', $classReflection->getName()); + + // drop the short trait name, only the namespace matters here + array_pop($nameParts); + + return array_any( + $nameParts, + static fn (string $namePart): bool => in_array($namePart, ['Test', 'Tests'], true) + ); + } + /** * @param string[] $names */