Repository: cobalt-org/liquid-rust (rust)

Issue:
Suport variable passing for include
~~Depends on #275~~


Resources
- [Relevant tests](https://github.com/cobalt-org/liquid-rust/blob/66607e980263d9e704fb34a789e9146124e5629e/tests/conformance_ruby/tags/include_tag_test.rs) (did I tag the 237 when I meant 310?)
- [Rust code](https://github.com/cobalt-org/liquid-rust/blob/master/crates/lib/src/stdlib/tags/include_tag.rs)
- [Ruby code](https://github.com/Shopify/liquid/blob/master/lib/liquid/tags/include.rb)

Pull request:
feat(lib): support for variable passing for `include`
This adds it both for Jekyll and Liquid versions of include which have different syntaxes.

Fixes #310


Patch:
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
index a0a7971..a43c360 100644
--- a/azure-pipelines.yml
+++ b/azure-pipelines.yml
@@ -75,7 +75,7 @@ stages:
       - template: install-rust.yml@templates
         parameters:
           rust: ${{ variables.minrust }}
-      - script: cargo check --all
+      - script: cargo check --all --bins --examples --tests
         displayName: cargo check
 - stage: style
   displayName: Style checks
@@ -97,11 +97,23 @@ stages:
       displayName: Committed
     condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest'))
   - job: style
-    displayName: Style linting
+    displayName: Style checking
+    pool:
+      vmImage: ${{ variables.linux_vm }}
+    steps:
+    - template: install-rust.yml@templates
+      parameters:
+        rust: stable
+        components:
+          - rustfmt
+    - script: cargo fmt --all -- --check
+      displayName: rustfmt
+  - job: lint
+    displayName: Linting
     strategy:
       matrix:
         current:
-          channel: stable
+          channel: ${{ variables.minrust }}
         next:
           channel: beta
     continueOnError: ${{ eq(variables.channel, 'beta') }}
@@ -112,15 +124,12 @@ stages:
       parameters:
         rust: $(channel)
         components:
-          - rustfmt
           - clippy
-    - script: cargo fmt --all -- --check
-      displayName: rustfmt
-    - script: cargo check --all
+    - script: cargo check --all --bins --examples --tests
       displayName: Warnings
       env:
         RUSTFLAGS: "-D warnings"
-    - script: cargo clippy --all -- -D warnings
+    - script: cargo clippy --all --bins --examples --tests -- -D warnings
       displayName: clippy
 - ${{ if ne('', variables.codecov_token) }}:
   - stage: coverage
diff --git a/crates/lib/src/jekyll/include_tag.rs b/crates/lib/src/jekyll/include_tag.rs
index ffc3969..3436278 100644
--- a/crates/lib/src/jekyll/include_tag.rs
+++ b/crates/lib/src/jekyll/include_tag.rs
@@ -1,28 +1,47 @@
 use std::io::Write;
 
-use liquid_core::error::ResultLiquidExt;
+use kstring::KString;
 use liquid_core::parser::TryMatchToken;
 use liquid_core::Expression;
 use liquid_core::Language;
 use liquid_core::Renderable;
-use liquid_core::Result;
 use liquid_core::Runtime;
 use liquid_core::ValueView;
+use liquid_core::{error::ResultLiquidExt, Object, Value};
+use liquid_core::{Error, Result};
 use liquid_core::{ParseTag, TagReflection, TagTokenIter};
 
 #[derive(Debug)]
 struct Include {
     partial: Expression,
+    vars: Vec<(KString, Expression)>,
 }
 
 impl Renderable for Include {
     fn render_to(&self, writer: &mut dyn Write, runtime: &mut Runtime<'_>) -> Result<()> {
         let name = self.partial.evaluate(runtime)?.render().to_string();
+
         runtime.run_in_named_scope(name.clone(), |mut scope| -> Result<()> {
+            if !self.vars.is_empty() {
+                let mut helper_vars = Object::new();
+
+                for (id, val) in &self.vars {
+                    helper_vars.insert(
+                        id.clone(),
+                        val.try_evaluate(scope)
+                            .ok_or_else(|| Error::with_msg("failed to evaluate value"))?
+                            .into_owned(),
+                    );
+                }
+
+                scope.stack_mut().set("include", Value::Object(helper_vars));
+            }
+
             let partial = scope
                 .partials()
                 .get(&name)
                 .trace_with(|| format!("{{% include {} %}}", self.partial).into())?;
+
             partial
                 .render_to(writer, &mut scope)
                 .trace_with(|| format!("{{% include {} %}}", self.partial).into())
@@ -69,12 +88,29 @@ impl ParseTag for IncludeTag {
             TryMatchToken::Fails(name) => name.as_str().to_string(),
         };
 
-        // no more arguments should be supplied, trying to supply them is an error
-        arguments.expect_nothing()?;
-
         let partial = Expression::with_literal(name);
 
-        Ok(Box::new(Include { partial }))
+        let mut vars: Vec<(KString, Expression)> = Vec::new();
+        while let Ok(next) = arguments.expect_next("") {
+            let id = next.expect_identifier().into_result()?.to_string();
+
+            arguments
+                .expect_next("\"=\" expected.")?
+                .expect_str("=")
+                .into_result_custom_msg("expected \"=\" to be used for the assignment")?;
+
+            vars.push((
+                id.into(),
+                arguments
+                    .expect_next("expected value")?
+                    .expect_value()
+                    .into_result()?,
+            ));
+        }
+
+        arguments.expect_nothing()?;
+
+        Ok(Box::new(Include { partial, vars }))
     }
 
     fn reflection(&self) -> &dyn TagReflection {
@@ -113,6 +149,8 @@ mod test {
         fn try_get<'a>(&'a self, name: &str) -> Option<borrow::Cow<'a, str>> {
             match name {
                 "example.txt" => Some(r#"{{'whooo' | size}}{%comment%}What happens{%endcomment%} {%if num < numTwo%}wat{%else%}wot{%endif%} {%if num > numTwo%}wat{%else%}wot{%endif%}"#.into()),
+                "example_var.txt" => Some(r#"{{include.example_var}}"#.into()),
+                "example_multi_var.txt" => Some(r#"{{include.example_var}} {{include.example}}"#.into()),
                 _ => None
             }
         }
@@ -179,6 +217,42 @@ mod test {
         assert_eq!(output, "5 wat wot");
     }
 
+    #[test]
+    fn include_varaible() {
+        let text = "{% include example_var.txt example_var=\"hello\" %}";
+        let options = options();
+        let template = parser::parse(text, &options)
+            .map(runtime::Template::new)
+            .unwrap();
+
+        let partials = partials::OnDemandCompiler::<TestSource>::empty()
+            .compile(::std::sync::Arc::new(options))
+            .unwrap();
+        let mut runtime = RuntimeBuilder::new()
+            .set_partials(partials.as_ref())
+            .build();
+        let output = template.render(&mut runtime).unwrap();
+        assert_eq!(output, "hello");
+    }
+
+    #[test]
+    fn include_mulitple_varaible() {
+        let text = "{% include example_multi_var.txt example_var=\"hello\" example=\"world\" %}";
+        let options = options();
+        let template = parser::parse(text, &options)
+            .map(runtime::Template::new)
+            .unwrap();
+
+        let partials = partials::OnDemandCompiler::<TestSource>::empty()
+            .compile(::std::sync::Arc::new(options))
+            .unwrap();
+        let mut runtime = RuntimeBuilder::new()
+            .set_partials(partials.as_ref())
+            .build();
+        let output = template.render(&mut runtime).unwrap();
+        assert_eq!(output, "hello world");
+    }
+
     #[test]
     fn no_file() {
         let text = "{% include 'file_does_not_exist.liquid' %}";
diff --git a/crates/lib/src/stdlib/blocks/for_block.rs b/crates/lib/src/stdlib/blocks/for_block.rs
index 4b9375c..28b61eb 100644
--- a/crates/lib/src/stdlib/blocks/for_block.rs
+++ b/crates/lib/src/stdlib/blocks/for_block.rs
@@ -175,6 +175,14 @@ impl Renderable for For {
                 runtime.run_in_scope(|mut scope| -> Result<()> {
                     let mut helper_vars = Object::new();
                     helper_vars.insert("length".into(), Value::scalar(range_len as i64));
+                    if let Ok(v) = scope
+                        .stack()
+                        .get(&[liquid_core::model::Scalar::new("forloop")])
+                    {
+                        helper_vars.insert("parentloop".into(), v.into_owned());
+                    } else {
+                        helper_vars.insert("parentloop".into(), Value::Nil);
+                    }
 
                     for (i, v) in range.into_iter().enumerate() {
                         helper_vars.insert("index0".into(), Value::scalar(i as i64));
@@ -1053,4 +1061,70 @@ mod test {
     )
             );
     }
+
+    #[test]
+    fn test_for_parentloop_nil_when_not_present() {
+        //NOTE: this test differs slightly from the liquid conformity test
+        let text = concat!(
+            "{% for inner in outer %}",
+            // the liquid test has `forloop.parentloop.index` here
+            "{{ forloop.parentloop }}.{{ forloop.index }} ",
+            "{% endfor %}"
+        );
+
+        let template = parser::parse(text, &options())
+            .map(runtime::Template::new)
+            .unwrap();
+
+        let mut runtime: Runtime<'_> = Default::default();
+        runtime.stack_mut().set_global(
+            "outer",
+            Value::Array(vec![
+                Value::Array(vec![
+                    Value::scalar(1f64),
+                    Value::scalar(1f64),
+                    Value::scalar(1f64),
+                ]),
+                Value::Array(vec![
+                    Value::scalar(1f64),
+                    Value::scalar(1f64),
+                    Value::scalar(1f64),
+                ]),
+            ]),
+        );
+        let output = template.render(&mut runtime).unwrap();
+        assert_eq!(output, ".1 .2 ");
+    }
+
+    #[test]
+    fn test_for_parentloop_references_parent_loop() {
+        let text = concat!(
+            "{% for inner in outer %}{% for k in inner %}",
+            "{{ forloop.parentloop.index }}.{{ forloop.index }} ",
+            "{% endfor %}{% endfor %}"
+        );
+
+        let template = parser::parse(text, &options())
+            .map(runtime::Template::new)
+            .unwrap();
+
+        let mut runtime: Runtime<'_> = Default::default();
+        runtime.stack_mut().set_global(
+            "outer",
+            Value::Array(vec![
+                Value::Array(vec![
+                    Value::scalar(1f64),
+                    Value::scalar(1f64),
+                    Value::scalar(1f64),
+                ]),
+                Value::Array(vec![
+                    Value::scalar(1f64),
+                    Value::scalar(1f64),
+                    Value::scalar(1f64),
+                ]),
+            ]),
+        );
+        let output = template.render(&mut runtime).unwrap();
+        assert_eq!(output, "1.1 1.2 1.3 2.1 2.2 2.3 ");
+    }
 }
diff --git a/crates/lib/src/stdlib/tags/include_tag.rs b/crates/lib/src/stdlib/tags/include_tag.rs
index 0beb7a9..5524c77 100644
--- a/crates/lib/src/stdlib/tags/include_tag.rs
+++ b/crates/lib/src/stdlib/tags/include_tag.rs
@@ -1,17 +1,19 @@
 use std::io::Write;
 
-use liquid_core::error::ResultLiquidExt;
+use kstring::KString;
 use liquid_core::Expression;
 use liquid_core::Language;
 use liquid_core::Renderable;
 use liquid_core::Runtime;
 use liquid_core::ValueView;
+use liquid_core::error::ResultLiquidExt;
 use liquid_core::{Error, Result};
 use liquid_core::{ParseTag, TagReflection, TagTokenIter};
 
 #[derive(Debug)]
 struct Include {
     partial: Expression,
+    vars: Vec<(KString, Expression)>,
 }
 
 impl Renderable for Include {
@@ -23,11 +25,27 @@ impl Renderable for Include {
                 .into_err();
         }
         let name = value.to_kstr().into_owned();
+
         runtime.run_in_named_scope(name.clone(), |mut scope| -> Result<()> {
+            // if there our additional varaibles creates a include object to access all the varaibles
+            // from e.g. { include 'image.html' path="foo.png" }
+            // then in image.html you could have <img src="{{include.path}}" />
+            if !self.vars.is_empty() {
+                for (id, val) in &self.vars {
+                    let value = val
+                        .try_evaluate(scope)
+                        .ok_or_else(|| Error::with_msg("failed to evaluate value"))?
+                        .into_owned();
+
+                    scope.stack_mut().set(id.clone(), value);
+                }
+            }
+
             let partial = scope
                 .partials()
                 .get(&name)
                 .trace_with(|| format!("{{% include {} %}}", self.partial).into())?;
+
             partial
                 .render_to(writer, &mut scope)
                 .trace_with(|| format!("{{% include {} %}}", self.partial).into())
@@ -68,10 +86,35 @@ impl ParseTag for IncludeTag {
 
         let partial = partial.expect_value().into_result()?;
 
-        // no more arguments should be supplied, trying to supply them is an error
+        let mut vars: Vec<(KString, Expression)> = Vec::new();
+        while let Ok(next) = arguments.expect_next("") {
+            let id = next.expect_identifier().into_result()?.to_string();
+
+            arguments
+                .expect_next("\":\" expected.")?
+                .expect_str(":")
+                .into_result_custom_msg("expected \":\" to be used for the assignment")?;
+
+            vars.push((
+                id.into(),
+                arguments
+                    .expect_next("expected value")?
+                    .expect_value()
+                    .into_result()?,
+            ));
+
+            if let Ok(comma) = arguments.expect_next(""){
+                // stop looking for varaibles if there is no comma
+                // currently allows for one trailing comma
+                if comma.expect_str(",").into_result().is_err(){
+                    break;
+                }
+            }
+        }
+
         arguments.expect_nothing()?;
 
-        Ok(Box::new(Include { partial }))
+        Ok(Box::new(Include { partial, vars }))
     }
 
     fn reflection(&self) -> &dyn TagReflection {
@@ -110,6 +153,8 @@ mod test {
         fn try_get<'a>(&'a self, name: &str) -> Option<borrow::Cow<'a, str>> {
             match name {
                 "example.txt" => Some(r#"{{'whooo' | size}}{%comment%}What happens{%endcomment%} {%if num < numTwo%}wat{%else%}wot{%endif%} {%if num > numTwo%}wat{%else%}wot{%endif%}"#.into()),
+                "example_var.txt" => Some(r#"{{example_var}}"#.into()),
+                "example_multi_var.txt" => Some(r#"{{example_var}} {{example}}"#.into()),
                 _ => None
             }
         }
@@ -176,6 +221,60 @@ mod test {
         assert_eq!(output, "5 wat wot");
     }
 
+    #[test]
+    fn include_varaible() {
+        let text = "{% include 'example_var.txt' example_var:\"hello\" %}";
+        let options = options();
+        let template = parser::parse(text, &options)
+            .map(runtime::Template::new)
+            .unwrap();
+
+        let partials = partials::OnDemandCompiler::<TestSource>::empty()
+            .compile(::std::sync::Arc::new(options))
+            .unwrap();
+        let mut runtime = RuntimeBuilder::new()
+            .set_partials(partials.as_ref())
+            .build();
+        let output = template.render(&mut runtime).unwrap();
+        assert_eq!(output, "hello");
+    }
+
+    #[test]
+    fn include_multiple_varaibles() {
+        let text = "{% include 'example_multi_var.txt' example_var:\"hello\", example:\"world\" %}";
+        let options = options();
+        let template = parser::parse(text, &options)
+            .map(runtime::Template::new)
+            .unwrap();
+
+        let partials = partials::OnDemandCompiler::<TestSource>::empty()
+            .compile(::std::sync::Arc::new(options))
+            .unwrap();
+        let mut runtime = RuntimeBuilder::new()
+            .set_partials(partials.as_ref())
+            .build();
+        let output = template.render(&mut runtime).unwrap();
+        assert_eq!(output, "hello world");
+    }
+
+    #[test]
+    fn include_multiple_varaibles_trailing_commma() {
+        let text = "{% include 'example_multi_var.txt' example_var:\"hello\", example:\"dogs\", %}";
+        let options = options();
+        let template = parser::parse(text, &options)
+            .map(runtime::Template::new)
+            .unwrap();
+
+        let partials = partials::OnDemandCompiler::<TestSource>::empty()
+            .compile(::std::sync::Arc::new(options))
+            .unwrap();
+        let mut runtime = RuntimeBuilder::new()
+            .set_partials(partials.as_ref())
+            .build();
+        let output = template.render(&mut runtime).unwrap();
+        assert_eq!(output, "hello dogs");
+    }
+
     #[test]
     fn no_file() {
         let text = "{% include 'file_does_not_exist.liquid' %}";


Test patch:
diff --git a/tests/conformance_ruby/tags/for_tag_test.rs b/tests/conformance_ruby/tags/for_tag_test.rs
index 26d8b08..891c31a 100644
--- a/tests/conformance_ruby/tags/for_tag_test.rs
+++ b/tests/conformance_ruby/tags/for_tag_test.rs
@@ -456,7 +456,6 @@ fn test_for_tag_string() {
 }
 
 #[test]
-#[should_panic] // liquid-rust#271
 fn test_for_parentloop_references_parent_loop() {
     assert_template_result!(
         "1.1 1.2 1.3 2.1 2.2 2.3 ",
diff --git a/tests/conformance_ruby/tags/include_tag_test.rs b/tests/conformance_ruby/tags/include_tag_test.rs
index 0e24832..ad8c879 100644
--- a/tests/conformance_ruby/tags/include_tag_test.rs
+++ b/tests/conformance_ruby/tags/include_tag_test.rs
@@ -81,7 +81,7 @@ fn test_include_tag_for() {
 }
 
 #[test]
-#[should_panic] // liquid-rust#237
+#[should_panic] // fails due to strict_varaibles
 fn test_include_tag_with_local_variables() {
     assert_template_result!(
         "Locale: test123 ",
@@ -92,7 +92,6 @@ fn test_include_tag_with_local_variables() {
 }
 
 #[test]
-#[should_panic] // liquid-rust#237
 fn test_include_tag_with_multiple_local_variables() {
     assert_template_result!(
         "Locale: test123 test321",
@@ -103,7 +102,6 @@ fn test_include_tag_with_multiple_local_variables() {
 }
 
 #[test]
-#[should_panic] // liquid-rust#237
 fn test_include_tag_with_multiple_local_variables_from_runtime() {
     assert_template_result!(
         "Locale: test123 test321",