Hello everyone,
Like a lot of libraries, we offer the possibility to configure behaviors
with Attributes. However in some cases it's wrongly configured by the user
and this wrong configuration cannot be detected on the attribute
constructor but afterwards.
In this case we may want to pinpoint which attribute (in which file and at
which line) cause this bad configuration. Since there was no method to
retrieve those information in the ReflectionAttribute I proposed a PR
https://github.com/php/php-src/pull/13889 to add those informations.
I do believe this will allow better DX for end user when correctly used,
Regards,
Joël Wurtz
--
Joël Wurtz
06 23 64 03 55
Jolicode, Expertise et Architecture Web
http://www.jolicode.com/
Hello everyone,
Like a lot of libraries, we offer the possibility to configure behaviors
with Attributes. However in some cases it's wrongly configured by the user
and this wrong configuration cannot be detected on the attribute
constructor but afterwards.In this case we may want to pinpoint which attribute (in which file and at
which line) cause this bad configuration. Since there was no method to
retrieve those information in the ReflectionAttribute I proposed a PR
https://github.com/php/php-src/pull/13889 to add those informations.I do believe this will allow better DX for end user when correctly used,
Would it make sense to not only add this for ReflectionAttribute, but also Function and/or others?
Functions have a range of lines associate with them. Could Attributes have that too?
cheers
Derick
Hello everyone,
Like a lot of libraries, we offer the possibility to configure
behaviors with Attributes. However in some cases it's wrongly
configured by the user and this wrong configuration cannot be detected
on the attribute constructor but afterwards.In this case we may want to pinpoint which attribute (in which file and
at which line) cause this bad configuration. Since there was no method
to retrieve those information in the ReflectionAttribute I proposed a
PR https://github.com/php/php-src/pull/13889 to add those informations.I do believe this will allow better DX for end user when correctly used,
Regards,
Joël Wurtz
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 cases in attribute design.)
--Larry Garfield
Hi Joel
Like a lot of libraries, we offer the possibility to configure behaviors with Attributes. However in some cases it's wrongly configured by the user and this wrong configuration cannot be detected on the attribute constructor but afterwards.
In this case we may want to pinpoint which attribute (in which file and at which line) cause this bad configuration. Since there was no method to retrieve those information in the ReflectionAttribute I proposed a PR https://github.com/php/php-src/pull/13889 to add those informations.
I do believe this will allow better DX for end user when correctly used,
I would propose negating and renaming isUserDefined() to isInternal(),
since we already have several such methods. As hinted in the PR, I
believe the implementation is wrong, or rather doesn't do what you
would expect it to do.
I'm also wondering whether it may be more useful to expose the
attribute class as getClass(), without instantiating the attribute
itself. This would allow you to check for isInternal() there, along
with all the other class reflection information.
Ilija
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 cases
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.
I would propose negating and renaming isUserDefined() to isInternal()
Both methods exists depending the context (there is a isUserDefined() in
the ReflectionClass class), but i'm fine with negating this if it's more
consistent
As hinted in the PR, I believe the implementation is wrong,
I think the implementation is correct, i answered in the PR, i will add
more tests case that explain what is wanted here
I'm also wondering whether it may be more useful to expose the attribute
class as getClass()
Do we not already have this with the getName() method ? or it can be wrong
if it's an alias by example ?
Joël
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 cases 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 3If 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 = new ReflectionClass(Entity::class);
$as = $rClass->getAttributes();
$attribs = array_map(fn(ReflectionAttribute $r) => $r->getInstance(), $as);
$values = array_map(fn(CustomAttribute $r) => $r->foo, $attribs);
if ($values is not unique) {
$msg = 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
Le ven. 5 avr. 2024 à 18:04, Larry Garfield larry@garfieldtech.com a
écrit :
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 cases
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 3If 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 = new ReflectionClass(Entity::class);
$as = $rClass->getAttributes();
$attribs = array_map(fn(ReflectionAttribute $r) => $r->getInstance(), $as);
$values = array_map(fn(CustomAttribute $r) => $r->foo, $attribs);if ($values is not unique) {
$msg = 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