diff --git a/config/sets/phpunit-code-quality.php b/config/sets/phpunit-code-quality.php index 73ed3bd9..7454a661 100644 --- a/config/sets/phpunit-code-quality.php +++ b/config/sets/phpunit-code-quality.php @@ -10,6 +10,7 @@ use Rector\PHPUnit\CodeQuality\Rector\Class_\InlineStubPropertyToCreateStubMethodCallRector; 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_\SingleMockPropertyTypeRector; use Rector\PHPUnit\CodeQuality\Rector\Class_\TestWithToDataProviderRector; use Rector\PHPUnit\CodeQuality\Rector\Class_\TypeWillReturnCallableArrowFunctionRector; @@ -148,6 +149,7 @@ EntityDocumentCreateMockToDirectNewRector::class, ReplaceAtMethodWithDesiredMatcherRector::class, BareCreateMockAssignToDirectUseRector::class, + RemoveNeverUsedMockPropertyRector::class, // readability NoSetupWithParentCallOverrideRector::class, diff --git a/rules-tests/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector/Fixture/fixture.php.inc b/rules-tests/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector/Fixture/fixture.php.inc new file mode 100644 index 00000000..46a32e72 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector/Fixture/fixture.php.inc @@ -0,0 +1,37 @@ +mockProperty = $this->createMock(\stdClass::class); + $this->mockProperty->expects($this->once()) + ->method('someMethod') + ->willReturn('someValue'); + } +} + +?> +----- + diff --git a/rules-tests/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector/Fixture/skip_if_passed.php.inc b/rules-tests/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector/Fixture/skip_if_passed.php.inc new file mode 100644 index 00000000..09d6fd91 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector/Fixture/skip_if_passed.php.inc @@ -0,0 +1,24 @@ +mockProperty = $this->createMock(\stdClass::class); + $this->mockProperty->expects($this->once()) + ->method('someMethod') + ->willReturn('someValue'); + } + + public function test() + { + $someMock = new \stdClass($this->mockProperty); + } +} diff --git a/rules-tests/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector/RemoveNeverUsedMockPropertyRectorTest.php b/rules-tests/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector/RemoveNeverUsedMockPropertyRectorTest.php new file mode 100644 index 00000000..d88a8d90 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector/RemoveNeverUsedMockPropertyRectorTest.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_/RemoveNeverUsedMockPropertyRector/config/configured_rule.php b/rules-tests/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector/config/configured_rule.php new file mode 100644 index 00000000..9e3f1712 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector/config/configured_rule.php @@ -0,0 +1,10 @@ +rule(RemoveNeverUsedMockPropertyRector::class); +}; diff --git a/rules/CodeQuality/NodeAnalyser/MockObjectExprDetector.php b/rules/CodeQuality/NodeAnalyser/MockObjectExprDetector.php index 5bd81179..641c1ecd 100644 --- a/rules/CodeQuality/NodeAnalyser/MockObjectExprDetector.php +++ b/rules/CodeQuality/NodeAnalyser/MockObjectExprDetector.php @@ -5,6 +5,7 @@ namespace Rector\PHPUnit\CodeQuality\NodeAnalyser; use PhpParser\Node\Expr; +use PhpParser\Node\Expr\CallLike; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\Variable; @@ -101,4 +102,31 @@ public function isPropertyUsedForMocking(Class_ $class, string $propertyName): b return false; } + + public function isPropertyMockObjectPassedAsArgument(Class_ $class, string $propertyName): bool + { + /** @var array $callLikes */ + $callLikes = $this->betterNodeFinder->findInstancesOfScoped($class->getMethods(), [CallLike::class]); + + foreach ($callLikes as $callLike) { + if ($callLike->isFirstClassCallable()) { + continue; + } + + foreach ($callLike->getArgs() as $arg) { + if (! $arg->value instanceof PropertyFetch) { + continue; + } + + $propertyFetch = $arg->value; + + // its used in arg + if ($this->nodeNameResolver->isName($propertyFetch->name, $propertyName)) { + return true; + } + } + } + + return false; + } } diff --git a/rules/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector.php b/rules/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector.php new file mode 100644 index 00000000..b7de62d3 --- /dev/null +++ b/rules/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector.php @@ -0,0 +1,198 @@ +mockProperty = $this->createMock(SomeClass::class); + $this->mockProperty->expects($this->once()) + ->method('someMethod') + ->willReturn('someValue'); + } +} +CODE_SAMPLE + + , + <<<'CODE_SAMPLE' +use PHPUnit\Framework\TestCase; + +final class SomeTest extends TestCase +{ + protected function setUp(): void + { + } +} +CODE_SAMPLE + ), + ] + ); + } + + /** + * @return array> + */ + public function getNodeTypes(): array + { + return [Class_::class]; + } + + /** + * @param Class_ $node + */ + public function refactor(Node $node): ?Node + { + if (! $this->testsNodeAnalyzer->isInTestClass($node)) { + return null; + } + + $setUpClassMethod = $node->getMethod(MethodName::SET_UP); + if (! $setUpClassMethod instanceof ClassMethod) { + return null; + } + + $propertyNamesToCreateMockMethodCalls = $this->mockObjectPropertyDetector->collectFromClassMethod( + $setUpClassMethod + ); + if ($propertyNamesToCreateMockMethodCalls === []) { + return null; + } + + $propertyNamesToRemove = []; + foreach (array_keys($propertyNamesToCreateMockMethodCalls) as $propertyName) { + if ($this->mockObjectExprDetector->isPropertyMockObjectPassedAsArgument($node, $propertyName)) { + continue; + } + + $propertyNamesToRemove[] = $propertyName; + } + + if ($propertyNamesToRemove === []) { + return null; + } + + // remove never used mock properties + foreach ($propertyNamesToRemove as $propertyNameToRemove) { + // 1. remove property + $this->removePropertyFromClass($node, $propertyNameToRemove); + + // 2. remove assign from setUp() + $this->removeMockPropertyFromSetUpMethod($setUpClassMethod, $propertyNameToRemove); + + // 3. remove expression method calls on this property + foreach ($node->getMethods() as $classMethod) { + foreach ((array) $classMethod->stmts as $key => $classMethodStmt) { + if (! $classMethodStmt instanceof Expression) { + continue; + } + + if (! $classMethodStmt->expr instanceof MethodCall) { + continue; + } + + $methodCall = $classMethodStmt->expr; + $currentMethodCall = $methodCall; + while ($currentMethodCall->var instanceof MethodCall) { + $currentMethodCall = $currentMethodCall->var; + } + + if (! $currentMethodCall->var instanceof PropertyFetch) { + continue; + } + + $propertyFetch = $currentMethodCall->var; + if (! $this->isName($propertyFetch->name, $propertyNameToRemove)) { + continue; + } + + unset($classMethod->stmts[$key]); + } + } + } + + return $node; + } + + private function removeMockPropertyFromSetUpMethod(ClassMethod $setUpClassMethod, string $propertyName): void + { + foreach ((array) $setUpClassMethod->stmts as $key => $classStmt) { + if (! $classStmt instanceof Expression) { + continue; + } + + if (! $classStmt->expr instanceof Assign) { + continue; + } + + $assign = $classStmt->expr; + if (! $assign->var instanceof PropertyFetch) { + continue; + } + + $assignedPropertyFetch = $assign->var; + if (! $this->isName($assignedPropertyFetch->name, $propertyName)) { + continue; + } + + unset($setUpClassMethod->stmts[$key]); + return; + } + } + + private function removePropertyFromClass(Class_ $class, string $propertyNameToRemove): void + { + foreach ($class->stmts as $key => $stmt) { + if (! $stmt instanceof Property) { + continue; + } + + if (! $this->isName($stmt, $propertyNameToRemove)) { + continue; + } + + unset($class->stmts[$key]); + } + } +}