Skip to content

@no-named-arguments leads to static analysis errors for variadic arguments #6163

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

Open
m-vo opened this issue Mar 26, 2025 · 8 comments
Open
Labels
type/bug Something is broken

Comments

@m-vo
Copy link

m-vo commented Mar 26, 2025

Q A
PHPUnit version at least version 9

Summary

The InvocationMocker class is annotated with @no-named-arguments - as this is on a class-level, it also affects the with() function, that only has a single variadic argument. This leads to static analysis issues.

Current behavior

When using PHPStan in a project where with() is used with an array (not a list), this may lead to a static analysis error (although the keys are ignored anyways):

 Method […]\InvocationMocker::willReturn() invoked with unpacked array with possibly string key, but it's not allowed because of @no-named-arguments.

How to reproduce

$input = ['a' => 'foo', 'b' => 'bar'];

$mock->with(...$input); // invalid
$mock->with(...array_values($input)); // valid

$mock->willReturn(/* … */);

Expected behavior

Both cases are a valid usage, so I'd expect there would be no static analysis errors.

@m-vo m-vo added the type/bug Something is broken label Mar 26, 2025
@sebastianbergmann
Copy link
Owner

This sounds like a shortcoming of PHPStan to me.

What do you think, @ondrejmirtes @staabm?

@m-vo
Copy link
Author

m-vo commented Mar 26, 2025

PHPUnit could in theory make use of the array keys of a ...$parameters argument, i.e. to map it as named parameters to the mocked function. But any name changes to the PHPUnit method signature would not break the consumer's code.

But in general, you could use the $parameters to invoke a library function, and then it would matter again. So not sure if PHPStan would have a way to detect this.

@ondrejmirtes
Copy link

Not sure what PHPStan should do here. This is named arguments call:

$input = ['a' => 'foo', 'b' => 'bar'];

$mock->with(...$input); // invalid

So PHPStan marks it as such. If you think there should be some exceptions to what is marked as valid and invalid with @no-named-arguments, please open a feature request on PHPStan's GitHub.

@ondrejmirtes
Copy link

My opinion is that if PHPUnit allows this use-case, there shouldn't be class-wide @no-named-arguments, but only above methods which do not support named arguments.

@sebastianbergmann
Copy link
Owner

PHPUnit has @no-named-arguments for all classes, interfaces, traits, functions because parameter names are not covered by the backward compatibility promise for PHPUnit.

That being said, the argument can be made to have an exception from this rule. However, I am not sure that the use-case mentioned here is actually supported.

@m-vo
Copy link
Author

m-vo commented Mar 26, 2025

However, I am not sure that the use-case mentioned here is actually supported

What do you mean? Why shouldn't it be OK to pass an associative array to a function that just decides to use its values? There is nothing bad in doing so.

@sebastianbergmann
Copy link
Owner

What does passing an associative array as an argument have to do with named arguments?

@staabm
Copy link
Contributor

staabm commented Mar 26, 2025

in combination with the ... operator the named keys of the argument array is mapped to the variable names
https://3v4l.org/gON2X

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something is broken
Projects
None yet
Development

No branches or pull requests

4 participants