Repository: djc/askama (rust)

Issue:
Add support for redefining the same variable with `let` (just as Rust permits)
[Discovered while working on #315 and #327.]

The following statements, although allowed by Rust, are disallowed by `askama`:
~~~~
{% let x = 1 %}
{% let x = 2 %}
~~~~

The failure is:
~~~~
cannot assign twice to immutable variable
help: make this binding mutable: `mut x`
~~~~


Pull request description:
Added support for shadowing variables
Fixes #329

Considering that it wasn't previously possible to shadow variables. Then this qualifies as a non-breaking change, as it only extends new functionality.

- [x] Updated generator, specifically adding `initialized` to `LocalMeta`
- [x] Added test case
- [x] Updated book

In short, if a `LocalMeta` is `initialized` then any subsequent `{% let ... %}` shadows and generates a new variable. The variable is literally redefined by `insert`ing a new one. Thereby making it possible to e.g. initialize and shadow in multiple nested branches.

To be consistent with Rust, `{% let (a, b) = ... %}` always shadows, and can't be used to assign (which it couldn't before either.)


Patch:
diff --git a/askama_shared/src/generator.rs b/askama_shared/src/generator.rs
index ab964e9..b5f98ec 100644
--- a/askama_shared/src/generator.rs
+++ b/askama_shared/src/generator.rs
@@ -876,16 +876,34 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
 
         match *var {
             Target::Name(name) => {
-                if !self.locals.contains(&name) {
+                let meta = self.locals.get(&name).cloned();
+
+                let shadowed = matches!(&meta, Some(meta) if meta.initialized);
+                if shadowed {
+                    // Need to flush the buffer if the variable is being shadowed,
+                    // to ensure the old variable is used.
+                    self.write_buf_writable(buf)?;
+                }
+                if shadowed || meta.is_none() {
                     buf.write("let ");
-                    self.locals.insert_with_default(name);
                 }
                 buf.write(name);
+
+                self.locals.insert(name, LocalMeta::initialized());
             }
             Target::Tuple(ref targets) => {
+                let shadowed = targets
+                    .iter()
+                    .any(|name| matches!(self.locals.get(&name), Some(meta) if meta.initialized));
+                if shadowed {
+                    // Need to flush the buffer if the variable is being shadowed,
+                    // to ensure the old variable is used.
+                    self.write_buf_writable(buf)?;
+                }
+
                 buf.write("let (");
                 for name in targets {
-                    self.locals.insert_with_default(name);
+                    self.locals.insert(name, LocalMeta::initialized());
                     buf.write(name);
                     buf.write(",");
                 }
@@ -1576,14 +1594,25 @@ impl Buffer {
     }
 }
 
-#[derive(Default)]
+#[derive(Clone, Default)]
 struct LocalMeta {
     refs: Option<String>,
+    initialized: bool,
 }
 
 impl LocalMeta {
+    fn initialized() -> Self {
+        Self {
+            refs: None,
+            initialized: true,
+        }
+    }
+
     fn with_ref(refs: String) -> Self {
-        Self { refs: Some(refs) }
+        Self {
+            refs: Some(refs),
+            initialized: true,
+        }
     }
 }
 
diff --git a/book/src/template_syntax.md b/book/src/template_syntax.md
index c3635eb..755d283 100644
--- a/book/src/template_syntax.md
+++ b/book/src/template_syntax.md
@@ -18,7 +18,7 @@ Assignments can't be imported by other templates.
 
 Assignments use the `let` tag:
 
-```text
+```jinja
 {% let name = user.name %}
 {% let len = name.len() %}
 
@@ -31,6 +31,16 @@ Assignments use the `let` tag:
 {{ val }}
 ```
 
+Like Rust, Askama also supports shadowing variables.
+
+```jinja
+{% let foo = "bar" %}
+{{ foo }}
+
+{% let foo = "baz" %}
+{{ foo }}
+```
+
 For compatibility with Jinja, `set` can be used in place of `let`.
 
 ## Filters


Test patch:
diff --git a/testing/templates/let-shadow.html b/testing/templates/let-shadow.html
new file mode 100644
index 0000000..938c5bf
--- /dev/null
+++ b/testing/templates/let-shadow.html
@@ -0,0 +1,22 @@
+{%- let a = 1 -%}
+{%- let b -%}
+
+{%- if cond -%}
+    {%- let b = 22 -%}
+    {{ b }}-
+
+    {%- let b = 33 -%}
+    {{ a }}-{{ b }}-
+{%- else -%}
+    {%- let b = 222 -%}
+    {{ b }}-
+
+    {%- let b = 333 -%}
+    {{ a }}-{{ b }}-
+
+    {%- let (a, b) = Self::tuple() -%}
+    {{ a }}-{{ b }}-
+{%- endif -%}
+
+{%- let a = 11 -%}
+{{ a }}-{{ b }}
diff --git a/testing/tests/vars.rs b/testing/tests/vars.rs
index d1c41f9..d70a084 100644
--- a/testing/tests/vars.rs
+++ b/testing/tests/vars.rs
@@ -46,6 +46,27 @@ fn test_let_decl() {
     assert_eq!(t.render().unwrap(), "bar");
 }
 
+#[derive(Template)]
+#[template(path = "let-shadow.html")]
+struct LetShadowTemplate {
+    cond: bool,
+}
+
+impl LetShadowTemplate {
+    fn tuple() -> (i32, i32) {
+        (4, 5)
+    }
+}
+
+#[test]
+fn test_let_shadow() {
+    let t = LetShadowTemplate { cond: true };
+    assert_eq!(t.render().unwrap(), "22-1-33-11-22");
+
+    let t = LetShadowTemplate { cond: false };
+    assert_eq!(t.render().unwrap(), "222-1-333-4-5-11-222");
+}
+
 #[derive(Template)]
 #[template(source = "{% for v in self.0 %}{{ v }}{% endfor %}", ext = "txt")]
 struct SelfIterTemplate(Vec<usize>);