On Sat, May 16, 2026 at 6:24 AM Tim Düsterhus <[email protected]> wrote:
> Hi > > Am 2026-05-12 22:37, schrieb Benjamin Außenhofer: > > I am not convinced this is needed. At every call site of > > $reflector->getAttributes() you could inject the reflector back into > > the > > attributes. > > I agree with Benjamin here and actually would go even further: Making > attribute instances aware of their target feels like a layering > violation. Attributes are intended to provide metadata, not behavior. > The behavior can then be added by whoever is consuming the attribute. > > The RFC itself contains one example with two possible use cases: > > - Further narrowing down TARGET_CLASS targets. For that I feel the > correct solution would be further splitting the target constants into > TARGET_CLASS_ONLY, TARGET_INTERFACE, TARGET_TRAIT, etc. > Currently, internal validators narrow things down in the following ways beyond TARGET_CLASS: * #[\AllowDynamicProperties]: prohibits usage on traits, interfaces, readonly classes, and enums ( https://github.com/php/php-src/blob/3ed80a154d589e3ea1b02f43fa1899ea9c46d70f/Zend/zend_attributes.c#L72 ) * #[\Attribute]: prohibits usage on traits, interfaces, enums, and abstract classes ( https://github.com/php/php-src/blob/3ed80a154d589e3ea1b02f43fa1899ea9c46d70f/Zend/zend_attributes.c#L93 ) * #[\Deprecated]: prohibits usage on anything other than traits ( https://github.com/php/php-src/blob/3ed80a154d589e3ea1b02f43fa1899ea9c46d70f/Zend/zend_attributes.c#L112 ) also * #[\NoDiscard]: prohibits usage on property hooks ( https://github.com/php/php-src/blob/3ed80a154d589e3ea1b02f43fa1899ea9c46d70f/Zend/zend_attributes.c#L235 ) If we wanted to split up the TARGET_* constants to provide more granularity, we would need, at the very least * TARGET_TRAIT * TARGET_INTERFACE * TARGET_ENUM * TARGET_CLASS_ONLY but then TARGET_CLASS_ONLY would not be enough to distinguish readonly classes or abstract classes. Would we want to add dedicated constants for each of those combinations? Not to mention the fact that developers might want some other kind of validation (e.g. attribute can only be used on classes that implement some specific interface). Allowing the attribute class to do the validation itself allows much more flexibility. > > - Adding side-effects to a constructor, specifically side-effects that > need to rely on global state. This is the layering violation I mentioned > above: This kind of logic should be performed by the service that is > reading out and constructing the attribute - something that necessarily > exists -, not by the attribute itself. > Then maybe that was a bad example. Let's take a look at e.g. the symfony/console #[Option] attribute (docs: https://symfony.com/doc/current/components/console/console_arguments.html#option-attribute-constraints), which currently ( https://github.com/symfony/symfony/blob/b01d14a27dcd5ca91c5af11ca3cb41ffbe639de7/src/Symfony/Component/Console/Attribute/Option.php) uses a static constructor `tryFrom()` with a ReflectionParameter or ReflectionProperty. That static constructor does a bunch of validation, but does not rely on global state * validating that options have default values * validating the type of the property/parameter that the attribute is applied to * validating that boolean options not be nullable when the default is a boolean * validating that nullable options have null as the default Trying to encode all of this in TARGET_* flags might be possible, but there are other attributes with different considerations that would need slightly different flags available, and eventually the set of flags would be too much. On the other hand, the current validation is entirely achieved by providing a reflection reference to what the attribute is applied to, which is what the RFC proposes. -Daniel
