chore: fix plus.*NonNumeric errors from PHPStan#9260
chore: fix plus.*NonNumeric errors from PHPStan#9260kubawerlos merged 2 commits intoPHP-CS-Fixer:masterfrom
plus.*NonNumeric errors from PHPStan#9260Conversation
| if (substr_count($content, "\n") > 1) { | ||
| // the final bit of the whitespace must be the next statement's indentation | ||
| $tokens[$index] = new Token([\T_WHITESPACE, $this->whitespacesConfig->getLineEnding().substr($content, strrpos($content, "\n") + 1)]); | ||
| $tokens[$index] = new Token([\T_WHITESPACE, $this->whitespacesConfig->getLineEnding().substr($content, (int) strrpos($content, "\n") + 1)]); |
There was a problem hiding this comment.
IMO it is not a good idea to cast the potential false value to int. I would still prefer something like #9058.
There was a problem hiding this comment.
We can change to thecodingmachine/safe, or we can create a custom wrapper.
IMO the bad idea is to keep them, which (with other baseline errors) is stopping from bumping PHPStan level up, which could uncover some actually buggy places in code.
There was a problem hiding this comment.
refs the discussion about strict types and casting here: #8877 (reply in thread)
There was a problem hiding this comment.
Each of the cases here are always integers, unless I was stupid and they could false.
I'm happy to be proven the latter with a test case.
There was a problem hiding this comment.
In #9059 I found a case where continuing with the false value (implicitly casted to 0) was a problem.
I think with the explicit casts it is harder to spot those issues. I found the bug after adding explicit validation via Str helper class.
IMO the bad idea is to keep them, which (with other baseline errors) is stopping from bumping PHPStan level up, which could uncover some actually buggy places in code.
I agree that we should work on increasing the phpstan level. And with #9058 and #9008 I also try to solve some pain points in higher levels.
There was a problem hiding this comment.
in most of the cases, that value should be int already. yet, it's easy to not see some edge case there, or get fooled after some refactoring (eg we now explicitly check \\ in string, so we know strpos won't be false, but after some refactoring, flow can be different),
but then, returning false is not error - it's that string was not found.
tbh, i would rather go with assertions of type before casting it.
No description provided.