On Sat, Jul 5, 2025, at 21:53, Daniel Scherzer wrote: > On Fri, Jul 4, 2025 at 4:50 PM Rob Landers <rob@bottled.codes> wrote: >> __ >> >> On Sun, Jun 22, 2025, at 22:00, Daniel Scherzer wrote: >>> On Tue, Jun 17, 2025 at 4:26 PM Daniel Scherzer >>> <daniel.e.scher...@gmail.com> wrote: >>>> Hi internals, >>>> >>>> I'd like to start the discussion for a new RFC about adding a >>>> `#[\DelayedTargetValidation]` attribute. >>>> >>>> * RFC: https://wiki.php.net/rfc/delayedtargetvalidation_attribute >>>> * Implementation: https://github.com/php/php-src/pull/18817 >>>> >>>> --Daniel >>> >>> >>> It seems a common point of discussion is the difference in behavior between >>> internal and userland attributes, so I wanted to clarify a few things: >>> >>> * Userland attributes are always metadata, in part because of the backwards >>> and forward compatibility that those attributes provide - the attribute >>> does not need to exist in order to be used, it just needs to exist (and >>> target the location) when you call ReflectionAttribute::newInstance() to >>> instantiate >>> * Internal attributes are a mix between metadata and a way to plug into the >>> engine. There were already existing ways to plug into the engine (e.g. >>> using magic methods or implementing the Countable or ArrayAccess >>> interfaces) but attributes provided a way to plug into the engine when you >>> don't just want to add a function override, but rather something else (like >>> indicating a parameter should be redacted in backtraces with >>> `#[\SensitiveParameter]`). It would probably be impossible to do that >>> securely with a userland attribute and userland error handler that manually >>> redacted things... >>> * Attributes were designed to have good compatibility - the syntax chosen >>> for PHP 8.0 was, in prior versions of PHP, used for comments - so any code >>> with attributes could also work in PHP 7.4, just without the metadata. >>> Similarly, by not validating userland attributes until they are accessed >>> with reflection, you can add an attribute that does not exist yet (e.g. if >>> using different versions of a library) with minimal complications. >>> * Internal attributes are validated at compile time - because they can be. >>> Internal attributes tell the engine to do something, and it makes sense (at >>> least to me) that if the engine cannot do that thing, there should be an >>> error without needing to wait for ReflectionAttribute::newInstance() to be >>> called. >>> >>> But, the validation of internal attributes at compile time means that they >>> lose the compatibility features of userland attributes, and that is what >>> this RFC is trying to address. To be clear, I think the difference in >>> behavior between userland and internal attributes is a) fundamentally based >>> on the difference in capabilities (pure metadata vs engine behavior) and b) >>> something that should not be changed without significant further >>> investigation. >> >> I don’t think this is quite right. Because non-existent attributes can be >> used, if you use attributes in your library, you already know to only ever, >> and I mean only ever, instantiate your own attributes. Attempting to >> instantiate someone else’s attributes can result in crashes. This means any >> delayed attributes will most likely never be validated by anything. >> > > Symfony instantiates arbitrary attributes, > https://github.com/symfony/symfony/pull/40307 - and while delayed attributes > may not end up being validated, that is a feature, not a bug - the goal is > that the validation not be performed at compile time, and so it was either > delay to runtime, or not do the validation at all. > >> >>> >>> This new `#[\DelayedTargetValidation]` is meant to simplify things, and to >>> partially unify the behavior of the errors - if you are getting any errors >>> from internal attributes and want to suppress them at compile time, just >>> add the new attribute everywhere. >>> >>> -Daniel >> >> I don’t think this is “forward compatibility”, this is completely disabling >> the attribute. >> >> — Rob > > > So I guess my use of "forward compatibility" was a bit confusing. This allows > a library to be backwards compatible (using PHP 8.6 attribute targets but > running on PHP 8.5 without errors) by making the language forward compatible > (allowing PHP 8.5 to process 8.6 attribute targets without errors, by > delaying those errors). The attribute is not disabled at all though. > > * If the attribute is used in a place where it would do anything (i.e. there > wouldn't be an error about being a bad target) then that attribute is used as > normal, and #[\DelayedTargetValidation] does not affect it > * If the attribute is used in a place where it would error, then it already > cannot do anything, and #[\DelayedTargetValidation] just delays the error - > the attribute is already going to be inactive anyway > > -Daniel >
Hmmm... I still don't get it. If you scan your codebase for these attributes (maybe using something like https://github.com/olvlvl/composer-attribute-collector) and then instantiate them ... then you are right back where you started. You'll end up getting the same validation errors you'd normally get. But I don't know why you'd do such a thing. You can use \NoDiscard today, but if you try to instantiate it -- it will crash without a polyfill. There are attributes used only by IDEs (like `\Pure`) and attempting to instantiate them will crash. This is why you don't instantiate attributes you don't own. By adding this, you force all framework to try instantiating these built-in attributes; and scanning for them. This increases the bug surface, boot times, and maintenance cost ... to basically end up right where we started. I just don't see the point of this. I think the problem is real, I just don't think this is the right solution. Instead, it would probably be simpler to backport a change to earlier versions of PHP that simply allows the future target without validation; or to do a two phase rollout of a target change (minor + 1: add the target without validation, then minor + 2, enable validation); or choose a different name. Another alternative would be to simply write code for the minimum version of PHP you support. For example, if you want to use the pipe operator, you either need to support 8.5 as a minimum, or not use it. — Rob
