diff --git a/config/set/solid/solid.yaml b/config/set/solid/solid.yaml index 57ef7d0bc31c..1dc9973d498d 100644 --- a/config/set/solid/solid.yaml +++ b/config/set/solid/solid.yaml @@ -5,3 +5,4 @@ services: Rector\SOLID\Rector\Class_\MakeUnusedClassesWithChildrenAbstractRector: null Rector\SOLID\Rector\Property\ChangeReadOnlyPropertyWithDefaultValueToConstantRector: null Rector\SOLID\Rector\ClassMethod\ChangeReadOnlyVariableWithDefaultValueToConstantRector: null + Rector\SOLID\Rector\Foreach_\ChangeNestedForeachIfsToEarlyContinueRector: null diff --git a/docs/AllRectorsOverview.md b/docs/AllRectorsOverview.md index 2250da7f3363..cf2992d5b6cb 100644 --- a/docs/AllRectorsOverview.md +++ b/docs/AllRectorsOverview.md @@ -1,4 +1,4 @@ -# All 456 Rectors Overview +# All 457 Rectors Overview - [Projects](#projects) - [General](#general) @@ -8034,6 +8034,40 @@ Change if/else value to early return
+### `ChangeNestedForeachIfsToEarlyContinueRector` + +- class: [`Rector\SOLID\Rector\Foreach_\ChangeNestedForeachIfsToEarlyContinueRector`](/../master/rules/solid/src/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector.php) +- [test fixtures](/../master/rules/solid/tests/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector/Fixture) + +Change nested ifs to foreach with continue + +```diff + class SomeClass + { + public function run() + { + $items = []; + + foreach ($values as $value) { +- if ($value === 5) { +- if ($value2 === 10) { +- $items[] = 'maybe'; +- } ++ if ($value !== 5) { ++ continue; + } ++ if ($value2 !== 10) { ++ continue; ++ } ++ ++ $items[] = 'maybe'; + } + } + } +``` + +
+ ### `ChangeNestedIfsToEarlyReturnRector` - class: [`Rector\SOLID\Rector\If_\ChangeNestedIfsToEarlyReturnRector`](/../master/rules/solid/src/Rector/If_/ChangeNestedIfsToEarlyReturnRector.php) diff --git a/rules/solid/src/NodeTransformer/ConditionInverter.php b/rules/solid/src/NodeTransformer/ConditionInverter.php new file mode 100644 index 000000000000..32064e3e2249 --- /dev/null +++ b/rules/solid/src/NodeTransformer/ConditionInverter.php @@ -0,0 +1,38 @@ +binaryOpManipulator = $binaryOpManipulator; + } + + public function createInvertedCondition(Expr $expr): Expr + { + // inverse condition + if ($expr instanceof BinaryOp) { + $inversedCondition = $this->binaryOpManipulator->invertCondition($expr); + if ($inversedCondition === null) { + return new BooleanNot($expr); + } + + return $inversedCondition; + } + + return new BooleanNot($expr); + } +} diff --git a/rules/solid/src/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector.php b/rules/solid/src/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector.php new file mode 100644 index 000000000000..315910e50d84 --- /dev/null +++ b/rules/solid/src/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector.php @@ -0,0 +1,155 @@ +ifManipulator = $ifManipulator; + $this->conditionInverter = $conditionInverter; + } + + public function getDefinition(): RectorDefinition + { + return new RectorDefinition('Change nested ifs to foreach with continue', [ + new CodeSample( + <<<'PHP' +class SomeClass +{ + public function run() + { + $items = []; + + foreach ($values as $value) { + if ($value === 5) { + if ($value2 === 10) { + $items[] = 'maybe'; + } + } + } + } +} +PHP +, + <<<'PHP' +class SomeClass +{ + public function run() + { + $items = []; + + foreach ($values as $value) { + if ($value !== 5) { + continue; + } + if ($value2 !== 10) { + continue; + } + + $items[] = 'maybe'; + } + } +} +PHP + + ), + ]); + } + + /** + * @return string[] + */ + public function getNodeTypes(): array + { + return [Foreach_::class]; + } + + /** + * @param Foreach_ $node + */ + public function refactor(Node $node): ?Node + { + $nestedIfsWithOnlyNonReturn = $this->ifManipulator->collectNestedIfsWithNonBreaking($node); + if ($nestedIfsWithOnlyNonReturn === []) { + return null; + } + + $this->processNestedIfsWithNonBreaking($node, $nestedIfsWithOnlyNonReturn); + + return $node; + } + + /** + * @param If_[] $nestedIfsWithOnlyReturn + */ + private function processNestedIfsWithNonBreaking(Foreach_ $foreach, array $nestedIfsWithOnlyReturn): void + { + // add nested if openly after this + $nestedIfsWithOnlyReturnCount = count($nestedIfsWithOnlyReturn); + + // clear + $foreach->stmts = []; + + foreach ($nestedIfsWithOnlyReturn as $key => $nestedIfWithOnlyReturn) { + // last item → the return node + if ($nestedIfsWithOnlyReturnCount === $key + 1) { + $finalReturn = clone $nestedIfWithOnlyReturn; + + $this->addInvertedIfStmtWithContinue($nestedIfWithOnlyReturn, $foreach); + + $foreach->stmts = array_merge($foreach->stmts, $finalReturn->stmts); + } else { + $this->addInvertedIfStmtWithContinue($nestedIfWithOnlyReturn, $foreach); + } + } + } + + private function addInvertedIfStmtWithContinue(If_ $nestedIfWithOnlyReturn, Foreach_ $foreach): void + { + $invertedCondition = $this->conditionInverter->createInvertedCondition($nestedIfWithOnlyReturn->cond); + + // special case + if ($invertedCondition instanceof BooleanNot && $invertedCondition->expr instanceof BooleanAnd) { + $booleanNotPartIf = new If_(new BooleanNot($invertedCondition->expr->left)); + $foreach->stmts[] = $booleanNotPartIf; + + $booleanNotPartIf = new If_(new BooleanNot($invertedCondition->expr->right)); + $foreach->stmts[] = $booleanNotPartIf; + + return; + } + + $nestedIfWithOnlyReturn->cond = $invertedCondition; + $nestedIfWithOnlyReturn->stmts = [new Continue_()]; + + $foreach->stmts[] = $nestedIfWithOnlyReturn; + } +} diff --git a/rules/solid/src/Rector/If_/ChangeNestedIfsToEarlyReturnRector.php b/rules/solid/src/Rector/If_/ChangeNestedIfsToEarlyReturnRector.php index e11c48b2c91b..801544683a6e 100644 --- a/rules/solid/src/Rector/If_/ChangeNestedIfsToEarlyReturnRector.php +++ b/rules/solid/src/Rector/If_/ChangeNestedIfsToEarlyReturnRector.php @@ -5,18 +5,16 @@ namespace Rector\SOLID\Rector\If_; use PhpParser\Node; -use PhpParser\Node\Expr; -use PhpParser\Node\Expr\BinaryOp; use PhpParser\Node\Expr\BinaryOp\BooleanAnd; use PhpParser\Node\Expr\BooleanNot; use PhpParser\Node\Stmt\If_; use PhpParser\Node\Stmt\Return_; -use Rector\Core\PhpParser\Node\Manipulator\BinaryOpManipulator; use Rector\Core\PhpParser\Node\Manipulator\IfManipulator; use Rector\Core\Rector\AbstractRector; use Rector\Core\RectorDefinition\CodeSample; use Rector\Core\RectorDefinition\RectorDefinition; use Rector\NodeTypeResolver\Node\AttributeKey; +use Rector\SOLID\NodeTransformer\ConditionInverter; /** * @see \Rector\SOLID\Tests\Rector\If_\ChangeNestedIfsToEarlyReturnRector\ChangeNestedIfsToEarlyReturnRectorTest @@ -29,14 +27,14 @@ final class ChangeNestedIfsToEarlyReturnRector extends AbstractRector private $ifManipulator; /** - * @var BinaryOpManipulator + * @var ConditionInverter */ - private $binaryOpManipulator; + private $conditionInverter; - public function __construct(IfManipulator $ifManipulator, BinaryOpManipulator $binaryOpManipulator) + public function __construct(IfManipulator $ifManipulator, ConditionInverter $conditionInverter) { $this->ifManipulator = $ifManipulator; - $this->binaryOpManipulator = $binaryOpManipulator; + $this->conditionInverter = $conditionInverter; } public function getDefinition(): RectorDefinition @@ -105,7 +103,7 @@ public function refactor(Node $node): ?Node return null; } - $this->processNestedIfsWIthOnlyReturn($node, $nestedIfsWithOnlyReturn, $nextNode); + $this->processNestedIfsWithOnlyReturn($node, $nestedIfsWithOnlyReturn, $nextNode); $this->removeNode($node); return null; @@ -114,7 +112,7 @@ public function refactor(Node $node): ?Node /** * @param If_[] $nestedIfsWithOnlyReturn */ - private function processNestedIfsWIthOnlyReturn(If_ $if, array $nestedIfsWithOnlyReturn, Return_ $nextReturn): void + private function processNestedIfsWithOnlyReturn(If_ $if, array $nestedIfsWithOnlyReturn, Return_ $nextReturn): void { // add nested if openly after this $nestedIfsWithOnlyReturnCount = count($nestedIfsWithOnlyReturn); @@ -134,7 +132,7 @@ private function addStandaloneIfsWithReturn(If_ $nestedIfWithOnlyReturn, If_ $if { $return = clone $return; - $invertedCondition = $this->createInvertedCondition($nestedIfWithOnlyReturn->cond); + $invertedCondition = $this->conditionInverter->createInvertedCondition($nestedIfWithOnlyReturn->cond); // special case if ($invertedCondition instanceof BooleanNot && $invertedCondition->expr instanceof BooleanAnd) { @@ -152,19 +150,4 @@ private function addStandaloneIfsWithReturn(If_ $nestedIfWithOnlyReturn, If_ $if $this->addNodeAfterNode($nestedIfWithOnlyReturn, $if); } - - private function createInvertedCondition(Expr $expr): Expr - { - // inverse condition - if ($expr instanceof BinaryOp) { - $inversedCondition = $this->binaryOpManipulator->invertCondition($expr); - if ($inversedCondition === null) { - return new BooleanNot($expr); - } - - return $inversedCondition; - } - - return new BooleanNot($expr); - } } diff --git a/rules/solid/tests/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector/ChangeNestedForeachIfsToEarlyContinueRectorTest.php b/rules/solid/tests/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector/ChangeNestedForeachIfsToEarlyContinueRectorTest.php new file mode 100644 index 000000000000..b836fb33368c --- /dev/null +++ b/rules/solid/tests/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector/ChangeNestedForeachIfsToEarlyContinueRectorTest.php @@ -0,0 +1,30 @@ +doTestFile($file); + } + + public function provideData(): Iterator + { + return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + protected function getRectorClass(): string + { + return ChangeNestedForeachIfsToEarlyContinueRector::class; + } +} diff --git a/rules/solid/tests/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector/Fixture/fixture.php.inc b/rules/solid/tests/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector/Fixture/fixture.php.inc new file mode 100644 index 000000000000..3c1dfa841dcb --- /dev/null +++ b/rules/solid/tests/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector/Fixture/fixture.php.inc @@ -0,0 +1,45 @@ + +----- + diff --git a/rules/solid/tests/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector/Fixture/multi_exprs.php.inc b/rules/solid/tests/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector/Fixture/multi_exprs.php.inc new file mode 100644 index 000000000000..746b9bebfb7a --- /dev/null +++ b/rules/solid/tests/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector/Fixture/multi_exprs.php.inc @@ -0,0 +1,49 @@ + +----- + diff --git a/rules/solid/tests/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector/Fixture/skip_text.php.inc b/rules/solid/tests/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector/Fixture/skip_text.php.inc new file mode 100644 index 000000000000..08e44096ab8f --- /dev/null +++ b/rules/solid/tests/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector/Fixture/skip_text.php.inc @@ -0,0 +1,20 @@ +betterStandardPrinter = $betterStandardPrinter; $this->constFetchManipulator = $constFetchManipulator; $this->stmtsManipulator = $stmtsManipulator; $this->nodeNameResolver = $nodeNameResolver; + $this->betterNodeFinder = $betterNodeFinder; } /** @@ -216,6 +226,43 @@ public function isIfOrIfElseWithFunctionCondition(If_ $if, string $functionName) return $this->nodeNameResolver->isName($if->cond, $functionName); } + /** + * @return If_[] + */ + public function collectNestedIfsWithNonBreaking(Foreach_ $foreach): array + { + if (count((array) $foreach->stmts) !== 1) { + return []; + } + + $onlyForeachStmt = $foreach->stmts[0]; + if (! $onlyForeachStmt instanceof If_) { + return []; + } + + $ifs = []; + + $currentIf = $onlyForeachStmt; + while ($this->isIfWithOnlyStmtIf($currentIf)) { + $ifs[] = $currentIf; + + $currentIf = $currentIf->stmts[0]; + } + + if ($this->betterNodeFinder->findInstanceOf($currentIf->stmts, Return_::class) !== []) { + return []; + } + + if ($this->betterNodeFinder->findInstanceOf($currentIf->stmts, Exit_::class) !== []) { + return []; + } + + // last node is with the expression + $ifs[] = $currentIf; + + return $ifs; + } + private function matchComparedAndReturnedNode(NotIdentical $notIdentical, Return_ $returnNode): ?Expr { if ($this->betterStandardPrinter->areNodesEqual(