Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use benevolent union for scalar in queries #447

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
37 changes: 34 additions & 3 deletions src/Type/Doctrine/Query/QueryResultTypeWalker.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ class QueryResultTypeWalker extends SqlWalker
/** @var bool */
private $hasGroupByClause;

/** @var bool */
private $hasWhereClause;

/**
* @param Query<mixed> $query
*/
Expand Down Expand Up @@ -134,6 +137,7 @@ public function __construct($query, $parserResult, array $queryComponents)
$this->nullableQueryComponents = [];
$this->hasAggregateFunction = false;
$this->hasGroupByClause = 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
Expand Down Expand Up @@ -176,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);

Expand Down Expand Up @@ -794,7 +799,7 @@ public function walkSelectExpression($selectExpression)

$type = $this->resolveDoctrineType($typeName, $enumType, $nullable);

$this->typeBuilder->addScalar($resultAlias, $type);
$this->addScalar($resultAlias, $type);

return '';
}
Expand Down Expand Up @@ -840,21 +845,32 @@ 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;
$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);
}
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 && !$originalType->equals($type)) {
$type = TypeUtils::toBenevolentUnion($type);
}
}

$this->typeBuilder->addScalar($resultAlias, $type);
$this->addScalar($resultAlias, $type);

return '';
}
Expand Down Expand Up @@ -1275,6 +1291,21 @@ public function walkResultVariable($resultVariable)
return $this->marshalType(new MixedType());
}

/**
* @param array-key $alias
*/
private function addScalar($alias, Type $type): void
{
// 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->hasWhereClause && $type instanceof UnionType) {
$type = TypeUtils::toBenevolentUnion($type);
}

$this->typeBuilder->addScalar($alias, $type);
}

private function unmarshalType(string $marshalledType): Type
{
$type = unserialize($marshalledType);
Expand Down
Loading