From 59ecb6ee6c8dd37645eeab7f7713f404538c2657 Mon Sep 17 00:00:00 2001 From: Ed Page 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 | 31 ++++++++++++++++++++++++++-- liquid-interpreter/src/expression.rs | 16 ++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/liquid-compiler/src/parser.rs b/liquid-compiler/src/parser.rs index 5b5079c68..355157dc6 100644 --- a/liquid-compiler/src/parser.rs +++ b/liquid-compiler/src/parser.rs @@ -57,11 +57,11 @@ pub fn unexpected_token_error_string(expected: &str, actual: Option) -> // creates an expression, which wraps everything that gets rendered fn parse_expression(tokens: &[Token], options: &LiquidOptions) -> Result> { 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)) } @@ -346,6 +346,7 @@ mod test_parse_expression { use liquid_interpreter::Context; use liquid_value::Object; use liquid_value::Value; + use liquid_value::Array; fn null_options() -> LiquidOptions { LiquidOptions::default() @@ -401,6 +402,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 { + match self { + Expression::Literal(x) => Some(x), + Expression::Variable(_) => None, + } + } + + /// Convert into a variable, if possible. + pub fn into_variable(self) -> Option { + match self { + Expression::Literal(_) => None, + Expression::Variable(x) => Some(x), + } + } + /// Convert to a `Value`. pub fn evaluate(&self, context: &Context) -> Result { let val = match *self {