Merge pull request #2907 from rectorphp/early-continue

[SOLID] Add ChangeNestedForeachIfsToEarlyContinueRector
This commit is contained in:
Tomas Votruba 2020-02-22 16:09:15 +01:00 committed by GitHub
commit afb9554414
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 429 additions and 27 deletions

View File

@ -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

View File

@ -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
<br>
### `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';
}
}
}
```
<br>
### `ChangeNestedIfsToEarlyReturnRector`
- class: [`Rector\SOLID\Rector\If_\ChangeNestedIfsToEarlyReturnRector`](/../master/rules/solid/src/Rector/If_/ChangeNestedIfsToEarlyReturnRector.php)

View File

@ -0,0 +1,38 @@
<?php
declare(strict_types=1);
namespace Rector\SOLID\NodeTransformer;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\BinaryOp;
use PhpParser\Node\Expr\BooleanNot;
use Rector\Core\PhpParser\Node\Manipulator\BinaryOpManipulator;
final class ConditionInverter
{
/**
* @var BinaryOpManipulator
*/
private $binaryOpManipulator;
public function __construct(BinaryOpManipulator $binaryOpManipulator)
{
$this->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);
}
}

View File

@ -0,0 +1,155 @@
<?php
declare(strict_types=1);
namespace Rector\SOLID\Rector\Foreach_;
use PhpParser\Node;
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
use PhpParser\Node\Expr\BooleanNot;
use PhpParser\Node\Stmt\Continue_;
use PhpParser\Node\Stmt\Foreach_;
use PhpParser\Node\Stmt\If_;
use Rector\Core\PhpParser\Node\Manipulator\IfManipulator;
use Rector\Core\Rector\AbstractRector;
use Rector\Core\RectorDefinition\CodeSample;
use Rector\Core\RectorDefinition\RectorDefinition;
use Rector\SOLID\NodeTransformer\ConditionInverter;
/**
* @see \Rector\SOLID\Tests\Rector\Foreach_\ChangeNestedForeachIfsToEarlyContinueRector\ChangeNestedForeachIfsToEarlyContinueRectorTest
*/
final class ChangeNestedForeachIfsToEarlyContinueRector extends AbstractRector
{
/**
* @var IfManipulator
*/
private $ifManipulator;
/**
* @var ConditionInverter
*/
private $conditionInverter;
public function __construct(IfManipulator $ifManipulator, ConditionInverter $conditionInverter)
{
$this->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;
}
}

View File

@ -5,18 +5,16 @@ declare(strict_types=1);
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 @@ PHP
return null;
}
$this->processNestedIfsWIthOnlyReturn($node, $nestedIfsWithOnlyReturn, $nextNode);
$this->processNestedIfsWithOnlyReturn($node, $nestedIfsWithOnlyReturn, $nextNode);
$this->removeNode($node);
return null;
@ -114,7 +112,7 @@ PHP
/**
* @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 @@ PHP
{
$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 @@ PHP
$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);
}
}

View File

@ -0,0 +1,30 @@
<?php
declare(strict_types=1);
namespace Rector\SOLID\Tests\Rector\Foreach_\ChangeNestedForeachIfsToEarlyContinueRector;
use Iterator;
use Rector\Core\Testing\PHPUnit\AbstractRectorTestCase;
use Rector\SOLID\Rector\Foreach_\ChangeNestedForeachIfsToEarlyContinueRector;
final class ChangeNestedForeachIfsToEarlyContinueRectorTest extends AbstractRectorTestCase
{
/**
* @dataProvider provideData()
*/
public function test(string $file): void
{
$this->doTestFile($file);
}
public function provideData(): Iterator
{
return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture');
}
protected function getRectorClass(): string
{
return ChangeNestedForeachIfsToEarlyContinueRector::class;
}
}

View File

@ -0,0 +1,45 @@
<?php
namespace Rector\SOLID\Tests\Rector\Foreach_\ChangeNestedForeachIfsToEarlyContinueRector\Fixture;
class SomeClass
{
public function run()
{
$items = [];
foreach ($values as $value) {
if ($value === 5) {
if ($value2 === 10) {
$items[] = 'maybe';
}
}
}
}
}
?>
-----
<?php
namespace Rector\SOLID\Tests\Rector\Foreach_\ChangeNestedForeachIfsToEarlyContinueRector\Fixture;
class SomeClass
{
public function run()
{
$items = [];
foreach ($values as $value) {
if ($value !== 5) {
continue;
}
if ($value2 !== 10) {
continue;
}
$items[] = 'maybe';
}
}
}
?>

View File

@ -0,0 +1,49 @@
<?php
namespace Rector\SOLID\Tests\Rector\Foreach_\ChangeNestedForeachIfsToEarlyContinueRector\Fixture;
class MultiExprs
{
public function run()
{
$items = [];
foreach ($values as $value) {
if ($value === 5) {
if ($value2 === 10) {
$items[] = 'maybe';
$items[] = 'no';
$items[] = 'yes';
}
}
}
}
}
?>
-----
<?php
namespace Rector\SOLID\Tests\Rector\Foreach_\ChangeNestedForeachIfsToEarlyContinueRector\Fixture;
class MultiExprs
{
public function run()
{
$items = [];
foreach ($values as $value) {
if ($value !== 5) {
continue;
}
if ($value2 !== 10) {
continue;
}
$items[] = 'maybe';
$items[] = 'no';
$items[] = 'yes';
}
}
}
?>

View File

@ -0,0 +1,20 @@
<?php
namespace Rector\SOLID\Tests\Rector\Foreach_\ChangeNestedForeachIfsToEarlyContinueRector\Fixture;
class SkipText
{
public function run()
{
$items = [];
foreach ($values as $value) {
if ($value === 5) {
if ($value2 === 10) {
$items[] = 'maybe';
return;
}
}
}
}
}

View File

@ -9,10 +9,13 @@ use PhpParser\Node\Expr;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\BinaryOp\Identical;
use PhpParser\Node\Expr\BinaryOp\NotIdentical;
use PhpParser\Node\Expr\Exit_;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Stmt\Foreach_;
use PhpParser\Node\Stmt\If_;
use PhpParser\Node\Stmt\Return_;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\Core\PhpParser\Printer\BetterStandardPrinter;
use Rector\NodeNameResolver\NodeNameResolver;
@ -38,16 +41,23 @@ final class IfManipulator
*/
private $nodeNameResolver;
/**
* @var BetterNodeFinder
*/
private $betterNodeFinder;
public function __construct(
BetterStandardPrinter $betterStandardPrinter,
ConstFetchManipulator $constFetchManipulator,
StmtsManipulator $stmtsManipulator,
NodeNameResolver $nodeNameResolver
NodeNameResolver $nodeNameResolver,
BetterNodeFinder $betterNodeFinder
) {
$this->betterStandardPrinter = $betterStandardPrinter;
$this->constFetchManipulator = $constFetchManipulator;
$this->stmtsManipulator = $stmtsManipulator;
$this->nodeNameResolver = $nodeNameResolver;
$this->betterNodeFinder = $betterNodeFinder;
}
/**
@ -216,6 +226,43 @@ final class IfManipulator
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(