Repository: sunng87/handlebars-rust (rust)

Issue:
Nested each broken in 2.0.0
I have some template that look like this:

```
{{#each protocol.classes as |class| ~}}
{{#each class.methods as |method| ~}}
{{method.id}}
{{/each ~}}
{{/each ~}}
```

With data that look like this (passed as "protocol" to the template):

```
{
  "classes": [
    {
      "methods": [
        "id": 1
      ]
    }
  ]
}
```

This works fine with handlebars 2.0.0-beta.2 but fails with 2.0.0

Pull request:
Fix/nested block param
Fixes #275 

Finally got a fix on 2.0 API  without ruining benchmark. The downside is we introduced some new APIs in `ScopedJson` and `PathAndJson`, which I think is a little fragile in terms of design.

The path system may receive an revamp in 3.0 but for now it works.

Patch:
diff --git a/src/context.rs b/src/context.rs
index 4cd3f55..6bb74ee 100644
--- a/src/context.rs
+++ b/src/context.rs
@@ -118,7 +118,7 @@ fn parse_json_visitor<'a, 'b: 'a>(
     path_context: &'a VecDeque<String>,
     relative_path: &'a str,
     block_params: &'b VecDeque<BlockParams>,
-) -> Result<(VecDeque<&'a str>, Option<Json>), RenderError> {
+) -> Result<(VecDeque<&'a str>, Option<&'b BlockParamHolder>), RenderError> {
     let mut path_stack = VecDeque::new();
     let parser = HandlebarsParser::parse(Rule::path, relative_path)
         .map(|p| p.flatten())
@@ -154,11 +154,11 @@ fn parse_json_visitor<'a, 'b: 'a>(
     }
 
     match used_block_param {
-        Some(BlockParamHolder::Value(ref v)) => {
+        Some(BlockParamHolder::Value(_)) => {
             parse_json_visitor_inner(&mut path_stack, relative_path)?;
             // drop first seg, which is block_param
             path_stack.pop_front();
-            Ok((path_stack, Some(v.clone())))
+            Ok((path_stack, used_block_param))
         }
         Some(BlockParamHolder::Path(ref paths)) => {
             parse_json_visitor_inner(&mut path_stack, relative_path)?;
@@ -169,7 +169,7 @@ fn parse_json_visitor<'a, 'b: 'a>(
                 path_stack.push_front(p)
             }
 
-            Ok((path_stack, None))
+            Ok((path_stack, used_block_param))
         }
         None => {
             parse_json_visitor_inner(&mut path_stack, relative_path)?;
@@ -247,11 +247,11 @@ impl Context {
         relative_path: &str,
         block_params: &VecDeque<BlockParams>,
     ) -> Result<ScopedJson<'reg, 'rc>, RenderError> {
-        let (paths, block_param_value) =
+        let (paths, block_param_holder) =
             parse_json_visitor(base_path, path_context, relative_path, block_params)?;
 
-        if let Some(block_param_value) = block_param_value {
-            let mut data = Some(&block_param_value);
+        if let Some(BlockParamHolder::Value(ref block_param_value)) = block_param_holder {
+            let mut data = Some(block_param_value);
             for p in paths.iter() {
                 data = get_data(data, p)?;
             }
@@ -263,9 +263,16 @@ impl Context {
             for p in paths.iter() {
                 data = get_data(data, p)?;
             }
-            Ok(data
-                .map(|v| ScopedJson::Context(v))
-                .unwrap_or_else(|| ScopedJson::Missing))
+
+            if let Some(BlockParamHolder::Path(_)) = block_param_holder {
+                Ok(data
+                    .map(|v| ScopedJson::BlockContext(v, join(&paths, ".")))
+                    .unwrap_or_else(|| ScopedJson::Missing))
+            } else {
+                Ok(data
+                    .map(|v| ScopedJson::Context(v))
+                    .unwrap_or_else(|| ScopedJson::Missing))
+            }
         }
     }
 
@@ -278,6 +285,19 @@ impl Context {
     }
 }
 
+fn join(segs: &VecDeque<&str>, sep: &str) -> String {
+    let mut out = String::new();
+    let mut iter = segs.into_iter();
+    if let Some(fst) = iter.next() {
+        out.push_str(fst);
+        for elt in iter {
+            out.push_str(sep);
+            out.push_str(elt);
+        }
+    }
+    out
+}
+
 #[cfg(test)]
 mod test {
     use crate::context::{self, BlockParams, Context};
diff --git a/src/helpers/helper_each.rs b/src/helpers/helper_each.rs
index 5664168..e0f0f62 100644
--- a/src/helpers/helper_each.rs
+++ b/src/helpers/helper_each.rs
@@ -38,7 +38,13 @@ impl HelperDef for EachHelper {
                     (true, &Json::Array(ref list)) => {
                         let len = list.len();
 
-                        let array_path = value.path().and_then(|p| rc.concat_path(p));
+                        let array_path = value.path().map(|p| {
+                            if value.is_absolute_path() {
+                                p.to_string()
+                            } else {
+                                format!("{}/{}", rc.get_path(), p)
+                            }
+                        });
 
                         for (i, _) in list.iter().enumerate().take(len) {
                             let mut local_rc = rc.derive();
@@ -83,7 +89,13 @@ impl HelperDef for EachHelper {
                     }
                     (true, &Json::Object(ref obj)) => {
                         let mut first: bool = true;
-                        let obj_path = value.path().and_then(|p| rc.concat_path(p));
+                        let obj_path = value.path().map(|p| {
+                            if value.is_absolute_path() {
+                                p.to_string()
+                            } else {
+                                format!("{}/{}", rc.get_path(), p)
+                            }
+                        });
 
                         for (k, _) in obj.iter() {
                             let mut local_rc = rc.derive();
diff --git a/src/helpers/helper_with.rs b/src/helpers/helper_with.rs
index d62d5f1..f43b8eb 100644
--- a/src/helpers/helper_with.rs
+++ b/src/helpers/helper_with.rs
@@ -35,7 +35,13 @@ impl HelperDef for WithHelper {
                 local_rc.push_local_path_root(local_path_root);
             }
             if not_empty {
-                let new_path = param.path().and_then(|p| local_rc.concat_path(p));
+                let new_path = param.path().map(|p| {
+                    if param.is_absolute_path() {
+                        p.to_string()
+                    } else {
+                        format!("{}/{}", rc.get_path(), p)
+                    }
+                });
                 if let Some(ref new_path) = new_path {
                     local_rc.set_path(new_path.clone());
                 }
diff --git a/src/partial.rs b/src/partial.rs
index 2556625..9801c77 100644
--- a/src/partial.rs
+++ b/src/partial.rs
@@ -18,8 +18,16 @@ fn render_partial<'reg: 'rc, 'rc>(
     out: &mut dyn Output,
 ) -> Result<(), RenderError> {
     // partial context path
-    if let Some(ref p) = d.param(0) {
-        if let Some(param_path) = p.path().and_then(|p| local_rc.concat_path(p)) {
+    if let Some(ref param_ctx) = d.param(0) {
+        let param_path = param_ctx.path().map(|p| {
+            if param_ctx.is_absolute_path() {
+                p.to_string()
+            } else {
+                format!("{}/{}", local_rc.get_path(), p)
+            }
+        });
+
+        if let Some(param_path) = param_path {
             local_rc.promote_local_vars();
             local_rc.set_path(param_path);
         }
diff --git a/src/render.rs b/src/render.rs
index 2b38ed7..292eb42 100644
--- a/src/render.rs
+++ b/src/render.rs
@@ -232,6 +232,7 @@ impl<'reg> RenderContext<'reg> {
         self.block_mut().path = path;
     }
 
+    #[deprecated]
     pub fn concat_path(&self, path_seg: &str) -> Option<String> {
         match context::get_in_block_params(&self.block.block_context, path_seg) {
             Some(BlockParamHolder::Path(paths)) => Some(paths.join("/")),
@@ -574,7 +575,7 @@ impl Parameter {
         ctx: &'rc Context,
         rc: &mut RenderContext<'reg>,
     ) -> Result<PathAndJson<'reg, 'rc>, RenderError> {
-        match *self {
+        match self {
             Parameter::Name(ref name) => {
                 if let Some(value) = rc.get_local_var(name) {
                     // local var, @first, @last for example
@@ -595,11 +596,20 @@ impl Parameter {
                         ScopedJson::Derived(json.as_json().clone()),
                     ))
                 } else {
-                    // failback to normal evaluation
-                    Ok(PathAndJson::new(
-                        Some(name.to_owned()),
-                        rc.evaluate(ctx, name)?,
-                    ))
+                    let value = rc.evaluate(ctx, name)?;
+                    if let Some(ref block_context_path) = value.block_context_path() {
+                        Ok(PathAndJson::new_absolute(
+                            Some(block_context_path.to_string()),
+                            value,
+                        ))
+                    } else {
+                        match value {
+                            // when evaluate result is a derived json, it indicates the
+                            // value is a block context value
+                            ScopedJson::Derived(_) => Ok(PathAndJson::new(None, value)),
+                            _ => Ok(PathAndJson::new(Some(name.to_owned()), value)),
+                        }
+                    }
                 }
             }
             Parameter::Literal(ref j) => Ok(PathAndJson::new(None, ScopedJson::Constant(j))),
@@ -1035,7 +1045,7 @@ fn test_key_with_slash() {
         .register_template_string("t", "{{#each .}}{{@key}}: {{this}}\n{{/each}}")
         .is_ok());
 
-    let r = r.render("t", &json!({"/foo": "bar"})).expect("should work");
+    let r = r.render("t", &json!({"/foo": "bar"})).unwrap();
 
     assert_eq!(r, "/foo: bar\n");
 }
diff --git a/src/value.rs b/src/value.rs
index 60dbcac..5a514d2 100644
--- a/src/value.rs
+++ b/src/value.rs
@@ -14,6 +14,9 @@ pub enum ScopedJson<'reg: 'rc, 'rc> {
     Constant(&'reg Json),
     Derived(Json),
     Context(&'rc Json),
+    // represents a block param json with resolve full path
+    // this path is different from `PathAndJson`
+    BlockContext(&'rc Json, String),
     Missing,
 }
 
@@ -24,6 +27,7 @@ impl<'reg: 'rc, 'rc> ScopedJson<'reg, 'rc> {
             ScopedJson::Constant(j) => j,
             ScopedJson::Derived(ref j) => j,
             ScopedJson::Context(j) => j,
+            ScopedJson::BlockContext(j, _) => j,
             _ => &DEFAULT_VALUE,
         }
     }
@@ -33,7 +37,7 @@ impl<'reg: 'rc, 'rc> ScopedJson<'reg, 'rc> {
     }
 
     pub fn is_missing(&self) -> bool {
-        match *self {
+        match self {
             ScopedJson::Missing => true,
             _ => false,
         }
@@ -43,6 +47,13 @@ impl<'reg: 'rc, 'rc> ScopedJson<'reg, 'rc> {
         let v = self.as_json();
         ScopedJson::Derived(v.clone())
     }
+
+    pub fn block_context_path(&self) -> Option<&String> {
+        match self {
+            ScopedJson::BlockContext(_, ref p) => Some(p),
+            _ => None,
+        }
+    }
 }
 
 impl<'reg: 'rc, 'rc> From<Json> for ScopedJson<'reg, 'rc> {
@@ -51,30 +62,65 @@ impl<'reg: 'rc, 'rc> From<Json> for ScopedJson<'reg, 'rc> {
     }
 }
 
+#[derive(Debug)]
+enum Path {
+    Relative(String),
+    Absolute(String),
+}
+
+impl Path {
+    pub fn get(&self) -> &String {
+        match self {
+            Path::Relative(ref s) => s,
+            Path::Absolute(ref s) => s,
+        }
+    }
+}
+
 /// Json wrapper that holds the Json value and reference path information
 ///
 #[derive(Debug)]
 pub struct PathAndJson<'reg: 'rc, 'rc> {
-    path: Option<String>,
+    path: Option<Path>,
     value: ScopedJson<'reg, 'rc>,
 }
 
 impl<'reg: 'rc, 'rc> PathAndJson<'reg, 'rc> {
     pub fn new(path: Option<String>, value: ScopedJson<'reg, 'rc>) -> PathAndJson<'reg, 'rc> {
-        PathAndJson { path, value }
+        PathAndJson {
+            path: path.map(|p| Path::Relative(p)),
+            value,
+        }
+    }
+
+    pub fn new_absolute(
+        path: Option<String>,
+        value: ScopedJson<'reg, 'rc>,
+    ) -> PathAndJson<'reg, 'rc> {
+        PathAndJson {
+            path: path.map(|p| Path::Absolute(p)),
+            value,
+        }
     }
 
     /// Returns relative path when the value is referenced
     /// If the value is from a literal, the path is `None`
     pub fn path(&self) -> Option<&String> {
-        self.path.as_ref()
+        self.path.as_ref().map(|p| p.get())
     }
 
     /// Return root level of this path if any
     pub fn path_root(&self) -> Option<&str> {
         self.path
             .as_ref()
-            .and_then(|p| p.split(|c| c == '.' || c == '/').nth(0))
+            .and_then(|p| p.get().split(|c| c == '.' || c == '/').nth(0))
+    }
+
+    pub fn is_absolute_path(&self) -> bool {
+        match self.path {
+            Some(Path::Absolute(_)) => true,
+            _ => false,
+        }
     }
 
     /// Returns the value
@@ -86,6 +132,10 @@ impl<'reg: 'rc, 'rc> PathAndJson<'reg, 'rc> {
     pub fn is_value_missing(&self) -> bool {
         self.value.is_missing()
     }
+
+    pub fn render(&self) -> String {
+        self.value.render()
+    }
 }
 
 /// Render Json data with default format


Test patch:
diff --git a/tests/block_context.rs b/tests/block_context.rs
index dae4b71..7d42db9 100644
--- a/tests/block_context.rs
+++ b/tests/block_context.rs
@@ -44,5 +44,33 @@ fn test_singular_and_pair_block_params() {
 
     let template =
         "{{#each this as |b index|}}{{b.value}}{{#each this as |value key|}}:{{key}},{{/each}}{{/each}}";
-    assert_eq!(hbs.render_template(template, &data).unwrap(), "11:value,22:value,");
+    assert_eq!(
+        hbs.render_template(template, &data).unwrap(),
+        "11:value,22:value,"
+    );
+}
+
+#[test]
+fn test_nested_each() {
+    let hbs = Handlebars::new();
+
+    let data = json!({
+        "classes": [
+            {
+                "methods": [
+                    {"id": 1},
+                    {"id": 2}
+                ]
+            },
+            {
+                "methods": [
+                    {"id": 3},
+                    {"id": 4}
+                ]
+            },
+        ],
+    });
+
+    let template = "{{#each classes as |class|}}{{#each class.methods as |method|}}{{method.id}};{{/each}}{{/each}}";
+    assert_eq!(hbs.render_template(template, &data).unwrap(), "1;2;3;4;");
 }