[SOLID] Change if && to early return

config/set/solid.php

@@ -8,6 +8,7 @@
 use Rector\SOLID\Rector\Class_\MakeUnusedClassesWithChildrenAbstractRector;
 use Rector\SOLID\Rector\Class_\RepeatedLiteralToClassConstantRector;
 use Rector\SOLID\Rector\Foreach_\ChangeNestedForeachIfsToEarlyContinueRector;
+use Rector\SOLID\Rector\If_\ChangeAndIfToEarlyReturnRector;
 use Rector\SOLID\Rector\If_\ChangeIfElseValueAssignToEarlyReturnRector;
 use Rector\SOLID\Rector\If_\ChangeNestedIfsToEarlyReturnRector;
 use Rector\SOLID\Rector\If_\RemoveAlwaysElseRector;
@@ -39,4 +40,6 @@
     $services->set(ChangeIfElseValueAssignToEarlyReturnRector::class);
 
     $services->set(UseMessageVariableForSprintfInSymfonyStyleRector::class);
+
+    $services->set(ChangeAndIfToEarlyReturnRector::class);
 };

rules/solid/src/NodeTransformer/ConditionInverter.php

@@ -33,6 +33,10 @@ public function createInvertedCondition(Expr $expr): Expr
             return $inversedCondition;
         }
 
+        if ($expr instanceof BooleanNot) {
+            return $expr->expr;
+        }
+
         return new BooleanNot($expr);
     }
 }

rules/solid/src/Rector/If_/ChangeAndIfToEarlyReturnRector.php

@@ -0,0 +1,227 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\SOLID\Rector\If_;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
+use PhpParser\Node\FunctionLike;
+use PhpParser\Node\Stmt;
+use PhpParser\Node\Stmt\If_;
+use PhpParser\Node\Stmt\Return_;
+use Rector\Core\PhpParser\Node\Manipulator\IfManipulator;
+use Rector\Core\PhpParser\Node\Manipulator\StmtsManipulator;
+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_\ChangeAndIfToEarlyReturnRector\ChangeAndIfToEarlyReturnRectorTest
+ */
+final class ChangeAndIfToEarlyReturnRector extends AbstractRector
+{
+    /**
+     * @var IfManipulator
+     */
+    private $ifManipulator;
+
+    /**
+     * @var ConditionInverter
+     */
+    private $conditionInverter;
+
+    /**
+     * @var StmtsManipulator
+     */
+    private $stmtsManipulator;
+
+    public function __construct(
+        ConditionInverter $conditionInverter,
+        IfManipulator $ifManipulator,
+        StmtsManipulator $stmtsManipulator
+    ) {
+        $this->ifManipulator = $ifManipulator;
+        $this->conditionInverter = $conditionInverter;
+        $this->stmtsManipulator = $stmtsManipulator;
+    }
+
+    public function getDefinition(): RectorDefinition
+    {
+        return new RectorDefinition('Changes if && to early return', [
+            new CodeSample(
+                <<<'CODE_SAMPLE'
+class SomeClass
+{
+    public function canDrive(Car $car)
+    {
+        if ($car->hasWheels && $car->hasFuel) {
+            return true;
+        }
+
+        return false;
+    }
+}
+CODE_SAMPLE
+
+                ,
+                <<<'CODE_SAMPLE'
+class SomeClass
+{
+    public function canDrive(Car $car)
+    {
+        if (!$car->hasWheels) {
+            return false;
+        }
+
+        if (!$car->hasFuel) {
+            return false;
+        }
+
+        return true;
+    }
+}
+CODE_SAMPLE
+            ),
+        ]);
+    }
+
+    /**
+     * @return string[]
+     */
+    public function getNodeTypes(): array
+    {
+        return [If_::class];
+    }
+
+    /**
+     * @param If_ $node
+     */
+    public function refactor(Node $node): ?Node
+    {
+        if ($this->shouldSkip($node)) {
+            return null;
+        }
+
+        $ifReturn = $this->getIfReturn($node);
+        if ($ifReturn === null) {
+            return null;
+        }
+
+        /** @var BooleanAnd $expr */
+        $expr = $node->cond;
+        $invertedLeftCondition = $this->conditionInverter->createInvertedCondition($expr->left);
+        $invertedRightCondition = $this->conditionInverter->createInvertedCondition($expr->right);
+
+        $firstIf = new If_($invertedLeftCondition);
+        $firstIf->stmts = [new Return_()];
+        $secondIf = new If_($invertedRightCondition);
+        $secondIf->stmts = [new Return_()];
+
+        $this->addNodeAfterNode($firstIf, $node);
+        $this->addNodeAfterNode($secondIf, $node);
+        $this->addNodeAfterNode($ifReturn, $node);
+        $this->removeNode($node);
+
+        $functionLikeReturn = $this->getFunctionLikeReturn($node);
+        if ($functionLikeReturn !== null) {
+            $this->removeNode($functionLikeReturn);
+        }
+
+        return null;
+    }
+
+    private function shouldSkip(If_ $if): bool
+    {
+        if (! $this->ifManipulator->isIfWithOnlyOneStmt($if)) {
+            return true;
+        }
+
+        if (! $this->ifManipulator->isIfFirstLevelStmt($if)) {
+            return true;
+        }
+
+        if (! $if->cond instanceof BooleanAnd) {
+            return true;
+        }
+
+        if ($this->hasMoreThanTwoConditions($if)) {
+            return true;
+        }
+
+        if (! $this->isFunctionLikeReturnsVoid($if)) {
+            return true;
+        }
+
+        if ($if->else !== null) {
+            return true;
+        }
+
+        if ($if->elseifs !== []) {
+            return true;
+        }
+
+        return ! $this->isLastIfOrBeforeLastReturn($if);
+    }
+
+    private function getIfReturn(If_ $if): ?Stmt
+    {
+        $ifStmt = end($if->stmts);
+        if ($ifStmt === false) {
+            return null;
+        }
+
+        return $ifStmt;
+    }
+
+    private function getFunctionLikeReturn(If_ $if): ?Return_
+    {
+        /** @var FunctionLike|null $functionLike */
+        $functionLike = $this->betterNodeFinder->findFirstParentInstanceOf($if, FunctionLike::class);
+        if ($functionLike === null) {
+            return null;
+        }
+
+        if ($functionLike->getStmts() === null) {
+            return null;
+        }
+
+        $return = $this->stmtsManipulator->getUnwrappedLastStmt($functionLike->getStmts());
+        if ($return === null) {
+            return null;
+        }
+
+        if (! $return instanceof Return_) {
+            return null;
+        }
+
+        return $return;
+    }
+
+    private function hasMoreThanTwoConditions(If_ $if): bool
+    {
+        $binaryOps = $this->betterNodeFinder->findInstanceOf($if->cond, BooleanAnd::class);
+        return count($binaryOps) >= 2;
+    }
+
+    private function isFunctionLikeReturnsVoid(If_ $if): bool
+    {
+        $return = $this->getFunctionLikeReturn($if);
+        if ($return === null) {
+            return true;
+        }
+
+        return $return->expr === null;
+    }
+
+    private function isLastIfOrBeforeLastReturn(If_ $if): bool
+    {
+        $nextNode = $if->getAttribute(AttributeKey::NEXT_NODE);
+        if ($nextNode === null) {
+            return true;
+        }
+        return $nextNode instanceof Return_;
+    }
+}

rules/solid/tests/Rector/If_/ChangeAndIfToEarlyReturnRector/ChangeAndIfToEarlyReturnRectorTest.php

@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\SOLID\Tests\Rector\If_\ChangeAndIfToEarlyReturnRector;
+
+use Iterator;
+use Rector\Core\Testing\PHPUnit\AbstractRectorTestCase;
+use Rector\SOLID\Rector\If_\ChangeAndIfToEarlyReturnRector;
+use Symplify\SmartFileSystem\SmartFileInfo;
+
+final class ChangeAndIfToEarlyReturnRectorTest 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 ChangeAndIfToEarlyReturnRector::class;
+    }
+}

rules/solid/tests/Rector/If_/ChangeAndIfToEarlyReturnRector/Fixture/boolean_not.php.inc

@@ -0,0 +1,37 @@
+<?php
+
+namespace Rector\SOLID\Tests\Rector\If_\ChangeAndIfToEarlyReturnRector\Fixture;
+
+class BooleanNotClass
+{
+    public function canDrive(Car $car)
+    {
+        if (!$car->hasWheels && !$car->hasFuel) {
+            $this->canDrive = true;
+        }
+
+        return;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\SOLID\Tests\Rector\If_\ChangeAndIfToEarlyReturnRector\Fixture;
+
+class BooleanNotClass
+{
+    public function canDrive(Car $car)
+    {
+        if ($car->hasWheels) {
+            return;
+        }
+        if ($car->hasFuel) {
+            return;
+        }
+        $this->canDrive = true;
+    }
+}
+
+?>

rules/solid/tests/Rector/If_/ChangeAndIfToEarlyReturnRector/Fixture/closure.php.inc

@@ -0,0 +1,53 @@
+<?php
+
+namespace Rector\SOLID\Tests\Rector\If_\ChangeAndIfToEarlyReturnRector\Fixture;
+
+use PhpParser\Node;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Name;
+
+class ClosureClass
+{
+    private function refactorStmts(array $nodes): array
+    {
+        $this->traverseNodesWithCallable($nodes, function (Node $node) {
+            if ($node instanceof Name && $node instanceof Identifier) {
+                $this->processNameOrIdentifier($node);
+            }
+
+            return;
+        });
+
+        return $nodes;
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\SOLID\Tests\Rector\If_\ChangeAndIfToEarlyReturnRector\Fixture;
+
+use PhpParser\Node;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Name;
+
+class ClosureClass
+{
+    private function refactorStmts(array $nodes): array
+    {
+        $this->traverseNodesWithCallable($nodes, function (Node $node) {
+            if (!$node instanceof Name) {
+                return;
+            }
+            if (!$node instanceof Identifier) {
+                return;
+            }
+            $this->processNameOrIdentifier($node);
+        });
+
+        return $nodes;
+    }
+}
+
+?>