Add ZEND_PARSE_PARAMETERS_NONE to private constructors#21344
Add ZEND_PARSE_PARAMETERS_NONE to private constructors#21344jordikroon wants to merge 2 commits intophp:masterfrom
Conversation
|
Welp I didn't expect any tests to fail, but the ReflectionAttribute construction test should not have parameters. So those have been removed from the test. |
|
I'm probably in the minority, but I'm going to suggest that this doesn't help - if someone tries calling a private constructor, it is more important to tell them that the constructor is private than to first validate the arguments |
|
I agree with @DanielEScherzer. No point in telling the user how to fix a broken function call that they'll have to remove anyway. |
|
I understand both point of views. The reason for this PR is because @ndossche mentioned it technically needs to be added to be fully correct. To make it match the stub I presume. To that part I agree. But I also understand that it might give a false positive when using a wrong implementation. That said it's likely also best to not add this line to the RFC PR. For me it's most important to have consistent (new) code. With many people reviewing code it's sometimes difficult to do what is best given contrary opinions. Which I fully understand. Given the difference in opinion it's probably best to not merge this. |
Formally this should also be added to private constructors, which I stumbled on when blocking __PHP_Incomplete_Class.
This PR adds
ZEND_PARSE_PARAMETERS_NONE()to all private constructors. No more follow-up PR's to be expected.