Repository: djc/askama
Pull request #390 (2020-12-01 02:45:07)

Fixed implicit borrow of expressions
This does not introduce any breaking changes<sup>\*</sup>.

The change fixes so expressions are now borrowed as a whole, instead of only the first value. Which would cause compile errors, for types that don't implement e.g. `Add<T> for &T`.

In short expressions now expand into `&(...)` instead of `&...`.

_<sup>\*</sup> Technically, it would be considered a breaking change. However, I'd say it's highly unlikely someone has implemented e.g. `Add<T> for &T` and not `Add<T> for T`. To be able to rely on something that could be considered inconsistent behavior._

-----

### Example 1

Minimal example:

```jinja
{{ true && true }}
```

Which expands like this:

**Before:** `&true && true`
**After:** `&(true && true)`

Resulting in the following compile error:

```rust
error[E0308]: mismatched types
   |
11 | #[derive(Template)]
   |          ^^^^^^^^ expected `bool`, found `&bool`
```

This isn't and was never an issue for e.g. `{{ 1 + 2 }}`, as the various operators e.g. `Add<&T> for T` are implemented for various types. However, this is not the case for short-circuit operators.

To have a more complete example of when this applies. I personally have a few macros, which accept booleans to decide on what it expands to.

```jinja
{% macro foo(b) %}
    {% if b.clone() %}
        ...
    {% endif %}
{% endmacro %}

{% call foo(true && true) %}
```

Which expands like this:

**Before:** `let (b) = (&true && true);`
**After:** `let (b) = (&(true && true));`

_The fact that `.clone()` is needed, is a whole other can of worms._

-----

### Example 2

Another example where this also manifests itself is:

```rust
#[derive(Template)]
#[template(path = "foo.html.jinja")]
struct Foo {
    time: SystemTime,
    dur: Duration,
}
```

```jinja
{{ self.time + self.dur }}
```

Once again only [`Add<Duration> for SystemTime`](AddDurTime) exists, so this also resulted in a compile error:

[AddDurTime]: https://doc.rust-lang.org/std/ops/trait.Add.html#impl-Add%3CDuration%3E-2

```rust
error[E0369]: cannot add `Duration` to `&SystemTime`
```



Patch:
diff --git a/askama_shared/src/generator.rs b/askama_shared/src/generator.rs
index a37ca7c..3a91e3c 100644
--- a/askama_shared/src/generator.rs
+++ b/askama_shared/src/generator.rs
@@ -717,10 +717,11 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
             }
             names.write(arg);
 
-            values.write("&");
+            values.write("&(");
             values.write(&self.visit_expr_root(args.get(i).ok_or_else(|| {
                 CompileError::String(format!("macro '{}' takes more than {} arguments", name, i))
             })?)?);
+            values.write(")");
             self.locals.insert(arg);
         }
 
@@ -944,7 +945,7 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
                     let expression = match wrapped {
                         Wrapped => expr_buf.buf,
                         Unwrapped => format!(
-                            "::askama::MarkupDisplay::new_unsafe(&{}, {})",
+                            "::askama::MarkupDisplay::new_unsafe(&({}), {})",
                             expr_buf.buf, self.input.escaper
                         ),
                     };


Test patch:
diff --git a/testing/templates/macro-short-circuit.html b/testing/templates/macro-short-circuit.html
new file mode 100644
index 0000000..4a19b86
--- /dev/null
+++ b/testing/templates/macro-short-circuit.html
@@ -0,0 +1,9 @@
+{% macro foo(b) -%}
+    {{ b }}
+{%- endmacro -%}
+{% call foo(true) -%}
+{% call foo(true && true) -%}
+{% call foo(true && true && true) -%}
+{% call foo(false) -%}
+{% call foo(false || true) -%}
+{% call foo(false || false || true) -%}
diff --git a/testing/tests/macro.rs b/testing/tests/macro.rs
index 51fbcdc..459b1e2 100644
--- a/testing/tests/macro.rs
+++ b/testing/tests/macro.rs
@@ -43,3 +43,13 @@ fn test_deep_import() {
     let t = DeepImportTemplate;
     assert_eq!(t.render().unwrap(), "foo");
 }
+
+#[derive(Template)]
+#[template(path = "macro-short-circuit.html")]
+struct ShortCircuitTemplate {}
+
+#[test]
+fn test_short_circuit() {
+    let t = ShortCircuitTemplate {};
+    assert_eq!(t.render().unwrap(), "truetruetruefalsetruetrue");
+}
diff --git a/testing/tests/operators.rs b/testing/tests/operators.rs
index 99a2a4c..05c1dab 100644
--- a/testing/tests/operators.rs
+++ b/testing/tests/operators.rs
@@ -53,3 +53,13 @@ fn test_ranges() {
     };
     assert_eq!(t.render().unwrap(), "abcd\nbcd\n\na\nab");
 }
+
+#[derive(Template)]
+#[template(source = "{{ true && true }}{{ false || true }}", ext = "txt")]
+struct ShortCircuitTemplate {}
+
+#[test]
+fn test_short_circuit() {
+    let t = ShortCircuitTemplate {};
+    assert_eq!(t.render().unwrap(), "truetrue");
+}