Skip to content

Commit 0c102ce

Browse files
JanTvrdikJanTvrdik
authored
AllowComparingOnlyComparableTypesRule: allow tuple comparison (#182)
1 parent 592ab7a commit 0c102ce

File tree

3 files changed

+86
-36
lines changed

3 files changed

+86
-36
lines changed

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ parameters:
139139
## Rules:
140140

141141
### allowComparingOnlyComparableTypes
142-
- Denies using comparison operators `>,<,<=,>=,<=>` over anything other than `int|string|float|DateTimeInterface`. Null is not allowed.
142+
- Denies using comparison operators `>,<,<=,>=,<=>` over anything other than `int|string|float|DateTimeInterface` or same size tuples containing comparable types. Null is not allowed.
143143
- Mixing different types in those operators is also forbidden, only exception is comparing floats with integers
144144
- Mainly targets to accidental comparisons of objects, enums or arrays which is valid in PHP, but very tricky
145145

‎src/Rule/AllowComparingOnlyComparableTypesRule.php

Lines changed: 61 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,9 @@
1919
use PHPStan\Type\ObjectType;
2020
use PHPStan\Type\StringType;
2121
use PHPStan\Type\Type;
22-
use PHPStan\Type\UnionType;
22+
use PHPStan\Type\TypeCombinator;
2323
use PHPStan\Type\VerbosityLevel;
24+
use function count;
2425

2526
/**
2627
* @implements Rule<BinaryOp>
@@ -52,11 +53,11 @@ public function processNode(Node $node, Scope $scope): array
5253
$leftType = $scope->getType($node->left);
5354
$rightType = $scope->getType($node->right);
5455

55-
$leftTypeDescribed = $leftType->describe(VerbosityLevel::typeOnly());
56-
$rightTypeDescribed = $rightType->describe(VerbosityLevel::typeOnly());
56+
$leftTypeDescribed = $leftType->describe($leftType->isArray()->no() ? VerbosityLevel::typeOnly() : VerbosityLevel::value());
57+
$rightTypeDescribed = $rightType->describe($rightType->isArray()->no() ? VerbosityLevel::typeOnly() : VerbosityLevel::value());
5758

5859
if (!$this->isComparable($leftType) || !$this->isComparable($rightType)) {
59-
$error = RuleErrorBuilder::message("Comparison {$leftTypeDescribed} {$node->getOperatorSigil()} {$rightTypeDescribed} contains non-comparable type, only int|float|string|DateTimeInterface is allowed.")
60+
$error = RuleErrorBuilder::message("Comparison {$leftTypeDescribed} {$node->getOperatorSigil()} {$rightTypeDescribed} contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.")
6061
->identifier('shipmonk.comparingNonComparableTypes')
6162
->build();
6263
return [$error];
@@ -79,7 +80,23 @@ private function isComparable(Type $type): bool
7980
$stringType = new StringType();
8081
$dateTimeType = new ObjectType(DateTimeInterface::class);
8182

82-
return $this->containsOnlyTypes($type, [$intType, $floatType, $stringType, $dateTimeType]);
83+
if ($this->containsOnlyTypes($type, [$intType, $floatType, $stringType, $dateTimeType])) {
84+
return true;
85+
}
86+
87+
if (!$type->isConstantArray()->yes() || !$type->isList()->yes()) {
88+
return false;
89+
}
90+
91+
foreach ($type->getConstantArrays() as $constantArray) {
92+
foreach ($constantArray->getValueTypes() as $valueType) {
93+
if (!$this->isComparable($valueType)) {
94+
return false;
95+
}
96+
}
97+
}
98+
99+
return true;
83100
}
84101

85102
private function isComparableTogether(Type $leftType, Type $rightType): bool
@@ -89,36 +106,53 @@ private function isComparableTogether(Type $leftType, Type $rightType): bool
89106
$stringType = new StringType();
90107
$dateTimeType = new ObjectType(DateTimeInterface::class);
91108

92-
return ($this->containsOnlyTypes($leftType, [$intType, $floatType]) && $this->containsOnlyTypes($rightType, [$intType, $floatType]))
93-
|| ($this->containsOnlyTypes($leftType, [$stringType]) && $this->containsOnlyTypes($rightType, [$stringType]))
94-
|| ($this->containsOnlyTypes($leftType, [$dateTimeType]) && $this->containsOnlyTypes($rightType, [$dateTimeType]));
95-
}
109+
if ($this->containsOnlyTypes($leftType, [$intType, $floatType])) {
110+
return $this->containsOnlyTypes($rightType, [$intType, $floatType]);
111+
}
96112

97-
/**
98-
* @param Type[] $allowedTypes
99-
*/
100-
private function containsOnlyTypes(Type $checkedType, array $allowedTypes): bool
101-
{
102-
$typesToCheck = $checkedType instanceof UnionType
103-
? $checkedType->getTypes()
104-
: [$checkedType];
113+
if ($this->containsOnlyTypes($leftType, [$stringType])) {
114+
return $this->containsOnlyTypes($rightType, [$stringType]);
115+
}
105116

106-
foreach ($typesToCheck as $typeToCheck) {
107-
$isWithinAllowed = false;
117+
if ($this->containsOnlyTypes($leftType, [$dateTimeType])) {
118+
return $this->containsOnlyTypes($rightType, [$dateTimeType]);
119+
}
108120

109-
foreach ($allowedTypes as $allowedType) {
110-
if ($allowedType->isSuperTypeOf($typeToCheck)->yes()) {
111-
$isWithinAllowed = true;
112-
break;
113-
}
121+
if ($leftType->isConstantArray()->yes()) {
122+
if (!$rightType->isConstantArray()->yes()) {
123+
return false;
114124
}
115125

116-
if (!$isWithinAllowed) {
117-
return false;
126+
foreach ($leftType->getConstantArrays() as $leftConstantArray) {
127+
foreach ($rightType->getConstantArrays() as $rightConstantArray) {
128+
$leftValueTypes = $leftConstantArray->getValueTypes();
129+
$rightValueTypes = $rightConstantArray->getValueTypes();
130+
131+
if (count($leftValueTypes) !== count($rightValueTypes)) {
132+
return false;
133+
}
134+
135+
for ($i = 0; $i < count($leftValueTypes); $i++) {
136+
if (!$this->isComparableTogether($leftValueTypes[$i], $rightValueTypes[$i])) {
137+
return false;
138+
}
139+
}
140+
}
118141
}
142+
143+
return true;
119144
}
120145

121-
return true;
146+
return false;
147+
}
148+
149+
/**
150+
* @param Type[] $allowedTypes
151+
*/
152+
private function containsOnlyTypes(Type $checkedType, array $allowedTypes): bool
153+
{
154+
$allowedType = TypeCombinator::union(...$allowedTypes);
155+
return $allowedType->isSuperTypeOf($checkedType)->yes();
122156
}
123157

124158
}

‎tests/Rule/data/AllowComparingOnlyComparableTypesRule/code.php

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,23 +25,39 @@ interface Bar {}
2525
int|float $intOrFloat,
2626
float $float,
2727
bool $bool,
28+
mixed $mixed,
2829
) {
29-
$foos > $foo; // error: Comparison array > AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface is allowed.
30-
$nullableInt > $int; // error: Comparison int|null > int contains non-comparable type, only int|float|string|DateTimeInterface is allowed.
31-
null > $int; // error: Comparison null > int contains non-comparable type, only int|float|string|DateTimeInterface is allowed.
32-
$foo > $foo2; // error: Comparison AllowComparingOnlyComparableTypesRule\Foo > AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface is allowed.
33-
$foo > $fooOrBar; // error: Comparison AllowComparingOnlyComparableTypesRule\Foo > AllowComparingOnlyComparableTypesRule\Bar|AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface is allowed.
34-
$foo > $fooAndBar; // error: Comparison AllowComparingOnlyComparableTypesRule\Foo > AllowComparingOnlyComparableTypesRule\Bar&AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface is allowed.
30+
$foos > $foo; // error: Comparison array > AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
31+
$nullableInt > $int; // error: Comparison int|null > int contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
32+
null > $int; // error: Comparison null > int contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
33+
$foo > $foo2; // error: Comparison AllowComparingOnlyComparableTypesRule\Foo > AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
34+
$foo > $fooOrBar; // error: Comparison AllowComparingOnlyComparableTypesRule\Foo > AllowComparingOnlyComparableTypesRule\Bar|AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
35+
$foo > $fooAndBar; // error: Comparison AllowComparingOnlyComparableTypesRule\Foo > AllowComparingOnlyComparableTypesRule\Bar&AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
3536
$string > 'foo';
3637
$int > 2;
3738
$float > 2;
3839
$int > $intOrFloat;
3940
$string > $intOrFloat; // error: Cannot compare different types in string > float|int.
40-
$bool > true; // error: Comparison bool > true contains non-comparable type, only int|float|string|DateTimeInterface is allowed.
41+
$bool > true; // error: Comparison bool > true contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
4142
$dateTime > $dateTimeImmutable;
42-
$dateTime > $foo; // error: Comparison DateTime > AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface is allowed.
43+
$dateTime > $foo; // error: Comparison DateTime > AllowComparingOnlyComparableTypesRule\Foo contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
4344

4445
$string > $int; // error: Cannot compare different types in string > int.
4546
$float > $int;
4647
$dateTime > $string; // error: Cannot compare different types in DateTime > string.
48+
49+
[$int, $string] > [$int, $string];
50+
[[$int]] > [[$int]];
51+
[$int, $float, $intOrFloat, $intOrFloat] > [$int, $int, $int, $float];
52+
[$int, $string] > $foos; // error: Comparison array{int, string} > array contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
53+
[$int] > [$int, $int]; // error: Cannot compare different types in array{int} > array{int, int}.
54+
[$int, $string] > [$int]; // error: Cannot compare different types in array{int, string} > array{int}.
55+
[$string, $int] > [$int, $string]; // error: Cannot compare different types in array{string, int} > array{int, string}.
56+
[$foo] > [$foo]; // error: Comparison array{AllowComparingOnlyComparableTypesRule\Foo} > array{AllowComparingOnlyComparableTypesRule\Foo} contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
57+
[$int] > [$mixed]; // error: Comparison array{int} > array{mixed} contains non-comparable type, only int|float|string|DateTimeInterface or comparable tuple is allowed.
58+
[$dateTime] > [[$dateTimeImmutable]]; // error: Cannot compare different types in array{DateTime} > array{array{DateTimeImmutable}}.
59+
60+
[0 => $int, 1 => $string] > [$int, $string];
61+
[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.
62+
['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.
4763
};

0 commit comments

Comments
 (0)