max()/min() should expect non-empty-array#2163
Conversation
|
the error in seems valid the error in seems valid the error in seems valid these error cases should check for non-empty-ness or should otherwise make sure the typen given to max/min is not empty. |
|
This pull request has been marked as ready for review. |
|
These failures are the reason why I don't like narrowing the parameter types of built-in functions... I don't see it being justified enough to force user to write |
|
I agree with this sentiment in general. in this case though the error we protect the user from is a fatal error (since php8+): |
|
Yeah but in existing code, it's very likely these arrays are always empty, although not annotated like that... |
|
not sure I get your point: do you mean there are a lot of situations in which the array will be empty but we don't know about it? |
I think maybe "are always non-empty" is what was meant? In the codebases that I've been analyzing recently, there are definitely edge cases where min/max will be passed an empty array unintentionally. No warning is emitted by PHPStan currently (as of 1.9.13), though I was actually more aware of these cases prior to the fix to the PHP 8+ min/max return types in #2161, since I would get an (erroneous) warning about using its result, e.g. It always seemed to me that PHPStan was designed to not let errors like these raise naturally, and so enforcing Either way, I fully trust you two to settle on the solution that is best for the PHP community. :) |
|
I think one way could be to enable the feature only in bleeding edge, or alternatively only in phpstan-strict-rules? |
I might be wrong but itt's not really valid for I would expect Currently it's considered as an array (https://phpstan.org/r/284ce5f5-f222-4518-8144-a90ead09b60c) |
To me, bleeding edge can be considered as "I'm sure we will release this feature in 2.0, but we want to avoid BC break". What about an |
|
thanks |
fix was easier then expected ;)
closes phpstan/phpstan#7239