diff --git a/src/Rules/Operators/InvalidComparisonOperationRule.php b/src/Rules/Operators/InvalidComparisonOperationRule.php index c4bfce8da1..b410ae0fb9 100644 --- a/src/Rules/Operators/InvalidComparisonOperationRule.php +++ b/src/Rules/Operators/InvalidComparisonOperationRule.php @@ -12,13 +12,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; @@ -59,7 +60,9 @@ 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) { return []; } @@ -80,10 +83,10 @@ public function processNode(Node $node, Scope $scope): array } 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) )) ) { @@ -113,45 +116,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(); } /** @return list */ diff --git a/tests/PHPStan/Rules/Operators/InvalidComparisonOperationRuleTest.php b/tests/PHPStan/Rules/Operators/InvalidComparisonOperationRuleTest.php index 40e6c1aae7..672a201ae1 100644 --- a/tests/PHPStan/Rules/Operators/InvalidComparisonOperationRuleTest.php +++ b/tests/PHPStan/Rules/Operators/InvalidComparisonOperationRuleTest.php @@ -14,10 +14,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), $this->getContainer()->getByType(OperatorTypeSpecifyingExtensionRegistryProvider::class), true, ); @@ -168,6 +170,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; + } +}