Skip to content

[DI] Add extra type check to php dumper #33958

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
Oct 12, 2019
Merged

[DI] Add extra type check to php dumper #33958

merged 1 commit into from
Oct 12, 2019

Conversation

gquemener
Copy link
Contributor

Q A
Branch? 4.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #33942
License MIT
Doc PR

This PR adds a missing type check in the PHP Dumper. The bug has been detected while working on the https://github.com/prooph/service-bus-symfony-bundle and I haven't been able to reproduce it within a minimalist testcase.

I would like to add a unit test to cover it once I have figured out the exact context in which the bug occurs.

Any help would be greatly appreciated to do so, especially from "senior" contributors of the DependencyInjection component, many thanks in advance!

You will find more information about this bug in the linked ticket above.

@nicolas-grekas nicolas-grekas changed the title bug #33942 [DI] Add extra type check to php dumper [DI] Add extra type check to php dumper Oct 12, 2019
@nicolas-grekas nicolas-grekas added this to the 4.3 milestone Oct 12, 2019
@nicolas-grekas
Copy link
Member

Thank you @gquemener.

nicolas-grekas added a commit that referenced this pull request Oct 12, 2019
This PR was merged into the 4.3 branch.

Discussion
----------

[DI] Add extra type check to php dumper

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #33942
| License       | MIT
| Doc PR

This PR adds a missing type check in the PHP Dumper. The bug has been detected while working on the https://github.com/prooph/service-bus-symfony-bundle and I haven't been able to reproduce it within a minimalist testcase.

I would like to add a unit test to cover it once I have figured out the exact context in which the bug occurs.

Any help would be greatly appreciated to do so, especially from "senior" contributors of the DependencyInjection component, many thanks in advance!

You will find more information about this bug in the linked ticket above.

Commits
-------

b17ebdf bug #33942 [DI] Add extra type check to php dumper
@nicolas-grekas nicolas-grekas merged commit b17ebdf into symfony:4.3 Oct 12, 2019
@gquemener gquemener deleted the php-dumper-alias-fix branch October 12, 2019 09:35
@gquemener
Copy link
Contributor Author

gquemener commented Oct 14, 2019

I've just realized that the PhpDumper::isSingleUsePrivateNode method has been introduced in the 4.2.0 release. This fix has been merged on the wrong branch then.

I hope you have a simple way to fix this, let me know if I should re-open a PR against the correct branch.

Sorry about that @nicolas-grekas !

@nicolas-grekas
Copy link
Member

4.2 is not maintained anymore, see https://symfony.com/releases

@gquemener
Copy link
Contributor Author

4.2 is not maintained anymore, see https://symfony.com/releases

Great! I missed this piece of information, thanks.

Cheers 👋

@fabpot fabpot mentioned this pull request Nov 1, 2019
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