Fixed a bug that caused internal errors if names where used as iteration

variable and regular variable *after* the loop if that variable was unused
*before* the loop.  (#331)

--HG--
branch : trunk

CHANGES

@@ -15,6 +15,9 @@ Version 2.2
 - Fixed a bug in the parser that made ``{{ foo[1, 2] }}`` impossible.
 - Made it possible to refer to names from outer scopes in included templates
   that were unused in the callers frame (#327)
+- Fixed a bug that caused internal errors if names where used as iteration
+  variable and regular variable *after* the loop if that variable was unused
+  *before* the loop.  (#331)
 
 Version 2.1.1
 -------------

jinja2/compiler.py

@@ -115,14 +115,6 @@ class Identifiers(object):
             return False
         return name in self.declared
 
-    def find_shadowed(self, extra=()):
-        """Find all the shadowed names.  extra is an iterable of variables
-        that may be defined with `add_special` which may occour scoped.
-        """
-        return (self.declared | self.outer_undeclared) & \
-               (self.declared_locally | self.declared_parameter) | \
-               set(x for x in extra if self.is_declared(x))
-
 
 class Frame(object):
     """Holds compile time information for us."""
@@ -151,15 +143,17 @@ class Frame(object):
         # the name of the block we're in, otherwise None.
         self.block = parent and parent.block or None
 
+        # a set of actually assigned names
+        self.assigned_names = set()
+
         # the parent of this frame
         self.parent = parent
 
         if parent is not None:
             self.identifiers.declared.update(
                 parent.identifiers.declared |
-                parent.identifiers.declared_locally |
                 parent.identifiers.declared_parameter |
-                parent.identifiers.undeclared
+                parent.assigned_names
             )
             self.identifiers.outer_undeclared.update(
                 parent.identifiers.undeclared -
@@ -184,6 +178,15 @@ class Frame(object):
         for node in nodes:
             visitor.visit(node)
 
+    def find_shadowed(self, extra=()):
+        """Find all the shadowed names.  extra is an iterable of variables
+        that may be defined with `add_special` which may occour scoped.
+        """
+        i = self.identifiers
+        return (i.declared | i.outer_undeclared) & \
+               (i.declared_locally | i.declared_parameter) | \
+               set(x for x in extra if i.is_declared(x))
+
     def inner(self):
         """Return an inner frame."""
         return Frame(self)
@@ -295,6 +298,9 @@ class FrameIdentifierVisitor(NodeVisitor):
     def visit_FilterBlock(self, node):
         self.visit(node.filter)
 
+    def visit_Scope(self, node):
+        """Stop visiting at scopes."""
+
     def visit_Block(self, node):
         """Stop visiting at blocks."""
 
@@ -538,10 +544,10 @@ class CodeGenerator(NodeVisitor):
         This also predefines locally declared variables from the loop
         body because under some circumstances it may be the case that
 
-        `extra_vars` is passed to `Identifiers.find_shadowed`.
+        `extra_vars` is passed to `Frame.find_shadowed`.
         """
         aliases = {}
-        for name in frame.identifiers.find_shadowed(extra_vars):
+        for name in frame.find_shadowed(extra_vars):
             aliases[name] = ident = self.temporary_identifier()
             self.writeline('%s = l_%s' % (ident, name))
         to_declare = set()
@@ -878,6 +884,7 @@ class CodeGenerator(NodeVisitor):
             self.write('module')
         if frame.toplevel and not node.target.startswith('_'):
             self.writeline('context.exported_vars.discard(%r)' % node.target)
+        frame.assigned_names.add(node.target)
 
     def visit_FromImport(self, node, frame):
         """Visit named imports."""
@@ -914,6 +921,7 @@ class CodeGenerator(NodeVisitor):
                 var_names.append(alias)
                 if not alias.startswith('_'):
                     discarded_names.append(alias)
+            frame.assigned_names.add(alias)
 
         if var_names:
             if len(var_names) == 1:
@@ -1079,6 +1087,7 @@ class CodeGenerator(NodeVisitor):
             self.writeline('context.vars[%r] = ' % node.name)
         self.write('l_%s = ' % node.name)
         self.macro_def(node, macro_frame)
+        frame.assigned_names.add(node.name)
 
     def visit_CallBlock(self, node, frame):
         children = node.iter_child_nodes(exclude=('call',))
@@ -1228,7 +1237,7 @@ class CodeGenerator(NodeVisitor):
         # names here.
         if frame.toplevel:
             assignment_frame = frame.copy()
-            assignment_frame.assigned_names = set()
+            assignment_frame.toplevel_assignments = set()
         else:
             assignment_frame = frame
         self.visit(node.target, assignment_frame)
@@ -1237,14 +1246,14 @@ class CodeGenerator(NodeVisitor):
 
         # make sure toplevel assignments are added to the context.
         if frame.toplevel:
-            public_names = [x for x in assignment_frame.assigned_names
+            public_names = [x for x in assignment_frame.toplevel_assignments
                             if not x.startswith('_')]
-            if len(assignment_frame.assigned_names) == 1:
-                name = iter(assignment_frame.assigned_names).next()
+            if len(assignment_frame.toplevel_assignments) == 1:
+                name = iter(assignment_frame.toplevel_assignments).next()
                 self.writeline('context.vars[%r] = l_%s' % (name, name))
             else:
                 self.writeline('context.vars.update({')
-                for idx, name in enumerate(assignment_frame.assigned_names):
+                for idx, name in enumerate(assignment_frame.toplevel_assignments):
                     if idx:
                         self.write(', ')
                     self.write('%r: l_%s' % (name, name))
@@ -1261,8 +1270,9 @@ class CodeGenerator(NodeVisitor):
 
     def visit_Name(self, node, frame):
         if node.ctx == 'store' and frame.toplevel:
-            frame.assigned_names.add(node.name)
+            frame.toplevel_assignments.add(node.name)
         self.write('l_' + node.name)
+        frame.assigned_names.add(node.name)
 
     def visit_Const(self, node, frame):
         val = node.value
@@ -1472,3 +1482,11 @@ class CodeGenerator(NodeVisitor):
 
     def visit_Break(self, node, frame):
         self.writeline('break', node)
+
+    def visit_Scope(self, node, frame):
+        scope_frame = frame.inner()
+        scope_frame.inspect(node.iter_child_nodes())
+        aliases = self.push_scope(scope_frame)
+        self.pull_locals(scope_frame)
+        self.blockvisit(node.body, scope_frame)
+        self.pop_scope(aliases, scope_frame)

jinja2/nodes.py

@@ -778,6 +778,11 @@ class Break(Stmt):
     """Break a loop."""
 
 
+class Scope(Stmt):
+    """An artificial scope."""
+    fields = ('body',)
+
+
 # make sure nobody creates custom nodes
 def _failing_new(*args, **kwargs):
     raise TypeError('can\'t create custom node types')

tests/test_ext.py

@@ -135,3 +135,26 @@ def test_streamfilter_extension():
     env.globals['gettext'] = lambda x: x.title()
     tmpl = env.from_string('Foo _(bar) Baz')
     assert tmpl.render() == 'Foo Bar Baz'
+
+
+class WithExtension(Extension):
+    tags = frozenset(['with'])
+
+    def parse(self, parser):
+        lineno = parser.stream.next().lineno
+        value = parser.parse_expression()
+        parser.stream.expect('name:as')
+        name = parser.stream.expect('name')
+
+        body = parser.parse_statements(['name:endwith'], drop_needle=True)
+        body.insert(0, nodes.Assign(nodes.Name(name.value, 'store'), value))
+        return nodes.Scope(body)
+
+
+def test_with_extension():
+    env = Environment(extensions=[WithExtension])
+    t = env.from_string('{{ a }}{% with 2 as a %}{{ a }}{% endwith %}{{ a }}')
+    assert t.render(a=1) == '121'
+
+    t = env.from_string('{% with 2 as a %}{{ a }}{% endwith %}{{ a }}')
+    assert t.render(a=3) == '23'

tests/test_forloop.py

@@ -175,3 +175,12 @@ def test_call_in_loop(env):
     {%- endfor -%}
     ''')
     assert t.render() == '[1][2][3]'
+
+
+def test_scoping_bug(env):
+    t = env.from_string('''
+    {%- for item in foo %}...{{ item }}...{% endfor %}
+    {%- macro item(a) %}...{{ a }}...{% endmacro %}
+    {{- item(2) -}}
+    ''')
+    assert t.render(foo=(1,)) == '...1......2...'

tests/test_heavy.py

@@ -0,0 +1,71 @@
+# -*- coding: utf-8 -*-
+"""
+    Heavy tests
+    ~~~~~~~~~~~
+
+    The tests in this module test complex Jinja2 situations to ensure
+    corner cases (unfortunately mostly undocumented scoping behavior)
+    does not change between versions.
+
+    :copyright: Copyright 2009 by the Jinja Team.
+    :license: BSD.
+"""
+
+
+def test_assigned_scoping(env):
+    t = env.from_string('''
+    {%- for item in (1, 2, 3, 4) -%}
+        [{{ item }}]
+    {%- endfor %}
+    {{- item -}}
+    ''')
+    assert t.render(item=42) == '[1][2][3][4]42'
+
+    t = env.from_string('''
+    {%- for item in (1, 2, 3, 4) -%}
+        [{{ item }}]
+    {%- endfor %}
+    {%- set item = 42 %}
+    {{- item -}}
+    ''')
+    assert t.render() == '[1][2][3][4]42'
+
+    t = env.from_string('''
+    {%- set item = 42 %}
+    {%- for item in (1, 2, 3, 4) -%}
+        [{{ item }}]
+    {%- endfor %}
+    {{- item -}}
+    ''')
+    assert t.render() == '[1][2][3][4]42'
+
+
+def test_closure_scoping(env):
+    t = env.from_string('''
+    {%- set wrapper = "<FOO>" %}
+    {%- for item in (1, 2, 3, 4) %}
+        {%- macro wrapper() %}[{{ item }}]{% endmacro %}
+        {{- wrapper() }}
+    {%- endfor %}
+    {{- wrapper -}}
+    ''')
+    assert t.render() == '[1][2][3][4]<FOO>'
+
+    t = env.from_string('''
+    {%- for item in (1, 2, 3, 4) %}
+        {%- macro wrapper() %}[{{ item }}]{% endmacro %}
+        {{- wrapper() }}
+    {%- endfor %}
+    {%- set wrapper = "<FOO>" %}
+    {{- wrapper -}}
+    ''')
+    assert t.render() == '[1][2][3][4]<FOO>'
+
+    t = env.from_string('''
+    {%- for item in (1, 2, 3, 4) %}
+        {%- macro wrapper() %}[{{ item }}]{% endmacro %}
+        {{- wrapper() }}
+    {%- endfor %}
+    {{- wrapper -}}
+    ''')
+    assert t.render(wrapper=23) == '[1][2][3][4]23'