[EarlyReturn] Handle Return Parent next if on ChangeAndIfToEarlyReturnRector (#5711)

* Add failing test fixture for ChangeAndIfToEarlyReturnRector

# Failing Test for ChangeAndIfToEarlyReturnRector

Based on https://getrector.org/demo/aef6b329-49b5-4f27-b23e-9e879a0e2e0b

* Closes #5707

* phpstan

* move find boolean and conditions to NodeRepository

* final touch

Co-authored-by: Markus Staab <maggus.staab@googlemail.com>
This commit is contained in:
Abdul Malik Ikhsan 2021-02-28 22:07:12 +07:00 committed by GitHub
parent df101bcbb0
commit a62bb98fe9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 108 additions and 23 deletions

View File

@ -8,7 +8,10 @@ use Nette\Utils\Arrays;
use Nette\Utils\Strings;
use PhpParser\Node;
use PhpParser\Node\Attribute;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\Array_;
use PhpParser\Node\Expr\BinaryOp;
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
use PhpParser\Node\Expr\ClassConstFetch;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\MethodCall;
@ -672,6 +675,26 @@ final class NodeRepository
return $callerObjectType->getClassName();
}
/**
* @return Expr[]
*/
public function findBooleanAndConditions(BooleanAnd $booleanAnd): array
{
$conditions = [];
while ($booleanAnd instanceof BinaryOp) {
$conditions[] = $booleanAnd->right;
$booleanAnd = $booleanAnd->left;
if (! $booleanAnd instanceof BooleanAnd) {
$conditions[] = $booleanAnd;
break;
}
}
krsort($conditions);
return $conditions;
}
private function collectArray(Array_ $array): void
{
$arrayCallable = $this->arrayCallableMethodReferenceAnalyzer->match($array);

View File

@ -6,8 +6,8 @@ namespace Rector\EarlyReturn\Rector\If_;
use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\BinaryOp;
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
use PhpParser\Node\Expr\Closure;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Continue_;
use PhpParser\Node\Stmt\Else_;
@ -115,7 +115,7 @@ CODE_SAMPLE
/** @var BooleanAnd $expr */
$expr = $node->cond;
$conditions = $this->getBooleanAndConditions($expr);
$conditions = $this->nodeRepository->findBooleanAndConditions($expr);
$ifNextReturnClone = $ifNextReturn instanceof Return_
? clone $ifNextReturn
: new Return_();
@ -187,26 +187,6 @@ CODE_SAMPLE
return ! $this->isLastIfOrBeforeLastReturn($if);
}
/**
* @return Expr[]
*/
private function getBooleanAndConditions(BooleanAnd $booleanAnd): array
{
$ifs = [];
while ($booleanAnd instanceof BinaryOp) {
$ifs[] = $booleanAnd->right;
$booleanAnd = $booleanAnd->left;
if (! $booleanAnd instanceof BooleanAnd) {
$ifs[] = $booleanAnd;
break;
}
}
krsort($ifs);
return $ifs;
}
private function isIfStmtExprUsedInNextReturn(If_ $if, Return_ $return): bool
{
if (! $return->expr instanceof Expr) {
@ -239,7 +219,12 @@ CODE_SAMPLE
? [new Continue_()]
: [$return];
foreach ($conditions as $condition) {
$getNextReturnExpr = $this->getNextReturnExpr($if);
if ($getNextReturnExpr instanceof Return_) {
$return->expr = $getNextReturnExpr->expr;
}
foreach ($conditions as $key => $condition) {
$invertedCondition = $this->conditionInverter->createInvertedCondition($condition);
$if = new If_($invertedCondition);
$if->stmts = $stmt;
@ -259,6 +244,18 @@ CODE_SAMPLE
return $nextNode;
}
private function getNextReturnExpr(If_ $if): ?Return_
{
$hasClosureParent = (bool) $this->betterNodeFinder->findParentType($if, Closure::class);
if ($hasClosureParent) {
return null;
}
return $this->betterNodeFinder->findFirstNext($if, function (Node $node): bool {
return $node instanceof Return_ && $node->expr instanceof Expr;
});
}
private function isIfInLoop(If_ $if): bool
{
$parentLoop = $this->betterNodeFinder->findParentTypes($if, self::LOOP_TYPES);

View File

@ -0,0 +1,65 @@
<?php
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfToEarlyReturnRector\Fixture;
class ReturnParentNext extends rex_var
{
protected function getOutput()
{
if ($this->hasArg('widget') && $this->getArg('widget')) {
} else {
if ($value && $this->hasArg('output') && 'id' != $this->getArg('output')) {
$value = rex_getUrl($value);
}
}
return $value;
}
}
abstract class rex_var {
/**
* Returns the output.
*
* @return false|string
*/
abstract protected function getOutput();
}
?>
-----
<?php
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfToEarlyReturnRector\Fixture;
class ReturnParentNext extends rex_var
{
protected function getOutput()
{
if ($this->hasArg('widget') && $this->getArg('widget')) {
} else {
if (!$value) {
return $value;
}
if (!$this->hasArg('output')) {
return $value;
}
if ('id' == $this->getArg('output')) {
return $value;
}
$value = rex_getUrl($value);
return $value;
}
return $value;
}
}
abstract class rex_var {
/**
* Returns the output.
*
* @return false|string
*/
abstract protected function getOutput();
}
?>