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 9280 - func_num_args() presence must not allow unlimited args #2374

Closed

Conversation

mvorisek
Copy link
Contributor

@mvorisek mvorisek commented May 8, 2023

No description provided.

@phpstan-bot
Copy link
Collaborator

You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x.

@mvorisek mvorisek changed the base branch from 1.11.x to 1.10.x May 8, 2023 08:47
@mvorisek mvorisek force-pushed the fix_too_many_args_with_func_num_args branch 3 times, most recently from 2cf7a6b to 5523ee4 Compare May 8, 2023 09:05
@mvorisek mvorisek force-pushed the fix_too_many_args_with_func_num_args branch from 14bb920 to 7ff4e30 Compare May 8, 2023 09:31
@mvorisek mvorisek force-pushed the fix_too_many_args_with_func_num_args branch from f538aab to 9e7fdbf Compare May 8, 2023 09:35
@mvorisek mvorisek marked this pull request as ready for review May 8, 2023 10:08
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@ondrejmirtes
Copy link
Member

Thank you, but I've already said in the issue: phpstan/phpstan#9280

This has been in PHPStan for a long time and there might be users relying on opposite behaviour than you want. I'm not willing to change it.
You can adjust your code not to use func_num_args instead.

@mvorisek
Copy link
Contributor Author

mvorisek commented May 8, 2023

I would really like to get this merged as func_num_args has legit uses on non-variadic methods. The tests also shows no big BC-break will be introduced, actually there is no single BC-break. If the variadic args are retrived using func_get_arg or func_get_args, which is typical for variadic functions with undeclared params, everything is analysed as before.

@mvorisek
Copy link
Contributor Author

@ondrejmirtes the E2E tests show no single issue, I have done a short brainstorming and with this PR I do not see any, even minor downside, using func_num_args without retriving the actual args (via func_get_arg/func_get_args) does not make much sense. Ondřej, what usecase do you have in your head?

The problem with the current (before this PR) behaviour is a lot of problems can be supressed and this is probably also the reason why noone reported it yet.

@mvorisek mvorisek deleted the fix_too_many_args_with_func_num_args branch April 22, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants