From 5ee90b96e3e65c2a484724b5d567ad13c30d447a Mon Sep 17 00:00:00 2001
From: Jan Nedbal <janedbal@gmail.com>
Date: Wed, 29 May 2024 15:02:27 +0200
Subject: [PATCH 1/3] QueryResultTypeWalker: fix TypedExpression handling;
COUNT(x) is always int
---
.../Doctrine/Query/QueryResultTypeWalker.php | 25 +-------
.../Query/QueryResultTypeWalkerTest.php | 57 ++++++++++---------
2 files changed, 32 insertions(+), 50 deletions(-)
diff --git a/src/Type/Doctrine/Query/QueryResultTypeWalker.php b/src/Type/Doctrine/Query/QueryResultTypeWalker.php
index d68bcdb8..08fc3bb4 100644
--- a/src/Type/Doctrine/Query/QueryResultTypeWalker.php
+++ b/src/Type/Doctrine/Query/QueryResultTypeWalker.php
@@ -3,7 +3,6 @@
namespace PHPStan\Type\Doctrine\Query;
use BackedEnum;
-use Doctrine\DBAL\Types\Types;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Query;
@@ -816,28 +815,8 @@ public function walkSelectExpression($selectExpression): string
$resultAlias = $selectExpression->fieldIdentificationVariable ?? $this->scalarResultCounter++;
$type = $this->unmarshalType($expr->dispatch($this));
- if (class_exists(TypedExpression::class) && $expr instanceof TypedExpression) {
- $enforcedType = $this->resolveDoctrineType(Types::INTEGER);
- $type = TypeTraverser::map($type, static function (Type $type, callable $traverse) use ($enforcedType): Type {
- if ($type instanceof UnionType || $type instanceof IntersectionType) {
- return $traverse($type);
- }
- if ($type instanceof NullType) {
- return $type;
- }
- if ($enforcedType->accepts($type, true)->yes()) {
- return $type;
- }
- if ($enforcedType instanceof StringType) {
- if ($type instanceof IntegerType || $type instanceof FloatType) {
- return TypeCombinator::union($type->toString(), $type);
- }
- if ($type instanceof BooleanType) {
- return TypeCombinator::union($type->toInteger()->toString(), $type);
- }
- }
- return $enforcedType;
- });
+ if ($expr instanceof TypedExpression) {
+ $type = $this->resolveDoctrineType($expr->getReturnType()->getName(), null, TypeCombinator::containsNull($type));
} else {
// Expressions default to Doctrine's StringType, whose
// convertToPHPValue() is a no-op. So the actual type depends on
diff --git a/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php b/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php
index 983362c2..0c2b1af0 100644
--- a/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php
+++ b/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php
@@ -9,7 +9,6 @@
use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\Column;
-use Doctrine\ORM\Query\AST\TypedExpression;
use Doctrine\ORM\Tools\SchemaTool;
use PHPStan\Testing\PHPStanTestCase;
use PHPStan\Type\Accessory\AccessoryNumericStringType;
@@ -611,9 +610,7 @@ public function getTestData(): iterable
],
[
new ConstantIntegerType(3),
- $this->hasTypedExpressions()
- ? $this->uint()
- : $this->uintStringified(),
+ new IntegerType(),
],
[
new ConstantIntegerType(4),
@@ -621,9 +618,7 @@ public function getTestData(): iterable
],
[
new ConstantIntegerType(5),
- $this->hasTypedExpressions()
- ? $this->uint()
- : $this->uintStringified(),
+ new IntegerType(),
],
[
new ConstantIntegerType(6),
@@ -645,6 +640,29 @@ public function getTestData(): iterable
',
];
+ yield 'count' => [
+ $this->constantArray([
+ [
+ new ConstantIntegerType(1),
+ new IntegerType(),
+ ],
+ [
+ new ConstantIntegerType(2),
+ new IntegerType(),
+ ],
+ [
+ new ConstantIntegerType(3),
+ new IntegerType(),
+ ],
+ ]),
+ '
+ SELECT COUNT(m.stringNullColumn),
+ COUNT(m.stringColumn),
+ COUNT(m)
+ FROM QueryResult\Entities\Many m
+ ',
+ ];
+
yield 'aggregate lowercase' => [
$this->constantArray([
[
@@ -678,9 +696,7 @@ public function getTestData(): iterable
],
[
new ConstantStringType('count'),
- $this->hasTypedExpressions()
- ? $this->uint()
- : $this->uintStringified(),
+ new IntegerType(),
],
]),
'
@@ -1346,23 +1362,17 @@ public function getTestData(): iterable
$this->constantArray([
[
new ConstantIntegerType(1),
- $this->hasTypedExpressions()
- ? $this->uint()
- : $this->uintStringified(),
+ new IntegerType(),
],
[
new ConstantIntegerType(2),
TypeCombinator::addNull(
- $this->hasTypedExpressions()
- ? $this->uint()
- : $this->uintStringified()
+ new IntegerType()
),
],
[
new ConstantIntegerType(3),
- $this->hasTypedExpressions()
- ? $this->uint()
- : $this->uintStringified(),
+ new IntegerType(),
],
]),
'
@@ -1554,9 +1564,7 @@ public function getTestData(): iterable
[new ConstantIntegerType(1), TypeCombinator::addNull($this->numericStringOrInt())],
[
new ConstantIntegerType(2),
- $this->hasTypedExpressions()
- ? $this->uint()
- : $this->uintStringified(),
+ new IntegerType(),
],
]),
'
@@ -1678,11 +1686,6 @@ private function unumericStringified(): Type
);
}
- private function hasTypedExpressions(): bool
- {
- return class_exists(TypedExpression::class);
- }
-
/**
* @param array<mixed[]> $arrays
*
From 3049ecd6d0e5ba71de6ec62cd5a77bfe22a1f538 Mon Sep 17 00:00:00 2001
From: Jan Nedbal <janedbal@gmail.com>
Date: Wed, 29 May 2024 15:27:25 +0200
Subject: [PATCH 2/3] Ensure int range is kept
---
.../Doctrine/Query/QueryResultTypeWalker.php | 9 +++++--
.../Query/QueryResultTypeWalkerTest.php | 27 +++++++++++--------
2 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/src/Type/Doctrine/Query/QueryResultTypeWalker.php b/src/Type/Doctrine/Query/QueryResultTypeWalker.php
index 08fc3bb4..8694d3a6 100644
--- a/src/Type/Doctrine/Query/QueryResultTypeWalker.php
+++ b/src/Type/Doctrine/Query/QueryResultTypeWalker.php
@@ -360,9 +360,11 @@ public function walkFunction($function): string
case $function instanceof AST\Functions\MaxFunction:
case $function instanceof AST\Functions\MinFunction:
case $function instanceof AST\Functions\SumFunction:
- case $function instanceof AST\Functions\CountFunction:
return $function->getSql($this);
+ case $function instanceof AST\Functions\CountFunction:
+ return $this->marshalType(IntegerRangeType::fromInterval(0, null));
+
case $function instanceof AST\Functions\AbsFunction:
$exprType = $this->unmarshalType($this->walkSimpleArithmeticExpression($function->simpleArithmeticExpression));
@@ -816,7 +818,10 @@ public function walkSelectExpression($selectExpression): string
$type = $this->unmarshalType($expr->dispatch($this));
if ($expr instanceof TypedExpression) {
- $type = $this->resolveDoctrineType($expr->getReturnType()->getName(), null, TypeCombinator::containsNull($type));
+ $type = TypeCombinator::intersect(
+ $type,
+ $this->resolveDoctrineType($expr->getReturnType()->getName(), null, TypeCombinator::containsNull($type))
+ );
} else {
// Expressions default to Doctrine's StringType, whose
// convertToPHPValue() is a no-op. So the actual type depends on
diff --git a/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php b/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php
index 0c2b1af0..9801fd5f 100644
--- a/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php
+++ b/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php
@@ -610,7 +610,7 @@ public function getTestData(): iterable
],
[
new ConstantIntegerType(3),
- new IntegerType(),
+ $this->uint(),
],
[
new ConstantIntegerType(4),
@@ -618,7 +618,7 @@ public function getTestData(): iterable
],
[
new ConstantIntegerType(5),
- new IntegerType(),
+ $this->uint(),
],
[
new ConstantIntegerType(6),
@@ -644,15 +644,15 @@ public function getTestData(): iterable
$this->constantArray([
[
new ConstantIntegerType(1),
- new IntegerType(),
+ $this->uint(),
],
[
new ConstantIntegerType(2),
- new IntegerType(),
+ $this->uint(),
],
[
new ConstantIntegerType(3),
- new IntegerType(),
+ $this->uint(),
],
]),
'
@@ -696,7 +696,7 @@ public function getTestData(): iterable
],
[
new ConstantStringType('count'),
- new IntegerType(),
+ $this->uint(),
],
]),
'
@@ -1362,23 +1362,28 @@ public function getTestData(): iterable
$this->constantArray([
[
new ConstantIntegerType(1),
- new IntegerType(),
+ $this->uint(),
],
[
new ConstantIntegerType(2),
TypeCombinator::addNull(
- new IntegerType()
+ $this->uint()
),
],
[
new ConstantIntegerType(3),
- new IntegerType(),
+ $this->uint(),
+ ],
+ [
+ new ConstantIntegerType(4),
+ $this->uint(),
],
]),
'
SELECT LENGTH(m.stringColumn),
LENGTH(m.stringNullColumn),
- LENGTH(\'foo\')
+ LENGTH(\'foo\'),
+ LENGTH(COALESCE(m.stringNullColumn, \'\'))
FROM QueryResult\Entities\Many m
',
];
@@ -1564,7 +1569,7 @@ public function getTestData(): iterable
[new ConstantIntegerType(1), TypeCombinator::addNull($this->numericStringOrInt())],
[
new ConstantIntegerType(2),
- new IntegerType(),
+ $this->uint(),
],
]),
'
From 0c75dec1390e746366a7729155b117532d923ad6 Mon Sep 17 00:00:00 2001
From: Jan Nedbal <janedbal@gmail.com>
Date: Wed, 29 May 2024 15:36:02 +0200
Subject: [PATCH 3/3] Fix dbal v4
---
src/Type/Doctrine/Query/QueryResultTypeWalker.php | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/Type/Doctrine/Query/QueryResultTypeWalker.php b/src/Type/Doctrine/Query/QueryResultTypeWalker.php
index 8694d3a6..b053a696 100644
--- a/src/Type/Doctrine/Query/QueryResultTypeWalker.php
+++ b/src/Type/Doctrine/Query/QueryResultTypeWalker.php
@@ -3,6 +3,7 @@
namespace PHPStan\Type\Doctrine\Query;
use BackedEnum;
+use Doctrine\DBAL\Types\Type as DbalType;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Query;
@@ -820,7 +821,7 @@ public function walkSelectExpression($selectExpression): string
if ($expr instanceof TypedExpression) {
$type = TypeCombinator::intersect(
$type,
- $this->resolveDoctrineType($expr->getReturnType()->getName(), null, TypeCombinator::containsNull($type))
+ $this->resolveDoctrineType(DbalType::lookupName($expr->getReturnType()), null, TypeCombinator::containsNull($type))
);
} else {
// Expressions default to Doctrine's StringType, whose