Better PHP8 type handling #918
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In case of nullable union types, '?' sign is not supported. The current solution for uniont types (the recursive call) prepended the '?' sign to the null type, so I had to handle this with an additional option for the formatType method of the ProxyGenerator.
PHP8 supports 'static' keyword as a return type. This should be copied to the proxy class without any transformation. This type is not a "builtin" type in php, so I should extend some if branches with an additional condition so allow this new keyword.
jellynoone
reviewed
Jan 17, 2021
With a name check it's more logical to check the null value, because this is the only one when the unnecessary '?' sign can occure. So no function parameter is required with this solution. Thanks @jellynoone
|
Can someone from the Doctrine team review the pull request? @beberlei @greg0ire By now the author, myself and doctrine/orm#8422 have found the issue being solved in this pull request. |
jellynoone
approved these changes
Jan 18, 2021
greg0ire
approved these changes
Jan 18, 2021
SenseException
approved these changes
Jan 19, 2021
|
Does this target the 3.1.x branch for a specific reason? This is a bugfix for me and should target 3.0.x Edit: Sorry, my mistake Github showed 3.0.3 as last release, but it was actually 3.1.0 already. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've found two type declaration related problems in proxy generator, when using it with PHP8.
First is the usage of static keyword, which should be "copied" to the proxy class, and also can be nullable.
function setId(int $id): ?staticThis is supported in this pull request.
Second is a fix for union types of PHP8, because in case of nullable union types the '?' sign is nut supported. It should be used in this way:
float|int|nullBut the proxy generator prepended the '?' to the null part of the union type:
float|int|?nullThis problem has been fixed with an optional argument in case of recursive call of formatType function.