From f06a2e1feafbe6612d53c9def07136522520fec2 Mon Sep 17 00:00:00 2001 From: Tim Date: Tue, 18 Aug 2026 11:10:41 -0700 Subject: [PATCH 1/2] fix: load negative integer literals as numbers (#307) `x = -3` serialized to the expression string `${-3}` rather than the int `-3`, while `x = -3.5` still produced a float. Downstream code reading numbers out of a parsed configuration got a string whenever the value happened to be integral, and the 7.x behaviour (fixed in #182) was lost. MINUS is both the unary sign and the binary subtraction operator, so the sign cannot simply be folded into INT_LITERAL: `10 -3` has to keep parsing as a subtraction. FLOAT_LITERAL escapes this only because its pattern cannot be confused with an operator followed by a digit. Recombine the two at serialization instead, where the parse has already settled the question: when a unary `-` is applied to something that serialized to a number, and the operation is the whole value, emit the negated number. Everything else keeps the `${...}` form -- `-var.count` has no literal value, `!flag` is not arithmetic, `1 + -3` is a larger expression whose operand must stay concatenable text, and a `force_operation_parentheses` result cannot carry its parentheses as a bare number. Add an `integers` round-trip suite mirroring the existing `floats` one, plus unit tests covering both halves of the trade-off. Without the fix, 12 of the new tests fail. --- CHANGELOG.md | 1 + hcl2/rules/expressions.py | 33 ++++++++++++- test/integration/hcl2_original/integers.tf | 18 +++++++ .../hcl2_reconstructed/integers.tf | 28 +++++++++++ .../json_reserialized/integers.json | 33 +++++++++++++ .../integration/json_serialized/integers.json | 33 +++++++++++++ test/unit/rules/test_expressions.py | 49 +++++++++++++++++++ test/unit/test_api.py | 43 ++++++++++++++++ 8 files changed, 236 insertions(+), 2 deletions(-) create mode 100644 test/integration/hcl2_original/integers.tf create mode 100644 test/integration/hcl2_reconstructed/integers.tf create mode 100644 test/integration/json_reserialized/integers.json create mode 100644 test/integration/json_serialized/integers.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b0148ee..4f4c5525 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed - Restore `py.typed` marker so type checkers recognize `hcl2` (and `cli`) as typed packages. ([#298](https://github.com/amplify-education/python-hcl2/issues/298)) +- Negative integer literals load as numbers again instead of `${-N}` expression strings, matching negative floats and the pre-8.x behaviour. ([#307](https://github.com/amplify-education/python-hcl2/issues/307)) ## \[8.1.2\] - 2026-04-10 diff --git a/hcl2/rules/expressions.py b/hcl2/rules/expressions.py index 5aa5f76e..15caa1c3 100644 --- a/hcl2/rules/expressions.py +++ b/hcl2/rules/expressions.py @@ -1,7 +1,7 @@ """Rule classes for HCL2 expressions, conditionals, and binary/unary operations.""" from abc import ABC -from typing import Any, Optional, Tuple +from typing import Any, Optional, Tuple, Union from lark.tree import Meta @@ -305,12 +305,41 @@ def serialize(self, options=SerializationOptions(), context=SerializationContext """Serialize to 'operator operand' string.""" with context.modify(inside_dollar_string=True): operator = self.operator.rstrip() - result = f"{operator}{self.expr_term.serialize(options, context)}" + operand = self.expr_term.serialize(options, context) + result = f"{operator}{operand}" if not context.inside_dollar_string: + # A negated numeric literal is a number, not an expression. The + # lexer splits `-3` into MINUS and INT_LITERAL because MINUS is also + # the binary operator (`1 -3` must stay a subtraction), so negative + # integers arrive here rather than as a single token. Recombining + # them keeps `-3` an int, matching `-3.5`, which FLOAT_LITERAL + # already matches whole. + negated = self._negate_numeric_literal(operator, operand, options) + if negated is not None: + return negated result = to_dollar_string(result) if options.force_operation_parentheses: result = self._wrap_into_parentheses(result, options, context) return result + + @staticmethod + def _negate_numeric_literal( + operator: str, operand: Any, options: SerializationOptions + ) -> Optional[Union[int, float]]: + """Return the negated value when this is `-` applied to a number. + + Returns None when the operation is anything else, so that the caller + falls back to the `${...}` expression form: `-var.x` has no literal + value, `!flag` is not arithmetic, and a scientific-notation operand + serializes to a string when `preserve_scientific_notation` is set. + Parenthesised output is likewise left alone, since a bare number cannot + carry the parentheses that option asks for. + """ + if operator != "-" or options.force_operation_parentheses: + return None + if isinstance(operand, bool) or not isinstance(operand, (int, float)): + return None + return -operand diff --git a/test/integration/hcl2_original/integers.tf b/test/integration/hcl2_original/integers.tf new file mode 100644 index 00000000..1a7d5d7d --- /dev/null +++ b/test/integration/hcl2_original/integers.tf @@ -0,0 +1,18 @@ +locals { + simple_int = 123 + zero = 0 + large_int = 9876543210 + negative_int = -42 + negative_one = -1 + negative_large = -9876543210 + int_calculation = 105 * 3 / 2 + int_subtraction = 10 - 3 + int_negated_reference = -var.count + int_comparison = 5 > 2 ? 1 : 0 + int_list = [1, 2, 3, -4, -5] + int_object = { + positive = 7 + negative = -7 + mixed = [-1, 0, 1] + } +} diff --git a/test/integration/hcl2_reconstructed/integers.tf b/test/integration/hcl2_reconstructed/integers.tf new file mode 100644 index 00000000..033e183d --- /dev/null +++ b/test/integration/hcl2_reconstructed/integers.tf @@ -0,0 +1,28 @@ +locals { + simple_int = 123 + zero = 0 + large_int = 9876543210 + negative_int = -42 + negative_one = -1 + negative_large = -9876543210 + int_calculation = 105 * 3 / 2 + int_subtraction = 10 - 3 + int_negated_reference = -var.count + int_comparison = 5 > 2 ? 1 : 0 + int_list = [ + 1, + 2, + 3, + -4, + -5, + ] + int_object = { + positive = 7, + negative = -7, + mixed = [ + -1, + 0, + 1, + ], + } +} diff --git a/test/integration/json_reserialized/integers.json b/test/integration/json_reserialized/integers.json new file mode 100644 index 00000000..5aeaa1c6 --- /dev/null +++ b/test/integration/json_reserialized/integers.json @@ -0,0 +1,33 @@ +{ + "locals": [ + { + "simple_int": 123, + "zero": 0, + "large_int": 9876543210, + "negative_int": -42, + "negative_one": -1, + "negative_large": -9876543210, + "int_calculation": "${105 * 3 / 2}", + "int_subtraction": "${10 - 3}", + "int_negated_reference": "${-var.count}", + "int_comparison": "${5 > 2 ? 1 : 0}", + "int_list": [ + 1, + 2, + 3, + -4, + -5 + ], + "int_object": { + "positive": 7, + "negative": -7, + "mixed": [ + -1, + 0, + 1 + ] + }, + "__is_block__": true + } + ] +} diff --git a/test/integration/json_serialized/integers.json b/test/integration/json_serialized/integers.json new file mode 100644 index 00000000..5aeaa1c6 --- /dev/null +++ b/test/integration/json_serialized/integers.json @@ -0,0 +1,33 @@ +{ + "locals": [ + { + "simple_int": 123, + "zero": 0, + "large_int": 9876543210, + "negative_int": -42, + "negative_one": -1, + "negative_large": -9876543210, + "int_calculation": "${105 * 3 / 2}", + "int_subtraction": "${10 - 3}", + "int_negated_reference": "${-var.count}", + "int_comparison": "${5 > 2 ? 1 : 0}", + "int_list": [ + 1, + 2, + 3, + -4, + -5 + ], + "int_object": { + "positive": 7, + "negative": -7, + "mixed": [ + -1, + 0, + 1 + ] + }, + "__is_block__": true + } + ] +} diff --git a/test/unit/rules/test_expressions.py b/test/unit/rules/test_expressions.py index 9b7e9a7f..545bbef5 100644 --- a/test/unit/rules/test_expressions.py +++ b/test/unit/rules/test_expressions.py @@ -424,6 +424,55 @@ def test_serialize_force_parens_with_expression_parent(self): self.assertEqual(result, "${(-x)}") +class TestUnaryOpRuleNegativeNumbers(TestCase): + """`-3` is the number -3, not the expression string "${-3}". + + The lexer cannot fold the sign into INT_LITERAL, because MINUS is also the + binary subtraction operator and `10 -3` has to stay a subtraction, so a + negative integer reaches serialization as MINUS applied to a literal. + """ + + def _make_unary(self, op_str, operand_val): + token_cls = MINUS_TOKEN if op_str == "-" else NOT_TOKEN + return UnaryOpRule([token_cls(op_str), _make_expr_term(operand_val)]) + + def test_negative_int_serializes_to_int(self): + rule = self._make_unary("-", 3) + self.assertEqual(rule.serialize(), -3) + + def test_negative_zero_serializes_to_int(self): + rule = self._make_unary("-", 0) + self.assertEqual(rule.serialize(), 0) + + def test_negative_float_serializes_to_float(self): + rule = self._make_unary("-", 3.5) + self.assertEqual(rule.serialize(), -3.5) + + def test_negated_identifier_stays_an_expression(self): + rule = self._make_unary("-", "var.count") + self.assertEqual(rule.serialize(), "${-var.count}") + + def test_not_operator_is_untouched(self): + rule = self._make_unary("!", 1) + self.assertEqual(rule.serialize(), "${!1}") + + def test_inside_dollar_string_stays_text(self): + """Within a larger expression the operand must remain concatenable.""" + rule = self._make_unary("-", 3) + ctx = SerializationContext(inside_dollar_string=True) + self.assertEqual(rule.serialize(context=ctx), "-3") + + def test_force_parens_keeps_expression_form(self): + """A bare number cannot carry the parentheses that option requests.""" + rule = self._make_unary("-", 3) + opts = SerializationOptions(force_operation_parentheses=True) + self.assertEqual(rule.serialize(options=opts), "${-3}") + + def test_boolean_operand_is_not_treated_as_a_number(self): + rule = self._make_unary("-", True) + self.assertEqual(rule.serialize(), "${-True}") + + # --- ExpressionRule._wrap_into_parentheses tests --- diff --git a/test/unit/test_api.py b/test/unit/test_api.py index 6af029a5..ea97b596 100644 --- a/test/unit/test_api.py +++ b/test/unit/test_api.py @@ -288,3 +288,46 @@ def test_query_file_object(self): self.assertIsInstance(result, DocumentView) attr = result.attribute("x") self.assertIsNotNone(attr) + + +class TestNegativeIntegerLiterals(TestCase): + """`-3` loads as the int -3, without disturbing subtraction. + + MINUS serves as both the unary sign and the binary subtraction operator, so + a negative integer cannot be folded into INT_LITERAL by the lexer without + breaking `10 -3`. These cases pin both halves of that trade-off. + """ + + def test_negative_int_is_an_int(self): + self.assertEqual(loads("x = -3\n"), {"x": -3}) + + def test_negative_int_matches_negative_float_handling(self): + self.assertEqual(loads("x = -3\ny = -3.5\n"), {"x": -3, "y": -3.5}) + + def test_negative_ints_in_tuple(self): + self.assertEqual(loads("x = [-1, 2, -30]\n"), {"x": [-1, 2, -30]}) + + def test_negative_ints_in_object(self): + self.assertEqual(loads("x = { a = -1, b = 2 }\n"), {"x": {"a": -1, "b": 2}}) + + def test_spaced_subtraction_is_still_an_expression(self): + self.assertEqual(loads("x = 10 - 3\n"), {"x": "${10 - 3}"}) + + def test_tight_subtraction_is_still_an_expression(self): + """`10 -3` is a subtraction, not two adjacent literals.""" + self.assertEqual(loads("x = 10 -3\n"), {"x": "${10 - 3}"}) + + def test_negated_reference_is_still_an_expression(self): + self.assertEqual(loads("x = -var.count\n"), {"x": "${-var.count}"}) + + def test_negation_inside_a_larger_expression(self): + self.assertEqual(loads("x = 1 + -3\n"), {"x": "${1 + -3}"}) + + def test_parenthesised_negation_is_still_an_expression(self): + self.assertEqual(loads("x = -(3)\n"), {"x": "${-(3)}"}) + + def test_scientific_notation_is_unaffected(self): + self.assertEqual(loads("x = -1e10\n"), {"x": "${-1e10}"}) + + def test_round_trip_through_dumps(self): + self.assertEqual(loads(dumps(loads("x = -3\n"))), {"x": -3}) From 238d4e501d13f951fd0b432e0950c58001a29e0d Mon Sep 17 00:00:00 2001 From: Kamil Kozik Date: Mon, 24 Aug 2026 16:07:02 +0200 Subject: [PATCH 2/2] test: cover the reachable paths in negative-literal serialization The bool-operand test asserted "${-True}", which no HCL input can produce: UnaryOpRule serializes its operand with inside_dollar_string set, and LiteralValueRule yields the string "true" in that context. Assert the helper contract directly instead, and add TestNegatedKeywords for the path a parse actually takes. -1e10 lexes as a single FLOAT_LITERAL and never reaches the unary path. Add the spaced form, which does, and whose operand is a string under preserve_scientific_notation. Pin -0 normalization, and add -0 and -true to the integers round-trip suite. The spaced forms stay out of that fixture because the direct pipeline normalizes the space away, which would break the byte-exact direct-reconstruct assertion. Co-Authored-By: Claude Opus 5 (1M context) --- test/integration/hcl2_original/integers.tf | 2 + .../hcl2_reconstructed/integers.tf | 2 + .../json_reserialized/integers.json | 2 + .../integration/json_serialized/integers.json | 2 + test/unit/rules/test_expressions.py | 18 +++++++- test/unit/test_api.py | 44 +++++++++++++++++++ 6 files changed, 68 insertions(+), 2 deletions(-) diff --git a/test/integration/hcl2_original/integers.tf b/test/integration/hcl2_original/integers.tf index 1a7d5d7d..5a92f56e 100644 --- a/test/integration/hcl2_original/integers.tf +++ b/test/integration/hcl2_original/integers.tf @@ -5,6 +5,8 @@ locals { negative_int = -42 negative_one = -1 negative_large = -9876543210 + negative_zero = -0 + negated_keyword = -true int_calculation = 105 * 3 / 2 int_subtraction = 10 - 3 int_negated_reference = -var.count diff --git a/test/integration/hcl2_reconstructed/integers.tf b/test/integration/hcl2_reconstructed/integers.tf index 033e183d..01918622 100644 --- a/test/integration/hcl2_reconstructed/integers.tf +++ b/test/integration/hcl2_reconstructed/integers.tf @@ -5,6 +5,8 @@ locals { negative_int = -42 negative_one = -1 negative_large = -9876543210 + negative_zero = 0 + negated_keyword = -true int_calculation = 105 * 3 / 2 int_subtraction = 10 - 3 int_negated_reference = -var.count diff --git a/test/integration/json_reserialized/integers.json b/test/integration/json_reserialized/integers.json index 5aeaa1c6..3502c100 100644 --- a/test/integration/json_reserialized/integers.json +++ b/test/integration/json_reserialized/integers.json @@ -7,6 +7,8 @@ "negative_int": -42, "negative_one": -1, "negative_large": -9876543210, + "negative_zero": 0, + "negated_keyword": "${-true}", "int_calculation": "${105 * 3 / 2}", "int_subtraction": "${10 - 3}", "int_negated_reference": "${-var.count}", diff --git a/test/integration/json_serialized/integers.json b/test/integration/json_serialized/integers.json index 5aeaa1c6..3502c100 100644 --- a/test/integration/json_serialized/integers.json +++ b/test/integration/json_serialized/integers.json @@ -7,6 +7,8 @@ "negative_int": -42, "negative_one": -1, "negative_large": -9876543210, + "negative_zero": 0, + "negated_keyword": "${-true}", "int_calculation": "${105 * 3 / 2}", "int_subtraction": "${10 - 3}", "int_negated_reference": "${-var.count}", diff --git a/test/unit/rules/test_expressions.py b/test/unit/rules/test_expressions.py index 545bbef5..ea59209a 100644 --- a/test/unit/rules/test_expressions.py +++ b/test/unit/rules/test_expressions.py @@ -469,8 +469,22 @@ def test_force_parens_keeps_expression_form(self): self.assertEqual(rule.serialize(options=opts), "${-3}") def test_boolean_operand_is_not_treated_as_a_number(self): - rule = self._make_unary("-", True) - self.assertEqual(rule.serialize(), "${-True}") + """`bool` subclasses `int`, so without the guard `-true` would serialize to -1. + + Asserted against the helper rather than `serialize()`: no HCL input can + route a Python bool here, because `UnaryOpRule` serializes its operand + with `inside_dollar_string` set, and `LiteralValueRule` yields the string + "true" in that context. See `TestNegatedKeywords` for the parsed path. + """ + options = SerializationOptions() + self.assertIsNone(UnaryOpRule._negate_numeric_literal("-", True, options)) + self.assertIsNone(UnaryOpRule._negate_numeric_literal("-", False, options)) + + def test_zero_operand_returns_zero_not_none(self): + """The caller must test `is not None`; plain truthiness would drop `-0`.""" + result = UnaryOpRule._negate_numeric_literal("-", 0, SerializationOptions()) + self.assertEqual(result, 0) + self.assertIsNotNone(result) # --- ExpressionRule._wrap_into_parentheses tests --- diff --git a/test/unit/test_api.py b/test/unit/test_api.py index ea97b596..4b58f646 100644 --- a/test/unit/test_api.py +++ b/test/unit/test_api.py @@ -327,7 +327,51 @@ def test_parenthesised_negation_is_still_an_expression(self): self.assertEqual(loads("x = -(3)\n"), {"x": "${-(3)}"}) def test_scientific_notation_is_unaffected(self): + """`-1e10` never reaches the unary path: it lexes as a single FLOAT_LITERAL.""" self.assertEqual(loads("x = -1e10\n"), {"x": "${-1e10}"}) + def test_spaced_negation_of_scientific_notation_stays_an_expression(self): + """The spaced form *is* a unary op, and its operand is a string. + + `preserve_scientific_notation` (on by default) keeps `1e10` as source + text, so there is no number to negate and the expression form stands. + """ + self.assertEqual(loads("x = - 1e10\n"), {"x": "${-1e10}"}) + + def test_spaced_negation_of_integer_is_still_a_number(self): + self.assertEqual(loads("x = - 3\n"), {"x": -3}) + + def test_negative_zero_normalises_to_zero(self): + """`-0` is 0. The dict path drops the sign; the direct path keeps the source.""" + self.assertEqual(loads("x = -0\n"), {"x": 0}) + self.assertEqual(dumps(loads("x = -0\n")), "x = 0\n") + def test_round_trip_through_dumps(self): self.assertEqual(loads(dumps(loads("x = -3\n"))), {"x": -3}) + + +class TestNegatedKeywords(TestCase): + """`-true` is not arithmetic, so it stays an expression. + + This is the parsed counterpart to the `bool` guard in + `UnaryOpRule._negate_numeric_literal`: a keyword operand is serialized with + `inside_dollar_string` set and so arrives as the string "true", never as a + Python bool. Without that distinction `bool` subclassing `int` would turn + `-true` into -1. + """ + + def test_negated_true(self): + self.assertEqual(loads("x = -true\n"), {"x": "${-true}"}) + + def test_negated_false(self): + self.assertEqual(loads("x = -false\n"), {"x": "${-false}"}) + + def test_negated_null(self): + self.assertEqual(loads("x = -null\n"), {"x": "${-null}"}) + + def test_not_operator_on_keyword(self): + self.assertEqual(loads("x = !true\n"), {"x": "${!true}"}) + + def test_bare_keywords_are_still_python_values(self): + """Outside an expression the keywords keep their Python mappings.""" + self.assertEqual(loads("x = true\ny = false\nz = null\n"), {"x": True, "y": False, "z": None})