diff --git a/config/sets/composer-based.php b/config/sets/composer-based.php index d97e81d0..0140d5f6 100644 --- a/config/sets/composer-based.php +++ b/config/sets/composer-based.php @@ -16,6 +16,7 @@ use Rector\PHPUnit\CodeQuality\Rector\Class_\AddStubIntersectionVarToStubPropertyRector; use Rector\PHPUnit\CodeQuality\Rector\ClassMethod\BareCreateMockAssignToDirectUseRector; use Rector\PHPUnit\PHPUnit110\Rector\CallLike\AssertContainsOnlyMethodCallRector; +use Rector\PHPUnit\PHPUnit110\Rector\ClassMethod\ExpectsParamToMockObjectRector; use Rector\PHPUnit\PHPUnit110\Rector\ClassMethod\MockObjectArgCreateStubToCreateMockRector; use Rector\PHPUnit\PHPUnit120\Rector\CallLike\CreateStubInCoalesceArgRector; use Rector\PHPUnit\PHPUnit120\Rector\CallLike\CreateStubOverCreateMockArgRector; @@ -55,6 +56,7 @@ // mocks back over stubs, where a mock object is required MockObjectArgCreateStubToCreateMockRector::class, + ExpectsParamToMockObjectRector::class, // deprecated in PHPUnit 11.5 AssertContainsOnlyMethodCallRector::class, diff --git a/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/ExpectsParamToMockObjectRectorTest.php b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/ExpectsParamToMockObjectRectorTest.php new file mode 100644 index 00000000..898988f7 --- /dev/null +++ b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/ExpectsParamToMockObjectRectorTest.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/ExpectsParamToMockObjectRector/Fixture/fixture.php.inc b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/fixture.php.inc new file mode 100644 index 00000000..3586752a --- /dev/null +++ b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/fixture.php.inc @@ -0,0 +1,37 @@ +expects($this->once()) + ->method('getId') + ->willReturn(1); + } +} + +?> +----- +expects($this->once()) + ->method('getId') + ->willReturn(1); + } +} + +?> diff --git a/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/only_expects_param.php.inc b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/only_expects_param.php.inc new file mode 100644 index 00000000..227e6820 --- /dev/null +++ b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/only_expects_param.php.inc @@ -0,0 +1,39 @@ +getId(); + + $user->expects($this->once()) + ->method('getId'); + } +} + +?> +----- +getId(); + + $user->expects($this->once()) + ->method('getId'); + } +} + +?> diff --git a/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_docblock_param.php.inc b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_docblock_param.php.inc new file mode 100644 index 00000000..6bd0f2af --- /dev/null +++ b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_docblock_param.php.inc @@ -0,0 +1,18 @@ +expects($this->once()) + ->method('getId'); + } +} diff --git a/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_intersection_param.php.inc b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_intersection_param.php.inc new file mode 100644 index 00000000..e1497e9d --- /dev/null +++ b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_intersection_param.php.inc @@ -0,0 +1,16 @@ +expects($this->once()) + ->method('getId'); + } +} diff --git a/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_mock_object_param.php.inc b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_mock_object_param.php.inc new file mode 100644 index 00000000..6a94e609 --- /dev/null +++ b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_mock_object_param.php.inc @@ -0,0 +1,15 @@ +expects($this->once()) + ->method('getId'); + } +} diff --git a/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_no_expects.php.inc b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_no_expects.php.inc new file mode 100644 index 00000000..16fe3ae2 --- /dev/null +++ b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_no_expects.php.inc @@ -0,0 +1,14 @@ +getId(); + } +} diff --git a/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_non_test_class.php.inc b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_non_test_class.php.inc new file mode 100644 index 00000000..395a9813 --- /dev/null +++ b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_non_test_class.php.inc @@ -0,0 +1,14 @@ +expects($this->once()) + ->method('getId'); + } +} diff --git a/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_public_method.php.inc b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_public_method.php.inc new file mode 100644 index 00000000..c6bb1160 --- /dev/null +++ b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Fixture/skip_public_method.php.inc @@ -0,0 +1,15 @@ +expects($this->once()) + ->method('getId'); + } +} diff --git a/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Source/SomeUser.php b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Source/SomeUser.php new file mode 100644 index 00000000..988bf5d1 --- /dev/null +++ b/rules-tests/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector/Source/SomeUser.php @@ -0,0 +1,13 @@ +rule(ExpectsParamToMockObjectRector::class); +}; diff --git a/rules/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector.php b/rules/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector.php new file mode 100644 index 00000000..25bbee79 --- /dev/null +++ b/rules/PHPUnit110/Rector/ClassMethod/ExpectsParamToMockObjectRector.php @@ -0,0 +1,188 @@ +=11.0'); + } + + public function getRuleDefinition(): RuleDefinition + { + return new RuleDefinition( + 'Change param type of a private test method to MockObject, when expects() is called on it', + [ + new CodeSample( + <<<'CODE_SAMPLE' +use PHPUnit\Framework\TestCase; + +final class SomeTest extends TestCase +{ + private function prepareUserMock(SomeUser $user): void + { + $user->expects($this->once()) + ->method('getId'); + } +} +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; + +final class SomeTest extends TestCase +{ + private function prepareUserMock(MockObject $user): void + { + $user->expects($this->once()) + ->method('getId'); + } +} +CODE_SAMPLE + ), + ] + ); + } + + /** + * @return array> + */ + public function getNodeTypes(): array + { + return [ClassMethod::class]; + } + + /** + * @param ClassMethod $node + */ + public function refactor(Node $node): ?ClassMethod + { + if (! $node->isPrivate()) { + return null; + } + + if ($node->stmts === null || $node->params === []) { + return null; + } + + if (! $this->testsNodeAnalyzer->isInTestClass($node)) { + return null; + } + + $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node); + + $hasChanged = false; + + foreach ($node->params as $param) { + if (! $param->var instanceof Variable) { + continue; + } + + $paramName = $this->getName($param->var); + if ($paramName === null) { + continue; + } + + if ($this->hasMockObjectType($param->type)) { + continue; + } + + // avoid contradicting the docblock type, that would have to be changed as well + if ($phpDocInfo->getParamTagValueByName($paramName) instanceof ParamTagValueNode) { + continue; + } + + if (! $this->isExpectsCalledOnVariable($node, $paramName)) { + continue; + } + + $param->type = new FullyQualified(PHPUnitClassName::MOCK_OBJECT); + $hasChanged = true; + } + + if (! $hasChanged) { + return null; + } + + return $node; + } + + private function hasMockObjectType(null|Identifier|Name|ComplexType $type): bool + { + if ($type instanceof Name) { + return $this->isName($type, PHPUnitClassName::MOCK_OBJECT); + } + + if ($type instanceof NullableType) { + return $this->hasMockObjectType($type->type); + } + + if ($type instanceof UnionType || $type instanceof IntersectionType) { + foreach ($type->types as $singleType) { + if ($this->hasMockObjectType($singleType)) { + return true; + } + } + } + + return false; + } + + private function isExpectsCalledOnVariable(ClassMethod $classMethod, string $paramName): bool + { + /** @var MethodCall[] $methodCalls */ + $methodCalls = $this->betterNodeFinder->findInstancesOfScoped((array) $classMethod->stmts, MethodCall::class); + + foreach ($methodCalls as $methodCall) { + if (! $methodCall->var instanceof Variable) { + continue; + } + + if (! $this->isName($methodCall->var, $paramName)) { + continue; + } + + if ($this->isName($methodCall->name, 'expects')) { + return true; + } + } + + return false; + } +}