[L5] Validation method returns data that's not been validated

[garygreen]

$input = [
    'framework' => 'Laravel',
    'version' => '8.90',
];

$rules = [
    'framework' => 'required|string',
];

$validator = \Validator::make($input, $rules);

dd($validator->valid());

Returns

array:2 [▼
  "framework" => "Laravel"
  "version" => "8.90"
]

Technically the 'version' hasn't been validated at all, so shouldn't form part of the valid data. Taylor, how do you feel about storing the rules which passed validation in validate() method and use that when plucking which data is valid?

It would be cool to use this method to pass the validated attributes / data straight onto your repository / eloquent. At the moment this method is kinda misleading?

[stuartmccord]

This is probably quite subjective, I would consider the version to be valid as it hasn't failed any validation rules. I think this functionality would be useful but should go in a different method.

[garygreen]

@stuartmccord to me it's not subjective at all. The valid method should be the inverse of invalid. Invalid operatives on rules that were RUN and deemed invalid. The valid method should also return data against rules which were RUN and valid. The keyword here being RUN. At the moment the valid method returns things in a different context to the invalid method.

[stuartmccord]

@garygreen I know what you meant but my point was that it depends how you want to define what is valid and invalid. In my opinion valid would be any data that did not fail and invalid is that which did, (and by not defining any rules for a field you are saying anything goes). By this definition the valid and invalid methods return the inverse of each other, (eg. your example returns an empty array).

If you exclude those that had no rules run then you could have the situation where data is neither valid or invalid, such as version in your example.

[garygreen]

I see where your coming from but I still think it should return data that was validated, not data that wasn't validated against any rules. That's the whole point of a validation class, to determine what data is valid and invalid; it's a black and white thing and shouldn't care about anything in the middle/gray area.

With what your saying, in theory, you could have data that wasn't validated returned in the invalid method. Why? Because you haven't determined if it's either valid or invalid. So based on your methodology it's "correct" output for both functions. To me that's illogical.

[stuartmccord]

What I was saying was that the grey area data should be returned by the valid method, the way Laravel does it now. We're probably not going to agree on what to do with that data, its not my call to make here, I'm happy to leave this for whoever needs to make that decision.

[manan-jadhav]

What about defining a new method passed()? It will contain the data that actually passed the rules. This method will stay alongside the valid and invalid methods.

[garygreen]

What I was saying was that the grey area data should be returned by the valid method

Why? In the example in my opening post version should be returned in invalid() because I KNOW it's not valid, but it's instead returned in valid(). It's an absurd assumption to make when it hasn't been validated at all so shouldn't be returned in either valid or invalid.

What about defining a new method passed()? It will contain the data that actually passed the rules. This method will stay alongside the valid and invalid methods.

Because this function should be the inverse of the invalid method (which returns only data that was not valid), therefore this function should return only data that was specifically validated. If anything there should just be a function for like unvalidated() which will return all the data that wasn't validated at all (because of no rules etc). That separates the real "valid" and "invalid" data from the "gray" data area.

[stuartmccord]

That is YOUR opinion on how to make A validator class, there are other considerations to make, particularly in a framework, such as how people want to use the feature and it has to work for the majority of people. I don't particularly want to type a load of empty array values when I KNOW that whatever the user types in will be allowed through.

As I've said already, I'm really not interested in discussing this any further with you, I've made my opinion clear for whoever is looking at this.

[Sladewill]

What is the point of the valid and invalid functions then if not to show the different messages, you already have the passes and fails methods, the valid and invalid should cleanup the data otherwise the functions are pointless.

[garygreen]

@GrahamCampbell If I was to do a PR proposal to fix this, would I target this at master or 5.0 ? (Assuming master as it would technically be breaking "fix" if people relied on it to return non-validated input).

[GrahamCampbell]

@garygreen ask @taylorotwell

[manan-jadhav]

I think this will be an added functionality, not a fix.

[GrahamCampbell]

This concept has been rejected by Taylor in that PR.

Changing this would be a massive BR break too.