From fe5cce28845c8c3e81448f2312847ee0f5f043a1 Mon Sep 17 00:00:00 2001 From: Margus Kaidja Date: Mon, 7 Nov 2022 13:26:32 +0200 Subject: [PATCH] Fix inheritance bug where property attributes werent inherited * Fix bug, where @property[<-read>|<-write>] tags were not inherited. * Remove support for providing accessors to private properties. * Remove support for private endpoints. --- README.md | 8 +++- src/ClassConf.php | 5 ++- src/Properties.php | 45 +++++++++++++------ tests/DocBlockTest.php | 15 +++++++ .../ParentTestClassWithPropertyAttributes.php | 33 ++++++++++++++ 5 files changed, 89 insertions(+), 17 deletions(-) create mode 100644 tests/ParentTestClassWithPropertyAttributes.php diff --git a/README.md b/README.md index 2ff6766..22c8c21 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ [![Tests](https://github.com/marguskaidja/php-accessors/actions/workflows/tests.yml/badge.svg)](https://github.com/marguskaidja/php-accessors/actions/workflows/tests.yml) # Accessors -Current library can create automatic accessors (e.g. _getters_ and _setters_) for object properties. It works by injecting a trait with [magic methods for property overloading](https://www.php.net/manual/en/language.oop5.overloading.php#language.oop5.overloading.members) into desired class, which will then handle situations, where _inaccessible_ (`private`/`protected`) property is accessed. +Current library can create automatic accessors (e.g. _getters_ and _setters_) for object properties. It works by injecting a trait with [magic methods for property overloading](https://www.php.net/manual/en/language.oop5.overloading.php#language.oop5.overloading.members) into desired class, which will then handle situations, where _inaccessible_ (aka `protected`) property is accessed. #### Features * Multiple accessor **syntaxes**: @@ -361,11 +361,12 @@ The 2 endpoints (`getFoo`/`setFoo`) will be called in every situation: * either when property is accessed with direct assignment syntax (e.g. `$a->foo`) * or when property is accessed with method syntax (e.g. `$a->getFoo()`) * if the visibility of endpoint is `public`, then it's naturally called by PHP engine. - * if it's `private`/`protected`, then it goes through `__call` magic method provided by `Accessible` trait. + * if it's `protected`, then it goes through `__call` magic method provided by `Accessible` trait. Notes: * Endpoint methods must start with string `set`, `get`, `isset`, `unset` or `with` and followed with property name. * Only instance methods are detected, `static` methods wont work. +* Only `public` or `protected` methods are detected, `private` methods wont work. * _mutator_ is bypassed and should be done inside the setter endpoint itself. * Return values from endpoints are handled as following. Values from: * `get` and `isset` are handed over to caller. @@ -572,6 +573,9 @@ Since `@property[<-read>|<-write>]` tags act also in exposing properties, you ge * `#[Immutable]`: turns an property or whole class immutable. When used on a class, then it must be defined on top of hierarchy. Once defined, it can't be disabled later. * `#[Format(class-string)]`: allows customization of accessor method names. +Notes: +* Only `protected` properties are supported. Due current library's implementation, `private` properties may introduce unexpected behaviour and anomalies in inheritance chain and are disregarded this time. + ### Accessor methods: #### Reading properties: diff --git a/src/ClassConf.php b/src/ClassConf.php index 74028f8..4ae725e 100644 --- a/src/ClassConf.php +++ b/src/ClassConf.php @@ -115,6 +115,7 @@ private function __construct( } $this->attributes = $this->attributes->mergeWithParent($parent->attributes); + $parentProperties = $parent->properties; } else { /** * Since this is the top of the hierachy using "Accessible" trait, assign default instance @@ -124,9 +125,11 @@ private function __construct( Format::class, new Format(Standard::class) ); + + $parentProperties = null; } - $this->properties = Properties::fromReflection($rfClass, $this->attributes); + $this->properties = Properties::fromReflection($rfClass, $this->attributes, $parentProperties); $this->getter = $this->createGetter(); $this->setter = $this->createSetter(); diff --git a/src/Properties.php b/src/Properties.php index 6a15ab5..a89322e 100644 --- a/src/Properties.php +++ b/src/Properties.php @@ -39,10 +39,15 @@ class Properties /** * @param ReflectionClass $rfClass * @param Attributes $classAttributes + * @param Properties|null $parentProperties * * @return self */ - public static function fromReflection(ReflectionClass $rfClass, Attributes $classAttributes): self + public static function fromReflection( + ReflectionClass $rfClass, + Attributes $classAttributes, + ?Properties $parentProperties + ): self { $that = new self(); @@ -62,10 +67,13 @@ public static function fromReflection(ReflectionClass $rfClass, Attributes $clas foreach ( $rfClass->getMethods( - ReflectionMethod::IS_PROTECTED | ReflectionMethod::IS_PRIVATE | ReflectionMethod::IS_PUBLIC + ReflectionMethod::IS_PROTECTED | ReflectionMethod::IS_PUBLIC ) as $rfMethod ) { - if ($rfMethod->isStatic()) { + if ( + $rfMethod->isStatic() + || $rfClass->name !== $rfMethod->getDeclaringClass()->name + ) { continue; } @@ -87,25 +95,34 @@ public static function fromReflection(ReflectionClass $rfClass, Attributes $clas */ foreach ( $rfClass->getProperties( - ReflectionProperty::IS_PRIVATE | ReflectionProperty::IS_PROTECTED | ReflectionProperty::IS_PUBLIC + ReflectionProperty::IS_PROTECTED | ReflectionProperty::IS_PUBLIC ) as $rfProperty ) { $name = $rfProperty->getName(); $nameLowerCase = strtolower($name); - if (isset($docBlockAttributes[$name])) { - $attributes = $docBlockAttributes[$name]->mergeWithParent($classAttributes); + if ( + null === $parentProperties + || $rfClass->name === $rfProperty->getDeclaringClass()->name + ) { + if (isset($docBlockAttributes[$name])) { + $attributes = $docBlockAttributes[$name]->mergeWithParent($classAttributes); + } else { + $attributes = $classAttributes; + } + + $p = new Property( + $rfProperty, + $attributes, + ($accessorEndpoints[$nameLowerCase] ?? []) + ); } else { - $attributes = $classAttributes; + $p = $parentProperties->findConf($name); } - $p = new Property( - $rfProperty, - $attributes, - ($accessorEndpoints[$nameLowerCase] ?? []) - ); - - $that->propertiesByLowerCase[$nameLowerCase] = ($that->properties[$name] = $p); + if (null !== $p) { + $that->propertiesByLowerCase[$nameLowerCase] = ($that->properties[$name] = $p); + } } return $that; diff --git a/tests/DocBlockTest.php b/tests/DocBlockTest.php index 7763c38..7581046 100644 --- a/tests/DocBlockTest.php +++ b/tests/DocBlockTest.php @@ -102,4 +102,19 @@ public function getP1Value(): string */ $obj->p1; } + + public function test_tag_in_parent_class_must_be_inherited(): void + { + $obj = new class extends ParentTestClassWithPropertyAttributes { + }; + + $expectedValue = 'new value'; + $obj->setParentProperty($expectedValue); + + $this->assertEquals( + $expectedValue, + $obj->getParentProperty() + ); + } + } diff --git a/tests/ParentTestClassWithPropertyAttributes.php b/tests/ParentTestClassWithPropertyAttributes.php new file mode 100644 index 0000000..483ae5a --- /dev/null +++ b/tests/ParentTestClassWithPropertyAttributes.php @@ -0,0 +1,33 @@ + + * @link https://github.com/marguskaidja/php-accessors + * @license http://www.opensource.org/licenses/mit-license.php MIT (see the LICENSE file) + */ + +declare(strict_types=1); + +namespace margusk\Accessors\Tests; + +use margusk\Accessors\Accessible; +use margusk\Accessors\Attr\Delete; +use margusk\Accessors\Attr\Get; +use margusk\Accessors\Attr\Set; + +use function htmlspecialchars; + +/** + * @property string $parentProperty + * + * @method string getParentProperty() + * @method self setParentProperty(string $value) + */ +class ParentTestClassWithPropertyAttributes +{ + use Accessible; + + protected string $parentProperty; +}