random_int() return type and parameters rule#99
Conversation
There was a problem hiding this comment.
This situation might be fine - the code might be correct. This is typically only reported on level 7 thanks to reportMaybes config option.
Feel free to also add this situation to acceptTypes.php in tests/PHPStan/Levels. If you add an offending code and re-run the tests, the asserting JSON files will be updated automatically. Most of the violations should be reported on level 6, but this one on level 7.
There was a problem hiding this comment.
This is the only thing I have not address yet.
There was a problem hiding this comment.
We might be able to get rid of all of these ifs with Type::isSuperTypeOf() method. It's really powerful :) If you imagine types as sets then this abstraction holds really well :)
There was a problem hiding this comment.
Yeah, the eureka moment was realising all I needed to do was create a type representing the allowable values for the $max parameter.
ondrejmirtes
left a comment
There was a problem hiding this comment.
Also, you should look into the level 7 stuff with reportMaybe I suggested in the first review. Otherwise it's mergeable! 👍
Yeah not entirely sure what should be considered a "maybe", if the max argument could be less than the min argument should that be a maybe? e.g. |
ondrejmirtes
left a comment
There was a problem hiding this comment.
Last change and we're good to go!
|
Thank you, I love it! |
This PR adds a return type extension which teaches PHPStan that
random_int()will return an integer between the min/max parameters. The code I've written to get the lower/upper bounds could probably be extracted to be reused as it should handle crazy types likeint<0,10>|20|25|int<30,100>which I don't think PHPStan will even create currently.Secondly there is a rule which performs validation of the input parameters to ensure
$min <= $maxalways holds true. Not sure on what level this should be configured or if it belongs in the strict rules repo.Unfortunately it's not possible to validate
random_int(PHP_INT_MAX, PHP_INT_MIN)as the min/max constants are just converted to a plain integer type, perhaps there is scope for{Min,Max}IntegerTypeclasses.