Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

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
  • Loading branch information...
commit 271a0eb911903c3e34c6ea484d00359e74fb589b 1 parent 7887a8c
@mitsuhiko authored
View
3  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
-------------
View
54 jinja2/compiler.py
@@ -115,14 +115,6 @@ def is_declared(self, name, local_only=False):
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 @@ def __init__(self, parent=None):
# 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 @@ def inspect(self, nodes, hard_scope=False):
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 @@ def visit_CallBlock(self, node):
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 @@ def push_scope(self, frame, extra_vars=()):
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 @@ def visit_Import(self, node, frame):
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 @@ def visit_FromImport(self, node, frame):
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 @@ def visit_Macro(self, node, frame):
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 @@ def visit_Assign(self, node, frame):
# 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 @@ def visit_Assign(self, node, frame):
# 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 @@ def visit_Assign(self, node, frame):
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 @@ def visit_Continue(self, node, frame):
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)
View
5 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')
View
23 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'
View
9 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...'
View
71 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'
Please sign in to comment.
Something went wrong with that request. Please try again.