From a62bb98fe93ceab9e9a5de3cc80199457bc6a2ea Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sun, 28 Feb 2021 22:07:12 +0700 Subject: [PATCH] [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 --- .../src/NodeCollector/NodeRepository.php | 23 +++++++ .../If_/ChangeAndIfToEarlyReturnRector.php | 43 ++++++------ .../Fixture/return_parent_next.php.inc | 65 +++++++++++++++++++ 3 files changed, 108 insertions(+), 23 deletions(-) create mode 100644 rules/early-return/tests/Rector/If_/ChangeAndIfToEarlyReturnRector/Fixture/return_parent_next.php.inc diff --git a/packages/node-collector/src/NodeCollector/NodeRepository.php b/packages/node-collector/src/NodeCollector/NodeRepository.php index 5bd54eeeac3..c8c6ab4dfff 100644 --- a/packages/node-collector/src/NodeCollector/NodeRepository.php +++ b/packages/node-collector/src/NodeCollector/NodeRepository.php @@ -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); diff --git a/rules/early-return/src/Rector/If_/ChangeAndIfToEarlyReturnRector.php b/rules/early-return/src/Rector/If_/ChangeAndIfToEarlyReturnRector.php index 85c25074579..e052ede9ca3 100644 --- a/rules/early-return/src/Rector/If_/ChangeAndIfToEarlyReturnRector.php +++ b/rules/early-return/src/Rector/If_/ChangeAndIfToEarlyReturnRector.php @@ -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); diff --git a/rules/early-return/tests/Rector/If_/ChangeAndIfToEarlyReturnRector/Fixture/return_parent_next.php.inc b/rules/early-return/tests/Rector/If_/ChangeAndIfToEarlyReturnRector/Fixture/return_parent_next.php.inc new file mode 100644 index 00000000000..44921afd687 --- /dev/null +++ b/rules/early-return/tests/Rector/If_/ChangeAndIfToEarlyReturnRector/Fixture/return_parent_next.php.inc @@ -0,0 +1,65 @@ +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(); +} +?> +----- +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(); +} +?>