From 51c3a853a74d0b933983e95a2c6f38d1fdf6512d Mon Sep 17 00:00:00 2001
From: Ed Page <eopage@gmail.com>
Date: Fri, 16 Nov 2018 18:44:07 -0700
Subject: [PATCH] fix: Deeply nested array indexes

The parser is pretty messed up.  I have no idea why it does what it does
but it has two different code paths for parsing variables.  In one, it
somehow was treationg `a.b` as a variable `"a.b"` and not `a["b"]`.

Not sure why it broke but it now works.

Fixes #230
---
 liquid-compiler/src/parser.rs        | 34 ++++++++++++++++++++++++++--
 liquid-interpreter/src/expression.rs | 16 +++++++++++++
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/liquid-compiler/src/parser.rs b/liquid-compiler/src/parser.rs
index 5b5079c68..7dc5fa23d 100644
--- a/liquid-compiler/src/parser.rs
+++ b/liquid-compiler/src/parser.rs
@@ -57,11 +57,14 @@ pub fn unexpected_token_error_string(expected: &str, actual: Option<String>) ->
 // creates an expression, which wraps everything that gets rendered
 fn parse_expression(tokens: &[Token], options: &LiquidOptions) -> Result<Box<Renderable>> {
     match tokens.get(0) {
-        Some(&Token::Identifier(ref x))
+        Some(&Token::Identifier(_))
             if tokens.len() > 1 && (tokens[1] == Token::Dot || tokens[1] == Token::OpenSquare) =>
         {
+            let mut result = tokens[0]
+                .to_arg()?
+                .into_variable()
+                .expect("identifiers must be variables");
             let indexes = parse_indexes(&tokens[1..])?;
-            let mut result = Variable::with_literal(x.clone());
             result.extend(indexes);
             Ok(Box::new(result))
         }
@@ -344,6 +347,7 @@ mod test_parse_expression {
     use super::*;
 
     use liquid_interpreter::Context;
+    use liquid_value::Array;
     use liquid_value::Object;
     use liquid_value::Value;
 
@@ -401,6 +405,32 @@ mod test_parse_expression {
         let result = result.render(&mut context).unwrap();
         assert_eq!("42", result);
     }
+
+    #[test]
+    fn array_index_access() {
+        let tokens = granularize("post[0]").unwrap();
+        let mut context = Context::new();
+        let mut post = Array::new();
+        post.push(Value::scalar(42i32));
+        context.stack_mut().set_global("post", Value::Array(post));
+
+        let result = parse_expression(&tokens, &null_options()).unwrap();
+        let result = result.render(&mut context).unwrap();
+        assert_eq!("42", result);
+    }
+
+    #[test]
+    fn mixed_access() {
+        let tokens = granularize("post.child[0]").unwrap();
+        let mut context = Context::new();
+        let mut post = Object::new();
+        post.insert("child".into(), Value::Array(vec![Value::scalar(42i32)]));
+        context.stack_mut().set_global("post", Value::Object(post));
+
+        let result = parse_expression(&tokens, &null_options()).unwrap();
+        let result = result.render(&mut context).unwrap();
+        assert_eq!("42", result);
+    }
 }
 
 #[cfg(test)]
diff --git a/liquid-interpreter/src/expression.rs b/liquid-interpreter/src/expression.rs
index dab0685d7..01dba4e6e 100644
--- a/liquid-interpreter/src/expression.rs
+++ b/liquid-interpreter/src/expression.rs
@@ -22,6 +22,22 @@ impl Expression {
         Expression::Literal(Value::scalar(literal))
     }
 
+    /// Convert into a literal if possible.
+    pub fn into_literal(self) -> Option<Value> {
+        match self {
+            Expression::Literal(x) => Some(x),
+            Expression::Variable(_) => None,
+        }
+    }
+
+    /// Convert into a variable, if possible.
+    pub fn into_variable(self) -> Option<Variable> {
+        match self {
+            Expression::Literal(_) => None,
+            Expression::Variable(x) => Some(x),
+        }
+    }
+
     /// Convert to a `Value`.
     pub fn evaluate(&self, context: &Context) -> Result<Value> {
         let val = match *self {