doctrine / common

Doctrine Common
https://www.doctrine-project.org/projects/common.html
MIT License
5.79k stars 294 forks source link

Entity method proxies generate incorrect parameters and result in compile error when object is used as a default value #965

Closed Amunak closed 2 years ago

Amunak commented 2 years ago

Bug Report

Q A
BC Break no
Version 2.12.3

Summary

As of PHP 8.1 it's possible to use objects as default values in properties, so we can use the shorthand function method($foo = new Bar). The proxy generator doesn't know how to deal with this and generates garbage in the signature instead of simply passing the definition over.

Current behavior

Properties with objects as default value have invalid signature in proxy classes. This results in Compile Error: Constant expression contains invalid operations.

How to reproduce

Given entity X:

namespace App\Entity;

use App\Foo;

class X {
    public function a($foo = new Foo) {}
    public function b(DateTimeInterface $date = new \DateTime('2000-01-01T12:00:00+00:00')){}
}

There will be the following proxy class generated:

class X extends \App\Entity\X implements \Doctrine\ORM\Proxy\Proxy
{
    // ....
    public function a($foo = App\Foo::__set_state(array(
)))
    {

        $this->__initializer__ && $this->__initializer__->__invoke($this, 'a', [$foo]);

        return parent::a($foo);
    }

    /**
     * {@inheritDoc}
     */
    public function b(\DateTimeInterface $date = DateTime::__set_state(array(
   'date' => '2000-01-01 12:00:00.000000',
   'timezone_type' => 1,
   'timezone' => '+00:00',
)))
    {

        $this->__initializer__ && $this->__initializer__->__invoke($this, 'b', [$date]);

        return parent::b($date);
    }
}

Note how it also gets the namespaces wrong in addition to attempting to instance(?) the object and fill it with data from time of the proxy generation (which is definitely wrong).

Expected behavior

The proxy generator generates a parameter where it simply re-uses the default value signature when an object is used as default parameter, only making sure the class uses absolute path.

I would expect the proxy class to look more like this:

class X extends \App\Entity\X implements \Doctrine\ORM\Proxy\Proxy
{
    // ....
    public function a($foo = new \App\Foo)
    {

        $this->__initializer__ && $this->__initializer__->__invoke($this, 'a', [$foo]);

        return parent::a($foo);
    }

    /**
     * {@inheritDoc}
     */
    public function b(\DateTimeInterface $date = \DateTime('2000-01-01T12:00:00+00:00'))
    {

        $this->__initializer__ && $this->__initializer__->__invoke($this, 'b', [$date]);

        return parent::b($date);
    }
}

Current workaround

Obviously this isn't a huge deal since one can simply assign the objects in the method body like always, but it's not optimal since that may force you to change the signature to allow for null values, which is slightly more ambiguous than the alternative.

class X {
    public function a($foo = null) { $foo ??= new Foo; }
    public function (null|\DateTimeInterface $date = null) { $date ??= new \DateTime('2000-01-01T12:00:00+00:00'); }
}
Amunak commented 2 years ago

Well I didn't search correctly, as this should probably be in doctrine/common and there's already a bug report for the same thing, though older version (doctrine/common#961).

greg0ire commented 2 years ago

Transferring to doctrine/common (again lol) then

greg0ire commented 2 years ago

Duplicate of #961