Fix: In _compile_sequence's validate_sequence (schema_builder.py), the loop... - #552
Open
M001N wants to merge 2 commits into
Open
Fix: In _compile_sequence's validate_sequence (schema_builder.py), the loop...#552M001N wants to merge 2 commits into
M001N wants to merge 2 commits into
Conversation
…homas#142) validate_sequence() tries each schema alternative for a list item and early-exits (re-raising immediately) whenever an alternative's error path is deeper than the item's own path -- on the assumption that a deeper path means the alternative's top-level shape matched and only a nested value is wrong, so surfacing that error immediately beats silently trying the remaining alternatives. A dict-schema alternative breaks that assumption: validate_mapping always reports 'extra keys not allowed' and 'required key not provided' errors one level deeper than the path it was given (it names the specific offending key), even when the alternative's keys have nothing to do with the data's keys at all. So for any dict alternative in a list, the heuristic fired on the very first non-matching alternative and aborted the try-next-alternative loop. Add _is_key_shape_mismatch() to recognize exactly those two error kinds at that one-level-deeper depth as shape mismatches, so the search keeps going to the next alternative instead of giving up. Any other error at that depth (e.g. a wrong-typed value for a key that genuinely matched) still aborts the search immediately, preserving the heuristic's original intent.
…tches too The list-of-dict-alternatives early-exit heuristic in validate_sequence previously only recognized plain 'extra keys not allowed' Invalid and RequiredFieldInvalid as dict-shape mismatches, via message/class inspection. This missed the case where a dict alternative's key is matched via a key validator/schema (In(...), Match(...), etc.) instead of a literal key -- such rejections surface as the key validator's own Invalid subclass at the same depth, and the old message/class-based check couldn't recognize them, so the early-exit fired incorrectly and aborted the search over remaining alternatives. Now validate_mapping tags every error that represents 'this candidate key didn't match the data's key at all' (a key validator rejecting the key, 'extra keys not allowed' for keys with no candidates, or a missing required key) with a _key_shape_mismatch marker at the point it's constructed, and _is_key_shape_mismatch simply checks for that marker. This covers all key-shape-mismatch sources generically instead of special-casing specific error messages/classes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Instead of trying to reverse-engineer 'is this a key-shape-mismatch error' from an error's class/message downstream, validate_mapping now tags every error it constructs that genuinely represents 'this candidate key didn't match the data's key at all' with a
_key_shape_mismatch = Truemarker attribute, at each of the three points such errors are created: (1) when a key candidate's ckey() call rejects the key (covers key validators like In/Match generically, not just literal keys), (2) when no candidate matched and 'extra keys not allowed' is raised, (3) when a required key is missing entirely. _is_key_shape_mismatch in schema_builder.py now just checks for that marker plus depth, so it generically covers every source of 'key didn't match' rather than special-casing specific error text/classes. Value-validation errors (a key that WAS matched but whose value is wrong) are never tagged, so the heuristic still correctly aborts the alternative search for genuine value-type errors like Case 1 in the issue.Problem
alecthomas/voluptuous issue reference: #142
Root Cause
In _compile_sequence's validate_sequence (schema_builder.py), the loop over schema alternatives for a list item aborts early via
if len(e.path) > len(index_path): raisewhenever an alternative's error path is one level deeper than the item's own path -- intended to mean 'top-level shape matched, nested detail is wrong, surface it immediately'. But validate_mapping (the dict validator) always reports key-related failures ('extra keys not allowed', missing required key, or a key validator/schema like In()/Match() rejecting the key) exactly one level deeper than the path it was given, even when the alternative's shape doesn't match the data at all. The prior attempt patched this by recognizing two specific error shapes (plain 'extra keys not allowed' Invalid and RequiredFieldInvalid) via message/class inspection in a new _is_key_shape_mismatch helper, but this missed the case where a dict alternative uses a key validator (In, Match, etc.) instead of a literal key: that rejection surfaces as the key validator's own differently-classed, differently-worded Invalid (e.g. InInvalid) at the same depth, which the message/class-based check couldn't recognize as a shape mismatch.Testing
PASS - full suite: 183 passed (including tests.md doctests); test_fix_142_list_of_dict_alternatives extended to cover: Case 0 (issue's reported bug) success, Case 1 (genuine value-type error) still correctly raised, 3-alternative case, Required-key shape mismatch, and the new key-validator cases (In and Match, both success and a real value-error-not-swallowed case). The 2 pre-existing --doctest-modules failures (Schema._compile_dict, Remove) were confirmed identical on both the base commit and this change via git stash comparison, so they are unrelated pre-existing issues, not a regression.
Related Issue
#142