From 271a0eb911903c3e34c6ea484d00359e74fb589b Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 11 Feb 2009 22:49:08 +0100 Subject: [PATCH] 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 | 3 ++ jinja2/compiler.py | 54 +++++++++++++++++++++----------- jinja2/nodes.py | 5 +++ tests/test_ext.py | 23 ++++++++++++++ tests/test_forloop.py | 9 ++++++ tests/test_heavy.py | 71 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 147 insertions(+), 18 deletions(-) create mode 100644 tests/test_heavy.py diff --git a/CHANGES b/CHANGES index 0f65730..c49797b 100644 --- a/CHANGES +++ b/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 ------------- diff --git a/jinja2/compiler.py b/jinja2/compiler.py index b6bf2eb..20ac03b 100644 --- a/jinja2/compiler.py +++ b/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) diff --git a/jinja2/nodes.py b/jinja2/nodes.py index 483366b..6383372 100644 --- a/jinja2/nodes.py +++ b/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') diff --git a/tests/test_ext.py b/tests/test_ext.py index 6c56c4a..ed2c683 100644 --- a/tests/test_ext.py +++ b/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' diff --git a/tests/test_forloop.py b/tests/test_forloop.py index b7079c8..c5bac48 100644 --- a/tests/test_forloop.py +++ b/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...' diff --git a/tests/test_heavy.py b/tests/test_heavy.py new file mode 100644 index 0000000..b115cbd --- /dev/null +++ b/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 = "" %} + {%- for item in (1, 2, 3, 4) %} + {%- macro wrapper() %}[{{ item }}]{% endmacro %} + {{- wrapper() }} + {%- endfor %} + {{- wrapper -}} + ''') + assert t.render() == '[1][2][3][4]' + + t = env.from_string(''' + {%- for item in (1, 2, 3, 4) %} + {%- macro wrapper() %}[{{ item }}]{% endmacro %} + {{- wrapper() }} + {%- endfor %} + {%- set wrapper = "" %} + {{- wrapper -}} + ''') + assert t.render() == '[1][2][3][4]' + + 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' -- 2.26.2