[DeadCode] Keep parent call delegation in case of accessibility override

packages/DeadCode/src/Rector/ClassMethod/RemoveDelegatingParentCallRector.php

@@ -6,15 +6,30 @@ use PhpParser\Node;
 use PhpParser\Node\Arg;
 use PhpParser\Node\Expr\StaticCall;
 use PhpParser\Node\Param;
+use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\Return_;
+use Rector\Exception\ShouldNotHappenException;
+use Rector\NodeContainer\ParsedNodesByType;
+use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\Rector\AbstractRector;
 use Rector\RectorDefinition\CodeSample;
 use Rector\RectorDefinition\RectorDefinition;
+use ReflectionMethod;
 
 final class RemoveDelegatingParentCallRector extends AbstractRector
 {
+    /**
+     * @var ParsedNodesByType
+     */
+    private $parsedNodesByType;
+
+    public function __construct(ParsedNodesByType $parsedNodesByType)
+    {
+        $this->parsedNodesByType = $parsedNodesByType;
+    }
+
     public function getDefinition(): RectorDefinition
     {
         return new RectorDefinition('', [
@@ -51,6 +66,15 @@ CODE_SAMPLE
      */
     public function refactor(Node $node): ?Node
     {
+        $classNode = $node->getAttribute(AttributeKey::CLASS_NODE);
+        if (! $classNode instanceof Class_) {
+            return null;
+        }
+
+        if ($classNode->extends === null) {
+            return null;
+        }
+
         if (count((array) $node->stmts) !== 1) {
             return null;
         }
@@ -115,6 +139,10 @@ CODE_SAMPLE
             return false;
         }
 
+        if ($this->isParentClassMethodVisibilityOverride($classMethod, $staticCall)) {
+            return false;
+        }
+
         return true;
     }
 
@@ -141,4 +169,34 @@ CODE_SAMPLE
 
         return true;
     }
+
+    private function isParentClassMethodVisibilityOverride(ClassMethod $classMethod, StaticCall $staticCall): bool
+    {
+        /** @var string $className */
+        $className = $staticCall->getAttribute(AttributeKey::CLASS_NAME);
+
+        $parentClassName = get_parent_class($className);
+        if ($parentClassName === false) {
+            throw new ShouldNotHappenException(__METHOD__);
+        }
+
+        /** @var string $methodName */
+        $methodName = $this->getName($staticCall);
+        $parentClassMethod = $this->parsedNodesByType->findMethod($methodName, $parentClassName);
+        if ($parentClassMethod !== null) {
+            if ($parentClassMethod->isProtected() && $classMethod->isPublic()) {
+                return true;
+            }
+        }
+
+        // 3rd party code
+        if (method_exists($parentClassName, $methodName)) {
+            $parentMethodReflection = new ReflectionMethod($parentClassName, $methodName);
+            if ($parentMethodReflection->isProtected() && $classMethod->isPublic()) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 }

packages/DeadCode/tests/Rector/ClassMethod/RemoveDelegatingParentCallRector/Fixture/fixture.php.inc

@@ -2,17 +2,14 @@
 
 namespace Rector\DeadCode\Tests\Rector\ClassMethod\RemoveDelegatingParentCallRector\Fixture;
 
-class SomeClass
+use PhpParser\PrettyPrinter\Standard;
+
+class SomeClass extends Standard
 {
     public function prettyPrint(array $stmts): string
     {
         return parent::prettyPrint($stmts);
     }
-
-    public function process(array $stmts): void
-    {
-        parent::process($stmts);
-    }
 }
 
 ?>
@@ -21,7 +18,9 @@ class SomeClass
 
 namespace Rector\DeadCode\Tests\Rector\ClassMethod\RemoveDelegatingParentCallRector\Fixture;
 
-class SomeClass
+use PhpParser\PrettyPrinter\Standard;
+
+class SomeClass extends Standard
 {
 }
 

packages/DeadCode/tests/Rector/ClassMethod/RemoveDelegatingParentCallRector/Fixture/skip_access_override.php.inc

@@ -0,0 +1,19 @@
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\ClassMethod\RemoveDelegatingParentCallRector\Fixture;
+
+class SkipAccessOverride extends ParentClassWithProtected
+{
+    public function getName()
+    {
+        return parent::getName();
+    }
+}
+
+class ParentClassWithProtected
+{
+    protected function getName()
+    {
+        return 'hello';
+    }
+}

packages/DeadCode/tests/Rector/ClassMethod/RemoveDelegatingParentCallRector/Fixture/skip_in_trait.php.inc

@@ -2,22 +2,10 @@
 
 namespace Rector\DeadCode\Tests\Rector\ClassMethod\RemoveDelegatingParentCallRector\Fixture;
 
-trait InTrait
+trait SkipInTrait
 {
     public function prettyPrint(array $stmts): string
     {
         return parent::prettyPrint($stmts);
     }
 }
-
-?>
------
-<?php
-
-namespace Rector\DeadCode\Tests\Rector\ClassMethod\RemoveDelegatingParentCallRector\Fixture;
-
-trait InTrait
-{
-}
-
-?>

packages/DeadCode/tests/Rector/ClassMethod/RemoveDelegatingParentCallRector/RemoveDelegatingParentCallRectorTest.php

@@ -11,10 +11,12 @@ final class RemoveDelegatingParentCallRectorTest extends AbstractRectorTestCase
     {
         $this->doTestFiles([
             __DIR__ . '/Fixture/fixture.php.inc',
-            __DIR__ . '/Fixture/in_trait.php.inc',
             // skip
+            // see https://3v4l.org/Plbu5
+            __DIR__ . '/Fixture/skip_access_override.php.inc',
             __DIR__ . '/Fixture/skip_extra_arguments.php.inc',
             __DIR__ . '/Fixture/skip_extra_content.php.inc',
+            __DIR__ . '/Fixture/skip_in_trait.php.inc',
             __DIR__ . '/Fixture/skip_different_method_name.php.inc',
             __DIR__ . '/Fixture/skip_changed_arguments.php.inc',
         ]);