Allow to rename method if in interface (#2362)

* add test fixture * allow rename method in interface
rectorphp · May 25, 2022 · 66dbb1e · 66dbb1e
1 parent 5d0138d
commit 66dbb1e
Show file tree

Hide file tree

Showing 7 changed files with 107 additions and 44 deletions.
diff --git a/packages/NodeTypeResolver/NodeTypeResolver.php b/packages/NodeTypeResolver/NodeTypeResolver.php
@@ -17,10 +17,10 @@
 use PhpParser\Node\Name;
 use PhpParser\Node\Name\FullyQualified;
 use PhpParser\Node\NullableType;
-use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\Property;
 use PHPStan\Analyser\Scope;
 use PHPStan\Broker\ClassAutoloadingException;
+use PHPStan\Reflection\ClassReflection;
 use PHPStan\Reflection\ReflectionProvider;
 use PHPStan\Type\Constant\ConstantBooleanType;
 use PHPStan\Type\FloatType;
@@ -36,7 +36,6 @@
 use PHPStan\Type\UnionType;
 use Rector\Core\Configuration\RenamedClassesDataCollector;
 use Rector\Core\NodeAnalyzer\ClassAnalyzer;
-use Rector\Core\PhpParser\Node\BetterNodeFinder;
 use Rector\NodeTypeResolver\Contract\NodeTypeResolverInterface;
 use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\NodeTypeResolver\NodeTypeCorrector\AccessoryNonEmptyStringTypeCorrector;
@@ -66,11 +65,12 @@ public function __construct(
         private readonly AccessoryNonEmptyStringTypeCorrector $accessoryNonEmptyStringTypeCorrector,
         private readonly IdentifierTypeResolver $identifierTypeResolver,
         private readonly RenamedClassesDataCollector $renamedClassesDataCollector,
-        private readonly BetterNodeFinder $betterNodeFinder,
         array $nodeTypeResolvers
     ) {
         foreach ($nodeTypeResolvers as $nodeTypeResolver) {
-            $this->addNodeTypeResolver($nodeTypeResolver);
+            foreach ($nodeTypeResolver->getNodeClasses() as $nodeClass) {
+                $this->nodeTypeResolvers[$nodeClass] = $nodeTypeResolver;
+            }
         }
     }
 
@@ -285,26 +285,29 @@ public function isMethodStaticCallOrClassMethodObjectType(Node $node, ObjectType
             return $this->isObjectType($node->class, $objectType);
         }
 
-        $class = $this->betterNodeFinder->findParentType($node, Class_::class);
-        if (! $class instanceof Class_) {
+        $scope = $node->getAttribute(AttributeKey::SCOPE);
+        if (! $scope instanceof Scope) {
+            return false;
+        }
+
+        $classReflection = $scope->getClassReflection();
+
+        if (! $classReflection instanceof ClassReflection) {
             return false;
         }
 
-        return $this->isObjectType($class, $objectType);
+        if ($classReflection->getName() === $objectType->getClassName()) {
+            return true;
+        }
+
+        return $classReflection->isSubclassOf($objectType->getClassName());
     }
 
     private function isUnionTypeable(Type $first, Type $second): bool
     {
         return ! $first instanceof UnionType && ! $second instanceof UnionType && ! $second instanceof NullType;
     }
 
-    private function addNodeTypeResolver(NodeTypeResolverInterface $nodeTypeResolver): void
-    {
-        foreach ($nodeTypeResolver->getNodeClasses() as $nodeClass) {
-            $this->nodeTypeResolvers[$nodeClass] = $nodeTypeResolver;
-        }
-    }
-
     private function isMatchingUnionType(Type $resolvedType, ObjectType $requiredObjectType): bool
     {
         $type = TypeCombinator::removeNull($resolvedType);

diff --git a/.../Rector/MethodCall/RenameMethodRector/Fixture/when_interface_and_parent_interface.php.inc b/.../Rector/MethodCall/RenameMethodRector/Fixture/when_interface_and_parent_interface.php.inc
@@ -0,0 +1,25 @@
+<?php
+
+namespace Rector\Tests\Renaming\Rector\MethodCall\RenameMethodRector\Fixture;
+
+use Rector\Tests\Renaming\Rector\MethodCall\RenameMethodRector\Source\DifferentInterface;
+
+interface WhenInterfaceAndParentInterface extends DifferentInterface
+{
+    public function renameMe(): int;
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\Renaming\Rector\MethodCall\RenameMethodRector\Fixture;
+
+use Rector\Tests\Renaming\Rector\MethodCall\RenameMethodRector\Source\DifferentInterface;
+
+interface WhenInterfaceAndParentInterface extends DifferentInterface
+{
+    public function toNewVersion(): int;
+}
+
+?>
diff --git a/...ector/Fixture/skip_when_interface.php.inc → ...tor/Fixture/when_parent_interface.php.inc b/...ector/Fixture/skip_when_interface.php.inc → ...tor/Fixture/when_parent_interface.php.inc
@@ -1,8 +1,10 @@
 <?php
 
-namespace Rector\Tests\Renaming\Rector\MethodCall\RenameMethodRector\Source;
+namespace Rector\Tests\Renaming\Rector\MethodCall\RenameMethodRector\Fixture;
 
-class SkipRenameMethodCall
+use Rector\Tests\Renaming\Rector\MethodCall\RenameMethodRector\Source\SubscriberInterface;
+
+final class SkipWhenParentInterface
 {
     public static function execute(): void
     {
@@ -11,12 +13,10 @@ class SkipRenameMethodCall
     }
 }
 
-class SomeSubscriber implements SubscriberInterface
+final class SomeSubscriber implements SubscriberInterface
 {
     public function old(): int
     {
         return 5;
     }
 }
-
-?>
diff --git a/rules-tests/Renaming/Rector/MethodCall/RenameMethodRector/Source/DifferentInterface.php b/rules-tests/Renaming/Rector/MethodCall/RenameMethodRector/Source/DifferentInterface.php
@@ -0,0 +1,8 @@
+<?php
+
+namespace Rector\Tests\Renaming\Rector\MethodCall\RenameMethodRector\Source;
+
+interface DifferentInterface
+{
+    public function renameMe();
+}
diff --git a/rules-tests/Renaming/Rector/MethodCall/RenameMethodRector/config/configured_rule.php b/rules-tests/Renaming/Rector/MethodCall/RenameMethodRector/config/configured_rule.php
@@ -8,20 +8,21 @@
 use Rector\Renaming\ValueObject\MethodCallRenameWithArrayKey;
 use Rector\Tests\Renaming\Rector\MethodCall\RenameMethodRector\Source\AbstractType;
 use Rector\Tests\Renaming\Rector\MethodCall\RenameMethodRector\Source\CustomType;
+use Rector\Tests\Renaming\Rector\MethodCall\RenameMethodRector\Source\DifferentInterface;
 use Rector\Tests\Renaming\Rector\MethodCall\RenameMethodRector\Source\Foo;
 use Rector\Tests\Renaming\Rector\MethodCall\RenameMethodRector\Source\NewInterface;
 use Rector\Tests\Renaming\Rector\MethodCall\RenameMethodRector\Source\SomeSubscriber;
 
 return static function (RectorConfig $rectorConfig): void {
-    $rectorConfig
-        ->ruleWithConfiguration(RenameMethodRector::class, [
-            new MethodCallRename(AbstractType::class, 'setDefaultOptions', 'configureOptions'),
-            new MethodCallRename('Nette\Utils\Html', 'add', 'addHtml'),
-            new MethodCallRename(CustomType::class, 'notify', '__invoke'),
-            new MethodCallRename(SomeSubscriber::class, 'old', 'new'),
-            new MethodCallRename(Foo::class, 'old', 'new'),
-            new MethodCallRename(NewInterface::class, 'some_old', 'some_new'),
-            // with array key
-            new MethodCallRenameWithArrayKey('Nette\Utils\Html', 'addToArray', 'addToHtmlArray', 'hey'),
-        ]);
+    $rectorConfig->ruleWithConfiguration(RenameMethodRector::class, [
+        new MethodCallRename(AbstractType::class, 'setDefaultOptions', 'configureOptions'),
+        new MethodCallRename('Nette\Utils\Html', 'add', 'addHtml'),
+        new MethodCallRename(CustomType::class, 'notify', '__invoke'),
+        new MethodCallRename(SomeSubscriber::class, 'old', 'new'),
+        new MethodCallRename(Foo::class, 'old', 'new'),
+        new MethodCallRename(NewInterface::class, 'some_old', 'some_new'),
+        new MethodCallRename(DifferentInterface::class, 'renameMe', 'toNewVersion'),
+        // with array key
+        new MethodCallRenameWithArrayKey('Nette\Utils\Html', 'addToArray', 'addToHtmlArray', 'hey'),
+    ]);
 };
diff --git a/rules/CodingStyle/Rector/MethodCall/PreferThisOrSelfMethodCallRector.php b/rules/CodingStyle/Rector/MethodCall/PreferThisOrSelfMethodCallRector.php
@@ -87,7 +87,10 @@ public function getNodeTypes(): array
     public function refactor(Node $node): ?Node
     {
         foreach ($this->typeToPreference as $type => $preference) {
-            if (! $this->nodeTypeResolver->isMethodStaticCallOrClassMethodObjectType($node, new ObjectType($type))) {
+            if (! $this->nodeTypeResolver->isMethodStaticCallOrClassMethodObjectType(
+                $node,
+                new ObjectType($type)
+            )) {
                 continue;
             }
 

diff --git a/rules/Renaming/Rector/MethodCall/RenameMethodRector.php b/rules/Renaming/Rector/MethodCall/RenameMethodRector.php
@@ -12,11 +12,12 @@
 use PhpParser\Node\Identifier;
 use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\ClassMethod;
+use PHPStan\Analyser\Scope;
 use PHPStan\Reflection\ClassReflection;
 use PHPStan\Reflection\ReflectionProvider;
 use Rector\Core\Contract\Rector\ConfigurableRectorInterface;
 use Rector\Core\NodeManipulator\ClassManipulator;
-use Rector\Core\Rector\AbstractRector;
+use Rector\Core\Rector\AbstractScopeAwareRector;
 use Rector\Core\Reflection\ReflectionResolver;
 use Rector\Renaming\Collector\MethodCallRenameCollector;
 use Rector\Renaming\Contract\MethodCallRenameInterface;
@@ -29,7 +30,7 @@
 /**
  * @see \Rector\Tests\Renaming\Rector\MethodCall\RenameMethodRector\RenameMethodRectorTest
  */
-final class RenameMethodRector extends AbstractRector implements ConfigurableRectorInterface
+final class RenameMethodRector extends AbstractScopeAwareRector implements ConfigurableRectorInterface
 {
     /**
      * @var MethodCallRenameInterface[]
@@ -74,15 +75,16 @@ public function getNodeTypes(): array
     /**
      * @param MethodCall|StaticCall|ClassMethod $node
      */
-    public function refactor(Node $node): ?Node
+    public function refactorWithScope(Node $node, Scope $scope): ?Node
     {
+        $classReflection = $scope->getClassReflection();
+
         foreach ($this->methodCallRenames as $methodCallRename) {
-            $implementsInterface = $this->classManipulator->hasParentMethodOrInterface(
-                $methodCallRename->getObjectType(),
-                $methodCallRename->getOldMethod(),
-                $methodCallRename->getNewMethod()
-            );
-            if ($implementsInterface) {
+            if (! $this->isName($node->name, $methodCallRename->getOldMethod())) {
+                continue;
+            }
+
+            if ($this->shouldKeepForParentInterface($methodCallRename, $node, $classReflection)) {
                 continue;
             }
 
@@ -93,10 +95,6 @@ public function refactor(Node $node): ?Node
                 continue;
             }
 
-            if (! $this->isName($node->name, $methodCallRename->getOldMethod())) {
-                continue;
-            }
-
             if ($this->shouldSkipClassMethod($node, $methodCallRename)) {
                 continue;
             }
@@ -167,4 +165,29 @@ private function shouldSkipForAlreadyExistingClassMethod(
 
         return (bool) $classLike->getMethod($methodCallRename->getNewMethod());
     }
+
+    private function shouldKeepForParentInterface(
+        MethodCallRenameInterface $methodCallRename,
+        ClassMethod|StaticCall|MethodCall $node,
+        ?ClassReflection $classReflection
+    ): bool {
+        if (! $node instanceof ClassMethod) {
+            return false;
+        }
+
+        if (! $classReflection instanceof ClassReflection) {
+            return false;
+        }
+
+        // interface can change current method, as parent contract is still valid
+        if (! $classReflection->isInterface()) {
+            return false;
+        }
+
+        return $this->classManipulator->hasParentMethodOrInterface(
+            $methodCallRename->getObjectType(),
+            $methodCallRename->getOldMethod(),
+            $methodCallRename->getNewMethod()
+        );
+    }
 }