AllowComparingOnlyComparableTypesRule: allow tuple comparison (#182)

shipmonk-rnd · Dec 1, 2023 · 0c102ce · 0c102ce
1 parent 592ab7a
commit 0c102ce
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 36 deletions.
diff --git a/README.md b/README.md
@@ -139,7 +139,7 @@ parameters:
 ## Rules:
 
 ### allowComparingOnlyComparableTypes
-- Denies using comparison operators `>,<,<=,>=,<=>` over anything other than `int|string|float|DateTimeInterface`. Null is not allowed.
+- Denies using comparison operators `>,<,<=,>=,<=>` over anything other than `int|string|float|DateTimeInterface` or same size tuples containing comparable types. Null is not allowed.
 - Mixing different types in those operators is also forbidden, only exception is comparing floats with integers
 - Mainly targets to accidental comparisons of objects, enums or arrays which is valid in PHP, but very tricky
 

diff --git a/src/Rule/AllowComparingOnlyComparableTypesRule.php b/src/Rule/AllowComparingOnlyComparableTypesRule.php
@@ -19,8 +19,9 @@
 use PHPStan\Type\ObjectType;
 use PHPStan\Type\StringType;
 use PHPStan\Type\Type;
-use PHPStan\Type\UnionType;
+use PHPStan\Type\TypeCombinator;
 use PHPStan\Type\VerbosityLevel;
+use function count;
 
 /**
  * @implements Rule<BinaryOp>
@@ -52,11 +53,11 @@ public function processNode(Node $node, Scope $scope): array
         $leftType = $scope->getType($node->left);
         $rightType = $scope->getType($node->right);
 
-        $leftTypeDescribed = $leftType->describe(VerbosityLevel::typeOnly());
-        $rightTypeDescribed = $rightType->describe(VerbosityLevel::typeOnly());
+        $leftTypeDescribed = $leftType->describe($leftType->isArray()->no() ? VerbosityLevel::typeOnly() : VerbosityLevel::value());
+        $rightTypeDescribed = $rightType->describe($rightType->isArray()->no() ? VerbosityLevel::typeOnly() : VerbosityLevel::value());
 
         if (!$this->isComparable($leftType) || !$this->isComparable($rightType)) {
-            $error = RuleErrorBuilder::message("Comparison {$leftTypeDescribed} {$node->getOperatorSigil()} {$rightTypeDescribed} contains non-comparable type, only int|float|string|DateTimeInterface is allowed.")
+            $error = RuleErrorBuilder::message("Comparison {$leftTypeDescribed} {$node->getOperatorSigil()} {$rightTypeDescribed} contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.")
                 ->identifier('shipmonk.comparingNonComparableTypes')
                 ->build();
             return [$error];
@@ -79,7 +80,23 @@ private function isComparable(Type $type): bool
         $stringType = new StringType();
         $dateTimeType = new ObjectType(DateTimeInterface::class);
 
-        return $this->containsOnlyTypes($type, [$intType, $floatType, $stringType, $dateTimeType]);
+        if ($this->containsOnlyTypes($type, [$intType, $floatType, $stringType, $dateTimeType])) {
+            return true;
+        }
+
+        if (!$type->isConstantArray()->yes() || !$type->isList()->yes()) {
+            return false;
+        }
+
+        foreach ($type->getConstantArrays() as $constantArray) {
+            foreach ($constantArray->getValueTypes() as $valueType) {
+                if (!$this->isComparable($valueType)) {
+                    return false;
+                }
+            }
+        }
+
+        return true;
     }
 
     private function isComparableTogether(Type $leftType, Type $rightType): bool
@@ -89,36 +106,53 @@ private function isComparableTogether(Type $leftType, Type $rightType): bool
         $stringType = new StringType();
         $dateTimeType = new ObjectType(DateTimeInterface::class);
 
-        return ($this->containsOnlyTypes($leftType, [$intType, $floatType]) && $this->containsOnlyTypes($rightType, [$intType, $floatType]))
-            || ($this->containsOnlyTypes($leftType, [$stringType]) && $this->containsOnlyTypes($rightType, [$stringType]))
-            || ($this->containsOnlyTypes($leftType, [$dateTimeType]) && $this->containsOnlyTypes($rightType, [$dateTimeType]));
-    }
+        if ($this->containsOnlyTypes($leftType, [$intType, $floatType])) {
+            return $this->containsOnlyTypes($rightType, [$intType, $floatType]);
+        }
 
-    /**
-     * @param Type[] $allowedTypes
-     */
-    private function containsOnlyTypes(Type $checkedType, array $allowedTypes): bool
-    {
-        $typesToCheck = $checkedType instanceof UnionType
-            ? $checkedType->getTypes()
-            : [$checkedType];
+        if ($this->containsOnlyTypes($leftType, [$stringType])) {
+            return $this->containsOnlyTypes($rightType, [$stringType]);
+        }
 
-        foreach ($typesToCheck as $typeToCheck) {
-            $isWithinAllowed = false;
+        if ($this->containsOnlyTypes($leftType, [$dateTimeType])) {
+            return $this->containsOnlyTypes($rightType, [$dateTimeType]);
+        }
 
-            foreach ($allowedTypes as $allowedType) {
-                if ($allowedType->isSuperTypeOf($typeToCheck)->yes()) {
-                    $isWithinAllowed = true;
-                    break;
-                }
+        if ($leftType->isConstantArray()->yes()) {
+            if (!$rightType->isConstantArray()->yes()) {
+                return false;
             }
 
-            if (!$isWithinAllowed) {
-                return false;
+            foreach ($leftType->getConstantArrays() as $leftConstantArray) {
+                foreach ($rightType->getConstantArrays() as $rightConstantArray) {
+                    $leftValueTypes = $leftConstantArray->getValueTypes();
+                    $rightValueTypes = $rightConstantArray->getValueTypes();
+
+                    if (count($leftValueTypes) !== count($rightValueTypes)) {
+                        return false;
+                    }
+
+                    for ($i = 0; $i < count($leftValueTypes); $i++) {
+                        if (!$this->isComparableTogether($leftValueTypes[$i], $rightValueTypes[$i])) {
+                            return false;
+                        }
+                    }
+                }
             }
+
+            return true;
         }
 
-        return true;
+        return false;
+    }
+
+    /**
+     * @param Type[] $allowedTypes
+     */
+    private function containsOnlyTypes(Type $checkedType, array $allowedTypes): bool
+    {
+        $allowedType = TypeCombinator::union(...$allowedTypes);
+        return $allowedType->isSuperTypeOf($checkedType)->yes();
     }
 
 }
diff --git a/tests/Rule/data/AllowComparingOnlyComparableTypesRule/code.php b/tests/Rule/data/AllowComparingOnlyComparableTypesRule/code.php
@@ -25,23 +25,39 @@ interface Bar {}
     int|float $intOrFloat,
     float $float,
     bool $bool,
+    mixed $mixed,
 ) {
-    $foos > $foo; // error: Comparison array > AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface is allowed.
-    $nullableInt > $int; // error: Comparison int|null > int contains non-comparable type, only int|float|string|DateTimeInterface is allowed.
-    null > $int; // error: Comparison null > int contains non-comparable type, only int|float|string|DateTimeInterface is allowed.
-    $foo > $foo2; // error: Comparison AllowComparingOnlyComparableTypesRule\Foo > AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface is allowed.
-    $foo > $fooOrBar; // error: Comparison AllowComparingOnlyComparableTypesRule\Foo > AllowComparingOnlyComparableTypesRule\Bar|AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface is allowed.
-    $foo > $fooAndBar; // error: Comparison AllowComparingOnlyComparableTypesRule\Foo > AllowComparingOnlyComparableTypesRule\Bar&AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface is allowed.
+    $foos > $foo; // error: Comparison array > AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
+    $nullableInt > $int; // error: Comparison int|null > int contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
+    null > $int; // error: Comparison null > int contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
+    $foo > $foo2; // error: Comparison AllowComparingOnlyComparableTypesRule\Foo > AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
+    $foo > $fooOrBar; // error: Comparison AllowComparingOnlyComparableTypesRule\Foo > AllowComparingOnlyComparableTypesRule\Bar|AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
+    $foo > $fooAndBar; // error: Comparison AllowComparingOnlyComparableTypesRule\Foo > AllowComparingOnlyComparableTypesRule\Bar&AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
     $string > 'foo';
     $int > 2;
     $float > 2;
     $int > $intOrFloat;
     $string > $intOrFloat; // error: Cannot compare different types in string > float|int.
-    $bool > true; // error: Comparison bool > true contains non-comparable type, only int|float|string|DateTimeInterface is allowed.
+    $bool > true; // error: Comparison bool > true contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
     $dateTime > $dateTimeImmutable;
-    $dateTime > $foo; // error: Comparison DateTime > AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface is allowed.
+    $dateTime > $foo; // error: Comparison DateTime > AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
 
     $string > $int; // error: Cannot compare different types in string > int.
     $float > $int;
     $dateTime > $string; // error: Cannot compare different types in DateTime > string.
+
+    [$int, $string] > [$int, $string];
+    [[$int]] > [[$int]];
+    [$int, $float, $intOrFloat, $intOrFloat] > [$int, $int, $int, $float];
+    [$int, $string] > $foos; // error: Comparison array{int, string} > array contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
+    [$int] > [$int, $int]; // error: Cannot compare different types in array{int} > array{int, int}.
+    [$int, $string] > [$int]; // error: Cannot compare different types in array{int, string} > array{int}.
+    [$string, $int] > [$int, $string]; // error: Cannot compare different types in array{string, int} > array{int, string}.
+    [$foo] > [$foo]; // error: Comparison array{AllowComparingOnlyComparableTypesRule\Foo} > array{AllowComparingOnlyComparableTypesRule\Foo} contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
+    [$int] > [$mixed]; // error: Comparison array{int} > array{mixed} contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
+    [$dateTime] > [[$dateTimeImmutable]]; // error: Cannot compare different types in array{DateTime} > array{array{DateTimeImmutable}}.
+
+    [0 => $int, 1 => $string] > [$int, $string];
+    [1 => $int, 0 => $string] > [$int, $string]; // error: Comparison array{1: int, 0: string} > array{int, string} contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
+    ['X' => $int, 'Y' => $string] > ['X' => $int, 'Y' => $string]; // error: Comparison array{X: int, Y: string} > array{X: int, Y: string} contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
 };