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

Named arguments when used with interfaces cannot be trusted #7434

Open
mad-briller opened this issue Jun 9, 2022 · 3 comments
Open

Named arguments when used with interfaces cannot be trusted #7434

mad-briller opened this issue Jun 9, 2022 · 3 comments

Comments

@mad-briller
Copy link
Contributor

mad-briller commented Jun 9, 2022

Feature Request

Since php8 we have gained the ability to use named arguments. However, named arguments and interfaces don't work very well together, due to the fact that interfaces do not enforce argument names and the implementation is free to change them.

This behaviour can be seen in the following playground:
https://phpstan.org/r/3938b4fd-1921-4bd2-8bc6-6139aedfe310

It would be great if phpstan could enforce the argument naming for contract implementations when the @no-named-parameters or no-named-arguments annotation is not present.

Did PHPStan help you today? Did it make you happy in any way?

phpstan helps me and my organization every day!

@ondrejmirtes
Copy link
Member

ondrejmirtes commented Jun 9, 2022

@jrmajor is now in the middle of implementing this feature. After phpstan/phpstan-src#1400 I think there are two more things to do:

  • Check that function/method is not called with named arguments when @no-named-arguments is present.
  • Optional rule enabled via an option that checks that parameter names are the same in child class/parent class/interface when @no-named-arguments is not present.

The 2nd point should be optional because not everyone uses named arguments, it'd be too annoying to require the parameter names to always match.

@phpstan-bot
Copy link
Contributor

@mad-briller After the latest push in 1.11.x, PHPStan now reports different result with your code snippet:

@@ @@
-PHP 8.0 – 8.1
+PHP 8.0 – 8.1 (2 errors)
 ==========
 
-No errors
+13: Non-abstract method Implementation::method() must contain a body.
+18: Non-abstract method ImplementationWithDifferentName::method() must contain a body.
 
-PHP 7.1 – 7.4 (1 error)
+PHP 7.1 – 7.4 (3 errors)
 ==========
 
+13: Non-abstract method Implementation::method() must contain a body.
+18: Non-abstract method ImplementationWithDifferentName::method() must contain a body.
 23: Named arguments are supported only on PHP 8.0 and later.
Full report

PHP 8.0 – 8.1 (2 errors)

Line Error
13 Non-abstract method Implementation::method() must contain a body.
18 Non-abstract method ImplementationWithDifferentName::method() must contain a body.

PHP 7.1 – 7.4 (3 errors)

Line Error
13 Non-abstract method Implementation::method() must contain a body.
18 Non-abstract method ImplementationWithDifferentName::method() must contain a body.
23 Named arguments are supported only on PHP 8.0 and later.

@donatj
Copy link

donatj commented Nov 30, 2023

Ran into this myself.

https://3v4l.org/eTqjU

https://phpstan.org/r/6ffc8c83-87f6-4a6e-a27a-87019836d1fe - reports no bugs even though the second call to test is unsafe.

The new warnings phpstan-bot are reporting here seem unrelated to the problem.

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

No branches or pull requests

4 participants