jlongster/nunjucks — pull request #68 by fooby (2013-04-03 04:50:46+00:00)
Title: Fix for referencing template variables assigned inside {% if  %} blocks

In nunjucks, the compiler tends to lose track of {% set %} assignments inside {% if %} blocks. e.g.

Jinja2:
```
>>> from jinja2 import Template
>>> t = '{% if true %}{% set x = 10 %}Inside if statement: x should be 10. It is: {{ x }}.{% else %}{% set x = 20 %}{% endif %} Outside if statement: x should be 10. It is: {{ x }} '
>>> Template(t).render()
```
Output:
```
u'Inside if statement: x should be 10. It is: 10. Outside if statement: x should be 10. It is: 10 '
```

Nunjucks:
```
> t = '{% if true %}{% set x = 10 %}Inside if statement: x should be 10. It is: {{ x }}.{% else %}{% set x = 20 %}{% endif %} Outside if statement: x should be 10. It is: {{ x }} '
nunjucks.compiler.compile(t)
tmpl = new nunjucks.Template(t);
tmpl.render();
```
Output:
```
"Inside if statement: x should be 10. It is: 10. Outside if statement: x should be 10. It is: undefined "
```

The reason is that compileSet generates a new temporary id everytime it encounters an assignment to a template variable. e.g. see the corresponding compiled output below:

Original output of `nunjucks.compiler.compile()`:
```javascript
function root(env, context, frame, runtime) {
var lineno = null;
var colno = null;
var output = "";
try {
if(true) {
var t_1 = 10;
frame.set("x", t_1);
if(!frame.parent) {
context.setVariable("x", t_1);
context.addExport("x");
}
output += "Inside if statement: x should be 10. It is: ";
output += runtime.suppressValue(t_1, env.autoesc);
output += ".";
}
else {
var t_2 = 20;
frame.set("x", t_2);
if(!frame.parent) {
context.setVariable("x", t_2);
context.addExport("x");
}
}
output += " Outside if statement: x should be 10. It is: ";
output += runtime.suppressValue(t_2, env.autoesc);
output += " ";
return output;
} catch (e) {
runtime.handleError(e, lineno, colno);
}
}
return {
root: root
};
```

The template variable "x" is first associated with the template variable t_1, then t_2. However, t_2 is only ever assigned inside the "else" block which never gets executed, meaning it's still undefined when it's referenced in the last output statements.

Jinja's approach seems to avoid this by generating internal variable ids based directly on the template names (i.e. "x" would be referenced as "l_x" in the autogenerated Python), ensuring that assignments to the same template variable name always go to the same internal variable.

So for nunjucks, the three possible approaches to fixing this are:
1. Keep a one-to-one mapping of template variables to temporary ids inside the current frame. (e.g. 'x' always maps to 't_1')
2. Copying Jinja's approach of generating internal variable names (perhaps t_)
3. Or instead stop using temporary variables and always dynamically lookup variables from the frame (using "runtime.contextOrFrameLookup"). Neater in code, but much more inefficient.

My patch went with approach 1, as I didn't realise about the possibility of approach 2 until writing this pull request up. Also the patch is a bit messy with multiple loops and sometimes relies on javascript var hoisting (as you'll see in the below output). But it does the job. Let me know what you think.

This patch would also fix the example given in https://github.com/jlongster/nunjucks/issues/50#issuecomment-13008256.

New output of `nunjucks.compiler.compile()`:
```javascript
function root(env, context, frame, runtime) {
var lineno = null;
var colno = null;
var output = "";
try {
if(true) {
var t_1; // < -- note reliance on hoisting
t_1 = 10;
frame.set("x", t_1);
if(!frame.parent) {
context.setVariable("x", t_1);
context.addExport("x");
}
output += "Inside if statement: x should be 10. It is: ";
output += runtime.suppressValue(t_1, env.autoesc);
output += ".";
}
else {
t_1 = 20;
frame.set("x", t_1);
if(!frame.parent) {
context.setVariable("x", t_1);
context.addExport("x");
}
}
output += " Outside if statement: x should be 10. It is: ";
output += runtime.suppressValue(t_1, env.autoesc);
output += " ";
return output;
} catch (e) {
runtime.handleError(e, lineno, colno);
}
}
return {
root: root
};
```