Proxies for __set/__unset need special handling if the parent implementation return type is explicitly declared as void

[DCoderLT]

The PHP documentation says that __set and __unset should return void, that their return values are ignored, and suggests these signatures:

public __set(string $name, mixed $value): void
public __unset(string $name): void

But if I actually declare the return types in my __set and __unset, the generated proxy methods still try to return the result of the proxied call. This results in a Compile Error: A void function must not return a value:

// my class
public function __set(string $name, mixed $value): void { ... }
public function __unset(string $name): void { ... }

// generated proxy
    public function __set($name, $value) : void
    {
        $this->initializerecc30 && ($this->initializerecc30->__invoke($valueHolder2e9e4, $this, '__set', array('name' => $name, 'value' => $value), $this->initializerecc30) || 1) && $this->valueHolder2e9e4 = $valueHolder2e9e4;

        if (isset(self::$publicPropertiesda7b8[$name])) {
            return ($this->valueHolder2e9e4->$name = $value); // ← uh oh
        }

        return $this->valueHolder2e9e4->__set($name, $value); // ← uh oh
    }

    public function __unset($name) : void
    {
        $this->initializerecc30 && ($this->initializerecc30->__invoke($valueHolder2e9e4, $this, '__unset', array('name' => $name), $this->initializerecc30) || 1) && $this->valueHolder2e9e4 = $valueHolder2e9e4;

        if (isset(self::$publicPropertiesda7b8[$name])) {
            unset($this->valueHolder2e9e4->$name);

            return;
        }

        return $this->valueHolder2e9e4->__unset($name); // ← uh oh
    }

[Ocramius]

Yes, this depends on the parent implementation regardless.

The PHP documentation is really mostly irrelevant: tests and php-src code are authoritative.

[Ocramius]

I suggest reproducing this on a test in the ProxyManager test suite: AFAIK, void is well covered, but feel free to expand on it where you see holes.

[X-Coder264]

@Ocramius Not sure why this issue was closed as it is still present/not fixed. I've just run into the same issue as the author of this issue during the generation of a proxy of the \Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository class (which has an explicit void return type for the __set and __unset methods in the \Symfony\Component\VarExporter\LazyGhostTrait trait which it uses since https://github.com/doctrine/DoctrineBundle/pull/1599).

https://github.com/Ocramius/ProxyManager/blob/2.14.1/src/ProxyManager/ProxyGenerator/LazyLoadingValueHolder/MethodGenerator/MagicSet.php#L49

There's no check at all whether the parent has a void return type or not, if it detects that there's a parent method it always calls the parent with a return.

[nicolas-grekas]

Using LazyGhostTrait together with this lib is likely going to explode. You need to figure out why you end up using both on the same class and keep only one of them.

[X-Coder264]

@nicolas-grekas We are not using both on the same class. Only proxy manager is being used on that class. We are using Symfony 5.4 in the app, but we have a transitive dependency which pulled in symfony/var-exporter 6.2 which has the LazyGhostTrait trait and everything works fine. We are using Doctrine ORM 2.13 and Doctrine bundle 2.7 and now I've wanted to update to ORM 2.14 and Doctrine bundle 2.8. After updating those dependencies and running bin/console ca:cl we get the Compile Error: A void function must not return a value error.

[nicolas-grekas]

I'd be happy to have a look at a reproducer if you can provide one please, to confirm whether there's something to fix in this repo or another.

[X-Coder264]

@nicolas-grekas :+1: I'll try to create a reproducer soon.

[X-Coder264]

@nicolas-grekas Here's a reproducer https://github.com/X-Coder264/proxy-reproducer-bug

It blows up when the repository service is fetched from the container. In our case that happens during the cache:clear command (as one of the cache warmers tries to instantiate that repository), in the reproducer you have to run bin/console foo to get the error (the repository is injected in that command).

[nicolas-grekas]

Thanks, makes sense now. The issue is indeed in this repo, which generates this code:

public function __set($name, $value) : void
{
    $this->initializer480f6 && ($this->initializer480f6->__invoke($valueHolderd4bf6, $this, '__set', array('name' => $name, 'value' => $value), $this->initializer480f6) || 1) && $this->valueHolderd4bf6 =    $valueHolderd4bf6;

    return $this->valueHolderd4bf6->__set($name, $value);
}

The return is wrong obviously. I guess this issue should be reopened + fixed. Anyone up to give it a try?

The work around is to not make your repository lazy. It should not be needed anyway.