Skip to content

Conversation

@janedbal
Copy link
Contributor

Separated from #506.

This is crucial for every codebase that utilizes custom types a lot. Previously any AVG(t.customTypedField) was mixed. Now, it can produce proper type.

} catch (DescriptorNotRegisteredException $e) {
continue;
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also have another idea how to implement this without the requirement of known parent. Then, you can extend even abstract Doctrine\DBAL\Types\Type and still have proper type (which is not working in the implementation here).

We use it our codebase, but it may feel a bit more hacky:

public function getDatabaseInternalType(): Type {
        /** @var DbalType $dbalType */
        $dbalType = new $this->type();
        $schemaColumn = new SchemaColumn('dummy', $dbalType, []);
        $fullSqlType = strtolower($dbalType->getSQLDeclaration($schemaColumn->toArray(), new MySQL80Platform()));

        $typeName = Strings::replace($fullSqlType, '~^([a-z ]+).*?$~', '$1');

        return match ($typeName) {
            'tinyint',
            'smallint',
            'mediumint',
            'int',
            'bigint' => new IntegerType(),
            'decimal',
            'numeric' => new IntersectionType([new StringType(), new AccessoryNumericStringType()]),
            'double precision' => new FloatType(),
            'varchar',
            'char',
            'binary',
            'tinytext',
            'text',
            'longtext',
            'json',
            'date',
            'datetime',
            'time' => new StringType(),
            default => throw new LogicException("Unexpected type: $fullSqlType"),
        };
    }

If you feel that this more generic approach is suitable even for phpstan-doctrine, I can try implementing it in #506 (as ReflectionDescriptor actually need to implement DoctrineTypeDriverAwareDescriptor).

Copy link
Member

Choose a reason for hiding this comment

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

What about combining both, to cover more use-cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If both, I think we can merge this and add that extra layer using getSQLDeclaration on top of this later.

@janedbal janedbal requested a review from ondrejmirtes June 25, 2024 10:30
@ondrejmirtes ondrejmirtes merged commit 012cf14 into phpstan:1.4.x Jun 25, 2024
@ondrejmirtes
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants