vimeo / psalm

A static analysis tool for finding errors in PHP applications
https://psalm.dev
MIT License
5.56k stars 660 forks source link

Annotate "non-castable" strings #9275

Open kkmuffme opened 1 year ago

kkmuffme commented 1 year ago

https://psalm.dev/r/61b9096bc4

In practice this often happens when some underlying function uses array_keys() where people sometimes then typecast to int - since those cases are often error handling cases, you end up with incorrect code that isn't easy to spot in testing (since those are rare error cases you have and the tests pass since in your error case you only have ints, unless you want to duplicate all tests)

Any suggestions how this could be improved to make a string type give a PossiblyInvalidCast (or maybe RiskyCast) in some cases? Maybe via annotation on the function? (@psalm-???)

psalm-github-bot[bot] commented 1 year ago

I found these snippets:

https://psalm.dev/r/61b9096bc4 ```php 5) { return rand(6, 10); } return uniqid(); } echo (int) foo(); ``` ``` Psalm output (using commit 5d1fe88): No issues! ```
orklah commented 1 year ago

We could probably allow only numeric-string to be casted into int. That would be veeery strict though, so probably under a config?

kkmuffme commented 1 year ago

What do you think about:

typecasting these types to anything except string (since there's already RedundantCast error for that case), should give a "RiskyCast" error: class-string interface-string trait-string enum-string callable-string non-falsy-string lowercase-string non-empty-string (up for discussion, but I think it makes sense, since you can use numeric-string for any non-empty numeric strings) non-empty-lowercase-string literal strings that are non-numeric

As for those types, the cast will always be 0 But it's specific enough, to not give unnecessary errors for generic string types.