From 0994ea0845d8d58c6c8d5a5db8e2c935b94cf9f4 Mon Sep 17 00:00:00 2001 From: Tim Date: Mon, 31 Aug 2026 13:46:21 -0700 Subject: [PATCH 1/3] fix: emit block line numbers when with_meta is set (#291) `SerializationOptions.with_meta` documented `__start_line__` and `__end_line__` keys, `hcl2tojson` exposed it as `--with-meta`, and the v8 migration guide told readers the v7 keys were "still available". None of that was true: v7 added the keys in `RuleTransformer.block`, the v8 rewrite moved block serialization into `BlockRule.serialize`, and the option went unread anywhere in the package. Emit them from `BlockRule.serialize`, on the same innermost dict the labels nest around and next to `__is_block__` -- the position v7 used. The output is byte-identical to 7.3.1's for the same input, nested blocks included. Two details the old implementation did not have to handle: - A tree built by the deserializer carries an empty `Meta`, whose `.line` raises. Serializing one with `with_meta` set now skips the keys rather than inventing zeros. - `dumps()` would otherwise write the metadata back out as HCL attributes, so both keys join the deserializer's reserved set. The two existing tests only asserted that the option was accepted, which is why nothing noticed; they now assert on the metadata itself. --- CHANGELOG.md | 4 +- docs/01_getting_started.md | 2 +- hcl2/const.py | 2 + hcl2/deserializer.py | 4 +- hcl2/rules/base.py | 9 ++- hcl2/utils.py | 4 +- test/unit/cli/test_hcl_to_json.py | 3 + test/unit/test_api.py | 7 ++- test/unit/test_with_meta.py | 96 +++++++++++++++++++++++++++++++ 9 files changed, 123 insertions(+), 8 deletions(-) create mode 100644 test/unit/test_with_meta.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 61dc143a..7d79ac6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## \[Unreleased\] -- Nothing yet. +### Fixed + +- `with_meta` emits `__start_line__` and `__end_line__` again. The option, the `hcl2tojson --with-meta` flag and the migration guide's promise that the v7 keys are "still available" all survived the v8 rewrite; the code that produced the keys did not, leaving the option read nowhere in the package. Blocks are annotated with the same spans 7.3.1 produced for the same input. ([#291](https://github.com/amplify-education/python-hcl2/issues/291)) ## \[8.1.3\] - 2026-08-26 diff --git a/docs/01_getting_started.md b/docs/01_getting_started.md index 431fc06b..b9796433 100644 --- a/docs/01_getting_started.md +++ b/docs/01_getting_started.md @@ -70,7 +70,7 @@ data = loads(text, serialization_options=SerializationOptions( | Field | Type | Default | Description | |---|---|---|-------------------------------------------------------------------------------------------------------------------------------------------------| | `with_comments` | `bool` | `True` | Include comments as `__comments__` and `__inline_comments__` keys (see [Comment Format](#comment-format)) | -| `with_meta` | `bool` | `False` | Add `__start_line__` / `__end_line__` metadata | +| `with_meta` | `bool` | `False` | Add `__start_line__` / `__end_line__` metadata to each block, alongside its attributes. Attributes carry no metadata of their own. | | `wrap_objects` | `bool` | `False` | Wrap object values as inline HCL2 strings | | `wrap_tuples` | `bool` | `False` | Wrap tuple values as inline HCL2 strings | | `explicit_blocks` | `bool` | `True` | Add `__is_block__: True` markers to blocks. **Mandatory for JSON->HCL2 deserialization and reconstruction.** | diff --git a/hcl2/const.py b/hcl2/const.py index 555c56aa..83a9a926 100644 --- a/hcl2/const.py +++ b/hcl2/const.py @@ -3,3 +3,5 @@ IS_BLOCK = "__is_block__" COMMENTS_KEY = "__comments__" INLINE_COMMENTS_KEY = "__inline_comments__" +START_LINE = "__start_line__" +END_LINE = "__end_line__" diff --git a/hcl2/deserializer.py b/hcl2/deserializer.py index 667e0b20..833d3441 100644 --- a/hcl2/deserializer.py +++ b/hcl2/deserializer.py @@ -9,7 +9,7 @@ from regex import regex -from hcl2.const import COMMENTS_KEY, INLINE_COMMENTS_KEY, IS_BLOCK +from hcl2.const import COMMENTS_KEY, END_LINE, INLINE_COMMENTS_KEY, IS_BLOCK, START_LINE from hcl2.parser import parser as _get_parser from hcl2.rules.abstract import LarkElement, LarkRule from hcl2.rules.base import ( @@ -369,7 +369,7 @@ def _deserialize_object_elem(self, key: Any, value: Any) -> ObjectElemRule: def _is_reserved_key(self, key: str) -> bool: """Check if a key is a reserved metadata key that should be skipped during deserialization.""" - return key in (IS_BLOCK, COMMENTS_KEY, INLINE_COMMENTS_KEY) + return key in (IS_BLOCK, COMMENTS_KEY, INLINE_COMMENTS_KEY, START_LINE, END_LINE) def _is_expression(self, value: Any) -> bool: return isinstance(value, str) and value.startswith("${") and value.endswith("}") diff --git a/hcl2/rules/base.py b/hcl2/rules/base.py index 625bd835..e54defe8 100644 --- a/hcl2/rules/base.py +++ b/hcl2/rules/base.py @@ -5,7 +5,7 @@ from lark.tree import Meta -from hcl2.const import INLINE_COMMENTS_KEY, IS_BLOCK +from hcl2.const import END_LINE, INLINE_COMMENTS_KEY, IS_BLOCK, START_LINE from hcl2.rules.abstract import LarkRule, LarkToken from hcl2.rules.expressions import ExprTermRule from hcl2.rules.literal_rules import IdentifierRule @@ -152,6 +152,13 @@ def serialize(self, options=SerializationOptions(), context=SerializationContext result = self._body.serialize(options) if options.explicit_blocks: result.update({IS_BLOCK: True}) + if options.with_meta: + # Alongside the body, not wrapping it: the keys land on the same + # innermost dict the labels nest around, which is where v7 put them. + # A tree built by the deserializer carries no positions, so an empty + # Meta means "no line numbers to report" rather than line zero. + if not self._meta.empty: + result.update({START_LINE: self._meta.line, END_LINE: self._meta.end_line}) labels = self._labels for label in reversed(labels[1:]): diff --git a/hcl2/utils.py b/hcl2/utils.py index 6e79f007..ff338589 100644 --- a/hcl2/utils.py +++ b/hcl2/utils.py @@ -20,7 +20,9 @@ class SerializationOptions: # Include __comments__ and __inline_comments__ keys in the output. with_comments: bool = True - # Add __start_line__ and __end_line__ metadata to each block/attribute. + # Add __start_line__ and __end_line__ metadata to each block. Attributes get + # none: an attribute serializes to its own {name: value} pair, which has + # nowhere to hang the keys without changing the shape of the value. with_meta: bool = False # Serialize nested objects as inline HCL strings (e.g. "${{key = value}}") # instead of Python dicts. diff --git a/test/unit/cli/test_hcl_to_json.py b/test/unit/cli/test_hcl_to_json.py index 3b41e606..497d9051 100644 --- a/test/unit/cli/test_hcl_to_json.py +++ b/test/unit/cli/test_hcl_to_json.py @@ -120,6 +120,9 @@ def test_with_meta_flag(self): result = json.loads(stdout.getvalue()) self.assertIn("resource", result) + body = result["resource"][0]['"a"']['"b"'] + self.assertEqual(body["__start_line__"], 1) + self.assertEqual(body["__end_line__"], 3) def test_no_comments_flag(self): hcl_with_comment = "# a comment\nx = 1\n" diff --git a/test/unit/test_api.py b/test/unit/test_api.py index d6599fec..1f16aba9 100644 --- a/test/unit/test_api.py +++ b/test/unit/test_api.py @@ -48,8 +48,11 @@ def test_with_serialization_options(self): def test_with_meta_option(self): result = loads(BLOCK_HCL, serialization_options=SerializationOptions(with_meta=True)) self.assertIn("resource", result) - # Verify the option is accepted and produces a dict with expected content - self.assertIsInstance(result, dict) + # Assert on the metadata itself, not just that the option is accepted: + # this test passed throughout #291, when the option emitted nothing. + body = result["resource"][0]['"aws_instance"']['"example"'] + self.assertEqual(body["__start_line__"], 1) + self.assertEqual(body["__end_line__"], 3) def test_block_parsing(self): result = loads(BLOCK_HCL) diff --git a/test/unit/test_with_meta.py b/test/unit/test_with_meta.py new file mode 100644 index 00000000..dfd9cbc5 --- /dev/null +++ b/test/unit/test_with_meta.py @@ -0,0 +1,96 @@ +# pylint: disable=C0103,C0114,C0115,C0116 +"""Regression tests for GH issue #291: `with_meta` produced no metadata. + +`SerializationOptions.with_meta` documents `__start_line__` and `__end_line__` +keys, `hcl2tojson` exposes it as `--with-meta`, and the v8 migration guide says +the v7 keys "are still available". None of that was true: v7 emitted the keys +from `RuleTransformer.block`, and the v8 rewrite moved block serialization to +`BlockRule.serialize` without carrying them over, leaving the option read +nowhere in the package. + +Line numbers below were checked against python-hcl2 7.3.1 on the same input, +so the values are v7's, not merely self-consistent. +""" + +from unittest import TestCase + +from hcl2.api import dumps, from_dict, loads, serialize +from hcl2.const import END_LINE, IS_BLOCK, START_LINE +from hcl2.utils import SerializationOptions + +_META = SerializationOptions(with_meta=True) + +NESTED_HCL = """resource "aws_instance" "web" { + ami = "ami-1" + + network_interface { + device_index = 0 + } +} + +variable "x" {} +""" + + +class TestWithMetaEmitsLineNumbers(TestCase): + def test_block_carries_its_line_span(self): + result = loads(NESTED_HCL, serialization_options=_META) + body = result["resource"][0]['"aws_instance"']['"web"'] + self.assertEqual(body[START_LINE], 1) + self.assertEqual(body[END_LINE], 7) + + def test_nested_block_carries_its_own_span(self): + result = loads(NESTED_HCL, serialization_options=_META) + body = result["resource"][0]['"aws_instance"']['"web"'] + interface = body["network_interface"][0] + self.assertEqual(interface[START_LINE], 4) + self.assertEqual(interface[END_LINE], 6) + + def test_empty_block_spans_one_line(self): + result = loads(NESTED_HCL, serialization_options=_META) + body = result["variable"][0]['"x"'] + self.assertEqual(body[START_LINE], 9) + self.assertEqual(body[END_LINE], 9) + + def test_off_by_default(self): + result = loads(NESTED_HCL) + body = result["resource"][0]['"aws_instance"']['"web"'] + self.assertNotIn(START_LINE, body) + self.assertNotIn(END_LINE, body) + + def test_attributes_get_no_metadata(self): + # An attribute serializes to its own {name: value} pair, so there is + # nowhere to put the keys. v7 did not annotate attributes either. + result = loads("x = 1\n", serialization_options=_META) + self.assertEqual(result, {"x": 1}) + + def test_independent_of_explicit_blocks(self): + options = SerializationOptions(with_meta=True, explicit_blocks=False) + result = loads(NESTED_HCL, serialization_options=options) + body = result["resource"][0]['"aws_instance"']['"web"'] + self.assertNotIn(IS_BLOCK, body) + self.assertEqual(body[START_LINE], 1) + + +class TestWithMetaRoundTrip(TestCase): + """The keys are metadata, so `dumps()` must not write them back as HCL.""" + + def test_metadata_keys_are_not_emitted_as_attributes(self): + data = loads(NESTED_HCL, serialization_options=_META) + hcl = dumps(data) + self.assertNotIn(START_LINE, hcl) + self.assertNotIn(END_LINE, hcl) + + def test_round_trip_matches_output_without_metadata(self): + with_meta = dumps(loads(NESTED_HCL, serialization_options=_META)) + without = dumps(loads(NESTED_HCL)) + self.assertEqual(with_meta, without) + + def test_a_tree_without_positions_reports_no_lines(self): + # A tree built by the deserializer carries an empty Meta. Asking for + # metadata there must skip the keys rather than raise or invent zeros. + tree = from_dict({"resource": [{"aws_instance": {"web": {IS_BLOCK: True}}}]}) + result = serialize(tree, serialization_options=_META) + body = result["resource"][0]["aws_instance"]["web"] + self.assertNotIn(START_LINE, body) + self.assertNotIn(END_LINE, body) From a5f3372f53736726b4e60fc72d73dfd31051bf16 Mon Sep 17 00:00:00 2001 From: Tim Date: Mon, 31 Aug 2026 15:21:23 -0700 Subject: [PATCH 2/3] feat: expose a block's line span on BlockView `with_meta` puts `__start_line__` and `__end_line__` in the serialized output, which is the right place for a dict but an awkward one for a query: reading a block's span meant serializing the block and descending past however many labels it has, or reaching into the rule's private `_meta`. Both are what a downstream Terraform block reader ended up doing. Add `start_line` and `end_line` to `BlockView`. They report the same numbers `with_meta` serializes -- asserted against it rather than restated -- and are `None` for a tree built by the deserializer, which carries no positions and whose empty `Meta` has no line attributes at all. `hq` picks them up through its existing property accessors, so `hq 'resource[*] | .start_line' main.tf` works with no further wiring. --- CHANGELOG.md | 4 ++++ docs/02_querying.md | 4 ++++ docs/04_hq.md | 2 +- hcl2/query/blocks.py | 29 ++++++++++++++++++++++++ test/unit/query/test_blocks.py | 40 ++++++++++++++++++++++++++++++++++ 5 files changed, 78 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d79ac6b..e4e5d460 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## \[Unreleased\] +### Added + +- `BlockView.start_line` and `BlockView.end_line`, so a block's span can be read from the query API without serializing it. `with_meta` puts the numbers in the output dict, which meant reaching them through the label nesting, or through the rule's private `_meta`. Both are `None` for a tree built by the deserializer, which carries no positions. `hq` picks them up through its property accessors: `hq 'resource[*] | .start_line' main.tf`. + ### Fixed - `with_meta` emits `__start_line__` and `__end_line__` again. The option, the `hcl2tojson --with-meta` flag and the migration guide's promise that the v7 keys are "still available" all survived the v8 rewrite; the code that produced the keys did not, leaving the option read nowhere in the package. Blocks are annotated with the same spans 7.3.1 produced for the same input. ([#291](https://github.com/amplify-education/python-hcl2/issues/291)) diff --git a/docs/02_querying.md b/docs/02_querying.md index eacbd3d7..59344c08 100644 --- a/docs/02_querying.md +++ b/docs/02_querying.md @@ -56,6 +56,8 @@ block.block_type # "resource" block.labels # ["resource", "aws_instance", "main"] block.name_labels # ["aws_instance", "main"] block.body # BodyView +block.start_line # 1 +block.end_line # 12 ``` | Property / Method | Returns | Description | @@ -64,6 +66,8 @@ block.body # BodyView | `labels` | `List[str]` | All labels as plain strings | | `name_labels` | `List[str]` | Labels after the block type (`labels[1:]`) | | `body` | `BodyView` | The block body | +| `start_line` | `int \| None` | Line the block opens on; `None` for a tree with no positions | +| `end_line` | `int \| None` | Line the block closes on; `None` for a tree with no positions | | `blocks(...)` | `List[BlockView]` | Nested blocks (delegates to body) | | `attributes(...)` | `List[AttributeView]` | Nested attributes (delegates to body) | | `attribute(name)` | `AttributeView \| None` | Single nested attribute | diff --git a/docs/04_hq.md b/docs/04_hq.md index 96b4e20b..c2337c98 100644 --- a/docs/04_hq.md +++ b/docs/04_hq.md @@ -98,7 +98,7 @@ hq 'x | length' file.tf --value | View Type | Available Properties | |---|---| -| `BlockView` | `.block_type` (e.g. `"resource"`), `.labels` (all labels including type), `.name_labels` (labels after the block type, e.g. `["aws_instance", "main"]`) | +| `BlockView` | `.block_type` (e.g. `"resource"`), `.labels` (all labels including type), `.name_labels` (labels after the block type, e.g. `["aws_instance", "main"]`), `.start_line` / `.end_line` (the block's span in the file) | | `AttributeView` | `.name` (attribute name), `.value` (serialized value) | | `FunctionCallView` | `.name` (function name), `.args` (argument list), `.has_ellipsis` | | `ForTupleView` | `.iterator_name`, `.second_iterator_name`, `.iterable`, `.value_expr`, `.has_condition`, `.condition` | diff --git a/hcl2/query/blocks.py b/hcl2/query/blocks.py index 269f2209..647f8e46 100644 --- a/hcl2/query/blocks.py +++ b/hcl2/query/blocks.py @@ -11,6 +11,19 @@ from hcl2.utils import SerializationOptions +def _meta_line(node: BlockRule, attribute: str) -> Optional[int]: + """Read a line number off a block's lark ``Meta``, or None when it has none. + + A tree built by the deserializer rather than the parser carries an empty + ``Meta``, whose line attributes do not exist at all. + """ + meta = node._meta # pylint: disable=protected-access + if meta.empty: + return None + line: int = getattr(meta, attribute) + return line + + def _label_to_str(label) -> str: """Convert a block label (IdentifierRule or StringRule) to a plain string.""" if isinstance(label, IdentifierRule): @@ -53,6 +66,22 @@ def name_labels(self) -> List[str]: """Return labels after the block type (labels[1:]) as plain strings.""" return self.labels[1:] + @property + def start_line(self) -> Optional[int]: + """Return the line the block opens on, or None if it has no position. + + The same number ``with_meta`` reports as ``__start_line__``, without + serializing the block to get at it. + """ + node: BlockRule = self._node # type: ignore[assignment] + return _meta_line(node, "line") + + @property + def end_line(self) -> Optional[int]: + """Return the line the block closes on, or None if it has no position.""" + node: BlockRule = self._node # type: ignore[assignment] + return _meta_line(node, "end_line") + @property def body(self) -> "NodeView": """Return the block body as a BodyView.""" diff --git a/test/unit/query/test_blocks.py b/test/unit/query/test_blocks.py index ae87f3c9..5c4c75b8 100644 --- a/test/unit/query/test_blocks.py +++ b/test/unit/query/test_blocks.py @@ -118,3 +118,43 @@ def test_no_adjacent_comments(self): block = doc.blocks("resource")[0] result = block.to_dict(options=self._OPTS) self.assertNotIn("__comments__", result) + + +class TestBlockViewLines(TestCase): + """`start_line` / `end_line` report the span `with_meta` serializes. + + The line numbers were reachable only by serializing the block with + `with_meta=True` and digging past the label nesting, or by reading the + rule's private `_meta`. Both are what these properties replace. + """ + + NESTED = 'resource "aws_instance" "web" {\n ami = "ami-1"\n\n network_interface {\n x = 0\n }\n}\n' + + def test_span_of_a_top_level_block(self): + block = DocumentView.parse(self.NESTED).blocks("resource")[0] + self.assertEqual((block.start_line, block.end_line), (1, 7)) + + def test_span_of_a_nested_block(self): + block = DocumentView.parse(self.NESTED).blocks("resource")[0] + nested = block.blocks("network_interface")[0] + self.assertEqual((nested.start_line, nested.end_line), (4, 6)) + + def test_an_empty_block_spans_one_line(self): + block = DocumentView.parse('variable "x" {}\n').blocks("variable")[0] + self.assertEqual((block.start_line, block.end_line), (1, 1)) + + def test_agrees_with_with_meta(self): + block = DocumentView.parse(self.NESTED).blocks("resource")[0] + body = block.to_dict(options=SerializationOptions(with_meta=True))['"aws_instance"']['"web"'] + self.assertEqual(block.start_line, body["__start_line__"]) + self.assertEqual(block.end_line, body["__end_line__"]) + + def test_a_block_without_a_position_reports_none(self): + # A tree built by the deserializer carries an empty Meta. + from hcl2.api import from_dict + from hcl2.query.body import DocumentView as Doc + + tree = from_dict({"resource": [{"aws_instance": {"web": {"__is_block__": True}}}]}) + block = Doc(tree).blocks("resource")[0] + self.assertIsNone(block.start_line) + self.assertIsNone(block.end_line) From f856730d888e085b70574bad60148aa82e39b8d4 Mon Sep 17 00:00:00 2001 From: Tim Date: Tue, 1 Sep 2026 16:04:54 -0700 Subject: [PATCH 3/3] test: pin what happens to an attribute named like a metadata key The keys travel in-band, in the same dict as the block's attributes, which is where v7 put them. That means the deserializer cannot tell a key it wrote from one the document declared, so `dumps(loads(...))` drops an attribute genuinely named `__start_line__`, and `with_meta` overwrites its value -- exactly as both already happened for `__is_block__`, `__comments__` and `__inline_comments__`. These tests state that rather than leave it to be discovered. Nothing short of moving all five keys out of band would make the metadata unambiguous, and that is a breaking change to the serialized shape, not a fix belonging to this option. --- test/unit/test_with_meta.py | 39 ++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/test/unit/test_with_meta.py b/test/unit/test_with_meta.py index dfd9cbc5..8834d167 100644 --- a/test/unit/test_with_meta.py +++ b/test/unit/test_with_meta.py @@ -15,7 +15,7 @@ from unittest import TestCase from hcl2.api import dumps, from_dict, loads, serialize -from hcl2.const import END_LINE, IS_BLOCK, START_LINE +from hcl2.const import COMMENTS_KEY, END_LINE, INLINE_COMMENTS_KEY, IS_BLOCK, START_LINE from hcl2.utils import SerializationOptions _META = SerializationOptions(with_meta=True) @@ -94,3 +94,40 @@ def test_a_tree_without_positions_reports_no_lines(self): body = result["resource"][0]["aws_instance"]["web"] self.assertNotIn(START_LINE, body) self.assertNotIn(END_LINE, body) + + +class TestUserAttributesNamedLikeMetadata(TestCase): + """An attribute genuinely named `__start_line__` collides with the metadata. + + The keys are carried in-band, in the same dict as the block's attributes, + which is where v7 put them and what the migration guide promises. That has + a cost: the deserializer cannot tell a metadata key it wrote from an + attribute the document really declared, so it drops both -- exactly as it + already dropped `__is_block__` and `__comments__` before these two keys + existed. `with_meta` additionally overwrites such an attribute. + + These pin the behaviour rather than bless it. Anything that made the + metadata unambiguous would have to move all five keys out of band, which is + a breaking change to the serialized shape, not a fix to this option. + """ + + RESERVED = (START_LINE, END_LINE, IS_BLOCK, COMMENTS_KEY, INLINE_COMMENTS_KEY) + + def test_a_reserved_name_does_not_survive_a_round_trip(self): + for key in self.RESERVED: + with self.subTest(key=key): + hcl = f'block "a" {{\n {key} = 99\n keep = 1\n}}\n' + written = dumps(loads(hcl)) + self.assertNotIn(key, written) + self.assertIn("keep", written) + + def test_with_meta_overwrites_an_attribute_of_the_same_name(self): + hcl = 'block "a" {\n __start_line__ = 99\n}\n' + body = loads(hcl, serialization_options=_META)["block"][0]['"a"'] + self.assertEqual(body[START_LINE], 1) + + def test_an_ordinary_dunder_attribute_is_untouched(self): + # Only the five names are reserved; nothing about the leading + # underscores makes an attribute metadata. + hcl = 'block "a" {\n __line__ = 99\n}\n' + self.assertIn("__line__", dumps(loads(hcl)))