Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Solid] Skip ChangeAndIfToEarlyReturnRector on nested if in loop #4757

Merged
merged 15 commits into from
Dec 3, 2020
25 changes: 21 additions & 4 deletions rules/solid/src/Rector/If_/ChangeAndIfToEarlyReturnRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
use PhpParser\Node\FunctionLike;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Continue_;
use PhpParser\Node\Stmt\Else_;
use PhpParser\Node\Stmt\ElseIf_;
use PhpParser\Node\Stmt\For_;
use PhpParser\Node\Stmt\Foreach_;
use PhpParser\Node\Stmt\If_;
Expand All @@ -28,6 +30,11 @@
*/
final class ChangeAndIfToEarlyReturnRector extends AbstractRector
{
/**
* @var string[]
*/
public const LOOP_TYPES = [Foreach_::class, For_::class, While_::class];

/**
* @var IfManipulator
*/
Expand Down Expand Up @@ -166,6 +173,10 @@ private function shouldSkip(If_ $if): bool
return true;
}

if ($this->isNestedIfInLoop($if)) {
return true;
}

return ! $this->isLastIfOrBeforeLastReturn($if);
}

Expand Down Expand Up @@ -243,10 +254,7 @@ private function getIfNextReturn(If_ $if): ?Return_

private function isIfInLoop(If_ $if): bool
{
$parentLoop = $this->betterNodeFinder->findFirstParentInstanceOf(
$if,
[Foreach_::class, For_::class, While_::class]
);
$parentLoop = $this->betterNodeFinder->findFirstParentInstanceOf($if, self::LOOP_TYPES);

return $parentLoop !== null;
}
Expand Down Expand Up @@ -291,6 +299,15 @@ private function isFunctionLikeReturnsVoid(If_ $if): bool
return $nonVoidReturns === [];
}

private function isNestedIfInLoop(If_ $if): bool
{
return $this->isIfInLoop($if)
&& (bool) $this->betterNodeFinder->findFirstParentInstanceOf(
$if,
[If_::class, Else_::class, ElseIf_::class]
);
}

private function isLastIfOrBeforeLastReturn(If_ $if): bool
{
$nextNode = $if->getAttribute(AttributeKey::NEXT_NODE);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

namespace Rector\SOLID\Tests\Rector\If_\ChangeAndIfToEarlyReturnRector\Fixture;

use PhpParser\Node\Identifier;
use PhpParser\Node\Name;

class SkipNestedIfInLoop
{
private function refactorStmts(array $nodes): void
{
$g = [];

foreach ($nodes as $node) {
$x = ['A', 'B'];
if (rand(1,2)) {
// something
} else {
if ($node instanceof Name && $node instanceof Identifier) {
$x = [];
}
}

$g = $x;
}
}
}

?>