Repository: djc/askama
Pull request #506 (2021-07-01 11:03:16)

Implement destructoring nested tuples on "let" and "for" statement target
This PR implements the parsing of nested tuples for the target of `{% let %}` statements. E.g.

```rs
{% let (a, (b, c)) = … %}
```

Kijewski: If this PR is accepted, my next two PRs will

1. introduce destructoring structs on the lhs, and 
2. the use of the same function for "match" / "if let" targets. 
vallentin: Nice 🎉 

Didn't look at the diffs yet, but from the title I assume it doesn't apply for e.g. `for (a, (b, c)) in ...`? Can the same parsing and generation be applied to that? Would be nice, as then the _last_ point in #451 can be closed :)

Kijewski: Yes, "for … in" uses the new parsing function: https://github.com/djc/askama/pull/506/files#diff-48297b9b3a3f70c8521bedbaa0601f9e2b405eaa7a99fbffe0d9131e3207e478L904-L907

I'll add a test for nested loops in for-loops.
vallentin: So, that's a yes. Nice 🎉 

It's much appreciated, that you've arrived and implemented all these nice to have features :)
Kijewski: Hehe, your welcome. Well, the library is well written, you guys answer fast, so it's quite easy to contribute.

Please have a look at the added test if I understood #451 correctly.
vallentin: > you guys answer fast

That's probably mostly due to luck + all of us being in the same timezone.

> Please have a look at the added test if I understood #451 correctly.

Yes, that looks great. Optionally, a simple `.iter().enumerate()` might also be nice, as a simple base case test. :)
Kijewski: I applied your suggestions.
Kijewski: I split the 1st commit into 6 commits. :-/ → #510
Kijewski: Rebased on #510.
Kijewski: Rebased on main. I could not reproduce the error in askama_rocket at home.
djc: Yeah, I think the askama_rocket issue was a nightly regression that has already been fixed.

Patch:
diff --git a/askama_shared/src/generator.rs b/askama_shared/src/generator.rs
index 281c3b0..a7f52ca 100644
--- a/askama_shared/src/generator.rs
+++ b/askama_shared/src/generator.rs
@@ -826,7 +826,7 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
             }
             Target::Tuple(targets) => targets
                 .iter()
-                .any(|name| self.is_shadowing_variable(&Target::Name(name))),
+                .any(|target| self.is_shadowing_variable(target)),
         }
     }
 
@@ -1523,8 +1523,8 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
             }
             Target::Tuple(targets) => {
                 buf.write("(");
-                for name in targets {
-                    self.visit_target(buf, initialized, &Target::Name(name));
+                for target in targets {
+                    self.visit_target(buf, initialized, target);
                     buf.write(",");
                 }
                 buf.write(")");
diff --git a/askama_shared/src/parser.rs b/askama_shared/src/parser.rs
index 5610a24..a1aabe7 100644
--- a/askama_shared/src/parser.rs
+++ b/askama_shared/src/parser.rs
@@ -3,8 +3,8 @@ use nom::bytes::complete::{escaped, is_not, tag, take_until};
 use nom::character::complete::{anychar, char, digit1};
 use nom::combinator::{complete, map, opt, recognize, value};
 use nom::error::{Error, ParseError};
-use nom::multi::{many0, many1, separated_list0, separated_list1};
-use nom::sequence::{delimited, pair, tuple};
+use nom::multi::{fold_many0, many0, many1, separated_list0, separated_list1};
+use nom::sequence::{delimited, pair, preceded, tuple};
 use nom::{self, error_position, Compare, IResult, InputTake};
 use std::str;
 
@@ -138,7 +138,7 @@ pub struct Macro<'a> {
 #[derive(Debug, PartialEq)]
 pub enum Target<'a> {
     Name(&'a str),
-    Tuple(Vec<&'a str>),
+    Tuple(Vec<Target<'a>>),
 }
 
 #[derive(Clone, Copy, Debug, PartialEq)]
@@ -404,17 +404,35 @@ fn variant_path(i: &[u8]) -> IResult<&[u8], MatchVariant<'_>> {
     })(i)
 }
 
-fn target_single(i: &[u8]) -> IResult<&[u8], Target<'_>> {
-    map(identifier, |s| Target::Name(s))(i)
-}
+fn target(i: &[u8]) -> IResult<&[u8], Target<'_>> {
+    let mut opt_opening_paren = map(opt(ws(tag("("))), |o| o.is_some());
+    let mut opt_closing_paren = map(opt(ws(tag(")"))), |o| o.is_some());
+
+    let (i, target_is_tuple) = opt_opening_paren(i)?;
+    if target_is_tuple {
+        let (i, is_empty_tuple) = opt_closing_paren(i)?;
+        if is_empty_tuple {
+            return Ok((i, Target::Tuple(vec![])));
+        }
 
-fn target_tuple(i: &[u8]) -> IResult<&[u8], Target<'_>> {
-    let parts = separated_list0(tag(","), ws(identifier));
-    let trailing = opt(ws(tag(",")));
-    let mut full = delimited(tag("("), tuple((parts, trailing)), tag(")"));
+        let (i, first_target) = target(i)?;
+        let (i, is_unused_paren) = opt_closing_paren(i)?;
+        if is_unused_paren {
+            return Ok((i, first_target));
+        }
 
-    let (i, (elems, _)) = full(i)?;
-    Ok((i, Target::Tuple(elems)))
+        let mut targets = vec![first_target];
+        let (i, _) = tuple((
+            fold_many0(preceded(ws(tag(",")), target), (), |_, target| {
+                targets.push(target);
+            }),
+            opt(ws(tag(","))),
+            ws(tag(")")),
+        ))(i)?;
+        Ok((i, Target::Tuple(targets)))
+    } else {
+        map(identifier, Target::Name)(i)
+    }
 }
 
 fn variant_name(i: &[u8]) -> IResult<&[u8], MatchVariant<'_>> {
@@ -883,7 +901,7 @@ fn block_let(i: &[u8]) -> IResult<&[u8], Node<'_>> {
     let mut p = tuple((
         opt(tag("-")),
         ws(alt((tag("let"), tag("set")))),
-        ws(alt((target_single, target_tuple))),
+        ws(target),
         opt(tuple((ws(tag("=")), ws(expr_any)))),
         opt(tag("-")),
     ));
@@ -903,7 +921,7 @@ fn block_for<'a>(i: &'a [u8], s: &'a Syntax<'a>) -> IResult<&'a [u8], Node<'a>>
     let mut p = tuple((
         opt(tag("-")),
         ws(tag("for")),
-        ws(alt((target_single, target_tuple))),
+        ws(target),
         ws(tag("in")),
         ws(expr_any),
         opt(tag("-")),


Test patch:
diff --git a/testing/templates/let-destruct-tuple.html b/testing/templates/let-destruct-tuple.html
new file mode 100644
index 0000000..395e872
--- /dev/null
+++ b/testing/templates/let-destruct-tuple.html
@@ -0,0 +1,3 @@
+{% let (a, ((b, c), (d))) = abcd %}{{a}}{{b}}{{c}}{{d}}
+{% let (a, (_, d)) = abcd %}{{a}}{{d}}
+{% let (((a))) = abcd.0 %}{{a}}
diff --git a/testing/tests/loops.rs b/testing/tests/loops.rs
index 7b008a2..fffb39d 100644
--- a/testing/tests/loops.rs
+++ b/testing/tests/loops.rs
@@ -1,3 +1,6 @@
+#![allow(clippy::type_complexity)]
+
+use std::fmt;
 use std::ops::Range;
 
 use askama::Template;
@@ -235,3 +238,69 @@ fn test_for_vec_attr_slice_shadowing() {
     };
     assert_eq!(t.render().unwrap(), "1 2 3 4 5 6 ");
 }
+
+struct NotClonable<T: fmt::Display>(T);
+
+impl<T: fmt::Display> fmt::Display for NotClonable<T> {
+    fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.0.fmt(formatter)
+    }
+}
+
+#[derive(Template)]
+#[template(
+    source = "{% for (((a,), b), c) in v %}{{a}}{{b}}{{c}}-{% endfor %}",
+    ext = "txt"
+)]
+struct ForDestructoringRefTupleTemplate<'a> {
+    v: &'a [(((char,), NotClonable<char>), &'a char)],
+}
+
+#[test]
+fn test_for_destructoring_ref_tuple() {
+    let v = [
+        ((('a',), NotClonable('b')), &'c'),
+        ((('d',), NotClonable('e')), &'f'),
+        ((('g',), NotClonable('h')), &'i'),
+    ];
+    let t = ForDestructoringRefTupleTemplate { v: &v };
+    assert_eq!(t.render().unwrap(), "abc-def-ghi-");
+}
+
+#[derive(Template)]
+#[template(
+    source = "{% for (((a,), b), c) in v %}{{a}}{{b}}{{c}}-{% endfor %}",
+    ext = "txt"
+)]
+struct ForDestructoringTupleTemplate<'a, const N: usize> {
+    v: [(((char,), NotClonable<char>), &'a char); N],
+}
+
+#[test]
+fn test_for_destructoring_tuple() {
+    let t = ForDestructoringTupleTemplate {
+        v: [
+            ((('a',), NotClonable('b')), &'c'),
+            ((('d',), NotClonable('e')), &'f'),
+            ((('g',), NotClonable('h')), &'i'),
+        ],
+    };
+    assert_eq!(t.render().unwrap(), "abc-def-ghi-");
+}
+
+#[derive(Template)]
+#[template(
+    source = "{% for (i, msg) in messages.iter().enumerate() %}{{i}}={{msg}}-{% endfor %}",
+    ext = "txt"
+)]
+struct ForEnumerateTemplate<'a> {
+    messages: &'a [&'a str],
+}
+
+#[test]
+fn test_for_enumerate() {
+    let t = ForEnumerateTemplate {
+        messages: &["hello", "world", "!"],
+    };
+    assert_eq!(t.render().unwrap(), "0=hello-1=world-2=!-");
+}
diff --git a/testing/tests/vars.rs b/testing/tests/vars.rs
index d70a084..75d10e5 100644
--- a/testing/tests/vars.rs
+++ b/testing/tests/vars.rs
@@ -91,3 +91,17 @@ fn test_if_let() {
     let t = IfLet { a: Some("foo") };
     assert_eq!(t.render().unwrap(), "foo");
 }
+
+#[derive(Template)]
+#[template(path = "let-destruct-tuple.html")]
+struct LetDestructTupleTemplate {
+    abcd: (char, ((char, char), char)),
+}
+
+#[test]
+fn test_destruct_tuple() {
+    let t = LetDestructTupleTemplate {
+        abcd: ('w', (('x', 'y'), 'z')),
+    };
+    assert_eq!(t.render().unwrap(), "wxyz\nwz\nw");
+}