From 5990bbbd71817f609134e97b1d2c24d125b7b80b Mon Sep 17 00:00:00 2001 From: Adrian Moennich Date: Tue, 22 Aug 2017 22:59:57 +0200 Subject: [PATCH] Compile `elif` tag to `elif` instead of `else: if` This avoids deep nesting in case of many `{% elif .. %}` blocks (which would fail during execution) and also deep recursion (which may fail during compilation) fixes #759 --- jinja2/compiler.py | 7 +++++++ jinja2/idtracking.py | 3 ++- jinja2/nodes.py | 2 +- jinja2/parser.py | 13 ++++++------- tests/test_core_tags.py | 9 +++++++++ tests/test_idtracking.py | 8 ++++---- 6 files changed, 29 insertions(+), 13 deletions(-) diff --git a/jinja2/compiler.py b/jinja2/compiler.py index e69e5f1be..a1bc554ad 100644 --- a/jinja2/compiler.py +++ b/jinja2/compiler.py @@ -1159,6 +1159,13 @@ def visit_If(self, node, frame): self.indent() self.blockvisit(node.body, if_frame) self.outdent() + for elif_ in node.elif_: + self.writeline('elif ', elif_) + self.visit(elif_.test, if_frame) + self.write(':') + self.indent() + self.blockvisit(elif_.body, if_frame) + self.outdent() if node.else_: self.writeline('else:') self.indent() diff --git a/jinja2/idtracking.py b/jinja2/idtracking.py index 59a621d9d..491bfe083 100644 --- a/jinja2/idtracking.py +++ b/jinja2/idtracking.py @@ -231,9 +231,10 @@ def inner_visit(nodes): return rv body_symbols = inner_visit(node.body) + elif_symbols = inner_visit(node.elif_) else_symbols = inner_visit(node.else_ or ()) - self.symbols.branch_update([body_symbols, else_symbols]) + self.symbols.branch_update([body_symbols, elif_symbols, else_symbols]) def visit_Macro(self, node, **kwargs): self.symbols.store(node.name) diff --git a/jinja2/nodes.py b/jinja2/nodes.py index eb8ea0d0c..9b2c98405 100644 --- a/jinja2/nodes.py +++ b/jinja2/nodes.py @@ -314,7 +314,7 @@ class For(Stmt): class If(Stmt): """If `test` is true, `body` is rendered, else `else_`.""" - fields = ('test', 'body', 'else_') + fields = ('test', 'body', 'elif_', 'else_') class Macro(Stmt): diff --git a/jinja2/parser.py b/jinja2/parser.py index f8d71bbe9..565e7c0c0 100644 --- a/jinja2/parser.py +++ b/jinja2/parser.py @@ -210,17 +210,16 @@ def parse_if(self): node.test = self.parse_tuple(with_condexpr=False) node.body = self.parse_statements(('name:elif', 'name:else', 'name:endif')) + node.elif_ = [] + node.else_ = [] token = next(self.stream) if token.test('name:elif'): - new_node = nodes.If(lineno=self.stream.current.lineno) - node.else_ = [new_node] - node = new_node + node = nodes.If(lineno=self.stream.current.lineno) + result.elif_.append(node) continue elif token.test('name:else'): - node.else_ = self.parse_statements(('name:endif',), - drop_needle=True) - else: - node.else_ = [] + result.else_ = self.parse_statements(('name:endif',), + drop_needle=True) break return result diff --git a/tests/test_core_tags.py b/tests/test_core_tags.py index a5c129797..477465fa1 100644 --- a/tests/test_core_tags.py +++ b/tests/test_core_tags.py @@ -249,6 +249,15 @@ def test_elif(self, env): %}...{% else %}XXX{% endif %}''') assert tmpl.render() == '...' + def test_elif_deep(self, env): + elifs = '\n'.join('{{% elif a == {0} %}}{0}'.format(i) + for i in range(1, 1000)) + tmpl = env.from_string('{{% if a == 0 %}}0{0}{{% else %}}x{{% endif %}}' + .format(elifs)) + for x in (0, 10, 999): + assert tmpl.render(a=x).strip() == str(x) + assert tmpl.render(a=1000).strip() == 'x' + def test_else(self, env): tmpl = env.from_string('{% if false %}XXX{% else %}...{% endif %}') assert tmpl.render() == '...' diff --git a/tests/test_idtracking.py b/tests/test_idtracking.py index ea01b17ef..29312d3e3 100644 --- a/tests/test_idtracking.py +++ b/tests/test_idtracking.py @@ -61,7 +61,7 @@ def test_complex(): nodes.Output([ nodes.Name('title_upper', 'load'), nodes.Call(nodes.Name('render_title', 'load'), [ - nodes.Const('Aha')], [], None, None)])], [])])]) + nodes.Const('Aha')], [], None, None)])], [], [])])]) for_loop = nodes.For( nodes.Name('item', 'store'), @@ -155,7 +155,7 @@ def test_if_branching_stores(): tmpl = nodes.Template([ nodes.If(nodes.Name('expression', 'load'), [ nodes.Assign(nodes.Name('variable', 'store'), - nodes.Const(42))], [])]) + nodes.Const(42))], [], [])]) sym = symbols_for_node(tmpl) assert sym.refs == { @@ -177,7 +177,7 @@ def test_if_branching_stores_undefined(): nodes.Assign(nodes.Name('variable', 'store'), nodes.Const(23)), nodes.If(nodes.Name('expression', 'load'), [ nodes.Assign(nodes.Name('variable', 'store'), - nodes.Const(42))], [])]) + nodes.Const(42))], [], [])]) sym = symbols_for_node(tmpl) assert sym.refs == { @@ -197,7 +197,7 @@ def test_if_branching_stores_undefined(): def test_if_branching_multi_scope(): for_loop = nodes.For(nodes.Name('item', 'store'), nodes.Name('seq', 'load'), [ nodes.If(nodes.Name('expression', 'load'), [ - nodes.Assign(nodes.Name('x', 'store'), nodes.Const(42))], []), + nodes.Assign(nodes.Name('x', 'store'), nodes.Const(42))], [], []), nodes.Include(nodes.Const('helper.html'), True, False) ], [], None, False)