Skip to content

Add missing descriptors for SmallFloatType and EnumType #656

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

Open
wants to merge 4 commits into
base: 2.0.x
Choose a base branch
from
Open
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
6 changes: 6 additions & 0 deletions extension.neon
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,9 @@ services:
-
class: PHPStan\Type\Doctrine\Descriptors\DecimalType
tags: [phpstan.doctrine.typeDescriptor]
-
class: PHPStan\Type\Doctrine\Descriptors\EnumType
tags: [phpstan.doctrine.typeDescriptor]
-
class: PHPStan\Type\Doctrine\Descriptors\FloatType
tags: [phpstan.doctrine.typeDescriptor]
Expand All @@ -374,6 +377,9 @@ services:
-
class: PHPStan\Type\Doctrine\Descriptors\SimpleArrayType
tags: [phpstan.doctrine.typeDescriptor]
-
class: PHPStan\Type\Doctrine\Descriptors\SmallFloatType
tags: [phpstan.doctrine.typeDescriptor]
-
class: PHPStan\Type\Doctrine\Descriptors\SmallIntType
tags: [phpstan.doctrine.typeDescriptor]
Expand Down
33 changes: 33 additions & 0 deletions phpstan-baseline-dbal-4.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
parameters:
ignoreErrors:
-
message: '#^Class Doctrine\\DBAL\\Types\\EnumType not found\.$#'
identifier: class.notFound
count: 1
path: src/Type/Doctrine/Descriptors/EnumType.php

-
message: '#^Method PHPStan\\Type\\Doctrine\\Descriptors\\EnumType\:\:getType\(\) should return class\-string\<Doctrine\\DBAL\\Types\\Type\> but returns string\.$#'
identifier: return.type
count: 1
path: src/Type/Doctrine/Descriptors/EnumType.php

-
message: '#^Class Doctrine\\DBAL\\Types\\SmallFloatType not found\.$#'
identifier: class.notFound
count: 1
path: src/Type/Doctrine/Descriptors/SmallFloatType.php

-
message: '#^Method PHPStan\\Type\\Doctrine\\Descriptors\\SmallFloatType\:\:getType\(\) should return class\-string\<Doctrine\\DBAL\\Types\\Type\> but returns string\.$#'
identifier: return.type
count: 1
path: src/Type/Doctrine/Descriptors/SmallFloatType.php

-
message: '#^Class Doctrine\\DBAL\\Types\\EnumType not found\.$#'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

At first, I tried adding new doctrine versions to composer.json, but it would require bigger effort to adjust this repo to support those everywhere, so I just referenced those not-yet-existing classes.

identifier: class.notFound
count: 1
path: src/Type/Doctrine/Query/QueryResultTypeWalker.php


6 changes: 6 additions & 0 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -317,3 +317,9 @@ parameters:
identifier: new.deprecatedClass
count: 1
path: tests/Type/Doctrine/DBAL/pdo.php

-
message: '#^Parameter references internal interface Doctrine\\ORM\\Query\\AST\\Phase2OptimizableConditional in its type\.$#'
identifier: parameter.internalInterface
count: 2
path: src/Type/Doctrine/Query/QueryResultTypeWalker.php
1 change: 1 addition & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ includes:
- phpstan-baseline.neon
- phpstan-baseline-deprecations.neon
- phpstan-baseline-dbal-3.neon
- phpstan-baseline-dbal-4.neon
- compatibility/orm-3-baseline.php
- vendor/phpstan/phpstan-strict-rules/rules.neon
- vendor/phpstan/phpstan-deprecation-rules/rules.neon
Expand Down
31 changes: 31 additions & 0 deletions src/Type/Doctrine/Descriptors/EnumType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php declare(strict_types = 1);

namespace PHPStan\Type\Doctrine\Descriptors;

use PHPStan\Type\StringType;
use PHPStan\Type\Type;

class EnumType implements DoctrineTypeDescriptor
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I wrong or without this type the data is considered as mixed ?

Now with this, it will report error foo be 'A'|'B' but is string on level 8 because the values option is not considered in the following code

#[ORM\Column(name: 'foo', type: Types::ENUM, options: ['values' => ['A', 'B'])]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Am I wrong or without this type the data is considered as mixed ?

You are correct.

{

public function getType(): string
{
return \Doctrine\DBAL\Types\EnumType::class;
}

public function getWritableToPropertyType(): Type
{
return new StringType();
}

public function getWritableToDatabaseType(): Type
{
return new StringType();
}

public function getDatabaseInternalType(): Type
{
return new StringType();
}

}
13 changes: 13 additions & 0 deletions src/Type/Doctrine/Descriptors/SmallFloatType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php declare(strict_types = 1);

namespace PHPStan\Type\Doctrine\Descriptors;

class SmallFloatType extends FloatType
Copy link
Member

Choose a reason for hiding this comment

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

No inheritance please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you sure? I believe SmallFloatType IS FloatType in terms of type descriptor.

{

public function getType(): string
{
return \Doctrine\DBAL\Types\SmallFloatType::class;
}

}
91 changes: 74 additions & 17 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\Types\EnumType as DbalEnumType;
use Doctrine\DBAL\Types\StringType as DbalStringType;
use Doctrine\DBAL\Types\Type as DbalType;
use Doctrine\ORM\EntityManagerInterface;
Expand All @@ -26,6 +27,7 @@
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\Constant\ConstantFloatType;
use PHPStan\Type\Constant\ConstantIntegerType;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\ConstantTypeHelper;
use PHPStan\Type\Doctrine\DescriptorNotRegisteredException;
use PHPStan\Type\Doctrine\DescriptorRegistry;
Expand Down Expand Up @@ -53,6 +55,7 @@
use function get_class;
use function gettype;
use function in_array;
use function is_array;
use function is_int;
use function is_numeric;
use function is_object;
Expand Down Expand Up @@ -286,13 +289,13 @@ public function walkPathExpression($pathExpr): string

switch ($pathExpr->type) {
case AST\PathExpression::TYPE_STATE_FIELD:
[$typeName, $enumType] = $this->getTypeOfField($class, $fieldName);
[$typeName, $enumType, $enumValues] = $this->getTypeOfField($class, $fieldName);

$nullable = $this->isQueryComponentNullable($dqlAlias)
|| $class->isNullable($fieldName)
|| $this->hasAggregateWithoutGroupBy();

$fieldType = $this->resolveDatabaseInternalType($typeName, $enumType, $nullable);
$fieldType = $this->resolveDatabaseInternalType($typeName, $enumType, $enumValues, $nullable);

return $this->marshalType($fieldType);

Expand Down Expand Up @@ -326,12 +329,12 @@ public function walkPathExpression($pathExpr): string
}

$targetFieldName = $identifierFieldNames[0];
[$typeName, $enumType] = $this->getTypeOfField($targetClass, $targetFieldName);
[$typeName, $enumType, $enumValues] = $this->getTypeOfField($targetClass, $targetFieldName);

$nullable = ($joinColumn['nullable'] ?? true)
|| $this->hasAggregateWithoutGroupBy();

$fieldType = $this->resolveDatabaseInternalType($typeName, $enumType, $nullable);
$fieldType = $this->resolveDatabaseInternalType($typeName, $enumType, $enumValues, $nullable);

return $this->marshalType($fieldType);

Expand Down Expand Up @@ -685,7 +688,7 @@ public function walkFunction($function): string
return $this->marshalType(new MixedType());
}

[$typeName, $enumType] = $this->getTypeOfField($targetClass, $targetFieldName);
[$typeName, $enumType, $enumValues] = $this->getTypeOfField($targetClass, $targetFieldName);

if (!isset($assoc['joinColumns'])) {
return $this->marshalType(new MixedType());
Expand All @@ -708,7 +711,7 @@ public function walkFunction($function): string
|| $this->isQueryComponentNullable($dqlAlias)
|| $this->hasAggregateWithoutGroupBy();

$fieldType = $this->resolveDatabaseInternalType($typeName, $enumType, $nullable);
$fieldType = $this->resolveDatabaseInternalType($typeName, $enumType, $enumValues, $nullable);

return $this->marshalType($fieldType);

Expand Down Expand Up @@ -1206,13 +1209,13 @@ public function walkSelectExpression($selectExpression): string
assert(array_key_exists('metadata', $qComp));
$class = $qComp['metadata'];

[$typeName, $enumType] = $this->getTypeOfField($class, $fieldName);
[$typeName, $enumType, $enumValues] = $this->getTypeOfField($class, $fieldName);

$nullable = $this->isQueryComponentNullable($dqlAlias)
|| $class->isNullable($fieldName)
|| $this->hasAggregateWithoutGroupBy();

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

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

Expand All @@ -1235,11 +1238,12 @@ public function walkSelectExpression($selectExpression): string
if (
$expr instanceof TypedExpression
&& !$expr->getReturnType() instanceof DbalStringType // StringType is no-op, so using TypedExpression with that does nothing
&& !$expr->getReturnType() instanceof DbalEnumType // EnumType is also no-op
) {
$dbalTypeName = DbalType::getTypeRegistry()->lookupName($expr->getReturnType());
$type = TypeCombinator::intersect( // e.g. count is typed as int, but we infer int<0, max>
$type,
$this->resolveDoctrineType($dbalTypeName, null, TypeCombinator::containsNull($type)),
$this->resolveDoctrineType($dbalTypeName, null, null, TypeCombinator::containsNull($type)),
);

if ($this->hasAggregateWithoutGroupBy() && !$expr instanceof AST\Functions\CountFunction) {
Expand Down Expand Up @@ -1997,7 +2001,7 @@ private function isQueryComponentNullable(string $dqlAlias): bool

/**
* @param ClassMetadata<object> $class
* @return array{string, ?class-string<BackedEnum>} Doctrine type name and enum type of field
* @return array{string, ?class-string<BackedEnum>, ?list<string>} Doctrine type name, enum type of field, enum values
*/
private function getTypeOfField(ClassMetadata $class, string $fieldName): array
{
Expand All @@ -2015,11 +2019,45 @@ private function getTypeOfField(ClassMetadata $class, string $fieldName): array
$enumType = null;
}

return [$type, $enumType];
return [$type, $enumType, $this->detectEnumValues($type, $metadata)];
}

/** @param ?class-string<BackedEnum> $enumType */
private function resolveDoctrineType(string $typeName, ?string $enumType = null, bool $nullable = false): Type
/**
* @param mixed $metadata
*
* @return list<string>|null
*/
private function detectEnumValues(string $typeName, $metadata): ?array
{
if ($typeName !== 'enum') {
return null;
}

$values = $metadata['options']['values'] ?? [];

if (!is_array($values) || count($values) === 0) {
return null;
}

foreach ($values as $value) {
if (!is_string($value)) {
return null;
}
}

return array_values($values);
}

/**
* @param ?class-string<BackedEnum> $enumType
* @param ?list<string> $enumValues
*/
private function resolveDoctrineType(
string $typeName,
?string $enumType = null,
?array $enumValues = null,
bool $nullable = false
): Type
{
try {
$type = $this->descriptorRegistry
Expand All @@ -2036,8 +2074,14 @@ private function resolveDoctrineType(string $typeName, ?string $enumType = null,
), ...TypeUtils::getAccessoryTypes($type));
}
}

if ($enumValues !== null) {
$enumValuesType = TypeCombinator::union(...array_map(static fn (string $value) => new ConstantStringType($value), $enumValues));
$type = TypeCombinator::intersect($enumValuesType, $type);
}

if ($type instanceof NeverType) {
$type = new MixedType();
$type = new MixedType();
}
} catch (DescriptorNotRegisteredException $e) {
if ($enumType !== null) {
Expand All @@ -2051,11 +2095,19 @@ private function resolveDoctrineType(string $typeName, ?string $enumType = null,
$type = TypeCombinator::addNull($type);
}

return $type;
return $type;
}

/** @param ?class-string<BackedEnum> $enumType */
private function resolveDatabaseInternalType(string $typeName, ?string $enumType = null, bool $nullable = false): Type
/**
* @param ?class-string<BackedEnum> $enumType
* @param ?list<string> $enumValues
*/
private function resolveDatabaseInternalType(
string $typeName,
?string $enumType = null,
?array $enumValues = null,
bool $nullable = false
): Type
{
try {
$descriptor = $this->descriptorRegistry->get($typeName);
Expand All @@ -2074,6 +2126,11 @@ private function resolveDatabaseInternalType(string $typeName, ?string $enumType
$type = TypeCombinator::intersect($enumType, $type);
}

if ($enumValues !== null) {
$enumValuesType = TypeCombinator::union(...array_map(static fn (string $value) => new ConstantStringType($value), $enumValues));
$type = TypeCombinator::intersect($enumValuesType, $type);
}

if ($nullable) {
$type = TypeCombinator::addNull($type);
}
Expand Down
34 changes: 34 additions & 0 deletions tests/Type/Doctrine/Query/QueryResultTypeWalkerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\UnionType;
use PHPStan\Type\VerbosityLevel;
use QueryResult\Entities\Embedded;
use QueryResult\Entities\JoinedChild;
Expand All @@ -44,6 +45,7 @@
use QueryResult\Entities\One;
use QueryResult\Entities\OneId;
use QueryResult\Entities\SingleTableChild;
use QueryResult\EntitiesDbal42\Dbal4Entity;
use QueryResult\EntitiesEnum\EntityWithEnum;
use QueryResult\EntitiesEnum\IntEnum;
use QueryResult\EntitiesEnum\StringEnum;
Expand Down Expand Up @@ -187,6 +189,15 @@ public static function setUpBeforeClass(): void
$em->persist($entityWithEnum);
}

if (InstalledVersions::satisfies(new VersionParser(), 'doctrine/dbal', '>=4.2')) {
assert(class_exists(Dbal4Entity::class));

$dbal4Entity = new Dbal4Entity();
$dbal4Entity->enum = 'a';
$dbal4Entity->smallfloat = 1.1;
$em->persist($dbal4Entity);
}

$em->flush();
}

Expand Down Expand Up @@ -1532,6 +1543,29 @@ private function yieldConditionalDataset(): iterable
];
}

if (InstalledVersions::satisfies(new VersionParser(), 'doctrine/dbal', '>=4.2')) {
yield 'enum and smallfloat' => [
$this->constantArray([
[
new ConstantStringType('enum'),
new UnionType([
new ConstantStringType('a'),
new ConstantStringType('b'),
new ConstantStringType('c'),
]),
],
[
new ConstantStringType('smallfloat'),
new FloatType(),
],
]),
'
SELECT e.enum, e.smallfloat
FROM QueryResult\EntitiesDbal42\Dbal4Entity e
',
];
}

$ormVersion = InstalledVersions::getVersion('doctrine/orm');
$hasOrm3 = $ormVersion !== null && strpos($ormVersion, '3.') === 0;

Expand Down
Loading
Loading