Merge pull request #894 from rectorphp/dead-code-2

[DeadCode] Add RemoveUnusedParameterRector

config/level/dead-code/dead-code.yml

@@ -6,3 +6,4 @@ services:
     Rector\DeadCode\Rector\ClassMethod\RemoveEmptyClassMethodRector: ~
     Rector\DeadCode\Rector\Property\RemoveUnusedPrivatePropertyRector: ~
     Rector\DeadCode\Rector\Assign\RemoveDoubleAssignRector: ~
+    Rector\DeadCode\Rector\ClassMethod\RemoveUnusedParameterRector: ~

packages/ContributorTools/src/TemplateVariablesFactory.php

@@ -87,6 +87,6 @@ CODE_SAMPLE;
             $arrayNodes[] = new ArrayItem(new ClassConstFetch(new FullyQualified($nodeType), 'class'));
         }
 
-        return $this->betterStandardPrinter->prettyPrint([new Array_($arrayNodes)]);
+        return $this->betterStandardPrinter->print(new Array_($arrayNodes));
     }
 }

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

@@ -0,0 +1,104 @@
+<?php declare(strict_types=1);
+
+namespace Rector\DeadCode\Rector\ClassMethod;
+
+use PhpParser\Node;
+use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\ClassMethod;
+use Rector\NodeTypeResolver\Node\Attribute;
+use Rector\PhpParser\Node\Maintainer\ClassMaintainer;
+use Rector\PhpParser\Node\Maintainer\ClassMethodMaintainer;
+use Rector\Rector\AbstractRector;
+use Rector\RectorDefinition\CodeSample;
+use Rector\RectorDefinition\RectorDefinition;
+
+final class RemoveUnusedParameterRector extends AbstractRector
+{
+    /**
+     * @var ClassMaintainer
+     */
+    private $classMaintainer;
+
+    /**
+     * @var ClassMethodMaintainer
+     */
+    private $classMethodMaintainer;
+
+    public function __construct(ClassMaintainer $classMaintainer, ClassMethodMaintainer $classMethodMaintainer)
+    {
+        $this->classMaintainer = $classMaintainer;
+        $this->classMethodMaintainer = $classMethodMaintainer;
+    }
+
+    public function getDefinition(): RectorDefinition
+    {
+        return new RectorDefinition('Remove unused parameter, if not required by interface or parent class', [
+            new CodeSample(
+                <<<'CODE_SAMPLE'
+class SomeClass
+{
+    public function __construct($value, $value2)
+    {
+         $this->value = $value;
+    }
+}
+CODE_SAMPLE
+                ,
+                <<<'CODE_SAMPLE'
+class SomeClass
+{
+    public function __construct($value)
+    {
+         $this->value = $value;
+    }
+}
+CODE_SAMPLE
+            ),
+        ]);
+    }
+
+    /**
+     * @return string[]
+     */
+    public function getNodeTypes(): array
+    {
+        return [ClassMethod::class];
+    }
+
+    /**
+     * @param ClassMethod $node
+     */
+    public function refactor(Node $node): ?Node
+    {
+        if (! $node->getAttribute(Attribute::CLASS_NODE) instanceof Class_) {
+            return null;
+        }
+
+        if ($node->params === []) {
+            return null;
+        }
+
+        /** @var string $class */
+        $class = $node->getAttribute(Attribute::CLASS_NAME);
+        if ($this->classMaintainer->hasParentMethodOrInterface($class, $this->getName($node))) {
+            return null;
+        }
+
+        $unusedParameters = [];
+        foreach ($node->params as $i => $param) {
+            if ($this->classMethodMaintainer->isParameterUsedMethod($param, $node)) {
+                // reset to keep order of removed arguments
+                $unusedParameters = [];
+                continue;
+            }
+
+            $unusedParameters[$i] = $param;
+        }
+
+        foreach ($unusedParameters as $unusedParameter) {
+            $this->removeNode($unusedParameter);
+        }
+
+        return $node;
+    }
+}

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

@@ -0,0 +1,27 @@
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\ClassMethod\RemoveUnusedParameterRector\Fixture;
+
+class SomeClass
+{
+    public function __construct($value, $value2)
+    {
+        $this->value = $value;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\ClassMethod\RemoveUnusedParameterRector\Fixture;
+
+class SomeClass
+{
+    public function __construct($value)
+    {
+        $this->value = $value;
+    }
+}
+
+?>

packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedParameterRector/Fixture/order.php.inc

@@ -0,0 +1,27 @@
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\ClassMethod\RemoveUnusedParameterRector\Fixture;
+
+class Order
+{
+    public function __construct($value, $value2, $value3, $value4)
+    {
+        $result = $value + $value3;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\ClassMethod\RemoveUnusedParameterRector\Fixture;
+
+class Order
+{
+    public function __construct($value, $value2, $value3)
+    {
+        $result = $value + $value3;
+    }
+}
+
+?>

packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedParameterRector/Fixture/parent_required.php.inc

@@ -0,0 +1,20 @@
+<?php
+
+namespace Rector\DeadCode\Tests\Rector\ClassMethod\RemoveUnusedParameterRector\Fixture;
+
+class StrictPapa
+{
+    public function getIt($value, $value2)
+    {
+        $combo = $value + $value2;
+    }
+
+}
+
+class ParentRequired extends StrictPapa
+{
+    public function getIt($value, $value2)
+    {
+        $this->value = $value;
+    }
+}

packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedParameterRector/RemoveUnusedParameterRectorTest.php

@@ -0,0 +1,23 @@
+<?php declare(strict_types=1);
+
+namespace Rector\DeadCode\Tests\Rector\ClassMethod\RemoveUnusedParameterRector;
+
+use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedParameterRector;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class RemoveUnusedParameterRectorTest extends AbstractRectorTestCase
+{
+    public function test(): void
+    {
+        $this->doTestFiles([
+            __DIR__ . '/Fixture/fixture.php.inc',
+            __DIR__ . '/Fixture/order.php.inc',
+            __DIR__ . '/Fixture/parent_required.php.inc',
+        ]);
+    }
+
+    protected function getRectorClass(): string
+    {
+        return RemoveUnusedParameterRector::class;
+    }
+}

packages/NodeTypeResolver/src/NodeTypeAnalyzer.php

@@ -215,9 +215,7 @@ final class NodeTypeAnalyzer
         }
 
         // are the same variables
-        if ($this->betterStandardPrinter->prettyPrint(
-            [$funcCallNode->args[2]->value]
-        ) !== $this->betterStandardPrinter->prettyPrint([$node])) {
+        if (! $this->betterStandardPrinter->areNodesEqual($funcCallNode->args[2]->value, $node)) {
             return $originalType;
         }
 

src/PhpParser/Node/Maintainer/CallMaintainer.php

@@ -150,7 +150,7 @@ final class CallMaintainer
             return false;
         }
 
-        $printedFunction = $this->betterStandardPrinter->prettyPrint($externalFunctionNode->stmts);
+        $printedFunction = $this->betterStandardPrinter->print($externalFunctionNode->stmts);
 
         return (bool) Strings::match($printedFunction, '#\b(' . implode('|', self::VARIADIC_FUNCTION_NAMES) . ')\b#');
     }

src/PhpParser/Node/Maintainer/ClassMethodMaintainer.php

@@ -2,12 +2,39 @@
 
 namespace Rector\PhpParser\Node\Maintainer;
 
+use PhpParser\Node;
+use PhpParser\Node\Param;
 use PhpParser\Node\Stmt\ClassMethod;
 use Rector\NodeTypeResolver\Node\Attribute;
+use Rector\PhpParser\Node\BetterNodeFinder;
+use Rector\PhpParser\Printer\BetterStandardPrinter;
 use function Safe\class_implements;
 
 final class ClassMethodMaintainer
 {
+    /**
+     * @var BetterNodeFinder
+     */
+    private $betterNodeFinder;
+
+    /**
+     * @var BetterStandardPrinter
+     */
+    private $betterStandardPrinter;
+
+    public function __construct(BetterNodeFinder $betterNodeFinder, BetterStandardPrinter $betterStandardPrinter)
+    {
+        $this->betterNodeFinder = $betterNodeFinder;
+        $this->betterStandardPrinter = $betterStandardPrinter;
+    }
+
+    public function isParameterUsedMethod(Param $param, ClassMethod $classMethod): bool
+    {
+        return (bool) $this->betterNodeFinder->findFirst($classMethod->stmts, function (Node $node) use ($param) {
+            return $this->betterStandardPrinter->areNodesEqual($node, $param->var);
+        });
+    }
+
     public function hasParentMethodOrInterfaceMethod(ClassMethod $classMethod): bool
     {
         $class = $classMethod->getAttribute(Attribute::CLASS_NAME);

src/PhpParser/Node/Maintainer/PropertyMaintainer.php

@@ -49,7 +49,7 @@ final class PropertyMaintainer
 
         return $this->betterNodeFinder->find($classNode, function (Node $node) use ($propertyNode) {
             // itself
-            if ($this->areNodesEqual($node, $propertyNode)) {
+            if ($this->betterStandardPrinter->areNodesEqual($node, $propertyNode)) {
                 return null;
             }
 
@@ -66,14 +66,4 @@ final class PropertyMaintainer
             return $node;
         });
     }
-
-    private function areNodesEqual(Node $firstNode, Node $secondNode): bool
-    {
-        return $this->printNode($firstNode) === $this->printNode($secondNode);
-    }
-
-    private function printNode(Node $firstNode): string
-    {
-        return $this->betterStandardPrinter->prettyPrint([$firstNode]);
-    }
 }

src/PhpParser/Printer/BetterStandardPrinter.php

@@ -47,6 +47,31 @@ final class BetterStandardPrinter extends Standard
         $this->insertionMap['Stmt_Function->returnType'] = [')', ': ', null];
     }
 
+    /**
+     * @param Node|Node[]|null $node
+     */
+    public function print($node): string
+    {
+        if ($node === null) {
+            $node = [];
+        }
+
+        if (! is_array($node)) {
+            $node = [$node];
+        }
+
+        return $this->prettyPrint($node);
+    }
+
+    /**
+     * @param Node|Node[] $firstNode
+     * @param Node|Node[] $secondNode
+     */
+    public function areNodesEqual($firstNode, $secondNode): bool
+    {
+        return $this->print($firstNode) === $this->print($secondNode);
+    }
+
     /**
      * Do not preslash all slashes (parent behavior), but only those:
      *

src/Rector/BetterStandardPrinterTrait.php

@@ -38,15 +38,7 @@ trait BetterStandardPrinterTrait
      */
     public function print($node): string
     {
-        if ($node === null) {
-            $node = [];
-        }
-
-        if (! is_array($node)) {
-            $node = [$node];
-        }
-
-        return $this->betterStandardPrinter->prettyPrint($node);
+        return $this->betterStandardPrinter->print($node);
     }
 
     /**
@@ -55,7 +47,7 @@ trait BetterStandardPrinterTrait
      */
     protected function areNodesEqual($firstNode, $secondNode): bool
     {
-        return $this->print($firstNode) === $this->print($secondNode);
+        return $this->betterStandardPrinter->areNodesEqual($firstNode, $secondNode);
     }
 
     /**

tests/PhpParser/Printer/BetterStandardPrinterTest.php

@@ -25,7 +25,7 @@ final class BetterStandardPrinterTest extends AbstractContainerAwareTestCase
      */
     public function testDoubleSlashEscaping(string $content, string $expectedOutput): void
     {
-        $printed = $this->betterStandardPrinter->prettyPrint([new String_($content)]);
+        $printed = $this->betterStandardPrinter->print(new String_($content));
         $this->assertSame($expectedOutput, $printed);
     }
 
@@ -38,10 +38,10 @@ final class BetterStandardPrinterTest extends AbstractContainerAwareTestCase
 
     public function testYield(): void
     {
-        $printed = $this->betterStandardPrinter->prettyPrint([new Yield_(new String_('value'))]);
+        $printed = $this->betterStandardPrinter->print(new Yield_(new String_('value')));
         $this->assertSame("yield 'value'", $printed);
 
-        $printed = $this->betterStandardPrinter->prettyPrint([new Yield_()]);
+        $printed = $this->betterStandardPrinter->print(new Yield_());
         $this->assertSame('yield', $printed);
     }
 }