Re: Re: [RFC] [Discussion] Add ReflectionAttribute::getCurrent()

Am 07.06.2026, 06:29:17 schrieb Benjamin Außenhofer <kontakt@beberlei.de>: > > > Am 05.06.2026, 23:52:54 schrieb Daniel Scherzer < > daniel.e.scherzer@gmail.com>: > >> On Mon, May 4, 2026 at 1:24 PM Daniel Scherzer < >> daniel.e.scherzer@gmail.com> wrote: >> >>> Hi internals, >>> >>> I'd like to start the discussion for a new RFC about adding a new >>> method, ReflectionAttribute::getCurrent(), to access the current reflection >>> target of an attribute. >>> >>> * RFC: >>> https://wiki.php.net/rfc/reflectionattribute-getcurrent >>> * Implementation: >>> https://github.com/php/php-src/pull/21440 >>> >>> Thanks, >>> -Daniel >>> >> >> Barring any additional feedback, I plan to open the vote on this in the >> next few days. >> >> -Daniel >> > > Hi Daniel, > > sorry i could not find the time to reply earlier. the idea in general is > good, but i still very much disagree about the static method to get the > reflector context. > > An attribute is often passed around as a configuration object or hierachy > down to other services, for example in Symfony validator. > > #[NotNull] > public $foo; > > $validator = new NotNullValidator(); > $validator->validate($instance->foo, $notNullAttribute); > > For testing purposes it makes sense in these examples to construct the > attributes directly using new SomeAttribute(). Now with this proposal, you > need to be careful and know from the outside, when that is possible, or > when you need to call it through ReflectionAttribute::newInstance(). The > coupling becomes very strong suddenly. > > When you create the attributes yourself and without calling newInstance(), > then the method does not work at all: > > $class = $reflectionAttribute->getName(); > new $class(…$this->getArguments())); // ReflectionAttribute::getCurrent() > returns null in this ctor. > > This is something the RFC is not clear about fully. There is the > definition of an „attribute constructor“, but a constructor of an Attribute > can be called many ways. This proposal is about the ctor only when called > through newInstance(). > > I propose instead that we extend the newInstance() factory method, to be > more versatile, and use the benefit here that we already have a factory > method where we can place this logic: > > <?php > > interface ReflectionAwareAttribute > { > public function targetsReflector(Reflector $reflector); > } > > class ReflectionAttribute > { > public function newInstance() > { > $class = $this->getName(); > $instance = new $class(...$this->getArguments()); > > if ($instance instanceof ReflectionAwareAttribute) { > $instance->targetsReflector($this); > } > > return $instance; > } > } > > This is much cleaner OOP wise. > > > 1. Increases testability of attribute classes that are reflection > aware, only need a Reflector instance, no need to have actual attribute > example set up. > 2. Uses the existing factory method to concentrate the construction > logic into one place. > 3. avoids static method call with unclear semantics on when > „getCurrent()“ returns something. > 4. if a user of attributes has to avoid newInstance() (for whatever > reason), they can call targetsReflector() themselves. There is no way to > „setCurrent()“ reflector however, hiding part of this new behavior in the > engine, where it could be 100% user controlled. > > Just came up with another benefit of my alternative proposal, it can be polyfilled much easier, which Symfony as a big attribute user is certainly interested in. A polyfill can ship the interface, and then at every callsite of newInstance() add another call: if (PHP_VERSION_ID <= 80600 && method_exists($attribute, 'targetsReflector‘ )) { $attribute->targetsReflector($reflector); } > The only „downside" is that you cant make properties set in > targetsReflector() readonly. > > But from a semantical point of view, the attribute constructor should not > have „magic" access to the reflector just to satisfy some own desire to > achieve more readonly/value objects. And a class with a constructor calling > a static method to get state cannot be called „simple“ or „value object“ in > my opinion anyways. > > Take: > > #[Entity(repositoryClass: UserRepository::class, table: ‚users’)] > #[Column(type: Types::STRING, name: ‚foo‘, nullable: false)] > > This is just a different way of writing: > new Entity(repositoryClass: UserRepository::class, table: ‚users’) > new Column(type: Types::STRING, name: ‚foo‘, nullable: false) > > Basic understanding of this for any PHP developer makes population of the > global ReflectionAttribute::getCurrent() seeem „magical“, something we > should avoid. > > greetings > Benjamin > > >