[9.x] Add validation Rule::raw($rule, $parameters)

[stevebauman]

Closes #43751
Related #43192
Related #40924

Description

This PR implements Rule::raw(), which will return a RawRule class instance to be consumed by the ValidationRuleParser, and transformed into a raw array based validation rule:

$data = ['foo' => 'bar'];

$rules = [
    'foo' => Rule::raw('in', ['bar', 'baz']),
];

Validator::make($data, $rules)->validate();

Problem

As described by @jyd and @timacdonald in #43751 and #43192, the validator can not properly parse reserved keywords used to extract rules, parameters, and parameter values (such as pipe | and comma ,):

$data = [
    '|foo' => 'bar',
    '|baz' => 'bar',
];

$rules = [
    '|foo' => 'same:|baz',
];

// BadMethodCallException with message ...
// 'Method Illuminate\Validation\Validator::validateBaz, does not exist.'
Validator::make($data, $rules)->validate();

Solution

With Rule::raw() we can supply any string based existing Laravel validation rule and so that reserved keywords can be used in validation rule parameters, as well as input field names:

$data = [
    '|foo' => 'bar',
    '|baz' => 'bar',
];

$rules = [
    '|foo' => Rule::raw('same', '|baz'),
];

// true
Validator::make($data, $rules)->validate();

Rule::raw() also supports parameter arrays:

$rules = [
    'foo' => Rule::raw('in', ['bar', 'baz']),
];

As well as key => value pairs:

$rules = [
    'file' => Rule::raw('dimensions', [
        'min_height' => 2,
        'ratio' => '3/2'
    ]),
];

As well rules without parameters and nesting:

$rules = [
    'foo' => [
        Rule::raw('required'),
        Rule::raw('in', ['bar', 'baz']),
    ]
];

As well as Rule::forEach():

$rules = [
    'items.*' => Rule::forEach(function () {
        return [
            'name' => Rule::raw('in', ['|foo', ';bar']),
        ];
    })
];

Let me know your thoughts and if you'd like anything changed or adjusted.

Thanks for your time!

[stevebauman]

This change has been added so that parameters that have been given as key => value pairs (such as with the dimensions rule), are properly pushed onto the result, rather than attempting to explode the array, throwing an exception.

A default empty array has also been added, so that the $result starts out as an array, allowing array_push to function. It previous would be null, until it being converted into an array via $result[$key].

[garygreen]

Is this needed? For rules that use those reserved characters | : you can just define the rules using the alternative syntax:

[
   ['rule', 'param1', 'param2', ....],
   ['rule', 'param1', 'param2', ....],
]

E.g:

$data = [
	'|name' => 'Gary',
	'name2' => 'Gary',
	'role' => 'user',
];
$rules = [
	'name2' => ['required', ['same', '|name']],
	'test' => ['string', ['required_if', '|name', 'Gary']],
	'role' => ['string', ['regex', '/^(super|admin)$/i']],
];

$v = Validator::make($data, $rules);

dd($v->errors());

Output:

 Illuminate\Support\MessageBag^ {#1454
  #messages: array:2 [
    "test" => array:1 [
      0 => "The test field is required when |name is Gary."
    ]
    "role" => array:1 [
      0 => "The role format is invalid."
    ]
  ]
  #format: ":message"
}

[taylorotwell]

See above @stevebauman

[stevebauman]

@jyd @timacdonald Does the syntax mentioned by @garygreen work for you? It appears this works for me testing locally.

[stevebauman]

Disregard the above. @garygreen, this syntax doesn't work with arrays:

$data = [
    'items' => [
        ['|name' => 'foo'],
    ]
];

$rules = [
    'items' => ['array'], 
    'items.*' => ['array', ['required_array_keys', '|name']]
];

// BadMethodCallException: Method Illuminate\Validation\Validator::validateName does not exist
Validator::make($data, $rules)->passes();

However, this works:

$data = [
    'items' => [
        '|name' => 'foo',
    ]
];

$rules = [
    'items' => ['array', ['required_array_keys', '|name']], 
];

// true
Validator::make($data, $rules)->passes();

[garygreen]

Hmm that's strange, I'm actually testing this on our Laravel 6 LTS release - your first code works in Laravel 6, it passes. So presumably something in a newer Laravel version has caused this?

Edit: seems to also work in Laravel 8... haven't tested 9 yet.

[garygreen]

Yep, your example is broken in Laravel 9... but Laravel 6, 7, 8 all works with your first example. What's changed in 9 to cause that bug? I have no idea.

[stevebauman]

@taylorotwell Maybe we should just revert Arr::flatten() here:

framework/src/Illuminate/Validation/ValidationRuleParser.php

Lines 154 to 160 in 2d6d02a

	foreach (Arr::flatten((array) $rules) as $rule) {
	if ($rule instanceof NestedRules) {
	$compiled = $rule->compile($key, $value, $data);
	$this->implicitAttributes = array_merge_recursive(
	$compiled->implicitAttributes,
	$this->implicitAttributes,

To it's previous state (before Laravel 9):

foreach ((array) $rules) as $rule) { 
    // ...
}

Rule::forEach() is not currently documented to support nesting in arrays (the below works now due to Arr::flatten):

https://laravel.com/docs/9.x/validation#accessing-nested-array-data

$data = [
    'items' => [
        ['name' => ['foo']],
    ],
];

$rules = [
    'items.*' => [
        Rule::forEach(function () {
            return ['name' => 'regex:/^(foo)$/i'];
        })
    ],
];

Validator::make($data, $rules)->passes();

The correct, documented syntax:

$data = [
    'items' => [
        ['name' => ['foo']],
    ],
];

$rules = [
    'items.*' => Rule::forEach(function () {
        return ['name' => 'regex:/^(foo)$/i'];
    }),
];

Validator::make($data, $rules)->passes();

Removing the mentioned Arr::flatten() will restore previous functionality, and having nested Rule::forEach()s can be marked as unintentional.

What are your thoughts?

---

Here's a branch where I've removed flatten and updated the tests:

9.x...stevebauman:framework:fix-nested-array-validation-rules

https://github.com/stevebauman/framework/actions/runs/2891116673

[garygreen]

It looks like the issue was accidentally introduced with this PR? #40498

I think it makes sense to revert a change that introduces that bug, especially as Laravel 6 LTS is coming to an end soon and many people will be looking to upgrade to Laravel 9 and could stumble into the issue.

having nested Rule::forEach()s can be marked as unintentional.

Does this mean the below is not possible? (I've yet to use Rule::forEach, so excuse my ignorance!)

$rules = [
    'items.*' => Rule::forEach(function () {
        return ['more_items' => Rule::forEach(function() {
            return ['name' =>  'regex:/^(foo)$/i'],
        });
    }),
];

[stevebauman]

It looks like the issue was accidentally introduced with this PR? #40498

@garygreen Well it was intentionally introduced to allow for multiple Rule::forEach() calls in a single rule 😅

$rules = [
    'users.*.name' => [
        Rule::forEach(function ($value, $attribute, $data) {
            // ...
        }),
        Rule::forEach(function ($value, $attribute, $data) {
            // ...
        }),
    ],
];

But it was never documented, and it introduced the regression we're talking about now 😞

Does this mean the below is not possible? (I've yet to use Rule::forEach, so excuse my ignorance!)

Nope, your example will still work -- see this test case in the above mentioned, fixed branch:

https://github.com/stevebauman/framework/blob/35c08742d5c7735cc826425c555b26bc8004db62/tests/Validation/ValidationForEachTest.php#L176-L188

It will only not work (if updated) when a Rule::forEach() is wrapped in an array. 👍

[garygreen]

@stevebauman Ahh that's cool then that nested one will still work. I thought the forEach meant it was doing a loop of some kind, really it's just a callback used to add additional rules? I would of named it Rule::push() or Rule::merge(). But guess it's a done deal now lol. Pretty cool feature!

[stevebauman]

@garygreen

I thought the forEach meant it was doing a loop of some kind, really it's just a callback used to add additional rules?

That's incorrect -- it is actually looping over each item in the array. It allows you to adjust validation rules per item, depending on the value of that item, as well as further nest array rules.

[jryd]

To throw my 2 cents in; I'm broadly supportive of the idea that we roll back the change to flatten the rules.

However, would this not be considered a "breaking" change given that users could be using the rule inside of an array of rules - even if it wasn't explicitly written to work that way, it worked and therefore there could be people that find it breaks by reverting that change.
Selfishly, I'd love to see it reverted to make my life easier, but I don't want to then cause grief for others; although the only mitigating factor is that it's a fairly easy fix with a clear to remedying any that are affected.

In the example of multiple Rule::forEach calls, I cannot think of a valid reason you'd want to do this. Considering that the rule returns an array of rules, multiple usages of Rule::forEach should just be squashed down to one call.

$rules = [
    'users.*.name' => [
        Rule::forEach(function ($value, $attribute, $data) {
            // ...
        }),
        Rule::forEach(function ($value, $attribute, $data) {
            // ...
        }),
    ],
];

Additionally, by rolling back the change you'd have the option of removing the fix put in place for regex rules as that shouldn't be needed anymore right?

[stevebauman]

@jryd

In the example of multiple Rule::forEach calls, I cannot think of a valid reason you'd want to do this. Considering that the rule returns an array of rules, multiple usages of Rule::forEach should just be squashed down to one call.

Yeah agreed 👍 . I initially did this so you can place rules alongside a Rule::forEach():

$rules = [
    'users.*.name' => [
        'in:foo,bar',
        Rule::forEach(function ($value, $attribute, $data) {
            // ...
        }),
    ],
];

But in hindsight, you might as well place it inside.

Additionally, by rolling back the change you'd have the option of removing the fix put in place for regex rules as that shouldn't be needed anymore right?

Yup I believe so. I (personally) think this would be the best way to go, so we can keep the Rule::forEach feature, while maintaining all backwards compatibility.

[garygreen]

@stevebauman Will you be making a PR for those two things? So that's for the array regression issue introduced in Laravel 9 (#40498) and removing the unnecessary regex thing (was that part of that same PR or different one?)

Also it might be worth chiming in on #43751 because it looks like nobody realised the alternative syntax could be used, so that whole discussion of hacky fixes and proposals wasn't necessary.

[stevebauman]

@garygreen I've just submitted it, please see #43897 👍

removing the unnecessary regex thing (was that part of that same PR or different one?)

Removing a protected function would be a major breaking change, so it will have to stay until Laravel 10. Though, it's also not causing any harm either -- so it'd be up to the maintainers if removing is worth it.