octoleo/rector
1
1
Fork 0
mirror of https://github.com/rectorphp/rector.git synced 2024-06-26 12:52:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

[EarlyReturn] Add ChangeAndIfContinueToMultiContinueRector (#5460)

Browse Source 
* [EarlyReturn] [WIP] Fixes #5230 Add ChangeAndIfContinueToContinueEarlyRector

* skip multi stmts

* skip not continue

* skip not and

* functional

* cs fix

* negated check

* not identical

* reduce repetitive method

* comment test

* comment fixture

* reduce complexity

* cs fix

* [ci-review] Rector Rectify

* using isIfWithOnly()

* phpstan

* use ConditionInverter service

Co-authored-by: kaizen-ci <info@kaizen-ci.org>
This commit is contained in:
Abdul Malik Ikhsan 2021-02-10 02:07:20 +07:00 committed by GitHub
parent 1a9544f5f4
commit 6b8789eeca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 542 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
rules/dead-code/src/Rector/FunctionLike/RemoveDuplicatedIfReturnRector.php
View File

@ -9,6 +9,7 @@ use PhpParser\Node\Expr\Variable;
use PhpParser\Node\FunctionLike;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\If_;
use PhpParser\Node\Stmt\Return_;
use PhpParser\NodeTraverser;
use Rector\Core\NodeManipulator\IfManipulator;
use Rector\Core\Rector\AbstractRector;
@ -122,7 +123,7 @@ CODE_SAMPLE
$modifiedVariableNames = [];
foreach ((array) $functionLike->getStmts() as $stmt) {
if (! $this->ifManipulator->isIfWithOnlyReturn($stmt)) {
if (! $this->ifManipulator->isIfWithOnly($stmt, Return_::class)) {
// variable modification
$modifiedVariableNames = array_merge(
$modifiedVariableNames,

2
rules/dead-code/src/Rector/If_/RemoveUnusedNonEmptyArrayBeforeForeachRector.php
View File

@ -104,7 +104,7 @@ CODE_SAMPLE
private function isUselessBeforeForeachCheck(If_ $if): bool
{
if (! $this->ifManipulator->isIfWithOnlyForeach($if)) {
if (! $this->ifManipulator->isIfWithOnly($if, Foreach_::class)) {
return false;
}

148
rules/early-return/src/Rector/If_/ChangeAndIfContinueToMultiContinueRector.php Normal file
View File

@ -0,0 +1,148 @@
<?php
declare(strict_types=1);
namespace Rector\EarlyReturn\Rector\If_;
use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
use PhpParser\Node\Stmt\Continue_;
use PhpParser\Node\Stmt\If_;
use Rector\Core\NodeManipulator\IfManipulator;
use Rector\Core\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
/**
* @see \Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfContinueToMultiContinueRector\ChangeAndIfContinueToMultiContinueRectorTest
*/
final class ChangeAndIfContinueToMultiContinueRector extends AbstractRector
{
/**
* @var IfManipulator
*/
private $ifManipulator;
public function __construct(IfManipulator $ifManipulator)
{
$this->ifManipulator = $ifManipulator;
}
public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Changes if && to early return', [
new CodeSample(
<<<'CODE_SAMPLE'
class SomeClass
{
public function canDrive(Car $newCar)
{
foreach ($cars as $car) {
if ($car->hasWheels() && $car->hasFuel()) {
continue;
}
$car->setWheel($newCar->wheel);
$car->setFuel($newCar->fuel);
}
}
}
CODE_SAMPLE
,
<<<'CODE_SAMPLE'
class SomeClass
{
public function canDrive(Car $newCar)
{
foreach ($cars as $car) {
if (! $car->hasWheels()) {
continue;
}
if (! $car->hasFuel()) {
continue;
}
$car->setWheel($newCar->wheel);
$car->setFuel($newCar->fuel);
}
}
}
CODE_SAMPLE
),
]);
}
/**
* @return string[]
*/
public function getNodeTypes(): array
{
return [If_::class];
}
/**
* @param If_ $node
*/
public function refactor(Node $node): ?Node
{
if (! $this->ifManipulator->isIfWithOnly($node, Continue_::class)) {
return null;
}
if (! $node->cond instanceof BooleanAnd) {
return null;
}
return $this->processMultiIfContinue($node);
}
private function processMultiIfContinue(If_ $if): If_
{
/** @var Continue_ $continue */
$continue = $if->stmts[0];
$ifs = $this->createMultipleIfs($if->cond, $continue, []);
$node = $if;
foreach ($ifs as $key => $if) {
if ($key === 0) {
$this->mirrorComments($if, $node);
}
$this->addNodeBeforeNode($if, $node);
}
$this->removeNode($node);
return $node;
}
/**
* @param If_[] $ifs
* @return If_[]
*/
private function createMultipleIfs(Expr $expr, Continue_ $continue, array $ifs): array
{
while ($expr instanceof BooleanAnd) {
$ifs = array_merge($ifs, $this->collectLeftBooleanAndToIfs($expr, $continue, $ifs));
$ifs[] = $this->ifManipulator->createIfNegation($expr->right, $continue);
$expr = $expr->right;
}
return $ifs + [$this->ifManipulator->createIfNegation($expr, $continue)];
}
/**
* @param If_[] $ifs
* @return If_[]
*/
private function collectLeftBooleanAndToIfs(BooleanAnd $booleanAnd, Continue_ $continue, array $ifs): array
{
$left = $booleanAnd->left;
if (! $left instanceof BooleanAnd) {
return [$this->ifManipulator->createIfNegation($left, $continue)];
}
return $this->createMultipleIfs($left, $continue, $ifs);
}
}

2
rules/early-return/src/Rector/If_/ChangeOrIfReturnToEarlyReturnRector.php
View File

@ -81,7 +81,7 @@ CODE_SAMPLE
*/
public function refactor(Node $node): ?Node
{
if (! $this->ifManipulator->isIfWithOnlyReturn($node)) {
if (! $this->ifManipulator->isIfWithOnly($node, Return_::class)) {
return null;
}

37
rules/early-return/src/Rector/Return_/ReturnBinaryAndToEarlyReturnRector.php
View File

@ -19,12 +19,23 @@ use Rector\Core\Rector\AbstractRector;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
use Rector\Core\NodeManipulator\IfManipulator;
/**
* @see \Rector\EarlyReturn\Tests\Rector\Return_\ReturnBinaryAndToEarlyReturnRector\ReturnBinaryAndToEarlyReturnRectorTest
*/
final class ReturnBinaryAndToEarlyReturnRector extends AbstractRector
{
/**
* @var IfManipulator
*/
private $ifManipulator;
public function __construct(IfManipulator $ifManipulator)
{
$this->ifManipulator = $ifManipulator;
}
public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Changes Single return of && && to early returns', [
@ -99,11 +110,11 @@ CODE_SAMPLE
{
while ($expr instanceof BooleanAnd) {
$ifNegations = array_merge($ifNegations, $this->collectLeftBooleanAndToIfs($expr, $return, $ifNegations));
$ifNegations[] = $this->createIfNegation($expr->right);
$ifNegations[] = $this->ifManipulator->createIfNegation($expr->right, new Return_($this->nodeFactory->createFalse()));
$expr = $expr->right;
}
return $ifNegations + [$this->createIfNegation($expr)];
return $ifNegations + [$this->ifManipulator->createIfNegation($expr, new Return_($this->nodeFactory->createFalse()))];
}
private function getLastReturnExpr(Expr $expr): Expr
@ -137,29 +148,9 @@ CODE_SAMPLE
{
$left = $booleanAnd->left;
if (! $left instanceof BooleanAnd) {
return [$this->createIfNegation($left)];
return [$this->ifManipulator->createIfNegation($left, new Return_($this->nodeFactory->createFalse()))];
}
return $this->createMultipleIfsNegation($left, $return, $ifNegations);
}
private function createIfNegation(Expr $expr): If_
{
if ($expr instanceof Identical) {
$expr = new NotIdentical($expr->left, $expr->right);
} elseif ($expr instanceof NotIdentical) {
$expr = new Identical($expr->left, $expr->right);
} elseif ($expr instanceof BooleanNot) {
$expr = $expr->expr;
} else {
$expr = new BooleanNot($expr);
}
return new If_(
$expr,
[
'stmts' => [new Return_($this->nodeFactory->createFalse())],
]
);
}
}

31
rules/early-return/tests/Rector/If_/ChangeAndIfContinueToMultiContinueRector/ChangeAndIfContinueToMultiContinueRectorTest.php Normal file
View File

@ -0,0 +1,31 @@
<?php
declare(strict_types=1);
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfContinueToMultiContinueRector;
use Iterator;
use Rector\EarlyReturn\Rector\If_\ChangeAndIfContinueToMultiContinueRector;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
use Symplify\SmartFileSystem\SmartFileInfo;
final class ChangeAndIfContinueToMultiContinueRectorTest extends AbstractRectorTestCase
{
/**
* @dataProvider provideData()
*/
public function test(SmartFileInfo $fileInfo): void
{
$this->doTestFileInfo($fileInfo);
}
public function provideData(): Iterator
{
return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture');
}
protected function getRectorClass(): string
{
return ChangeAndIfContinueToMultiContinueRector::class;
}
}

42
rules/early-return/tests/Rector/If_/ChangeAndIfContinueToMultiContinueRector/Fixture/fixture.php.inc Normal file
View File

@ -0,0 +1,42 @@
<?php
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfContinueToMultiContinueRector\Fixture;
class Fixture
{
public function canDrive(Car $newCar)
{
foreach ($cars as $car) {
if ($car->hasWheels() && $car->hasFuel()) {
continue;
}
$car->setWheel($newCar->wheel);
$car->setFuel($newCar->fuel);
}
}
}
?>
-----
<?php
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfContinueToMultiContinueRector\Fixture;
class Fixture
{
public function canDrive(Car $newCar)
{
foreach ($cars as $car) {
if (!$car->hasWheels()) {
continue;
}
if (!$car->hasFuel()) {
continue;
}
$car->setWheel($newCar->wheel);
$car->setFuel($newCar->fuel);
}
}
}
?>

44
rules/early-return/tests/Rector/If_/ChangeAndIfContinueToMultiContinueRector/Fixture/has_comment.php.inc Normal file
View File

@ -0,0 +1,44 @@
<?php
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfContinueToMultiContinueRector\Fixture;
class HasComment
{
public function canDrive(Car $newCar)
{
foreach ($cars as $car) {
// a comment
if ($car->hasWheels() && $car->hasFuel()) {
continue;
}
$car->setWheel($newCar->wheel);
$car->setFuel($newCar->fuel);
}
}
}
?>
-----
<?php
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfContinueToMultiContinueRector\Fixture;
class HasComment
{
public function canDrive(Car $newCar)
{
foreach ($cars as $car) {
// a comment
if (!$car->hasWheels()) {
continue;
}
if (!$car->hasFuel()) {
continue;
}
$car->setWheel($newCar->wheel);
$car->setFuel($newCar->fuel);
}
}
}
?>

42
rules/early-return/tests/Rector/If_/ChangeAndIfContinueToMultiContinueRector/Fixture/identical.php.inc Normal file
View File

@ -0,0 +1,42 @@
<?php
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfContinueToMultiContinueRector\Fixture;
class Identical
{
public function canDrive(Car $newCar)
{
foreach ($cars as $car) {
if ($car->getWheel() === 4 && $car->getFuel() === 'full') {
continue;
}
$car->setWheel($newCar->wheel);
$car->setFuel($newCar->fuel);
}
}
}
?>
-----
<?php
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfContinueToMultiContinueRector\Fixture;
class Identical
{
public function canDrive(Car $newCar)
{
foreach ($cars as $car) {
if ($car->getWheel() !== 4) {
continue;
}
if ($car->getFuel() !== 'full') {
continue;
}
$car->setWheel($newCar->wheel);
$car->setFuel($newCar->fuel);
}
}
}
?>

45
rules/early-return/tests/Rector/If_/ChangeAndIfContinueToMultiContinueRector/Fixture/many_and.php.inc Normal file
View File

@ -0,0 +1,45 @@
<?php
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfContinueToMultiContinueRector\Fixture;
class ManyAnd
{
public function canDrive(Car $newCar)
{
foreach ($cars as $car) {
if ($car->hasWheels() && $car->hasFuel() && $car->isRunnable()) {
continue;
}
$car->setWheel($newCar->wheel);
$car->setFuel($newCar->fuel);
}
}
}
?>
-----
<?php
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfContinueToMultiContinueRector\Fixture;
class ManyAnd
{
public function canDrive(Car $newCar)
{
foreach ($cars as $car) {
if (!$car->hasWheels()) {
continue;
}
if (!$car->hasFuel()) {
continue;
}
if (!$car->isRunnable()) {
continue;
}
$car->setWheel($newCar->wheel);
$car->setFuel($newCar->fuel);
}
}
}
?>

45
rules/early-return/tests/Rector/If_/ChangeAndIfContinueToMultiContinueRector/Fixture/negated.php.inc Normal file
View File

@ -0,0 +1,45 @@
<?php
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfContinueToMultiContinueRector\Fixture;
class Negated
{
public function canDrive(Car $newCar)
{
foreach ($cars as $car) {
if (! $car->hasWheels() && ! $car->hasFuel() && ! $car->isRunnable()) {
continue;
}
$car->setWheel($newCar->wheel);
$car->setFuel($newCar->fuel);
}
}
}
?>
-----
<?php
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfContinueToMultiContinueRector\Fixture;
class Negated
{
public function canDrive(Car $newCar)
{
foreach ($cars as $car) {
if ($car->hasWheels()) {
continue;
}
if ($car->hasFuel()) {
continue;
}
if ($car->isRunnable()) {
continue;
}
$car->setWheel($newCar->wheel);
$car->setFuel($newCar->fuel);
}
}
}
?>

42
rules/early-return/tests/Rector/If_/ChangeAndIfContinueToMultiContinueRector/Fixture/not_identical.php.inc Normal file
View File

@ -0,0 +1,42 @@
<?php
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfContinueToMultiContinueRector\Fixture;
class NotIdentical
{
public function canDrive(Car $newCar)
{
foreach ($cars as $car) {
if ($car->getWheel() !== 4 && $car->getFuel() !== 'full') {
continue;
}
$car->setWheel($newCar->wheel);
$car->setFuel($newCar->fuel);
}
}
}
?>
-----
<?php
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfContinueToMultiContinueRector\Fixture;
class NotIdentical
{
public function canDrive(Car $newCar)
{
foreach ($cars as $car) {
if ($car->getWheel() === 4) {
continue;
}
if ($car->getFuel() === 'full') {
continue;
}
$car->setWheel($newCar->wheel);
$car->setFuel($newCar->fuel);
}
}
}
?>

21
rules/early-return/tests/Rector/If_/ChangeAndIfContinueToMultiContinueRector/Fixture/skip_multi_stmts.php.inc Normal file
View File

@ -0,0 +1,21 @@
<?php
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfContinueToMultiContinueRector\Fixture;
class SkipMultiStmts
{
public function canDrive(Car $newCar)
{
foreach ($cars as $car) {
if ($car->hasWheels() && $car->hasFuel()) {
executeSideEffect();
continue;
}
$car->setWheel($newCar->wheel);
$car->setFuel($newCar->fuel);
}
}
}
?>

20
rules/early-return/tests/Rector/If_/ChangeAndIfContinueToMultiContinueRector/Fixture/skip_not_and.php.inc Normal file
View File

@ -0,0 +1,20 @@
<?php
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfContinueToMultiContinueRector\Fixture;
class SkipNotAnd
{
public function canDrive(Car $newCar)
{
foreach ($cars as $car) {
if ($car->hasWheels() || $car->hasFuel()) {
continue;
}
$car->setWheel($newCar->wheel);
$car->setFuel($newCar->fuel);
}
}
}
?>

20
rules/early-return/tests/Rector/If_/ChangeAndIfContinueToMultiContinueRector/Fixture/skip_not_continue.php.inc Normal file
View File

@ -0,0 +1,20 @@
<?php
namespace Rector\EarlyReturn\Tests\Rector\If_\ChangeAndIfContinueToMultiContinueRector\Fixture;
class SkipNotContinue
{
public function canDrive(Car $newCar)
{
foreach ($cars as $car) {
if ($car->hasWheels() && $car->hasFuel()) {
break;
}
$car->setWheel($newCar->wheel);
$car->setFuel($newCar->fuel);
}
}
}
?>

40
src/NodeManipulator/IfManipulator.php
View File

@ -14,6 +14,7 @@ use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Expression;
use PhpParser\Node\Stmt\Foreach_;
use PhpParser\Node\Stmt\If_;
@ -21,6 +22,7 @@ use PhpParser\Node\Stmt\Return_;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\Core\PhpParser\Node\Value\ValueResolver;
use Rector\Core\PhpParser\Printer\BetterStandardPrinter;
use Rector\EarlyReturn\NodeTransformer\ConditionInverter;
use Rector\NodeNameResolver\NodeNameResolver;
final class IfManipulator
@ -50,18 +52,25 @@ final class IfManipulator
*/
private $valueResolver;
/**
* @var ConditionInverter
*/
private $conditionInverter;
public function __construct(
BetterNodeFinder $betterNodeFinder,
BetterStandardPrinter $betterStandardPrinter,
NodeNameResolver $nodeNameResolver,
StmtsManipulator $stmtsManipulator,
ValueResolver $valueResolver
ValueResolver $valueResolver,
ConditionInverter $conditionInverter
) {
$this->betterStandardPrinter = $betterStandardPrinter;
$this->stmtsManipulator = $stmtsManipulator;
$this->nodeNameResolver = $nodeNameResolver;
$this->betterNodeFinder = $betterNodeFinder;
$this->valueResolver = $valueResolver;
$this->conditionInverter = $conditionInverter;
}
/**
@ -281,7 +290,7 @@ final class IfManipulator
return $ifs;
}
public function isIfWithOnlyReturn(Node $node): bool
public function isIfWithOnly(Node $node, string $className): bool
{
if (! $node instanceof If_) {
return false;
@ -291,20 +300,7 @@ final class IfManipulator
return false;
}
return $this->hasOnlyStmtOfType($node, Return_::class);
}
public function isIfWithOnlyForeach(Node $node): bool
{
if (! $node instanceof If_) {
return false;
}
if (! $this->isIfWithoutElseAndElseIfs($node)) {
return false;
}
return $this->hasOnlyStmtOfType($node, Foreach_::class);
return $this->hasOnlyStmtOfType($node, $className);
}
public function isIfWithOnlyOneStmt(If_ $if): bool
@ -345,6 +341,18 @@ final class IfManipulator
return ! (bool) $if->elseifs;
}
public function createIfNegation(Expr $expr, Stmt $stmt): If_
{
$expr = $this->conditionInverter->createInvertedCondition($expr);
return new If_(
$expr,
[
'stmts' => [$stmt],
]
);
}
private function matchComparedAndReturnedNode(NotIdentical $notIdentical, Return_ $return): ?Expr
{
if ($this->betterStandardPrinter->areNodesEqual(