DictTransformer.expr_term: do not discard parentheses (#199)

Author: kkozik-amplify

hcl2/hcl2.lark

@@ -35,8 +35,10 @@ PERCENT : "%"
 DOUBLE_AMP : "&&"
 DOUBLE_PIPE : "||"
 PLUS : "+"
+LPAR : "("
+RPAR : ")"
 
-expr_term : "(" new_line_or_comment? expression new_line_or_comment? ")"
+expr_term : LPAR new_line_or_comment? expression new_line_or_comment? RPAR
             | float_lit
             | int_lit
             | STRING_LIT

hcl2/reconstructor.py

@@ -631,12 +631,7 @@ def _transform_value_to_expr_term(self, value, level) -> Union[Token, Tree]:
                     raise RuntimeError("Token must be `EQ (=)` rule")
 
                 parsed_value = attribute.children[2]
-
-                if parsed_value.data == Token("RULE", "expr_term"):
-                    return parsed_value
-
-                # wrap other types of syntax as an expression (in parentheses)
-                return Tree(Token("RULE", "expr_term"), [parsed_value])
+                return parsed_value
 
             # otherwise it's just a string.
             return Tree(

hcl2/transformer.py

@@ -59,10 +59,9 @@ def expr_term(self, args: List) -> Any:
         if args[0] == "null":
             return None
 
-        # if the expression starts with a paren then unwrap it
-        if args[0] == "(":
-            return args[1]
-        # otherwise return the value itself
+        if args[0] == "(" and args[-1] == ")":
+            return "".join(str(arg) for arg in args)
+
         return args[0]
 
     def index_expr_term(self, args: List) -> str:

test/helpers/terraform-config-json/backend.json

@@ -7,7 +7,7 @@
     },
     {
       "aws": {
-        "region": "${var.backup_region}",
+        "region": "${(var.backup_region)}",
         "alias": "backup"
       }
     }

test/helpers/terraform-config-json/locals_embedded_condition.json

@@ -2,7 +2,7 @@
   "locals": [
     {
       "terraform": {
-        "channels": "${local.running_in_ci ? local.ci_channels : local.local_channels}",
+        "channels": "${(local.running_in_ci ? local.ci_channels : local.local_channels)}",
         "authentication": [],
         "foo": null
       }

test/helpers/terraform-config-json/route_table.json

@@ -3,7 +3,7 @@
     {
       "aws_route": {
         "tgw": {
-          "count": "${var.tgw_name == \"\" ? 0 : var.number_of_az}",
+          "count": "${(var.tgw_name == \"\" ? 0 : var.number_of_az)}",
           "route_table_id": "${aws_route_table.rt[count.index].id}",
           "destination_cidr_block": "10.0.0.0/8",
           "transit_gateway_id": "${data.aws_ec2_transit_gateway.tgw[0].id}"
@@ -13,7 +13,7 @@
     {
       "aws_route": {
         "tgw-dot-index": {
-          "count": "${var.tgw_name == \"\" ? 0 : var.number_of_az}",
+          "count": "${(var.tgw_name == \"\" ? 0 : var.number_of_az)}",
           "route_table_id": "${aws_route_table.rt[count.index].id}",
           "destination_cidr_block": "10.0.0.0/8",
           "transit_gateway_id": "${data.aws_ec2_transit_gateway.tgw[0].id}"

test/helpers/terraform-config-json/variables.json

@@ -51,7 +51,7 @@
     },
     {
       "route53_forwarding_rule_shares": "${{for forwarding_rule_key in keys(var.route53_resolver_forwarding_rule_shares) : \"${forwarding_rule_key}\" => {\"aws_account_ids\": \"${[for account_name in var.route53_resolver_forwarding_rule_shares[forwarding_rule_key].aws_account_names : module.remote_state_subaccounts.map[account_name].outputs[\"aws_account_id\"]]}\"}}}",
-      "has_valid_forwarding_rules_template_inputs": "${length(keys(var.forwarding_rules_template.copy_resolver_rules)) > 0 && length(var.forwarding_rules_template.replace_with_target_ips) > 0 && length(var.forwarding_rules_template.exclude_cidrs) > 0}",
+      "has_valid_forwarding_rules_template_inputs": "${(length(keys(var.forwarding_rules_template.copy_resolver_rules)) > 0 && length(var.forwarding_rules_template.replace_with_target_ips) > 0 && length(var.forwarding_rules_template.exclude_cidrs) > 0)}",
       "for_whitespace": "${{for i in [1, 2, 3] : i => i}}"
     },
     {

test/unit/test_builder.py

@@ -42,7 +42,7 @@ def test_locals_embdedded_condition_tf(self):
         builder.block(
             "locals",
             terraform={
-                "channels": "${local.running_in_ci ? local.ci_channels : local.local_channels}",
+                "channels": "${(local.running_in_ci ? local.ci_channels : local.local_channels)}",
                 "authentication": [],
                 "foo": None,
             },

test/unit/test_hcl2_syntax.py

@@ -174,3 +174,20 @@ def test_null(self):
 
         result = self.load_to_dict(identifier)
         self.assertDictEqual(result, expected)
+
+    def test_expr_term_parentheses(self):
+        literals = {
+            "a = 1 * 2 + 3": {"a": "${1 * 2 + 3}"},
+            "b = 1 * (2 + 3)": {"b": "${1 * (2 + 3)}"},
+            "c = (1 * (2 + 3))": {"c": "${(1 * (2 + 3))}"},
+            "conditional = value == null ? 1 : 0": {
+                "conditional": "${value == None ? 1 : 0}"
+            },
+            "conditional = (value == null ? 1 : 0)": {
+                "conditional": "${(value == None ? 1 : 0)}"
+            },
+        }
+
+        for actual, expected in literals.items():
+            result = self.load_to_dict(actual)
+            self.assertDictEqual(result, expected)

test/unit/test_load.py

@@ -53,5 +53,5 @@ def check_terraform(self, hcl_path_str: str):
 
             json_dict = json.load(json_file)
             self.assertDictEqual(
-                hcl2_dict, json_dict, f"failed comparing {hcl_path_str}"
+                hcl2_dict, json_dict, f"\n\nfailed comparing {hcl_path_str}"
             )