Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:109692
MIME-Version: 1.0
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>
 <231390ee-ff9c-4f7b-918a-4f87a676613e@www.fastmail.com> <CAEPJdunDbMY6AdTRTfQ8VEnwY-ey6jTG2sKFHz=kqoykvSc89A@mail.gmail.com>
 <11f35e10-5bee-4b28-b784-69f7f5f2f11e@www.fastmail.com> <CAOWwgpndWLjAPw1DQvJvjAL8bw=39Q_wdc1SOrf2qomfrsKJcg@mail.gmail.com>
In-Reply-To: <CAOWwgpndWLjAPw1DQvJvjAL8bw=39Q_wdc1SOrf2qomfrsKJcg@mail.gmail.com>
Date: Thu, 16 Apr 2020 20:22:12 +0200
Message-ID: <CAEPJdukqr-yPi83mCZa09F+Px2H0iQ2DDxdfX__Eq4LMUZzQ5Q@mail.gmail.com>
To: Nicolas Grekas <nicolas.grekas+php@gmail.com>
Cc: Larry Garfield <larry@garfieldtech.com>, php internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000009a754005a36c8391"
Subject: Re: [PHP-DEV] Re: [RFC] Attributes v2
From: kontakt@beberlei.de (Benjamin Eberlei)

--0000000000009a754005a36c8391
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Apr 16, 2020 at 4:41 PM Nicolas Grekas <nicolas.grekas+php@gmail.co=
m>
wrote:

> Le jeu. 16 avr. 2020 =C3=A0 16:29, Larry Garfield <larry@garfieldtech.com=
> a
> =C3=A9crit :
>
>> On Thu, Apr 16, 2020, at 1:46 AM, Benjamin Eberlei wrote:
>>
>> > > > > 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 befo=
re
>> > > 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 =3D
>> > > 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 forme=
d
>> 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 regul=
ar
>> > > > 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 w=
e
>> need
>> > > to
>> > > > have a one liner here.
>> > >
>> > > It depends on the annotation, I suppose.  If I'm requesting a specif=
ic
>> > > 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 questio=
n
>> 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()).  Weir=
d
>> 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. :-)
>> > >
>> >
>> > You are not safe from these problems when using Doctrine Annotations
>> either
>> > (missing library or class does not exist) and it fails exactly the sam=
e
>> way
>> > as trying to instantiate something that doesn't exist.
>> >
>> > I also realized why IS_INSTANCEOF is not the default, because it needs
>> to
>> > resolve all attributes to classes to perform the check. This triggers
>> > autoloading *all* attributes of the reflected declaration (even the on=
es
>> > not requested), so we felt it should not be the default.
>>
>> Ah, valid.  I suppose that's an unavoidable result of allowing
>> non-class-mapped attributes, which means anyone building on it is stuck
>> doing their own class_exists() check for everything.
>>
>> Sad panda.  (Still very +1 on the RFC, just sad panda about these
>> details.)
>>
>
>
> Same here, sad panda:
> - we're going to miss nested structure instantly - I wish we could try
> harder here. I'm actually wondering if using the syntax for object litera=
ls
> would be the solution here? (we don't have any yet, but some proposals ha=
ve
> been mentioned recently.
>

Nested attributes are intentionally not part of this RFC, because I believe
it is better to attempt to integrate it with as many existing concepts as
possible, with the potential to benefit from future additions (named
function params, constant AST/expression improvements). Inventing a
completely new syntax will cause future problems for the language, and the
extensibility of attributes themselves.

I can't guarantee you a timeframe for any of that, as mentioned before, I
see it much like scalar types missing nullable in PHP 7. Nikita mentioned
potential improvements to constant AST in a reply to his ctor promotion
RFC, Tyson also discussed it a few weeks ago, and I plan to look into this
myself if this RFC is accepted. And then there is also the potential of the
named params RFC being revived as discussed a few days ago.

Your mention of object literal syntax is a very good example why nested is
not in this RFC. if we invent our own nested attributes syntax now,
deviating from PHPs expressions, we might never be able to integrate
general expression improvements into attributes and be left with two
syntaxes that can't benefit from each other. In addition we will be left to
maintaining both.

Hypothetically, if we had named params and object initializer in PHP (which
makes no sense, because they somewhat do the same thing), plus the ability
to evaluate more expressions, then the following would automatically be
possible in an attribute:

<<ORM\JoinTable(name =3D> "users", joinColumns =3D> [new ORM\JoinColumn{ na=
me =3D
"id, referencedColumnName =3D "user_id" }])>>

Then ReflectionAttribute::getArguments() returns: ["name" =3D> "users",
"joinColumns" =3D> [object(ORM\JoinColumn) { "name" =3D> "id,
"referencedColumnName" =3D> "user_id" }]].

Hope this clarifies a bit on the RFCs approach.


> - having declarative statements possibly turn into failures is also
> unfortunate. This makes me wonder if the attributes shouldn't be turned
> into plain arrays instead? The logic to hydrate objects would then be mov=
ed
> to some userland libs. Would that make sense? (I know, that conflicts wit=
h
> my previous proposal, I'm just trying to explore :))
>

The attributes can be turned into arrays as well, see the
ReflectionAttribute object in the RFC getName and getArguments functions.
Only ReflectionAttribute::getAsObject can turn into a failure. This is the
reason Reflection*::getAttribute does not immediately convert to objects.
This gives the most flexibility to userland, but also establishes the
convention how attributes are "mapped" to objects.


>
> Nicolas
>

--0000000000009a754005a36c8391--