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 to ContainerInterface #44

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

Add generics to ContainerInterface #44

wants to merge 1 commit into from
+3 −1

Conversation

nicolas-grekas
Copy link

WDYT?

@mnapoli
Copy link
Member

mnapoli commented May 5, 2022

Duplicates #35?

@nicolas-grekas
Copy link
Author

nicolas-grekas commented May 5, 2022 •
edited
Loading

Quite close indeed. I'd just not add generics to the keys. Despite #35 being closed, I still think this would be a good idea. Locators scoped to some interface are quite common. It'd be great to hint autocompletion about them:
ContainerInterface<Foo> $container->get('bar')->... and be suggested with what Foo provides.

The fact that psalm/phpstan would complain about this shouldn't be a blocker to me. Eg they could stop complaining when the template type is of mixed.

@nicolas-grekas nicolas-grekas mentioned this pull request May 5, 2022
Merged
ro0NL
ro0NL approved these changes May 6, 2022
View reviewed changes
Copy link

@ro0NL ro0NL left a comment •
edited
Loading

Choose a reason for hiding this comment

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

psalm will also complain about missing template params when extending ContainerInterface, expecting 1

which is kinda weird due of mixed implying a default type (proven by template-less arguments https://psalm.dev/r/da9d16aecc)

phpstan still requires a type for arguments https://phpstan.org/r/9dc90c25-c7ab-4117-a686-91c060627217

nevertheless :shipit:

@ro0NL
Copy link

ro0NL commented May 6, 2022 •
edited
Loading

to clarify, TS is super explicit here :) T extends String vs T extends String = String vs T = String vs T

nicolas-grekas added a commit to symfony/symfony that referenced this pull request May 7, 2022
@nicolas-grekas
minor #46267 [Contracts/Service] Add generics to ServiceProviderInter…
7e9e713 
…face (nicolas-grekas)

This PR was merged into the 6.1 branch.

Discussion
----------

[Contracts/Service] Add generics to ServiceProviderInterface

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

Should allow using `ServiceProviderInterface<MyInterface>` and have autocompletion when doing `$provider->get->...`.

Submitted upstream as php-fig/container#44

Commits
-------

2aa459a [Contracts/Service] Add generics to ServiceProviderInterface
@loic425 loic425 mentioned this pull request Jan 11, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ro0NL ro0NL ro0NL approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nicolas-grekas @mnapoli @ro0NL