Draft: Explicit checking of parameter type values. #257
Conversation
patka-123
changed the title
|
Just hypothetically, why would somebody prefer public static function contains($value, $subString, $message = '')
{
static::string($value, 'Expected value parameter to be a string. Got: %s');
static::string($subString, 'Expected value parameter to be a string. Got: %s');over public static function contains(string $value, string $subString, $message = '')
{? |
| @@ -1736,6 +1789,8 @@ public static function validArrayKey($value, $message = '') | |||
| */ | |||
| public static function count($array, $number, $message = '') | |||
| { | |||
| static::natural($number, 'Expected number parameter to be a natural number. Got: %s'); | |||
There was a problem hiding this comment.
This is a BC break, as now floats will no loner work.
Although count will always return an int, 3.0 == 3 will be true.
And while it may not be logical in most cases, there is no reason count can't return a negative int. Which we should also allow
| * @param string $message | ||
| * | ||
| * @throws InvalidArgumentException | ||
| */ | ||
| public static function minCount($array, $min, $message = '') | ||
| { | ||
| static::natural($min, 'Expected min parameter to be a natural number. Got: %s'); |
There was a problem hiding this comment.
We should keep allowing floats
| @@ -1751,16 +1806,18 @@ public static function count($array, $number, $message = '') | |||
| * Does not check if $array is countable, this can generate a warning on php versions after 7.2. | |||
| * | |||
| * @param Countable|array $array | |||
| * @param int|float $min | |||
| * @param int $min | |||
There was a problem hiding this comment.
Any change in the params is a BC break, as peoples static analysis will break on this.
| @@ -1895,6 +1957,8 @@ public static function isNonEmptyMap($array, $message = '') | |||
| */ | |||
| public static function uuid($value, $message = '') | |||
| { | |||
| static::string($value, 'Expected value parameter to be a string. Got: %s'); | |||
There was a problem hiding this comment.
This isn't correct, as an object with __toString will also work here, e.g: https://3v4l.org/6n4rK
This is so for basically all assertions that operate on strings.
| @@ -1677,6 +1724,9 @@ public static function methodNotExists($classOrObject, $method, $message = '') | |||
| */ | |||
| public static function keyExists($array, $key, $message = '') | |||
| { | |||
| static::is_array($array, 'Expected array parameter to be a valid array. Got: %s'); | |||
There was a problem hiding this comment.
is_array is not a valid assertion, you propbably meant to use isArray
| @@ -1404,6 +1440,9 @@ public static function maxLength($value, $max, $message = '') | |||
| */ | |||
| public static function lengthBetween($value, $min, $max, $message = '') | |||
| { | |||
| static::positiveInteger($min, 'Expected min parameter to be a postive integer. Got: %s'); | |||
There was a problem hiding this comment.
This breaks with floats, and also no longer allows 0, same with the other length related assertions
In order to fix #119 I started to add assertions on the functions that might generate a warning. In the end I decided to check all other functions as well.
There were also some incorrect type annotations. I carefully checked that functions that needed to be mixed stayed that way (etc).