Merge pull request #697 from rectorphp/cq-negative

[CodeQuality] Expand SimplifyConditionsRector by Identical comparisons

packages/Symfony/src/Rector/DependencyInjection/ContainerBuilderCompileEnvArgumentRector.php

@@ -55,7 +55,7 @@ final class ContainerBuilderCompileEnvArgumentRector extends AbstractRector
             return null;
         }
 
-        if ((count($node->args) !== 1) === false) {
+        if (count($node->args) === 1) {
             return null;
         }
 

src/Rector/CodeQuality/SimplifyConditionsRector.php

@@ -13,6 +13,7 @@ use PhpParser\Node\Expr\BinaryOp\NotIdentical;
 use PhpParser\Node\Expr\BinaryOp\Smaller;
 use PhpParser\Node\Expr\BinaryOp\SmallerOrEqual;
 use PhpParser\Node\Expr\BooleanNot;
+use Rector\NodeAnalyzer\ConstFetchAnalyzer;
 use Rector\Rector\AbstractRector;
 use Rector\RectorDefinition\CodeSample;
 use Rector\RectorDefinition\RectorDefinition;
@@ -22,7 +23,7 @@ final class SimplifyConditionsRector extends AbstractRector
     /**
      * @var string[]
      */
-    private $binaryOpClassMap = [
+    private $binaryOpClassesToInversedClasses = [
         Identical::class => NotIdentical::class,
         NotIdentical::class => Identical::class,
         Equal::class => NotEqual::class,
@@ -33,6 +34,16 @@ final class SimplifyConditionsRector extends AbstractRector
         SmallerOrEqual::class => Greater::class,
     ];
 
+    /**
+     * @var ConstFetchAnalyzer
+     */
+    private $constFetchAnalyzer;
+
+    public function __construct(ConstFetchAnalyzer $constFetchAnalyzer)
+    {
+        $this->constFetchAnalyzer = $constFetchAnalyzer;
+    }
+
     public function getDefinition(): RectorDefinition
     {
         return new RectorDefinition(
@@ -46,26 +57,82 @@ final class SimplifyConditionsRector extends AbstractRector
      */
     public function getNodeTypes(): array
     {
-        return [BooleanNot::class];
+        return [BooleanNot::class, Identical::class];
     }
 
     /**
-     * @param BooleanNot $notNode
+     * @param BooleanNot|Identical $node
      */
-    public function refactor(Node $notNode): ?Node
+    public function refactor(Node $node): ?Node
     {
-        if (! $notNode->expr instanceof BinaryOp) {
+        if ($node instanceof BooleanNot) {
-            return $notNode;
+            return $this->processBooleanNot($node);
         }
 
-        $binaryOpType = get_class($notNode->expr);
+        if ($node instanceof Identical) {
+            return $this->processIdenticalAndNotIdentical($node);
+        }
+    }
 
-        if (! isset($this->binaryOpClassMap[$binaryOpType])) {
+    private function processBooleanNot(BooleanNot $node): Node
-            return $notNode;
+    {
+        if (! $node->expr instanceof BinaryOp) {
+            return $node;
         }
 
-        $newBinaryOp = $this->binaryOpClassMap[$binaryOpType];
+        if ($this->shouldSkip($node->expr)) {
+            return $node;
+        }
 
-        return new $newBinaryOp($notNode->expr->left, $notNode->expr->right);
+        return $this->createInversedBooleanOp($node->expr);
+    }
+
+    private function processIdenticalAndNotIdentical(BinaryOp $node): Node
+    {
+        if ($node->left instanceof Identical || $node->left instanceof NotIdentical) {
+            $subBinaryOpNode = $node->left;
+            $shouldInverse = $this->constFetchAnalyzer->isFalse($node->right);
+        } elseif ($node->right instanceof Identical || $node->right instanceof NotIdentical) {
+            $subBinaryOpNode = $node->right;
+            $shouldInverse = $this->constFetchAnalyzer->isFalse($node->left);
+        } else {
+            return $node;
+        }
+
+        if ($shouldInverse) {
+            return $this->createInversedBooleanOp($subBinaryOpNode);
+        }
+
+        return $subBinaryOpNode;
+    }
+
+    private function createInversedBooleanOp(BinaryOp $binaryOpNode): BinaryOp
+    {
+        $binaryOpNodeClass = get_class($binaryOpNode);
+
+        // we can't invert that
+        if (! isset($this->binaryOpClassesToInversedClasses[$binaryOpNodeClass])) {
+            return $binaryOpNode;
+        }
+
+        $inversedBinaryOpNodeClass = $this->binaryOpClassesToInversedClasses[$binaryOpNodeClass];
+
+        return new $inversedBinaryOpNodeClass($binaryOpNode->left, $binaryOpNode->right);
+    }
+
+    /**
+     * Skip too nested binary || binary > binary combinations
+     */
+    private function shouldSkip(BinaryOp $binaryOpNode): bool
+    {
+        if ($binaryOpNode->left instanceof BinaryOp) {
+            return true;
+        }
+
+        if ($binaryOpNode->right instanceof BinaryOp) {
+            return true;
+        }
+
+        return false;
     }
 }

tests/Rector/CodeQuality/SimplifyConditionsRector/Correct/correct2.php.inc

@@ -0,0 +1,17 @@
+<?php declare(strict_types=1);
+
+if ($value !== null) {
+    echo 'maybe';
+}
+
+if ($value === null) {
+    echo 'maybe';
+}
+
+if ($value === null) {
+    echo 'maybe';
+}
+
+if ($value !== null) {
+    echo 'maybe';
+}

tests/Rector/CodeQuality/SimplifyConditionsRector/Correct/correct3.php.inc

@@ -0,0 +1,5 @@
+<?php declare(strict_types=1);
+
+if ($a === '-' && ! $start && ! ($i < $l && $s[$i] === ']')) {
+    echo 'maybe';
+}

tests/Rector/CodeQuality/SimplifyConditionsRector/SimplifyConditionsRectorTest.php

@@ -21,6 +21,8 @@ final class SimplifyConditionsRectorTest extends AbstractRectorTestCase
     public function provideWrongToFixedFiles(): Iterator
     {
         yield [__DIR__ . '/Wrong/wrong.php.inc', __DIR__ . '/Correct/correct.php.inc'];
+        yield [__DIR__ . '/Wrong/wrong2.php.inc', __DIR__ . '/Correct/correct2.php.inc'];
+        yield [__DIR__ . '/Wrong/wrong3.php.inc', __DIR__ . '/Correct/correct3.php.inc'];
     }
 
     protected function provideConfig(): string

tests/Rector/CodeQuality/SimplifyConditionsRector/Wrong/wrong2.php.inc

@@ -0,0 +1,17 @@
+<?php declare(strict_types=1);
+
+if (($value === null) === false) {
+    echo 'maybe';
+}
+
+if (($value === null) === true) {
+    echo 'maybe';
+}
+
+if (false === ($value !== null)) {
+    echo 'maybe';
+}
+
+if (true === ($value !== null)) {
+    echo 'maybe';
+}

tests/Rector/CodeQuality/SimplifyConditionsRector/Wrong/wrong3.php.inc

@@ -0,0 +1,5 @@
+<?php declare(strict_types=1);
+
+if ($a === '-' && ! $start && ! ($i < $l && $s[$i] === ']')) {
+    echo 'maybe';
+}