From b441aa047c33a742a7869f7a9c79a249120f0007 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 14:53:02 +0100 Subject: [PATCH 01/22] Testing code against the `ClassWithMixedProperties` asset for better coverage --- .../BindProxyPropertiesTest.php | 40 +++++++++++-------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyPropertiesTest.php b/tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyPropertiesTest.php index a0815e10b..5a841718a 100644 --- a/tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyPropertiesTest.php +++ b/tests/ProxyManagerTest/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyPropertiesTest.php @@ -20,9 +20,9 @@ use PHPUnit_Framework_TestCase; use ProxyManager\ProxyGenerator\AccessInterceptorScopeLocalizer\MethodGenerator\BindProxyProperties; +use ProxyManagerTestAsset\ClassWithMixedProperties; use ProxyManagerTestAsset\ClassWithPrivateProperties; use ProxyManagerTestAsset\ClassWithProtectedProperties; -use ProxyManagerTestAsset\ClassWithPublicProperties; use ReflectionClass; use Zend\Code\Generator\PropertyGenerator; @@ -85,36 +85,42 @@ public function testSignature() public function testBodyStructure() { $method = new BindProxyProperties( - new ReflectionClass(ClassWithPublicProperties::class), + new ReflectionClass(ClassWithMixedProperties::class), $this->prefixInterceptors, $this->suffixInterceptors ); - $this->assertSame( - '$this->property0 = & $localizedObject->property0; + $expectedCode = <<<'PHP' +$this->publicProperty0 = & $localizedObject->publicProperty0; -$this->property1 = & $localizedObject->property1; - -$this->property2 = & $localizedObject->property2; +$this->publicProperty1 = & $localizedObject->publicProperty1; -$this->property3 = & $localizedObject->property3; +$this->publicProperty2 = & $localizedObject->publicProperty2; -$this->property4 = & $localizedObject->property4; +$this->protectedProperty0 = & $localizedObject->protectedProperty0; -$this->property5 = & $localizedObject->property5; +$this->protectedProperty1 = & $localizedObject->protectedProperty1; -$this->property6 = & $localizedObject->property6; +$this->protectedProperty2 = & $localizedObject->protectedProperty2; -$this->property7 = & $localizedObject->property7; +\Closure::bind(function () use ($localizedObject) { + $this->privateProperty0 = & $localizedObject->privateProperty0; +}, $this, 'ProxyManagerTestAsset\\ClassWithMixedProperties')->__invoke(); -$this->property8 = & $localizedObject->property8; +\Closure::bind(function () use ($localizedObject) { + $this->privateProperty1 = & $localizedObject->privateProperty1; +}, $this, 'ProxyManagerTestAsset\\ClassWithMixedProperties')->__invoke(); -$this->property9 = & $localizedObject->property9; +\Closure::bind(function () use ($localizedObject) { + $this->privateProperty2 = & $localizedObject->privateProperty2; +}, $this, 'ProxyManagerTestAsset\\ClassWithMixedProperties')->__invoke(); $this->pre = $prefixInterceptors; -$this->post = $suffixInterceptors;', - $method->getBody() - ); +$this->post = $suffixInterceptors; +PHP; + + + $this->assertSame($expectedCode, $method->getBody()); } public function testBodyStructureWithProtectedProperties() From 1f6084f229ef8c00a23b134650bd580cee859bb4 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 14:53:50 +0100 Subject: [PATCH 02/22] `ClassWithMixedProperties` should also contain static properties for completeness --- tests/ProxyManagerTestAsset/ClassWithMixedProperties.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/ProxyManagerTestAsset/ClassWithMixedProperties.php b/tests/ProxyManagerTestAsset/ClassWithMixedProperties.php index 3ebf27cbf..ba23636ee 100644 --- a/tests/ProxyManagerTestAsset/ClassWithMixedProperties.php +++ b/tests/ProxyManagerTestAsset/ClassWithMixedProperties.php @@ -26,6 +26,12 @@ */ class ClassWithMixedProperties { + public static $publicStaticProperty = 'publicStaticProperty'; + + protected static $protectedStaticProperty = 'protectedStaticProperty'; + + private static $privateStaticProperty = 'privateStaticProperty'; + public $publicProperty0 = 'publicProperty0'; public $publicProperty1 = 'publicProperty1'; From d21ebf2fc1a560b468a3e647f65e506e213ad617 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:01:31 +0100 Subject: [PATCH 03/22] StaticProxyConstructor for null objects should skip non-static properties --- .../StaticProxyConstructor.php | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/ProxyManager/ProxyGenerator/NullObject/MethodGenerator/StaticProxyConstructor.php b/src/ProxyManager/ProxyGenerator/NullObject/MethodGenerator/StaticProxyConstructor.php index 74a3d20a1..d16ca8546 100644 --- a/src/ProxyManager/ProxyGenerator/NullObject/MethodGenerator/StaticProxyConstructor.php +++ b/src/ProxyManager/ProxyGenerator/NullObject/MethodGenerator/StaticProxyConstructor.php @@ -39,13 +39,12 @@ public function __construct(ReflectionClass $originalClass) { parent::__construct('staticProxyConstructor', [], static::FLAG_PUBLIC | static::FLAG_STATIC); - /* @var $publicProperties \ReflectionProperty[] */ - $publicProperties = $originalClass->getProperties(ReflectionProperty::IS_PUBLIC); - $nullableProperties = []; - - foreach ($publicProperties as $publicProperty) { - $nullableProperties[] = '$instance->' . $publicProperty->getName() . ' = null;'; - } + $nullableProperties = array_map( + function (ReflectionProperty $publicProperty) { + return '$instance->' . $publicProperty->getName() . ' = null;'; + }, + $this->getInstancePublicProperties($originalClass) + ); $this->setDocblock("Constructor for null object initialization"); $this->setBody( @@ -56,4 +55,19 @@ public function __construct(ReflectionClass $originalClass) . 'return $instance;' ); } + + /** + * @param ReflectionClass $originalClass + * + * @return ReflectionProperty[] + */ + private function getInstancePublicProperties(ReflectionClass $originalClass) + { + return array_filter( + $originalClass->getProperties(ReflectionProperty::IS_PUBLIC), + function (ReflectionProperty $property) { + return ! $property->isStatic(); + } + ); + } } From d7c9d7143446d1c70a4272ca6cbb6fe8c79ab5d7 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:10:27 +0100 Subject: [PATCH 04/22] Removing unused private method (util class already exists for that purpose) --- .../MethodGenerator/StaticProxyConstructor.php | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/src/ProxyManager/ProxyGenerator/NullObject/MethodGenerator/StaticProxyConstructor.php b/src/ProxyManager/ProxyGenerator/NullObject/MethodGenerator/StaticProxyConstructor.php index d16ca8546..4993b46d2 100644 --- a/src/ProxyManager/ProxyGenerator/NullObject/MethodGenerator/StaticProxyConstructor.php +++ b/src/ProxyManager/ProxyGenerator/NullObject/MethodGenerator/StaticProxyConstructor.php @@ -19,6 +19,7 @@ namespace ProxyManager\ProxyGenerator\NullObject\MethodGenerator; use ProxyManager\Generator\MethodGenerator; +use ProxyManager\ProxyGenerator\Util\Properties; use ReflectionClass; use ReflectionProperty; @@ -43,7 +44,7 @@ public function __construct(ReflectionClass $originalClass) function (ReflectionProperty $publicProperty) { return '$instance->' . $publicProperty->getName() . ' = null;'; }, - $this->getInstancePublicProperties($originalClass) + Properties::fromReflectionClass($originalClass)->getPublicProperties() ); $this->setDocblock("Constructor for null object initialization"); @@ -55,19 +56,4 @@ function (ReflectionProperty $publicProperty) { . 'return $instance;' ); } - - /** - * @param ReflectionClass $originalClass - * - * @return ReflectionProperty[] - */ - private function getInstancePublicProperties(ReflectionClass $originalClass) - { - return array_filter( - $originalClass->getProperties(ReflectionProperty::IS_PUBLIC), - function (ReflectionProperty $property) { - return ! $property->isStatic(); - } - ); - } } From ceacafff09376ae20a5d4e49f33b1745bb805aa7 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:11:28 +0100 Subject: [PATCH 05/22] Removing unused variable --- .../Functional/MultipleProxyGenerationTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/ProxyManagerTest/Functional/MultipleProxyGenerationTest.php b/tests/ProxyManagerTest/Functional/MultipleProxyGenerationTest.php index 9241f0f2a..67e1ca505 100644 --- a/tests/ProxyManagerTest/Functional/MultipleProxyGenerationTest.php +++ b/tests/ProxyManagerTest/Functional/MultipleProxyGenerationTest.php @@ -71,8 +71,7 @@ public function testCanGenerateMultipleDifferentProxiesForSameClass($className) $initializer = function () { }; - $reflectionClass = new ReflectionClass($className); - $generated = [ + $generated = [ $ghostProxyFactory->createProxy($className, $initializer), $virtualProxyFactory->createProxy($className, $initializer), $accessInterceptorFactory->createProxy(new $className()), From f4f0ca27e2a75b034dbc87c246929991f9ca7abd Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:11:46 +0100 Subject: [PATCH 06/22] Skipping static properties when binding properties in a scope localizer --- .../MethodGenerator/BindProxyProperties.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php b/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php index 3fe306946..299341b42 100644 --- a/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php +++ b/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php @@ -64,6 +64,10 @@ public function __construct( foreach ($originalClass->getProperties() as $originalProperty) { $propertyName = $originalProperty->getName(); + if ($originalProperty->isStatic()) { + continue; + } + if ($originalProperty->isPrivate()) { $localizedProperties[] = "\\Closure::bind(function () use (\$localizedObject) {\n " . '$this->' . $propertyName . ' = & $localizedObject->' . $propertyName . ";\n" From 70161a82c8a315d5d9102980fe57fdb828a70616 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:15:10 +0100 Subject: [PATCH 07/22] Refactoring code to use the `Properties` utility class rather than direct `ReflectionClass#getProperties()` access --- .../MethodGenerator/BindProxyProperties.php | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php b/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php index 299341b42..101fe42e3 100644 --- a/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php +++ b/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php @@ -20,6 +20,7 @@ use ProxyManager\Generator\MethodGenerator; use ProxyManager\Generator\ParameterGenerator; +use ProxyManager\ProxyGenerator\Util\Properties; use ReflectionClass; use Zend\Code\Generator\PropertyGenerator; @@ -61,21 +62,22 @@ public function __construct( $localizedProperties = []; - foreach ($originalClass->getProperties() as $originalProperty) { - $propertyName = $originalProperty->getName(); + $properties = Properties::fromReflectionClass($originalClass); - if ($originalProperty->isStatic()) { - continue; - } + foreach ($properties->getPrivateProperties() as $property) { + $propertyName = $property->getName(); - if ($originalProperty->isPrivate()) { - $localizedProperties[] = "\\Closure::bind(function () use (\$localizedObject) {\n " - . '$this->' . $propertyName . ' = & $localizedObject->' . $propertyName . ";\n" - . '}, $this, ' . var_export($originalProperty->getDeclaringClass()->getName(), true) - . ')->__invoke();'; - } else { - $localizedProperties[] = '$this->' . $propertyName . ' = & $localizedObject->' . $propertyName . ";"; - } + $localizedProperties[] = "\\Closure::bind(function () use (\$localizedObject) {\n " + . '$this->' . $propertyName . ' = & $localizedObject->' . $propertyName . ";\n" + . '}, $this, ' . var_export($property->getDeclaringClass()->getName(), true) + . ')->__invoke();'; + } + + /* @var $property \ReflectionProperty */ + foreach (array_merge($properties->getPublicProperties(), $properties->getProtectedProperties()) as $property) { + $propertyName = $property->getName(); + + $localizedProperties[] = '$this->' . $propertyName . ' = & $localizedObject->' . $propertyName . ";"; } $this->setDocblock( From a81dc9a00cc11344881af293238a7cc7926f67ea Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:16:00 +0100 Subject: [PATCH 08/22] Simplified code via existing utility API --- .../MethodGenerator/BindProxyProperties.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php b/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php index 101fe42e3..e82e55ef7 100644 --- a/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php +++ b/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php @@ -73,8 +73,7 @@ public function __construct( . ')->__invoke();'; } - /* @var $property \ReflectionProperty */ - foreach (array_merge($properties->getPublicProperties(), $properties->getProtectedProperties()) as $property) { + foreach ($properties->getAccessibleProperties() as $property) { $propertyName = $property->getName(); $localizedProperties[] = '$this->' . $propertyName . ' = & $localizedObject->' . $propertyName . ";"; From 327aedd70ef8c3fe7204285631898c411d4841b1 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:16:58 +0100 Subject: [PATCH 09/22] Private properties should be handled after accessible properties, as per code specification --- .../MethodGenerator/BindProxyProperties.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php b/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php index e82e55ef7..17a01b0ab 100644 --- a/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php +++ b/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php @@ -64,6 +64,12 @@ public function __construct( $properties = Properties::fromReflectionClass($originalClass); + foreach ($properties->getAccessibleProperties() as $property) { + $propertyName = $property->getName(); + + $localizedProperties[] = '$this->' . $propertyName . ' = & $localizedObject->' . $propertyName . ";"; + } + foreach ($properties->getPrivateProperties() as $property) { $propertyName = $property->getName(); @@ -73,12 +79,6 @@ public function __construct( . ')->__invoke();'; } - foreach ($properties->getAccessibleProperties() as $property) { - $propertyName = $property->getName(); - - $localizedProperties[] = '$this->' . $propertyName . ' = & $localizedObject->' . $propertyName . ";"; - } - $this->setDocblock( "@override constructor to setup interceptors\n\n" . "@param \\" . $originalClass->getName() . " \$localizedObject\n" From 45e652e0c5c31a0c57193758066497203b2e5e7b Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:20:10 +0100 Subject: [PATCH 10/22] Simplified structure of the method generator by using the parent class' constructor --- .../MethodGenerator/BindProxyProperties.php | 35 ++++++++----------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php b/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php index 17a01b0ab..7196de5e0 100644 --- a/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php +++ b/src/ProxyManager/ProxyGenerator/AccessInterceptorScopeLocalizer/MethodGenerator/BindProxyProperties.php @@ -44,21 +44,20 @@ public function __construct( PropertyGenerator $prefixInterceptors, PropertyGenerator $suffixInterceptors ) { - parent::__construct('bindProxyProperties', [], static::FLAG_PRIVATE); - - $localizedObject = new ParameterGenerator('localizedObject'); - $prefix = new ParameterGenerator('prefixInterceptors'); - $suffix = new ParameterGenerator('suffixInterceptors'); - - $localizedObject->setType($originalClass->getName()); - $prefix->setDefaultValue([]); - $suffix->setDefaultValue([]); - $prefix->setType('array'); - $suffix->setType('array'); - - $this->setParameter($localizedObject); - $this->setParameter($prefix); - $this->setParameter($suffix); + parent::__construct( + 'bindProxyProperties', + [ + new ParameterGenerator('localizedObject', $originalClass->getName()), + new ParameterGenerator('prefixInterceptors', 'array', []), + new ParameterGenerator('suffixInterceptors', 'array', []), + ], + static::FLAG_PRIVATE, + null, + "@override constructor to setup interceptors\n\n" + . "@param \\" . $originalClass->getName() . " \$localizedObject\n" + . "@param \\Closure[] \$prefixInterceptors method interceptors to be used before method logic\n" + . "@param \\Closure[] \$suffixInterceptors method interceptors to be used before method logic" + ); $localizedProperties = []; @@ -79,12 +78,6 @@ public function __construct( . ')->__invoke();'; } - $this->setDocblock( - "@override constructor to setup interceptors\n\n" - . "@param \\" . $originalClass->getName() . " \$localizedObject\n" - . "@param \\Closure[] \$prefixInterceptors method interceptors to be used before method logic\n" - . "@param \\Closure[] \$suffixInterceptors method interceptors to be used before method logic" - ); $this->setBody( (empty($localizedProperties) ? '' : implode("\n\n", $localizedProperties) . "\n\n") . '$this->' . $prefixInterceptors->getName() . " = \$prefixInterceptors;\n" From ed6cd092dceef7b803f1ae33d6a4e09b5ae99b50 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:22:38 +0100 Subject: [PATCH 11/22] Using `Properties` utility class instead of direct `ReflectionClass` usage --- .../MethodGenerator/StaticProxyConstructor.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/ProxyManager/ProxyGenerator/RemoteObject/MethodGenerator/StaticProxyConstructor.php b/src/ProxyManager/ProxyGenerator/RemoteObject/MethodGenerator/StaticProxyConstructor.php index f57271600..02744b261 100644 --- a/src/ProxyManager/ProxyGenerator/RemoteObject/MethodGenerator/StaticProxyConstructor.php +++ b/src/ProxyManager/ProxyGenerator/RemoteObject/MethodGenerator/StaticProxyConstructor.php @@ -21,6 +21,7 @@ use ProxyManager\Factory\RemoteObject\AdapterInterface; use ProxyManager\Generator\MethodGenerator; use ProxyManager\Generator\ParameterGenerator; +use ProxyManager\ProxyGenerator\Util\Properties; use ReflectionClass; use Zend\Code\Generator\PropertyGenerator; @@ -61,10 +62,8 @@ public function __construct(ReflectionClass $originalClass, PropertyGenerator $a . '$instance = (new \ReflectionClass(get_class()))->newInstanceWithoutConstructor();' . "\n\n" . '$instance->' . $adapterName . ' = $' . $adapterName . ';'; - foreach ($originalClass->getProperties() as $property) { - if ($property->isPublic() && ! $property->isStatic()) { - $body .= "\nunset(\$instance->" . $property->getName() . ');'; - } + foreach (Properties::fromReflectionClass($originalClass)->getPublicProperties() as $property) { + $body .= "\nunset(\$instance->" . $property->getName() . ');'; } $this->setBody($body . "\n\nreturn \$instance;"); From 7d3b226f8850fcf1184810b73e0d5e201fb96978 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:24:03 +0100 Subject: [PATCH 12/22] Refactoring logic to avoid useless mutation --- .../MethodGenerator/StaticProxyConstructor.php | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/ProxyManager/ProxyGenerator/RemoteObject/MethodGenerator/StaticProxyConstructor.php b/src/ProxyManager/ProxyGenerator/RemoteObject/MethodGenerator/StaticProxyConstructor.php index 02744b261..8aebae0de 100644 --- a/src/ProxyManager/ProxyGenerator/RemoteObject/MethodGenerator/StaticProxyConstructor.php +++ b/src/ProxyManager/ProxyGenerator/RemoteObject/MethodGenerator/StaticProxyConstructor.php @@ -42,17 +42,13 @@ class StaticProxyConstructor extends MethodGenerator */ public function __construct(ReflectionClass $originalClass, PropertyGenerator $adapter) { - parent::__construct( - 'staticProxyConstructor', - [], - MethodGenerator::FLAG_PUBLIC | MethodGenerator::FLAG_STATIC - ); - $adapterName = $adapter->getName(); - $this->setParameter(new ParameterGenerator($adapterName, AdapterInterface::class)); - - $this->setDocblock( + parent::__construct( + 'staticProxyConstructor', + [new ParameterGenerator($adapterName, AdapterInterface::class)], + MethodGenerator::FLAG_PUBLIC | MethodGenerator::FLAG_STATIC, + null, 'Constructor for remote object control\n\n' . '@param \\ProxyManager\\Factory\\RemoteObject\\AdapterInterface \$adapter' ); From 23a1c1239d69c6217942bc369e7145a2352a5456 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:26:17 +0100 Subject: [PATCH 13/22] Using `Properties` utility class instead of direct `ReflectionClass` usage --- .../MethodGenerator/MagicWakeup.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/ProxyManager/ProxyGenerator/AccessInterceptor/MethodGenerator/MagicWakeup.php b/src/ProxyManager/ProxyGenerator/AccessInterceptor/MethodGenerator/MagicWakeup.php index 11a35eb65..eb4abba8d 100644 --- a/src/ProxyManager/ProxyGenerator/AccessInterceptor/MethodGenerator/MagicWakeup.php +++ b/src/ProxyManager/ProxyGenerator/AccessInterceptor/MethodGenerator/MagicWakeup.php @@ -19,6 +19,7 @@ namespace ProxyManager\ProxyGenerator\AccessInterceptor\MethodGenerator; use ProxyManager\Generator\MagicMethodGenerator; +use ProxyManager\ProxyGenerator\Util\Properties; use ReflectionClass; use ReflectionProperty; @@ -39,13 +40,12 @@ public function __construct(ReflectionClass $originalClass) { parent::__construct($originalClass, '__wakeup'); - /* @var $publicProperties \ReflectionProperty[] */ - $publicProperties = $originalClass->getProperties(ReflectionProperty::IS_PUBLIC); - $unsetProperties = []; - - foreach ($publicProperties as $publicProperty) { - $unsetProperties[] = '$this->' . $publicProperty->getName(); - } + $unsetProperties = array_map( + function (ReflectionProperty $property) { + return '$this->' . $property->getName(); + }, + Properties::fromReflectionClass($originalClass)->getPublicProperties() + ); $this->setBody($unsetProperties ? 'unset(' . implode(', ', $unsetProperties) . ");" : ''); } From 729774a0f7dc8c0c2ded83e68e463c214e6a663f Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:27:21 +0100 Subject: [PATCH 14/22] Using `Properties` utility class instead of direct `ReflectionClass` usage --- .../MethodGenerator/StaticProxyConstructor.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/StaticProxyConstructor.php b/src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/StaticProxyConstructor.php index 4b8c17c9b..6f9623ee2 100644 --- a/src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/StaticProxyConstructor.php +++ b/src/ProxyManager/ProxyGenerator/AccessInterceptorValueHolder/MethodGenerator/StaticProxyConstructor.php @@ -20,6 +20,7 @@ use ProxyManager\Generator\MethodGenerator; use ProxyManager\Generator\ParameterGenerator; +use ProxyManager\ProxyGenerator\Util\Properties; use ReflectionClass; use ReflectionProperty; use Zend\Code\Generator\PropertyGenerator; @@ -60,8 +61,7 @@ public function __construct( $this->setParameter($prefix); $this->setParameter($suffix); - /* @var $publicProperties \ReflectionProperty[] */ - $publicProperties = $originalClass->getProperties(ReflectionProperty::IS_PUBLIC); + $publicProperties = Properties::fromReflectionClass($originalClass)->getPublicProperties(); $unsetProperties = []; foreach ($publicProperties as $publicProperty) { From 300635376c49165a7e9305515b255593ecd99ee3 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:34:17 +0100 Subject: [PATCH 15/22] `PublicPropertiesMap` should ignore static properties --- .../PropertyGenerator/PublicPropertiesMapTest.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/ProxyManagerTest/ProxyGenerator/PropertyGenerator/PublicPropertiesMapTest.php b/tests/ProxyManagerTest/ProxyGenerator/PropertyGenerator/PublicPropertiesMapTest.php index 70b131ebf..665006810 100644 --- a/tests/ProxyManagerTest/ProxyGenerator/PropertyGenerator/PublicPropertiesMapTest.php +++ b/tests/ProxyManagerTest/ProxyGenerator/PropertyGenerator/PublicPropertiesMapTest.php @@ -20,6 +20,7 @@ use PHPUnit_Framework_TestCase; use ProxyManager\ProxyGenerator\PropertyGenerator\PublicPropertiesMap; +use ProxyManagerTestAsset\ClassWithMixedProperties; use ProxyManagerTestAsset\ClassWithPublicProperties; use ProxyManagerTestAsset\EmptyClass; use ReflectionClass; @@ -58,4 +59,13 @@ public function testClassWithPublicProperties() $this->assertSame('private', $publicProperties->getVisibility()); $this->assertFalse($publicProperties->isEmpty()); } + + public function testClassWithMixedProperties() + { + $publicProperties = new PublicPropertiesMap( + new ReflectionClass(ClassWithMixedProperties::class) + ); + + $this->assertCount(3, $publicProperties->getDefaultValue()->getValue()); + } } From ff5afb8eaceebe7f1328b13fcffa8b4782b6313b Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:34:40 +0100 Subject: [PATCH 16/22] `PublicPropertiesMap` ignores static properties --- .../ProxyGenerator/PropertyGenerator/PublicPropertiesMap.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ProxyManager/ProxyGenerator/PropertyGenerator/PublicPropertiesMap.php b/src/ProxyManager/ProxyGenerator/PropertyGenerator/PublicPropertiesMap.php index b36bd0c67..1d0cbd3a1 100644 --- a/src/ProxyManager/ProxyGenerator/PropertyGenerator/PublicPropertiesMap.php +++ b/src/ProxyManager/ProxyGenerator/PropertyGenerator/PublicPropertiesMap.php @@ -19,6 +19,7 @@ namespace ProxyManager\ProxyGenerator\PropertyGenerator; use ProxyManager\Generator\Util\UniqueIdentifierGenerator; +use ProxyManager\ProxyGenerator\Util\Properties; use ReflectionClass; use ReflectionProperty; use Zend\Code\Generator\PropertyGenerator; @@ -43,7 +44,7 @@ public function __construct(ReflectionClass $originalClass) { parent::__construct(UniqueIdentifierGenerator::getIdentifier('publicProperties')); - foreach ($originalClass->getProperties(ReflectionProperty::IS_PUBLIC) as $publicProperty) { + foreach (Properties::fromReflectionClass($originalClass)->getPublicProperties() as $publicProperty) { $this->publicProperties[$publicProperty->getName()] = true; } From 3a91ae64d81d478ca1f9060ef274c017257b3b57 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:39:19 +0100 Subject: [PATCH 17/22] `Properties#getInstanceProperties()` should provide a list of all non-static class properties --- .../ProxyGenerator/Util/PropertiesTest.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/ProxyManagerTest/ProxyGenerator/Util/PropertiesTest.php b/tests/ProxyManagerTest/ProxyGenerator/Util/PropertiesTest.php index dfe1a46fa..7eaadbb33 100644 --- a/tests/ProxyManagerTest/ProxyGenerator/Util/PropertiesTest.php +++ b/tests/ProxyManagerTest/ProxyGenerator/Util/PropertiesTest.php @@ -186,4 +186,13 @@ public function testGetGroupedPrivatePropertiesWithInheritedProperties() $this->assertInstanceOf(ReflectionProperty::class, $group2['property8']); $this->assertInstanceOf(ReflectionProperty::class, $group2['property9']); } + + public function testGetInstanceProperties() + { + $properties = Properties::fromReflectionClass( + new ReflectionClass(ClassWithMixedProperties::class) + ); + + $this->assertCount(9, $properties->getInstanceProperties()); + } } From b22d1655caaf690cdc9a494178d5bffc8b712073 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:39:33 +0100 Subject: [PATCH 18/22] `Properties#getInstanceProperties()` provides a list of all non-static class properties --- src/ProxyManager/ProxyGenerator/Util/Properties.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/ProxyManager/ProxyGenerator/Util/Properties.php b/src/ProxyManager/ProxyGenerator/Util/Properties.php index 95496b542..0f4080eab 100644 --- a/src/ProxyManager/ProxyGenerator/Util/Properties.php +++ b/src/ProxyManager/ProxyGenerator/Util/Properties.php @@ -142,4 +142,12 @@ public function getGroupedPrivateProperties() return $propertiesMap; } + + /** + * @return ReflectionProperty[] indexed by the property internal visibility-aware name (\0*\0propertyName) + */ + public function getInstanceProperties() + { + return array_merge($this->getAccessibleProperties(), $this->getPrivateProperties()); + } } From e942958f37427b4a707b27ebc8154e47c3b1fa27 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:41:11 +0100 Subject: [PATCH 19/22] Test code should only verify instance properties --- .../Functional/LazyLoadingGhostFunctionalTest.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/ProxyManagerTest/Functional/LazyLoadingGhostFunctionalTest.php b/tests/ProxyManagerTest/Functional/LazyLoadingGhostFunctionalTest.php index aa1d33703..6e0fd4e90 100644 --- a/tests/ProxyManagerTest/Functional/LazyLoadingGhostFunctionalTest.php +++ b/tests/ProxyManagerTest/Functional/LazyLoadingGhostFunctionalTest.php @@ -27,6 +27,7 @@ use ProxyManager\GeneratorStrategy\FileWriterGeneratorStrategy; use ProxyManager\Proxy\GhostObjectInterface; use ProxyManager\ProxyGenerator\LazyLoadingGhostGenerator; +use ProxyManager\ProxyGenerator\Util\Properties; use ProxyManagerTestAsset\BaseClass; use ProxyManagerTestAsset\ClassWithCollidingPrivateInheritedProperties; use ProxyManagerTestAsset\ClassWithCounterConstructor; @@ -573,7 +574,9 @@ function ($proxy, $method, $params, & $initializer, array $properties) { } ); - foreach ((new ReflectionClass(ClassWithMixedProperties::class))->getProperties() as $property) { + $reflectionClass = new ReflectionClass(ClassWithMixedProperties::class); + + foreach (Properties::fromReflectionClass($reflectionClass)->getInstanceProperties() as $property) { $property->setAccessible(true); $this->assertSame(str_replace('Property', '', $property->getName()), $property->getValue($proxy)); From 08b66f912ca0d09572ea9c1ad0cb73d6860d3311 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:44:43 +0100 Subject: [PATCH 20/22] Value holder constructor should ignore static properties --- .../MethodGenerator/ConstructorTest.php | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/ProxyManagerTest/ProxyGenerator/ValueHolder/MethodGenerator/ConstructorTest.php b/tests/ProxyManagerTest/ProxyGenerator/ValueHolder/MethodGenerator/ConstructorTest.php index f9184d14e..2fa0d7d64 100644 --- a/tests/ProxyManagerTest/ProxyGenerator/ValueHolder/MethodGenerator/ConstructorTest.php +++ b/tests/ProxyManagerTest/ProxyGenerator/ValueHolder/MethodGenerator/ConstructorTest.php @@ -20,6 +20,7 @@ use PHPUnit_Framework_TestCase; use ProxyManager\ProxyGenerator\ValueHolder\MethodGenerator\Constructor; +use ProxyManagerTestAsset\ClassWithMixedProperties; use ProxyManagerTestAsset\EmptyClass; use ProxyManagerTestAsset\ProxyGenerator\LazyLoading\MethodGenerator\ClassWithTwoPublicProperties; use ReflectionClass; @@ -126,4 +127,34 @@ public function testBodyStructureWithPhp4StyleConstructor() $constructor->getBody() ); } + + public function testBodyStructureWithStaticProperties() + { + /* @var $valueHolder PropertyGenerator|\PHPUnit_Framework_MockObject_MockObject */ + $valueHolder = $this->getMock(PropertyGenerator::class); + + $valueHolder->expects($this->any())->method('getName')->will($this->returnValue('foo')); + + $constructor = Constructor::generateMethod(new ReflectionClass(ClassWithMixedProperties::class), $valueHolder); + + $this->assertSame('__construct', $constructor->getName()); + $this->assertCount(0, $constructor->getParameters()); + + $expectedCode = <<<'PHP' +static $reflection; + +if (! $this->foo) { + $reflection = $reflection ?: new \ReflectionClass('ProxyManagerTestAsset\\ClassWithMixedProperties'); + $this->foo = $reflection->newInstanceWithoutConstructor(); + + unset($this->publicProperty0); + unset($this->publicProperty1); + unset($this->publicProperty2); +} + +$this->foo->__construct(); +PHP; + + $this->assertSame($expectedCode, $constructor->getBody()); + } } From 0c55819a6a6100c9392513d637333e6580743231 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:45:09 +0100 Subject: [PATCH 21/22] Skipping static properties in constructor codegen --- .../ProxyGenerator/ValueHolder/MethodGenerator/Constructor.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ProxyManager/ProxyGenerator/ValueHolder/MethodGenerator/Constructor.php b/src/ProxyManager/ProxyGenerator/ValueHolder/MethodGenerator/Constructor.php index be1c3414d..8243e9cbe 100644 --- a/src/ProxyManager/ProxyGenerator/ValueHolder/MethodGenerator/Constructor.php +++ b/src/ProxyManager/ProxyGenerator/ValueHolder/MethodGenerator/Constructor.php @@ -20,6 +20,7 @@ use ProxyManager\Generator\MethodGenerator; use ProxyManager\Generator\ParameterGenerator; +use ProxyManager\ProxyGenerator\Util\Properties; use ReflectionClass; use ReflectionProperty; use Zend\Code\Generator\PropertyGenerator; @@ -88,7 +89,7 @@ private static function getUnsetPropertiesString(ReflectionClass $class) function (ReflectionProperty $unsetProperty) { return 'unset($this->' . $unsetProperty->getName() . ');'; }, - $class->getProperties(ReflectionProperty::IS_PUBLIC) + Properties::fromReflectionClass($class)->getPublicProperties() ) ); From e4eeab8546d16c27689c4ed28b9fad086e00c50d Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 2 Jan 2015 15:49:28 +0100 Subject: [PATCH 22/22] Using `Properties` utility instead of direct reflection also for test classes --- .../AccessInterceptorScopeLocalizerFunctionalTest.php | 3 ++- .../Functional/LazyLoadingGhostFunctionalTest.php | 2 +- .../Functional/LazyLoadingGhostPerformanceTest.php | 3 ++- .../ProxyGenerator/NullObjectGeneratorTest.php | 3 ++- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/ProxyManagerTest/Functional/AccessInterceptorScopeLocalizerFunctionalTest.php b/tests/ProxyManagerTest/Functional/AccessInterceptorScopeLocalizerFunctionalTest.php index f9e14dee9..679605eb0 100644 --- a/tests/ProxyManagerTest/Functional/AccessInterceptorScopeLocalizerFunctionalTest.php +++ b/tests/ProxyManagerTest/Functional/AccessInterceptorScopeLocalizerFunctionalTest.php @@ -25,6 +25,7 @@ use ProxyManager\GeneratorStrategy\EvaluatingGeneratorStrategy; use ProxyManager\Proxy\AccessInterceptorInterface; use ProxyManager\ProxyGenerator\AccessInterceptorScopeLocalizerGenerator; +use ProxyManager\ProxyGenerator\Util\Properties; use ProxyManagerTestAsset\BaseClass; use ProxyManagerTestAsset\ClassWithCounterConstructor; use ProxyManagerTestAsset\ClassWithPublicArrayProperty; @@ -387,7 +388,7 @@ private function assertProxySynchronized($instance, AccessInterceptorInterface $ { $reflectionClass = new ReflectionClass($instance); - foreach ($reflectionClass->getProperties() as $property) { + foreach (Properties::fromReflectionClass($reflectionClass)->getInstanceProperties() as $property) { $property->setAccessible(true); $this->assertSame( diff --git a/tests/ProxyManagerTest/Functional/LazyLoadingGhostFunctionalTest.php b/tests/ProxyManagerTest/Functional/LazyLoadingGhostFunctionalTest.php index 6e0fd4e90..f70bd0113 100644 --- a/tests/ProxyManagerTest/Functional/LazyLoadingGhostFunctionalTest.php +++ b/tests/ProxyManagerTest/Functional/LazyLoadingGhostFunctionalTest.php @@ -665,7 +665,7 @@ private function createInitializer($className, $realInstance, Mock $initializerM $initializer = null; $reflectionClass = new ReflectionClass($realInstance); - foreach ($reflectionClass->getProperties() as $property) { + foreach (Properties::fromReflectionClass($reflectionClass)->getInstanceProperties() as $property) { $property->setAccessible(true); $property->setValue($proxy, $property->getValue($realInstance)); } diff --git a/tests/ProxyManagerTest/Functional/LazyLoadingGhostPerformanceTest.php b/tests/ProxyManagerTest/Functional/LazyLoadingGhostPerformanceTest.php index 379480a35..9e48d1bbd 100644 --- a/tests/ProxyManagerTest/Functional/LazyLoadingGhostPerformanceTest.php +++ b/tests/ProxyManagerTest/Functional/LazyLoadingGhostPerformanceTest.php @@ -23,6 +23,7 @@ use ProxyManager\GeneratorStrategy\EvaluatingGeneratorStrategy; use ProxyManager\Proxy\GhostObjectInterface; use ProxyManager\ProxyGenerator\LazyLoadingGhostGenerator; +use ProxyManager\ProxyGenerator\Util\Properties; use ProxyManagerTestAsset\BaseClass; use ReflectionClass; use stdClass; @@ -128,7 +129,7 @@ public function getTestedClasses() $reflectionProperties = []; $reflectionClass = new ReflectionClass($testedClass[0]); - foreach ($reflectionClass->getProperties() as $property) { + foreach (Properties::fromReflectionClass($reflectionClass)->getInstanceProperties() as $property) { $property->setAccessible(true); $reflectionProperties[$property->getName()] = $property; diff --git a/tests/ProxyManagerTest/ProxyGenerator/NullObjectGeneratorTest.php b/tests/ProxyManagerTest/ProxyGenerator/NullObjectGeneratorTest.php index 5f272bfab..6df43802c 100644 --- a/tests/ProxyManagerTest/ProxyGenerator/NullObjectGeneratorTest.php +++ b/tests/ProxyManagerTest/ProxyGenerator/NullObjectGeneratorTest.php @@ -24,6 +24,7 @@ use ProxyManager\GeneratorStrategy\EvaluatingGeneratorStrategy; use ProxyManager\Proxy\NullObjectInterface; use ProxyManager\ProxyGenerator\NullObjectGenerator; +use ProxyManager\ProxyGenerator\Util\Properties; use ProxyManagerTestAsset\BaseClass; use ProxyManagerTestAsset\BaseInterface; use ProxyManagerTestAsset\ClassWithByRefMagicMethods; @@ -74,7 +75,7 @@ public function testGeneratesValidCode($className) $proxyGenerated = $generatedClassName::staticProxyConstructor(); - foreach ($generatedReflection->getProperties(ReflectionProperty::IS_PUBLIC) as $property) { + foreach (Properties::fromReflectionClass($generatedReflection)->getPublicProperties() as $property) { $this->assertNull($proxyGenerated->$property); }