Error returned by "Any" dict validator should be of the closest alternative

[ydidwania]

Validation of Any(..) is done in the order in which the alternatives are written. When using dicts, if none of them match, the deepest error is returned. The error (that is returned in the end) is updated only when it is more nested in an alternate dict than the already existing error.

If all dicts are of the same level, the error is from the first dict in Any(..). It would be better if Any() is made to return the error of the closest alternative. Or provide a discriminant function in the parameters used to generate error messages.

For the example below the validation error for all three types is at the same level of the dict. The error for type A is returned as that is the first to be validated.

from voluptuous import Schema

sch = Schema({
    # this field is keyed by 'type'
    'implementation': Any({
        'type': 'A',
        'a-value': str,
    }, {
        'type': 'B',
        'b-value': int,
    }, {
        'type': 'C',
        'c-value': bool,
    })  
})

# should be OK
sch({
    'implementation': {
        'type': 'A',
        'a-value': 'hello',}  
})

# but this one gives a very confusing error message, because Voluptuous does
# not realize this is a (malformed) type-C implementation. Most confusingly,
# it says the value for 'type' is not allowed, when really it is!
# raises(
#     er.MultipleInvalid, 
#    'extra keys not allowed @ data['implementation']['c-value']', 
#    'not a valid value for dictionary value @ data['implementation']['type']'
# )

sch({                                                                                                                                                                                                                         
    'implementation': {
        'type': 'C',
        'c-value': None, }
})

Probably, the error best suited here is

expected str for dictionary value @ data['implementation']['c-value']

Maybe we could return the one with shortest errors list (https://github.com/alecthomas/voluptuous/blob/master/voluptuous/schema_builder.py#L536) among all alternatives.
Original discussion on BMO (reported by @djmitche)

[alecthomas]

What does "closest" mean? What's the heuristic that you'd use?

[alecthomas]

BTW, there's a msg=<str> keyword argument for most/all of the validators to allow for more useful error messages.

[ydidwania]

'Closest' - We could define it by the alternative which has the fewest errors, say the smallest list of errors.
This can be the default behavior, and the discrimiinant argument can be used to give the users mcontrol over how they define the 'closest alternative'.

Yes, but that argument can not reply with validation specific error messages like those provided in the comments to the code above, right?
Those just substitute whatever error message Any(..) returns. What I was looking at is in the code example above, the error message returned should be of validation with type-C implementation, rather than with type-A implementation (which is the first alternative), so that we know where to look.

The confusing error message is because it doesn't know which of the Any(..) alternatives you meant. It doesn't know about the field that discriminates between the alternatives (in the example written above, implementation).

[alecthomas]

Smallest list of errors sounds reasonable. I'd happily accept a PR. I'm not that enthused about a discriminant argument though.

[djmitche]

Before we jump into smallest-list-of-errors, can I make one more case for the discriminant? Here is one of several use-cases we have in Firefox, and you can see that worker is a kind of "union" of a few different schemas, where the implementation field discriminates between them. So from the user's perspective, the overall schema doesn't mean "any of these", it means "a specific one of these subschemas, as identified by ".

When the schema does not validate, none of those subschemas validate without errors, and Voluptuous must choose which subschema's errors to show. The current approach is subschema[0] which is clearly not optimal in many cases. Choosing the shortest list of errors seems a decent heuristic, but could still produce confusing error messages in some cases, when it conflicts with the user's intuition of which subschema was selected.

I recognize that maybe this is different from what Any currently means, especially if the discriminant is used in the validation process, and not just in the error-message-generation process. Other options:

• Only consult the discriminant function when validation of the Any schema has failed (in which case, perhaps shortest-list-of-errors could be the default discriminant); or
• Create a new type (Union perhaps?) that requires a discriminant which it uses in validation, but otherwise acts a lot like Any does now.

[alecthomas]

Makes sense. I have no problem with another type such as Union or Switch 👍

[djmitche]

I like Switch

[djmitche]

@ydidwania did you want to pick this project back up?

[ydidwania]

I did start with going through the relevant code, but got caught up with some work. I will be able to finish this up by the end of November. Will that be ok?

…

On Fri, Oct 26, 2018 at 7:44 PM Dustin J. Mitchell ***@***.***> wrote: @ydidwania <https://github.com/ydidwania> did you want to pick this project back up? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#360 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AVzU-VuuB9njrae3Hi-lmX6z0cExJiroks5uoxitgaJpZM4WieoJ> .

[djmitche]

Sounds good!