Fixed & wrote tests for inclusive range functions. (#3 #1)

Inclusive range is now stable so it doesn't need to be an extra feature, it still is here for backwards compatibility. (#16)
ChrisBuchholz · Jul 15, 2018 · baba62c · baba62c
1 parent 06cc629
commit baba62c
Show file tree

Hide file tree

Showing 5 changed files with 105 additions and 62 deletions.
diff --git a/src/lib.rs b/src/lib.rs
@@ -1,5 +1,3 @@
-#![cfg_attr(feature = "inclusive_range", feature(inclusive_range, inclusive_range_syntax))]
-
 extern crate serde;
 extern crate serde_json;
 #[macro_use]
@@ -52,9 +50,9 @@ pub trait Accord {
 ///     let password = "kfjsdkfjsdkfjfksjdfkdsfjs".to_string();
 ///     let age = 25;
 ///
-///     let _ = rules!(email, [length(5...64), contains("@"), contains(".")]);
-///     let _ = rules!(password, [length(8...64)]);
-///     let _ = rules!(age, [range(12...127)]);
+///     let _ = rules!(email, [length(5..=64), contains("@"), contains(".")]);
+///     let _ = rules!(password, [length(8..=64)]);
+///     let _ = rules!(age, [range(12..=127)]);
 /// }
 /// ```
 ///
@@ -93,9 +91,9 @@ pub trait Accord {
 ///     let age = 25;
 ///
 ///     let _ = rules!{
-///         "email" => email => [length(5...64), contains("@"), contains(".")],
-///         "password" => password => [length(8...64)],
-///         "age" => age => [range(12...127)]
+///         "email" => email => [length(5..=64), contains("@"), contains(".")],
+///         "password" => password => [length(8..=64)],
+///         "age" => age => [range(12..=127)]
 ///     };
 /// }
 /// ```

diff --git a/src/validators/length.rs b/src/validators/length.rs
@@ -75,56 +75,61 @@ pub fn length_if_present(mi: usize, ma: usize) -> Box<Fn(&Option<String>) -> ::V
     })
 }
 
+// TODO: this could be better rust-lang/rust#48111 & rust-lang/rust#32311
+
 #[cfg(feature = "inclusive_range")]
 /// Enforce that a string is minimum `mi` and maximum `ma` characters long.
 pub fn length(range: RangeInclusive<usize>) -> Box<Fn(&String) -> ::ValidatorResult> {
+    // do bounds checking here so we can panic early if needed
+    if range.end() <= range.start() {
+        panic!("Invalid range!"); // TODO: Bad way to do this.
+    }
+
     Box::new(move |s: &String| {
-        match range {
-            RangeInclusive::NonEmpty { ref start, ref end } => {
-                match (min(*start)(s), max(*end)(s)) {
-                    (Err(_), Err(_)) => {
-                        Err(::Invalid {
-                            msg: "Must not be less characters than %1 and not more than %2."
-                                .to_string(),
-                            args: vec![start.to_string(), end.to_string()],
-                            human_readable: format!("Must contain between {} and {} characters", mi, ma)
-                        })
-                    }
-                    (Err(e), _) => Err(e),
-                    (_, Err(e)) => Err(e),
-                    (_, _) => Ok(()),
-                }
+        let start = range.start();
+        let end = range.end();
+        match (min(*start)(s), max(*end)(s)) {
+            (Ok(_), Ok(_)) => Ok(()),
+            _ => {
+                Err(::Invalid {
+                    msg: "Must not be less characters than %1 and not more than %2."
+                        .to_string(),
+                    args: vec![start.to_string(), end.to_string()],
+                    human_readable: format!("Must contain between {} and {} characters", start, end)
+                })
             }
-            _ => panic!("range must be a RangeInclusive::NonEmpty"),
         }
     })
 }
 
+// TODO: this could be better rust-lang/rust#48111 & rust-lang/rust#32311
+
 #[cfg(feature = "inclusive_range")]
 /// Enforce that a string is minimum `mi` and maximum `ma` characters long if it is present. Always ok if not present.
 pub fn length_if_present(range: RangeInclusive<usize>) -> Box<Fn(&Option<String>) -> ::ValidatorResult> {
+    // do bounds checking here so we can panic early if needed
+    if range.end() <= range.start() {
+        panic!("Invalid range!"); 
+    }
+
     Box::new(move |s: &Option<String>| {
         if s.is_none() {
             return Ok(());
         }
         let s = s.as_ref().unwrap();
-        match range {
-            RangeInclusive::NonEmpty { ref start, ref end } => {
-                match (min(*start)(s), max(*end)(s)) {
-                    (Err(_), Err(_)) => {
-                        Err(::Invalid {
-                            msg: "Must not be less characters than %1 and not more than %2."
-                                .to_string(),
-                            args: vec![start.to_string(), end.to_string()],
-                            human_readable: format!("Must contain between {} and {} characters", start, end)
-                        })
-                    }
-                    (Err(e), _) => Err(e),
-                    (_, Err(e)) => Err(e),
-                    (_, _) => Ok(()),
-                }
+
+        let start = range.start();
+        let end = range.end();
+        match (min(*start)(s), max(*end)(s)) {
+            (Ok(_), Ok(_)) => Ok(()),
+            _ => {
+                Err(::Invalid {
+                    msg: "Must not be less characters than %1 and not more than %2."
+                        .to_string(),
+                    args: vec![start.to_string(), end.to_string()],
+                    human_readable: format!("Must contain between {} and {} characters", start, end)
+                })
             }
-            _ => panic!("range must be a RangeInclusive::NonEmpty"),
         }
     })
 }

diff --git a/src/validators/range.rs b/src/validators/range.rs
@@ -1,6 +1,5 @@
 use std::fmt::Display;
 
-
 #[cfg(feature = "inclusive_range")]
 use std::ops::RangeInclusive;
 
@@ -24,20 +23,65 @@ pub fn range<T: 'static + PartialOrd + Display + Clone>(a: T,
 #[cfg(feature = "inclusive_range")]
 pub fn range<T: 'static + PartialOrd + Display + Clone>(range: RangeInclusive<T>)
                                                         -> Box<Fn(&T) -> ::ValidatorResult> {
+    // do bounds checking here so we can panic early if needed
+    if range.end() <= range.start() {
+        panic!("Invalid range!"); // TODO: Bad way to do this.
+    }
+
     Box::new(move |s: &T| {
-        match range {
-            RangeInclusive::NonEmpty { ref start, ref end } => {
-                if *s >= *start && *s <= *end {
-                    Ok(())
-                } else {
-                    Err(::Invalid {
-                        msg: "Must be in the range %1..%2.".to_string(),
-                        args: vec![start.to_string(), end.to_string()],
-						human_readable: format!("Must be between {} and {}", start, end)
-                    })
-                }
-            } 
-            _ => panic!("range must be a RangeInclusive::NonEmpty"),
+        let start = range.start();
+        let end = range.end();
+
+        if *s >= *start && *s <= *end {
+            Ok(())
+        } else {
+            Err(::Invalid {
+                msg: "Must be in the range %1..%2.".to_string(),
+                args: vec![start.to_string(), end.to_string()],
+				human_readable: format!("Must be between {} and {}", start, end)
+            })
         }
     })
+}
+
+#[cfg(test)]
+#[cfg(not(feature = "inclusive_range"))]
+mod tests {
+    use super::*;
+
+    // range
+
+    #[test]
+    pub fn range_valid() {
+        assert!(range(1, 100)(&1).is_ok());
+        assert!(range(1, 100)(&50).is_ok());
+        assert!(range(1, 100)(&100).is_ok());
+    }
+
+    #[test]
+    pub fn range_invalid() {
+        assert!(range(1, 100)(&0).is_err());
+        assert!(range(1, 100)(&101).is_err());
+    }
+}
+
+#[cfg(test)]
+#[cfg(feature = "inclusive_range")]
+mod tests {
+    use super::*;
+
+    // range
+
+    #[test]
+    pub fn range_valid() {
+        assert!(range(1..=100)(&1).is_ok());
+        assert!(range(1..=100)(&50).is_ok());
+        assert!(range(1..=100)(&100).is_ok());
+    }
+
+    #[test]
+    pub fn range_invalid() {
+        assert!(range(1..=100)(&0).is_err());
+        assert!(range(1..=100)(&101).is_err());
+    }
 }
diff --git a/tests/account.rs b/tests/account.rs
@@ -1,5 +1,3 @@
-#![cfg_attr(feature = "inclusive_range", feature(inclusive_range_syntax))]
-
 #[macro_use]
 extern crate accord;
 
@@ -27,10 +25,10 @@ impl Accord for Account {
     #[cfg(feature = "inclusive_range")]
     fn validate(&self) -> AccordResult {
         rules!{
-            "name" => self.name => [length(1...64)],
+            "name" => self.name => [length(1..=64)],
 			"username" => self.username => [alphanumeric_dashes()],
-            "email" => self.email => [length(5...64), contains("@"), contains(".")],
-            "age" => self.age => [range(12...127)]
+            "email" => self.email => [length(5..=64), contains("@"), contains(".")],
+            "age" => self.age => [range(12..=127)]
         }
     }
 }

diff --git a/tests/credentials.rs b/tests/credentials.rs
@@ -1,5 +1,3 @@
-#![cfg_attr(feature = "inclusive_range", feature(inclusive_range_syntax))]
-
 #[macro_use]
 extern crate accord;
 
@@ -23,7 +21,7 @@ impl Accord for Credentials {
     #[cfg(feature = "inclusive_range")]
     fn validate(&self) -> AccordResult {
         rules!{
-            "email" => self.email => [length(5...64), contains("@"), contains(".")],
+            "email" => self.email => [length(5..=64), contains("@"), contains(".")],
             "password" => self.password => [not_contain_any(&["1234", "admin", "password"])]
         }
     }