Skip to content

Add Reflection*::getAttribute(): ?ReflectionAttribute method#22092

Open
vudaltsov wants to merge 1 commit into
php:masterfrom
vudaltsov:get-attribute
Open

Add Reflection*::getAttribute(): ?ReflectionAttribute method#22092
vudaltsov wants to merge 1 commit into
php:masterfrom
vudaltsov:get-attribute

Conversation

@vudaltsov
Copy link
Copy Markdown
Contributor

Currently, getting a single attribute requires either getAttributes()[0] ?? null or a temporary variable. This is the most common case since attributes are non-repeatable by default.

I propose to add getAttribute() to all reflection classes that have getAttributes(), with the same signature and filtering behavior as getAttributes().

There is some code duplication between read_attribute() and read_attributes() — I'm not sure it's worth eliminating, but happy to refactor if needed.

Also: does this need an RFC? I've seen small non-breaking changes like this are sometimes merged without an RFC.

@TimWolla
Copy link
Copy Markdown
Member

Currently, getting a single attribute requires either getAttributes()[0] ?? null or a temporary variable

What about https://www.php.net/manual/en/function.array-first.php?

@vudaltsov
Copy link
Copy Markdown
Contributor Author

Yes, I use that function in 8.5, but it seems that I get the first attribute in 99% of the cases, so it makes sense to add the method anyway.

@TimWolla
Copy link
Copy Markdown
Member

I don't particularly like the dedicated method: If there is more than one attribute it silently “loses data”. If the “first (and only) attribute only” is the desired semantic then doing that in userland has become trivial with array_first(). I don't see value in increasing the API surface to avoid a function call, particularly one that hints that the code might be unsafe, since there could be more attributes lurking in the result.

@vudaltsov
Copy link
Copy Markdown
Contributor Author

vudaltsov commented May 19, 2026

The thing is, 99% of the time you want to pass $name, since you want to get a specific attribute and call ->newInstance() on it without touching anything else. In that case you're not "losing" anything — you're targeting a specific attribute you know to be non-repeatable.

Maybe the method should be called getFirstAttributeOf($name) with a required parameter to make the intent explicit?

@TimWolla
Copy link
Copy Markdown
Member

Maybe the method should be called getFirstAttributeOf($name) with a required parameter to make the intent explicit?

That would be explicit, but would not be meaningfully simpler than just using array_first(), while still increasing the API surface which comes with documentation cost:

$a = array_first($r->getAttributes(Foo::class));
$a = $r->getFirstAttributeOf(Foo::class));

I believe good language design is having multiple simple features that compose well, not having an explicit function for everything, since the former makes the language much easier to learn and more flexible with regard to new use cases.

Given the disagreement here, I'm also marking the PR as “Requires RFC”.

@vudaltsov
Copy link
Copy Markdown
Contributor Author

I actually tend to agree with you. I will think about this for a day and then either write to internals or just close the PR. Thank you for a quick feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants