From 68906c7e178d486c9f6de95b70b40e8af70793f8 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sun, 22 May 2022 18:42:14 +0700 Subject: [PATCH] [DeadCode] Skip used in compact() on RemoveUnusedConstructorParamRector (#2345) * [DeadCode] Skip used in compact() on RemoveUnusedConstructorParamRector * Fixed :tada: * phpstan * phpstan * [ci-review] Rector Rectify Co-authored-by: GitHub Action --- .../Fixture/skip_used_in_compact.php.inc | 13 ++++++++ .../NodeCollector/UnusedParameterResolver.php | 6 ++-- src/NodeAnalyzer/ParamAnalyzer.php | 33 +++++++++++++++---- .../ClassMethodManipulator.php | 29 ---------------- 4 files changed, 42 insertions(+), 39 deletions(-) create mode 100644 rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedConstructorParamRector/Fixture/skip_used_in_compact.php.inc diff --git a/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedConstructorParamRector/Fixture/skip_used_in_compact.php.inc b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedConstructorParamRector/Fixture/skip_used_in_compact.php.inc new file mode 100644 index 00000000000..258e40750a3 --- /dev/null +++ b/rules-tests/DeadCode/Rector/ClassMethod/RemoveUnusedConstructorParamRector/Fixture/skip_used_in_compact.php.inc @@ -0,0 +1,13 @@ +data = compact('hey', 'man'); + } +} diff --git a/rules/DeadCode/NodeCollector/UnusedParameterResolver.php b/rules/DeadCode/NodeCollector/UnusedParameterResolver.php index 189f47431aa..d7a01e85f0a 100644 --- a/rules/DeadCode/NodeCollector/UnusedParameterResolver.php +++ b/rules/DeadCode/NodeCollector/UnusedParameterResolver.php @@ -6,12 +6,12 @@ use PhpParser\Node\Param; use PhpParser\Node\Stmt\ClassMethod; -use Rector\Core\NodeManipulator\ClassMethodManipulator; +use Rector\Core\NodeAnalyzer\ParamAnalyzer; final class UnusedParameterResolver { public function __construct( - private readonly ClassMethodManipulator $classMethodManipulator + private readonly ParamAnalyzer $paramAnalyzer ) { } @@ -30,7 +30,7 @@ public function resolve(ClassMethod $classMethod): array continue; } - if ($this->classMethodManipulator->isParameterUsedInClassMethod($param, $classMethod)) { + if ($this->paramAnalyzer->isParamUsedInClassMethod($classMethod, $param)) { continue; } diff --git a/src/NodeAnalyzer/ParamAnalyzer.php b/src/NodeAnalyzer/ParamAnalyzer.php index 7265746303e..9cb6a26cab9 100644 --- a/src/NodeAnalyzer/ParamAnalyzer.php +++ b/src/NodeAnalyzer/ParamAnalyzer.php @@ -7,20 +7,25 @@ use PhpParser\Node; use PhpParser\Node\Expr\Closure; use PhpParser\Node\Expr\ConstFetch; +use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\Variable; use PhpParser\Node\NullableType; use PhpParser\Node\Param; use PhpParser\Node\Stmt\ClassMethod; +use Rector\Core\NodeManipulator\FuncCallManipulator; use Rector\Core\PhpParser\Comparing\NodeComparator; use Rector\Core\PhpParser\Node\BetterNodeFinder; use Rector\Core\PhpParser\Node\Value\ValueResolver; +use Rector\NodeNameResolver\NodeNameResolver; final class ParamAnalyzer { public function __construct( private readonly BetterNodeFinder $betterNodeFinder, private readonly NodeComparator $nodeComparator, - private readonly ValueResolver $valueResolver + private readonly ValueResolver $valueResolver, + private readonly NodeNameResolver $nodeNameResolver, + private readonly FuncCallManipulator $funcCallManipulator ) { } @@ -29,7 +34,7 @@ public function isParamUsedInClassMethod(ClassMethod $classMethod, Param $param) return (bool) $this->betterNodeFinder->findFirstInFunctionLikeScoped($classMethod, function (Node $node) use ( $param ): bool { - if (! $node instanceof Variable && ! $node instanceof Closure) { + if (! $node instanceof Variable && ! $node instanceof Closure && ! $node instanceof FuncCall) { return false; } @@ -37,13 +42,16 @@ public function isParamUsedInClassMethod(ClassMethod $classMethod, Param $param) return $this->nodeComparator->areNodesEqual($node, $param->var); } - foreach ($node->uses as $use) { - if ($this->nodeComparator->areNodesEqual($use->var, $param->var)) { - return true; - } + if ($node instanceof Closure) { + return $this->isVariableInClosureUses($node, $param->var); } - return false; + if (! $this->nodeNameResolver->isName($node, 'compact')) { + return false; + } + + $arguments = $this->funcCallManipulator->extractArgumentsFromCompactFuncCalls([$node]); + return $this->nodeNameResolver->isNames($param, $arguments); }); } @@ -78,4 +86,15 @@ public function hasDefaultNull(Param $param): bool { return $param->default instanceof ConstFetch && $this->valueResolver->isNull($param->default); } + + private function isVariableInClosureUses(Closure $closure, Variable $variable): bool + { + foreach ($closure->uses as $use) { + if ($this->nodeComparator->areNodesEqual($use->var, $variable)) { + return true; + } + } + + return false; + } } diff --git a/src/NodeManipulator/ClassMethodManipulator.php b/src/NodeManipulator/ClassMethodManipulator.php index 499c4431616..9d0804b8ebd 100644 --- a/src/NodeManipulator/ClassMethodManipulator.php +++ b/src/NodeManipulator/ClassMethodManipulator.php @@ -5,7 +5,6 @@ namespace Rector\Core\NodeManipulator; use PhpParser\Node; -use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\Variable; use PhpParser\Node\Name\FullyQualified; use PhpParser\Node\Param; @@ -14,7 +13,6 @@ use PHPStan\Reflection\ClassReflection; use PHPStan\Type\ObjectType; use Rector\Core\Exception\ShouldNotHappenException; -use Rector\Core\PhpParser\Comparing\NodeComparator; use Rector\Core\PhpParser\Node\BetterNodeFinder; use Rector\Core\Reflection\ReflectionResolver; use Rector\Core\ValueObject\MethodName; @@ -27,37 +25,10 @@ public function __construct( private readonly BetterNodeFinder $betterNodeFinder, private readonly NodeNameResolver $nodeNameResolver, private readonly NodeTypeResolver $nodeTypeResolver, - private readonly NodeComparator $nodeComparator, - private readonly FuncCallManipulator $funcCallManipulator, private readonly ReflectionResolver $reflectionResolver ) { } - public function isParameterUsedInClassMethod(Param $param, ClassMethod $classMethod): bool - { - $isUsedDirectly = (bool) $this->betterNodeFinder->findFirst( - (array) $classMethod->stmts, - fn (Node $node): bool => $this->nodeComparator->areNodesEqual($node, $param->var) - ); - - if ($isUsedDirectly) { - return true; - } - - /** @var FuncCall[] $compactFuncCalls */ - $compactFuncCalls = $this->betterNodeFinder->find((array) $classMethod->stmts, function (Node $node): bool { - if (! $node instanceof FuncCall) { - return false; - } - - return $this->nodeNameResolver->isName($node, 'compact'); - }); - - $arguments = $this->funcCallManipulator->extractArgumentsFromCompactFuncCalls($compactFuncCalls); - - return $this->nodeNameResolver->isNames($param, $arguments); - } - public function isNamedConstructor(ClassMethod $classMethod): bool { if (! $this->nodeNameResolver->isName($classMethod, MethodName::CONSTRUCT)) {