Skip to content

Services ids from the ContainerBuilder can be integers #32549

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
Closed
mathroc opened this issue Jul 15, 2019 · 4 comments
Closed

Services ids from the ContainerBuilder can be integers #32549

mathroc opened this issue Jul 15, 2019 · 4 comments
Labels
Bug DependencyInjection Status: Needs Review

Comments

@mathroc
Copy link
Contributor

mathroc commented Jul 15, 2019

ContainerBuilder::getServiceIds() can return integers. It tries to cast all ids to string on input but because they're used as array keys, they are converted to integer if they are numeric

related issue : sonata-project/SonataAdminBundle#5638

It was easier to to a PR against sonata-admin-bundle but I think it should be fixed in Symfony too
@phansys phansys mentioned this issue Jul 15, 2019
Merged
@ro0NL
Copy link
Contributor

ro0NL commented Jul 16, 2019

at least we have a potential type violation here:

symfony/src/Symfony/Component/DependencyInjection/Dumper/GraphvizDumper.php

Lines 173 to 178 in 2c9a196

foreach ($container->getServiceIds() as $id) {
if (\array_key_exists($id, $container->getAliases())) {
continue;
}
if (!$container->hasDefinition($id)) {

Casting to string on-call sounds reasonable.

@stof
Copy link
Member

stof commented Jul 16, 2019

@ro0NL there is no type violation here, as GraphizDumper doesn't run in strict mode.

@ro0NL
Copy link
Contributor

ro0NL commented Jul 16, 2019 •
edited
Loading

@stof , fair .. i meant a conceptual violation. We expect string[] basically.

@stof
Copy link
Member

stof commented Jul 16, 2019

Well, the violation is more in the implementation of getServiceIds IMO. As we expect service ids to be strings, we should return an array of strings there to avoid issues.

mathroc added a commit to mathroc/symfony that referenced this issue Jul 16, 2019
@mathroc
ContainerBuilder::getServiceIds() should return an array of string
7b01b7f 
see symfony#32549
mathroc added a commit to mathroc/symfony that referenced this issue Jul 16, 2019
@mathroc
Container::getServiceIds() should return an array of string
5ee1aea 
see symfony#32549
@mathroc mathroc mentioned this issue Jul 16, 2019
Merged
mathroc added a commit to mathroc/symfony that referenced this issue Jul 16, 2019
@mathroc
Container*::getServiceIds() should return an array of string
0699716 
see symfony#32549
mathroc added a commit to mathroc/symfony that referenced this issue Jul 16, 2019
@mathroc
Container*::getServiceIds() should return an array of string
be9c0fb 
see symfony#32549
mathroc added a commit to mathroc/symfony that referenced this issue Jul 16, 2019
@mathroc
Container*::getServiceIds() should return an array of string
24b298d 
see symfony#32549
mathroc added a commit to mathroc/symfony that referenced this issue Jul 16, 2019
@mathroc
Container*::getServiceIds() should return an array of string
9c88caa 
see symfony#32549
@fabpot fabpot closed this as completed Jul 17, 2019
fabpot added a commit that referenced this issue Jul 17, 2019
@fabpot
bug #32563 Container*::getServiceIds() should return strings (mathroc)
04b9ce3 
This PR was merged into the 3.4 branch.

Discussion
----------

Container*::getServiceIds() should return strings

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | o
| Tests pass?   | yes
| Fixed tickets | #32549
| License       | MIT

Cast services ids to string in `Container*::getServiceIds()`

Commits
-------

9c88caa Container*::getServiceIds() should return an array of string
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug DependencyInjection Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @mathroc @stof @ro0NL @xabbuh @carsonbot