diff --git a/config/sets/phpunit-code-quality.php b/config/sets/phpunit-code-quality.php index 277318ed..6b2c801b 100644 --- a/config/sets/phpunit-code-quality.php +++ b/config/sets/phpunit-code-quality.php @@ -51,6 +51,7 @@ use Rector\PHPUnit\CodeQuality\Rector\MethodCall\WithCallbackIdenticalToStandaloneAssertsRector; use Rector\PHPUnit\CodeQuality\Rector\StmtsAwareInterface\DeclareStrictTypesTestsRector; use Rector\PHPUnit\PHPUnit120\Rector\CallLike\CreateStubOverCreateMockArgRector; +use Rector\PHPUnit\PHPUnit120\Rector\Class_\PropertyCreateMockToCreateStubRector; use Rector\PHPUnit\PHPUnit120\Rector\ClassMethod\ExpressionCreateMockToCreateStubRector; use Rector\PHPUnit\PHPUnit60\Rector\MethodCall\GetMockBuilderGetMockToCreateMockRector; use Rector\PHPUnit\PHPUnit90\Rector\MethodCall\ReplaceAtMethodWithDesiredMatcherRector; @@ -130,6 +131,7 @@ // stub over mock CreateStubOverCreateMockArgRector::class, ExpressionCreateMockToCreateStubRector::class, + PropertyCreateMockToCreateStubRector::class, // @test first, enable later // \Rector\PHPUnit\CodeQuality\Rector\Expression\ConfiguredMockEntityToSetterObjectRector::class, diff --git a/config/sets/phpunit120.php b/config/sets/phpunit120.php index 5f3cec26..bf480d6a 100644 --- a/config/sets/phpunit120.php +++ b/config/sets/phpunit120.php @@ -5,6 +5,7 @@ use Rector\Config\RectorConfig; use Rector\PHPUnit\PHPUnit120\Rector\CallLike\CreateStubOverCreateMockArgRector; use Rector\PHPUnit\PHPUnit120\Rector\Class_\AssertIsTypeMethodCallRector; +use Rector\PHPUnit\PHPUnit120\Rector\Class_\PropertyCreateMockToCreateStubRector; use Rector\PHPUnit\PHPUnit120\Rector\Class_\RemoveOverrideFinalConstructTestCaseRector; use Rector\PHPUnit\PHPUnit120\Rector\ClassMethod\ExpressionCreateMockToCreateStubRector; @@ -16,6 +17,7 @@ // stubs over mocks CreateStubOverCreateMockArgRector::class, ExpressionCreateMockToCreateStubRector::class, + PropertyCreateMockToCreateStubRector::class, // experimental, from PHPUnit 12.5.2 // @see https://github.com/sebastianbergmann/phpunit/commit/24c208d6a340c3071f28a9b5cce02b9377adfd43 diff --git a/rules-tests/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector/Fixture/fixture.php.inc b/rules-tests/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector/Fixture/fixture.php.inc new file mode 100644 index 00000000..4b28ea54 --- /dev/null +++ b/rules-tests/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector/Fixture/fixture.php.inc @@ -0,0 +1,55 @@ +someMock = $this->createMock(\stdClass::class); + } + + public function testThis() + { + $this->assertSame('...', $this->someMock); + } + + public function testThat() + { + $this->assertSame('...', $this->someMock); + } +} + +?> +----- +someMock = $this->createStub(\stdClass::class); + } + + public function testThis() + { + $this->assertSame('...', $this->someMock); + } + + public function testThat() + { + $this->assertSame('...', $this->someMock); + } +} + +?> diff --git a/rules-tests/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector/Fixture/skip_property_used.php.inc b/rules-tests/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector/Fixture/skip_property_used.php.inc new file mode 100644 index 00000000..cd88b7f2 --- /dev/null +++ b/rules-tests/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector/Fixture/skip_property_used.php.inc @@ -0,0 +1,26 @@ +someMock = $this->createMock(\stdClass::class); + } + + public function testThis() + { + $this->someMock->expects($this->atLeastOnce())->method('something'); + $this->assertSame('...', $this->someMock); + } + + public function testThat() + { + $this->assertSame('...', $this->someMock); + } +} diff --git a/rules-tests/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector/PropertyCreateMockToCreateStubRectorTest.php b/rules-tests/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector/PropertyCreateMockToCreateStubRectorTest.php new file mode 100644 index 00000000..0b2c25bd --- /dev/null +++ b/rules-tests/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector/PropertyCreateMockToCreateStubRectorTest.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/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector/Source/ClassWithDependency.php b/rules-tests/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector/Source/ClassWithDependency.php new file mode 100644 index 00000000..0201b78e --- /dev/null +++ b/rules-tests/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector/Source/ClassWithDependency.php @@ -0,0 +1,16 @@ +dependency; + } +} diff --git a/rules-tests/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector/config/configured_rule.php b/rules-tests/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector/config/configured_rule.php new file mode 100644 index 00000000..9a2816bd --- /dev/null +++ b/rules-tests/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector/config/configured_rule.php @@ -0,0 +1,9 @@ +withRules(rules: [PropertyCreateMockToCreateStubRector::class]); diff --git a/rules/CodeQuality/NodeAnalyser/MockObjectExprDetector.php b/rules/CodeQuality/NodeAnalyser/MockObjectExprDetector.php index 4d485e75..a916ba74 100644 --- a/rules/CodeQuality/NodeAnalyser/MockObjectExprDetector.php +++ b/rules/CodeQuality/NodeAnalyser/MockObjectExprDetector.php @@ -6,7 +6,9 @@ use PhpParser\Node\Expr; use PhpParser\Node\Expr\MethodCall; +use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\Variable; +use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; use Rector\NodeNameResolver\NodeNameResolver; use Rector\PhpParser\Node\BetterNodeFinder; @@ -58,4 +60,25 @@ public function isUsedForMocking(Expr $expr, ClassMethod $classMethod): bool return false; } + + public function isPropertyUsedForMocking(Class_ $class, string $propertyName): bool + { + // find out, how many are used in call likes as args + /** @var array $methodCalls */ + $methodCalls = $this->betterNodeFinder->findInstancesOfScoped($class->getMethods(), [MethodCall::class]); + + foreach ($methodCalls as $methodCall) { + if (! $methodCall->var instanceof PropertyFetch) { + continue; + } + + $propertyFetch = $methodCall->var; + if ($this->nodeNameResolver->isName($propertyFetch->name, $propertyName)) { + // variable is being called on, most like mocking, lets skip + return true; + } + } + + return false; + } } diff --git a/rules/CodeQuality/NodeAnalyser/MockObjectPropertyDetector.php b/rules/CodeQuality/NodeAnalyser/MockObjectPropertyDetector.php index 2e97092c..232bfbe7 100644 --- a/rules/CodeQuality/NodeAnalyser/MockObjectPropertyDetector.php +++ b/rules/CodeQuality/NodeAnalyser/MockObjectPropertyDetector.php @@ -4,12 +4,23 @@ namespace Rector\PHPUnit\CodeQuality\NodeAnalyser; +use PhpParser\Node\Expr\Assign; +use PhpParser\Node\Expr\MethodCall; +use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Name\FullyQualified; +use PhpParser\Node\Stmt\ClassMethod; +use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\Property; +use Rector\NodeNameResolver\NodeNameResolver; use Rector\PHPUnit\Enum\PHPUnitClassName; -final class MockObjectPropertyDetector +final readonly class MockObjectPropertyDetector { + public function __construct( + private NodeNameResolver $nodeNameResolver + ) { + } + public function detect(Property $property): bool { if (! $property->type instanceof FullyQualified) { @@ -18,4 +29,48 @@ public function detect(Property $property): bool return $property->type->toString() === PHPUnitClassName::MOCK_OBJECT; } + + /** + * @return array + */ + public function collectFromClassMethod(ClassMethod $classMethod): array + { + $propertyNamesToCreateMockMethodCalls = []; + + foreach ((array) $classMethod->stmts as $stmt) { + if (! $stmt instanceof Expression) { + continue; + } + + if (! $stmt->expr instanceof Assign) { + continue; + } + + $assign = $stmt->expr; + + if (! $assign->var instanceof PropertyFetch) { + continue; + } + + if (! $assign->expr instanceof MethodCall) { + continue; + } + + $methodCall = $assign->expr; + if (! $this->nodeNameResolver->isName($methodCall->name, 'createMock')) { + continue; + } + + $propertyFetch = $assign->var; + $propertyName = $this->nodeNameResolver->getName($propertyFetch->name); + + if (! is_string($propertyName)) { + continue; + } + + $propertyNamesToCreateMockMethodCalls[$propertyName] = $methodCall; + } + + return $propertyNamesToCreateMockMethodCalls; + } } diff --git a/rules/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector.php b/rules/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector.php new file mode 100644 index 00000000..78e4af34 --- /dev/null +++ b/rules/PHPUnit120/Rector/Class_/PropertyCreateMockToCreateStubRector.php @@ -0,0 +1,153 @@ +shouldSkipClass($node)) { + return null; + } + + /** @var ClassMethod $setUpClassMethod */ + $setUpClassMethod = $node->getMethod(MethodName::SET_UP); + + $propertyNamesToCreateMockMethodCalls = $this->mockObjectPropertyDetector->collectFromClassMethod( + $setUpClassMethod + ); + if ($propertyNamesToCreateMockMethodCalls === []) { + return null; + } + + $hasChanged = false; + + // find property fetch usage, is it exnted with method expectaitions? + foreach ($propertyNamesToCreateMockMethodCalls as $propertyName => $createMockMethodCall) { + if ($this->mockObjectExprDetector->isPropertyUsedForMocking($node, $propertyName)) { + continue; + } + + $createMockMethodCall->name = new Identifier('createStub'); + $hasChanged = true; + + // update property type + $property = $node->getProperty($propertyName); + /** @var Property $property */ + $property->type = new FullyQualified(PHPUnitClassName::STUB); + } + + if (! $hasChanged) { + return null; + } + + return $node; + } + + public function getRuleDefinition(): RuleDefinition + { + return new RuleDefinition( + 'Change mock object property that is never mocked to createStub()', + [ + new CodeSample( + <<<'CODE_SAMPLE' +use PHPUnit\Framework\TestCase; + +final class SomeTest extends TestCase +{ + private \PHPUnit\Framework\MockObject\MockObject $someServiceMock; + + protected function setUp(): void + { + $this->someServiceMock = $this->createMock(SomeService::class); + } + + public function testOne(): void + { + $someObject = new SomeClass($this->someServiceMock); + } + + public function testTwo(): void + { + $someObject = new AnotherClass($this->someServiceMock); + } +} +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +use PHPUnit\Framework\TestCase; + +final class SomeTest extends TestCase +{ + private \PHPUnit\Framework\MockObject\Stub\Stub $someServiceMock; + + protected function setUp(): void + { + $this->someServiceMock = $this->createStub(SomeService::class); + } + + public function testOne(): void + { + $someObject = new SomeClass($this->someServiceMock); + } + + public function testTwo(): void + { + $someObject = new AnotherClass($this->someServiceMock); + } +} +CODE_SAMPLE + ), + ] + ); + } + + private function shouldSkipClass(Class_ $class): bool + { + if (! $this->testsNodeAnalyzer->isInTestClass($class)) { + return true; + } + + $setUpClassMethod = $class->getMethod(MethodName::SET_UP); + + // the setup class method must be here, so we have a place where the createMock() is used + return ! $setUpClassMethod instanceof ClassMethod; + } +} diff --git a/src/Enum/PHPUnitClassName.php b/src/Enum/PHPUnitClassName.php index 5171edfa..0af96492 100644 --- a/src/Enum/PHPUnitClassName.php +++ b/src/Enum/PHPUnitClassName.php @@ -25,6 +25,8 @@ final class PHPUnitClassName public const string MOCK_OBJECT = 'PHPUnit\Framework\MockObject\MockObject'; + public const string STUB = 'PHPUnit\Framework\MockObject\Stub'; + /** * @var string[] */