Skip to content

Commit

Permalink
Remove PARENT_NODE from RemoveUnusedConstructorParamRector (#4014)
Browse files Browse the repository at this point in the history
  • Loading branch information
TomasVotruba committed May 29, 2023
1 parent 32510e0 commit f15cbd7
Show file tree
Hide file tree
Showing 12 changed files with 32 additions and 183 deletions.
10 changes: 0 additions & 10 deletions packages/NodeRemoval/NodeRemover.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,4 @@ public function removeNode(Node $node): void
$this->nodesToRemoveCollector->addNodeToRemove($node);
$this->rectorChangeCollector->notifyNodeFileInfo($node);
}

/**
* @param Node[] $nodes
*/
public function removeNodes(array $nodes): void
{
foreach ($nodes as $node) {
$this->removeNode($node);
}
}
}
1 change: 1 addition & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,7 @@ parameters:

# resolve before 1.0
- '#Parameter \$nodesToAddCollector of (.*?) has typehint with deprecated class Rector\\PostRector\\Collector\\NodesToAddCollector#'
- '#Cognitive complexity for "Rector\\Removing\\NodeManipulator\\ComplexNodeRemover\:\:removeConstructorDependency\(\)" is 13, keep it under 10#'

- '#Call to deprecated method addNodeToRemove\(\) of class Rector\\PostRector\\Collector\\NodesToRemoveCollector\:#'
- '#Call to deprecated method removeNode\(\) of class Rector\\NodeRemoval\\NodeRemover#'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ namespace Rector\Tests\DeadCode\Rector\Property\RemoveUnusedPrivatePropertyRecto

final class ConstructorWithMiddleParams
{
public function __construct(int $contentFinder, \stdClass $stdClass)
public function __construct(\stdClass $stdClass)
{
$this->init($stdClass);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ final class DemoSetting extends AbstractSetting
private PageInterface $page;
private SectionInterface $section;

public function __construct(PageInterface $page, SectionInterface $section)
public function __construct(PageInterface $page, SectionInterface $section)
{
$this->page = $page;
$this->section = $section;
Expand All @@ -35,7 +35,7 @@ final class DemoSetting extends AbstractSetting
{
private PageInterface $page;

public function __construct(PageInterface $page, SectionInterface $section)
public function __construct(PageInterface $page, SectionInterface $section)
{
$this->page = $page;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
use Rector\Core\NodeAnalyzer\ParamAnalyzer;
use Rector\Core\Rector\AbstractRector;
use Rector\Core\ValueObject\MethodName;
use Rector\Removing\NodeManipulator\ComplexNodeRemover;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

Expand All @@ -21,7 +20,6 @@ final class RemoveUnusedConstructorParamRector extends AbstractRector
{
public function __construct(
private readonly ParamAnalyzer $paramAnalyzer,
private readonly ComplexNodeRemover $complexNodeRemover
) {
}

Expand Down Expand Up @@ -62,57 +60,56 @@ public function __construct($hey)
*/
public function getNodeTypes(): array
{
return [ClassMethod::class];
return [Class_::class];
}

/**
* @param ClassMethod $node
* @param Class_ $node
*/
public function refactor(Node $node): ?Node
{
if (! $this->isName($node, MethodName::CONSTRUCT)) {
$constructorClassMethod = $node->getMethod(MethodName::CONSTRUCT);
if (! $constructorClassMethod instanceof ClassMethod) {
return null;
}

if ($node->params === []) {
if ($constructorClassMethod->params === []) {
return null;
}

if ($this->paramAnalyzer->hasPropertyPromotion($node->params)) {
if ($this->paramAnalyzer->hasPropertyPromotion($constructorClassMethod->params)) {
return null;
}

$class = $this->betterNodeFinder->findParentType($node, Class_::class);
if (! $class instanceof Class_) {
if ($constructorClassMethod->isAbstract()) {
return null;
}

if ($node->isAbstract()) {
$changedConstructorClassMethod = $this->processRemoveParams($constructorClassMethod);
if (! $changedConstructorClassMethod instanceof ClassMethod) {
return null;
}

return $this->processRemoveParams($node);
return $node;
}

private function processRemoveParams(ClassMethod $classMethod): ?ClassMethod
{
$paramKeysToBeRemoved = [];
$hasChanged = false;

foreach ($classMethod->params as $key => $param) {
if ($this->paramAnalyzer->isParamUsedInClassMethod($classMethod, $param)) {
continue;
}

$paramKeysToBeRemoved[] = $key;
unset($classMethod->params[$key]);
$hasChanged = true;
}

$removedParamKeys = $this->complexNodeRemover
->processRemoveParamWithKeys($classMethod->params, $paramKeysToBeRemoved);

if ($removedParamKeys === []) {
return null;
if ($hasChanged) {
return $classMethod;
}

return $classMethod;
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,7 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node
return null;
}

/** @var Class_ $classLike */
$classLike = $this->betterNodeFinder->findParentType($node, Class_::class);
$this->propertyFetchWithConstFetchReplacer->replace($classLike, $node);

$this->propertyFetchWithConstFetchReplacer->replace($class, $node);
return $this->classConstantFactory->createFromProperty($node);
}

Expand Down
47 changes: 11 additions & 36 deletions rules/Removing/NodeManipulator/ComplexNodeRemover.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\StaticPropertyFetch;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Param;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Expression;
Expand Down Expand Up @@ -111,45 +110,15 @@ public function removePropertyAndUsages(

$this->removeConstructorDependency($class, $propertyName);

$this->nodeRemover->removeNodes($expressions);
foreach ($expressions as $expression) {
$this->nodeRemover->removeNode($expression);
}

$this->nodeRemover->removeNode($property);

return true;
}

/**
* @param Param[] $params
* @param int[] $paramKeysToBeRemoved
* @return int[]
*/
public function processRemoveParamWithKeys(array $params, array $paramKeysToBeRemoved): array
{
$totalKeys = count($params) - 1;
$removedParamKeys = [];

foreach ($paramKeysToBeRemoved as $paramKeyToBeRemoved) {
$startNextKey = $paramKeyToBeRemoved + 1;
for ($nextKey = $startNextKey; $nextKey <= $totalKeys; ++$nextKey) {
if (! isset($params[$nextKey])) {
// no next param, break the inner loop, remove the param
break;
}

if (in_array($nextKey, $paramKeysToBeRemoved, true)) {
// keep searching next key not in $paramKeysToBeRemoved
continue;
}

return [];
}

$this->nodeRemover->removeNode($params[$paramKeyToBeRemoved]);
$removedParamKeys[] = $paramKeyToBeRemoved;
}

return $removedParamKeys;
}

private function removeConstructorDependency(Class_ $class, string $propertyName): void
{
$classMethod = $class->getMethod(MethodName::CONSTRUCT);
Expand Down Expand Up @@ -197,7 +166,13 @@ private function removeConstructorDependency(Class_ $class, string $propertyName
return;
}

$this->processRemoveParamWithKeys($classMethod->getParams(), $paramKeysToBeRemoved);
foreach (array_keys($classMethod->params) as $key) {
if (! in_array($key, $paramKeysToBeRemoved, true)) {
continue;
}

unset($classMethod->params[$key]);
}
}

/**
Expand Down

This file was deleted.

1 change: 0 additions & 1 deletion tests/Issues/Issue7495/Fixture/fixture.php.inc
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ class Fixture
private Registry $coreRegistry;

public function __construct(
UrlInterface $urlInterface,
Template\Context $context,
GetInvoicesHelper $invoicesHelper,
Registry $registry,
Expand Down

This file was deleted.

This file was deleted.

12 changes: 0 additions & 12 deletions tests/Issues/RemoveUnusedParamInMiddle/config/configured_rule.php

This file was deleted.

0 comments on commit f15cbd7

Please sign in to comment.