diff --git a/rules-tests/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector/Fixture/skip_if_assigned.php.inc b/rules-tests/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector/Fixture/skip_if_assigned.php.inc new file mode 100644 index 00000000..f42cec57 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector/Fixture/skip_if_assigned.php.inc @@ -0,0 +1,22 @@ +mockProperty = $this->createMock(\stdClass::class); + $this->mockProperty->expects($this->once()) + ->method('someMethod') + ->willReturn('someValue'); + + $anotherObject = new \stdClass(); + $anotherObject->nested = $this->mockProperty; + } +} diff --git a/rules/CodeQuality/NodeAnalyser/MockObjectExprDetector.php b/rules/CodeQuality/NodeAnalyser/MockObjectExprDetector.php index ea078d66..5bd81179 100644 --- a/rules/CodeQuality/NodeAnalyser/MockObjectExprDetector.php +++ b/rules/CodeQuality/NodeAnalyser/MockObjectExprDetector.php @@ -5,8 +5,6 @@ namespace Rector\PHPUnit\CodeQuality\NodeAnalyser; use PhpParser\Node\Expr; -use PhpParser\Node\Expr\Array_; -use PhpParser\Node\Expr\CallLike; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\Variable; @@ -103,46 +101,4 @@ 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; - } - } - } - - /** @var array $arrays */ - $arrays = $this->betterNodeFinder->findInstancesOfScoped($class->getMethods(), [Array_::class]); - foreach ($arrays as $array) { - foreach ($array->items as $arrayItem) { - if (! $arrayItem->value instanceof PropertyFetch) { - continue; - } - - $propertyFetch = $arrayItem->value; - 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 index 9eb38bfc..a54b69dc 100644 --- a/rules/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector.php +++ b/rules/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector.php @@ -12,7 +12,8 @@ use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\Property; -use Rector\PHPUnit\CodeQuality\NodeAnalyser\MockObjectExprDetector; +use Rector\PhpParser\Node\BetterNodeFinder; +use Rector\PhpParser\NodeFinder\PropertyFetchFinder; use Rector\PHPUnit\CodeQuality\NodeAnalyser\MockObjectPropertyDetector; use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer; use Rector\Rector\AbstractRector; @@ -28,7 +29,8 @@ final class RemoveNeverUsedMockPropertyRector extends AbstractRector public function __construct( private readonly TestsNodeAnalyzer $testsNodeAnalyzer, private readonly MockObjectPropertyDetector $mockObjectPropertyDetector, - private readonly MockObjectExprDetector $mockObjectExprDetector, + private readonly PropertyFetchFinder $propertyFetchFinder, + private readonly BetterNodeFinder $betterNodeFinder, ) { } @@ -102,15 +104,7 @@ public function refactor(Node $node): ?Node return null; } - $propertyNamesToRemove = []; - foreach (array_keys($propertyNamesToCreateMockMethodCalls) as $propertyName) { - if ($this->mockObjectExprDetector->isPropertyMockObjectPassedAsArgument($node, $propertyName)) { - continue; - } - - $propertyNamesToRemove[] = $propertyName; - } - + $propertyNamesToRemove = $this->resolvePropertyNamesToRemove($propertyNamesToCreateMockMethodCalls, $node); if ($propertyNamesToRemove === []) { return null; } @@ -197,4 +191,47 @@ private function removePropertyFromClass(Class_ $class, string $propertyNameToRe unset($class->stmts[$key]); } } + + /** + * @param array $propertyNamesToCreateMockMethodCalls + * @return string[] + */ + private function resolvePropertyNamesToRemove(array $propertyNamesToCreateMockMethodCalls, Class_ $class): array + { + $propertyNamesToRemove = []; + foreach (array_keys($propertyNamesToCreateMockMethodCalls) as $propertyName) { + $allPropertyFetches = $this->propertyFetchFinder->findLocalPropertyFetchesByName($class, $propertyName); + + /** @var MethodCall[] $methodCalls */ + $methodCalls = $this->betterNodeFinder->findInstancesOfScoped($class->getMethods(), MethodCall::class); + + $propertyFetchesMethodCalls = []; + foreach ($methodCalls as $methodCall) { + if ($methodCall->isFirstClassCallable()) { + continue; + } + + if (! $methodCall->var instanceof PropertyFetch) { + continue; + } + + $propertyFetch = $methodCall->var; + if (! $this->isName($propertyFetch->name, $propertyName)) { + continue; + } + + // used in method call, skip removal + $propertyFetchesMethodCalls[] = $methodCall; + } + + // -1 for the assign in setUp() method + if ((count($allPropertyFetches) - 1) !== count($propertyFetchesMethodCalls)) { + continue; + } + + $propertyNamesToRemove[] = $propertyName; + } + + return $propertyNamesToRemove; + } }