Skip to content

Commit 92d6809

Browse files
Fix InvalidComparisonOperationRule
1 parent d81cb77 commit 92d6809

File tree

3 files changed

+74
-39
lines changed

3 files changed

+74
-39
lines changed

src/Rules/Operators/InvalidComparisonOperationRule.php

Lines changed: 14 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,14 @@
99
use PHPStan\Rules\RuleErrorBuilder;
1010
use PHPStan\Rules\RuleLevelHelper;
1111
use PHPStan\ShouldNotHappenException;
12-
use PHPStan\Type\BenevolentUnionType;
12+
use PHPStan\Type\ArrayType;
1313
use PHPStan\Type\ErrorType;
1414
use PHPStan\Type\FloatType;
1515
use PHPStan\Type\IntegerType;
16+
use PHPStan\Type\MixedType;
17+
use PHPStan\Type\NullType;
1618
use PHPStan\Type\ObjectWithoutClassType;
1719
use PHPStan\Type\Type;
18-
use PHPStan\Type\TypeCombinator;
1920
use PHPStan\Type\UnionType;
2021
use PHPStan\Type\VerbosityLevel;
2122
use function get_class;
@@ -51,15 +52,17 @@ public function processNode(Node $node, Scope $scope): array
5152
return [];
5253
}
5354

54-
if ($this->isNumberType($scope, $node->left) && $this->isNumberType($scope, $node->right)) {
55+
$isLeftNumberType = $this->isNumberType($scope, $node->left);
56+
$isRightNumberType = $this->isNumberType($scope, $node->right);
57+
if (($isLeftNumberType && $isRightNumberType) || (!$isLeftNumberType && !$isRightNumberType)) {
5558
return [];
5659
}
5760

5861
if (
59-
($this->isNumberType($scope, $node->left) && (
62+
($isLeftNumberType && (
6063
$this->isPossiblyNullableObjectType($scope, $node->right) || $this->isPossiblyNullableArrayType($scope, $node->right)
6164
))
62-
|| ($this->isNumberType($scope, $node->right) && (
65+
|| ($isRightNumberType && (
6366
$this->isPossiblyNullableObjectType($scope, $node->left) || $this->isPossiblyNullableArrayType($scope, $node->left)
6467
))
6568
) {
@@ -125,45 +128,18 @@ private function isNumberType(Scope $scope, Node\Expr $expr): bool
125128

126129
private function isPossiblyNullableObjectType(Scope $scope, Node\Expr $expr): bool
127130
{
128-
$acceptedType = new ObjectWithoutClassType();
131+
$type = $scope->getType($expr);
132+
$acceptedType = new UnionType([new ObjectWithoutClassType(), new NullType()]);
129133

130-
$type = $this->ruleLevelHelper->findTypeToCheck(
131-
$scope,
132-
$expr,
133-
'',
134-
static fn (Type $type): bool => $acceptedType->isSuperTypeOf($type)->yes(),
135-
)->getType();
136-
137-
if ($type instanceof ErrorType) {
138-
return false;
139-
}
140-
141-
if (TypeCombinator::containsNull($type) && !$type->isNull()->yes()) {
142-
$type = TypeCombinator::removeNull($type);
143-
}
144-
145-
$isSuperType = $acceptedType->isSuperTypeOf($type);
146-
if ($type instanceof BenevolentUnionType) {
147-
return !$isSuperType->no();
148-
}
149-
150-
return $isSuperType->yes();
134+
return !$type->isNull()->yes() && $acceptedType->isSuperTypeOf($type)->yes();
151135
}
152136

153137
private function isPossiblyNullableArrayType(Scope $scope, Node\Expr $expr): bool
154138
{
155-
$type = $this->ruleLevelHelper->findTypeToCheck(
156-
$scope,
157-
$expr,
158-
'',
159-
static fn (Type $type): bool => $type->isArray()->yes(),
160-
)->getType();
161-
162-
if (TypeCombinator::containsNull($type) && !$type->isNull()->yes()) {
163-
$type = TypeCombinator::removeNull($type);
164-
}
139+
$type = $scope->getType($expr);
140+
$acceptedType = new UnionType([new ArrayType(new MixedType(), new MixedType()), new NullType()]);
165141

166-
return !($type instanceof ErrorType) && $type->isArray()->yes();
142+
return !$type->isNull()->yes() && $acceptedType->isSuperTypeOf($type)->yes();
167143
}
168144

169145
}

tests/PHPStan/Rules/Operators/InvalidComparisonOperationRuleTest.php

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@
1313
class InvalidComparisonOperationRuleTest extends RuleTestCase
1414
{
1515

16+
private bool $checkUnion = true;
17+
1618
protected function getRule(): Rule
1719
{
1820
return new InvalidComparisonOperationRule(
19-
new RuleLevelHelper(self::createReflectionProvider(), true, false, true, false, false, false, true),
21+
new RuleLevelHelper(self::createReflectionProvider(), true, false, $this->checkUnion, false, false, false, true),
2022
);
2123
}
2224

@@ -165,6 +167,21 @@ public function testRuleWithNullsafeVariant(): void
165167
]);
166168
}
167169

170+
public function testBug3364(): void
171+
{
172+
$this->checkUnion = false;
173+
$this->analyse([__DIR__ . '/data/bug-3364.php'], [
174+
[
175+
'Comparison operation "!=" between array<int|string>|null and 1 results in an error.',
176+
18,
177+
],
178+
[
179+
'Comparison operation "!=" between object|null and 1 results in an error.',
180+
26,
181+
],
182+
]);
183+
}
184+
168185
public function testBug11119(): void
169186
{
170187
$this->analyse([__DIR__ . '/data/bug-11119.php'], []);
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug3364;
4+
5+
class HelloWorld
6+
{
7+
/**
8+
* @param string|array<int|string>|null $value
9+
*/
10+
public function transform($value) {
11+
$value != 1;
12+
}
13+
14+
/**
15+
* @param array<int|string>|null $value
16+
*/
17+
public function transform2($value) {
18+
$value != 1;
19+
}
20+
21+
22+
/**
23+
* @param object|null $value
24+
*/
25+
public function transform3($value) {
26+
$value != 1;
27+
}
28+
29+
/**
30+
* @param array<int|string>|object $value
31+
*/
32+
public function transform4($value) {
33+
$value != 1;
34+
}
35+
36+
/**
37+
* @param null $value
38+
*/
39+
public function transform5($value) {
40+
$value != 1;
41+
}
42+
}

0 commit comments

Comments
 (0)