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

Variable flagged as "might not be defined" when declared before an exception can be thrown. #4838

Closed
frankdejonge opened this issue Apr 11, 2021 · 8 comments

Comments

@frankdejonge
Copy link

Bug report

When a variable is set in a block that later in the block throws an error, a "variable might not be set" error is raised incorrectly.

Code snippet that reproduces the problem

https://phpstan.org/r/8dbd457d-5547-483e-972c-ff5d32725d20

Expected output

No errors were found.

@ondrejmirtes
Copy link
Member

PHPStan considers all built-in functions to possibly throw an exception, because of TypeError and ArgumentCountError usually. I could probably change the logic so that built-in functions without any required parameters will not throw anything.

@frankdejonge
Copy link
Author

frankdejonge commented Apr 11, 2021

I think that makes sense for functions without parameters. But if I understand that correctly, any core function that accepts parameters are expected to throw a TypeError, therefor cannot be trusted even if the type and number of the incoming parameters is known?

@ondrejmirtes
Copy link
Member

That's difficult - if I marked the function as not throwing when the number and types of arguments match, the user might still choose to catch the exception because someone might not observe the types "enforced" by PHPDocs and therefore the exception might really get thrown. But with the proposed logic, the catch would be marked as dead...

@ondrejmirtes
Copy link
Member

Also, you can avoid this problem by moving it out of try {}.

@frankdejonge
Copy link
Author

I realise I'm saying this to the person who's dealing with this every day, but I've always thought that is exactly where SA delivers value. Confidently reducing the number of possible error scenarios by ensuring types (and therefor type errors) are correctly handled. What makes a core function different in this regard different from any other function?

Put differently: When I declare a function and all the types match and the number of arguments match as well, there is no error flagged. However, when this function is declared in the core, there is. is that correct or am I oversimplifying it?

@ondrejmirtes
Copy link
Member

The problem is that PHPStan might not recognize all errors states and I don't want to encourage people removing their catch (\Throwable) when they still might be needed. For example PHP 8 introduced ValueError, and PHPStan doesn't have the information when a function might throw that.

@ondrejmirtes
Copy link
Member

Fixed: phpstan/phpstan-src@09d3488

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

No branches or pull requests

2 participants