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

covariant return type error not detected #9014

Closed
staabm opened this issue Mar 9, 2023 · 10 comments
Closed

covariant return type error not detected #9014

staabm opened this issue Mar 9, 2023 · 10 comments

Comments

@staabm
Copy link
Contributor

staabm commented Mar 9, 2023

Bug report

phpstan can detect covariant return types in situations like https://phpstan.org/r/892b3d92-db4c-4fd3-9d5f-b47fb5bf3355.
detecting these is important, because they turn into fatal errors at runtime https://3v4l.org/jdXiU

Code snippet that reproduces the problem

It turns out though, that phpstan does only compare the method in question with the top-most baseclass.
when more layers of inheritance are involved the covariant type is not detected and the runtime fatal error keeps hidden

https://phpstan.org/r/2c6d3ee9-55cb-4ad9-9afd-c6e293f1b3f2
https://3v4l.org/OPasV

Expected output

A non-ignorable error on line 28

Return type mixed of method extended::renderForUser() is not covariant with return type string of method middle::renderForUser().
@ondrejmirtes
Copy link
Member

Why is this a problem? You already have the error for middle.

@staabm
Copy link
Contributor Author

staabm commented Mar 9, 2023

in our case extended was on the phpstan analyze path and middle and base both were defined in a vendor/ package which we did not scan with phpstan - because its just a dependency

@ondrejmirtes
Copy link
Member

Yeah, but middle is already wrong which should be fixed first and foremost. If the problem wasn't in the dependency then PHPStan wouldn't have anything to report.

@staabm
Copy link
Contributor Author

staabm commented Mar 9, 2023

ok, let me be more precise about the layers involved :-).

the base-package contains middle and base:
https://phpstan.org/r/d645ea74-7b18-4145-8488-e5bba9864797
which is from the base-package point of view all green.

the project which consumes the base-package via composer declares only the extended class, which analyzed on its own, without analyzing middle and base does not report the non-ignorable problem either.
https://phpstan.org/r/00ca30ad-0420-459a-a25a-658b9c7f7f67

only when all 3 classes are on the analyzed path, I get the error

@ondrejmirtes
Copy link
Member

Oh right, didn't realize that https://phpstan.org/r/d645ea74-7b18-4145-8488-e5bba9864797 is valid.

@ondrejmirtes
Copy link
Member

@phpstan-bot
Copy link
Contributor

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

@@ @@
+PHP 7.4 – 8.2 (4 errors)
+==========
+
  7: Method base::renderForUser() has no return type specified.
 17: Method middle::renderForUser() should return string but return statement is missing.
-28: Method extended::renderForUser() has no return type specified.
+28: Method extended::renderForUser() has no return type specified.
+28: Return type mixed of method extended::renderForUser() is not covariant with return type string of method middle::renderForUser().
+
+PHP 7.2 – 7.3 (4 errors)
+==========
+
+ 7: Method base::renderForUser() has no return type specified.
+17: Method middle::renderForUser() should return string but return statement is missing.
+28: Method extended::renderForUser() has no return type specified.
+28: Return type mixed of method extended::renderForUser() is not compatible with return type string of method middle::renderForUser().
Full report

PHP 7.4 – 8.2 (4 errors)

Line Error
7 Method base::renderForUser() has no return type specified.
17 Method middle::renderForUser() should return string but return statement is missing.
28 Method extended::renderForUser() has no return type specified.
28 Return type mixed of method extended::renderForUser() is not covariant with return type string of method middle::renderForUser().

PHP 7.2 – 7.3 (4 errors)

Line Error
7 Method base::renderForUser() has no return type specified.
17 Method middle::renderForUser() should return string but return statement is missing.
28 Method extended::renderForUser() has no return type specified.
28 Return type mixed of method extended::renderForUser() is not compatible with return type string of method middle::renderForUser().

@phpstan-bot
Copy link
Contributor

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

@@ @@
+PHP 7.4 – 8.2 (4 errors)
+==========
+
  7: Method base::renderForUser() has no return type specified.
 15: Method middle::renderForUser() should return string but return statement is missing.
-26: Method extended::renderForUser() has no return type specified.
+26: Method extended::renderForUser() has no return type specified.
+26: Return type mixed of method extended::renderForUser() is not covariant with return type string of method middle::renderForUser().
+
+PHP 7.2 – 7.3 (4 errors)
+==========
+
+ 7: Method base::renderForUser() has no return type specified.
+15: Method middle::renderForUser() should return string but return statement is missing.
+26: Method extended::renderForUser() has no return type specified.
+26: Return type mixed of method extended::renderForUser() is not compatible with return type string of method middle::renderForUser().
Full report

PHP 7.4 – 8.2 (4 errors)

Line Error
7 Method base::renderForUser() has no return type specified.
15 Method middle::renderForUser() should return string but return statement is missing.
26 Method extended::renderForUser() has no return type specified.
26 Return type mixed of method extended::renderForUser() is not covariant with return type string of method middle::renderForUser().

PHP 7.2 – 7.3 (4 errors)

Line Error
7 Method base::renderForUser() has no return type specified.
15 Method middle::renderForUser() should return string but return statement is missing.
26 Method extended::renderForUser() has no return type specified.
26 Return type mixed of method extended::renderForUser() is not compatible with return type string of method middle::renderForUser().

@phpstan-bot
Copy link
Contributor

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

@@ @@
-No errors
+15: Method Bar::test() overrides method Foo::test() but misses parameter #2 $test.
Full report
Line Error
15 Method Bar::test() overrides method Foo::test() but misses parameter #2 $test.

Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants