Skip to content

[PropertyInfo] Fix BC issue in phpDoc Reflection library #35134

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

Conversation

jaapio
Copy link
Contributor

@jaapio jaapio commented Dec 28, 2019

Q A
Branch? 3.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #35077
License MIT

The used phpDocumentor library DocBlockReflection contained a BC break
that broke this component. The patch was applied in the recently released v4.3.4
version. But since it is unclear how long this issue existed it is not possible
to exclude a certain version. Therefor also \RuntimeExpception needs to be caught.

The BC break is possibly caused by a change in the TypeResolver library used by the
DocBlockReflection which is now supporting the more popular generics notation for arrays.

This PR might need some tests but the current test cases are not very clear to me. Instead of patching the code we could also try to ban the broken versions of the used phpdoc libraries, but that would require much more testing, and doesn't really add any value. Especially because the DocBlockReflection and TypeResolver are used by over half a million projects. It would raise more questions than just patching the behavior of the PropertyInfo component.

We are sorry that this issue slipt through our QA pipeline. The linked issue already showed that the issue is now fixed by just doing a composer update but it is not very convenient to leave this known issue in symfony.

The used phpDocumentor library DocBlockReflection contained an BC break
that broke this component. The patch was applied in the recent released v4.3.4
version. But since it is unclear how long this issue existed it is not possible
to exclude a certain version. Therefor also `\RuntimeExpception` needs to be catched.

The BC break is possibly caused by a change in the TypeResolver library used by the
DocBlockReflection which is now supporting the more populair generics notation for arrays.
@jaapio jaapio requested a review from dunglas as a code owner December 28, 2019 21:21
@nicolas-grekas nicolas-grekas changed the title Fix BC issue in phpDoc Reflection library [PropertyInfo] Fix BC issue in phpDoc Reflection library Dec 28, 2019
@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Dec 28, 2019
@nicolas-grekas
Copy link
Member

Thank you @jaapio.

nicolas-grekas added a commit that referenced this pull request Jan 4, 2020
…jaapio)

This PR was merged into the 3.4 branch.

Discussion
----------

[PropertyInfo] Fix BC issue in phpDoc Reflection library

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #35077
| License       | MIT

The used phpDocumentor library DocBlockReflection contained a BC break
that broke this component. The patch was applied in the recently released v4.3.4
version. But since it is unclear how long this issue existed it is not possible
to exclude a certain version. Therefor also `\RuntimeExpception` needs to be caught.

The BC break is possibly caused by a change in the TypeResolver library used by the
DocBlockReflection which is now supporting the more popular generics notation for arrays.

This PR might need some tests but the current test cases are not very clear to me. Instead of patching the code we could also try to ban the broken versions of the used phpdoc libraries, but that would require much more testing, and doesn't really add any value. Especially because the DocBlockReflection and TypeResolver are used by over half a million projects. It would raise more questions than just patching the behavior of the PropertyInfo component.

We are sorry that this issue slipt through our QA pipeline. The linked issue already showed that the issue is now fixed by just doing a `composer update` but it is not very convenient to leave this known issue in symfony.

Commits
-------

bad07ec Fix BC issue in phpDoc Reflection library
@nicolas-grekas nicolas-grekas merged commit bad07ec into symfony:3.4 Jan 4, 2020
@jaapio
Copy link
Contributor Author

jaapio commented Jan 5, 2020

Thanks for the quick reviews! And all your awesome work

@jaapio jaapio deleted the ticket_35077-ban-broken-phpdoc-lib branch January 6, 2020 15:08
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.

4 participants