Repository: tu6ge/valitron (language: rust)

Issue:
Repeat call rule method, it need remove duplicate rules
https://github.com/tu6ge/valitron/blob/bf1f20485f9c93697f46235ecf0c7fb266af942c/src/register/mod.rs#L574-L581


Discussion / hints:

Fix patch:
diff --git a/src/rule/mod.rs b/src/rule/mod.rs
index 472e1c0..ef69109 100644
--- a/src/rule/mod.rs
+++ b/src/rule/mod.rs
@@ -113,8 +113,22 @@ where
     where
         R2: Rule<(), Message = M>,
     {
+        let is_dup = {
+            if R::THE_NAME != R2::THE_NAME {
+                false
+            } else {
+                match R::THE_NAME {
+                    "custom" => false,
+                    _ => true,
+                }
+            }
+        };
         RuleList {
-            list: vec![ErasedRule::new(self), ErasedRule::new(other)],
+            list: if is_dup {
+                vec![ErasedRule::new(self)]
+            } else {
+                vec![ErasedRule::<M>::new(self), ErasedRule::new(other)]
+            },
             ..Default::default()
         }
     }
@@ -157,12 +171,43 @@ impl<M> Clone for RuleList<M> {
 }
 
 impl<M> RuleList<M> {
+    pub fn remove_duplicate(&mut self, other: &ErasedRule<M>) {
+        let name = other.name();
+
+        let duplicate_rules: Vec<usize> = self
+            .list
+            .iter()
+            .enumerate()
+            .filter(|(_index, exist_rule)| {
+                if exist_rule.name() != name {
+                    return false;
+                }
+                match name {
+                    "custom" => false,
+                    _ => true,
+                }
+            })
+            .map(|(index, _)| index)
+            .rev()
+            .collect();
+
+        for index in duplicate_rules {
+            // Use `swap_remove` to get better performence because we don't
+            // mind the order of rule list. If the order should be kept in
+            // the future, please use `remove` instead of `swap_remove`.
+            self.list.swap_remove(index);
+        }
+    }
+
     pub fn and<R>(mut self, other: R) -> Self
     where
         R: Rule<(), Message = M>,
         M: 'static,
     {
-        self.list.push(ErasedRule::new(other));
+        let other = ErasedRule::new(other);
+        self.remove_duplicate(&other);
+
+        self.list.push(other);
         self
     }
 
@@ -204,6 +249,10 @@ impl<M> RuleList<M> {
     }
 
     pub(crate) fn merge(&mut self, other: &mut RuleList<M>) {
+        for new_rule in &other.list {
+            self.remove_duplicate(new_rule);
+        }
+
         self.list.append(&mut other.list);
         self.is_bail = self.is_bail || other.is_bail;
     }


Test patch:
diff --git a/src/register/tests.rs b/src/register/tests.rs
index 1643374..6b8d20c 100644
--- a/src/register/tests.rs
+++ b/src/register/tests.rs
@@ -14,7 +14,7 @@ fn test_validator_error_serialize() {
 
 #[cfg(feature = "full")]
 #[test]
-fn repect_insert_rules() {
+fn repeat_insert_rules() {
     use crate::{
         available::{Range, Required, Trim},
         RuleExt,
@@ -25,7 +25,7 @@ fn repect_insert_rules() {
         .rule("foo", Range::new(1..2));
 
     let vec = validate.rules.get(&FieldNames::new("foo".into())).unwrap();
-    assert!(vec.len() == 2);
+    assert_eq!(vec.len(), 2);
     assert!(vec.is_bail() == false);
 
     let validate = Validator::new()
@@ -33,7 +33,7 @@ fn repect_insert_rules() {
         .rule("foo", Range::new(1..2));
 
     let vec = validate.rules.get(&FieldNames::new("foo".into())).unwrap();
-    assert!(vec.len() == 3);
+    assert_eq!(vec.len(), 3);
     assert!(vec.is_bail() == true);
 
     let validate = Validator::new()
@@ -41,16 +41,37 @@ fn repect_insert_rules() {
         .rule("foo", Range::new(1..2).and(Trim).bail());
 
     let vec = validate.rules.get(&FieldNames::new("foo".into())).unwrap();
-    assert!(vec.len() == 3);
+    assert_eq!(vec.len(), 3);
     assert!(vec.is_bail() == true);
 
     let validate = Validator::new()
         .rule("foo", Required.and(Trim).bail())
         .rule("foo", Range::new(1..2).and(Trim).bail());
 
-    // TODO need remove duplicates
     let vec = validate.rules.get(&FieldNames::new("foo".into())).unwrap();
-    assert!(vec.len() == 4);
+    assert_eq!(vec.len(), 3);
+    assert!(vec.is_bail() == true);
+
+    let validate = Validator::new()
+        .rule("foo", Required.and(Trim).and(Required).bail())
+        .rule("foo", Range::new(1..2).and(Trim).and(Required).bail());
+
+    let vec = validate.rules.get(&FieldNames::new("foo".into())).unwrap();
+    assert_eq!(vec.len(), 3);
+    assert!(vec.is_bail() == true);
+
+    let validate = Validator::new()
+        .rule("foo", Required.and(Trim).and(Required).bail())
+        .rule("bar", Required.and(Trim).and(Required).bail())
+        .rule("foo", Range::new(1..2).and(Trim).and(Required).bail())
+        .rule("bar", Range::new(1..2).and(Trim).and(Required).bail());
+
+    let vec = validate.rules.get(&FieldNames::new("foo".into())).unwrap();
+    assert_eq!(vec.len(), 3);
+    assert!(vec.is_bail() == true);
+
+    let vec = validate.rules.get(&FieldNames::new("bar".into())).unwrap();
+    assert_eq!(vec.len(), 3);
     assert!(vec.is_bail() == true);
 }