fix operator precedence

kkozik-amplify · kkozik-amplify · commit ca192325cc03 · 2026-02-22T16:00:32.000+01:00
diff --git a/hcl2/rule_transformer/hcl2.lark b/hcl2/rule_transformer/hcl2.lark
@@ -24,7 +24,6 @@ FLOAT_LITERAL: (NEGATIVE_DECIMAL? DECIMAL+ | NEGATIVE_DECIMAL+) "." DECIMAL+ (EX
                | (NEGATIVE_DECIMAL? DECIMAL+ | NEGATIVE_DECIMAL+) (EXP_MARK)
 
 // Operators
-BINARY_OP : DOUBLE_EQ | NEQ | LT | GT | LEQ | GEQ | MINUS | ASTERISK | SLASH | PERCENT | DOUBLE_AMP | DOUBLE_PIPE | PLUS
 DOUBLE_EQ : "=="
 NEQ : "!="
 LT : "<"
@@ -99,16 +98,61 @@ string_part: STRING_CHARS
            | interpolation
 
 // Expressions
-?expression : expr_term | operation | conditional
+?expression : or_expr QMARK new_line_or_comment? expression new_line_or_comment? COLON new_line_or_comment? expression -> conditional
+            | or_expr
 interpolation: INTERP_START expression RBRACE
-conditional : expression QMARK new_line_or_comment? expression new_line_or_comment? COLON new_line_or_comment? expression
 
-// Operations
-?operation : unary_op | binary_op
+// Operator precedence ladder (lowest to highest)
+// Each level uses left recursion for left-associativity.
+// Rule aliases (-> binary_op, -> binary_term, -> binary_operator) maintain
+// transformer compatibility with BinaryOpRule / BinaryTermRule / BinaryOperatorRule.
+
+// Logical OR
+?or_expr : or_expr or_binary_term new_line_or_comment? -> binary_op
+         | and_expr
+or_binary_term : or_binary_operator new_line_or_comment? and_expr -> binary_term
+!or_binary_operator : DOUBLE_PIPE -> binary_operator
+
+// Logical AND
+?and_expr : and_expr and_binary_term new_line_or_comment? -> binary_op
+          | eq_expr
+and_binary_term : and_binary_operator new_line_or_comment? eq_expr -> binary_term
+!and_binary_operator : DOUBLE_AMP -> binary_operator
+
+// Equality
+?eq_expr : eq_expr eq_binary_term new_line_or_comment? -> binary_op
+         | rel_expr
+eq_binary_term : eq_binary_operator new_line_or_comment? rel_expr -> binary_term
+!eq_binary_operator : DOUBLE_EQ -> binary_operator
+                    | NEQ -> binary_operator
+
+// Relational
+?rel_expr : rel_expr rel_binary_term new_line_or_comment? -> binary_op
+          | add_expr
+rel_binary_term : rel_binary_operator new_line_or_comment? add_expr -> binary_term
+!rel_binary_operator : LT -> binary_operator
+                     | GT -> binary_operator
+                     | LEQ -> binary_operator
+                     | GEQ -> binary_operator
+
+// Additive
+?add_expr : add_expr add_binary_term new_line_or_comment? -> binary_op
+          | mul_expr
+add_binary_term : add_binary_operator new_line_or_comment? mul_expr -> binary_term
+!add_binary_operator : PLUS -> binary_operator
+                     | MINUS -> binary_operator
+
+// Multiplicative
+?mul_expr : mul_expr mul_binary_term new_line_or_comment? -> binary_op
+          | unary_expr
+mul_binary_term : mul_binary_operator new_line_or_comment? unary_expr -> binary_term
+!mul_binary_operator : ASTERISK -> binary_operator
+                     | SLASH -> binary_operator
+                     | PERCENT -> binary_operator
+
+// Unary (highest precedence for operations)
+?unary_expr : unary_op | expr_term
 !unary_op : (MINUS | NOT) expr_term
-binary_op : expression binary_term new_line_or_comment?
-binary_term : binary_operator new_line_or_comment? expression
-!binary_operator : BINARY_OP
 
 // Expression terms
 expr_term : LPAR new_line_or_comment? expression new_line_or_comment? RPAR
diff --git a/hcl2/rule_transformer/reconstructor.py b/hcl2/rule_transformer/reconstructor.py
@@ -6,12 +6,32 @@
 from hcl2.rule_transformer.rules.for_expressions import ForIntroRule
 from hcl2.rule_transformer.rules.literal_rules import IdentifierRule
 from hcl2.rule_transformer.rules.strings import StringRule
-from hcl2.rule_transformer.rules.expressions import ExprTermRule, ConditionalRule
+from hcl2.rule_transformer.rules.expressions import (
+    ExprTermRule,
+    ConditionalRule,
+    UnaryOpRule,
+)
 
 
 class HCLReconstructor:
     """This class converts a Lark.Tree AST back into a string representing the underlying HCL code."""
 
+    _binary_op_types = {
+        "DOUBLE_EQ",
+        "NEQ",
+        "LT",
+        "GT",
+        "LEQ",
+        "GEQ",
+        "MINUS",
+        "ASTERISK",
+        "SLASH",
+        "PERCENT",
+        "DOUBLE_AMP",
+        "DOUBLE_PIPE",
+        "PLUS",
+    }
+
     def __init__(self):
         self._reset_state()
 
@@ -105,8 +125,14 @@ def _should_add_space_before(
             if tokens.EQ.lark_name() in [token_type, self._last_token_name]:
                 return True
 
-            # space around binary operators
-            if tokens.BINARY_OP.lark_name() in [token_type, self._last_token_name]:
+            # Don't add space around operator tokens inside unary_op
+            if parent_rule_name == UnaryOpRule.lark_name():
+                return False
+
+            if (
+                token_type in self._binary_op_types
+                or self._last_token_name in self._binary_op_types
+            ):
                 return True
 
         elif isinstance(current_node, Tree):
@@ -130,7 +156,14 @@ def _reconstruct_tree(self, tree: Tree, parent_rule_name: str = None) -> List[st
         result = []
         rule_name = tree.data
 
-        if rule_name == ExprTermRule.lark_name():
+        if rule_name == UnaryOpRule.lark_name():
+            for i, child in enumerate(tree.children):
+                result.extend(self._reconstruct_node(child, rule_name))
+                if i == 0:
+                    # Suppress space between unary operator and its operand
+                    self._last_was_space = True
+
+        elif rule_name == ExprTermRule.lark_name():
             # Check if parenthesized
             if (
                 len(tree.children) >= 3
diff --git a/hcl2/rule_transformer/rules/expressions.py b/hcl2/rule_transformer/rules/expressions.py
@@ -26,8 +26,30 @@ class ExpressionRule(InlineCommentMixIn, ABC):
     def lark_name() -> str:
         return "expression"
 
-    def __init__(self, children, meta: Optional[Meta] = None):
+    def __init__(
+        self, children, meta: Optional[Meta] = None, parentheses: bool = False
+    ):
         super().__init__(children, meta)
+        self._parentheses = parentheses
+
+    def _wrap_into_parentheses(
+        self, value: str, options=SerializationOptions(), context=SerializationContext()
+    ) -> str:
+        # do not wrap into parentheses if
+        #   1. already wrapped or
+        #   2. is top-level expression (unless explicitly wrapped)
+        if context.inside_parentheses:
+            return value
+        # Look through ExprTermRule wrapper to determine if truly nested
+        parent = getattr(self, "parent", None)
+        if parent is None:
+            return value
+        if isinstance(parent, ExprTermRule):
+            if not isinstance(parent.parent, ExpressionRule):
+                return value
+        elif not isinstance(parent, ExpressionRule):
+            return value
+        return wrap_into_parentheses(value)
 
 
 class ExprTermRule(ExpressionRule):
@@ -47,18 +69,18 @@ def lark_name() -> str:
         return "expr_term"
 
     def __init__(self, children, meta: Optional[Meta] = None):
-        self._parentheses = False
+        parentheses = False
         if (
             isinstance(children[0], LarkToken)
             and children[0].lark_name() == "LPAR"
             and isinstance(children[-1], LarkToken)
             and children[-1].lark_name() == "RPAR"
         ):
-            self._parentheses = True
+            parentheses = True
         else:
             children = [None, *children, None]
         self._insert_optionals(children, [1, 3])
-        super().__init__(children, meta)
+        super().__init__(children, meta, parentheses)
 
     @property
     def parentheses(self) -> bool:
@@ -71,7 +93,10 @@ def expression(self) -> ExpressionRule:
     def serialize(
         self, options=SerializationOptions(), context=SerializationContext()
     ) -> Any:
-        result = self.expression.serialize(options, context)
+        with context.modify(
+            inside_parentheses=self.parentheses or context.inside_parentheses
+        ):
+            result = self.expression.serialize(options, context)
 
         if self.parentheses:
             result = wrap_into_parentheses(result)
@@ -127,6 +152,9 @@ def serialize(
         if not context.inside_dollar_string:
             result = to_dollar_string(result)
 
+        if options.force_operation_parentheses:
+            result = self._wrap_into_parentheses(result, options, context)
+
         return result
 
 
@@ -192,6 +220,9 @@ def serialize(
 
         if not context.inside_dollar_string:
             result = to_dollar_string(result)
+
+        if options.force_operation_parentheses:
+            result = self._wrap_into_parentheses(result, options, context)
         return result
 
 
@@ -214,6 +245,14 @@ def expr_term(self):
     def serialize(
         self, options=SerializationOptions(), context=SerializationContext()
     ) -> Any:
-        return to_dollar_string(
-            f"{self.operator}{self.expr_term.serialize(options, context)}"
-        )
+
+        with context.modify(inside_dollar_string=True):
+            result = f"{self.operator}{self.expr_term.serialize(options, context)}"
+
+        if not context.inside_dollar_string:
+            result = to_dollar_string(result)
+
+        if options.force_operation_parentheses:
+            result = self._wrap_into_parentheses(result, options, context)
+
+        return result
diff --git a/hcl2/rule_transformer/utils.py b/hcl2/rule_transformer/utils.py
@@ -15,11 +15,13 @@ class SerializationOptions:
     wrap_tuples: bool = False
     explicit_blocks: bool = True
     preserve_heredocs: bool = True
+    force_operation_parentheses: bool = False
 
 
 @dataclass
 class SerializationContext:
     inside_dollar_string: bool = False
+    inside_parentheses: bool = False
 
     def replace(self, **kwargs) -> "SerializationContext":
         return replace(self, **kwargs)
