begin refactoring ValLineError collection

Author: davidhewitt

src/errors/line_error.rs

@@ -162,3 +162,108 @@ impl ToPyObject for InputValue {
         }
     }
 }
+
+pub struct LineErrorCollector {
+    errors: Vec<ValLineError>,
+    capacity: usize,
+}
+
+impl LineErrorCollector {
+    pub fn new() -> Self {
+        Self {
+            errors: Vec::new(),
+            capacity: 0,
+        }
+    }
+
+    pub fn with_capacity(capacity: usize) -> Self {
+        Self {
+            // as this is used on the error pathway, avoid allocating until the first error
+            errors: Vec::new(),
+            capacity,
+        }
+    }
+
+    pub fn ensure_empty(self) -> ValResult<()> {
+        if self.errors.is_empty() {
+            Ok(())
+        } else {
+            Err(ValError::LineErrors(self.errors))
+        }
+    }
+
+    pub fn push(&mut self, error: ValLineError) {
+        self.allocate_if_needed();
+        self.errors.push(error);
+    }
+
+    fn collect(&mut self, errors: Vec<ValLineError>) {
+        self.allocate_if_needed();
+        self.errors.extend(errors);
+    }
+
+    fn allocate_if_needed(&mut self) {
+        if self.errors.is_empty() && self.capacity > 0 {
+            self.errors.reserve(self.capacity);
+        }
+    }
+}
+
+/// Helper trait only implemented for `ValResult` to allow chaining of `collect_line_error`
+pub trait ValResultExt<T> {
+    /// If `self` is an `Err`, collect the line errors into the `collector` and return the error.
+    fn collect_line_errors(
+        self,
+        collector: &mut LineErrorCollector,
+        location: impl Into<LocItem>,
+    ) -> Result<Option<T>, ValidationControlFlow>;
+}
+
+impl<T> ValResultExt<T> for ValResult<T> {
+    #[inline]
+    fn collect_line_errors(
+        self,
+        collector: &mut LineErrorCollector,
+        location: impl Into<LocItem>,
+    ) -> Result<Option<T>, ValidationControlFlow> {
+        match self {
+            Ok(value) => Ok(Some(value)),
+            Err(ValError::LineErrors(line_errors)) => {
+                extend_collector(line_errors, collector, location.into());
+                Ok(None)
+            }
+            Err(ValError::InternalErr(err)) => Err(ValidationControlFlow::InternalErr(err)),
+            Err(ValError::Omit) => Err(ValidationControlFlow::Omit),
+            Err(ValError::UseDefault) => Err(ValidationControlFlow::UseDefault),
+        }
+    }
+}
+
+#[cold]
+fn extend_collector(line_errors: Vec<ValLineError>, collector: &mut LineErrorCollector, location: LocItem) {
+    collector.collect(
+        line_errors
+            .into_iter()
+            .map(|line_error| line_error.with_outer_location(location.clone()))
+            .collect(),
+    );
+}
+
+/// ValError, minus the LineErrors variant.
+///
+/// TODO: maybe rework ValError to contain this.
+pub enum ValidationControlFlow {
+    InternalErr(PyErr),
+    Omit,
+    UseDefault,
+}
+
+impl From<ValidationControlFlow> for ValError {
+    fn from(control_flow: ValidationControlFlow) -> Self {
+        match control_flow {
+            ValidationControlFlow::InternalErr(err) => ValError::InternalErr(err),
+            ValidationControlFlow::Omit => ValError::Omit,
+            ValidationControlFlow::UseDefault => ValError::UseDefault,
+        }
+    }
+}

src/errors/mod.rs

@@ -6,7 +6,10 @@ mod types;
 mod validation_exception;
 mod value_exception;
 
-pub use self::line_error::{InputValue, ToErrorValue, ValError, ValLineError, ValResult};
+pub use self::line_error::{
+    InputValue, LineErrorCollector, ToErrorValue, ValError, ValLineError, ValResult, ValResultExt,
+    ValidationControlFlow,
+};
 pub use self::location::LocItem;
 pub use self::types::{list_all_errors, ErrorType, ErrorTypeDefaults, Number};
 pub use self::validation_exception::ValidationError;

src/validators/dict.rs

@@ -3,7 +3,10 @@ use pyo3::prelude::*;
 use pyo3::types::PyDict;
 
 use crate::build_tools::is_strict;
-use crate::errors::{LocItem, ValError, ValLineError, ValResult};
+use crate::errors::LineErrorCollector;
+use crate::errors::ValResultExt;
+use crate::errors::ValidationControlFlow;
+use crate::errors::{LocItem, ValError, ValResult};
 use crate::input::BorrowInput;
 use crate::input::ConsumeIterator;
 use crate::input::{Input, ValidatedDict};
@@ -108,7 +111,7 @@ where
     type Output = ValResult<PyObject>;
     fn consume_iterator(self, iterator: impl Iterator<Item = ValResult<(Key, Value)>>) -> ValResult<PyObject> {
         let output = PyDict::new_bound(self.py);
-        let mut errors: Vec<ValLineError> = Vec::new();
+        let mut errors = LineErrorCollector::new();
 
         for item_result in iterator {
             let (key, value) = item_result?;
@@ -124,28 +127,24 @@ where
                 Err(ValError::Omit) => continue,
                 Err(err) => return Err(err),
             };
-            let output_value = match self.value_validator.validate(self.py, value.borrow_input(), self.state) {
-                Ok(value) => Some(value),
-                Err(ValError::LineErrors(line_errors)) => {
-                    for err in line_errors {
-                        errors.push(err.with_outer_location(key.clone()));
-                    }
-                    None
-                }
-                Err(ValError::Omit) => continue,
-                Err(err) => return Err(err),
+            let output_value = match self
+                .value_validator
+                .validate(self.py, value.borrow_input(), self.state)
+                .collect_line_errors(&mut errors, key)
+            {
+                Ok(output_value) => output_value,
+                Err(ValidationControlFlow::Omit) => continue,
+                Err(err) => return Err(err.into()),
             };
             if let (Some(key), Some(value)) = (output_key, output_value) {
                 output.set_item(key, value)?;
             }
         }
 
-        if errors.is_empty() {
-            let input = self.input;
-            length_check!(input, "Dictionary", self.min_length, self.max_length, output);
-            Ok(output.into())
-        } else {
-            Err(ValError::LineErrors(errors))
-        }
+        errors.ensure_empty()?;
+
+        let input = self.input;
+        length_check!(input, "Dictionary", self.min_length, self.max_length, output);
+        Ok(output.into())
     }
 }

src/validators/model_fields.rs

@@ -7,7 +7,9 @@ use ahash::AHashSet;
 
 use crate::build_tools::py_schema_err;
 use crate::build_tools::{is_strict, schema_or_config_same, ExtraBehavior};
+use crate::errors::LineErrorCollector;
 use crate::errors::LocItem;
+use crate::errors::ValResultExt;
 use crate::errors::{ErrorType, ErrorTypeDefaults, ValError, ValLineError, ValResult};
 use crate::input::ConsumeIterator;
 use crate::input::{BorrowInput, Input, ValidatedDict, ValidationMatch};
@@ -148,7 +150,8 @@ impl Validator for ModelFieldsValidator {
 
         let model_dict = PyDict::new_bound(py);
         let mut model_extra_dict_op: Option<Bound<PyDict>> = None;
-        let mut errors: Vec<ValLineError> = Vec::with_capacity(self.fields.len());
+        let mut errors = LineErrorCollector::with_capacity(self.fields.len());
+
         let mut fields_set_vec: Vec<Py<PyString>> = Vec::with_capacity(self.fields.len());
         let mut fields_set_count: usize = 0;
 
@@ -165,22 +168,20 @@ impl Validator for ModelFieldsValidator {
             let state = &mut state.rebind_extra(|extra| extra.data = Some(model_dict.clone()));
 
             for field in &self.fields {
-                let op_key_value = match dict.get_item(&field.lookup_key) {
-                    Ok(v) => v,
-                    Err(ValError::LineErrors(line_errors)) => {
-                        for err in line_errors {
-                            errors.push(err.with_outer_location(&field.name));
-                        }
-                        continue;
-                    }
-                    Err(err) => return Err(err),
+                let Some(op_key_value) = dict
+                    .get_item(&field.lookup_key)
+                    .collect_line_errors(&mut errors, &field.name)?
+                else {
+                    continue;
                 };
+
                 if let Some((lookup_path, value)) = op_key_value {
                     if let Some(ref mut used_keys) = used_keys {
                         // key is "used" whether or not validation passes, since we want to skip this key in
                         // extra logic either way
                         used_keys.insert(lookup_path.first_key());
                     }
+
                     match field.validator.validate(py, value.borrow_input(), state) {
                         Ok(value) => {
                             model_dict.set_item(&field.name_py, value)?;
@@ -231,7 +232,7 @@ impl Validator for ModelFieldsValidator {
             struct ValidateToModelExtra<'a, 's, 'py> {
                 py: Python<'py>,
                 used_keys: AHashSet<&'a str>,
-                errors: &'a mut Vec<ValLineError>,
+                errors: &'a mut LineErrorCollector,
                 fields_set_vec: &'a mut Vec<Py<PyString>>,
                 extra_behavior: ExtraBehavior,
                 extras_validator: Option<&'a CombinedValidator>,
@@ -287,17 +288,12 @@ impl Validator for ModelFieldsValidator {
                             ExtraBehavior::Allow => {
                                 let py_key = either_str.as_py_string(self.py, self.state.cache_str());
                                 if let Some(validator) = self.extras_validator {
-                                    match validator.validate(self.py, value, self.state) {
-                                        Ok(value) => {
-                                            model_extra_dict.set_item(&py_key, value)?;
-                                            self.fields_set_vec.push(py_key.into());
-                                        }
-                                        Err(ValError::LineErrors(line_errors)) => {
-                                            for err in line_errors {
-                                                self.errors.push(err.with_outer_location(raw_key.clone()));
-                                            }
-                                        }
-                                        Err(err) => return Err(err),
+                                    if let Some(value) = validator
+                                        .validate(self.py, value, self.state)
+                                        .collect_line_errors(self.errors, raw_key.clone())?
+                                    {
+                                        model_extra_dict.set_item(&py_key, value)?;
+                                        self.fields_set_vec.push(py_key.into());
                                     }
                                 } else {
                                     model_extra_dict.set_item(&py_key, value.to_object(self.py))?;
@@ -325,20 +321,18 @@ impl Validator for ModelFieldsValidator {
             }
         }
 
-        if !errors.is_empty() {
-            Err(ValError::LineErrors(errors))
-        } else {
-            let fields_set = PySet::new_bound(py, &fields_set_vec)?;
-            state.add_fields_set(fields_set_count);
+        errors.ensure_empty()?;
 
-            // if we have extra=allow, but we didn't create a dict because we were validating
-            // from attributes, set it now so __pydantic_extra__ is always a dict if extra=allow
-            if matches!(self.extra_behavior, ExtraBehavior::Allow) && model_extra_dict_op.is_none() {
-                model_extra_dict_op = Some(PyDict::new_bound(py));
-            };
+        let fields_set = PySet::new_bound(py, &fields_set_vec)?;
+        state.add_fields_set(fields_set_count);
 
-            Ok((model_dict, model_extra_dict_op, fields_set).to_object(py))
-        }
+        // if we have extra=allow, but we didn't create a dict because we were validating
+        // from attributes, set it now so __pydantic_extra__ is always a dict if extra=allow
+        if matches!(self.extra_behavior, ExtraBehavior::Allow) && model_extra_dict_op.is_none() {
+            model_extra_dict_op = Some(PyDict::new_bound(py));
+        };
+
+        Ok((model_dict, model_extra_dict_op, fields_set).to_object(py))
     }
 
     fn validate_assignment<'py>(