8000 [ DependencyInjection] Container::getServiceIds() missing from any interface · Issue #15336 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[ DependencyInjection] Container::getServiceIds() missing from any interface #15336

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

Closed
znerol opened this issue Jul 22, 2015 · 12 comments
Closed

Comments

@znerol
Copy link
Contributor
znerol commented Jul 22, 2015

The public method Container::getServiceIds() is not declared in any interface. This method is used by several Symfony bundles both in test as well as in production code. It is also required in Drupal.

I guess an appropriate place for the method declaration would be in IntrospectableContainerInterface.

Could this go into 2.8 or should I only open up a PR for master?

@stof
Copy link
Member
stof commented Jul 22, 2015

Adding it in an existing interface is not possible (it is a BC break). But we could indeed add a new interface for that.

@znerol
Copy link
Contributor Author
znerol commented Jul 22, 2015

Even if the interface lacks the @api tag?

@znerol
Copy link
Contributor Author
znerol commented Jul 22, 2015

Background on the IntrospectableContainerInterface: #3557

@stof
Copy link
Member
stof commented Jul 22, 2015

@znerol adding methods in an interface is always a BC break. Even though non-api interfaces may be allowed to do such BC break according to our policy, we also have a note sayign that it should be avoided whenever possible. In the current case, it does not deserve a BC break IMO (thus we should probably tag IntrospectableContainerInterface as @api btw).
Thus, getting the list of service ids does not really belong to IntrospectableContainerInterface IMO. It is a different kind of feature than the one defined in the IntrospectableContainerInterface.

@znerol
Copy link
Contributor Author
znerol commented Jul 22, 2015

Diffing the container class and the interface I found some more which do not seem to be declared anywhere:

public function compile()
public function getParameterBag()
public function isFrozen()

Before I will cook up an interface for getServiceIds(), do you think that any of those methods is related enough such that I can include it also?

@fabpot
Copy link
Member
fabpot commented Jul 22, 2015

I don't see why all methods should be in an interface.

@znerol
Copy link
Contributor Author
znerol commented Jul 22, 2015

Perhaps at least those which are used in other bundles/component/projects?

@fabpot
Copy link
Member
fabpot commented Jul 22, 2015

But then again, why? what does it bring to the table? I mean, you can only type hint by one interface, so adding another one would not help, right?

@stof
Copy link
Member
stof commented Jul 22, 2015

compile does not make sense in the interface IMO. It is only useful during container building

@znerol
Copy link
Contributor Author
znerol commented Jul 23, 2015

I examined the usage of the other methods mentioned in one of my comments above. Those are indeed only used inside the component.

Regarding getServiceIds(), my goal is to understand whether it makes sense to declare that in some interface in Symfony. This is in order to make it possible for Drupal to specify what kind of container it requires. Since over there we also use initialized() a lot, the new method declaration either needs to go into IntrospectableContainerInterface or into a new child interface.

So, is it worth to submit a PR and if yes which approach should it implement?

@fabpot
Copy link
Member
fabpot commented Jul 23, 2015

initialized() is indeed the only method that should have been in the interface, but it's not because it was added late in the game. I propose to add it to the interface in Symfony 3.0.

@znerol
Copy link
Contributor Author
znerol commented Jul 23, 2015

Filed #15346

@fabpot Does this mean that this issue here is a wont-fix?

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

No branches or pull requests

3 participants
0