Skip to content

Commit

Permalink
Compile elif tag to elif instead of else: if
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ThiefMaster committed Aug 22, 2017
1 parent d117425 commit cde2a54
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 13 deletions.
3 changes: 3 additions & 0 deletions CHANGES
Expand Up @@ -10,6 +10,9 @@ Version 2.9.7
(`#718`_)
- Resolved a bug where getting debug locals for tracebacks could
modify template context.
- Fixed a bug where having many `{% elif ... %}` blocks resulted in a
"too many levels of indentation" error. These blocks now compile to
native `elif ..:` instead of `else: if ..:` (#759)

.. _#718: https://github.com/pallets/jinja/pull/718

Expand Down
7 changes: 7 additions & 0 deletions jinja2/compiler.py
Expand Up @@ -1129,6 +1129,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()
Expand Down
3 changes: 2 additions & 1 deletion jinja2/idtracking.py
Expand Up @@ -222,9 +222,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)
Expand Down
2 changes: 1 addition & 1 deletion jinja2/nodes.py
Expand Up @@ -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):
Expand Down
13 changes: 6 additions & 7 deletions jinja2/parser.py
Expand Up @@ -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

Expand Down
9 changes: 9 additions & 0 deletions tests/test_core_tags.py
Expand Up @@ -222,6 +222,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() == '...'
Expand Down
8 changes: 4 additions & 4 deletions tests/test_idtracking.py
Expand Up @@ -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'),
Expand Down Expand Up @@ -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 == {
Expand All @@ -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 == {
Expand All @@ -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)

Expand Down

0 comments on commit cde2a54

Please sign in to comment.