Improve IfToSpaceshipRector (#3871)

rectorphp · May 16, 2023 · d42fd12 · d42fd12
1 parent d2ae72e
commit d42fd12
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 20 deletions.
diff --git a/rules-tests/Php70/Rector/If_/IfToSpaceshipRector/Fixture/ascending_flip.php.inc b/rules-tests/Php70/Rector/If_/IfToSpaceshipRector/Fixture/ascending_flip.php.inc
@@ -16,3 +16,22 @@ final class AscendingFlip
 
     }
 }
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\Php70\Rector\If_\IfToSpaceshipRector\Fixture;
+
+final class AscendingFlip
+{
+    public function run($languages)
+    {
+        usort($languages, function ($ascendingFirst, $ascendingSecond) {
+            return $ascendingFirst[0] <=> $ascendingSecond[0];
+        });
+
+    }
+}
+
+?>
diff --git a/rules-tests/Php70/Rector/If_/IfToSpaceshipRector/Fixture/descending_flip.php.inc b/rules-tests/Php70/Rector/If_/IfToSpaceshipRector/Fixture/descending_flip.php.inc
@@ -18,3 +18,20 @@ final class DescendingFlip
 
 ?>
 
+-----
+<?php
+
+namespace Rector\Tests\Php70\Rector\If_\IfToSpaceshipRector\Fixture;
+
+final class DescendingFlip
+{
+    public function run(array $languages)
+    {
+        usort($languages, function ($firstLanguage, $secondLanguage) {
+            return $secondLanguage[0] <=> $firstLanguage[0];
+        });
+    }
+}
+
+?>
+
diff --git a/rules/Php70/NodeAnalyzer/BattleshipTernaryAnalyzer.php b/rules/Php70/NodeAnalyzer/BattleshipTernaryAnalyzer.php
@@ -26,10 +26,6 @@ public function __construct(
      */
     public function isGreaterLowerCompareReturnOneAndMinusOne(Ternary $ternary, ComparedExprs $comparedExprs): ?string
     {
-        if (! $ternary->if instanceof Expr) {
-            return null;
-        }
-
         if ($ternary->cond instanceof Greater) {
             return $this->evaluateGreater($ternary->cond, $ternary, $comparedExprs);
         }
@@ -54,23 +50,22 @@ private function evaluateGreater(Greater $greater, Ternary $ternary, ComparedExp
             return null;
         }
 
-        if (! $this->nodeComparator->areNodesEqual($greater->left, $comparedExprs->getFirstExpr())) {
-            return null;
+        if (
+            $this->nodeComparator->areNodesEqual($greater->left, $comparedExprs->getFirstExpr()) &&
+            $this->nodeComparator->areNodesEqual($greater->right, $comparedExprs->getSecondExpr())
+        ) {
+            return $this->evaluateTernaryDesc($ternary);
         }
 
-        if (! $this->nodeComparator->areNodesEqual($greater->right, $comparedExprs->getSecondExpr())) {
+        if (!$this->nodeComparator->areNodesEqual($greater->right, $comparedExprs->getFirstExpr())) {
             return null;
         }
 
-        if ($this->isValueOneAndMinusOne($ternary->if, $ternary->else)) {
-            return BattleshipCompareOrder::DESC;
-        }
-
-        if ($this->isValueOneAndMinusOne($ternary->else, $ternary->if)) {
-            return BattleshipCompareOrder::ASC;
+        if (!$this->nodeComparator->areNodesEqual($greater->left, $comparedExprs->getSecondExpr())) {
+            return null;
         }
 
-        return null;
+        return $this->evaluateTernaryAsc($ternary);
     }
 
     /**
@@ -86,11 +81,39 @@ private function evaluateSmaller(Smaller $smaller, Ternary $ternary, ComparedExp
             return null;
         }
 
-        if (! $this->nodeComparator->areNodesEqual($smaller->left, $comparedExprs->getFirstExpr())) {
+        if (
+            $this->nodeComparator->areNodesEqual($smaller->left, $comparedExprs->getFirstExpr()) &&
+            $this->nodeComparator->areNodesEqual($smaller->right, $comparedExprs->getSecondExpr())
+        ) {
+            return $this->evaluateTernaryAsc($ternary);
+        }
+
+        if (!$this->nodeComparator->areNodesEqual($smaller->right, $comparedExprs->getFirstExpr())) {
             return null;
         }
 
-        if (! $this->nodeComparator->areNodesEqual($smaller->right, $comparedExprs->getSecondExpr())) {
+        if (!$this->nodeComparator->areNodesEqual($smaller->left, $comparedExprs->getSecondExpr())) {
+            return null;
+        }
+
+        return $this->evaluateTernaryDesc($ternary);
+    }
+
+    private function isValueOneAndMinusOne(Expr $firstExpr, Expr $seconcExpr): bool
+    {
+        if (! $this->valueResolver->isValue($firstExpr, 1)) {
+            return false;
+        }
+
+        return $this->valueResolver->isValue($seconcExpr, -1);
+    }
+
+    /**
+     * @return BattleshipCompareOrder::*|null
+     */
+    private function evaluateTernaryAsc(Ternary $ternary): ?string
+    {
+        if (! $ternary->if instanceof Expr) {
             return null;
         }
 
@@ -105,12 +128,23 @@ private function evaluateSmaller(Smaller $smaller, Ternary $ternary, ComparedExp
         return null;
     }
 
-    private function isValueOneAndMinusOne(Expr $firstExpr, Expr $seconcExpr): bool
+    /**
+     * @return BattleshipCompareOrder::*|null
+     */
+    private function evaluateTernaryDesc(Ternary $ternary): ?string
     {
-        if (! $this->valueResolver->isValue($firstExpr, 1)) {
-            return false;
+        if (! $ternary->if instanceof Expr) {
+            return null;
         }
 
-        return $this->valueResolver->isValue($seconcExpr, -1);
+        if ($this->isValueOneAndMinusOne($ternary->if, $ternary->else)) {
+            return BattleshipCompareOrder::DESC;
+        }
+
+        if ($this->isValueOneAndMinusOne($ternary->else, $ternary->if)) {
+            return BattleshipCompareOrder::ASC;
+        }
+
+        return null;
     }
 }