doctrine / orm

Doctrine Object Relational Mapper (ORM)
https://www.doctrine-project.org/projects/orm.html
MIT License
9.86k stars 2.5k forks source link

Custom DQL functions: TypedExpression returning StringType does not cast to string #11537

Open janedbal opened 2 days ago

janedbal commented 2 days ago

While implementing better expression type inference in phpstan-doctrine, I added support even for TypedExpression as those are the only way how to have type inference with custom functions.

But the problem is that it is designed to use Type::convertToPHPValue which is no-op for StringType (it keeps whatever the value is, no casting performed). This means that all TypedExpressions returning StringType are actually not typed at all.

I kept this exception in phpstan-doctrine, but this feels like a design flaw and can definitelly cause some WTF moments.

Obviously, anybody can solve it by custom Type that do cast to string, but that requires deeper knowledge of how things work.

use Doctrine\DBAL\Types\Type;
use Doctrine\DBAL\Types\Types;
use Doctrine\ORM\Query\AST\Functions\FunctionNode;
use Doctrine\ORM\Query\Parser;
use Doctrine\ORM\Query\SqlWalker;
use Doctrine\ORM\Query\TokenType;

class Date extends FunctionNode implements TypedExpression
{
    public $date;

    public function getSql(SqlWalker $sqlWalker): string
    {
        return 'DATE(' . $sqlWalker->walkArithmeticPrimary($this->date) . ')';
    }

    public function parse(Parser $parser): void
    {
        $parser->match(TokenType::T_IDENTIFIER);
        $parser->match(TokenType::T_OPEN_PARENTHESIS);

        $this->date = $parser->ArithmeticPrimary();

        $parser->match(TokenType::T_CLOSE_PARENTHESIS);
    }

    public function getReturnType(): Type
    {
        return Type::getType(Types::STRING); // "broken"
    }
}

I dont really have any proposal of how to make this better. Maybe just document it?

janedbal commented 2 days ago

Related issues:

derrabus commented 2 days ago

StringType is meant to be used on VARCHAR data coming from a database server. Since no DBMS supported by DBAL would return a VARCHAR as something other than a string, we don't perform any type conversion here.

janedbal commented 2 days ago

I understand that, but since you are using Types even in other contexts (like TypedExpression), it causes issues like described above.

stof commented 2 days ago

given that the ORM currently does not allow registering custom boolean functions for instance (see the list of Configuration::addCustom*Function methods), adding an explicit cast in StringType could actually break some existing cases.