mirror of
https://github.com/rectorphp/rector.git
synced 2025-01-18 22:08:00 +01:00
[DeadCode] Do not remove variable that is re-assigned
This commit is contained in:
parent
f7043f956a
commit
3f2f64de22
2
ecs.yaml
2
ecs.yaml
@ -56,6 +56,8 @@ parameters:
|
|||||||
|
|
||||||
skip:
|
skip:
|
||||||
Symplify\CodingStandard\Sniffs\Architecture\DuplicatedClassShortNameSniff: ~
|
Symplify\CodingStandard\Sniffs\Architecture\DuplicatedClassShortNameSniff: ~
|
||||||
|
# skip temporary due to missing "import" feature in PhpStorm
|
||||||
|
SlevomatCodingStandard\Sniffs\Namespaces\ReferenceUsedNamesOnlySniff.PartialUse: ~
|
||||||
|
|
||||||
# run manually from time to time - performance demanding + not to bother user with it
|
# run manually from time to time - performance demanding + not to bother user with it
|
||||||
Symplify\CodingStandard\Fixer\Order\PropertyOrderByComplexityFixer: ~
|
Symplify\CodingStandard\Fixer\Order\PropertyOrderByComplexityFixer: ~
|
||||||
|
@ -77,9 +77,37 @@ CODE_SAMPLE
|
|||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if ($this->shouldSkipDueToForeachOverride($node, $previousExpression)) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
// no calls on right, could hide e.g. array_pop()|array_shift()
|
// no calls on right, could hide e.g. array_pop()|array_shift()
|
||||||
$this->removeNode($node);
|
$this->removeNode($node);
|
||||||
|
|
||||||
return $node;
|
return $node;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private function shouldSkipDueToForeachOverride(Assign $assign, Node $node): bool
|
||||||
|
{
|
||||||
|
// is nested in a foreach and the previous expression is not?
|
||||||
|
$nodePreviousForeach = $this->betterNodeFinder->findFirstParentInstanceOf($assign, Node\Stmt\Foreach_::class);
|
||||||
|
|
||||||
|
$previousExpressionPreviousForeach = $this->betterNodeFinder->findFirstParentInstanceOf(
|
||||||
|
$node,
|
||||||
|
Node\Stmt\Foreach_::class
|
||||||
|
);
|
||||||
|
|
||||||
|
if ($nodePreviousForeach !== $previousExpressionPreviousForeach) {
|
||||||
|
if ($nodePreviousForeach instanceof Node\Stmt\Foreach_ && $assign->var instanceof Variable) {
|
||||||
|
// is value changed inside the foreach?
|
||||||
|
|
||||||
|
$variableAssigns = $this->betterNodeFinder->findAssignsOfVariable($nodePreviousForeach, $assign->var);
|
||||||
|
|
||||||
|
// there is probably value override
|
||||||
|
return count($variableAssigns) >= 2;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return false;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
@ -0,0 +1,25 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
namespace Rector\DeadCode\Tests\Rector\Assign\RemoveDoubleAssignRector\Fixture;
|
||||||
|
|
||||||
|
class KeepArrayReset
|
||||||
|
{
|
||||||
|
public function create()
|
||||||
|
{
|
||||||
|
$values = [];
|
||||||
|
$unusedParameters = [];
|
||||||
|
|
||||||
|
foreach ($values as $param) {
|
||||||
|
if (true) {
|
||||||
|
// reset
|
||||||
|
$unusedParameters = [];
|
||||||
|
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
$unusedParameters[] = $param;
|
||||||
|
}
|
||||||
|
|
||||||
|
return $unusedParameters;
|
||||||
|
}
|
||||||
|
}
|
@ -14,6 +14,7 @@ final class RemoveDoubleAssignRectorTest extends AbstractRectorTestCase
|
|||||||
__DIR__ . '/Fixture/calls.php.inc',
|
__DIR__ . '/Fixture/calls.php.inc',
|
||||||
__DIR__ . '/Fixture/keep_dim_assign.php.inc',
|
__DIR__ . '/Fixture/keep_dim_assign.php.inc',
|
||||||
__DIR__ . '/Fixture/property_assign.php.inc',
|
__DIR__ . '/Fixture/property_assign.php.inc',
|
||||||
|
__DIR__ . '/Fixture/keep_array_reset.php.inc',
|
||||||
]);
|
]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -7,6 +7,7 @@ use PhpParser\Node\Stmt;
|
|||||||
use PhpParser\Node\Stmt\Expression;
|
use PhpParser\Node\Stmt\Expression;
|
||||||
use PhpParser\NodeFinder;
|
use PhpParser\NodeFinder;
|
||||||
use Rector\NodeTypeResolver\Node\Attribute;
|
use Rector\NodeTypeResolver\Node\Attribute;
|
||||||
|
use Rector\PhpParser\Printer\BetterStandardPrinter;
|
||||||
|
|
||||||
final class BetterNodeFinder
|
final class BetterNodeFinder
|
||||||
{
|
{
|
||||||
@ -15,9 +16,31 @@ final class BetterNodeFinder
|
|||||||
*/
|
*/
|
||||||
private $nodeFinder;
|
private $nodeFinder;
|
||||||
|
|
||||||
public function __construct(NodeFinder $nodeFinder)
|
/**
|
||||||
|
* @var BetterStandardPrinter
|
||||||
|
*/
|
||||||
|
private $betterStandardPrinter;
|
||||||
|
|
||||||
|
public function __construct(NodeFinder $nodeFinder, BetterStandardPrinter $betterStandardPrinter)
|
||||||
{
|
{
|
||||||
$this->nodeFinder = $nodeFinder;
|
$this->nodeFinder = $nodeFinder;
|
||||||
|
$this->betterStandardPrinter = $betterStandardPrinter;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function findFirstParentInstanceOf(Node $node, string $type): ?Node
|
||||||
|
{
|
||||||
|
/** @var Node|null $parentNode */
|
||||||
|
$parentNode = $node->getAttribute(Attribute::PARENT_NODE);
|
||||||
|
do {
|
||||||
|
if ($parentNode instanceof $type) {
|
||||||
|
return $parentNode;
|
||||||
|
}
|
||||||
|
if ($parentNode === null) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
} while ($parentNode = $parentNode->getAttribute(Attribute::PARENT_NODE));
|
||||||
|
|
||||||
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
public function findFirstAncestorInstanceOf(Node $node, string $type): ?Node
|
public function findFirstAncestorInstanceOf(Node $node, string $type): ?Node
|
||||||
@ -102,4 +125,24 @@ final class BetterNodeFinder
|
|||||||
|
|
||||||
return $this->findFirstPrevious($previousExpression, $filter);
|
return $this->findFirstPrevious($previousExpression, $filter);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @return Node\Expr\Assign[]
|
||||||
|
*/
|
||||||
|
public function findAssignsOfVariable(Node $node, Node\Expr\Variable $variable): array
|
||||||
|
{
|
||||||
|
$assignNodes = $this->findInstanceOf($node, Node\Expr\Assign::class);
|
||||||
|
|
||||||
|
return array_filter($assignNodes, function (Node\Expr\Assign $assign) use ($variable) {
|
||||||
|
if ($this->betterStandardPrinter->areNodesEqual($assign->var, $variable)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($assign->var instanceof Node\Expr\ArrayDimFetch) {
|
||||||
|
return $this->betterStandardPrinter->areNodesEqual($assign->var->var, $variable);
|
||||||
|
}
|
||||||
|
|
||||||
|
return false;
|
||||||
|
});
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
@ -7,14 +7,14 @@ use PhpParser\Node\Expr\Array_;
|
|||||||
use PhpParser\Node\Expr\Variable;
|
use PhpParser\Node\Expr\Variable;
|
||||||
use PhpParser\Node\Stmt\Class_;
|
use PhpParser\Node\Stmt\Class_;
|
||||||
use PhpParser\Node\Stmt\ClassLike;
|
use PhpParser\Node\Stmt\ClassLike;
|
||||||
use PhpParser\NodeFinder;
|
|
||||||
use PhpParser\NodeTraverser;
|
use PhpParser\NodeTraverser;
|
||||||
use PhpParser\ParserFactory;
|
use PhpParser\ParserFactory;
|
||||||
use PHPUnit\Framework\TestCase;
|
use Rector\HttpKernel\RectorKernel;
|
||||||
use Rector\NodeTypeResolver\NodeVisitor\ParentAndNextNodeVisitor;
|
use Rector\NodeTypeResolver\NodeVisitor\ParentAndNextNodeVisitor;
|
||||||
use Rector\PhpParser\Node\BetterNodeFinder;
|
use Rector\PhpParser\Node\BetterNodeFinder;
|
||||||
|
use Symplify\PackageBuilder\Tests\AbstractKernelTestCase;
|
||||||
|
|
||||||
final class BetterNodeFinderTest extends TestCase
|
final class BetterNodeFinderTest extends AbstractKernelTestCase
|
||||||
{
|
{
|
||||||
/**
|
/**
|
||||||
* @var Node[]
|
* @var Node[]
|
||||||
@ -28,7 +28,9 @@ final class BetterNodeFinderTest extends TestCase
|
|||||||
|
|
||||||
protected function setUp(): void
|
protected function setUp(): void
|
||||||
{
|
{
|
||||||
$this->betterNodeFinder = new BetterNodeFinder(new NodeFinder());
|
$this->bootKernel(RectorKernel::class);
|
||||||
|
|
||||||
|
$this->betterNodeFinder = self::$container->get(BetterNodeFinder::class);
|
||||||
$this->nodes = $this->createNodesFromFile(__DIR__ . '/Source/SomeFile.php.inc');
|
$this->nodes = $this->createNodesFromFile(__DIR__ . '/Source/SomeFile.php.inc');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Loading…
x
Reference in New Issue
Block a user