improve check for used mock property (#646)

Author: TomasVotruba

rules-tests/CodeQuality/Rector/Class_/RemoveNeverUsedMockPropertyRector/Fixture/skip_if_assigned.php.inc

@@ -0,0 +1,22 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\Class_\RemoveNeverUsedMockPropertyRector\Fixture;
+
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
+
+final class SkipIfAssigned extends TestCase
+{
+    private MockObject $mockProperty;
+
+    protected function setUp(): void
+    {
+        $this->mockProperty = $this->createMock(\stdClass::class);
+        $this->mockProperty->expects($this->once())
+            ->method('someMethod')
+            ->willReturn('someValue');
+
+        $anotherObject = new \stdClass();
+        $anotherObject->nested = $this->mockProperty;
+    }
+}

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<Expr\CallLike> $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<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;
-    }
 }

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<string, MethodCall> $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;
+    }
 }