From 66587ce989e5a478e0bb165371fa2b9d42b7040f Mon Sep 17 00:00:00 2001 From: Kevin Brown-Silva Date: Mon, 2 May 2022 15:33:58 -0600 Subject: [PATCH] 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. --- CHANGES.rst | 3 +++ src/jinja2/idtracking.py | 17 +++++++---------- tests/test_regression.py | 10 ++++++++++ 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 2b8179855..b6a5a1af5 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -45,6 +45,9 @@ Unreleased filter. :issue:`1624` - Using ``set`` for multiple assignment (``a, b = 1, 2``) does not fail when the target is a namespace attribute. :issue:`1413` +- Using ``set`` in all branches of ``{% if %}{% elif %}{% else %}`` blocks + does not cause the variable to be considered initially undefined. + :issue:`1253` Version 3.1.4 diff --git a/src/jinja2/idtracking.py b/src/jinja2/idtracking.py index cb4bccb0e..e6dd8cd11 100644 --- a/src/jinja2/idtracking.py +++ b/src/jinja2/idtracking.py @@ -121,23 +121,20 @@ def load(self, name: str) -> None: self._define_ref(name, load=(VAR_LOAD_RESOLVE, name)) def branch_update(self, branch_symbols: t.Sequence["Symbols"]) -> None: - stores: t.Dict[str, int] = {} + stores: t.Set[str] = set() + for branch in branch_symbols: - for target in branch.stores: - if target in self.stores: - continue - stores[target] = stores.get(target, 0) + 1 + stores.update(branch.stores) + + stores.difference_update(self.stores) for sym in branch_symbols: self.refs.update(sym.refs) self.loads.update(sym.loads) self.stores.update(sym.stores) - for name, branch_count in stores.items(): - if branch_count == len(branch_symbols): - continue - - target = self.find_ref(name) # type: ignore + for name in stores: + target = self.find_ref(name) assert target is not None, "should not happen" if self.parent is not None: diff --git a/tests/test_regression.py b/tests/test_regression.py index 10df2d1bd..93d72c5e6 100644 --- a/tests/test_regression.py +++ b/tests/test_regression.py @@ -750,6 +750,16 @@ def is_foo(ctx, s): assert tmpl.render() == "foo" +def test_load_parameter_when_set_in_all_if_branches(env): + tmpl = env.from_string( + "{% if True %}{{ a.b }}{% set a = 1 %}" + "{% elif False %}{% set a = 2 %}" + "{% else %}{% set a = 3 %}{% endif %}" + "{{ a }}" + ) + assert tmpl.render(a={"b": 0}) == "01" + + @pytest.mark.parametrize("unicode_char", ["\N{FORM FEED}", "\x85"]) def test_unicode_whitespace(env, unicode_char): content = "Lorem ipsum\n" + unicode_char + "\nMore text"