Skip to content

Improve QueryResultDynamicReturnTypeExtension #520

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.

Sign up for GitHub

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

Closed
wants to merge 15 commits into from
Closed

Improve QueryResultDynamicReturnTypeExtension #520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
09084c5
Improve QueryResultDynamicReturnTypeExtension
VincentLanglet Dec 26, 2022
88928ed
Solve phpstan deprecation
VincentLanglet Feb 25, 2023
11e5333
Avoid false positive about array result
VincentLanglet Apr 5, 2023
3ce312a
Use benevolent union for scalar in queries
VincentLanglet Apr 5, 2023
2b3fe65
Only use benevolent when where clause is used
VincentLanglet Apr 5, 2023
10480a6
Use benevolent union when we cast values
VincentLanglet Apr 5, 2023
651c3d9
Fix tests
VincentLanglet Apr 6, 2023
2516fe5
Avoid useless benevolent unions
VincentLanglet Apr 6, 2023
93b6bbc
Fix more tests
VincentLanglet Apr 6, 2023
601591e
Fix check for where condition
VincentLanglet Apr 6, 2023
032df01
Fix cs
VincentLanglet Apr 6, 2023
6e89ae2
Rely on default type when it cannot be precisely inferred
VincentLanglet Jan 17, 2024
0b882d9
Fix cs
VincentLanglet Jan 17, 2024
7ec8ad2
Fix
VincentLanglet Apr 18, 2024
aded0cf
Fix
VincentLanglet Apr 18, 2024
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
221 changes: 199 additions & 22 deletions src/Type/Doctrine/Query/QueryResultDynamicReturnTypeExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,22 @@
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;
use PHPStan\Type\IntegerType;
use PHPStan\Type\IterableType;
use PHPStan\Type\MixedType;
use PHPStan\Type\NullType;
use PHPStan\Type\ObjectWithoutClassType;
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;

final class QueryResultDynamicReturnTypeExtension implements DynamicMethodReturnTypeExtension
{
Expand All @@ -32,14 +40,32 @@ final class QueryResultDynamicReturnTypeExtension implements DynamicMethodReturn
'getSingleResult' => 0,
];

private const METHOD_HYDRATION_MODE = [
'getArrayResult' => AbstractQuery::HYDRATE_ARRAY,
'getScalarResult' => AbstractQuery::HYDRATE_SCALAR,
'getSingleColumnResult' => AbstractQuery::HYDRATE_SCALAR_COLUMN,
'getSingleScalarResult' => AbstractQuery::HYDRATE_SINGLE_SCALAR,
];

/** @var ObjectMetadataResolver */
private $objectMetadataResolver;

public function __construct(
ObjectMetadataResolver $objectMetadataResolver
)
{
$this->objectMetadataResolver = $objectMetadataResolver;
}

public function getClass(): string
{
return AbstractQuery::class;
}

public function isMethodSupported(MethodReflection $methodReflection): bool
{
return isset(self::METHOD_HYDRATION_MODE_ARG[$methodReflection->getName()]);
return isset(self::METHOD_HYDRATION_MODE_ARG[$methodReflection->getName()])
|| isset(self::METHOD_HYDRATION_MODE[$methodReflection->getName()]);
}

public function getTypeFromMethodCall(
Expand All @@ -50,21 +76,23 @@ public function getTypeFromMethodCall(
{
$methodName = $methodReflection->getName();

if (!isset(self::METHOD_HYDRATION_MODE_ARG[$methodName])) {
throw new ShouldNotHappenException();
}
if (isset(self::METHOD_HYDRATION_MODE[$methodName])) {
$hydrationMode = new ConstantIntegerType(self::METHOD_HYDRATION_MODE[$methodName]);
} elseif (isset(self::METHOD_HYDRATION_MODE_ARG[$methodName])) {
$argIndex = self::METHOD_HYDRATION_MODE_ARG[$methodName];
$args = $methodCall->getArgs();

$argIndex = self::METHOD_HYDRATION_MODE_ARG[$methodName];
$args = $methodCall->getArgs();

if (isset($args[$argIndex])) {
$hydrationMode = $scope->getType($args[$argIndex]->value);
if (isset($args[$argIndex])) {
$hydrationMode = $scope->getType($args[$argIndex]->value);
} else {
$parametersAcceptor = ParametersAcceptorSelector::selectSingle(
$methodReflection->getVariants()
);
$parameter = $parametersAcceptor->getParameters()[$argIndex];
$hydrationMode = $parameter->getDefaultValue() ?? new NullType();
}
} else {
$parametersAcceptor = ParametersAcceptorSelector::selectSingle(
$methodReflection->getVariants()
);
$parameter = $parametersAcceptor->getParameters()[$argIndex];
$hydrationMode = $parameter->getDefaultValue() ?? new NullType();
throw new ShouldNotHappenException();
}

$queryType = $scope->getType($methodCall->var);
Expand Down Expand Up @@ -98,23 +126,58 @@ private function getMethodReturnTypeForHydrationMode(
return $this->originalReturnType($methodReflection);
}

if (!$this->isObjectHydrationMode($hydrationMode)) {
// We support only HYDRATE_OBJECT. For other hydration modes, we
// return the declared return type of the method.
if (!$hydrationMode instanceof ConstantIntegerType) {
return $this->originalReturnType($methodReflection);
}

$singleResult = false;
switch ($hydrationMode->getValue()) {
case AbstractQuery::HYDRATE_OBJECT:
break;
case AbstractQuery::HYDRATE_ARRAY:
$queryResultType = $this->getArrayHydratedReturnType($queryResultType);
break;
case AbstractQuery::HYDRATE_SCALAR:
$queryResultType = $this->getScalarHydratedReturnType($queryResultType);
break;
case AbstractQuery::HYDRATE_SINGLE_SCALAR:
$singleResult = true;
$queryResultType = $this->getSingleScalarHydratedReturnType($queryResultType);
break;
case AbstractQuery::HYDRATE_SIMPLEOBJECT:
$queryResultType = $this->getSimpleObjectHydratedReturnType($queryResultType);
break;
case AbstractQuery::HYDRATE_SCALAR_COLUMN:
$queryResultType = $this->getScalarColumnHydratedReturnType($queryResultType);
break;
default:
return $this->originalReturnType($methodReflection);
}

if ($queryResultType === null) {
return $this->originalReturnType($methodReflection);
}

switch ($methodReflection->getName()) {
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,
$queryResultType
);
default:
if ($singleResult) {
return $queryResultType;
}

if ($queryKeyType->isNull()->yes()) {
return AccessoryArrayListType::intersectWith(new ArrayType(
new IntegerType(),
Expand All @@ -128,13 +191,127 @@ private function getMethodReturnTypeForHydrationMode(
}
}

private function isObjectHydrationMode(Type $type): bool
/**
* When we're array-hydrating object, we're not sure of the shape of the array.
* We could return `new ArrayTyp(new MixedType(), new MixedType())`
* but the lack of precision in the array keys/values would give false positive.
*
* @see https://github.com/phpstan/phpstan-doctrine/pull/412#issuecomment-1497092934
*/
private function getArrayHydratedReturnType(Type $queryResultType): ?Type
{
$objectManager = $this->objectMetadataResolver->getObjectManager();

$mixedFound = false;
$queryResultType = TypeTraverser::map(
$queryResultType,
static function (Type $type, callable $traverse) use ($objectManager, &$mixedFound): Type {
$isObject = (new ObjectWithoutClassType())->isSuperTypeOf($type);
if ($isObject->no()) {
return $traverse($type);
}
if (
$isObject->maybe()
|| !$type instanceof TypeWithClassName
|| $objectManager === null
) {
$mixedFound = true;

return new MixedType();
}

/** @var class-string $className */
$className = $type->getClassName();
if (!$objectManager->getMetadataFactory()->hasMetadataFor($className)) {
return $traverse($type);
}

$mixedFound = true;

return new MixedType();
}
);

return $mixedFound ? null : $queryResultType;
}

/**
* When we're scalar-hydrating object, we're not sure of the shape of the array.
* We could return `new ArrayTyp(new MixedType(), new MixedType())`
* but the lack of precision in the array keys/values would give false positive.
*
* @see https://github.com/phpstan/phpstan-doctrine/pull/453#issuecomment-1895415544
*/
private function getScalarHydratedReturnType(Type $queryResultType): ?Type
{
if (!$queryResultType->isArray()->yes()) {
return null;
}

foreach ($queryResultType->getArrays() as $arrayType) {
$itemType = $arrayType->getItemType();

if (
!(new ObjectWithoutClassType())->isSuperTypeOf($itemType)->no()
|| !$itemType->isArray()->no()
) {
// We could return `new ArrayTyp(new MixedType(), new MixedType())`
// but the lack of precision in the array keys/values would give false positive
// @see https://github.com/phpstan/phpstan-doctrine/pull/453#issuecomment-1895415544
return null;
}
}

return $queryResultType;
}

private function getSimpleObjectHydratedReturnType(Type $queryResultType): ?Type
{
if (!$type instanceof ConstantIntegerType) {
return false;
if ((new ObjectWithoutClassType())->isSuperTypeOf($queryResultType)->yes()) {
return $queryResultType;
}

return null;
}

private function getSingleScalarHydratedReturnType(Type $queryResultType): ?Type
{
$queryResultType = $this->getScalarHydratedReturnType($queryResultType);
if ($queryResultType === null || !$queryResultType->isConstantArray()->yes()) {
return null;
}

$types = [];
foreach ($queryResultType->getConstantArrays() as $constantArrayType) {
$values = $constantArrayType->getValueTypes();
if (count($values) !== 1) {
return null;
}

$types[] = $constantArrayType->getFirstIterableValueType();
}

return TypeCombinator::union(...$types);
}

private function getScalarColumnHydratedReturnType(Type $queryResultType): ?Type
{
$queryResultType = $this->getScalarHydratedReturnType($queryResultType);
if ($queryResultType === null || !$queryResultType->isConstantArray()->yes()) {
return null;
}

$types = [];
foreach ($queryResultType->getConstantArrays() as $constantArrayType) {
$values = $constantArrayType->getValueTypes();
if (count($values) !== 1) {
return null;
}

$types[] = $constantArrayType->getFirstIterableValueType();
}

return $type->getValue() === AbstractQuery::HYDRATE_OBJECT;
return TypeCombinator::union(...$types);
}

private function originalReturnType(MethodReflection $methodReflection): Type
Expand Down
37 changes: 34 additions & 3 deletions src/Type/Doctrine/Query/QueryResultTypeWalker.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ class QueryResultTypeWalker extends SqlWalker
/** @var bool */
private $hasGroupByClause;

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

/**
* @param Query<mixed> $query
*/
Expand Down Expand Up @@ -136,6 +139,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 @@ -175,6 +179,7 @@ public function walkSelectStatement(AST\SelectStatement $AST): string
$this->typeBuilder->setSelectQuery();
$this->hasAggregateFunction = $this->hasAggregateFunction($AST);
$this->hasGroupByClause = $AST->groupByClause !== null;
$this->hasWhereClause = $AST->whereClause !== null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this ever works? Because QueryBuilderGetQueryDynamicReturnTypeExtension::METHODS_NOT_AFFECTING_RESULT_TYPE contains all where methods.

And if you remove those, you will break codebases using dynamic approaches in those methods (if those have doctrine.reportDynamicQueryBuilders: true).


$this->walkFromClause($AST->fromClause);

Expand Down Expand Up @@ -797,7 +802,7 @@ public function walkSelectExpression($selectExpression): string

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

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

return '';
}
Expand Down Expand Up @@ -843,21 +848,32 @@ public function walkSelectExpression($selectExpression): string
// 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 @@ -1298,6 +1314,21 @@ public function walkResultVariable($resultVariable): string
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
Loading