Skip to content

[DependencyInjection] Add iterable to possible binding type #44979

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

Merged
merged 1 commit into from
Jan 12, 2022
Merged

[DependencyInjection] Add iterable to possible binding type #44979

merged 1 commit into from
Jan 12, 2022

Conversation

sveneld
Copy link
Contributor

@sveneld sveneld commented Jan 11, 2022

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
License MIT

When iterable type is set in binding like in example https://symfony.com/doc/current/service_container.html#binding-arguments-by-name-or-type, system tries to autoload class iterable here src/Symfony/Component/DependencyInjection/Compiler/ResolveBindingsPass.php:137

if (is_subclass_of($m[1], \UnitEnum::class)) {

@nicolas-grekas
Copy link
Member

Please add a test case that covers the situation.

@sveneld
Copy link
Contributor Author

sveneld commented Jan 11, 2022

Add test for this case

@sveneld
Copy link
Contributor Author

sveneld commented Jan 12, 2022

@nicolas-grekas done

@@ -212,6 +213,31 @@ public function testEmptyBindingTypehint()
$pass->process($container);
}

public function testIterableBindingTypehint()
{
spl_autoload_register(
Copy link
Member

Choose a reason for hiding this comment

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

is this required? if not let's remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's required to show the problem, if this function will run it will mean that code of ResolveBindingsPass.php working incorrect

$pass = new ResolveBindingsPass();
$pass->process($container);

$this->assertInstanceOf(
Copy link
Member

Choose a reason for hiding this comment

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

on one line please

);

$container = new ContainerBuilder();
$bindings = [
Copy link
Member

Choose a reason for hiding this comment

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

the variable could be removed

@nicolas-grekas nicolas-grekas modified the milestones: 5.4, 4.4 Jan 12, 2022
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

(for 4.4)

@nicolas-grekas
Copy link
Member

Thank you @sveneld.

@nicolas-grekas nicolas-grekas merged commit 9f89250 into symfony:4.4 Jan 12, 2022
@sveneld sveneld deleted the fix_ResolveBindingsPass branch January 12, 2022 13:18
This was referenced Jan 28, 2022
This was referenced Jan 28, 2022
nicolas-grekas added a commit that referenced this pull request Feb 16, 2022
This PR was submitted for the 5.4 branch but it was merged into the 4.4 branch instead.

Discussion
----------

[DependencyInjection] Fix type binding

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| License       | MIT

If $type is a scalar compiler pass should not check it in function is_subclass_of($type, \UnitEnum::class), because is_subclass_of trying to autoload class with name array, string, etc.

Related to #44979

Commits
-------

3754018 Fix type binding
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.

3 participants