Skip to content

Commit

Permalink
[DeadCode] Do not remove used parameter on RemoveUnusedPrivatePropert…
Browse files Browse the repository at this point in the history
…yRector (#1337)

* [DeadCode] Handle RemoveUnusedPromotedPropertyRector + RemoveUnusedPrivatePropertyRector

* rem

* move external to Source

* eol

* fix

* eol

* [ci-review] Rector Rectify

* fix

Co-authored-by: GitHub Action <action@github.com>
  • Loading branch information
samsonasik and actions-user committed Nov 29, 2021
1 parent 4ad4d16 commit 50bbdd1
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

namespace Rector\Tests\DeadCode\Rector\Property\RemoveUnusedPrivatePropertyRector\Fixture;

use Rector\Tests\DeadCode\Rector\Property\RemoveUnusedPrivatePropertyRector\Source\AbstractSetting;

final class DemoSetting extends AbstractSetting
{
private PageInterface $page;
private SectionInterface $section;

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

parent::__construct($page, $section);
}

public function getPage()
{
return $this->page;
}
}

?>
-----
<?php

namespace Rector\Tests\DeadCode\Rector\Property\RemoveUnusedPrivatePropertyRector\Fixture;

use Rector\Tests\DeadCode\Rector\Property\RemoveUnusedPrivatePropertyRector\Source\AbstractSetting;

final class DemoSetting extends AbstractSetting
{
private PageInterface $page;

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

parent::__construct($page, $section);
}

public function getPage()
{
return $this->page;
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

declare(strict_types=1);

namespace Rector\Tests\DeadCode\Rector\Property\RemoveUnusedPrivatePropertyRector\Source;

abstract class AbstractSetting
{
public function __construct(PageInterface $page, SectionInterface $section)
{
// do something with $page and $section
$page->runSomething();
$section->runSomething();
}
}
39 changes: 39 additions & 0 deletions rules/Removing/NodeManipulator/ComplexNodeRemover.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use PhpParser\Node\Expr\Assign;
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\ClassLike;
Expand Down Expand Up @@ -153,6 +154,9 @@ private function removeConstructorDependency(Assign $assign): void

$params = $constructClassMethod->getParams();
$paramKeysToBeRemoved = [];

/** @var Variable[] $variables */
$variables = $this->resolveVariables($constructClassMethod);
foreach ($params as $key => $param) {
$variable = $this->betterNodeFinder->findFirst(
(array) $constructClassMethod->stmts,
Expand All @@ -171,12 +175,47 @@ private function removeConstructorDependency(Assign $assign): void
continue;
}

if ($this->isInVariables($variables, $assign)) {
continue;
}

$paramKeysToBeRemoved[] = $key;
}

$this->processRemoveParamWithKeys($params, $paramKeysToBeRemoved);
}

/**
* @return Variable[]
*/
private function resolveVariables(ClassMethod $classMethod): array
{
return $this->betterNodeFinder->find(
(array) $classMethod->stmts,
function (Node $subNode): bool {
if (! $subNode instanceof Variable) {
return false;
}

return $this->isExpressionVariableNotAssign($subNode);
}
);
}

/**
* @param Variable[] $variables
*/
private function isInVariables(array $variables, Assign $assign): bool
{
foreach ($variables as $variable) {
if ($this->nodeComparator->areNodesEqual($assign->expr, $variable)) {
return true;
}
}

return false;
}

/**
* @param Param[] $params
* @param int[] $paramKeysToBeRemoved
Expand Down

0 comments on commit 50bbdd1

Please sign in to comment.