Skip to content

Commit

Permalink
[DeadCode] Skip used in next For_/Foreach_ on RemoveDeadIfForeachForR…
Browse files Browse the repository at this point in the history
…ector (#5154)

* [DeadCode] Skip used in next For_/Foreach_ on RemoveDeadIfForeachForRector

* [DeadCode] Skip used in next For_/Foreach_ on RemoveDeadIfForeachForRector
  • Loading branch information
samsonasik committed Oct 11, 2023
1 parent 6d96068 commit 0eb6000
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 23 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

namespace Rector\Tests\DeadCode\Rector\For_\RemoveDeadIfForeachForRector\Fixture;

class SkipUsedInNextStmt
{
public function run($resultLen, $result, $decimalSep)
{
for ($i = $resultLen - 1; $i >= 0 && $result[$i] === '0'; $i--) {
;
}

if ($i >= 0 && $result[$i] === $decimalSep) {
$i--;
}

$result = substr($result, 0, $i + 1);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

namespace Rector\Tests\DeadCode\Rector\For_\RemoveDeadIfForeachForRector\Fixture;

class SkipUsedInNextStmt2
{
public function run($resultLen, $result, $decimalSep)
{
foreach ($result as $i => $value) {
;
}

if ($i >= 0 && $result[$i] === $decimalSep) {
$i--;
}

$result = substr($result, 0, $i + 1);
}
}
110 changes: 87 additions & 23 deletions rules/DeadCode/Rector/For_/RemoveDeadIfForeachForRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\CallLike;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Stmt\Else_;
use PhpParser\Node\Stmt\For_;
use PhpParser\Node\Stmt\Foreach_;
use PhpParser\Node\Stmt\If_;
use PhpParser\NodeTraverser;
use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
use Rector\Core\NodeManipulator\StmtsManipulator;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\Core\Rector\AbstractRector;
use Rector\EarlyReturn\NodeTransformer\ConditionInverter;
Expand All @@ -24,9 +26,12 @@
*/
final class RemoveDeadIfForeachForRector extends AbstractRector
{
private bool $hasChanged = false;

public function __construct(
private readonly ConditionInverter $conditionInverter,
private readonly BetterNodeFinder $betterNodeFinder
private readonly BetterNodeFinder $betterNodeFinder,
private readonly StmtsManipulator $stmtsManipulator
) {
}

Expand Down Expand Up @@ -71,45 +76,104 @@ public function run($value)
*/
public function getNodeTypes(): array
{
return [For_::class, If_::class, Foreach_::class];
return [StmtsAwareInterface::class];
}

/**
* @param For_|If_|Foreach_ $node
* @param StmtsAwareInterface $node
*/
public function refactor(Node $node): Node|null|int
{
if ($node instanceof If_) {
if ($node->stmts !== []) {
return null;
}
if ($node->stmts === null) {
return null;
}

if ($node->elseifs !== []) {
return null;
$this->hasChanged = false;
foreach ($node->stmts as $key => $stmt) {
if (! $stmt instanceof If_ && ! $stmt instanceof For_ && ! $stmt instanceof Foreach_) {
continue;
}

// useless if ()
if (! $node->else instanceof Else_) {
if ($this->hasNodeSideEffect($node->cond)) {
return null;
}
if ($stmt->stmts !== []) {
continue;
}

return NodeTraverser::REMOVE_NODE;
if ($stmt instanceof If_) {
$this->processIf($stmt, $key, $node);
continue;
}

$node->cond = $this->conditionInverter->createInvertedCondition($node->cond);
$node->stmts = $node->else->stmts;
$node->else = null;
$this->processForForeach($stmt, $key, $node);
}

if ($this->hasChanged) {
return $node;
}

// nothing to "for"
if ($node->stmts === []) {
return NodeTraverser::REMOVE_NODE;
return null;
}

private function processIf(If_ $if, int $key, StmtsAwareInterface $stmtsAware): void
{
if ($if->elseifs !== []) {
return;
}

return null;
// useless if ()
if (! $if->else instanceof Else_) {
if ($this->hasNodeSideEffect($if->cond)) {
return;
}

unset($stmtsAware->stmts[$key]);
$this->hasChanged = true;
return;
}

$if->cond = $this->conditionInverter->createInvertedCondition($if->cond);
$if->stmts = $if->else->stmts;
$if->else = null;

$this->hasChanged = true;
}

private function processForForeach(For_|Foreach_ $for, int $key, StmtsAwareInterface $stmtsAware): void
{
if ($for instanceof For_) {
$variables = $this->betterNodeFinder->findInstanceOf(
[...$for->init, ...$for->cond, ...$for->loop],
Variable::class
);
foreach ($variables as $variable) {
if ($this->stmtsManipulator->isVariableUsedInNextStmt(
$stmtsAware,
$key + 1,
(string) $this->getName($variable)
)) {
return;
}
}

unset($stmtsAware->stmts[$key]);
$this->hasChanged = true;

return;
}

$exprs = array_filter([$for->expr, $for->valueVar, $for->valueVar]);
$variables = $this->betterNodeFinder->findInstanceOf($exprs, Variable::class);
foreach ($variables as $variable) {
if ($this->stmtsManipulator->isVariableUsedInNextStmt(
$stmtsAware,
$key + 1,
(string) $this->getName($variable)
)) {
return;
}
}

unset($stmtsAware->stmts[$key]);
$this->hasChanged = true;
}

private function hasNodeSideEffect(Expr $expr): bool
Expand Down

0 comments on commit 0eb6000

Please sign in to comment.