diff --git a/rules-tests/EarlyReturn/Rector/Return_/ReturnBinaryOrToEarlyReturnRector/Fixture/and_in_next.php.inc b/rules-tests/EarlyReturn/Rector/Return_/ReturnBinaryOrToEarlyReturnRector/Fixture/and_in_next.php.inc new file mode 100644 index 00000000000..71d884055e8 --- /dev/null +++ b/rules-tests/EarlyReturn/Rector/Return_/ReturnBinaryOrToEarlyReturnRector/Fixture/and_in_next.php.inc @@ -0,0 +1,33 @@ +something() || $this->somethingelse() && $this->anotherelse() || $this->last(); + } +} + +?> +----- +something()) { + return true; + } + if ($this->somethingelse() && $this->anotherelse()) { + return true; + } + return (bool) $this->last(); + } +} + +?> diff --git a/rules-tests/EarlyReturn/Rector/Return_/ReturnBinaryOrToEarlyReturnRector/Fixture/and_nested_deep.php.inc b/rules-tests/EarlyReturn/Rector/Return_/ReturnBinaryOrToEarlyReturnRector/Fixture/and_nested_deep.php.inc new file mode 100644 index 00000000000..a55827d018e --- /dev/null +++ b/rules-tests/EarlyReturn/Rector/Return_/ReturnBinaryOrToEarlyReturnRector/Fixture/and_nested_deep.php.inc @@ -0,0 +1,39 @@ +donation === null + || ($this->commission > 0 && $this->transaction === null) + || $this->somethingElse() + || $this->anotherThing(); + } +} + +?> +----- +donation === null) { + return true; + } + if ($this->commission > 0 && $this->transaction === null) { + return true; + } + if ($this->somethingElse()) { + return true; + } + return (bool) $this->anotherThing(); + } +} + +?> diff --git a/rules-tests/EarlyReturn/Rector/Return_/ReturnBinaryOrToEarlyReturnRector/Fixture/skip_and_in_next.php.inc b/rules-tests/EarlyReturn/Rector/Return_/ReturnBinaryOrToEarlyReturnRector/Fixture/skip_and_in_next.php.inc deleted file mode 100644 index 2c6dd2ce696..00000000000 --- a/rules-tests/EarlyReturn/Rector/Return_/ReturnBinaryOrToEarlyReturnRector/Fixture/skip_and_in_next.php.inc +++ /dev/null @@ -1,13 +0,0 @@ -something() || $this->somethingelse() && $this->anotherelse() || $this->last(); - } -} - -?> diff --git a/rules/EarlyReturn/Rector/Return_/ReturnBinaryOrToEarlyReturnRector.php b/rules/EarlyReturn/Rector/Return_/ReturnBinaryOrToEarlyReturnRector.php index bad0b22ac7b..0d761d8925b 100644 --- a/rules/EarlyReturn/Rector/Return_/ReturnBinaryOrToEarlyReturnRector.php +++ b/rules/EarlyReturn/Rector/Return_/ReturnBinaryOrToEarlyReturnRector.php @@ -6,7 +6,6 @@ use PhpParser\Node; use PhpParser\Node\Expr; -use PhpParser\Node\Expr\BinaryOp\BooleanAnd; use PhpParser\Node\Expr\BinaryOp\BooleanOr; use PhpParser\Node\Stmt\If_; use PhpParser\Node\Stmt\Return_; @@ -89,12 +88,15 @@ public function refactor(Node $node): ?Node $booleanOr = $stmt->expr; + // the right side becomes the final return, the left operands become early returns $left = $booleanOr->left; - $ifs = $this->createMultipleIfs($left, $stmt, []); + $leftOperands = $left instanceof BooleanOr ? $this->flattenBooleanOr($left) : [$left]; - // ensure ifs not removed by other rules - if ($ifs === []) { - continue; + $ifs = []; + foreach ($leftOperands as $leftOperand) { + $ifs[] = new If_($leftOperand, [ + 'stmts' => [new Return_($this->nodeFactory->createTrue())], + ]); } if (! $this->callAnalyzer->doesIfHasObjectCall($ifs)) { @@ -119,56 +121,17 @@ public function refactor(Node $node): ?Node } /** - * @param If_[] $ifs - * @return If_[] - */ - private function createMultipleIfs(Expr $expr, Return_ $return, array $ifs): array - { - while ($expr instanceof BooleanOr) { - $ifs = [...$ifs, ...$this->collectLeftBooleanOrToIfs($expr, $return, $ifs)]; - $ifs[] = new If_($expr->right, [ - 'stmts' => [new Return_($this->nodeFactory->createTrue())], - ]); - - $expr = $expr->right; - if ($expr instanceof BooleanAnd) { - return []; - } - - if (! $expr instanceof BooleanOr) { - continue; - } - - return []; - } - - $lastIf = new If_($expr, [ - 'stmts' => [new Return_($this->nodeFactory->createTrue())], - ]); - - // if empty, fallback to last if - if ($ifs === []) { - return [$lastIf]; - } - - return $ifs; - } - - /** - * @param If_[] $ifs - * @return If_[] + * Flatten a left-associative "||" chain into its operands, in source order. + * + * @return Expr[] */ - private function collectLeftBooleanOrToIfs(BooleanOr $booleanOr, Return_ $return, array $ifs): array + private function flattenBooleanOr(BooleanOr $booleanOr): array { $left = $booleanOr->left; - if (! $left instanceof BooleanOr) { - $returnTrueIf = new If_($left, [ - 'stmts' => [new Return_($this->nodeFactory->createTrue())], - ]); - return [$returnTrueIf]; - } + $operands = $left instanceof BooleanOr ? $this->flattenBooleanOr($left) : [$left]; + $operands[] = $booleanOr->right; - return $this->createMultipleIfs($left, $return, $ifs); + return $operands; } }