Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HttpKernel] Add optional $className param to ControllerEvent::getAttributes() #50335

Merged
merged 1 commit into from Jun 3, 2023

Conversation

HypeMC
Copy link
Contributor

@HypeMC HypeMC commented May 16, 2023

Q A
Branch? 6.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

A minor DX improvement since most of the time only one type of attribute is needed, eg:

public function onKernelControllerArguments(ControllerArgumentsEvent $event)
{
    $attributes = $event->getAttributes(SomeAttribute::class);

    // ...
}

@nicolas-grekas
Copy link
Member

nicolas-grekas commented May 16, 2023

Is this really worth it?

-$attributes = $event->getAttributes(SomeAttribute::class);
+$attributes = $event->getAttributes()[SomeAttribute::class] ?? null;

@stof
Copy link
Member

stof commented May 16, 2023

@nicolas-grekas the difference is that the new argument has a generic signature, telling SA tools that all returned objects are an instance of the provided class name.
For the case of returning all attributes grouped by a class name, there is no interoperable way to indicate that in phpdoc (Psalm has a class-string-map type, but no other tool support it)

@nicolas-grekas
Copy link
Member

Wouldn't this work?
@return array<class-string<T>, list<T>>

@stof
Copy link
Member

stof commented May 16, 2023

@nicolas-grekas no. This won't work as it would force using the same T (and inferring it from somewhere).

That's precisely why Psalm introduced a class-string-map type, which would be exactly what we need here if it was interoperable.

@fabpot
Copy link
Member

fabpot commented Jun 3, 2023

Thank you @HypeMC.

@fabpot fabpot merged commit 08b5907 into symfony:6.4 Jun 3, 2023
8 of 9 checks passed
@HypeMC HypeMC deleted the getattributes-classname branch June 3, 2023 16:02
This was referenced Oct 21, 2023
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.

None yet

6 participants