Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for non-nullable typed properties #685

Open
wants to merge 21 commits into
base: 2.15.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@

use Laminas\Code\Generator\ParameterGenerator;
use Laminas\Code\Generator\PropertyGenerator;
use ProxyManager\Exception\UnsupportedProxiedClassException;
use ProxyManager\Generator\MethodGenerator;
use ProxyManager\ProxyGenerator\Util\Properties;
use ReflectionClass;

use function implode;
use function var_export;
use function sprintf;

use const PHP_EOL;

/**
* The `bindProxyProperties` method implementation for access interceptor scope localizers
Expand Down Expand Up @@ -42,38 +43,19 @@ public function __construct(
. '@param \\Closure[] $suffixInterceptors method interceptors to be used before method logic'
);

$localizedProperties = [];
$properties = Properties::fromReflectionClass($originalClass);
$nonReferenceableProperties = $properties
->onlyNonReferenceableProperties()
->onlyInstanceProperties();
$properties = Properties::fromReflectionClass($originalClass);

if (! $nonReferenceableProperties->empty()) {
throw UnsupportedProxiedClassException::nonReferenceableLocalizedReflectionProperties(
$originalClass,
$nonReferenceableProperties
$bodyLines = ['$class = new \ReflectionObject($localizedObject);'];
foreach ($properties->getInstanceProperties() as $property) {
$bodyLines[] = sprintf(
'$this->bindProxyProperty($localizedObject, $class, \'%s\');',
$property->getName()
);
}

foreach ($properties->getAccessibleProperties() as $property) {
$propertyName = $property->getName();

$localizedProperties[] = '$this->' . $propertyName . ' = & $localizedObject->' . $propertyName . ';';
}

foreach ($properties->getPrivateProperties() as $property) {
$propertyName = $property->getName();
$bodyLines[] = sprintf('$this->%s = $prefixInterceptors;', $prefixInterceptors->getName());
$bodyLines[] = sprintf('$this->%s = $suffixInterceptors;', $suffixInterceptors->getName());

$localizedProperties[] = "\\Closure::bind(function () use (\$localizedObject) {\n "
. '$this->' . $propertyName . ' = & $localizedObject->' . $propertyName . ";\n"
. '}, $this, ' . var_export($property->getDeclaringClass()->getName(), true)
. ')->__invoke();';
}

$this->setBody(
($localizedProperties ? implode("\n\n", $localizedProperties) . "\n\n" : '')
. '$this->' . $prefixInterceptors->getName() . " = \$prefixInterceptors;\n"
. '$this->' . $suffixInterceptors->getName() . ' = $suffixInterceptors;'
);
$this->setBody(implode(PHP_EOL, $bodyLines));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
<?php

declare(strict_types=1);

namespace ProxyManager\ProxyGenerator\AccessInterceptorScopeLocalizer\MethodGenerator;

use Laminas\Code\Generator\ParameterGenerator;
use ProxyManager\Exception\UnsupportedProxiedClassException;
use ProxyManager\Generator\MethodGenerator;
use ReflectionClass;

use function strtr;

class BindProxyProperty extends MethodGenerator
{
public function __construct(ReflectionClass $originalClass)
{
parent::__construct(
'bindProxyProperty',
[
new ParameterGenerator('localizedObject', $originalClass->getName()),
new ParameterGenerator('class', ReflectionClass::class),
new ParameterGenerator('propertyName', 'string'),
],
self::FLAG_PRIVATE
);

$bodyTemplate = <<<'CODE'
$originalClass = $class;
while (! $class->hasProperty($propertyName)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This operation should be performed during compilation, rather than during runtime: it's quite expensive to traverse ancestors.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ocramius unfortunately I don't understand how to perform this during compilation: different properties would have different declaring classes. So how this can be done during compilation?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done - I'm generating a $classesMap variable at build time, which has instances of ReflectionClass for private properties, and then use it when localizing private props.

$class = $class->getParentClass();
}
$property = $class->getProperty($propertyName);
$property->setAccessible(true);
if (!$property->isInitialized($localizedObject)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this correctly, the difference here is that we aren't creating an "empty shell", but proxying when the other class is correctly initialized?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes: for now proxy can be created only when properties are nullable and have default value in class itself.
Thus this library tries to be responsible for user classes and objects.
This PR moves responsibility of initializing object to the user of the library: class may have non-nullable typed properties, but as long as concrete instance being proxied is properly initialized - it can be proxied.

throw new \{ EXCEPTION_CLASS }(
sprintf(
'Cannot create reference for property $%s of class %s: property must be initialized',
$property->getName(),
$originalClass->getName()
)
);
}
if (!$property->isPrivate()) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This call can be prevented by inspecting the reflection information for a property upfront

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

$this->{$propertyName} = & $localizedObject->{$propertyName};
return;
}

\Closure::bind(
function () use ($localizedObject, $propertyName) {
$this->{$propertyName} = & $localizedObject->{$propertyName};
},
$this,
$property->getDeclaringClass()->getName()
)->__invoke();
CODE;

$this->setBody(
strtr(
$bodyTemplate,
['{ EXCEPTION_CLASS }' => UnsupportedProxiedClassException::class]
)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use ProxyManager\ProxyGenerator\AccessInterceptor\PropertyGenerator\MethodPrefixInterceptors;
use ProxyManager\ProxyGenerator\AccessInterceptor\PropertyGenerator\MethodSuffixInterceptors;
use ProxyManager\ProxyGenerator\AccessInterceptorScopeLocalizer\MethodGenerator\BindProxyProperties;
use ProxyManager\ProxyGenerator\AccessInterceptorScopeLocalizer\MethodGenerator\BindProxyProperty;
use ProxyManager\ProxyGenerator\AccessInterceptorScopeLocalizer\MethodGenerator\InterceptedMethod;
use ProxyManager\ProxyGenerator\AccessInterceptorScopeLocalizer\MethodGenerator\MagicClone;
use ProxyManager\ProxyGenerator\AccessInterceptorScopeLocalizer\MethodGenerator\MagicGet;
Expand Down Expand Up @@ -71,6 +72,7 @@ static function (MethodGenerator $generatedMethod) use ($originalClass, $classGe
),
[
new StaticProxyConstructor($originalClass),
new BindProxyProperty($originalClass),
new BindProxyProperties($originalClass, $prefixInterceptors, $suffixInterceptors),
new SetMethodPrefixInterceptor($prefixInterceptors),
new SetMethodSuffixInterceptor($suffixInterceptors),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use ProxyManagerTestAsset\ClassWithDynamicArgumentsMethod;
use ProxyManagerTestAsset\ClassWithMethodWithByRefVariadicFunction;
use ProxyManagerTestAsset\ClassWithMethodWithVariadicFunction;
use ProxyManagerTestAsset\ClassWithNonNullableTypedProperties;
use ProxyManagerTestAsset\ClassWithParentHint;
use ProxyManagerTestAsset\ClassWithPublicArrayPropertyAccessibleViaMethod;
use ProxyManagerTestAsset\ClassWithPublicProperties;
Expand All @@ -26,6 +27,8 @@
use ProxyManagerTestAsset\EmptyClass;
use ProxyManagerTestAsset\VoidCounter;
use ReflectionClass;
use ReflectionObject;
use ReflectionType;
use stdClass;

use function array_values;
Expand Down Expand Up @@ -247,6 +250,13 @@ public function testPropertyExistence(object $instance, AccessInterceptorInterfa
self::assertSame(isset($instance->$publicProperty), isset($proxy->$publicProperty));
$this->assertProxySynchronized($instance, $proxy);

$class = new ReflectionObject($instance);
$property = $class->getProperty($publicProperty);
$propertyType = $property->getType();
if ($propertyType instanceof ReflectionType && ! $propertyType->allowsNull()) {
return;
}

$instance->$publicProperty = null;
self::assertFalse(isset($proxy->$publicProperty));
$this->assertProxySynchronized($instance, $proxy);
Expand Down Expand Up @@ -396,6 +406,26 @@ public static function getProxyMethods(): array
['parameter' => $empty],
$empty,
],
[
new ClassWithNonNullableTypedProperties(
'privatePropertyValue',
'protectedPropertyValue',
'prublicPropertyValue'
),
'getPrivateProperty',
[],
'privatePropertyValue',
],
[
new ClassWithNonNullableTypedProperties(
'privatePropertyValue',
'protectedPropertyValue',
'prublicPropertyValue'
),
'getProtectedProperty',
[],
'protectedPropertyValue',
],
];
}

Expand All @@ -406,15 +436,26 @@ public static function getProxyMethods(): array
*/
public function getPropertyAccessProxies(): array
{
$instance = new BaseClass();
$baseClassInstance = new BaseClass();
$classWithNonNullablePropertiesInstance = new ClassWithNonNullableTypedProperties(
'privatePropertyValue',
'protectedPropertyValue',
'publicPropertyValue'
);

return [
[
$instance,
(new AccessInterceptorScopeLocalizerFactory())->createProxy($instance),
$baseClassInstance,
(new AccessInterceptorScopeLocalizerFactory())->createProxy($baseClassInstance),
'publicProperty',
'publicPropertyDefault',
],
[
$classWithNonNullablePropertiesInstance,
(new AccessInterceptorScopeLocalizerFactory())->createProxy($classWithNonNullablePropertiesInstance),
'publicProperty',
'publicPropertyValue',
],
];
}

Expand Down
Loading