Skip to content

Commit 66587ce

Browse files
kevin-browndavidism
authored andcommitted
Fix bug where set would sometimes fail within if
There was a bug that came as the result of an early optimization done within ID tracking that caused loading parameters to fail in a very specific and rare edge case. That edge case only occurred when the parameter was being set within all 3 standard branches of an if block, since the optimization would assume that the parameter was never being referenced and was only ever being set. This would cause the variable to be set to undefined. The fix for this was to remove the optimization and still continue to load in the parameter even if it is set in all 3 branches.
1 parent fbc3a69 commit 66587ce

File tree

3 files changed

+20
-10
lines changed

3 files changed

+20
-10
lines changed

CHANGES.rst

+3
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ Unreleased
4545
filter. :issue:`1624`
4646
- Using ``set`` for multiple assignment (``a, b = 1, 2``) does not fail when the
4747
target is a namespace attribute. :issue:`1413`
48+
- Using ``set`` in all branches of ``{% if %}{% elif %}{% else %}`` blocks
49+
does not cause the variable to be considered initially undefined.
50+
:issue:`1253`
4851

4952

5053
Version 3.1.4

src/jinja2/idtracking.py

+7-10
Original file line numberDiff line numberDiff line change
@@ -121,23 +121,20 @@ def load(self, name: str) -> None:
121121
self._define_ref(name, load=(VAR_LOAD_RESOLVE, name))
122122

123123
def branch_update(self, branch_symbols: t.Sequence["Symbols"]) -> None:
124-
stores: t.Dict[str, int] = {}
124+
stores: t.Set[str] = set()
125+
125126
for branch in branch_symbols:
126-
for target in branch.stores:
127-
if target in self.stores:
128-
continue
129-
stores[target] = stores.get(target, 0) + 1
127+
stores.update(branch.stores)
128+
129+
stores.difference_update(self.stores)
130130

131131
for sym in branch_symbols:
132132
self.refs.update(sym.refs)
133133
self.loads.update(sym.loads)
134134
self.stores.update(sym.stores)
135135

136-
for name, branch_count in stores.items():
137-
if branch_count == len(branch_symbols):
138-
continue
139-
140-
target = self.find_ref(name) # type: ignore
136+
for name in stores:
137+
target = self.find_ref(name)
141138
assert target is not None, "should not happen"
142139

143140
if self.parent is not None:

tests/test_regression.py

+10
Original file line numberDiff line numberDiff line change
@@ -750,6 +750,16 @@ def is_foo(ctx, s):
750750
assert tmpl.render() == "foo"
751751

752752

753+
def test_load_parameter_when_set_in_all_if_branches(env):
754+
tmpl = env.from_string(
755+
"{% if True %}{{ a.b }}{% set a = 1 %}"
756+
"{% elif False %}{% set a = 2 %}"
757+
"{% else %}{% set a = 3 %}{% endif %}"
758+
"{{ a }}"
759+
)
760+
assert tmpl.render(a={"b": 0}) == "01"
761+
762+
753763
@pytest.mark.parametrize("unicode_char", ["\N{FORM FEED}", "\x85"])
754764
def test_unicode_whitespace(env, unicode_char):
755765
content = "Lorem ipsum\n" + unicode_char + "\nMore text"

0 commit comments

Comments
 (0)