From f4ec555b598094883626e085009796bbf14b7d29 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Wed, 5 Apr 2023 19:58:58 +0200 Subject: [PATCH 1/8] Use benevolent union for scalar in queries --- .../Doctrine/Query/QueryResultTypeWalker.php | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Type/Doctrine/Query/QueryResultTypeWalker.php b/src/Type/Doctrine/Query/QueryResultTypeWalker.php index c98604b2..66fe7cb5 100644 --- a/src/Type/Doctrine/Query/QueryResultTypeWalker.php +++ b/src/Type/Doctrine/Query/QueryResultTypeWalker.php @@ -794,7 +794,7 @@ public function walkSelectExpression($selectExpression) $type = $this->resolveDoctrineType($typeName, $enumType, $nullable); - $this->typeBuilder->addScalar($resultAlias, $type); + $this->addScalar($resultAlias, $type); return ''; } @@ -854,7 +854,7 @@ public function walkSelectExpression($selectExpression) }); } - $this->typeBuilder->addScalar($resultAlias, $type); + $this->addScalar($resultAlias, $type); return ''; } @@ -1275,6 +1275,18 @@ public function walkResultVariable($resultVariable) return $this->marshalType(new MixedType()); } + /** + * @param array-key $alias + */ + private function addScalar($alias, Type $type): void + { + if ($type instanceof UnionType) { + $type = TypeUtils::toBenevolentUnion($type); + } + + $this->typeBuilder->addScalar($alias, $type); + } + private function unmarshalType(string $marshalledType): Type { $type = unserialize($marshalledType); From b4c31521d4e8eb4418b95f2b0404c1135800ac71 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Wed, 5 Apr 2023 21:04:39 +0200 Subject: [PATCH 2/8] Only use benevolent when where clause is used --- src/Type/Doctrine/Query/QueryResultTypeWalker.php | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/Type/Doctrine/Query/QueryResultTypeWalker.php b/src/Type/Doctrine/Query/QueryResultTypeWalker.php index 66fe7cb5..c7243183 100644 --- a/src/Type/Doctrine/Query/QueryResultTypeWalker.php +++ b/src/Type/Doctrine/Query/QueryResultTypeWalker.php @@ -106,6 +106,9 @@ class QueryResultTypeWalker extends SqlWalker /** @var bool */ private $hasGroupByClause; + /** @var bool */ + private $hasCondition; + /** * @param Query $query */ @@ -134,6 +137,7 @@ public function __construct($query, $parserResult, array $queryComponents) $this->nullableQueryComponents = []; $this->hasAggregateFunction = false; $this->hasGroupByClause = false; + $this->hasCondition = false; // The object is instantiated by Doctrine\ORM\Query\Parser, so receiving // dependencies through the constructor is not an option. Instead, we @@ -589,6 +593,8 @@ public function walkOrderByItem($orderByItem) */ public function walkHavingClause($havingClause) { + $this->hasCondition = true; + return $this->marshalType(new MixedType()); } @@ -993,6 +999,8 @@ public function walkUpdateItem($updateItem) */ public function walkWhereClause($whereClause) { + $this->hasCondition = true; + return $this->marshalType(new MixedType()); } @@ -1280,7 +1288,10 @@ public function walkResultVariable($resultVariable) */ private function addScalar($alias, Type $type): void { - if ($type instanceof UnionType) { + // Since we don't check the condition inside the WHERE or HAVING + // conditions, we cannot be sure all the union types are correct. + // For exemple, a condition `WHERE foo.bar IS NOT NULL` could be added. + if ($this->hasCondition && $type instanceof UnionType) { $type = TypeUtils::toBenevolentUnion($type); } From 7ca3bc14fd9130fbef01c56208e7d8e7456cb9e3 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Wed, 5 Apr 2023 21:09:29 +0200 Subject: [PATCH 3/8] Use benevolent union when we cast values --- src/Type/Doctrine/Query/QueryResultTypeWalker.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Type/Doctrine/Query/QueryResultTypeWalker.php b/src/Type/Doctrine/Query/QueryResultTypeWalker.php index c7243183..8e390376 100644 --- a/src/Type/Doctrine/Query/QueryResultTypeWalker.php +++ b/src/Type/Doctrine/Query/QueryResultTypeWalker.php @@ -846,18 +846,27 @@ public function walkSelectExpression($selectExpression) // the driver and PHP version. // Here we assume that the value may or may not be casted to // string by the driver. - $type = TypeTraverser::map($type, static function (Type $type, callable $traverse): Type { + $casted = false; + $type = TypeTraverser::map($type, static function (Type $type, callable $traverse) use (&$casted): Type { if ($type instanceof UnionType || $type instanceof IntersectionType) { return $traverse($type); } if ($type instanceof IntegerType || $type instanceof FloatType) { + $casted = true; return TypeCombinator::union($type->toString(), $type); } if ($type instanceof BooleanType) { + $casted = true; return TypeCombinator::union($type->toInteger()->toString(), $type); } return $traverse($type); }); + + // Since we made supposition about possibly casted values, + // we can only provide a benevolent union. + if ($casted && $type instanceof UnionType) { + $type = TypeUtils::toBenevolentUnion($type); + } } $this->addScalar($resultAlias, $type); From 4fec09dc9c1b9cde6f9ed9349c2b5b65e841a623 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Thu, 6 Apr 2023 09:29:32 +0200 Subject: [PATCH 4/8] Fix tests --- .../Query/QueryResultTypeWalkerTest.php | 186 ++++++++++-------- 1 file changed, 100 insertions(+), 86 deletions(-) diff --git a/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php b/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php index e880b56a..dc717be9 100644 --- a/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php +++ b/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php @@ -13,6 +13,7 @@ use Doctrine\ORM\Tools\SchemaTool; use PHPStan\Testing\PHPStanTestCase; use PHPStan\Type\Accessory\AccessoryNumericStringType; +use PHPStan\Type\BenevolentUnionType; use PHPStan\Type\Constant\ConstantArrayTypeBuilder; use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\Constant\ConstantStringType; @@ -28,6 +29,7 @@ use PHPStan\Type\StringType; use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; +use PHPStan\Type\TypeUtils; use PHPStan\Type\UnionType; use PHPStan\Type\VerbosityLevel; use QueryResult\Entities\Embedded; @@ -543,12 +545,12 @@ public function getTestData(): iterable ], [ new ConstantIntegerType(2), - TypeCombinator::union( + TypeUtils::toBenevolentUnion(TypeCombinator::union( new ConstantIntegerType(1), new ConstantIntegerType(2), new ConstantStringType('1'), new ConstantStringType('2') - ), + )), ], [ new ConstantIntegerType(3), @@ -595,7 +597,7 @@ public function getTestData(): iterable ], [ new ConstantIntegerType(1), - TypeCombinator::addNull($this->intStringified()), + $this->intStringified(true), ], [ new ConstantIntegerType(2), @@ -609,7 +611,7 @@ public function getTestData(): iterable ], [ new ConstantIntegerType(4), - TypeCombinator::addNull($this->intStringified()), + $this->intStringified(true), ], [ new ConstantIntegerType(5), @@ -619,7 +621,7 @@ public function getTestData(): iterable ], [ new ConstantIntegerType(6), - TypeCombinator::addNull($this->intStringified()), + $this->intStringified(true), ], [ new ConstantIntegerType(7), @@ -645,7 +647,7 @@ public function getTestData(): iterable ], [ new ConstantStringType('max'), - TypeCombinator::addNull($this->intStringified()), + $this->intStringified(true), ], [ new ConstantStringType('arithmetic'), @@ -677,10 +679,10 @@ public function getTestData(): iterable $this->constantArray([ [ new ConstantIntegerType(1), - TypeCombinator::union( + TypeUtils::toBenevolentUnion(TypeCombinator::union( new ConstantStringType('1'), new ConstantIntegerType(1) - ), + )), ], [new ConstantIntegerType(2), new ConstantStringType('hello')], ]), @@ -694,11 +696,11 @@ public function getTestData(): iterable $this->constantArray([ [ new ConstantIntegerType(1), - TypeCombinator::union( + TypeUtils::toBenevolentUnion(TypeCombinator::union( new ConstantIntegerType(1), new ConstantStringType('1'), new NullType() - ), + )), ], ]), ' @@ -711,10 +713,10 @@ public function getTestData(): iterable $this->constantArray([ [ new ConstantIntegerType(1), - TypeCombinator::union( + TypeUtils::toBenevolentUnion(TypeCombinator::union( new StringType(), new IntegerType() - ), + )), ], [ new ConstantIntegerType(2), @@ -740,10 +742,10 @@ public function getTestData(): iterable $this->constantArray([ [ new ConstantIntegerType(1), - TypeCombinator::union( + TypeUtils::toBenevolentUnion(TypeCombinator::union( new StringType(), new ConstantIntegerType(0) - ), + )), ], ]), ' @@ -760,10 +762,10 @@ public function getTestData(): iterable $this->constantArray([ [ new ConstantIntegerType(1), - TypeCombinator::union( + TypeUtils::toBenevolentUnion(TypeCombinator::union( new StringType(), new ConstantIntegerType(0) - ), + )), ], ]), ' @@ -780,12 +782,12 @@ public function getTestData(): iterable $this->constantArray([ [ new ConstantIntegerType(1), - TypeCombinator::union( + TypeUtils::toBenevolentUnion(TypeCombinator::union( new ConstantIntegerType(0), new ConstantIntegerType(1), new ConstantStringType('0'), new ConstantStringType('1') - ), + )), ], ]), ' @@ -801,12 +803,12 @@ public function getTestData(): iterable $this->constantArray([ [ new ConstantIntegerType(1), - TypeCombinator::union( + TypeUtils::toBenevolentUnion(TypeCombinator::union( new ConstantIntegerType(0), new ConstantIntegerType(1), new ConstantStringType('0'), new ConstantStringType('1') - ), + )), ], ]), ' @@ -822,31 +824,31 @@ public function getTestData(): iterable $this->constantArray([ [ new ConstantIntegerType(1), - TypeCombinator::union( + TypeUtils::toBenevolentUnion(TypeCombinator::union( new ConstantIntegerType(1), new ConstantStringType('1') - ), + )), ], [ new ConstantIntegerType(2), - TypeCombinator::union( + TypeUtils::toBenevolentUnion(TypeCombinator::union( new ConstantIntegerType(0), new ConstantStringType('0') - ), + )), ], [ new ConstantIntegerType(3), - TypeCombinator::union( + TypeUtils::toBenevolentUnion(TypeCombinator::union( new ConstantIntegerType(1), new ConstantStringType('1') - ), + )), ], [ new ConstantIntegerType(4), - TypeCombinator::union( + TypeUtils::toBenevolentUnion(TypeCombinator::union( new ConstantIntegerType(0), new ConstantStringType('0') - ), + )), ], ]), ' @@ -1018,10 +1020,10 @@ public function getTestData(): iterable ], [ new ConstantIntegerType(2), - TypeCombinator::union( + TypeUtils::toBenevolentUnion(TypeCombinator::union( new ConstantIntegerType(1), new ConstantStringType('1') - ), + )), ], [ new ConstantStringType('intColumn'), @@ -1065,7 +1067,7 @@ public function getTestData(): iterable yield 'new arguments affect scalar counter' => [ $this->constantArray([ - [new ConstantIntegerType(5), TypeCombinator::addNull($this->intStringified())], + [new ConstantIntegerType(5), $this->intStringified(true)], [new ConstantIntegerType(0), new ObjectType(ManyId::class)], [new ConstantIntegerType(1), new ObjectType(OneId::class)], ]), @@ -1082,7 +1084,7 @@ public function getTestData(): iterable [new ConstantStringType('intColumn'), new IntegerType()], [new ConstantIntegerType(1), $this->intStringified()], [new ConstantIntegerType(2), $this->intStringified()], - [new ConstantIntegerType(3), TypeCombinator::addNull($this->intStringified())], + [new ConstantIntegerType(3), $this->intStringified(true)], [new ConstantIntegerType(4), $this->intStringified()], [new ConstantIntegerType(5), $this->intStringified()], [new ConstantIntegerType(6), $this->numericStringified()], @@ -1104,9 +1106,9 @@ public function getTestData(): iterable yield 'abs function' => [ $this->constantArray([ [new ConstantIntegerType(1), $this->unumericStringified()], - [new ConstantIntegerType(2), TypeCombinator::addNull($this->unumericStringified())], + [new ConstantIntegerType(2), $this->unumericStringified(true)], [new ConstantIntegerType(3), $this->unumericStringified()], - [new ConstantIntegerType(4), TypeCombinator::union($this->unumericStringified())], + [new ConstantIntegerType(4), $this->unumericStringified()], ]), ' SELECT ABS(m.intColumn), @@ -1120,7 +1122,7 @@ public function getTestData(): iterable yield 'bit_and function' => [ $this->constantArray([ [new ConstantIntegerType(1), $this->uintStringified()], - [new ConstantIntegerType(2), TypeCombinator::addNull($this->uintStringified())], + [new ConstantIntegerType(2), $this->uintStringified(true)], [new ConstantIntegerType(3), $this->uintStringified()], ]), ' @@ -1134,7 +1136,7 @@ public function getTestData(): iterable yield 'bit_or function' => [ $this->constantArray([ [new ConstantIntegerType(1), $this->uintStringified()], - [new ConstantIntegerType(2), TypeCombinator::addNull($this->uintStringified())], + [new ConstantIntegerType(2), $this->uintStringified(true)], [new ConstantIntegerType(3), $this->uintStringified()], ]), ' @@ -1210,8 +1212,8 @@ public function getTestData(): iterable yield 'date_diff function' => [ $this->constantArray([ [new ConstantIntegerType(1), $this->numericStringified()], - [new ConstantIntegerType(2), TypeCombinator::addNull($this->numericStringified())], - [new ConstantIntegerType(3), TypeCombinator::addNull($this->numericStringified())], + [new ConstantIntegerType(2), $this->numericStringified(true)], + [new ConstantIntegerType(3), $this->numericStringified(true)], [new ConstantIntegerType(4), $this->numericStringified()], ]), ' @@ -1253,11 +1255,9 @@ public function getTestData(): iterable ], [ new ConstantIntegerType(2), - TypeCombinator::addNull( - $this->hasTypedExpressions() - ? $this->uint() - : $this->uintStringified() - ), + $this->hasTypedExpressions() + ? $this->uint(true) + : $this->uintStringified(true) ], [ new ConstantIntegerType(3), @@ -1278,8 +1278,8 @@ public function getTestData(): iterable yield 'locate function' => [ $this->constantArray([ [new ConstantIntegerType(1), $this->uintStringified()], - [new ConstantIntegerType(2), TypeCombinator::addNull($this->uintStringified())], - [new ConstantIntegerType(3), TypeCombinator::addNull($this->uintStringified())], + [new ConstantIntegerType(2), $this->uintStringified(true)], + [new ConstantIntegerType(3), $this->uintStringified(true)], [new ConstantIntegerType(4), $this->uintStringified()], ]), ' @@ -1321,8 +1321,8 @@ public function getTestData(): iterable yield 'mod function' => [ $this->constantArray([ [new ConstantIntegerType(1), $this->uintStringified()], - [new ConstantIntegerType(2), TypeCombinator::addNull($this->uintStringified())], - [new ConstantIntegerType(3), TypeCombinator::addNull($this->uintStringified())], + [new ConstantIntegerType(2), $this->uintStringified(true)], + [new ConstantIntegerType(3), $this->uintStringified(true)], [new ConstantIntegerType(4), $this->uintStringified()], ]), ' @@ -1336,7 +1336,7 @@ public function getTestData(): iterable yield 'mod function error' => [ $this->constantArray([ - [new ConstantIntegerType(1), TypeCombinator::addNull($this->uintStringified())], + [new ConstantIntegerType(1), $this->uintStringified(true)], ]), ' SELECT MOD(10, NULLIF(m.intColumn, m.intColumn)) @@ -1395,15 +1395,15 @@ public function getTestData(): iterable yield 'identity function' => [ $this->constantArray([ - [new ConstantIntegerType(1), TypeCombinator::addNull($this->numericStringOrInt())], - [new ConstantIntegerType(2), $this->numericStringOrInt()], - [new ConstantIntegerType(3), TypeCombinator::addNull($this->numericStringOrInt())], + [new ConstantIntegerType(1), $this->intStringified(true)], + [new ConstantIntegerType(2), $this->intStringified()], + [new ConstantIntegerType(3), $this->intStringified(true)], [new ConstantIntegerType(4), TypeCombinator::addNull(new StringType())], [new ConstantIntegerType(5), TypeCombinator::addNull(new StringType())], - [new ConstantIntegerType(6), TypeCombinator::addNull($this->numericStringOrInt())], + [new ConstantIntegerType(6), $this->intStringified(true)], [new ConstantIntegerType(7), TypeCombinator::addNull(new MixedType())], - [new ConstantIntegerType(8), TypeCombinator::addNull($this->numericStringOrInt())], - [new ConstantIntegerType(9), TypeCombinator::addNull($this->numericStringOrInt())], + [new ConstantIntegerType(8), $this->intStringified(true)], + [new ConstantIntegerType(9), $this->intStringified(true)], ]), ' SELECT IDENTITY(m.oneNull), @@ -1422,7 +1422,7 @@ public function getTestData(): iterable yield 'select nullable association' => [ $this->constantArray([ - [new ConstantIntegerType(1), TypeCombinator::addNull($this->numericStringOrInt())], + [new ConstantIntegerType(1), $this->intStringified(true)], ]), ' SELECT DISTINCT(m.oneNull) @@ -1432,7 +1432,7 @@ public function getTestData(): iterable yield 'select non null association' => [ $this->constantArray([ - [new ConstantIntegerType(1), $this->numericStringOrInt()], + [new ConstantIntegerType(1), $this->intStringified()], ]), ' SELECT DISTINCT(m.one) @@ -1442,7 +1442,7 @@ public function getTestData(): iterable yield 'select default nullability association' => [ $this->constantArray([ - [new ConstantIntegerType(1), TypeCombinator::addNull($this->numericStringOrInt())], + [new ConstantIntegerType(1), $this->intStringified(true)], ]), ' SELECT DISTINCT(m.oneDefaultNullability) @@ -1452,7 +1452,7 @@ public function getTestData(): iterable yield 'select non null association in aggregated query' => [ $this->constantArray([ - [new ConstantIntegerType(1), TypeCombinator::addNull($this->numericStringOrInt())], + [new ConstantIntegerType(1), $this->intStringified(true)], [ new ConstantIntegerType(2), $this->hasTypedExpressions() @@ -1522,17 +1522,6 @@ private function constantArray(array $elements): Type return $builder->getArray(); } - private function numericStringOrInt(): Type - { - return new UnionType([ - new IntegerType(), - new IntersectionType([ - new StringType(), - new AccessoryNumericStringType(), - ]), - ]); - } - private function numericString(): Type { return new IntersectionType([ @@ -1541,42 +1530,67 @@ private function numericString(): Type ]); } - private function uint(): Type + private function uint(bool $nullable = false): Type { - return IntegerRangeType::fromInterval(0, null); + $type = IntegerRangeType::fromInterval(0, null); + if ($nullable) { + TypeCombinator::addNull($type); + } + + return $type; } - private function intStringified(): Type + private function intStringified(bool $nullable = false): Type { - return TypeCombinator::union( + $types = [ new IntegerType(), - $this->numericString() - ); + $this->numericString(), + ]; + if ($nullable) { + $types[] = new NullType(); + } + + return TypeUtils::toBenevolentUnion(TypeCombinator::union(...$types)); } - private function uintStringified(): Type + private function uintStringified(bool $nullable = false): Type { - return TypeCombinator::union( + $types = [ $this->uint(), - $this->numericString() - ); + $this->numericString(), + ]; + if ($nullable) { + $types[] = new NullType(); + } + + return TypeUtils::toBenevolentUnion(TypeCombinator::union(...$types)); } - private function numericStringified(): Type + private function numericStringified(bool $nullable = false): Type { - return TypeCombinator::union( + $types = [ new FloatType(), new IntegerType(), - $this->numericString() - ); + $this->numericString(), + ]; + if ($nullable) { + $types[] = new NullType(); + } + + return TypeUtils::toBenevolentUnion(TypeCombinator::union(...$types)); } - private function unumericStringified(): Type + private function unumericStringified(bool $nullable = false): Type { - return TypeCombinator::union( + $types = [ new FloatType(), IntegerRangeType::fromInterval(0, null), $this->numericString() - ); + ]; + if ($nullable) { + $types[] = new NullType(); + } + + return TypeUtils::toBenevolentUnion(TypeCombinator::union(...$types)); } private function hasTypedExpressions(): bool From 4ac5a291a431e00c6914f1a33d3818eb2f6e6a53 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Thu, 6 Apr 2023 09:43:46 +0200 Subject: [PATCH 5/8] Avoid useless benevolent unions --- src/Type/Doctrine/Query/QueryResultTypeWalker.php | 4 +++- .../Doctrine/Query/QueryResultTypeWalkerTest.php | 12 ++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Type/Doctrine/Query/QueryResultTypeWalker.php b/src/Type/Doctrine/Query/QueryResultTypeWalker.php index 8e390376..2fd757d4 100644 --- a/src/Type/Doctrine/Query/QueryResultTypeWalker.php +++ b/src/Type/Doctrine/Query/QueryResultTypeWalker.php @@ -847,6 +847,8 @@ public function walkSelectExpression($selectExpression) // Here we assume that the value may or may not be casted to // string by the driver. $casted = false; + $originalType = $type; + $type = TypeTraverser::map($type, static function (Type $type, callable $traverse) use (&$casted): Type { if ($type instanceof UnionType || $type instanceof IntersectionType) { return $traverse($type); @@ -864,7 +866,7 @@ public function walkSelectExpression($selectExpression) // Since we made supposition about possibly casted values, // we can only provide a benevolent union. - if ($casted && $type instanceof UnionType) { + if ($casted && $type instanceof UnionType && !$originalType->equals($type)) { $type = TypeUtils::toBenevolentUnion($type); } } diff --git a/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php b/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php index dc717be9..c7c2d41c 100644 --- a/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php +++ b/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php @@ -713,10 +713,10 @@ public function getTestData(): iterable $this->constantArray([ [ new ConstantIntegerType(1), - TypeUtils::toBenevolentUnion(TypeCombinator::union( + TypeCombinator::union( new StringType(), new IntegerType() - )), + ), ], [ new ConstantIntegerType(2), @@ -742,10 +742,10 @@ public function getTestData(): iterable $this->constantArray([ [ new ConstantIntegerType(1), - TypeUtils::toBenevolentUnion(TypeCombinator::union( + TypeCombinator::union( new StringType(), new ConstantIntegerType(0) - )), + ), ], ]), ' @@ -762,10 +762,10 @@ public function getTestData(): iterable $this->constantArray([ [ new ConstantIntegerType(1), - TypeUtils::toBenevolentUnion(TypeCombinator::union( + TypeCombinator::union( new StringType(), new ConstantIntegerType(0) - )), + ), ], ]), ' From 2ef94e12302a46eec5186afc8e262a693309d6d9 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Thu, 6 Apr 2023 09:51:03 +0200 Subject: [PATCH 6/8] Fix more tests --- .../QueryResultDynamicReturnTypeExtension.php | 9 ++++++++- .../Doctrine/Query/QueryResultTypeWalkerTest.php | 6 ++---- .../Type/Doctrine/data/QueryResult/queryResult.php | 14 +++++++------- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/Type/Doctrine/Query/QueryResultDynamicReturnTypeExtension.php b/src/Type/Doctrine/Query/QueryResultDynamicReturnTypeExtension.php index 7d047768..96b3f7bc 100644 --- a/src/Type/Doctrine/Query/QueryResultDynamicReturnTypeExtension.php +++ b/src/Type/Doctrine/Query/QueryResultDynamicReturnTypeExtension.php @@ -10,6 +10,7 @@ use PHPStan\ShouldNotHappenException; use PHPStan\Type\Accessory\AccessoryArrayListType; use PHPStan\Type\ArrayType; +use PHPStan\Type\BenevolentUnionType; use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\Doctrine\ObjectMetadataResolver; use PHPStan\Type\DynamicMethodReturnTypeExtension; @@ -21,6 +22,7 @@ use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; use PHPStan\Type\TypeTraverser; +use PHPStan\Type\TypeUtils; use PHPStan\Type\TypeWithClassName; use PHPStan\Type\VoidType; use function count; @@ -156,7 +158,12 @@ private function getMethodReturnTypeForHydrationMode( case 'getSingleResult': return $queryResultType; case 'getOneOrNullResult': - return TypeCombinator::addNull($queryResultType); + $nullableQueryResultType = TypeCombinator::addNull($queryResultType); + if ($queryResultType instanceof BenevolentUnionType) { + $nullableQueryResultType = TypeUtils::toBenevolentUnion($nullableQueryResultType); + } + + return $nullableQueryResultType; case 'toIterable': return new IterableType( $queryKeyType->isNull()->yes() ? new IntegerType() : $queryKeyType, diff --git a/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php b/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php index c7c2d41c..b97aaa79 100644 --- a/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php +++ b/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php @@ -13,7 +13,6 @@ use Doctrine\ORM\Tools\SchemaTool; use PHPStan\Testing\PHPStanTestCase; use PHPStan\Type\Accessory\AccessoryNumericStringType; -use PHPStan\Type\BenevolentUnionType; use PHPStan\Type\Constant\ConstantArrayTypeBuilder; use PHPStan\Type\Constant\ConstantIntegerType; use PHPStan\Type\Constant\ConstantStringType; @@ -30,7 +29,6 @@ use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; use PHPStan\Type\TypeUtils; -use PHPStan\Type\UnionType; use PHPStan\Type\VerbosityLevel; use QueryResult\Entities\Embedded; use QueryResult\Entities\JoinedChild; @@ -1257,7 +1255,7 @@ public function getTestData(): iterable new ConstantIntegerType(2), $this->hasTypedExpressions() ? $this->uint(true) - : $this->uintStringified(true) + : $this->uintStringified(true), ], [ new ConstantIntegerType(3), @@ -1584,7 +1582,7 @@ private function unumericStringified(bool $nullable = false): Type $types = [ new FloatType(), IntegerRangeType::fromInterval(0, null), - $this->numericString() + $this->numericString(), ]; if ($nullable) { $types[] = new NullType(); diff --git a/tests/Type/Doctrine/data/QueryResult/queryResult.php b/tests/Type/Doctrine/data/QueryResult/queryResult.php index d8681e76..7062d41e 100644 --- a/tests/Type/Doctrine/data/QueryResult/queryResult.php +++ b/tests/Type/Doctrine/data/QueryResult/queryResult.php @@ -383,31 +383,31 @@ public function testReturnTypeOfQueryMethodsWithExplicitSingleScalarHydrationMod '); assertType( - 'int<0, max>|numeric-string', + '(int<0, max>|numeric-string)', $query->getResult(AbstractQuery::HYDRATE_SINGLE_SCALAR) ); assertType( - 'int<0, max>|numeric-string', + '(int<0, max>|numeric-string)', $query->getSingleScalarResult() ); assertType( - 'int<0, max>|numeric-string', + '(int<0, max>|numeric-string)', $query->execute(null, AbstractQuery::HYDRATE_SINGLE_SCALAR) ); assertType( - 'int<0, max>|numeric-string', + '(int<0, max>|numeric-string)', $query->executeIgnoreQueryCache(null, AbstractQuery::HYDRATE_SINGLE_SCALAR) ); assertType( - 'int<0, max>|numeric-string', + '(int<0, max>|numeric-string)', $query->executeUsingQueryCache(null, AbstractQuery::HYDRATE_SINGLE_SCALAR) ); assertType( - 'int<0, max>|numeric-string', + '(int<0, max>|numeric-string)', $query->getSingleResult(AbstractQuery::HYDRATE_SINGLE_SCALAR) ); assertType( - 'int<0, max>|numeric-string|null', + '(int<0, max>|numeric-string|null)', $query->getOneOrNullResult(AbstractQuery::HYDRATE_SINGLE_SCALAR) ); } From 7382b34a2f795589d6f476bf67af9a7e7e528309 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Thu, 6 Apr 2023 10:27:06 +0200 Subject: [PATCH 7/8] Fix check for where condition --- .../Doctrine/Query/QueryResultTypeWalker.php | 13 +++++-------- .../Query/QueryResultTypeWalkerTest.php | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/Type/Doctrine/Query/QueryResultTypeWalker.php b/src/Type/Doctrine/Query/QueryResultTypeWalker.php index 2fd757d4..b1ff71f5 100644 --- a/src/Type/Doctrine/Query/QueryResultTypeWalker.php +++ b/src/Type/Doctrine/Query/QueryResultTypeWalker.php @@ -107,7 +107,7 @@ class QueryResultTypeWalker extends SqlWalker private $hasGroupByClause; /** @var bool */ - private $hasCondition; + private $hasWhereClause; /** * @param Query $query @@ -137,7 +137,7 @@ public function __construct($query, $parserResult, array $queryComponents) $this->nullableQueryComponents = []; $this->hasAggregateFunction = false; $this->hasGroupByClause = false; - $this->hasCondition = false; + $this->hasWhereClause = false; // The object is instantiated by Doctrine\ORM\Query\Parser, so receiving // dependencies through the constructor is not an option. Instead, we @@ -180,6 +180,7 @@ public function walkSelectStatement(AST\SelectStatement $AST) $this->typeBuilder->setSelectQuery(); $this->hasAggregateFunction = $this->hasAggregateFunction($AST); $this->hasGroupByClause = $AST->groupByClause !== null; + $this->hasWhereClause = $AST->whereClause !== null; $this->walkFromClause($AST->fromClause); @@ -593,8 +594,6 @@ public function walkOrderByItem($orderByItem) */ public function walkHavingClause($havingClause) { - $this->hasCondition = true; - return $this->marshalType(new MixedType()); } @@ -1010,8 +1009,6 @@ public function walkUpdateItem($updateItem) */ public function walkWhereClause($whereClause) { - $this->hasCondition = true; - return $this->marshalType(new MixedType()); } @@ -1299,10 +1296,10 @@ public function walkResultVariable($resultVariable) */ private function addScalar($alias, Type $type): void { - // Since we don't check the condition inside the WHERE or HAVING + // Since we don't check the condition inside the WHERE // conditions, we cannot be sure all the union types are correct. // For exemple, a condition `WHERE foo.bar IS NOT NULL` could be added. - if ($this->hasCondition && $type instanceof UnionType) { + if ($this->hasWhereClause && $type instanceof UnionType) { $type = TypeUtils::toBenevolentUnion($type); } diff --git a/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php b/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php index b97aaa79..9b81d5df 100644 --- a/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php +++ b/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php @@ -441,6 +441,25 @@ public function getTestData(): iterable ', ]; + yield 'scalar with where condition' => [ + $this->constantArray([ + [new ConstantStringType('intColumn'), new IntegerType()], + [new ConstantStringType('stringColumn'), new StringType()], + [ + new ConstantStringType('stringNullColumn'), + TypeUtils::toBenevolentUnion(TypeCombinator::addNull(new StringType())) + ], + [new ConstantStringType('datetimeColumn'), new ObjectType(DateTime::class)], + [new ConstantStringType('datetimeImmutableColumn'), new ObjectType(DateTimeImmutable::class)], + ]), + ' + SELECT m.intColumn, m.stringColumn, m.stringNullColumn, + m.datetimeColumn, m.datetimeImmutableColumn + FROM QueryResult\Entities\Many m + WHERE m.stringNullColumn IS NOT NULL + ', + ]; + yield 'scalar with alias' => [ $this->constantArray([ [new ConstantStringType('i'), new IntegerType()], From 86e52ac600e9b91f7b381f859eebfd1031a94c49 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Thu, 6 Apr 2023 10:37:39 +0200 Subject: [PATCH 8/8] Fix cs --- tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php b/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php index 9b81d5df..cea058d0 100644 --- a/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php +++ b/tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php @@ -447,7 +447,7 @@ public function getTestData(): iterable [new ConstantStringType('stringColumn'), new StringType()], [ new ConstantStringType('stringNullColumn'), - TypeUtils::toBenevolentUnion(TypeCombinator::addNull(new StringType())) + TypeUtils::toBenevolentUnion(TypeCombinator::addNull(new StringType())), ], [new ConstantStringType('datetimeColumn'), new ObjectType(DateTime::class)], [new ConstantStringType('datetimeImmutableColumn'), new ObjectType(DateTimeImmutable::class)],