Skip to content
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

Fix error when proxying a method with a mixed argument with a null default value #38

Merged

Conversation

aboks
Copy link
Contributor

@aboks aboks commented Mar 19, 2024

Without this change, trying to proxy a Symfony\Component\Console\Command\Command would result in an InvalidArgumentException Type "mixed" cannot be nullable from the Laminas code generator. This is because of the addArgument method of the Command class having a mixed $default = null parameter. The error can be seen as a regression in 1.0.17 caused by #37.

@nicolas-grekas
Copy link
Collaborator

Ah indeed, thanks
Can you please have a look at the failures on 7.*?

@aboks
Copy link
Contributor Author

aboks commented Mar 20, 2024

Sure! I've changed the test to be a bit more specific (as the version requirements would probably be different for other data sets in the original data provider) and made the test skip in PHP < 8.0 (as mixed types were not possible in those versions).

…fault value

Without this change, trying to proxy a Symfony\Component\Console\Command\Command
would result in an InvalidArgumentException `Type "mixed" cannot be nullable` from
the Laminas code generator.
@nicolas-grekas nicolas-grekas force-pushed the mixed-arg-with-null-default-value branch 2 times, most recently from 239c53c to 2c8a6cf Compare March 20, 2024 12:51
@nicolas-grekas nicolas-grekas merged commit 2c8a6cf into FriendsOfPHP:1.x Mar 20, 2024
8 of 9 checks passed
@nicolas-grekas
Copy link
Collaborator

Thank you @aboks (please use spaces for your next PRs if any, that's the CS here ;) )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants