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.

Sign up for GitHub

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

Jump to bottom

Add generics #20076

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add generics #20076

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions framework/di/Container.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ class Container extends Component
* In this case, the constructor parameters and object configurations will be used
* only if the class is instantiated the first time.
*
* @param string|Instance $class the class Instance, name, or an alias name (e.g. `foo`) that was previously
* @template T
* @param class-string<T>|Instance $class the class Instance, name, or an alias name (e.g. `foo`) that was previously
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that Psalm syntax?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this should work in Psalm,

https://psalm.dev/docs/annotating_code/templated_annotations/#param-class-stringt

I've tested and used this frequently in PHPStorm and PHPStan. No first-hand experience in Psalm, but the docs imply the syntax is the same. That said I'm getting an error in their playground,

https://psalm.dev/r/da35f91c85

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error seems to be because Pslam is more strict and won't let you new a generic class-string because we don't know what the constructor requires. In this case Yii's DI will handle all that for us so suppressing the Psalm error should be fine and would be handled by other tests/types elsewhere.

Related issue, vimeo/psalm#8628

* registered via [[set()]] or [[setSingleton()]].
* @param array $params a list of constructor parameter values. Use one of two definitions:
* - Parameters as name-value pairs, for example: `['posts' => PostRepository::class]`.
Expand All @@ -154,7 +155,7 @@ class Container extends Component
* parameter list.
* Dependencies indexed by name and by position in the same array are not allowed.
* @param array $config a list of name-value pairs that will be used to initialize the object properties.
* @return object an instance of the requested class.
* @return T an instance of the requested class.
* @throws InvalidConfigException if the class cannot be recognized or correspond to an invalid definition
* @throws NotInstantiableException If resolved to an abstract class or an interface (since 2.0.9)
*/
Expand Down
Loading