Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122974
Precedence: bulk
MIME-Version: 1.0
References: <CAF-fPAk0JxToi=dEupJHOEhsb3QiA-M1x_9znAFXfDSbCxvNzQ@mail.gmail.com>
 <CAPyj-LCLc4XF=yndX8ecT1xnTP=yzziUm4ttu86uKMfdF=8W5g@mail.gmail.com>
 <CAF-fPA=mEoVTkPRJhZ7cZjtuaeraNbBBbAQfDTwy_0Aggk7g0Q@mail.gmail.com> <1c510d08-eaa8-4821-bf1b-6072fe5cb2ff@app.fastmail.com>
In-Reply-To: <1c510d08-eaa8-4821-bf1b-6072fe5cb2ff@app.fastmail.com>
Date: Fri, 5 Apr 2024 18:40:32 +0200
Message-ID: <CAF-fPAm0SxD=jC571uJSw9JnBCY_upeCt4YE7szVi47Lcv=+MQ@mail.gmail.com>
Subject: Re: [PHP-DEV] Proposal: retrieve line, filename and if user defined
 for ReflectionAttribute
To: Larry Garfield <larry@garfieldtech.com>
Cc: php internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000e56c7906155c1dcb"
From: jwurtz@jolicode.com (Joel Wurtz)

--000000000000e56c7906155c1dcb
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Le ven. 5 avr. 2024 =C3=A0 18:04, Larry Garfield <larry@garfieldtech.com> a
=C3=A9crit :

> On Fri, Apr 5, 2024, at 2:20 PM, Joel Wurtz wrote:
> >> Would it make sense to not only add this for ReflectionAttribute, but
> also Function and/or others?
> >
> > There may be case where this makes sense but not necessarily in the use
> > case that i explain, and don't want to add more to this proposal, it's
> > also missing in ReflectionParameter, ReflectionProperty and certainly
> > many others if we go on this path then there is a lot of changes to
> > apply and will induce more friction for this proposal, i prefer to add
> > small parts so we don't deviate from the original use case.
> >
> >> Do you have an example of how you expect this to be used?  I'm having =
a
> hard time understanding how I'd leverage this.  (I maintain an attribute
> enhancement library, Crell/AttributeUtils, so I've hit a lot of edge case=
s
> in attribute design.)
> >
> > Sure, let's say we have the following class
> >
> > #[CustomAttribute(foo: 'foo')
> > #[CustomAttribute(foo: 'foo')
> > class Entity {}
> >
> > And that later i have something that read those attributes, but i want
> > to throw an Exception when multiple attribute defined the same value
> > for the foo property (as it should be unique in my use case), i would
> > not be able to do that in the constructor since i don't have the
> > context of others attributes in this case, so i want to throw a custom
> > exception explaining it's caused by the attribute "CustomAttribute" in
> > the file "Entity.php" at line 4, as there is the same value in the file
> > "Entity.php" at line 3
> >
> > If I throw an exception without this message the error will pinpoint to
> > where I threw the exception and it may be confusing for the end user,
> > by having this he will know where to look and what to modify to make
> > its code correct.
>
> OK, so the idea is you could do something like:
>
> $rClass =3D new ReflectionClass(Entity::class);
> $as =3D $rClass->getAttributes();
> $attribs =3D array_map(fn(ReflectionAttribute $r) =3D> $r->getInstance(),=
 $as);
> $values =3D array_map(fn(CustomAttribute $r) =3D> $r->foo, $attribs);
>
> if ($values is not unique) {
>   $msg =3D sprintf('You have too many %s on in file %s on line %d',
> Entity::class, $as[0]->getFile(), $as[0]->getLine());
>   throw new Exception($msg);
> }
>
> (Or something like that.)
>
> That works, I suppose, but couldn't that also be done with
> ReflectionClass::getStartLine() and ReflectionClass::getFileName()?
>
> --Larry Garfield
>

Yes, it could work for a class, but not for a ReflectionProperty or
ReflectionParameter also we only have the line for the start of the class,
not the line of the attribute, in case where there is a lot of attributes
(i.e doctrine entity in a symfony project with validation , serializer, api
resource, etc ....) being able to have the correct line is better (even
more if you have repeated attributes, and one work but not the other)

I'm wondering if this would be useful to also have the offset when there is
multiple attribute on the same line (like attribute for ReflectionParameter=
)

Joel

--000000000000e56c7906155c1dcb
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div><div>Le=C2=A0ven. 5 avr. 2024 =C3=A0=C2=A018:04, Larr=
y Garfield &lt;<a href=3D"mailto:larry@garfieldtech.com">larry@garfieldtech=
.com</a>&gt; a =C3=A9crit=C2=A0:<div class=3D"gmail_quote"><blockquote clas=
s=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid r=
gb(204,204,204);padding-left:1ex">On Fri, Apr 5, 2024, at 2:20 PM, Joel Wur=
tz wrote:<br>
&gt;&gt; Would it make sense to not only add this for ReflectionAttribute, =
but also Function and/or others?<br>
&gt;<br>
&gt; There may be case where this makes sense but not necessarily in the us=
e <br>
&gt; case that i explain, and don&#39;t want to add more to this proposal, =
it&#39;s <br>
&gt; also missing in ReflectionParameter, ReflectionProperty and certainly =
<br>
&gt; many others if we go on this path then there is a lot of changes to <b=
r>
&gt; apply and will induce more friction for this proposal, i prefer to add=
 <br>
&gt; small parts so we don&#39;t deviate from the original use case.<br>
&gt;<br>
&gt;&gt; Do you have an example of how you expect this to be used?=C2=A0 I&=
#39;m having a hard time understanding how I&#39;d leverage this.=C2=A0 (I =
maintain an attribute enhancement library, Crell/AttributeUtils, so I&#39;v=
e hit a lot of edge cases in attribute design.)<br>
&gt;<br>
&gt; Sure, let&#39;s say we have the following class<br>
&gt;<br>
&gt; #[CustomAttribute(foo: &#39;foo&#39;)<br>
&gt; #[CustomAttribute(foo: &#39;foo&#39;)<br>
&gt; class Entity {}<br>
&gt;<br>
&gt; And that later i have something that read those attributes, but i want=
 <br>
&gt; to throw an Exception when multiple attribute defined the same value <=
br>
&gt; for the foo property (as it should be unique in my use case), i would =
<br>
&gt; not be able to do that in the constructor since i don&#39;t have the <=
br>
&gt; context of others attributes in this case, so i want to throw a custom=
 <br>
&gt; exception explaining it&#39;s caused by the attribute &quot;CustomAttr=
ibute&quot; in <br>
&gt; the file &quot;Entity.php&quot; at line 4, as there is the same value =
in the file <br>
&gt; &quot;Entity.php&quot; at line 3<br>
&gt;<br>
&gt; If I throw an exception without this message the error will pinpoint t=
o <br>
&gt; where I threw the exception and it may be confusing for the end user, =
<br>
&gt; by having this he will know where to look and what to modify to make <=
br>
&gt; its code correct.<br>
<br>
OK, so the idea is you could do something like:<br>
<br>
$rClass =3D new ReflectionClass(Entity::class);<br>
$as =3D $rClass-&gt;getAttributes();<br>
$attribs =3D array_map(fn(ReflectionAttribute $r) =3D&gt; $r-&gt;getInstanc=
e(), $as);<br>
$values =3D array_map(fn(CustomAttribute $r) =3D&gt; $r-&gt;foo, $attribs);=
<br>
<br>
if ($values is not unique) {<br>
=C2=A0 $msg =3D sprintf(&#39;You have too many %s on in file %s on line %d&=
#39;, Entity::class, $as[0]-&gt;getFile(), $as[0]-&gt;getLine());<br>
=C2=A0 throw new Exception($msg);<br>
}<br>
<br>
(Or something like that.)<br>
<br>
That works, I suppose, but couldn&#39;t that also be done with ReflectionCl=
ass::getStartLine() and ReflectionClass::getFileName()?=C2=A0 <br>
<br>
--Larry Garfield<br>
</blockquote></div><br></div>Yes, it could work for a class, but not for a =
ReflectionProperty or ReflectionParameter also we only have the line for th=
e start of the class, not the line of the attribute, in case where there is=
 a lot of attributes (i.e doctrine entity in a symfony project with validat=
ion , serializer, api resource, etc ....) being able to have the correct li=
ne is better (even more if you have repeated attributes, and one work but n=
ot the other)<br><br></div>I&#39;m wondering if this would be useful to als=
o have the offset when there is multiple attribute on the same line (like a=
ttribute for ReflectionParameter)<br clear=3D"all"><div><div><br></div><div=
>Joel<br></div></div></div>

--000000000000e56c7906155c1dcb--