improve GetAndSetToMethodCallRector

docs/AllRectorsOverview.md

@@ -2714,7 +2714,7 @@ Changes rand, srand and getrandmax by new md_* alternatives.
 
 - class: `Rector\Php\Rector\FuncCall\PregReplaceEModifierRector`
 
-The /e modifier is no longer supported, use preg_replace_callback instead 
+The /e modifier is no longer supported, use preg_replace_callback instead
 
 ```diff
  class SomeClass

docs/NodesOverview.md

@@ -910,7 +910,7 @@ if (true) {
 
 ```php
 ?>
-<strong>feel</strong><?php 
+<strong>feel</strong><?php
 ```
 <br>
 

src/PhpParser/Node/Manipulator/ClassManipulator.php

@@ -240,6 +240,25 @@ final class ClassManipulator
         });
     }
 
+    public function hasPropertyFetchAsProperty(Class_ $class, Node\Expr\PropertyFetch $propertyFetch): bool
+    {
+        if (! $this->nameResolver->isName($propertyFetch->var, 'this')) {
+            return false;
+        }
+
+        foreach ((array) $class->stmts as $classStmt) {
+            if (! $classStmt instanceof Property) {
+                continue;
+            }
+
+            if ($this->nameResolver->areNamesEqual($classStmt->props[0], $propertyFetch)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     private function tryInsertBeforeFirstMethod(Class_ $classNode, Stmt $stmt): bool
     {
         foreach ($classNode->stmts as $key => $classElementNode) {

src/PhpParser/Node/Manipulator/PropertyFetchManipulator.php

@@ -4,7 +4,6 @@ namespace Rector\PhpParser\Node\Manipulator;
 
 use PhpParser\Node;
 use PhpParser\Node\Expr\PropertyFetch;
-use PhpParser\Node\Expr\Variable;
 use PHPStan\Analyser\Scope;
 use PHPStan\Broker\Broker;
 use PHPStan\Type\ObjectType;
@@ -34,20 +33,36 @@ final class PropertyFetchManipulator
      */
     private $nameResolver;
 
-    public function __construct(NodeTypeResolver $nodeTypeResolver, Broker $broker, NameResolver $nameResolver)
-    {
+    /**
+     * @var ClassManipulator
+     */
+    private $classManipulator;
+
+    public function __construct(
+        NodeTypeResolver $nodeTypeResolver,
+        Broker $broker,
+        NameResolver $nameResolver,
+        ClassManipulator $classManipulator
+    ) {
         $this->nodeTypeResolver = $nodeTypeResolver;
         $this->broker = $broker;
         $this->nameResolver = $nameResolver;
+        $this->classManipulator = $classManipulator;
     }
 
     public function isPropertyToSelf(PropertyFetch $propertyFetch): bool
     {
-        if (! $propertyFetch->var instanceof Variable) {
+        if (! $this->nameResolver->isName($propertyFetch->var, 'this')) {
             return false;
         }
 
-        return $propertyFetch->var->name === 'this';
+        /** @var Node\Stmt\Class_|null $class */
+        $class = $propertyFetch->getAttribute(Attribute::CLASS_NODE);
+        if ($class === null) {
+            return false;
+        }
+
+        return $this->classManipulator->hasPropertyFetchAsProperty($class, $propertyFetch);
     }
 
     public function isMagicOnType(Node $node, string $type): bool

src/PhpParser/Node/Resolver/NameResolver.php

@@ -192,4 +192,9 @@ final class NameResolver
 
         return (string) $node->name;
     }
+
+    public function areNamesEqual(Node $firstNode, Node $secondNode): bool
+    {
+        return $this->resolve($firstNode) === $this->resolve($secondNode);
+    }
 }

src/Rector/MagicDisclosure/GetAndSetToMethodCallRector.php

@@ -7,7 +7,7 @@ use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Expr\PropertyFetch;
 use PhpParser\Node\Expr\Variable;
-use PhpParser\Node\Stmt\If_;
+use Rector\NodeTypeResolver\Node\Attribute;
 use Rector\PhpParser\Node\Manipulator\PropertyFetchManipulator;
 use Rector\Rector\AbstractRector;
 use Rector\RectorDefinition\ConfiguredCodeSample;
@@ -83,54 +83,23 @@ CODE_SAMPLE
      */
     public function getNodeTypes(): array
     {
-        return [Assign::class, If_::class];
+        return [Assign::class, PropertyFetch::class];
     }
 
     /**
-     * @param Assign|If_ $node
+     * @param Assign|PropertyFetch $node
      */
     public function refactor(Node $node): ?Node
     {
         if ($node instanceof Assign) {
-            if ($node->expr instanceof PropertyFetch) {
-                return $this->processMagicGet($node);
-            }
-
             if ($node->var instanceof PropertyFetch) {
                 return $this->processMagicSet($node);
             }
+
+            return null;
         }
 
-        if ($node instanceof If_ && $node->cond instanceof PropertyFetch) {
-            return $this->processMagicGetOnIf($node);
-        }
-
-        return null;
-    }
-
-    private function processMagicGet(Assign $assign): ?Node
-    {
-        /** @var PropertyFetch $propertyFetchNode */
-        $propertyFetchNode = $assign->expr;
-
-        foreach ($this->typeToMethodCalls as $type => $transformation) {
-            if (! $this->isType($propertyFetchNode, $type)) {
-                continue;
-            }
-
-            if (! $this->propertyFetchManipulator->isMagicOnType($propertyFetchNode, $type)) {
-                continue;
-            }
-
-            $assign->expr = $this->createMethodCallNodeFromPropertyFetchNode(
-                $propertyFetchNode,
-                $transformation['get']
-            );
-
-            return $assign;
-        }
-
-        return null;
+        return $this->processPropertyFetch($node);
     }
 
     private function processMagicSet(Assign $assign): ?Node
@@ -139,15 +108,7 @@ CODE_SAMPLE
         $propertyFetchNode = $assign->var;
 
         foreach ($this->typeToMethodCalls as $type => $transformation) {
-            if (! $this->isType($propertyFetchNode, $type)) {
-                continue;
-            }
-
-            if ($this->propertyFetchManipulator->isPropertyToSelf($propertyFetchNode)) {
-                continue;
-            }
-
-            if (! $this->propertyFetchManipulator->isMagicOnType($propertyFetchNode, $type)) {
+            if ($this->shouldSkipPropertyFetch($propertyFetchNode, $type)) {
                 continue;
             }
 
@@ -161,31 +122,6 @@ CODE_SAMPLE
         return null;
     }
 
-    private function processMagicGetOnIf(If_ $if): ?Node
-    {
-        /** @var PropertyFetch $propertyFetchNode */
-        $propertyFetchNode = $if->cond;
-
-        foreach ($this->typeToMethodCalls as $type => $transformation) {
-            if (! $this->isType($propertyFetchNode, $type)) {
-                continue;
-            }
-
-            if (! $this->propertyFetchManipulator->isMagicOnType($propertyFetchNode, $type)) {
-                continue;
-            }
-
-            $if->cond = $this->createMethodCallNodeFromPropertyFetchNode(
-                $propertyFetchNode,
-                $transformation['get']
-            );
-
-            return $if;
-        }
-
-        return null;
-    }
-
     private function createMethodCallNodeFromPropertyFetchNode(
         PropertyFetch $propertyFetch,
         string $method
@@ -206,4 +142,40 @@ CODE_SAMPLE
 
         return $this->createMethodCall($variableNode, $method, [$this->getName($propertyFetch), $node]);
     }
+
+    private function shouldSkipPropertyFetch(PropertyFetch $propertyFetch, string $type): bool
+    {
+        if (! $this->isType($propertyFetch, $type)) {
+            return true;
+        }
+
+        if (! $this->propertyFetchManipulator->isMagicOnType($propertyFetch, $type)) {
+            return true;
+        }
+
+        // $this->value = $value
+        return $this->propertyFetchManipulator->isPropertyToSelf($propertyFetch);
+    }
+
+    private function processPropertyFetch(PropertyFetch $propertyFetch): ?MethodCall
+    {
+        foreach ($this->typeToMethodCalls as $type => $transformation) {
+            if ($this->shouldSkipPropertyFetch($propertyFetch, $type)) {
+                continue;
+            }
+
+            // setter, skip
+            $parentNode = $propertyFetch->getAttribute(Attribute::PARENT_NODE);
+
+            if ($parentNode instanceof Assign) {
+                if ($parentNode->var === $propertyFetch) {
+                    continue;
+                }
+            }
+
+            return $this->createMethodCallNodeFromPropertyFetchNode($propertyFetch, $transformation['get']);
+        }
+
+        return null;
+    }
 }

src/Rector/NameResolverTrait.php

@@ -31,7 +31,7 @@ trait NameResolverTrait
 
     public function areNamesEqual(Node $firstNode, Node $secondNode): bool
     {
-        return $this->getName($firstNode) === $this->getName($secondNode);
+        return $this->nameResolver->areNamesEqual($firstNode, $secondNode);
     }
 
     public function isNameInsensitive(Node $node, string $name): bool

tests/Rector/MagicDisclosure/GetAndSetToMethodCallRector/Fixture/klarka.php.inc

@@ -6,11 +6,21 @@ use Rector\Tests\Rector\MagicDisclosure\GetAndSetToMethodCallRector\Source\Klark
 
 class KlarkaExtended extends Klarka
 {
+    private $existingProperty;
+
     public function run()
     {
         if ($this->leafBreadcrumbCategory) {
             $category = $this->leafBreadcrumbCategory;
         }
+
+        while ($this->leafLet !== 5) {
+        }
+
+        while ($this->existingProperty !== 5) {
+        }
+        while ($this->existingProperty) {
+        }
     }
 }
 
@@ -24,11 +34,21 @@ use Rector\Tests\Rector\MagicDisclosure\GetAndSetToMethodCallRector\Source\Klark
 
 class KlarkaExtended extends Klarka
 {
+    private $existingProperty;
+
     public function run()
     {
         if ($this->get('leafBreadcrumbCategory')) {
             $category = $this->get('leafBreadcrumbCategory');
         }
+
+        while ($this->get('leafLet') !== 5) {
+        }
+
+        while ($this->existingProperty !== 5) {
+        }
+        while ($this->existingProperty) {
+        }
     }
 }
 

tests/Rector/MagicDisclosure/GetAndSetToMethodCallRector/GetAndSetToMethodCallRectorTest.php

@@ -12,7 +12,7 @@ final class GetAndSetToMethodCallRectorTest extends AbstractRectorTestCase
     public function test(): void
     {
         $this->doTestFiles([
-            __DIR__ . '/Fixture/fixture.php.inc',
+            __DIR__ . '/Fixture/get.php.inc',
             __DIR__ . '/Fixture/fixture2.php.inc',
             __DIR__ . '/Fixture/fixture3.php.inc',
             __DIR__ . '/Fixture/klarka.php.inc',