Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:109637
User-Agent: Cyrus-JMAP/3.1.7-1130-gd0f8b30-fmstable-20200414v1
Mime-Version: 1.0
Message-ID: <231390ee-ff9c-4f7b-918a-4f87a676613e@www.fastmail.com>
In-Reply-To: 
 <CAEPJdun15CfEjxBsbMbnv8XWHnOvdZ0fnB-E1aYsrPbRWavVTw@mail.gmail.com>
References: 
 <CAEPJdunSr8PkBtSLO8g_JMLzM4cFVnXsQSEMFmu2RN25XT=grw@mail.gmail.com>
 <CAEPJdumF8GaAqLj+=gNi-dMP1GYSvk_DYJuy3gG0N5J1vSXRqQ@mail.gmail.com>
 <16620f3a-1eb6-437e-aa6f-05e5be2aa713@www.fastmail.com>
 <CAEPJdun15CfEjxBsbMbnv8XWHnOvdZ0fnB-E1aYsrPbRWavVTw@mail.gmail.com>
Date: Tue, 14 Apr 2020 11:13:08 -0500
To: "php internals" <internals@lists.php.net>
Content-Type: text/plain
Subject: Re: [PHP-DEV] Re: [RFC] Attributes v2
From: larry@garfieldtech.com ("Larry Garfield")

On Tue, Apr 14, 2020, at 10:42 AM, Benjamin Eberlei wrote:
> On Tue, Apr 14, 2020 at 5:24 PM Larry Garfield <larry@garfieldtech.com>
> wrote:

> > 2. Regarding sub-annotations, can you still do classes as parameters even
> > if not as an annotation marker?  Eg:
> >
> > <<Foo(1, "B", Bar('blah'))>>
> > function foo()
> >
> > Or is that also a no-go?
> >
> 
> This is a no go because it would require reimplementing constant ASTs,
> which is as of now 300 lines of tricky code evaluating ASTs and allowing
> this would also clash with Bar("Blah") reading like a function call, which
> is confusing and would prevent reconciliation with constant ASTs in the
> future.

Sad panda.

> > 3. I see the most common case for attributes being getting the object
> > version.  With the reflection API as currently described, I see two
> > shortcomings.
> >
> > A) I can't tell if an attribute has a valid object or not before trying to
> > access it, which would presumably fail spectacularly.  I believe we need a
> > way to know if getObject() is going to return a valid value before trying
> > to call it.  I think this is a hard-requirement.
> >
> > B) Related, as is getting all attributes as objects looks to be rather
> > clunky.
> >
> > $attribute_objectgs = array_filter(array_map(function(ReflectionAttribute
> > $r) {
> >   if ($r->getObject()) { // Needs something better here.
> >     return $r->getObject();
> >   }
> > }, $obj->getAttributes()));
> >
> > That's gross. :-)  Can "get all the attributes that can be formed into
> > objects" be its own operation?  $obj->getAttributeObjects() or some such,
> > that skips over non-instantiable attributes and instantiates the rest?
> >
> 
> I don't see A.) what would you do when the object instantiation fails? You
> would throw an exception I presume, let the engine throw the regular
> TypeError, ArgumentError, Error if class not exists that everyone is
> already familiar with.
> 
> For B.) I believe you are extrapolating based on your own use case. Working
> with Reflection is usually a lot of boilerplate, I don't believe we need to
> have a one liner here.

It depends on the annotation, I suppose.  If I'm requesting a specific annotation by name, presumably I know if it is supposed to have an associated class.  If it's supposed to but it's missing, that's a legit class-not-found exception/error.

However, I'm thinking of cases where code is integrating with a 3rd party optionally, through an annotation.  In that case it's a fair question of whether the class will be defined or not based on whether some other library is present.

Similarly, if a bit of code is requesting all attributes (as above) rather than just specific ones by name, it wouldn't know if a given attribute is supposed to be defined or not; as written, class-less attributes are supported.

I suppose the workaround would be class_exists($r->getName()).  Weird but I guess works?  It would have to be documented as a thing you should do, though, which implies to me that it could be made cleaner.

That reflection is usually clunky today (true) is to me not a compelling argument that it shouldn't be made less clunky. :-)

--Larry Garfield