Allowing non-nullable properties for AccessInterceptorScopeLocalizer

[petr-buchin]

This is a follow up on #682.

After doing some research, I realized that enabling AccessInterceptorScopeLocalizer to work with non-nullable properties is pretty straightforward.

For now checking for non-accessible properties looks like follows in class Properties:

return new self(array_filter($this->properties, static function (ReflectionProperty $property): bool {
    if (! $property->hasType()) {
        return false;
    }

    return ! array_key_exists(
        $property->getName(),
        // https://bugs.php.net/bug.php?id=77673
        array_flip(array_keys($property->getDeclaringClass()->getDefaultProperties()))
    );
}));

To fix this behavior and allow non-nullable properties on objects being proxied, this code can be changed to something like follows:

return new self(array_filter($this->properties, static function (ReflectionProperty $property) use($realObject): bool {
    if (! $property->hasType()) {
        return false;
    }
    
    if ($property->getType()->allowsNull() && $property->getType()->hasDefaultValue()) {
        return false;
    }

    return ! $property->isInitialized($realObject);
}));

This code allows to proxy objects with non-nullable properties and moves responsibility of initializing properties to the user of this library.

However, the problem is that classes Properties and BindProxyProperties does not have access to the object being proxied, thus cannot use method ReflectionProperty::isInitialized().

It's possible to pass the instance of an object being proxied to this classes, but that would require changing signatures of some interface methods, and that would be a BC break.

I was thinking about using a temporary workaround (before next major release), such as ProxyContext class with static property, like ProxyContext::$objectBeingProxied, that would allow classes like BindProxyProperties to have access to the object being proxied without changing interfaces, but that is admittedly clunky.

@Ocramius do you have any thoughts on this?

[Ocramius]

@petr-buchin Properties is a build-time construct, therefore use ($realObject) cannot work there in first place.

A runtime reflection call at instantiation, if needed, needs to be baked into the generated code.

[petr-buchin]

@Ocramius thanks for quick answer.

Probably this logic may be baked into the generated code, if needed.

But what do you think about the proposed solution?

If you agree with the solution itself, I could try to implement it.

[Ocramius]

But what do you think about the proposed solution?

Adding a runtime object to the Properties signature is a no-go, as explained: that's a build-time component of the \Generator\ sub-namespace, and should not be used at runtime.

Adding code to the generated named constructors is feasible, but we need an extensive integration test verifying that it works as expected.

[petr-buchin]

Adding code to the generated named constructors is feasible, but we need an extensive integration test verifying that it works as expected.

@Ocramius thanks, I'll try to implement it along with tests and create a PR

[Ocramius]

@petr-buchin please do start from integration test and build a PoC starting from there, even if quick & dirty.

Do not start from the sources, but really do start from the test suite, by adding a class that looks like what you are trying to proxy, and making it work from there (even by having other tests failing, as a start).

[petr-buchin]

@Ocramius no problems, I'll go with TDD here, thanks!