On Mon, Mar 9, 2020 at 8:19 PM Andrea Faulds <a...@ajf.me> wrote:
> Benjamin Eberlei wrote:
> > I want to resurrect Dmitrys Attributes RFC that was rejected for 7.1 in
> > 2016 with a few changes, incorporating feedback from the mailing list
> back
> > then and from talking to previous no voters.
> >
> > The RFC is at https://wiki.php.net/rfc/attributes_v2
>
> Hi,
>
> I have concerns about these two statements in the RFC:
>
> > The name of an attribute is resolved against the currently active
> namespace import scope during compilation. The resolved class names are
> then autoloaded to make sure they exist.
>
> > Consistent with PHP expressions in general, no validation is
> performed if the provided attribute arguments are fullfilling the
> contract of the attribute class constructor. This would happen only when
> accessing attributes as objects in the Reflection API (below).
>
> These two details are inconsistent with eachother: use of an annotation
> triggers an autoload, yet we aren't using the class that is autoloaded
> to validate it? This seems quite wasteful: if we have loaded the class,
> we might as well use it to check the arguments are correct. Also, why
> are we privileging the class existing over the arguments to the class
> being correct? If the arguments can be validated at Reflection time,
> surely the autoloading can be done then too? Both types of coding
> mistake are important.
>
> It also seems inconsistent with existing PHP behaviour, I think normally
> mentioning a class either triggers an immediate autoload and actual
> execution/validation (`new`) or it doesn't (a type declaration). This
> proposal is a strange half-way house.
>
> Is this being done to avoid paying the cost of creating the object at
> compilation time? Because I think triggering the autoload is going to be
> expensive anyway, possibly moreso.
>
Thinking about how I got to my thought process here, I believe it was
something like
When compiling
new Foo("abcdefg", 1234);
then it resolves the class name (does it autoload?) but it wouldn't check
if the arguments match until its actually executed.
I believe I am wrong to propose autoloading directly at compiling the code
is not how PHP does it. It should autoload only during
Reflection*::getAttributes(). this is something I haven't implemented in
the prototype PR yet, maybe I would have seen the problems.
Going to mule this one over and adjust the RFC and this thread with an
update.
> On a different note, the wording here is syntactically ambiguous. It can
> be read as both "if the provided attribute arguments are fullfilling the
> contract […], then no validation is performed" and "no validation is
> performed as to whether the provided attribute arguments are fullfilling
> the contract". I read it as the former the first time, which confused me
> for a moment.
>
Will update to clarify.
>
> Another thing:
>
> > Thanks to class name resolving, IDEs or static analysis tools can
> perform this validation for the developer.
>
> Is this referencing the autoloading behaviour? I don't see why that
> would be required. (You could also be referring to the fact you use
> classes, which IDEs can look for, instead of arbitrary string
> attributes, which IDEs can not, which does make sense.)
>
That is right, an IDE can find this out without the autoloading behavior.
This sentence means like you inferred, that because attributes are
semantically mapping to class names, that is the reason why IDEs can better
validate them in their own static analysis.
>
> Thanks,
> Andrea
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
