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

QueryResultTypeWalker: resolve if int/float/bool is fetched as native type or stringified #506

Draft
wants to merge 9 commits into
base: 1.3.x
Choose a base branch
from
13 changes: 13 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,19 @@ $query->getOneOrNullResult(Query::HYDRATE_OBJECT); // User

This is due to the design of the `Query` class preventing from determining the hydration mode used by these functions unless it is specified explicitly during the call.

### Numeric types inferring

By default, any expression like `MAX(e.id)` results in `int|numeric-string` as certain drivers & PHP versions do cast the result to string.
If you are using setup that does not do that, you can disable stringification of such expressions by setting `parameters.doctrine.stringifyExpressions` to `false`.

This should be accurate behaviour for [PHP 8.1.25+ with PDO driver](https://github.com/php/php-src/blob/php-8.1.25/UPGRADING#L122-L139) and disabled `PDO::ATTR_STRINGIFY_FETCHES` (which is default).

```neon
parameters:
doctrine:
stringifyExpressions: false
```

## Custom types

If your application uses custom Doctrine types, you can write your own type descriptors to analyse them properly.
Expand Down
4 changes: 4 additions & 0 deletions extension.neon
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ parameters:
objectManagerLoader: null
searchOtherMethodsForQueryBuilderBeginning: true
queryBuilderFastAlgorithm: false
stringifyExpressions: true
featureToggles:
skipCheckGenericClasses:
- Doctrine\ODM\MongoDB\Mapping\ClassMetadata
Expand Down Expand Up @@ -65,6 +66,7 @@ parametersSchema:
objectManagerLoader: schema(string(), nullable())
searchOtherMethodsForQueryBuilderBeginning: bool()
queryBuilderFastAlgorithm: bool()
stringifyExpressions: bool()
])

conditionalTags:
Expand Down Expand Up @@ -115,6 +117,7 @@ services:
arguments:
queryBuilderClass: %doctrine.queryBuilderClass%
argumentsProcessor: @doctrineQueryBuilderArgumentsProcessor
stringifyExpressions: %doctrine.stringifyExpressions%
tags:
- phpstan.broker.dynamicMethodReturnTypeExtension
-
Expand All @@ -138,6 +141,7 @@ services:
class: PHPStan\Type\Doctrine\CreateQueryDynamicReturnTypeExtension
arguments:
objectMetadataResolver: @PHPStan\Type\Doctrine\ObjectMetadataResolver
stringifyExpressions: %doctrine.stringifyExpressions%
tags:
- phpstan.broker.dynamicMethodReturnTypeExtension
-
Expand Down
1 change: 1 addition & 0 deletions rules.neon
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ parametersSchema:
reportDynamicQueryBuilders: bool()
reportUnknownTypes: bool()
allowNullablePropertyForRequiredField: bool()
stringifyExpressions: bool()
])

rules:
Expand Down
8 changes: 6 additions & 2 deletions src/Type/Doctrine/CreateQueryDynamicReturnTypeExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,14 @@ final class CreateQueryDynamicReturnTypeExtension implements DynamicMethodReturn
/** @var DescriptorRegistry */
private $descriptorRegistry;

public function __construct(ObjectMetadataResolver $objectMetadataResolver, DescriptorRegistry $descriptorRegistry)
/** @var bool */
private $stringifyExpressions;

public function __construct(ObjectMetadataResolver $objectMetadataResolver, DescriptorRegistry $descriptorRegistry, bool $stringifyExpressions)
{
$this->objectMetadataResolver = $objectMetadataResolver;
$this->descriptorRegistry = $descriptorRegistry;
$this->stringifyExpressions = $stringifyExpressions;
}

public function getClass(): string
Expand Down Expand Up @@ -86,7 +90,7 @@ public function getTypeFromMethodCall(

try {
$query = $em->createQuery($queryString);
QueryResultTypeWalker::walk($query, $typeBuilder, $this->descriptorRegistry);
QueryResultTypeWalker::walk($query, $typeBuilder, $this->descriptorRegistry, $this->stringifyExpressions);
} catch (ORMException | DBALException | NewDBALException | CommonException $e) {
return new QueryType($queryString, null, null);
} catch (AssertionError $e) {
Expand Down
42 changes: 39 additions & 3 deletions src/Type/Doctrine/Query/QueryResultTypeWalker.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace PHPStan\Type\Doctrine\Query;

use BackedEnum;
use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\ClassMetadataInfo;
Expand Down Expand Up @@ -42,11 +43,13 @@
use function get_class;
use function gettype;
use function intval;
use function is_bool;
use function is_numeric;
use function is_object;
use function is_string;
use function serialize;
use function sprintf;
use function strpos;
use function strtolower;
use function unserialize;

Expand All @@ -64,6 +67,8 @@ class QueryResultTypeWalker extends SqlWalker

private const HINT_DESCRIPTOR_REGISTRY = self::class . '::HINT_DESCRIPTOR_REGISTRY';

private const HINT_STRINGIFY_EXPRESSIONS = self::class . '::HINT_STRINGIFY_EXPRESSIONS';

/**
* Counter for generating unique scalar result.
*
Expand Down Expand Up @@ -106,14 +111,18 @@ class QueryResultTypeWalker extends SqlWalker
/** @var bool */
private $hasGroupByClause;

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

/**
* @param Query<mixed> $query
*/
public static function walk(Query $query, QueryResultTypeBuilder $typeBuilder, DescriptorRegistry $descriptorRegistry): void
public static function walk(Query $query, QueryResultTypeBuilder $typeBuilder, DescriptorRegistry $descriptorRegistry, bool $stringifyExpressions): void
{
$query->setHint(Query::HINT_CUSTOM_OUTPUT_WALKER, self::class);
$query->setHint(self::HINT_TYPE_MAPPING, $typeBuilder);
$query->setHint(self::HINT_DESCRIPTOR_REGISTRY, $descriptorRegistry);
$query->setHint(self::HINT_STRINGIFY_EXPRESSIONS, $stringifyExpressions);

$parser = new Parser($query);
$parser->parse();
Expand Down Expand Up @@ -165,6 +174,19 @@ public function __construct($query, $parserResult, array $queryComponents)

$this->descriptorRegistry = $descriptorRegistry;

$stringifyExpressions = $this->query->getHint(self::HINT_STRINGIFY_EXPRESSIONS);

if (!is_bool($stringifyExpressions)) {
throw new ShouldNotHappenException(sprintf(
'Expected the query hint %s to contain a %s, but got a %s',
self::HINT_STRINGIFY_EXPRESSIONS,
'boolean',
is_object($stringifyExpressions) ? get_class($stringifyExpressions) : gettype($stringifyExpressions)
));
}

$this->stringifyExpressions = $stringifyExpressions;

parent::__construct($query, $parserResult, $queryComponents);
}

Expand Down Expand Up @@ -834,7 +856,7 @@ public function walkSelectExpression($selectExpression)
}
return $enforcedType;
});
} else {
} elseif ($this->stringifyExpressions) {
// Expressions default to Doctrine's StringType, whose
// convertToPHPValue() is a no-op. So the actual type depends on
// the driver and PHP version.
Expand Down Expand Up @@ -1108,7 +1130,21 @@ public function walkLiteral($literal)
if (floatval(intval($value)) === floatval($value)) {
$type = new ConstantIntegerType((int) $value);
} else {
$type = new ConstantFloatType((float) $value);

if ($this->em->getConnection()->getDatabasePlatform() instanceof AbstractMySQLPlatform) {

// both PDO_mysql and mysqli hydrates 123.4 literals as string no matter the configuration (e.g. PDO::ATTR_STRINGIFY_FETCHES being false) and PHP version
// the only way to force float is to use 123.4e0 scientific notation
// https://dev.mysql.com/doc/refman/8.0/en/number-literals.html
janedbal marked this conversation as resolved.
Show resolved Hide resolved

if (strpos((string) $value, 'e') !== false || strpos((string) $value, 'E') !== false) {
janedbal marked this conversation as resolved.
Show resolved Hide resolved
$type = new ConstantFloatType((float) $value);
} else {
$type = new ConstantStringType((string) (float) $value);
}
} else {
$type = new ConstantFloatType((float) $value);
}
}

break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,19 +68,24 @@ class QueryBuilderGetQueryDynamicReturnTypeExtension implements DynamicMethodRet
/** @var OtherMethodQueryBuilderParser */
private $otherMethodQueryBuilderParser;

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

public function __construct(
ObjectMetadataResolver $objectMetadataResolver,
ArgumentsProcessor $argumentsProcessor,
?string $queryBuilderClass,
DescriptorRegistry $descriptorRegistry,
OtherMethodQueryBuilderParser $otherMethodQueryBuilderParser
OtherMethodQueryBuilderParser $otherMethodQueryBuilderParser,
bool $stringifyExpressions
)
{
$this->objectMetadataResolver = $objectMetadataResolver;
$this->argumentsProcessor = $argumentsProcessor;
$this->queryBuilderClass = $queryBuilderClass;
$this->descriptorRegistry = $descriptorRegistry;
$this->otherMethodQueryBuilderParser = $otherMethodQueryBuilderParser;
$this->stringifyExpressions = $stringifyExpressions;
}

public function getClass(): string
Expand Down Expand Up @@ -202,7 +207,7 @@ private function getQueryType(string $dql): Type

try {
$query = $em->createQuery($dql);
QueryResultTypeWalker::walk($query, $typeBuilder, $this->descriptorRegistry);
QueryResultTypeWalker::walk($query, $typeBuilder, $this->descriptorRegistry, $this->stringifyExpressions);
} catch (ORMException | DBALException | CommonException $e) {
return new QueryType($dql, null);
} catch (AssertionError $e) {
Expand Down