mirror_php/rector
1
0
Fork 0
mirror of https://github.com/rectorphp/rector.git synced 2025-04-22 00:12:29 +02:00
Code Issues Projects Releases Wiki Activity

[SOLID] Add ChangeNestedIfsToEarlyReturnRector

Browse Source
This commit is contained in:
TomasVotruba 2020-01-05 13:10:01 +01:00
parent badbdc5f7b
commit 6cf187d37a
11 changed files with 397 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
config/set/solid/early-return.yaml
View File

@ -1,2 +1,3 @@
services:
Rector\SOLID\Rector\If_\RemoveAlwaysElseRector: null
Rector\SOLID\Rector\If_\ChangeNestedIfsToEarlyReturnRector: ~

48
packages/CodeQuality/src/Rector/BinaryOp/SimplifyDeMorganBinaryRector.php
View File

@ -5,12 +5,10 @@ declare(strict_types=1);
namespace Rector\CodeQuality\Rector\BinaryOp;
use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\BinaryOp;
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
use PhpParser\Node\Expr\BinaryOp\BooleanOr;
use PhpParser\Node\Expr\BooleanNot;
use Rector\PhpParser\Node\AssignAndBinaryMap;
use Rector\PhpParser\Node\Manipulator\BinaryOpManipulator;
use Rector\Rector\AbstractRector;
use Rector\RectorDefinition\CodeSample;
use Rector\RectorDefinition\RectorDefinition;
@ -23,13 +21,13 @@ use Rector\RectorDefinition\RectorDefinition;
final class SimplifyDeMorganBinaryRector extends AbstractRector
{
/**
* @var AssignAndBinaryMap
* @var BinaryOpManipulator
*/
private $assignAndBinaryMap;
private $binaryOpManipulator;
public function __construct(AssignAndBinaryMap $assignAndBinaryMap)
public function __construct(BinaryOpManipulator $binaryOpManipulator)
{
$this->assignAndBinaryMap = $assignAndBinaryMap;
$this->binaryOpManipulator = $binaryOpManipulator;
}
public function getDefinition(): RectorDefinition
@ -77,40 +75,6 @@ PHP
return null;
}
$inversedNode = $this->assignAndBinaryMap->getInversed($node->expr);
if ($inversedNode === null) {
if ($node->expr instanceof BooleanOr) {
$inversedNode = BooleanAnd::class;
} else {
return null;
}
}
// no nesting
if ($node->expr->left instanceof BooleanOr) {
return null;
}
if ($node->expr->right instanceof BooleanOr) {
return null;
}
return new $inversedNode($this->inverseNode($node->expr->left), $this->inverseNode($node->expr->right));
}
private function inverseNode(Expr $expr): Node
{
if ($expr instanceof BinaryOp) {
$inversedBinaryOp = $this->assignAndBinaryMap->getInversed($expr);
if ($inversedBinaryOp) {
return new $inversedBinaryOp($expr->left, $expr->right);
}
}
if ($expr instanceof BooleanNot) {
return $expr->expr;
}
return new BooleanNot($expr);
return $this->binaryOpManipulator->inverseBinaryOp($node->expr);
}
}

8
packages/CodeQuality/src/Rector/FuncCall/ChangeArrayPushToArrayAssignRector.php
View File

@ -8,6 +8,7 @@ use PhpParser\Node;
use PhpParser\Node\Expr\ArrayDimFetch;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Stmt\Expression;
use Rector\Rector\AbstractRector;
use Rector\RectorDefinition\CodeSample;
use Rector\RectorDefinition\RectorDefinition;
@ -71,6 +72,13 @@ PHP
$position = 1;
while (isset($node->args[$position])) {
$assign = new Assign($arrayDimFetch, $node->args[$position]->value);
// keep comments of first line
if ($position === 1) {
$assign = new Expression($assign);
$assign->setAttribute('comments', $node->getComments());
}
$this->addNodeAfterNode($assign, $node);
++$position;

2
packages/CodeQuality/tests/Rector/FuncCall/ChangeArrayPushToArrayAssignRector/Fixture/push_multiple.php.inc
View File

@ -7,6 +7,7 @@ class PushMultiple
public function run()
{
$items = [];
// keep comment
array_push($items, $item, $item2, $item3);
}
}
@ -22,6 +23,7 @@ class PushMultiple
public function run()
{
$items = [];
// keep comment
$items[] = $item;
$items[] = $item2;
$items[] = $item3;

128
packages/SOLID/src/Rector/If_/ChangeNestedIfsToEarlyReturnRector.php Normal file
View File

@ -0,0 +1,128 @@
<?php
declare(strict_types=1);
namespace Rector\SOLID\Rector\If_;
use PhpParser\Node;
use PhpParser\Node\Stmt\If_;
use PhpParser\Node\Stmt\Return_;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\PhpParser\Node\Manipulator\BinaryOpManipulator;
use Rector\PhpParser\Node\Manipulator\IfManipulator;
use Rector\Rector\AbstractRector;
use Rector\RectorDefinition\CodeSample;
use Rector\RectorDefinition\RectorDefinition;
/**
* @see \Rector\SOLID\Tests\Rector\If_\ChangeNestedIfsToEarlyReturnRector\ChangeNestedIfsToEarlyReturnRectorTest
*/
final class ChangeNestedIfsToEarlyReturnRector extends AbstractRector
{
/**
* @var IfManipulator
*/
private $ifManipulator;
/**
* @var BinaryOpManipulator
*/
private $binaryOpManipulator;
public function __construct(IfManipulator $ifManipulator, BinaryOpManipulator $binaryOpManipulator)
{
$this->ifManipulator = $ifManipulator;
$this->binaryOpManipulator = $binaryOpManipulator;
}
public function getDefinition(): RectorDefinition
{
return new RectorDefinition('Change nested ifs to early return', [
new CodeSample(
<<<'PHP'
class SomeClass
{
public function run()
{
if ($value === 5) {
if ($value2 === 10) {
return 'yes';
}
}
return 'no';
}
}
PHP
,
<<<'PHP'
class SomeClass
{
public function run()
{
if ($value !== 5) {
return 'no';
}
if ($value2 === 10) {
return 'yes';
}
return 'no';
}
}
PHP
),
]);
}
/**
* @return string[]
*/
public function getNodeTypes(): array
{
return [If_::class];
}
/**
* @param If_ $node
*/
public function refactor(Node $node): ?Node
{
// A. next node is return
$nextNode = $node->getAttribute(AttributeKey::NEXT_NODE);
if (! $nextNode instanceof Return_) {
return null;
}
$nestedIfsWithOnlyReturn = $this->ifManipulator->collectNestedIfsWithOnlyReturn($node);
if ($nestedIfsWithOnlyReturn === []) {
return null;
}
// add nested if openly after this
$nestedIfsWithOnlyReturnCount = count($nestedIfsWithOnlyReturn);
foreach ($nestedIfsWithOnlyReturn as $key => $nestedIfWithOnlyReturn) {
// last item → the return node
if ($nestedIfsWithOnlyReturnCount === $key + 1) {
$this->addNodeAfterNode($nestedIfWithOnlyReturn, $node);
} else {
$inversedCondition = $this->binaryOpManipulator->inverseCondition($nestedIfWithOnlyReturn->cond);
if ($inversedCondition === null) {
return null;
}
$nestedIfWithOnlyReturn->cond = $inversedCondition;
$nestedIfWithOnlyReturn->stmts = [clone $nextNode];
$this->addNodeAfterNode($nestedIfWithOnlyReturn, $node);
}
}
$this->removeNode($node);
return null;
}
}

30
packages/SOLID/tests/Rector/If_/ChangeNestedIfsToEarlyReturnRector/ChangeNestedIfsToEarlyReturnRectorTest.php Normal file
View File

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

40
packages/SOLID/tests/Rector/If_/ChangeNestedIfsToEarlyReturnRector/Fixture/fixture.php.inc Normal file
View File

@ -0,0 +1,40 @@
<?php
namespace Rector\SOLID\Tests\Rector\If_\ChangeNestedIfsToEarlyReturnRector\Fixture;
class SomeClass
{
public function run()
{
if ($value === 5) {
if ($value2 === 10) {
return 'yes';
}
}
return 'no';
}
}
?>
-----
<?php
namespace Rector\SOLID\Tests\Rector\If_\ChangeNestedIfsToEarlyReturnRector\Fixture;
class SomeClass
{
public function run()
{
if ($value !== 5) {
return 'no';
}
if ($value2 === 10) {
return 'yes';
}
return 'no';
}
}
?>

45
packages/SOLID/tests/Rector/If_/ChangeNestedIfsToEarlyReturnRector/Fixture/three_nesting.php.inc Normal file
View File

@ -0,0 +1,45 @@
<?php
namespace Rector\SOLID\Tests\Rector\If_\ChangeNestedIfsToEarlyReturnRector\Fixture;
class ThreeNesting
{
public function run($value)
{
if ($value === 5) {
if ($value === 15) {
if ($value === 30) {
return 'yes';
}
}
}
return 'no';
}
}
?>
-----
<?php
namespace Rector\SOLID\Tests\Rector\If_\ChangeNestedIfsToEarlyReturnRector\Fixture;
class ThreeNesting
{
public function run($value)
{
if ($value !== 5) {
return 'no';
}
if ($value !== 15) {
return 'no';
}
if ($value === 30) {
return 'yes';
}
return 'no';
}
}
?>

85
src/PhpParser/Node/Manipulator/BinaryOpManipulator.php
View File

@ -5,11 +5,25 @@ declare(strict_types=1);
namespace Rector\PhpParser\Node\Manipulator;
use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\BinaryOp;
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
use PhpParser\Node\Expr\BinaryOp\BooleanOr;
use Rector\Exception\ShouldNotHappenException;
use Rector\PhpParser\Node\AssignAndBinaryMap;
final class BinaryOpManipulator
{
/**
* @var AssignAndBinaryMap
*/
private $assignAndBinaryMap;
public function __construct(AssignAndBinaryMap $assignAndBinaryMap)
{
$this->assignAndBinaryMap = $assignAndBinaryMap;
}
/**
* Tries to match left or right parts (xor),
* returns null or match on first condition and then second condition. No matter what the origin order is.
@ -40,6 +54,47 @@ final class BinaryOpManipulator
return null;
}
public function inverseBinaryOp(BinaryOp $binaryOp): ?BinaryOp
{
// no nesting
if ($binaryOp->left instanceof BooleanOr) {
return null;
}
if ($binaryOp->right instanceof BooleanOr) {
return null;
}
$inversedNodeClass = $this->resolveInversedNodeClass($binaryOp);
if ($inversedNodeClass === null) {
return null;
}
$firstInversedNode = $this->inverseNode($binaryOp->left);
$secondInversedNode = $this->inverseNode($binaryOp->right);
return new $inversedNodeClass($firstInversedNode, $secondInversedNode);
}
public function inverseCondition(BinaryOp $binaryOp): ?BinaryOp
{
// no nesting
if ($binaryOp->left instanceof BooleanOr) {
return null;
}
if ($binaryOp->right instanceof BooleanOr) {
return null;
}
$inversedNodeClass = $this->resolveInversedNodeClass($binaryOp);
if ($inversedNodeClass === null) {
return null;
}
return new $inversedNodeClass($binaryOp->left, $binaryOp->right);
}
private function validateCondition($firstCondition): void
{
if (is_callable($firstCondition)) {
@ -66,4 +121,34 @@ final class BinaryOpManipulator
return is_a($node, $condition, true);
};
}
private function inverseNode(Expr $expr): Node
{
if ($expr instanceof BinaryOp) {
$inversedBinaryOp = $this->assignAndBinaryMap->getInversed($expr);
if ($inversedBinaryOp) {
return new $inversedBinaryOp($expr->left, $expr->right);
}
}
if ($expr instanceof Expr\BooleanNot) {
return $expr->expr;
}
return new Expr\BooleanNot($expr);
}
private function resolveInversedNodeClass(BinaryOp $binaryOp): ?string
{
$inversedNodeClass = $this->assignAndBinaryMap->getInversed($binaryOp);
if ($inversedNodeClass !== null) {
return $inversedNodeClass;
}
if ($binaryOp instanceof BooleanOr) {
return BooleanAnd::class;
}
return null;
}
}

51
src/PhpParser/Node/Manipulator/IfManipulator.php
View File

@ -123,6 +123,15 @@ final class IfManipulator
return null;
}
public function isIfWithOnlyStmtIf(If_ $if): bool
{
if (! $this->isIfWithoutElseAndElseIfs($if)) {
return false;
}
return $this->hasOnlyStmtOfType($if, If_::class);
}
public function isEarlyElse(If_ $if): bool
{
if (! $this->isAlwaysAllowedType((array) $if->stmts, self::ALLOWED_BREAKING_NODE_TYPES)) {
@ -138,6 +147,39 @@ final class IfManipulator
return $if->else !== null;
}
public function hasOnlyStmtOfType(If_ $if, string $desiredType): bool
{
if (count($if->stmts) !== 1) {
return false;
}
return is_a($if->stmts[0], $desiredType);
}
/**
* @return If_[]
*/
public function collectNestedIfsWithOnlyReturn(If_ $if): array
{
$ifs = [];
$currentIf = $if;
while ($this->isIfWithOnlyStmtIf($currentIf)) {
$ifs[] = $currentIf;
$currentIf = $currentIf->stmts[0];
}
if (! $this->hasOnlyStmtOfType($currentIf, Return_::class)) {
return [];
}
// last node is with the return value
$ifs[] = $currentIf;
return $ifs;
}
private function matchComparedAndReturnedNode(NotIdentical $notIdentical, Return_ $returnNode): ?Expr
{
if ($this->betterStandardPrinter->areNodesEqual($notIdentical->left, $returnNode->expr)) {
@ -205,4 +247,13 @@ final class IfManipulator
return false;
}
private function isIfWithoutElseAndElseIfs(If_ $if): bool
{
if ($if->else !== null) {
return false;
}
return ! (bool) $if->elseifs;
}
}

2
src/Rector/MethodCall/MethodCallToReturnRector.php
View File

@ -82,7 +82,7 @@ PHP
}
/**
* @param Node\Stmt\Expression $node
* @param Expression $node
*/
public function refactor(Node $node): ?Node
{