[DeadCode] Add support remove unused in between private method parame…

…ter for RemoveUnusedPrivateMethodParameterRector (#1321) * [DeadCode] Add support remove in between private method parameter for remove param+arg in caller in RemoveUnusedPrivateMethodParameterRector * implemented * phpstan * update test * [ci-review] Rector Rectify * [ci-review] Rector Rectify * final touch: use in_array when possible * final touch: continue to next method if no callers in a method Co-authored-by: GitHub Action <action@github.com>
rectorphp · Nov 27, 2021 · 40ec551 · 40ec551
1 parent 9f7b0e3
commit 40ec551
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 19 deletions.
diff --git a/...ClassMethod/RemoveUnusedPrivateMethodParameterRector/Fixture/in_between_parameter.php.inc b/...ClassMethod/RemoveUnusedPrivateMethodParameterRector/Fixture/in_between_parameter.php.inc
@@ -2,7 +2,7 @@
 
 namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodParameterRector\Fixture;
 
-final class SkipInBetweenParameter
+final class InBetweenParameter
 {
     private $value;
     private $value3;
@@ -12,4 +12,34 @@ final class SkipInBetweenParameter
         $this->value = $value;
         $this->value3 = $value3;
     }
+
+    public function execute()
+    {
+        $this->run('a', 'b', 'c');
+    }
 }
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodParameterRector\Fixture;
+
+final class InBetweenParameter
+{
+    private $value;
+    private $value3;
+
+    private function run($value, $value3)
+    {
+        $this->value = $value;
+        $this->value3 = $value3;
+    }
+
+    public function execute()
+    {
+        $this->run('a', 'c');
+    }
+}
+
+?>
diff --git a/rules/Autodiscovery/Rector/Class_/MoveServicesBySuffixToDirectoryRector.php b/rules/Autodiscovery/Rector/Class_/MoveServicesBySuffixToDirectoryRector.php
@@ -85,7 +85,7 @@ public function getNodeTypes(): array
      */
     public function refactor(Node $node): ?Node
     {
-        $this->processGroupNamesBySuffix($this->file->getSmartFileInfo(), $this->file, $this->groupNamesBySuffix);
+        $this->processGroupNamesBySuffix($this->file->getSmartFileInfo(), $this->groupNamesBySuffix);
 
         return null;
     }
@@ -110,11 +110,8 @@ public function configure(array $configuration): void
      *
      * @param string[] $groupNamesBySuffix
      */
-    private function processGroupNamesBySuffix(
-        SmartFileInfo $smartFileInfo,
-        File $file,
-        array $groupNamesBySuffix
-    ): void {
+    private function processGroupNamesBySuffix(SmartFileInfo $smartFileInfo, array $groupNamesBySuffix): void
+    {
         foreach ($groupNamesBySuffix as $groupNames) {
             // has class suffix
             $suffixPattern = '\w+' . $groupNames . '(Test)?\.php$';

diff --git a/rules/DeadCode/NodeCollector/UnusedParameterResolver.php b/rules/DeadCode/NodeCollector/UnusedParameterResolver.php
@@ -7,14 +7,11 @@
 use PhpParser\Node\Param;
 use PhpParser\Node\Stmt\ClassMethod;
 use Rector\Core\NodeManipulator\ClassMethodManipulator;
-use Rector\Core\ValueObject\MethodName;
-use Rector\NodeNameResolver\NodeNameResolver;
 
 final class UnusedParameterResolver
 {
     public function __construct(
-        private ClassMethodManipulator $classMethodManipulator,
-        private NodeNameResolver $nodeNameResolver
+        private ClassMethodManipulator $classMethodManipulator
     ) {
     }
 
@@ -33,11 +30,6 @@ public function resolve(ClassMethod $classMethod): array
             }
 
             if ($this->classMethodManipulator->isParameterUsedInClassMethod($param, $classMethod)) {
-                // reset to keep order of removed arguments, if not construtctor - probably autowired
-                if (! $this->nodeNameResolver->isName($classMethod, MethodName::CONSTRUCT)) {
-                    $unusedParameters = [];
-                }
-
                 continue;
             }
 

diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodParameterRector.php b/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodParameterRector.php
@@ -5,7 +5,10 @@
 namespace Rector\DeadCode\Rector\ClassMethod;
 
 use PhpParser\Node;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Param;
+use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\ClassMethod;
 use PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode;
 use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTagRemover;
@@ -81,10 +84,79 @@ public function refactor(Node $node): ?Node
 
         $this->removeNodes($unusedParameters);
         $this->clearPhpDocInfo($node, $unusedParameters);
+        $this->removeCallerArgs($node, $unusedParameters);
 
         return $node;
     }
 
+    /**
+     * @param Param[] $unusedParameters
+     */
+    private function removeCallerArgs(ClassMethod $classMethod, array $unusedParameters): void
+    {
+        $classLike = $this->betterNodeFinder->findParentType($classMethod, ClassLike::class);
+        if (! $classLike instanceof ClassLike) {
+            return;
+        }
+
+        $methods = $classLike->getMethods();
+        if ($methods === []) {
+            return;
+        }
+
+        $methodName = $this->nodeNameResolver->getName($classMethod);
+        $keysArg = array_keys($unusedParameters);
+
+        foreach ($methods as $method) {
+            /** @var MethodCall[] $callers */
+            $callers = $this->resolveCallers($method, $methodName);
+            if ($callers === []) {
+                continue;
+            }
+
+            foreach ($callers as $caller) {
+                $this->cleanupArgs($caller, $keysArg);
+            }
+        }
+    }
+
+    /**
+     * @param int[] $keysArg
+     */
+    private function cleanupArgs(MethodCall $methodCall, array $keysArg): void
+    {
+        $args = $methodCall->getArgs();
+        foreach (array_keys($args) as $key) {
+            if (in_array($key, $keysArg, true)) {
+                unset($args[$key]);
+            }
+        }
+
+        $methodCall->args = $args;
+    }
+
+    /**
+     * @return MethodCall[]
+     */
+    private function resolveCallers(ClassMethod $classMethod, string $methodName): array
+    {
+        return $this->betterNodeFinder->find($classMethod, function (Node $subNode) use ($methodName): bool {
+            if (! $subNode instanceof MethodCall) {
+                return false;
+            }
+
+            if (! $subNode->var instanceof Variable) {
+                return false;
+            }
+
+            if (! $this->nodeNameResolver->isName($subNode->var, 'this')) {
+                return false;
+            }
+
+            return $this->nodeNameResolver->isName($subNode->name, $methodName);
+        });
+    }
+
     private function shouldSkip(ClassMethod $classMethod): bool
     {
         if (! $classMethod->isPrivate()) {

diff --git a/rules/Php71/Rector/TryCatch/MultiExceptionCatchRector.php b/rules/Php71/Rector/TryCatch/MultiExceptionCatchRector.php
@@ -75,7 +75,7 @@ public function refactor(Node $node): ?Node
                 continue;
             }
 
-            $collectedTypes = $this->collectTypesFromCatchedByIds($node, $catchKeys);
+            $collectedTypes = $this->collectTypesFromCatchedByIds($catchKeys);
 
             /** @var Catch_ $firstCatch */
             $firstCatch = array_shift($catchKeys);
@@ -112,7 +112,7 @@ private function collectCatchKeysByContent(TryCatch $tryCatch): array
      * @param Catch_[] $catches
      * @return Name[]
      */
-    private function collectTypesFromCatchedByIds(TryCatch $tryCatch, array $catches): array
+    private function collectTypesFromCatchedByIds(array $catches): array
     {
         $collectedTypes = [];
 

diff --git a/...ullableParamNamespaced/Fixture/do_not_add_short_fqcn_class_name_docblock_nullable.php.inc b/...ullableParamNamespaced/Fixture/do_not_add_short_fqcn_class_name_docblock_nullable.php.inc
@@ -37,7 +37,7 @@ class DoNotAddShortFQCNClassNameDocblockNullable
             return false;
         }
 
-        return $this->callPrivateMethod($classLike, true);
+        return $this->callPrivateMethod($classLike);
     }
 
     private function callPrivateMethod(ClassLike $classLike)