fix by only running nested if outer is not run

CHANGELOG.md

@@ -1,5 +1,10 @@
 ## Changelog
 
+## 0.19.1 (unreleased)
+
+- Bug fix for nested issue with custom only running nested if outer passes
+
+
 ## 0.19.0 (2024/11/03)
 
 - Swap to using proc-macro-error-2 instead of proc-macro-error for Syn

validator/src/types.rs

@@ -181,9 +181,10 @@ impl ValidationErrors {
     fn add_nested(&mut self, field: &'static str, errors: ValidationErrorsKind) {
         if let Vacant(entry) = self.0.entry(field) {
             entry.insert(errors);
-        } else {
-            panic!("Attempt to replace non-empty ValidationErrors entry");
         }
+        // Else case here is simply do nothing. Pretty sure this is caught earlier to not run but since
+        // If we get here, just not adding it does the same thing (although waste extra compute)
+        // probably better to just ignore here.
     }
 
     #[must_use]

validator_derive/src/tokens/nested.rs

@@ -5,6 +5,8 @@ pub fn nested_tokens(
     field_name_str: &str,
 ) -> proc_macro2::TokenStream {
     quote! {
-        errors.merge_self(#field_name_str, (&#field_name).validate());
+        if let std::collections::hash_map::Entry::Vacant(entry) = errors.0.entry(#field_name_str) {
+            errors.merge_self(#field_name_str, (&#field_name).validate());
+        }
     }
 }

validator_derive_tests/tests/nested.rs

@@ -1,6 +1,7 @@
 use serde::Serialize;
 use std::{
     borrow::Cow,
+    cmp::Ordering,
     collections::{HashMap, HashSet},
 };
 use validator::{
@@ -847,58 +848,6 @@ fn test_field_validations_take_priority_over_nested_validations() {
     }
 }
 
-#[test]
-#[should_panic(expected = "Attempt to replace non-empty ValidationErrors entry")]
-#[allow(unused)]
-fn test_field_validation_errors_replaced_with_nested_validations_fails() {
-    #[derive(Debug)]
-    struct ParentWithOverridingStructValidations {
-        child: Vec<Child>,
-    }
-
-    #[derive(Debug, Validate, Serialize)]
-    struct Child {
-        #[validate(length(min = 1))]
-        value: String,
-    }
-
-    impl Validate for ParentWithOverridingStructValidations {
-        // Evaluating structs after fields validations have discovered errors should fail because
-        // field validations are expected to take priority over nested struct validations
-        #[allow(unused_mut)]
-        fn validate(&self) -> Result<(), ValidationErrors> {
-            // First validate the length of the vector:
-            let mut errors = ValidationErrors::new();
-            if !self.child.validate_length(Some(2u64), None, None) {
-                let mut err = ValidationError::new("length");
-                err.add_param(Cow::from("min"), &2u64);
-                err.add_param(Cow::from("value"), &&self.child);
-                errors.add("child", err);
-            }
-
-            // Then validate the nested vector of structs without checking for existing field errors:
-            let mut result = if errors.is_empty() { Ok(()) } else { Err(errors) };
-            {
-                let results: Vec<_> = self
-                    .child
-                    .iter()
-                    .map(|child| {
-                        let mut result = Ok(());
-                        result = ValidationErrors::merge(result, "child", child.validate());
-                        result
-                    })
-                    .collect();
-                result = ValidationErrors::merge_all(result, "child", results);
-            }
-            result
-        }
-    }
-
-    let instance =
-        ParentWithOverridingStructValidations { child: vec![Child { value: String::new() }] };
-    instance.validate();
-}
-
 #[test]
 #[should_panic(
     expected = "Attempt to add field validation to a non-Field ValidationErrorsKind instance"
@@ -962,3 +911,50 @@ where
     let errors = errors.clone();
     f(errors.errors().clone());
 }
+
+#[test]
+fn fails_nested_validation_multiple_members_and_custom_validations() {
+    #[derive(Validate)]
+    struct Root {
+        #[validate(length(min = 5, max = 10))]
+        value: String,
+        #[validate(nested, custom(function = "all_value1s_are_unique"))]
+        a: Vec<A>,
+    }
+
+    #[derive(Serialize, Validate)]
+    struct A {
+        #[validate(length(min = 5, max = 10))]
+        value1: String,
+        #[validate(length(min = 5, max = 10))]
+        value2: String,
+    }
+
+    // Top-level custom validation
+    fn all_value1s_are_unique(items: &[A]) -> Result<(), ValidationError> {
+        let unique_value1s: HashSet<String> =
+            HashSet::from_iter(items.iter().map(|a| a.value1.to_string()));
+        match unique_value1s.len().cmp(&items.len()) {
+            Ordering::Equal => Ok(()),
+            _ => Err(ValidationError::new("not all value1s are unique")),
+        }
+    }
+
+    let root = Root {
+        value: "valid".to_string(),
+        // "a" should be invalid because multiple items have the same "value1"
+        // but also "a[1].value2" should be invalid because it is too long.
+        a: vec![
+            A { value1: "non unique".to_string(), value2: "a value 2".to_string() },
+            A { value1: "non unique".to_string(), value2: "b value 2 too long".to_string() },
+            A { value1: "unique-ish".to_string(), value2: "c value 2".to_string() },
+        ],
+    };
+
+    // This is currently panicking with "Attempt to replace non-empty ValidationErrors entry"
+    let result = root.validate().unwrap();
+
+    // Not sure what the correct full assertion should be,
+    // but the panic isn't letting me get any further.
+    //assert_ne!(result, Ok(()))
+}