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

is_subclass_of is true only for parent classes, not the same one #6305

Closed
mvorisek opened this issue Jan 4, 2022 · 13 comments · Fixed by phpstan/phpstan-src#1044
Closed
Labels
Milestone

Comments

@mvorisek
Copy link
Contributor

mvorisek commented Jan 4, 2022

Bug report

is_subclass_of(new A(), A::class) is false, phpstan assumes true

Code snippet that reproduces the problem

https://3v4l.org/Z4Yb2

https://phpstan.org/r/b41d286a-72cb-45a7-b48f-5dbedc4dee73

Expected output

Call to function is_subclass_of() with B and 'A' will always evaluate to true. only

@staabm
Copy link
Contributor

staabm commented Jan 13, 2022

@ondrejmirtes
Copy link
Member

The problem is that PHPStan isn't able to represent "subclass of A but not A".

@ondrejmirtes
Copy link
Member

But I guess it could be if-elsed out in IsSubclassOfFunctionTypeSpecifyingExtension.

@herndlm
Copy link
Contributor

herndlm commented Mar 2, 2022

because @staabm found that further up - https://github.com/phpstan/phpstan-src/blob/7667265bba564eb61711ad52376a6e7823708bda/src/Type/Php/TypeSpecifyingFunctionsDynamicReturnTypeExtension.php#L40
should is_a be added there as well? I'm not entirely sure what affects this will have

@mvorisek
Copy link
Contributor Author

mvorisek commented Mar 2, 2022

https://phpstan.org/r/d16311bf-3c5a-4e70-84d0-6da5cf01efa2 seems quite broken to me

@herndlm
Copy link
Contributor

herndlm commented Mar 2, 2022

is_subclass_of is fine I think, but it seems weird to me that $b is considered to be A only in https://phpstan.org/r/045b5d07-ca5c-45c9-8cd1-3665296236b3

@staabm
Copy link
Contributor

staabm commented Mar 2, 2022

is_subclass_of is fine I think, but it seems weird to me that $b is considered to be A only

I think so too

@mvorisek
Copy link
Contributor Author

mvorisek commented Mar 2, 2022

https://phpstan.org/r/d37934ec-1b06-428a-ad62-bd4b9f54e905

l11 and l17 errors seems wrong to me

@staabm
Copy link
Contributor

staabm commented Mar 2, 2022

l11 and l17 errors seems wrong to me

checking a condition which already was checked one layer above is redundant. if you don't find a more complete example, IMO phpstan is correct.

@mvorisek
Copy link
Contributor Author

mvorisek commented Mar 2, 2022

I agree, but if it is part of a more complex condition, it can be needed. And MUST always result the same output.

@staabm
Copy link
Contributor

staabm commented Mar 2, 2022

I agree, but if it is part of a more complex condition, it can be needed. And MUST always result the same output.

please find a example illustrating your case and open a new issue with it

@herndlm
Copy link
Contributor

herndlm commented Mar 3, 2022

Fyi I'm trying to fix this properly, I have something that is working, but need to clean it up and add more tests still

@github-actions
Copy link

github-actions bot commented Apr 4, 2022

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 Apr 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants