diff --git a/config/level/dead-code/dead-code.yml b/config/level/dead-code/dead-code.yml index f10ec7b8990..ba186901fb3 100644 --- a/config/level/dead-code/dead-code.yml +++ b/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: ~ diff --git a/packages/ContributorTools/src/TemplateVariablesFactory.php b/packages/ContributorTools/src/TemplateVariablesFactory.php index 3913aef0629..b1293a48c76 100644 --- a/packages/ContributorTools/src/TemplateVariablesFactory.php +++ b/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)); } } diff --git a/packages/DeadCode/src/Rector/ClassMethod/RemoveUnusedParameterRector.php b/packages/DeadCode/src/Rector/ClassMethod/RemoveUnusedParameterRector.php new file mode 100644 index 00000000000..4c27d4d5bc4 --- /dev/null +++ b/packages/DeadCode/src/Rector/ClassMethod/RemoveUnusedParameterRector.php @@ -0,0 +1,104 @@ +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; + } +} diff --git a/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedParameterRector/Fixture/fixture.php.inc b/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedParameterRector/Fixture/fixture.php.inc new file mode 100644 index 00000000000..fef91219db0 --- /dev/null +++ b/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedParameterRector/Fixture/fixture.php.inc @@ -0,0 +1,27 @@ +value = $value; + } +} + +?> +----- +value = $value; + } +} + +?> diff --git a/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedParameterRector/Fixture/order.php.inc b/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedParameterRector/Fixture/order.php.inc new file mode 100644 index 00000000000..2599ad25b15 --- /dev/null +++ b/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedParameterRector/Fixture/order.php.inc @@ -0,0 +1,27 @@ + +----- + diff --git a/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedParameterRector/Fixture/parent_required.php.inc b/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedParameterRector/Fixture/parent_required.php.inc new file mode 100644 index 00000000000..523876c4aa3 --- /dev/null +++ b/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedParameterRector/Fixture/parent_required.php.inc @@ -0,0 +1,20 @@ +value = $value; + } +} diff --git a/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedParameterRector/RemoveUnusedParameterRectorTest.php b/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedParameterRector/RemoveUnusedParameterRectorTest.php new file mode 100644 index 00000000000..b9d0a25aa3e --- /dev/null +++ b/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedParameterRector/RemoveUnusedParameterRectorTest.php @@ -0,0 +1,23 @@ +doTestFiles([ + __DIR__ . '/Fixture/fixture.php.inc', + __DIR__ . '/Fixture/order.php.inc', + __DIR__ . '/Fixture/parent_required.php.inc', + ]); + } + + protected function getRectorClass(): string + { + return RemoveUnusedParameterRector::class; + } +} diff --git a/packages/NodeTypeResolver/src/NodeTypeAnalyzer.php b/packages/NodeTypeResolver/src/NodeTypeAnalyzer.php index 7d12c19b024..1e18d82abc7 100644 --- a/packages/NodeTypeResolver/src/NodeTypeAnalyzer.php +++ b/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; } diff --git a/src/PhpParser/Node/Maintainer/CallMaintainer.php b/src/PhpParser/Node/Maintainer/CallMaintainer.php index 584fede1a9a..5c833885657 100644 --- a/src/PhpParser/Node/Maintainer/CallMaintainer.php +++ b/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#'); } diff --git a/src/PhpParser/Node/Maintainer/ClassMethodMaintainer.php b/src/PhpParser/Node/Maintainer/ClassMethodMaintainer.php index 9b441e655a5..100fcb63d4a 100644 --- a/src/PhpParser/Node/Maintainer/ClassMethodMaintainer.php +++ 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); diff --git a/src/PhpParser/Node/Maintainer/PropertyMaintainer.php b/src/PhpParser/Node/Maintainer/PropertyMaintainer.php index bd416dfdce4..43e1751c0fc 100644 --- a/src/PhpParser/Node/Maintainer/PropertyMaintainer.php +++ b/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]); - } } diff --git a/src/PhpParser/Printer/BetterStandardPrinter.php b/src/PhpParser/Printer/BetterStandardPrinter.php index e3deec9f9d6..90e2cc03c1a 100644 --- a/src/PhpParser/Printer/BetterStandardPrinter.php +++ b/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: * diff --git a/src/Rector/BetterStandardPrinterTrait.php b/src/Rector/BetterStandardPrinterTrait.php index 4328aecf0e9..3110d9dd98c 100644 --- a/src/Rector/BetterStandardPrinterTrait.php +++ b/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); } /** diff --git a/tests/PhpParser/Printer/BetterStandardPrinterTest.php b/tests/PhpParser/Printer/BetterStandardPrinterTest.php index de0e5e71657..7bd535d1263 100644 --- a/tests/PhpParser/Printer/BetterStandardPrinterTest.php +++ b/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); } }