Repository: phpactor/phpactor (php)

Issue:
Rename static method broken since optimisation merged 10th Jun
![image](https://github.com/phpactor/phpactor/assets/530801/fcf59818-92b2-4a86-ab53-1f0f54eadd65)

- renaming from `buildConditionMap` to `buildCondit` the original reference is not replaced (possibly because we do not find statc method calls?)

Pull request:
Fix static private member renaming for #2672
Fix #2673 

Patch:
diff --git a/lib/Rename/Adapter/ReferenceFinder/VariableRenamer.php b/lib/Rename/Adapter/ReferenceFinder/VariableRenamer.php
index 83555966..df3945f1 100644
--- a/lib/Rename/Adapter/ReferenceFinder/VariableRenamer.php
+++ b/lib/Rename/Adapter/ReferenceFinder/VariableRenamer.php
@@ -4,6 +4,7 @@ namespace Phpactor\Rename\Adapter\ReferenceFinder;
 
 use Microsoft\PhpParser\Node;
 use Microsoft\PhpParser\Node\CatchClause;
+use Microsoft\PhpParser\Node\Expression\ScopedPropertyAccessExpression;
 use Microsoft\PhpParser\Node\Expression\Variable;
 use Microsoft\PhpParser\Node\Parameter;
 use Microsoft\PhpParser\Node\PropertyDeclaration;
@@ -14,6 +15,11 @@ class VariableRenamer extends AbstractReferenceRenamer
 {
     public function getRenameRangeForNode(Node $node): ?ByteOffsetRange
     {
+        // do not try and rename static property names
+        if ($node->parent instanceof ScopedPropertyAccessExpression) {
+            return null;
+        }
+
         if (
             $node instanceof Variable &&
             !$node->getFirstAncestor(PropertyDeclaration::class)
@@ -21,6 +27,7 @@ class VariableRenamer extends AbstractReferenceRenamer
             return $this->offsetRangeFromToken($node->name, true);
         }
 
+
         if ($node instanceof Parameter && $node->visibilityToken) {
             return null;
         }
diff --git a/lib/WorseReflection/Bridge/TolerantParser/Reflection/ReflectionNavigation.php b/lib/WorseReflection/Bridge/TolerantParser/Reflection/ReflectionNavigation.php
index 983a1f0b..428f2a73 100644
--- a/lib/WorseReflection/Bridge/TolerantParser/Reflection/ReflectionNavigation.php
+++ b/lib/WorseReflection/Bridge/TolerantParser/Reflection/ReflectionNavigation.php
@@ -18,17 +18,25 @@ class ReflectionNavigation
     }
 
     /**
-     * @return NavigatorElementCollection<ReflectionMethodCall>
+     * @return NavigatorElementCollection<AbstractReflectionMethodCall>
      */
     public function methodCalls(): NavigatorElementCollection
     {
         $calls = [];
         foreach ($this->node->getDescendantNodes() as $node) {
+            if ($node instanceof ScopedPropertyAccessExpression) {
+                if (!$node->parent instanceof CallExpression) {
+                    continue;
+                }
+                $calls[] = new ReflectionStaticMethodCall($this->locator, new Frame(), $node);
+                continue;
+            }
             if ($node instanceof MemberAccessExpression) {
                 if (!$node->parent instanceof CallExpression) {
                     continue;
                 }
                 $calls[] = new ReflectionMethodCall($this->locator, new Frame(), $node);
+                continue;
             }
         }
         return new NavigatorElementCollection($calls);
@@ -40,12 +48,16 @@ class ReflectionNavigation
     }
 
     /**
-     * @return NavigatorElementCollection<ReflectionPropertyAccess>
+     * @return NavigatorElementCollection<ReflectionPropertyAccess|ReflectionStaticMemberAccess>
      */
     public function propertyAccesses(): NavigatorElementCollection
     {
         $elements = [];
         foreach ($this->node->getDescendantNodes() as $node) {
+            if ($node instanceof ScopedPropertyAccessExpression) {
+                $elements[] = new ReflectionStaticMemberAccess($this->locator, new Frame(), $node);
+                continue;
+            }
             if (!$node instanceof MemberAccessExpression) {
                 continue;
             }
@@ -55,6 +67,7 @@ class ReflectionNavigation
 
             $elements[] = new ReflectionPropertyAccess($node);
         }
+
         return new NavigatorElementCollection($elements);
     }
 
diff --git a/lib/WorseReflection/Bridge/TolerantParser/Reflection/ReflectionStaticMemberAccess.php b/lib/WorseReflection/Bridge/TolerantParser/Reflection/ReflectionStaticMemberAccess.php
index 6b3de229..26c676a0 100644
--- a/lib/WorseReflection/Bridge/TolerantParser/Reflection/ReflectionStaticMemberAccess.php
+++ b/lib/WorseReflection/Bridge/TolerantParser/Reflection/ReflectionStaticMemberAccess.php
@@ -60,7 +60,7 @@ class ReflectionStaticMemberAccess implements ReflectionNode
 
     public function name(): string
     {
-        return NodeUtil::nameFromTokenOrNode($this->node, $this->node->memberName);
+        return ltrim(NodeUtil::nameFromTokenOrNode($this->node, $this->node->memberName), '$');
     }
 
     public function scope(): ReflectionScope
@@ -72,7 +72,7 @@ class ReflectionStaticMemberAccess implements ReflectionNode
     {
         $memberName = $this->node->memberName;
         return ByteOffsetRange::fromInts(
-            $memberName->getStartPosition(),
+            $memberName->getStartPosition() + 1,
             $memberName->getEndPosition()
         );
     }
diff --git a/lib/WorseReflection/Core/NavigatorElementCollection.php b/lib/WorseReflection/Core/NavigatorElementCollection.php
index 137179e0..d3c9b665 100644
--- a/lib/WorseReflection/Core/NavigatorElementCollection.php
+++ b/lib/WorseReflection/Core/NavigatorElementCollection.php
@@ -9,7 +9,7 @@ use RuntimeException;
 use Traversable;
 
 /**
- * @template T of object
+ * @template-covariant T of object
  * @implements IteratorAggregate<array-key,T>
  */
 class NavigatorElementCollection implements IteratorAggregate


Test patch:
diff --git a/lib/Rename/Tests/Adapter/WorseReflection/WorseReflectionMemberRenamerTest.php b/lib/Rename/Tests/Adapter/WorseReflection/WorseReflectionMemberRenamerTest.php
index c7b58c2d..f3e31d66 100644
--- a/lib/Rename/Tests/Adapter/WorseReflection/WorseReflectionMemberRenamerTest.php
+++ b/lib/Rename/Tests/Adapter/WorseReflection/WorseReflectionMemberRenamerTest.php
@@ -47,6 +47,23 @@ class WorseReflectionMemberRenamerTest extends RenamerTestCase
                 self::assertTrue($reflection->methods()->has('newName'));
             }
         ];
+        yield 'private static method declaration' => [
+            'member_renamer/method_declaration_static_private',
+            function (Reflector $reflector, Renamer $renamer): Generator {
+                $reflection = $reflector->reflectClass('ClassOne');
+                $method = $reflection->methods()->get('foobar');
+
+                return $renamer->rename(
+                    $reflection->sourceCode(),
+                    $method->nameRange()->start(),
+                    'newName'
+                );
+            },
+            function (Reflector $reflector): void {
+                $reflection = $reflector->reflectClass('ClassOne');
+                self::assertTrue($reflection->methods()->has('newName'));
+            }
+        ];
         yield 'private property declaration' => [
             'member_renamer/property_declaration_private',
             function (Reflector $reflector, Renamer $renamer): Generator {
@@ -64,6 +81,23 @@ class WorseReflectionMemberRenamerTest extends RenamerTestCase
                 self::assertTrue($reflection->properties()->has('newName'));
             }
         ];
+        yield 'private static property declaration' => [
+            'member_renamer/property_declaration_static_private',
+            function (Reflector $reflector, Renamer $renamer): Generator {
+                $reflection = $reflector->reflectClass('ClassOne');
+                $method = $reflection->properties()->get('foobar');
+
+                return $renamer->rename(
+                    $reflection->sourceCode(),
+                    $method->nameRange()->start(),
+                    'newName'
+                );
+            },
+            function (Reflector $reflector): void {
+                $reflection = $reflector->reflectClass('ClassOne');
+                self::assertTrue($reflection->properties()->has('newName'));
+            }
+        ];
         yield 'private promoted property declaration' => [
             'member_renamer/property_promoted_private',
             function (Reflector $reflector, Renamer $renamer): Generator {
diff --git a/lib/Rename/Tests/Cases/member_renamer/method_declaration_static_private/ClassOne.ph b/lib/Rename/Tests/Cases/member_renamer/method_declaration_static_private/ClassOne.ph
new file mode 100644
index 00000000..f4d151c4
--- /dev/null
+++ b/lib/Rename/Tests/Cases/member_renamer/method_declaration_static_private/ClassOne.ph
@@ -0,0 +1,14 @@
+<?php
+
+class ClassOne
+{
+
+    public function hello(): string
+    {
+        return self::foobar();
+    }
+    private static function foobar(): string
+    {
+        return 'foobar';
+    }
+}
diff --git a/lib/Rename/Tests/Cases/member_renamer/method_declaration_static_private/test.ph b/lib/Rename/Tests/Cases/member_renamer/method_declaration_static_private/test.ph
new file mode 100644
index 00000000..8b69860f
--- /dev/null
+++ b/lib/Rename/Tests/Cases/member_renamer/method_declaration_static_private/test.ph
@@ -0,0 +1,10 @@
+<?php
+
+require __DIR__ . '/ClassOne.php';
+
+$one = new ClassOne();
+if (!$one->hello() === 'foobar') {
+    echo 'expected "foobar" but didn\'t get it';
+    exit(127);
+}
+
diff --git a/lib/Rename/Tests/Cases/member_renamer/property_declaration_static_private/ClassOne.ph b/lib/Rename/Tests/Cases/member_renamer/property_declaration_static_private/ClassOne.ph
new file mode 100644
index 00000000..c0f0a3d9
--- /dev/null
+++ b/lib/Rename/Tests/Cases/member_renamer/property_declaration_static_private/ClassOne.ph
@@ -0,0 +1,16 @@
+<?php
+
+class ClassOne
+{
+    private static string $foobar;
+
+    public function __construct(string $foobar)
+    {
+        self::$foobar = $foobar;
+    }
+
+    public function foobar(): string
+    {
+        return self::$foobar;
+    }
+}
diff --git a/lib/Rename/Tests/Cases/member_renamer/property_declaration_static_private/test.ph b/lib/Rename/Tests/Cases/member_renamer/property_declaration_static_private/test.ph
new file mode 100644
index 00000000..f0926427
--- /dev/null
+++ b/lib/Rename/Tests/Cases/member_renamer/property_declaration_static_private/test.ph
@@ -0,0 +1,9 @@
+<?php
+
+require __DIR__ . '/ClassOne.php';
+
+$two = new ClassOne('foobar');
+if (!$two->foobar() === 'foobar') {
+    echo 'expected "foobar" but didn\'t get it';
+    exit(127);
+}
diff --git a/lib/Rename/Tests/RenamerTestCase.php b/lib/Rename/Tests/RenamerTestCase.php
index b6af2e16..76ab5964 100644
--- a/lib/Rename/Tests/RenamerTestCase.php
+++ b/lib/Rename/Tests/RenamerTestCase.php
@@ -3,6 +3,7 @@
 namespace Phpactor\Rename\Tests;
 
 use Closure;
+use RuntimeException;
 use Generator;
 use PHPUnit\Framework\TestCase;
 use Phpactor\Indexer\Adapter\Worse\WorseRecordReferenceEnhancer;
@@ -50,6 +51,16 @@ abstract class RenamerTestCase extends TestCase
     public function testRename(string $path, Closure $operation, Closure $assertion): void
     {
         $basePath = __DIR__ . '/Cases/' . $path;
+
+
+        if (!file_exists($basePath)) {
+            throw new RuntimeException(sprintf(
+                'Case path "%s" does not yet exist',
+                $basePath
+            ));
+        }
+
+
         foreach ((array)glob($basePath . '/**.ph') as $path) {
             $this->workspace()->put(
                 'project/' . ((string)substr((string)$path, strlen($basePath))) . 'p',