Skip to content

Commit

Permalink
improve quality of too short / too long error messages (#990)
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt authored Sep 26, 2023
1 parent e610984 commit 8435153
Show file tree
Hide file tree
Showing 10 changed files with 56 additions and 76 deletions.
7 changes: 4 additions & 3 deletions src/errors/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ error_types! {
TooLong {
field_type: {ctx_type: String, ctx_fn: field_from_context},
max_length: {ctx_type: usize, ctx_fn: field_from_context},
actual_length: {ctx_type: usize, ctx_fn: field_from_context},
actual_length: {ctx_type: Option<usize>, ctx_fn: field_from_context},
},
// ---------------------
// generic collection and iteration errors
Expand Down Expand Up @@ -630,7 +630,7 @@ impl ErrorType {
..
} => {
let expected_plural = plural_s(*min_length);
to_string_render!(tmpl, field_type, min_length, actual_length, expected_plural)
to_string_render!(tmpl, field_type, min_length, actual_length, expected_plural,)
}
Self::TooLong {
field_type,
Expand All @@ -639,7 +639,8 @@ impl ErrorType {
..
} => {
let expected_plural = plural_s(*max_length);
to_string_render!(tmpl, field_type, max_length, actual_length, expected_plural)
let actual_length = actual_length.map_or(Cow::Borrowed("more"), |v| Cow::Owned(v.to_string()));
to_string_render!(tmpl, field_type, max_length, actual_length, expected_plural,)
}
Self::IterationError { error, .. } => render!(tmpl, error),
Self::StringTooShort { min_length, .. } => to_string_render!(tmpl, min_length),
Expand Down
72 changes: 25 additions & 47 deletions src/input/return_enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,54 +106,33 @@ struct MaxLengthCheck<'a, INPUT> {
max_length: Option<usize>,
field_type: &'a str,
input: &'a INPUT,
known_input_length: usize,
actual_length: Option<usize>,
}

impl<'a, INPUT: Input<'a>> MaxLengthCheck<'a, INPUT> {
fn new(max_length: Option<usize>, field_type: &'a str, input: &'a INPUT, known_input_length: usize) -> Self {
fn new(max_length: Option<usize>, field_type: &'a str, input: &'a INPUT, actual_length: Option<usize>) -> Self {
Self {
current_length: 0,
max_length,
field_type,
input,
known_input_length,
actual_length,
}
}

fn incr(&mut self) -> ValResult<'a, ()> {
match self.max_length {
Some(max_length) => {
self.current_length += 1;
if self.current_length > max_length {
let biggest_length = if self.known_input_length > self.current_length {
self.known_input_length
} else {
self.current_length
};
return Err(ValError::new(
ErrorType::TooLong {
field_type: self.field_type.to_string(),
max_length,
actual_length: biggest_length,
context: None,
},
self.input,
));
}
}
None => {
self.current_length += 1;
if self.current_length > self.known_input_length {
return Err(ValError::new(
ErrorType::TooLong {
field_type: self.field_type.to_string(),
max_length: self.known_input_length,
actual_length: self.current_length,
context: None,
},
self.input,
));
}
if let Some(max_length) = self.max_length {
self.current_length += 1;
if self.current_length > max_length {
return Err(ValError::new(
ErrorType::TooLong {
field_type: self.field_type.to_string(),
max_length,
actual_length: self.actual_length,
context: None,
},
self.input,
));
}
}
Ok(())
Expand Down Expand Up @@ -255,13 +234,15 @@ fn validate_iter_to_set<'a, 's>(
Ok(item) => {
set.build_add(item)?;
if let Some(max_length) = max_length {
let actual_length = set.build_len();
if actual_length > max_length {
if set.build_len() > max_length {
return Err(ValError::new(
ErrorType::TooLong {
field_type: field_type.to_string(),
max_length,
actual_length,
// The logic here is that it doesn't matter how many elements the
// input actually had; all we know is it had more than the allowed
// number of deduplicated elements.
actual_length: None,
context: None,
},
input,
Expand Down Expand Up @@ -335,10 +316,9 @@ impl<'a> GenericIterable<'a> {
validator: &'s CombinedValidator,
state: &mut ValidationState,
) -> ValResult<'a, Vec<PyObject>> {
let capacity = self
.generic_len()
.unwrap_or_else(|| max_length.unwrap_or(DEFAULT_CAPACITY));
let max_length_check = MaxLengthCheck::new(max_length, field_type, input, capacity);
let actual_length = self.generic_len();
let capacity = actual_length.unwrap_or(DEFAULT_CAPACITY);
let max_length_check = MaxLengthCheck::new(max_length, field_type, input, actual_length);

macro_rules! validate {
($iter:expr) => {
Expand Down Expand Up @@ -394,10 +374,8 @@ impl<'a> GenericIterable<'a> {
field_type: &'static str,
max_length: Option<usize>,
) -> ValResult<'a, Vec<PyObject>> {
let capacity = self
.generic_len()
.unwrap_or_else(|| max_length.unwrap_or(DEFAULT_CAPACITY));
let max_length_check = MaxLengthCheck::new(max_length, field_type, input, capacity);
let actual_length = self.generic_len();
let max_length_check = MaxLengthCheck::new(max_length, field_type, input, actual_length);

match self {
GenericIterable::List(collection) => {
Expand Down
2 changes: 1 addition & 1 deletion src/validators/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ impl ValidatorIterator {
ErrorType::TooLong {
field_type: "Generator".to_string(),
max_length,
actual_length: index + 1,
actual_length: None,
context: None,
},
$iter.input_as_error_value(py),
Expand Down
2 changes: 1 addition & 1 deletion src/validators/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ macro_rules! length_check {
crate::errors::ErrorType::TooLong {
field_type: $field_type.to_string(),
max_length,
actual_length,
actual_length: Some(actual_length),
context: None,
},
$input,
Expand Down
20 changes: 11 additions & 9 deletions src/validators/tuple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,7 @@ fn validate_tuple_positional<'s, 'data, T: Iterator<Item = PyResult<&'data I>>,
extras_validator: &Option<Box<CombinedValidator>>,
items_validators: &[CombinedValidator],
collection_iter: &mut T,
collection_len: Option<usize>,
expected_length: usize,
actual_length: Option<usize>,
) -> ValResult<'data, ()> {
for (index, validator) in items_validators.iter().enumerate() {
match collection_iter.next() {
Expand Down Expand Up @@ -167,7 +166,7 @@ fn validate_tuple_positional<'s, 'data, T: Iterator<Item = PyResult<&'data I>>,
errors.extend(
line_errors
.into_iter()
.map(|err| err.with_outer_location((index + expected_length).into())),
.map(|err| err.with_outer_location((index + items_validators.len()).into())),
);
}
Err(ValError::Omit) => (),
Expand All @@ -177,8 +176,8 @@ fn validate_tuple_positional<'s, 'data, T: Iterator<Item = PyResult<&'data I>>,
errors.push(ValLineError::new(
ErrorType::TooLong {
field_type: "Tuple".to_string(),
max_length: expected_length,
actual_length: collection_len.unwrap_or(index),
max_length: items_validators.len(),
actual_length,
context: None,
},
input,
Expand All @@ -204,8 +203,12 @@ impl Validator for TuplePositionalValidator {
state: &mut ValidationState,
) -> ValResult<'data, PyObject> {
let collection = input.validate_tuple(state.strict_or(self.strict))?;
let expected_length = self.items_validators.len();
let collection_len = collection.generic_len();
let actual_length = collection.generic_len();
let expected_length = if self.extras_validator.is_some() {
actual_length.unwrap_or(self.items_validators.len())
} else {
self.items_validators.len()
};

let mut output: Vec<PyObject> = Vec::with_capacity(expected_length);
let mut errors: Vec<ValLineError> = Vec::new();
Expand All @@ -221,8 +224,7 @@ impl Validator for TuplePositionalValidator {
&self.extras_validator,
&self.items_validators,
&mut $collection_iter,
collection_len,
expected_length,
actual_length,
)?
}};
}
Expand Down
6 changes: 3 additions & 3 deletions tests/validators/test_frozenset.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,20 +148,20 @@ def generate_repeats():
(
{'max_length': 3},
{1, 2, 3, 4},
Err('Frozenset should have at most 3 items after validation, not 4 [type=too_long,'),
Err('Frozenset should have at most 3 items after validation, not more [type=too_long,'),
),
(
{'items_schema': {'type': 'int'}, 'max_length': 3},
{1, 2, 3, 4},
Err('Frozenset should have at most 3 items after validation, not 4 [type=too_long,'),
Err('Frozenset should have at most 3 items after validation, not more [type=too_long,'),
),
# length check after set creation
({'max_length': 3}, [1, 1, 2, 2, 3, 3], {1, 2, 3}),
({'max_length': 3}, generate_repeats(), {1, 2, 3}),
(
{'max_length': 3},
infinite_generator(),
Err('Frozenset should have at most 3 items after validation, not 4 [type=too_long,'),
Err('Frozenset should have at most 3 items after validation, not more [type=too_long,'),
),
],
)
Expand Down
8 changes: 4 additions & 4 deletions tests/validators/test_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,9 @@ def test_too_long(py_and_json: PyAndJson):
{
'type': 'too_long',
'loc': (),
'msg': 'Generator should have at most 2 items after validation, not 3',
'msg': 'Generator should have at most 2 items after validation, not more',
'input': [1, 2, 3],
'ctx': {'field_type': 'Generator', 'max_length': 2, 'actual_length': 3},
'ctx': {'field_type': 'Generator', 'max_length': 2, 'actual_length': None},
}
]

Expand Down Expand Up @@ -167,8 +167,8 @@ def test_generator_too_long():
'type': 'too_long',
'loc': (),
'input': HasRepr(IsStr(regex='<generator object gen at .+>')),
'msg': 'Generator should have at most 2 items after validation, not 3',
'ctx': {'field_type': 'Generator', 'max_length': 2, 'actual_length': 3},
'msg': 'Generator should have at most 2 items after validation, not more',
'ctx': {'field_type': 'Generator', 'max_length': 2, 'actual_length': None},
}
]

Expand Down
3 changes: 1 addition & 2 deletions tests/validators/test_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,14 +160,13 @@ def test_list_error(input_value, index):
(
{'max_length': 44},
infinite_generator(),
Err('List should have at most 44 items after validation, not 45 [type=too_long,'),
Err('List should have at most 44 items after validation, not more [type=too_long,'),
),
(
{'max_length': 4, 'items_schema': {'type': 'int'}},
[0, 1, 2, 3, 4, 5, 6, 7, 8],
Err('List should have at most 4 items after validation, not 9 [type=too_long,'),
),
({}, infinite_generator(), Err('List should have at most 10 items after validation, not 11 [type=too_long,')),
],
)
def test_list_length_constraints(kwargs: Dict[str, Any], input_value, expected):
Expand Down
6 changes: 3 additions & 3 deletions tests/validators/test_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,12 @@ def generate_repeats():
(
{'max_length': 3},
{1, 2, 3, 4},
Err('Set should have at most 3 items after validation, not 4 [type=too_long,'),
Err('Set should have at most 3 items after validation, not more [type=too_long,'),
),
(
{'max_length': 3},
[1, 2, 3, 4],
Err('Set should have at most 3 items after validation, not 4 [type=too_long,'),
Err('Set should have at most 3 items after validation, not more [type=too_long,'),
),
({'max_length': 3, 'items_schema': {'type': 'int'}}, {1, 2, 3, 4}, Err('type=too_long,')),
({'max_length': 3, 'items_schema': {'type': 'int'}}, [1, 2, 3, 4], Err('type=too_long,')),
Expand All @@ -141,7 +141,7 @@ def generate_repeats():
(
{'max_length': 3},
infinite_generator(),
Err('Set should have at most 3 items after validation, not 4 [type=too_long,'),
Err('Set should have at most 3 items after validation, not more [type=too_long,'),
),
],
ids=repr,
Expand Down
6 changes: 3 additions & 3 deletions tests/validators/test_tuple.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,13 @@ def test_tuple_strict_fails_without_tuple(wrong_coll_type: Type[Any], mode, item
),
(
{'max_length': 3},
[1, 2, 3, 4],
Err('Tuple should have at most 3 items after validation, not 4 [type=too_long,'),
[1, 2, 3, 4, 5],
Err('Tuple should have at most 3 items after validation, not 5 [type=too_long,'),
),
(
{'max_length': 3},
infinite_generator(),
Err('Tuple should have at most 3 items after validation, not 4 [type=too_long,'),
Err('Tuple should have at most 3 items after validation, not more [type=too_long,'),
),
],
ids=repr,
Expand Down

0 comments on commit 8435153

Please sign in to comment.