Ocramius/ProxyManager

Promoted readonly properties are not respected

bobvandevijver opened this issue · 7 comments

When using a promoted readonly only property in a class, this library will generate code that tries to change the readonly property and therefor breaking execution. Consider the following class (all have been stripped to only show the constructor):

class SystemNotificationService extends AbstractCacheableService
{
  public function __construct(
      private readonly SystemNotificationRepository $systemNotificationRepository,
      private readonly SerializerInterface $serializer,
      Features $features,
      string $kernelEnvironment,
      string $commitHash,
      string $httpHost,
      string $cacheDir)
  {
    parent::__construct($features, $kernelEnvironment, $commitHash, $httpHost, $cacheDir);
  }
}
abstract class AbstractCacheableService
{
  public function __construct(
      protected readonly Features $features,
      string $kernelEnvironment,
      string $commitHash,
      string $httpHost,
      private readonly string $cacheDir)
  {
  }
}

will generate the following proxy:

class SystemNotificationService_6ddbdeb extends \App\Service\SystemNotificationService implements \ProxyManager\Proxy\VirtualProxyInterface
{
    public function __construct(private \JMS\Serializer\SerializerInterface $serializer, \App\Features $features)
    {
        static $reflection;

        if (! $this->valueHolder06ae0) {
            $reflection = $reflection ?? new \ReflectionClass('App\\Service\\SystemNotificationService');
            $this->valueHolder06ae0 = $reflection->newInstanceWithoutConstructor();
        unset($this->features);

        \Closure::bind(function (\App\Service\SystemNotificationService $instance) {
            unset($instance->systemNotificationRepository, $instance->serializer);
        }, $this, 'App\\Service\\SystemNotificationService')->__invoke($this);

        \Closure::bind(function (\App\Service\AbstractCacheableService $instance) {
            unset($instance->cache, $instance->cacheHash, $instance->clearedTags, $instance->clearedKeys, $instance->cacheDir);
        }, $this, 'App\\Service\\AbstractCacheableService')->__invoke($this);

        }

        $this->valueHolder06ae0->__construct($systemNotificationRepository, $serializer, $features, $kernelEnvironment, $commitHash, $httpHost, $cacheDir);
    }
}

and thus throw Cannot unset readonly property App\Service\AbstractCacheableService::$features from scope ContainerXH6sJpc\SystemNotificationService_6ddbdeb, caused by the unset($instance->features).

Note that the same will also happen with the systemNotificationRepository and serializer properties (unset($instance->systemNotificationRepository, $instance->serializer);).

Be aware that this library is not compatible with PHP 8.1.0: there's still lots to be done to include 8.1.0 within the supported version range.

In fact, 8.1.0 is explicitly excluded:

"php": "~7.4.1 || ~8.0.0",

In fact, I'm using https://github.com/FriendsOfPHP/proxy-manager-lts which allows everything above 7.1.

https://github.com/FriendsOfPHP/proxy-manager-lts/blob/72294147d32a227ffadf1abd2f712da128e36a52/composer.json#L29

They state that issues should be opened here, but I will cross post one there that references this issue.

That project is literally "how to step on @Ocramius' face without any respect", so I really don't care about what they do

@bobvandevijver to be clear, the issue you reported is very clear and very much in need of research (which I will do, once I have time - mostly focused on roave/better-reflection for PHP 8.1, so I'm "recovering" from that 🤣).

My previous comments is just a rant about the fact that these forks are generally irresponsible towards end users, so it makes me extremely angry (to the point that I skip working on the library due to my mood) every time they come up.

@Ocramius Luckily I understood :) I am sincerely sorry to hear you feel this way, unfortunately not something I can directly influence as the fork is being forced by default by the Symfony framework in order to use lazy services, which is how I ended up here. I almost regret mentioning it as it clearly triggered you 😶

Anyways, I do hope you will find some time in the near future to bring full PHP 8.1 support to this library (I only just hit this issue and am using 8.1 already since the 8.1.1 release, so it feels like it mostly works fine already). Keep up the good work! 👍🏻