From 4acfa67e59ff15846dc14cf7bf578b7db46b99b3 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 5 Aug 2026 23:55:56 +0200 Subject: [PATCH] [EarlyReturn] Deprecate ChangeOrIfContinueToMultiContinueRector Splitting a single || condition into multiple continue ifs makes code longer and harder to read in most cases. Removed from early-return set. --- config/set/early-return.php | 2 - ...eOrIfContinueToMultiContinueRectorTest.php | 28 ------ .../Fixture/fixture.php.inc | 42 --------- .../Fixture/has_comment.php.inc | 44 --------- .../Fixture/many_and.php.inc | 45 --------- .../Fixture/negated.php.inc | 45 --------- .../Fixture/not_identical.php.inc | 42 --------- .../Fixture/skip_and.php.inc | 20 ---- .../Fixture/skip_multi_stmts.php.inc | 21 ----- .../Fixture/skip_not_continue.php.inc | 20 ---- .../Fixture/split_or_identical.php.inc | 42 --------- .../config/configured_rule.php | 9 -- ...hangeOrIfContinueToMultiContinueRector.php | 91 ++----------------- 13 files changed, 9 insertions(+), 442 deletions(-) delete mode 100644 rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/ChangeOrIfContinueToMultiContinueRectorTest.php delete mode 100644 rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/fixture.php.inc delete mode 100644 rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/has_comment.php.inc delete mode 100644 rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/many_and.php.inc delete mode 100644 rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/negated.php.inc delete mode 100644 rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/not_identical.php.inc delete mode 100644 rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/skip_and.php.inc delete mode 100644 rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/skip_multi_stmts.php.inc delete mode 100644 rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/skip_not_continue.php.inc delete mode 100644 rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/split_or_identical.php.inc delete mode 100644 rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/config/configured_rule.php diff --git a/config/set/early-return.php b/config/set/early-return.php index 582b6f3c69d..1215ce33c67 100644 --- a/config/set/early-return.php +++ b/config/set/early-return.php @@ -6,7 +6,6 @@ use Rector\EarlyReturn\Rector\Foreach_\ChangeNestedForeachIfsToEarlyContinueRector; use Rector\EarlyReturn\Rector\If_\ChangeIfElseValueAssignToEarlyReturnRector; use Rector\EarlyReturn\Rector\If_\ChangeNestedIfsToEarlyReturnRector; -use Rector\EarlyReturn\Rector\If_\ChangeOrIfContinueToMultiContinueRector; use Rector\EarlyReturn\Rector\If_\RemoveAlwaysElseRector; use Rector\EarlyReturn\Rector\Return_\PreparedValueToEarlyReturnRector; use Rector\EarlyReturn\Rector\Return_\ReturnBinaryOrToEarlyReturnRector; @@ -18,7 +17,6 @@ ChangeIfElseValueAssignToEarlyReturnRector::class, ChangeNestedIfsToEarlyReturnRector::class, RemoveAlwaysElseRector::class, - ChangeOrIfContinueToMultiContinueRector::class, PreparedValueToEarlyReturnRector::class, ReturnBinaryOrToEarlyReturnRector::class, ReturnEarlyIfVariableRector::class, diff --git a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/ChangeOrIfContinueToMultiContinueRectorTest.php b/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/ChangeOrIfContinueToMultiContinueRectorTest.php deleted file mode 100644 index ec052f2aad2..00000000000 --- a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/ChangeOrIfContinueToMultiContinueRectorTest.php +++ /dev/null @@ -1,28 +0,0 @@ -doTestFile($filePath); - } - - public static function provideData(): Iterator - { - return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); - } - - public function provideConfigFilePath(): string - { - return __DIR__ . '/config/configured_rule.php'; - } -} diff --git a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/fixture.php.inc b/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/fixture.php.inc deleted file mode 100644 index 46001e4c2da..00000000000 --- a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/fixture.php.inc +++ /dev/null @@ -1,42 +0,0 @@ -hasWheels() || $car->hasFuel()) { - continue; - } - $car->setWheel($newCar->wheel); - $car->setFuel($newCar->fuel); - } - } -} - -?> ------ -hasWheels()) { - continue; - } - if ($car->hasFuel()) { - continue; - } - $car->setWheel($newCar->wheel); - $car->setFuel($newCar->fuel); - } - } -} - -?> diff --git a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/has_comment.php.inc b/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/has_comment.php.inc deleted file mode 100644 index e2f560a1a7b..00000000000 --- a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/has_comment.php.inc +++ /dev/null @@ -1,44 +0,0 @@ -hasWheels() || $car->hasFuel()) { - continue; - } - $car->setWheel($newCar->wheel); - $car->setFuel($newCar->fuel); - } - } -} - -?> ------ -hasWheels()) { - continue; - } - if ($car->hasFuel()) { - continue; - } - $car->setWheel($newCar->wheel); - $car->setFuel($newCar->fuel); - } - } -} - -?> diff --git a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/many_and.php.inc b/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/many_and.php.inc deleted file mode 100644 index 49401d15aef..00000000000 --- a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/many_and.php.inc +++ /dev/null @@ -1,45 +0,0 @@ -hasWheels() || $car->hasFuel() || $car->isRunnable()) { - continue; - } - $car->setWheel($newCar->wheel); - $car->setFuel($newCar->fuel); - } - } -} - -?> ------ -hasWheels()) { - continue; - } - if ($car->hasFuel()) { - continue; - } - if ($car->isRunnable()) { - continue; - } - $car->setWheel($newCar->wheel); - $car->setFuel($newCar->fuel); - } - } -} - -?> diff --git a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/negated.php.inc b/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/negated.php.inc deleted file mode 100644 index 46e3a760fa7..00000000000 --- a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/negated.php.inc +++ /dev/null @@ -1,45 +0,0 @@ -hasWheels() || ! $car->hasFuel() || ! $car->isRunnable()) { - continue; - } - $car->setWheel($newCar->wheel); - $car->setFuel($newCar->fuel); - } - } -} - -?> ------ -hasWheels()) { - continue; - } - if (! $car->hasFuel()) { - continue; - } - if (! $car->isRunnable()) { - continue; - } - $car->setWheel($newCar->wheel); - $car->setFuel($newCar->fuel); - } - } -} - -?> diff --git a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/not_identical.php.inc b/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/not_identical.php.inc deleted file mode 100644 index cd31b1b95d5..00000000000 --- a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/not_identical.php.inc +++ /dev/null @@ -1,42 +0,0 @@ -getWheel() !== 4 || $car->getFuel() !== 'full') { - continue; - } - $car->setWheel($newCar->wheel); - $car->setFuel($newCar->fuel); - } - } -} - -?> ------ -getWheel() !== 4) { - continue; - } - if ($car->getFuel() !== 'full') { - continue; - } - $car->setWheel($newCar->wheel); - $car->setFuel($newCar->fuel); - } - } -} - -?> diff --git a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/skip_and.php.inc b/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/skip_and.php.inc deleted file mode 100644 index 7593c4eafcf..00000000000 --- a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/skip_and.php.inc +++ /dev/null @@ -1,20 +0,0 @@ -hasWheels() && $car->hasFuel()) { - continue; - } - - $car->setWheel($newCar->wheel); - $car->setFuel($newCar->fuel); - } - } -} - -?> diff --git a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/skip_multi_stmts.php.inc b/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/skip_multi_stmts.php.inc deleted file mode 100644 index 2b02f83459b..00000000000 --- a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/skip_multi_stmts.php.inc +++ /dev/null @@ -1,21 +0,0 @@ -hasWheels() || $car->hasFuel()) { - executeSideEffect(); - continue; - } - - $car->setWheel($newCar->wheel); - $car->setFuel($newCar->fuel); - } - } -} - -?> diff --git a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/skip_not_continue.php.inc b/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/skip_not_continue.php.inc deleted file mode 100644 index 2f9b3b0b5c4..00000000000 --- a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/skip_not_continue.php.inc +++ /dev/null @@ -1,20 +0,0 @@ -hasWheels() || $car->hasFuel()) { - break; - } - - $car->setWheel($newCar->wheel); - $car->setFuel($newCar->fuel); - } - } -} - -?> diff --git a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/split_or_identical.php.inc b/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/split_or_identical.php.inc deleted file mode 100644 index 30f4e9d0a47..00000000000 --- a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/Fixture/split_or_identical.php.inc +++ /dev/null @@ -1,42 +0,0 @@ -getWheel() === 4 || $car->getFuel() === 'full') { - continue; - } - $car->setWheel($newCar->wheel); - $car->setFuel($newCar->fuel); - } - } -} - -?> ------ -getWheel() === 4) { - continue; - } - if ($car->getFuel() === 'full') { - continue; - } - $car->setWheel($newCar->wheel); - $car->setFuel($newCar->fuel); - } - } -} - -?> diff --git a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/config/configured_rule.php b/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/config/configured_rule.php deleted file mode 100644 index dc88fbf53fc..00000000000 --- a/rules-tests/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector/config/configured_rule.php +++ /dev/null @@ -1,9 +0,0 @@ -withRules([ChangeOrIfContinueToMultiContinueRector::class]); diff --git a/rules/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector.php b/rules/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector.php index 300d8bde87b..888943e85bc 100644 --- a/rules/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector.php +++ b/rules/EarlyReturn/Rector/If_/ChangeOrIfContinueToMultiContinueRector.php @@ -5,25 +5,18 @@ namespace Rector\EarlyReturn\Rector\If_; use PhpParser\Node; -use PhpParser\Node\Expr; -use PhpParser\Node\Expr\BinaryOp\BooleanOr; -use PhpParser\Node\Stmt\Continue_; use PhpParser\Node\Stmt\If_; -use Rector\NodeManipulator\IfManipulator; +use Rector\Configuration\Deprecation\Contract\DeprecatedInterface; +use Rector\Exception\ShouldNotHappenException; use Rector\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; /** - * @see \Rector\Tests\EarlyReturn\Rector\If_\ChangeOrIfContinueToMultiContinueRector\ChangeOrIfContinueToMultiContinueRectorTest + * @deprecated This rule is deprecated, as splitting a single condition into multiple ifs makes the code longer and harder to read in most cases. */ -final class ChangeOrIfContinueToMultiContinueRector extends AbstractRector +final class ChangeOrIfContinueToMultiContinueRector extends AbstractRector implements DeprecatedInterface { - public function __construct( - private readonly IfManipulator $ifManipulator - ) { - } - public function getRuleDefinition(): RuleDefinition { return new RuleDefinition('Change `if a || b` to early return', [ @@ -79,78 +72,12 @@ public function getNodeTypes(): array /** * @param If_ $node - * @return null|If_[] - */ - public function refactor(Node $node): ?array - { - if (! $this->ifManipulator->isIfWithOnly($node, Continue_::class)) { - return null; - } - - if (! $node->cond instanceof BooleanOr) { - return null; - } - - return $this->processMultiIfContinue($node); - } - - /** - * @return null|If_[] - */ - private function processMultiIfContinue(If_ $if): ?array - { - $node = clone $if; - /** @var Continue_ $continue */ - $continue = $if->stmts[0]; - $ifs = $this->createMultipleIfs($if->cond, $continue, []); - - // ensure ifs not removed by other rules - if ($ifs === []) { - return null; - } - - $this->mirrorComments($ifs[0], $node); - return $ifs; - } - - /** - * @param If_[] $ifs - * @return If_[] - */ - private function createMultipleIfs(Expr $expr, Continue_ $continue, array $ifs): array - { - while ($expr instanceof BooleanOr) { - $ifs = [...$ifs, ...$this->collectLeftBooleanOrToIfs($expr, $continue, $ifs)]; - $ifs[] = new If_($expr->right, [ - 'stmts' => [$continue], - ]); - - $expr = $expr->right; - } - - $lastContinueIf = new If_($expr, [ - 'stmts' => [$continue], - ]); - - // the + is on purpose here, to keep only single continue as last - return $ifs + [$lastContinueIf]; - } - - /** - * @param If_[] $ifs - * @return If_[] */ - private function collectLeftBooleanOrToIfs(BooleanOr $booleanOr, Continue_ $continue, array $ifs): array + public function refactor(Node $node): ?Node { - $left = $booleanOr->left; - if (! $left instanceof BooleanOr) { - $if = new If_($left, [ - 'stmts' => [$continue], - ]); - - return [$if]; - } - - return $this->createMultipleIfs($left, $continue, $ifs); + throw new ShouldNotHappenException(sprintf( + '"%s" rule is deprecated, as splitting a single condition into multiple ifs makes the code longer and harder to read', + self::class + )); } }