Repository: Keats/tera
Pull request #63 (2016-10-04 21:18:25)

Add tests and remove using AST in tests fn

Keats: @SergioBenitez remove the AST from the tests fn, error reporting still kinda suck as it doesn't tell you files/lines but that's the same issue with filters and I need to come up with a better way for both
SergioBenitez: Looks pretty good overall! Left some comments here and there.
Keats: Updated the PR, thanks for the review @SergioBenitez !

Patch:
diff --git a/README.md b/README.md
index e73cbb8..9641177 100644
--- a/README.md
+++ b/README.md
@@ -215,12 +215,34 @@ is defined, you would write:
 ... don't use user here ...
 {% end %}
 ```
+Note that testers allow expressions, so the following is a valid test as well:
+
+```
+{% if my_number + 1 is odd %}
+ blabla
+{% endif %}
+```
 
 Here are the currently implemented testers:
 
 #### defined
 Returns true if the given variable is defined.
 
+#### undefined
+Returns true if the given variable is undefined.
+
+#### odd
+Returns true if the given variable is an odd number.
+
+#### even
+Returns true if the given variable is an even number.
+
+#### string
+Returns true if the given variable is a string.
+
+#### number
+Returns true if the given variable is a number.
+
 ### Filters
 Variables can be modified by filters. 
 Filters are separated from the variable by a pipe symbol (`|`) and may have named arguments in parentheses. Multiple filters can be chained: the output of one filter is applied to the next.
diff --git a/src/context.rs b/src/context.rs
index e62e84d..38ffa38 100644
--- a/src/context.rs
+++ b/src/context.rs
@@ -97,3 +97,9 @@ impl ValueTruthy for Value {
         }
     }
 }
+
+
+/// Converts a dotted path to a json pointer one
+pub fn get_json_pointer(key: &str) -> String {
+    ["/", &key.replace(".", "/")].join("")
+}
diff --git a/src/render.rs b/src/render.rs
index 24156d6..aeb9167 100644
--- a/src/render.rs
+++ b/src/render.rs
@@ -2,7 +2,7 @@ use std::collections::LinkedList;
 
 use serde_json::value::{Value, to_value};
 
-use context::{Context, ValueRender, ValueNumber, ValueTruthy};
+use context::{Context, ValueRender, ValueNumber, ValueTruthy, get_json_pointer};
 use template::Template;
 use errors::TeraResult;
 use errors::TeraError::*;
@@ -65,19 +65,12 @@ impl<'a> Renderer<'a> {
         }
     }
 
-    // Converts a dotted path to a json pointer one
-    // TODO: should belong on the context itself eventually once
-    // we move from json
-    fn get_json_pointer(&self, key: &str) -> String {
-        ["/", &key.replace(".", "/")].join("")
-    }
-
     // Lookup a variable name from the context and takes into
     // account for loops variables
     fn lookup_variable(&self, key: &str) -> TeraResult<Value> {
         // Look in the plain context if we aren't in a for loop
         if self.for_loops.is_empty() {
-            return self.context.pointer(&self.get_json_pointer(key)).cloned()
+            return self.context.pointer(&get_json_pointer(key)).cloned()
                 .ok_or_else(|| FieldNotFound(key.to_string()));
         }
 
@@ -91,7 +84,9 @@ impl<'a> Renderer<'a> {
                 // might be a struct or some nested structure
                 if key.contains('.') {
                     let new_key = key.split_terminator('.').skip(1).collect::<Vec<&str>>().join(".");
-                    return value.pointer(&self.get_json_pointer(&new_key)).cloned().ok_or_else(|| FieldNotFound(key.to_string()));
+                    return value.pointer(&get_json_pointer(&new_key))
+                        .cloned()
+                        .ok_or_else(|| FieldNotFound(key.to_string()));
                 } else {
                     return Ok(value.clone());
                 }
@@ -107,9 +102,7 @@ impl<'a> Renderer<'a> {
         }
 
         // dummy statement to satisfy the compiler
-        // TODO: make it so that's not needed
-        self.context.pointer(&self.get_json_pointer(key)).cloned()
-            .ok_or_else(|| FieldNotFound(key.to_string()))
+        unreachable!();
     }
 
     // Gets an identifier and return its json value
@@ -207,17 +200,11 @@ impl<'a> Renderer<'a> {
             },
             Test { expression, name, params } => {
                 let tester = try!(self.tera.get_tester(&name));
-                let context = match self.context.as_object() {
-                    Some(map) => map,
-                    None => return Err(Internal("context is not an object.".into()))
-                };
-
-                let mut value_params = LinkedList::new();
+                let mut value_params = vec![];
                 for param in params {
-                    value_params.push_back(try!(self.eval_expression(param)));
+                    value_params.push(try!(self.eval_expression(param)));
                 }
-
-                tester(context, &expression, value_params)
+                tester(&name, self.eval_expression(*expression).ok(), value_params)
             },
             Logic { lhs, rhs, operator } => {
                 match operator.as_str() {
diff --git a/src/tera.rs b/src/tera.rs
index 199470d..437f559 100644
--- a/src/tera.rs
+++ b/src/tera.rs
@@ -121,6 +121,11 @@ impl Tera {
 
     fn register_tera_testers(&mut self) {
         self.register_tester("defined", testers::defined);
+        self.register_tester("undefined", testers::undefined);
+        self.register_tester("odd", testers::odd);
+        self.register_tester("even", testers::even);
+        self.register_tester("string", testers::string);
+        self.register_tester("number", testers::number);
     }
 }
 


Test patch:
diff --git a/src/testers.rs b/src/testers.rs
index 9618fdf..253f6e5 100644
--- a/src/testers.rs
+++ b/src/testers.rs
@@ -1,30 +1,128 @@
 use errors::{TeraResult, TeraError};
-use serde_json::value::{Map, Value};
-use parser::Node;
+use serde_json::value::{Value};
+use context::ValueNumber;
 
-use std::collections::LinkedList;
 
-// TODO: Don't expose the AST to tester functions.
-pub type TesterFn = fn(context: &Map<String, Value>,
-                       value: &Node,
-                       params: LinkedList<Value>)
-                       -> TeraResult<bool>;
 
-/// Returns true if `value` is defined in the given context. Otherwise, returns
-/// false.
-pub fn defined(context: &Map<String, Value>, value: &Node, params: LinkedList<Value>)
-        -> TeraResult<bool> {
-    if params.len() != 0 {
-        return Err(TeraError::TestError("defined".to_string(),
-            "defined should not be called with parameters".to_string()))
+pub type TesterFn = fn(&str, Option<Value>, Vec<Value>) -> TeraResult<bool>;
+
+
+// Some helper functions to remove boilerplate with tester error handling
+fn number_args_allowed(arg_name: &str, tester_name: &str, max: usize, args_len: usize) -> TeraResult<()> {
+    if max == 0 && args_len > max {
+        return Err(TeraError::TestError(
+            tester_name.to_string(),
+            format!(
+                "{} was called on the variable {} with some arguments \
+                but this test doesn't take arguments.",
+                tester_name, arg_name
+            )
+        ))
+    }
+
+    if args_len > max {
+        return Err(TeraError::TestError(
+            tester_name.to_string(),
+            format!(
+                "{} was called on the variable {} with {} arguments, the max number is {}. ",
+                tester_name, arg_name, args_len, max
+            )
+        ))
+    }
+
+    Ok(())
+}
+
+// Called to check if the Value is defined and return an Err if not
+fn value_defined(arg_name: &str, tester_name: &str, value: &Option<Value>) -> TeraResult<()> {
+    if value.is_none() {
+        return Err(TeraError::TestError(
+            tester_name.to_string(),
+            format!("{} was called on the variable {}, which is undefined", tester_name, arg_name)
+        ));
     }
 
-    let name = match *value {
-        Node::Identifier { ref name, .. } => name,
-        _ => return Err(TeraError::TestError("defined".to_string(),
-                "defined can only be called on identifiers".to_string()))
-    };
+    Ok(())
+}
+
+/// Returns true if `value` is defined. Otherwise, returns false.
+pub fn defined(name: &str, value: Option<Value>, params: Vec<Value>) -> TeraResult<bool> {
+    try!(number_args_allowed(name, "defined", 0, params.len()));
 
-    Ok(context.contains_key(name))
+    Ok(value.is_some())
 }
 
+/// Returns true if `value` is undefined. Otherwise, returns false.
+pub fn undefined(name: &str, value: Option<Value>, params: Vec<Value>) -> TeraResult<bool> {
+    try!(number_args_allowed(name, "undefined", 0, params.len()));
+
+    Ok(value.is_none())
+}
+
+/// Returns true if `value` is a string. Otherwise, returns false.
+pub fn string(name: &str, value: Option<Value>, params: Vec<Value>) -> TeraResult<bool> {
+    try!(number_args_allowed(name, "string", 0, params.len()));
+    try!(value_defined(name, "string", &value));
+
+    match value {
+        Some(Value::String(_)) => Ok(true),
+        _ => Ok(false)
+    }
+}
+
+/// Returns true if `value` is a number. Otherwise, returns false.
+pub fn number(name: &str, value: Option<Value>, params: Vec<Value>) -> TeraResult<bool> {
+    try!(number_args_allowed(name, "number", 0, params.len()));
+    try!(value_defined(name, "number", &value));
+
+    match value {
+        Some(Value::I64(_)) | Some(Value::F64(_)) | Some(Value::U64(_)) => Ok(true),
+        _ => Ok(false)
+    }
+}
+
+/// Returns true if `value` is an odd number. Otherwise, returns false.
+pub fn odd(name: &str, value: Option<Value>, params: Vec<Value>) -> TeraResult<bool> {
+    try!(number_args_allowed(name, "odd", 0, params.len()));
+    try!(value_defined(name, "odd", &value));
+
+    match value.and_then(|v| v.to_number().ok()) {
+        Some(f) => Ok(f % 2.0 != 0.0),
+        _ => Err(TeraError::TestError(
+            "odd".to_string(),
+            "odd can only be called on numbers".to_string()
+        ))
+    }
+}
+
+
+/// Returns true if `value` is an even number. Otherwise, returns false.
+pub fn even(name: &str, value: Option<Value>, params: Vec<Value>) -> TeraResult<bool> {
+    try!(number_args_allowed(name, "even", 0, params.len()));
+    try!(value_defined(name, "even", &value));
+
+    let is_odd = try!(odd(name, value, params));
+    Ok(!is_odd)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::{defined, string};
+
+    use serde_json::value::{to_value};
+
+    #[test]
+    fn test_number_args_ok() {
+        assert!(defined("", None, vec![]).is_ok())
+    }
+
+    #[test]
+    fn test_too_many_args() {
+        assert!(defined("", None, vec![to_value(1)]).is_err())
+    }
+
+    #[test]
+    fn test_value_defined() {
+        assert!(string("", None, vec![]).is_err())
+    }
+}
diff --git a/tests/expected/variable_tests.html b/tests/expected/variable_tests.html
index 6b71af8..e59deed 100644
--- a/tests/expected/variable_tests.html
+++ b/tests/expected/variable_tests.html
@@ -1,3 +1,13 @@
 <h1>Hello!</h1>
 
 <h1>Hello again!</h1>
+
+Blabla
+
+String
+
+Odd
+
+Even
+
+Number
diff --git a/tests/templates/variable_tests.html b/tests/templates/variable_tests.html
index 788be99..c240e6a 100644
--- a/tests/templates/variable_tests.html
+++ b/tests/templates/variable_tests.html
@@ -9,3 +9,31 @@
 {% else %}
   <h1>Oh no!</h1>
 {% endif %}
+
+{% if blabla is undefined %}
+    Blabla
+{% endif %}
+
+{% if username is string %}
+    String
+{% elif product is string %}
+    Wat
+{% endif %}
+
+{% if number_reviews + 1 is odd %}
+    Odd
+{% elif number_reviews is odd %}
+    Nop
+{% endif %}
+
+{% if number_reviews is even %}
+    Even
+{% elif number_reviews + 1 is even %}
+    Nop2
+{% endif %}
+
+{% if number_reviews is number %}
+    Number
+{% elif product is number %}
+    Wat2
+{% endif %}