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.4.x
Choose a base branch
from
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,12 @@ $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.

### Expression types inferring

Whether `MAX(e.id)` is fetched as `string` or `int` highly [depends on drivers, their setup and PHP version](https://github.com/janedbal/php-database-drivers-fetch-test).
This extension copies the logic from linked analysis, autodetects your setup and provides accurate results for `pdo_mysql`, `mysqli`, `pdo_sqlite`, `sqlite3`, `pdo_pgsql` and `pgsql`.
Any other driver will result in union with stringified version, e.g. `numeric-string|int`.

### Problematic approaches

Not every QueryBuilder can be statically analysed, here are few advices to maximize type inferring:
Expand Down
8 changes: 8 additions & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,11 @@ parameters:
- '#^Cannot call method getWrappedResourceHandle\(\) on class\-string\|object\.$#'
path: tests/Platform/QueryResultTypeWalkerFetchTypeMatrixTest.php
reportUnmatched: false
-
message: '#^Call to function method_exists\(\) with Doctrine\\DBAL\\Connection and ''getNativeConnection'' will always evaluate to true\.$#' # needed for older DBAL versions
path: src/Type/Doctrine/Query/QueryResultTypeWalker.php
-
messages: # needed for older DBAL versions (fails only on PHP 7.3)
- '#^Class Doctrine\\DBAL\\Driver\\PgSQL\\Driver not found\.$#'
- '#^Class Doctrine\\DBAL\\Driver\\SQLite3\\Driver not found\.$#'
path: src/Type/Doctrine/Query/QueryResultTypeWalker.php
9 changes: 7 additions & 2 deletions src/Type/Doctrine/CreateQueryDynamicReturnTypeExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Doctrine\Persistence\Mapping\MappingException;
use PhpParser\Node\Expr\MethodCall;
use PHPStan\Analyser\Scope;
use PHPStan\Php\PhpVersion;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\Doctrine\Query\QueryResultTypeBuilder;
Expand All @@ -37,10 +38,14 @@ final class CreateQueryDynamicReturnTypeExtension implements DynamicMethodReturn
/** @var DescriptorRegistry */
private $descriptorRegistry;

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

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

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

try {
$query = $em->createQuery($queryString);
QueryResultTypeWalker::walk($query, $typeBuilder, $this->descriptorRegistry);
QueryResultTypeWalker::walk($query, $typeBuilder, $this->descriptorRegistry, $this->phpVersion);
} catch (ORMException | DBALException | NewDBALException | CommonException | MappingException | \Doctrine\ORM\Exception\ORMException $e) {
return new QueryType($queryString, null, null);
} catch (AssertionError $e) {
Expand Down
11 changes: 11 additions & 0 deletions src/Type/Doctrine/DefaultDescriptorRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,15 @@ public function get(string $type): DoctrineTypeDescriptor
return $this->descriptors[$typeClass];
}

/**
* @throws DescriptorNotRegisteredException
*/
public function getByClassName(string $className): DoctrineTypeDescriptor
{
if (!isset($this->descriptors[$className])) {
throw new DescriptorNotRegisteredException();
}
return $this->descriptors[$className];
}

}
3 changes: 2 additions & 1 deletion src/Type/Doctrine/Descriptors/ArrayType.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace PHPStan\Type\Doctrine\Descriptors;

use Doctrine\DBAL\Driver;
use PHPStan\Type\MixedType;
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
Expand All @@ -24,7 +25,7 @@ public function getWritableToDatabaseType(): Type
return new \PHPStan\Type\ArrayType(new MixedType(), new MixedType());
}

public function getDatabaseInternalType(): Type
public function getDatabaseInternalType(Driver $driver): Type
{
return new StringType();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Type/Doctrine/Descriptors/AsciiStringType.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace PHPStan\Type\Doctrine\Descriptors;

use Doctrine\DBAL\Driver;
use PHPStan\Type\StringType;
use PHPStan\Type\Type;

Expand All @@ -23,7 +24,7 @@ public function getWritableToDatabaseType(): Type
return new StringType();
}

public function getDatabaseInternalType(): Type
public function getDatabaseInternalType(Driver $driver): Type
{
return new StringType();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Type/Doctrine/Descriptors/BigIntType.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace PHPStan\Type\Doctrine\Descriptors;

use Composer\InstalledVersions;
use Doctrine\DBAL\Driver;
use PHPStan\Type\Accessory\AccessoryNumericStringType;
use PHPStan\Type\IntegerType;
use PHPStan\Type\StringType;
Expand Down Expand Up @@ -33,7 +34,7 @@ public function getWritableToDatabaseType(): Type
return TypeCombinator::union(new StringType(), new IntegerType());
}

public function getDatabaseInternalType(): Type
public function getDatabaseInternalType(Driver $driver): Type
{
return new IntegerType();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Type/Doctrine/Descriptors/BinaryType.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace PHPStan\Type\Doctrine\Descriptors;

use Doctrine\DBAL\Driver;
use PHPStan\Type\MixedType;
use PHPStan\Type\ResourceType;
use PHPStan\Type\StringType;
Expand All @@ -25,7 +26,7 @@ public function getWritableToDatabaseType(): Type
return new MixedType();
}

public function getDatabaseInternalType(): Type
public function getDatabaseInternalType(Driver $driver): Type
{
return new StringType();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Type/Doctrine/Descriptors/BlobType.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace PHPStan\Type\Doctrine\Descriptors;

use Doctrine\DBAL\Driver;
use PHPStan\Type\MixedType;
use PHPStan\Type\ResourceType;
use PHPStan\Type\Type;
Expand All @@ -24,7 +25,7 @@ public function getWritableToDatabaseType(): Type
return new MixedType();
}

public function getDatabaseInternalType(): Type
public function getDatabaseInternalType(Driver $driver): Type
{
return new MixedType();
}
Expand Down
12 changes: 9 additions & 3 deletions src/Type/Doctrine/Descriptors/BooleanType.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

namespace PHPStan\Type\Doctrine\Descriptors;

use Doctrine\DBAL\Driver;
use Doctrine\DBAL\Driver\PDO\PgSQL\Driver as PdoPgSQLDriver;
use Doctrine\DBAL\Driver\PgSQL\Driver as PgSQLDriver;
use PHPStan\Type\Constant\ConstantIntegerType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
Expand All @@ -24,12 +27,15 @@
return new \PHPStan\Type\BooleanType();
}

public function getDatabaseInternalType(): Type
public function getDatabaseInternalType(Driver $driver): Type
{
if ($driver instanceof PgSQLDriver || $driver instanceof PdoPgSQLDriver) {

Check failure on line 32 in src/Type/Doctrine/Descriptors/BooleanType.php

View workflow job for this annotation

GitHub Actions / PHPStan (7.3)

Class Doctrine\DBAL\Driver\PgSQL\Driver not found.
return new \PHPStan\Type\BooleanType();
}

return TypeCombinator::union(
new ConstantIntegerType(0),
new ConstantIntegerType(1),
new \PHPStan\Type\BooleanType()
new ConstantIntegerType(1)
);
}

Expand Down
3 changes: 2 additions & 1 deletion src/Type/Doctrine/Descriptors/DateImmutableType.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace PHPStan\Type\Doctrine\Descriptors;

use DateTimeImmutable;
use Doctrine\DBAL\Driver;
use PHPStan\Type\ObjectType;
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
Expand All @@ -25,7 +26,7 @@ public function getWritableToDatabaseType(): Type
return new ObjectType(DateTimeImmutable::class);
}

public function getDatabaseInternalType(): Type
public function getDatabaseInternalType(Driver $driver): Type
{
return new StringType();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Type/Doctrine/Descriptors/DateIntervalType.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace PHPStan\Type\Doctrine\Descriptors;

use DateInterval;
use Doctrine\DBAL\Driver;
use PHPStan\Type\ObjectType;
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
Expand All @@ -25,7 +26,7 @@ public function getWritableToDatabaseType(): Type
return new ObjectType(DateInterval::class);
}

public function getDatabaseInternalType(): Type
public function getDatabaseInternalType(Driver $driver): Type
{
return new StringType();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Type/Doctrine/Descriptors/DateTimeImmutableType.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace PHPStan\Type\Doctrine\Descriptors;

use DateTimeImmutable;
use Doctrine\DBAL\Driver;
use PHPStan\Type\ObjectType;
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
Expand All @@ -25,7 +26,7 @@ public function getWritableToDatabaseType(): Type
return new ObjectType(DateTimeImmutable::class);
}

public function getDatabaseInternalType(): Type
public function getDatabaseInternalType(Driver $driver): Type
{
return new StringType();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Type/Doctrine/Descriptors/DateTimeType.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use DateTime;
use DateTimeInterface;
use Doctrine\DBAL\Driver;
use PHPStan\Type\ObjectType;
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
Expand All @@ -26,7 +27,7 @@ public function getWritableToDatabaseType(): Type
return new ObjectType(DateTimeInterface::class);
}

public function getDatabaseInternalType(): Type
public function getDatabaseInternalType(Driver $driver): Type
{
return new StringType();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Type/Doctrine/Descriptors/DateTimeTzImmutableType.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace PHPStan\Type\Doctrine\Descriptors;

use DateTimeImmutable;
use Doctrine\DBAL\Driver;
use PHPStan\Type\ObjectType;
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
Expand All @@ -25,7 +26,7 @@ public function getWritableToDatabaseType(): Type
return new ObjectType(DateTimeImmutable::class);
}

public function getDatabaseInternalType(): Type
public function getDatabaseInternalType(Driver $driver): Type
{
return new StringType();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Type/Doctrine/Descriptors/DateTimeTzType.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use DateTime;
use DateTimeInterface;
use Doctrine\DBAL\Driver;
use PHPStan\Type\ObjectType;
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
Expand All @@ -26,7 +27,7 @@ public function getWritableToDatabaseType(): Type
return new ObjectType(DateTimeInterface::class);
}

public function getDatabaseInternalType(): Type
public function getDatabaseInternalType(Driver $driver): Type
{
return new StringType();
}
Expand Down
3 changes: 2 additions & 1 deletion src/Type/Doctrine/Descriptors/DateType.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use DateTime;
use DateTimeInterface;
use Doctrine\DBAL\Driver;
use PHPStan\Type\ObjectType;
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
Expand All @@ -26,7 +27,7 @@ public function getWritableToDatabaseType(): Type
return new ObjectType(DateTimeInterface::class);
}

public function getDatabaseInternalType(): Type
public function getDatabaseInternalType(Driver $driver): Type
{
return new StringType();
}
Expand Down
16 changes: 14 additions & 2 deletions src/Type/Doctrine/Descriptors/DecimalType.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@

namespace PHPStan\Type\Doctrine\Descriptors;

use Doctrine\DBAL\Driver;
use Doctrine\DBAL\Driver\PDO\SQLite\Driver as PdoSqliteDriver;
use Doctrine\DBAL\Driver\SQLite3\Driver as Sqlite3Driver;
use PHPStan\Type\Accessory\AccessoryNumericStringType;
use PHPStan\Type\FloatType;
use PHPStan\Type\IntegerType;
use PHPStan\Type\IntersectionType;
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
Expand All @@ -27,9 +31,17 @@
return TypeCombinator::union(new StringType(), new FloatType(), new IntegerType());
}

public function getDatabaseInternalType(): Type
public function getDatabaseInternalType(Driver $driver): Type
{
return TypeCombinator::union(new FloatType(), new IntegerType());
if ($driver instanceof Sqlite3Driver || $driver instanceof PdoSqliteDriver) {

Check failure on line 36 in src/Type/Doctrine/Descriptors/DecimalType.php

View workflow job for this annotation

GitHub Actions / PHPStan (7.3)

Class Doctrine\DBAL\Driver\SQLite3\Driver not found.
return new FloatType();
}

// TODO use mixed as fallback for any untested driver or some guess?
return new IntersectionType([
new StringType(),
new AccessoryNumericStringType(),
]);
}

}
22 changes: 21 additions & 1 deletion src/Type/Doctrine/Descriptors/DoctrineTypeDescriptor.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace PHPStan\Type\Doctrine\Descriptors;

use Doctrine\DBAL\Driver;
use PHPStan\Type\Type;

/** @api */
Expand All @@ -13,10 +14,29 @@ interface DoctrineTypeDescriptor
*/
public function getType(): string;

/**
* This is used for inferring direct column results, e.g. SELECT e.field
* It should comply with convertToPHPValue return value
*/
public function getWritableToPropertyType(): Type;

public function getWritableToDatabaseType(): Type;

public function getDatabaseInternalType(): Type;
/**
* This is used for inferring how database fetches column of such type
* It should return the native type without stringification that may occur on certain PHP versions or driver configuration
*
* This is not used for direct column type inferring,
* but when such column appears in expression like SELECT MAX(e.field)
*
* See: https://github.com/janedbal/php-database-drivers-fetch-test
*
* mysql sqlite pdo_pgsql pgsql
* - decimal: string float string string
* - float: float float string float
* - bigint: int int int int
* - bool: int int bool bool
*/
public function getDatabaseInternalType(Driver $driver): Type;
Copy link
Contributor

Choose a reason for hiding this comment

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

This change will be a BC break since the interface has @api annotation.

Not sure which strategy should be followed to avoid this...
Should another interface be introduced ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, another interface somehow.


}