From 92d6809afbb57b3e3a031e5caab1ea2074e0fc64 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 26 Jul 2025 23:35:51 +0200 Subject: [PATCH] Fix InvalidComparisonOperationRule --- .../InvalidComparisonOperationRule.php | 52 +++++-------------- .../InvalidComparisonOperationRuleTest.php | 19 ++++++- .../PHPStan/Rules/Operators/data/bug-3364.php | 42 +++++++++++++++ 3 files changed, 74 insertions(+), 39 deletions(-) create mode 100644 tests/PHPStan/Rules/Operators/data/bug-3364.php diff --git a/src/Rules/Operators/InvalidComparisonOperationRule.php b/src/Rules/Operators/InvalidComparisonOperationRule.php index e3a0af0e6f..8bd4d1bae3 100644 --- a/src/Rules/Operators/InvalidComparisonOperationRule.php +++ b/src/Rules/Operators/InvalidComparisonOperationRule.php @@ -9,13 +9,14 @@ use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Rules\RuleLevelHelper; use PHPStan\ShouldNotHappenException; -use PHPStan\Type\BenevolentUnionType; +use PHPStan\Type\ArrayType; use PHPStan\Type\ErrorType; use PHPStan\Type\FloatType; use PHPStan\Type\IntegerType; +use PHPStan\Type\MixedType; +use PHPStan\Type\NullType; use PHPStan\Type\ObjectWithoutClassType; use PHPStan\Type\Type; -use PHPStan\Type\TypeCombinator; use PHPStan\Type\UnionType; use PHPStan\Type\VerbosityLevel; use function get_class; @@ -51,15 +52,17 @@ public function processNode(Node $node, Scope $scope): array return []; } - if ($this->isNumberType($scope, $node->left) && $this->isNumberType($scope, $node->right)) { + $isLeftNumberType = $this->isNumberType($scope, $node->left); + $isRightNumberType = $this->isNumberType($scope, $node->right); + if (($isLeftNumberType && $isRightNumberType) || (!$isLeftNumberType && !$isRightNumberType)) { return []; } if ( - ($this->isNumberType($scope, $node->left) && ( + ($isLeftNumberType && ( $this->isPossiblyNullableObjectType($scope, $node->right) || $this->isPossiblyNullableArrayType($scope, $node->right) )) - || ($this->isNumberType($scope, $node->right) && ( + || ($isRightNumberType && ( $this->isPossiblyNullableObjectType($scope, $node->left) || $this->isPossiblyNullableArrayType($scope, $node->left) )) ) { @@ -125,45 +128,18 @@ private function isNumberType(Scope $scope, Node\Expr $expr): bool private function isPossiblyNullableObjectType(Scope $scope, Node\Expr $expr): bool { - $acceptedType = new ObjectWithoutClassType(); + $type = $scope->getType($expr); + $acceptedType = new UnionType([new ObjectWithoutClassType(), new NullType()]); - $type = $this->ruleLevelHelper->findTypeToCheck( - $scope, - $expr, - '', - static fn (Type $type): bool => $acceptedType->isSuperTypeOf($type)->yes(), - )->getType(); - - if ($type instanceof ErrorType) { - return false; - } - - if (TypeCombinator::containsNull($type) && !$type->isNull()->yes()) { - $type = TypeCombinator::removeNull($type); - } - - $isSuperType = $acceptedType->isSuperTypeOf($type); - if ($type instanceof BenevolentUnionType) { - return !$isSuperType->no(); - } - - return $isSuperType->yes(); + return !$type->isNull()->yes() && $acceptedType->isSuperTypeOf($type)->yes(); } private function isPossiblyNullableArrayType(Scope $scope, Node\Expr $expr): bool { - $type = $this->ruleLevelHelper->findTypeToCheck( - $scope, - $expr, - '', - static fn (Type $type): bool => $type->isArray()->yes(), - )->getType(); - - if (TypeCombinator::containsNull($type) && !$type->isNull()->yes()) { - $type = TypeCombinator::removeNull($type); - } + $type = $scope->getType($expr); + $acceptedType = new UnionType([new ArrayType(new MixedType(), new MixedType()), new NullType()]); - return !($type instanceof ErrorType) && $type->isArray()->yes(); + return !$type->isNull()->yes() && $acceptedType->isSuperTypeOf($type)->yes(); } } diff --git a/tests/PHPStan/Rules/Operators/InvalidComparisonOperationRuleTest.php b/tests/PHPStan/Rules/Operators/InvalidComparisonOperationRuleTest.php index f8163f171b..6d346dd2a2 100644 --- a/tests/PHPStan/Rules/Operators/InvalidComparisonOperationRuleTest.php +++ b/tests/PHPStan/Rules/Operators/InvalidComparisonOperationRuleTest.php @@ -13,10 +13,12 @@ class InvalidComparisonOperationRuleTest extends RuleTestCase { + private bool $checkUnion = true; + protected function getRule(): Rule { return new InvalidComparisonOperationRule( - new RuleLevelHelper(self::createReflectionProvider(), true, false, true, false, false, false, true), + new RuleLevelHelper(self::createReflectionProvider(), true, false, $this->checkUnion, false, false, false, true), ); } @@ -165,6 +167,21 @@ public function testRuleWithNullsafeVariant(): void ]); } + public function testBug3364(): void + { + $this->checkUnion = false; + $this->analyse([__DIR__ . '/data/bug-3364.php'], [ + [ + 'Comparison operation "!=" between array|null and 1 results in an error.', + 18, + ], + [ + 'Comparison operation "!=" between object|null and 1 results in an error.', + 26, + ], + ]); + } + public function testBug11119(): void { $this->analyse([__DIR__ . '/data/bug-11119.php'], []); diff --git a/tests/PHPStan/Rules/Operators/data/bug-3364.php b/tests/PHPStan/Rules/Operators/data/bug-3364.php new file mode 100644 index 0000000000..764b8c0b96 --- /dev/null +++ b/tests/PHPStan/Rules/Operators/data/bug-3364.php @@ -0,0 +1,42 @@ +|null $value + */ + public function transform($value) { + $value != 1; + } + + /** + * @param array|null $value + */ + public function transform2($value) { + $value != 1; + } + + + /** + * @param object|null $value + */ + public function transform3($value) { + $value != 1; + } + + /** + * @param array|object $value + */ + public function transform4($value) { + $value != 1; + } + + /** + * @param null $value + */ + public function transform5($value) { + $value != 1; + } +}