Fix wrong coalesce type inference

phpstan · Jun 25, 2024 · 3d10eab · 3d10eab
1 parent 3369068
commit 3d10eab
Show file tree

Hide file tree

Showing 2 changed files with 99 additions and 16 deletions.
diff --git a/src/Type/Doctrine/Query/QueryResultTypeWalker.php b/src/Type/Doctrine/Query/QueryResultTypeWalker.php
@@ -14,6 +14,7 @@
 use Doctrine\ORM\Query\ParserResult;
 use Doctrine\ORM\Query\SqlWalker;
 use PHPStan\ShouldNotHappenException;
+use PHPStan\Type\Accessory\AccessoryNumericStringType;
 use PHPStan\Type\BooleanType;
 use PHPStan\Type\Constant\ConstantBooleanType;
 use PHPStan\Type\Constant\ConstantFloatType;
@@ -575,6 +576,79 @@ public function walkFunction($function): string
 		}
 	}
 
+	private function createFloat(bool $nullable): Type
+	{
+		$float = new FloatType();
+		return $nullable ? TypeCombinator::addNull($float) : $float;
+	}
+
+	private function createFloatOrInt(bool $nullable): Type // @phpstan-ignore-line unused, but kept to ease conflict resolution with origin (#506)
+	{
+		$union = TypeCombinator::union(
+			new FloatType(),
+			new IntegerType()
+		);
+		return $nullable ? TypeCombinator::addNull($union) : $union;
+	}
+
+	private function createInteger(bool $nullable): Type
+	{
+		$integer = new IntegerType();
+		return $nullable ? TypeCombinator::addNull($integer) : $integer;
+	}
+
+	private function createNonNegativeInteger(bool $nullable): Type // @phpstan-ignore-line unused, but kept to ease conflict resolution with origin (#506)
+	{
+		$integer = IntegerRangeType::fromInterval(0, null);
+		return $nullable ? TypeCombinator::addNull($integer) : $integer;
+	}
+
+	private function createNumericString(bool $nullable): Type
+	{
+		$numericString = TypeCombinator::intersect(
+			new StringType(),
+			new AccessoryNumericStringType()
+		);
+
+		return $nullable ? TypeCombinator::addNull($numericString) : $numericString;
+	}
+
+	private function createString(bool $nullable): Type
+	{
+		$string = new StringType();
+		return $nullable ? TypeCombinator::addNull($string) : $string;
+	}
+
+	/**
+	 * E.g. to ensure SUM(1) is inferred as int, not 1
+	 */
+	private function generalizeConstantType(Type $type, bool $makeNullable): Type
+	{
+		$containsNull = $this->canBeNull($type);
+		$typeNoNull = TypeCombinator::removeNull($type);
+
+		if (!$typeNoNull->isConstantScalarValue()->yes()) {
+			$result = $type;
+
+		} elseif ($typeNoNull->isInteger()->yes()) {
+			$result = $this->createInteger($containsNull);
+
+		} elseif ($typeNoNull->isFloat()->yes()) {
+			$result = $this->createFloat($containsNull);
+
+		} elseif ($typeNoNull->isNumericString()->yes()) {
+			$result = $this->createNumericString($containsNull);
+
+		} elseif ($typeNoNull->isString()->yes()) {
+			$result = $this->createString($containsNull);
+
+		} else {
+			$result = $type;
+		}
+
+		return $makeNullable ? TypeCombinator::addNull($result) : $result;
+	}
+
 	/**
 	 * @param AST\OrderByClause $orderByClause
 	 */
@@ -642,7 +716,10 @@ public function walkCoalesceExpression($coalesceExpression): string
 			$type = $this->unmarshalType($expression->dispatch($this));
 			$allTypesContainNull = $allTypesContainNull && TypeCombinator::containsNull($type);
 
-			$expressionTypes[] = $type;
+			// Some drivers manipulate the types, lets avoid false positives by generalizing constant types
+			// e.g. sqlsrv: "COALESCE returns the data type of value with the highest precedence"
+			// e.g. mysql: COALESCE(1, 'foo') === '1' (undocumented? https://gist.github.com/jrunning/4535434)
+			$expressionTypes[] = $this->generalizeConstantType($type, false);
 		}
 
 		$type = TypeCombinator::union(...$expressionTypes);
@@ -1400,6 +1477,11 @@ private function resolveDatabaseInternalType(string $typeName, ?string $enumType
 		return $type;
 	}
 
+	private function canBeNull(Type $type): bool
+	{
+		return !$type->isSuperTypeOf(new NullType())->no();
+	}
+
 	private function toNumericOrNull(Type $type): Type
 	{
 		return TypeTraverser::map($type, static function (Type $type, callable $traverse): Type {

diff --git a/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php b/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php
@@ -548,26 +548,18 @@ public function getTestData(): iterable
 				$this->constantArray([
 					[
 						new ConstantIntegerType(1),
-						TypeCombinator::union(
-							new ConstantStringType('a'),
-							new ConstantStringType('b')
-						),
+						new StringType(),
 					],
 					[
 						new ConstantIntegerType(2),
 						TypeCombinator::union(
-							new ConstantIntegerType(1),
-							new ConstantIntegerType(2),
-							new ConstantStringType('1'),
-							new ConstantStringType('2')
+							$this->numericString(),
+							new IntegerType()
 						),
 					],
 					[
 						new ConstantIntegerType(3),
-						TypeCombinator::union(
-							new ConstantStringType('1'),
-							new ConstantStringType('2')
-						),
+						$this->numericString(),
 					],
 				]),
 				'
@@ -842,11 +834,20 @@ public function getTestData(): iterable
 					new ConstantIntegerType(3),
 					$this->intStringified(),
 				],
+				[
+					new ConstantIntegerType(4),
+					TypeCombinator::union(
+						new IntegerType(),
+						new FloatType(),
+						$this->numericString()
+					),
+				],
 			]),
 			'
 				SELECT		COALESCE(m.stringNullColumn, m.intColumn, false),
 							COALESCE(m.stringNullColumn, m.stringNullColumn),
-							COALESCE(NULLIF(m.intColumn, 1), 0)
+							COALESCE(NULLIF(m.intColumn, 1), 0),
+							COALESCE(1, 1.1)
 				FROM		QueryResult\Entities\Many m
 			',
 		];
@@ -1142,8 +1143,8 @@ public function getTestData(): iterable
 					[
 						new ConstantIntegerType(2),
 						TypeCombinator::union(
-							new ConstantIntegerType(1),
-							new ConstantStringType('1')
+							new IntegerType(),
+							$this->numericString()
 						),
 					],
 					[