Fix InvalidComparisonOperationRule

Author: VincentLanglet

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) || (!$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<IdentifierRuleError> */

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<int|string>|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'], []);

tests/PHPStan/Rules/Operators/data/bug-3364.php

@@ -0,0 +1,42 @@
+<?php declare(strict_types = 1);
+
+namespace Bug3364;
+
+class HelloWorld
+{
+	/**
+	 * @param string|array<int|string>|null $value
+	 */
+	public function transform($value) {
+		$value != 1;
+	}
+
+	/**
+	 * @param array<int|string>|null $value
+	 */
+	public function transform2($value) {
+		$value != 1;
+	}
+
+
+	/**
+	 * @param object|null $value
+	 */
+	public function transform3($value) {
+		$value != 1;
+	}
+
+	/**
+	 * @param array<int|string>|object $value
+	 */
+	public function transform4($value) {
+		$value != 1;
+	}
+
+	/**
+	 * @param null $value
+	 */
+	public function transform5($value) {
+		$value != 1;
+	}
+}