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.

Sign up for GitHub

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

Jump to bottom

match() false-positive with treatPhpDocTypesAsCertain: false #5454

Closed
dktapps opened this issue Aug 11, 2021 · 8 comments
Closed

match() false-positive with treatPhpDocTypesAsCertain: false #5454

dktapps opened this issue Aug 11, 2021 · 8 comments

Comments

@dktapps
Copy link
Contributor

dktapps commented Aug 11, 2021

Bug report

Code snippet that reproduces the problem

https://phpstan.org/r/3b328b19-5f95-41a4-821b-aebd56cfc4d7
https://phpstan.org/r/6c518228-b50f-4408-8d09-42d1ae8b9e36

Expected output

No error should be reported in either case.

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

It caught some bugs in code I wrote today that I don't have to think about anymore thanks to PHPStan. It's a great relief for my overstressed brain.
@dktapps
Copy link
Contributor Author

dktapps commented Aug 11, 2021

Seems like this has something to do with the number of match arms. If I remove one of the non-FP lines, one of the FP lines stops having an error: https://phpstan.org/r/15bc7c6b-d793-4e33-9cae-36f77625cc33

dktapps added a commit to pmmp/PocketMine-MP that referenced this issue Aug 12, 2021
@dktapps
Terminal: use match instead of switch in toANSI()
6a2a4cc 
phpstan seems to have some trouble with large match statements (phpstan/phpstan#5454)
@ondrejmirtes
Copy link
Member

If we get rid of #3555 then this will also be fixed.

@dktapps
Copy link
Contributor Author

dktapps commented Aug 15, 2021

I don't really see why #3555 should need to be fixed. This problem only arises when PhpDoc certainty is disabled. There's no uncertain types involved here anyway. If the problem is type exclusion, match arms anyway shouldn't be duplicated.

@ondrejmirtes
Copy link
Member

It's the same underlying issue. If you remove this piece of code https://github.com/phpstan/phpstan-src/blob/cee6f684d0e57091435dfb03ce7d597c438fe29a/src/Type/TypeCombinator.php#L292-L300, it will start working. Feel free to try that out locally.

@dktapps
Copy link
Contributor Author

dktapps commented Aug 16, 2021 •
edited

I don't doubt it, I just don't understand why that issue has any bearing considering there's no unions or PhpDocs involved here.

@ondrejmirtes
Copy link
Member

I managed to solve this.

Look forward to the next release (0.12.95) :)

@dktapps
Copy link
Contributor Author

dktapps commented Aug 18, 2021

Awesome, thank you!
I really started using match() a lot in PM development since enforcing 8.0 minimum so I was running into this a lot 😄

ondrejmirtes added a commit to phpstan/phpstan-src that referenced this issue Aug 31, 2021
@ondrejmirtes
Regression tests
3c41113 
Ref phpstan/phpstan#4793
Ref phpstan/phpstan#5062
Ref phpstan/phpstan#5447
Ref phpstan/phpstan#5454
Ref phpstan/phpstan#3366
Ref phpstan/phpstan#5072
Ref phpstan/phpstan#3530
Ref phpstan/phpstan#5530
Ref phpstan/phpstan#1861
Ref phpstan/phpstan#4843
Ref phpstan/phpstan#4602
Ref phpstan/phpstan#4499
Ref phpstan/phpstan#2142
@github-actions
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 Sep 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ondrejmirtes @dktapps