From ee832194cd9f55f75e5a51359b709d535efe957f Mon Sep 17 00:00:00 2001 From: Kevin Brown-Silva Date: Mon, 2 May 2022 12:01:08 -0600 Subject: [PATCH 1/2] Add support for namespaces in tuple assignment This fixes a bug that existed because namespaces within `{% set %}` were treated as a special case. This special case had the side-effect of bypassing the code which allows for tuples to be assigned to. The solution was to make tuple handling (and by extension, primary token handling) aware of namespaces so that namespace tokens can be handled appropriately. This is handled in a backwards-compatible way which ensures that we do not try to parse namespace tokens when we otherwise would be expecting to parse out name tokens with attributes. Namespace instance checks are moved earlier, and deduplicated, so that all checks are done before the assignment. Otherwise, the check could be emitted in the middle of the tuple. --- CHANGES.rst | 2 ++ docs/templates.rst | 3 +++ src/jinja2/compiler.py | 23 ++++++++++++++++------- src/jinja2/parser.py | 30 +++++++++++++++++------------- tests/test_core_tags.py | 8 ++++++++ 5 files changed, 46 insertions(+), 20 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 521f5a08a..2b8179855 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -43,6 +43,8 @@ Unreleased - ``urlize`` does not add ``mailto:`` to values like `@a@b`. :pr:`1870` - Tests decorated with `@pass_context`` can be used with the ``|select`` filter. :issue:`1624` +- Using ``set`` for multiple assignment (``a, b = 1, 2``) does not fail when the + target is a namespace attribute. :issue:`1413` Version 3.1.4 diff --git a/docs/templates.rst b/docs/templates.rst index 8db8ccaf9..9f376a13c 100644 --- a/docs/templates.rst +++ b/docs/templates.rst @@ -1678,6 +1678,9 @@ The following functions are available in the global scope by default: .. versionadded:: 2.10 + .. versionchanged:: 3.2 + Namespace attributes can be assigned to in multiple assignment. + Extensions ---------- diff --git a/src/jinja2/compiler.py b/src/jinja2/compiler.py index ca079070a..0666cddf7 100644 --- a/src/jinja2/compiler.py +++ b/src/jinja2/compiler.py @@ -1581,6 +1581,22 @@ def visit_Output(self, node: nodes.Output, frame: Frame) -> None: def visit_Assign(self, node: nodes.Assign, frame: Frame) -> None: self.push_assign_tracking() + + # NSRef can only ever be used during assignment so we need to check + # to make sure that it is only being used to assign using a Namespace. + # This check is done here because it is used an expression during the + # assignment and therefore cannot have this check done when the NSRef + # node is visited + for nsref in node.find_all(nodes.NSRef): + ref = frame.symbols.ref(nsref.name) + self.writeline(f"if not isinstance({ref}, Namespace):") + self.indent() + self.writeline( + "raise TemplateRuntimeError" + '("cannot assign attribute on non-namespace object")' + ) + self.outdent() + self.newline(node) self.visit(node.target, frame) self.write(" = ") @@ -1641,13 +1657,6 @@ def visit_NSRef(self, node: nodes.NSRef, frame: Frame) -> None: # `foo.bar` notation they will be parsed as a normal attribute access # when used anywhere but in a `set` context ref = frame.symbols.ref(node.name) - self.writeline(f"if not isinstance({ref}, Namespace):") - self.indent() - self.writeline( - "raise TemplateRuntimeError" - '("cannot assign attribute on non-namespace object")' - ) - self.outdent() self.writeline(f"{ref}[{node.attr!r}]") def visit_Const(self, node: nodes.Const, frame: Frame) -> None: diff --git a/src/jinja2/parser.py b/src/jinja2/parser.py index 22f3f81f7..107232631 100644 --- a/src/jinja2/parser.py +++ b/src/jinja2/parser.py @@ -487,21 +487,18 @@ def parse_assign_target( """ target: nodes.Expr - if with_namespace and self.stream.look().type == "dot": - token = self.stream.expect("name") - next(self.stream) # dot - attr = self.stream.expect("name") - target = nodes.NSRef(token.value, attr.value, lineno=token.lineno) - elif name_only: + if name_only: token = self.stream.expect("name") target = nodes.Name(token.value, "store", lineno=token.lineno) else: if with_tuple: target = self.parse_tuple( - simplified=True, extra_end_rules=extra_end_rules + simplified=True, + extra_end_rules=extra_end_rules, + with_namespace=with_namespace, ) else: - target = self.parse_primary() + target = self.parse_primary(with_namespace=with_namespace) target.set_ctx("store") @@ -643,7 +640,7 @@ def parse_unary(self, with_filter: bool = True) -> nodes.Expr: node = self.parse_filter_expr(node) return node - def parse_primary(self) -> nodes.Expr: + def parse_primary(self, with_namespace: bool = False) -> nodes.Expr: token = self.stream.current node: nodes.Expr if token.type == "name": @@ -651,6 +648,11 @@ def parse_primary(self) -> nodes.Expr: node = nodes.Const(token.value in ("true", "True"), lineno=token.lineno) elif token.value in ("none", "None"): node = nodes.Const(None, lineno=token.lineno) + elif with_namespace and self.stream.look().type == "dot": + next(self.stream) # token + next(self.stream) # dot + attr = self.stream.current + node = nodes.NSRef(token.value, attr.value, lineno=token.lineno) else: node = nodes.Name(token.value, "load", lineno=token.lineno) next(self.stream) @@ -683,6 +685,7 @@ def parse_tuple( with_condexpr: bool = True, extra_end_rules: t.Optional[t.Tuple[str, ...]] = None, explicit_parentheses: bool = False, + with_namespace: bool = False, ) -> t.Union[nodes.Tuple, nodes.Expr]: """Works like `parse_expression` but if multiple expressions are delimited by a comma a :class:`~jinja2.nodes.Tuple` node is created. @@ -704,13 +707,14 @@ def parse_tuple( """ lineno = self.stream.current.lineno if simplified: - parse = self.parse_primary - elif with_condexpr: - parse = self.parse_expression + + def parse() -> nodes.Expr: + return self.parse_primary(with_namespace=with_namespace) + else: def parse() -> nodes.Expr: - return self.parse_expression(with_condexpr=False) + return self.parse_expression(with_condexpr=with_condexpr) args: t.List[nodes.Expr] = [] is_tuple = False diff --git a/tests/test_core_tags.py b/tests/test_core_tags.py index 4bb95e024..2d847a2c9 100644 --- a/tests/test_core_tags.py +++ b/tests/test_core_tags.py @@ -538,6 +538,14 @@ def test_namespace_macro(self, env_trim): ) assert tmpl.render() == "13|37" + def test_namespace_set_tuple(self, env_trim): + tmpl = env_trim.from_string( + "{% set ns = namespace(a=12, b=36) %}" + "{% set ns.a, ns.b = ns.a + 1, ns.b + 1 %}" + "{{ ns.a }}|{{ ns.b }}" + ) + assert tmpl.render() == "13|37" + def test_block_escaping_filtered(self): env = Environment(autoescape=True) tmpl = env.from_string( From b8f4831d41e6a7cb5c40d42f074ffd92d2daccfc Mon Sep 17 00:00:00 2001 From: David Lord Date: Fri, 20 Dec 2024 14:02:31 -0800 Subject: [PATCH 2/2] more comments about nsref assignment only emit nsref instance check once per ref name refactor primary name parsing a bit --- src/jinja2/compiler.py | 24 ++++++++++++++++-------- src/jinja2/parser.py | 18 +++++++++++------- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/jinja2/compiler.py b/src/jinja2/compiler.py index 0666cddf7..a4ff6a1b1 100644 --- a/src/jinja2/compiler.py +++ b/src/jinja2/compiler.py @@ -1582,12 +1582,19 @@ def visit_Output(self, node: nodes.Output, frame: Frame) -> None: def visit_Assign(self, node: nodes.Assign, frame: Frame) -> None: self.push_assign_tracking() - # NSRef can only ever be used during assignment so we need to check - # to make sure that it is only being used to assign using a Namespace. - # This check is done here because it is used an expression during the - # assignment and therefore cannot have this check done when the NSRef - # node is visited + # ``a.b`` is allowed for assignment, and is parsed as an NSRef. However, + # it is only valid if it references a Namespace object. Emit a check for + # that for each ref here, before assignment code is emitted. This can't + # be done in visit_NSRef as the ref could be in the middle of a tuple. + seen_refs: t.Set[str] = set() + for nsref in node.find_all(nodes.NSRef): + if nsref.name in seen_refs: + # Only emit the check for each reference once, in case the same + # ref is used multiple times in a tuple, `ns.a, ns.b = c, d`. + continue + + seen_refs.add(nsref.name) ref = frame.symbols.ref(nsref.name) self.writeline(f"if not isinstance({ref}, Namespace):") self.indent() @@ -1653,9 +1660,10 @@ def visit_Name(self, node: nodes.Name, frame: Frame) -> None: self.write(ref) def visit_NSRef(self, node: nodes.NSRef, frame: Frame) -> None: - # NSRefs can only be used to store values; since they use the normal - # `foo.bar` notation they will be parsed as a normal attribute access - # when used anywhere but in a `set` context + # NSRef is a dotted assignment target a.b=c, but uses a[b]=c internally. + # visit_Assign emits code to validate that each ref is to a Namespace + # object only. That can't be emitted here as the ref could be in the + # middle of a tuple assignment. ref = frame.symbols.ref(node.name) self.writeline(f"{ref}[{node.attr!r}]") diff --git a/src/jinja2/parser.py b/src/jinja2/parser.py index 107232631..f4117754a 100644 --- a/src/jinja2/parser.py +++ b/src/jinja2/parser.py @@ -641,21 +641,24 @@ def parse_unary(self, with_filter: bool = True) -> nodes.Expr: return node def parse_primary(self, with_namespace: bool = False) -> nodes.Expr: + """Parse a name or literal value. If ``with_namespace`` is enabled, also + parse namespace attr refs, for use in assignments.""" token = self.stream.current node: nodes.Expr if token.type == "name": + next(self.stream) if token.value in ("true", "false", "True", "False"): node = nodes.Const(token.value in ("true", "True"), lineno=token.lineno) elif token.value in ("none", "None"): node = nodes.Const(None, lineno=token.lineno) - elif with_namespace and self.stream.look().type == "dot": - next(self.stream) # token - next(self.stream) # dot - attr = self.stream.current + elif with_namespace and self.stream.current.type == "dot": + # If namespace attributes are allowed at this point, and the next + # token is a dot, produce a namespace reference. + next(self.stream) + attr = self.stream.expect("name") node = nodes.NSRef(token.value, attr.value, lineno=token.lineno) else: node = nodes.Name(token.value, "load", lineno=token.lineno) - next(self.stream) elif token.type == "string": next(self.stream) buf = [token.value] @@ -693,8 +696,9 @@ def parse_tuple( if no commas where found. The default parsing mode is a full tuple. If `simplified` is `True` - only names and literals are parsed. The `no_condexpr` parameter is - forwarded to :meth:`parse_expression`. + only names and literals are parsed; ``with_namespace`` allows namespace + attr refs as well. The `no_condexpr` parameter is forwarded to + :meth:`parse_expression`. Because tuples do not require delimiters and may end in a bogus comma an extra hint is needed that marks the end of a tuple. For example