Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Weird numeric constraint behaviour #37529

Closed
SCIF opened this issue May 29, 2021 · 11 comments
Closed

Weird numeric constraint behaviour #37529

SCIF opened this issue May 29, 2021 · 11 comments

Comments

@SCIF
Copy link
Contributor

SCIF commented May 29, 2021

  • Laravel Version: 8.40.0
  • PHP Version: 7.4.19
  • Database Driver & Version: doesn't matter

Description:

A quote from https://laravel.com/docs/8.x/validation#rule-numeric

nullable
The field under validation may be null.

numeric
The field under validation must be numeric

So when I supply a value '' I'm expecting to have a failure as none of rules are met.

Steps To Reproduce:

$v = app('validator');
$try = $v->make(['empty' => ''], ['empty' => 'nullable|numeric');
>>> $try->validated();
=> [
     "empty" => "",
   ]
>>> is_numeric('');
=> false
@SCIF SCIF changed the title Weird numeric behaviour Weird numeric constraint behaviour May 29, 2021
@devcircus
Copy link
Contributor

Might be the "ConvertEmptyStringsToNull" middleware that runs by default in Laravel applications. You should be able to remove that from the kernel.php and get the result you want. However, you may want to rethink the logic, as converting empty strings to null is the most common way of handling empty strings in form submissions.

@SCIF
Copy link
Contributor Author

SCIF commented May 29, 2021

Hi @devcircus , sorry but your suggestion is actually a workaround for sourcing data from a request. The question is about validation and constraints so doesn't matter where the data comes from.

@driesvints
Copy link
Member

So the way the validator works is that it'll allow the data to pass when it's not "required". You aren't requiring empty to be present so therefor it's not checked to be numeric either. It passes because an empty string doesn't holds a value in the validator's eyes and therefor can pass through. If you add required to your ruleset you should get the behavior that you're looking for:

$v->make(['empty' => ''], ['empty' => 'required|numeric');

@SCIF
Copy link
Contributor Author

SCIF commented May 31, 2021

Hi @driesvints , thanks for the response.

However, it's far from obvious behavior and must be clearly stated.

Moreover, it doesn't work and doesn't provide the required (and mentioned in docs) behavior:

$try = $v->make(['empty' => null], ['empty' => 'numeric|required']);
>>> $try->errors()->all();
=> [
     "The empty must be a number.",
     "The empty field is required.",
   ]

@driesvints
Copy link
Member

@SCIF in that case you need to add the nullable rule as well. We definitely welcome prs to the docs if you think something can be more clear 👍

@SCIF
Copy link
Contributor Author

SCIF commented May 31, 2021

Hi @driesvints , I checked that behavior before filing this issue:

>>> $try = $v->make(['empty' => null], ['empty' => 'numeric|required|nullable']);
>>> $try->errors()->all();
=> [
     "The empty field is required.",
   ]

@driesvints
Copy link
Member

I'm sorry but that looks good to me. Like I said if any is unclear feel free to send in a pr to the docs.

@SCIF
Copy link
Contributor Author

SCIF commented May 31, 2021

So there is no way to be sure whether a field has value of null or numeric?

@SCIF
Copy link
Contributor Author

SCIF commented May 31, 2021

I'm sorry but that looks good to me. Like I said if any is unclear feel free to send in a pr to the docs.

Sorry, but I don't really understand how to explain that check for null and check for is_numeric doesn't allow me to check whether value is null|numeric (in psalm notation). Both checks are simple and clear but doesn't work. Should I add to both rules «Be aware, it works occasionally»? :)

@SCIF
Copy link
Contributor Author

SCIF commented May 31, 2021

Or may be nullable check should be considered «conflicting for combining»? I.e. nullable cannot be used in any conjunction with other type constraints and in fact means '' or null? That would explain the behavior a little bit as I'm still not certain how does it work (disregard unclear of docs, just to achieve my goals I have to guess/brut force combinations of constraints).

@JonathanGawrych
Copy link
Contributor

I'm having the same issue. The problem goes all the way to #18376, 4.5 years ago. Before v5.4.16, nullable would override implicit rules (required*, present, filled, accepted), but now it does not. In order to solve this, I used this stack overflow answer to extend the Validator and remove the "is implicit" check from isNotNullIfMarkedAsNullable, so now that I can do required|integer|nullable and then things behave like expected:

>>> validator([], ['foo' => ['required','nullable','integer']])->errors()->toArray();
=> ["foo" => ["The foo field is required."]]
>>> validator(['foo' => ''], ['foo' => ['required','nullable','integer']])->errors()->toArray();
=> ["foo" => ["The foo field is required."]]
>>> validator(['foo' => 5], ['foo' => ['required','nullable','integer']])->errors()->toArray();
=> []
>>> validator(['foo' => null], ['foo' => ['required','nullable','integer']])->errors()->toArray();
=> []

If you still want to be able to omit the field entirely, change required to filled

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants