Ocramius / ProxyManager

🎩✨🌈 OOP Proxy wrappers/utilities - generates and manages proxies of your objects
MIT License
4.96k stars 187 forks source link

Setting property default value to enum points to invalid namespace #754

Open IonBazan opened 2 years ago

IonBazan commented 2 years ago

When generating a proxy where a default value is enum using PHP 8.1 (yes, I know...), the default value is missing leading slash, pointing to non-existent class.

namespace App\Domain\Transaction;

enum Status: string
{
    case INITIATED = 'INITIATED';
}
class Transaction
{
// ...
    protected Status $state = \App\Domain\Transaction\Status::INITIATED;
}

Generated proxy:

namespace MongoDBODMProxies\__PM__\App\Domain\Document\Transaction\Transaction;

class Generatedd19772223e8a9012106a186d3aa68e25 extends \App\Domain\Document\Transaction\Transaction implements \ProxyManager\Proxy\GhostObjectInterface
{
    private function callInitializer7e974($methodName, array $parameters)
    {
        if ($this->initializationTracker75370 || ! $this->initializer83563) {
            return;
        }

        $this->initializationTracker75370 = true;

        $this->state = App\Domain\Transaction\Status::INITIATED; // note missing leading backslash producing invalid namespace name
        // ...
    }
}

This is caused by var_export which implies root namespace, while generated proxy is its own namespace: https://github.com/Ocramius/ProxyManager/blob/8846623bea8749ded206db3ce701e4e508d516db/src/ProxyManager/ProxyGenerator/LazyLoadingGhost/MethodGenerator/CallInitializer.php#L171

We could either explicitly check for enum_exists and prefix it with \ just to make things work but I'm not sure if it's right solution so just creating an issue to track this problem.

Ocramius commented 2 years ago

I think the reflection API does not give you default values for object types here.

The fact that a property can be an enum by default, without going through property initializers, seems like a hole in the RFC 🤔

I suggest raising a language bug.

Ocramius commented 2 years ago

Hmm, no, the reflection API works as expected: https://3v4l.org/jBhDg

Ocramius commented 2 years ago

The fact that var_export() operates this way makes it non-fixable here, because any constant expression composing over multiple constants would result in a problem.

For example:

var_export(['key' => Foo::BAR]);

I suggest proposing that the var_export() behaviour is to be fixed in php-src directly (for enums), so that it always refers to a FQCN, rather than a relative symbol.

Ocramius commented 2 years ago

Reported @ https://github.com/php/php-src/issues/8232

Ocramius commented 2 years ago

Proposed upstream patch @ https://github.com/php/php-src/pull/8233 - please review that, when you can :) EDIT: you already did, thanks!

IonBazan commented 2 years ago

image

Ocramius commented 2 years ago

Gonna be fixed in PHP 8.2: https://externals.io/message/117466#117532

Ref: https://github.com/php/php-src/issues/8232 Ref: https://github.com/php/php-src/pull/8233 Ref: https://github.com/php/doc-en/pull/1472

IonBazan commented 2 years ago

Sweet! @Ocramius

nicolas-grekas commented 2 years ago

Enums don't work as defaults yet, but neither do objects as defaults.

The way to support both is to use the string representation of ReflectionPropery|ReflectionParameter since https://github.com/php/php-src/pull/7540

Unfortunately the recent change on var_export might not help... :sweat_smile:

Ocramius commented 2 years ago

Some partial support for this will come with PHP 8.2, as per:

nicolas-grekas commented 2 years ago

FYI I'm making some experiments on https://github.com/FriendsOfPHP/proxy-manager-lts/pull/17 to fix this issue and also add support for "new in initializers". I'll contribute back here if you think the approach I'm going to end up with would work for you.

nicolas-grekas commented 2 years ago

Issue fixed on FriendsOfPHP/proxy-manager-lts, submitted here as https://github.com/Ocramius/ProxyManager/pull/755