somnambulist-tech / validation

A re-write of rakit/validation, a standalone validation library inspired by Laravel Validation
MIT License
44 stars 13 forks source link

TypeError is thrown if rules like Url,Regex recieves default NULL value #11

Closed jagdtiger9 closed 1 year ago

jagdtiger9 commented 1 year ago

Somnambulist\Components\Validation\Rules\Url::validateCommonScheme(): Argument #1 ($value) must be of type string, null given, called in .../vendor/somnambulist/validation/src/Rules/Url.php on line 32

So it's impossible to use them as is in cases when validated values are optional

dave-redfern commented 1 year ago

@jagdtiger9 did you use the nullable or sometimes rules to flag that the validation can be skipped when missing values? e.g.: sometimes|regex:\d+.

dave-redfern commented 1 year ago

I've added an explicit cast regardless, 1.5.2 has this added.

jagdtiger9 commented 1 year ago

@jagdtiger9 did you use the nullable or sometimes rules to flag that the validation can be skipped when missing values? e.g.: sometimes|regex:\d+.

Sorry, missed that option I did it rakit-like way

So 'value' => 'nullable|url' is OK, and 'value' => 'url' gets TypeError in case value is undefined But doesn't that conflict 'required|url' statement semantically? No 'required' means null is allowed, so 'nullable' is kind of redundant Just need some clarification please

dave-redfern commented 1 year ago

Pedantically, rakit allows it because it does not use strict_types, whereas this port does. So really it should have always failed with an error, but PHP automagically does the type-juggling so it "works" even if it is not "correct".

I guess the "best" solution is to add additional type casting so these rules will work the same as rakit, at least for the 1.X branch, and then for 2.X maybe require using nullable/sometimes for optional or null values and everything else is treated as being required?

I am open to comments on that type of change. From my PoV semantically: why would url allow null since the rule appears to want a valid URL (for example), vs nullable|url that would mean explicitly null or a valid url. Thoughts?

jagdtiger9 commented 1 year ago

That sounds reasonable, 'nullable|url' do it in a more explicit way and 'required' rule has no meaning in that case but still TypeError is an operation fail, not a validation error

And there are some rules like 'date' or 'digits' that use typecast string to avoid TypeError and 'required' rule is meaningfull - 'required|date'