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

Not throwing Method ... overrides method ... but misses parameter when compatible with an interface but not parent. #7859

Closed
jlekowski opened this issue Aug 25, 2022 · 5 comments

Comments

@jlekowski
Copy link

Bug report

An overridden method signature different than in a parent class is considered correct even though PHP throw Fatal error.

I believe the issue comes from the fact that the method is compared to its prototype https://github.com/phpstan/phpstan-src/blob/1.8.x/src/Rules/Methods/MethodParameterComparisonHelper.php#L35 and the prototype is taken from here https://github.com/ondrejmirtes/BetterReflection/blob/5.8.x/src/Reflection/ReflectionMethod.php#L171, which is looking for an interface instead of the immediate parent method.

Code snippet that reproduces the problem

<?php declare(strict_types = 1);

interface A
{
	public function test(): void;
}

class B implements A
{
	public function test(mixed $a = null): void
	{
	}
}

class C extends B
{
	public function test(): void
	{
	}
}

or links below:
https://phpstan.org/r/dafd527b-bf9f-484d-b660-237ef64309a7 - the error is marked for ExtendingClassNotImplementingSomeInterface class only and ExtendingClassImplementingSomeInterface is considered to be correct.
An example where PHP throws the following error https://3v4l.org/1PYEd#v8.1.9

Fatal error: Declaration of ExtendingClassImplementingSomeInterface::getList(mixed $a): void must be compatible with ImplementingSomeInterface::getList(mixed $a, bool $b = false): void in /in/1PYEd on line 19

Expected output

Method ExtendingClassImplementingSomeInterface::getList() overrides method ImplementingSomeInterface::getList() but misses parameter #2 $b.

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

I run it before each commit :)

@canvural
Copy link
Contributor

Maybe the same as #7388?

@jlekowski
Copy link
Author

@canvural, it is very similar, thanks for the reference.
I'm not sure if the bug you reported is caused by the same logic. In my case, I can see it is the method's prototype being taken from the (parent's) interface and the immediate parent method is ignored.

@phpstan-bot
Copy link
Contributor

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

@@ @@
+19: Method ExtendingClassImplementingSomeInterface::getList() overrides method ImplementingSomeInterface::getList() but misses parameter #2 $b.
 35: Method ExtendingClassNotImplementingSomeInterface::getList() overrides method NotImplementingSomeInterface::getList() but misses parameter #2 $b.
Full report
Line Error
19 Method ExtendingClassImplementingSomeInterface::getList() overrides method ImplementingSomeInterface::getList() but misses parameter #2 $b.
35 Method ExtendingClassNotImplementingSomeInterface::getList() overrides method NotImplementingSomeInterface::getList() but misses parameter #2 $b.

@jlekowski
Copy link
Author

Thanks @ondrejmirtes 👍

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

4 participants