From 1265fc66b04a6cd9802f7572c8c2d9fcd4b6383a Mon Sep 17 00:00:00 2001 From: Tim Date: Tue, 1 Sep 2026 21:19:54 -0700 Subject: [PATCH 01/11] feat: carry serializer metadata beside the mapping (#331) `__is_block__`, `__comments__` and `__inline_comments__` are the serializer's, but HCL reserves none of those names. A document may declare an attribute called any of them, and in-band one of the two has to lose: on read the marker overwrites the attribute, on write `_is_reserved_key` drops it, and by then the dict holds one value with nothing to say which happened. `metadata_sidecar=True` puts the three on the object instead. `loads` returns an `HclDict` -- a `dict` subclass, so equality, iteration, `json.dumps` and everything else behave as before -- whose `hcl_meta` carries what used to sit among the keys. The mapping then holds attributes and nothing else, and there is nothing left to collide with. `dumps` reads whichever form it is handed, so a dict built by hand with the old keys still writes, and a document round-trips through either. Off by default, for two reasons worth stating rather than discovering: the keys are a documented part of the output shape, and JSON cannot carry a sidecar -- `json.dumps` of an `HclDict` yields the attributes alone. Anyone serializing to JSON wants the in-band form. --- CHANGELOG.md | 4 +- hcl2/deserializer.py | 32 ++++++-- hcl2/meta.py | 61 ++++++++++++++ hcl2/rules/base.py | 18 ++++- hcl2/utils.py | 6 ++ test/unit/test_metadata_sidecar.py | 125 +++++++++++++++++++++++++++++ 6 files changed, 235 insertions(+), 11 deletions(-) create mode 100644 hcl2/meta.py create mode 100644 test/unit/test_metadata_sidecar.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 61dc143a..85e8751f 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. +### Added + +- `SerializationOptions.metadata_sidecar`, which carries `__is_block__`, `__comments__` and `__inline_comments__` beside the mapping rather than among its keys. HCL reserves none of those names, so a document may declare an attribute called any of them -- and in-band one of the two has to lose: on read the marker overwrites the attribute, on write the deserializer drops it, and by then the dict holds a single value with no way to tell which happened. With the option set, `loads` returns an `HclDict`, a `dict` subclass whose `hcl_meta` holds the three, so the mapping contains attributes and nothing else. `dumps` accepts either form, including a hand-built dict using the old keys. Off by default: the keys are a documented part of the output shape, and JSON cannot carry a sidecar. ([#331](https://github.com/amplify-education/python-hcl2/issues/331)) ## \[8.1.3\] - 2026-08-26 diff --git a/hcl2/deserializer.py b/hcl2/deserializer.py index 667e0b20..505bbf0c 100644 --- a/hcl2/deserializer.py +++ b/hcl2/deserializer.py @@ -10,6 +10,7 @@ from regex import regex from hcl2.const import COMMENTS_KEY, INLINE_COMMENTS_KEY, IS_BLOCK +from hcl2.meta import meta_of from hcl2.parser import parser as _get_parser from hcl2.rules.abstract import LarkElement, LarkRule from hcl2.rules.base import ( @@ -144,7 +145,7 @@ def _deserialize_block_elements(self, value: dict) -> List[LarkElement]: else: # otherwise it's just an attribute - if not self._is_reserved_key(key): + if not self._is_reserved_key(key, value): children.append(self._deserialize_attribute(key, val)) return children @@ -294,8 +295,8 @@ def _deserialize_block(self, first_label: str, value: dict) -> BlockRule: body = value # Keep peeling off single-key layers until we hit the body (dict with IS_BLOCK) - while isinstance(body, dict) and not body.get(IS_BLOCK): - non_block_keys = [k for k in body.keys() if not self._is_reserved_key(k)] + while isinstance(body, dict) and not self._is_marked_block(body): + non_block_keys = [k for k in body.keys() if not self._is_reserved_key(k, body)] if len(non_block_keys) == 1: # This is another label level label = non_block_keys[0] @@ -367,10 +368,23 @@ def _deserialize_object_elem(self, key: Any, value: Any) -> ObjectElemRule: return ObjectElemRule(result) - def _is_reserved_key(self, key: str) -> bool: - """Check if a key is a reserved metadata key that should be skipped during deserialization.""" + def _is_reserved_key(self, key: str, container: Optional[dict] = None) -> bool: + """Whether *key* in *container* is metadata rather than an attribute. + + A container carrying its metadata beside the mapping reserves nothing: + every key in it is an attribute the document declared, including one + spelled `__is_block__`. Only the in-band form has to reserve the names, + and only there can it lose an attribute to one. + """ + if container is not None and meta_of(container) is not None: + return False return key in (IS_BLOCK, COMMENTS_KEY, INLINE_COMMENTS_KEY) + def _is_marked_block(self, body: dict) -> bool: + """Whether *body* is itself a block, in whichever form marks it.""" + meta = meta_of(body) + return meta.is_block if meta is not None else bool(body.get(IS_BLOCK)) + def _is_expression(self, value: Any) -> bool: return isinstance(value, str) and value.startswith("${") and value.endswith("}") @@ -387,8 +401,12 @@ def _is_block(self, value: Any) -> bool: return False def _contains_block_marker(self, obj: dict) -> bool: - """Recursively check if a dict contains IS_BLOCK marker anywhere""" - if obj.get(IS_BLOCK): + """Recursively check whether a dict is marked as a block, in either form""" + meta = meta_of(obj) + if meta is not None: + if meta.is_block: + return True + elif obj.get(IS_BLOCK): return True for value in obj.values(): if isinstance(value, dict) and self._contains_block_marker(value): diff --git a/hcl2/meta.py b/hcl2/meta.py new file mode 100644 index 00000000..d4575356 --- /dev/null +++ b/hcl2/meta.py @@ -0,0 +1,61 @@ +"""Out-of-band metadata for serialized bodies. + +The serializer has three things to say about a body that are not attributes of +it: that it is a block, what comments surround it, and which of those were +inline. They have always travelled as `__is_block__`, `__comments__` and +`__inline_comments__` keys in the same dict as the attributes, which works only +while no document declares an attribute by those names. HCL puts no such name +out of reach, so one that does loses either the attribute or the metadata, +silently and in both directions. + +`HclDict` carries them beside the mapping instead. It is a `dict`, so every +consumer that reads attributes keeps working unchanged, and `hcl_meta` holds +what used to sit among them. +""" + +from dataclasses import dataclass, field +from typing import Any, Dict, List, Optional + + +@dataclass +class HclMeta: + """What the serializer knows about a body that is not one of its attributes.""" + + is_block: bool = False + comments: List[dict] = field(default_factory=list) + inline_comments: List[dict] = field(default_factory=list) + + def is_empty(self) -> bool: + """Whether there is nothing here worth carrying.""" + return not (self.is_block or self.comments or self.inline_comments) + + +class HclDict(Dict[str, Any]): + """A dict whose HCL metadata lives on the object rather than among the keys. + + Equality, iteration, `json.dumps` and every other mapping operation behave + exactly as `dict` does -- the metadata is deliberately not part of the + mapping, so a document declaring an attribute called `__is_block__` gets + that attribute back and nothing else. + + JSON cannot carry the sidecar. Serializing an `HclDict` yields the + attributes alone, which is why the in-band keys remain the default. + """ + + __slots__ = ("hcl_meta",) + + def __init__(self, *args: Any, meta: Optional[HclMeta] = None, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + self.hcl_meta = meta if meta is not None else HclMeta() + + def __repr__(self) -> str: + """Show the metadata, so a debugging session does not have to guess.""" + if self.hcl_meta.is_empty(): + return super().__repr__() + return f"{super().__repr__()} + {self.hcl_meta!r}" + + +def meta_of(value: Any) -> Optional[HclMeta]: + """Return the metadata carried beside *value*, or None if it carries none.""" + meta = getattr(value, "hcl_meta", None) + return meta if isinstance(meta, HclMeta) else None diff --git a/hcl2/rules/base.py b/hcl2/rules/base.py index 625bd835..367a83f1 100644 --- a/hcl2/rules/base.py +++ b/hcl2/rules/base.py @@ -5,7 +5,8 @@ from lark.tree import Meta -from hcl2.const import INLINE_COMMENTS_KEY, IS_BLOCK +from hcl2.const import COMMENTS_KEY, INLINE_COMMENTS_KEY, IS_BLOCK +from hcl2.meta import HclDict, HclMeta, meta_of from hcl2.rules.abstract import LarkRule, LarkToken from hcl2.rules.expressions import ExprTermRule from hcl2.rules.literal_rules import IdentifierRule @@ -87,9 +88,16 @@ def serialize(self, options=SerializationOptions(), context=SerializationContext if child_comments: comments.extend(child_comments) + if options.metadata_sidecar: + meta = HclMeta() + if options.with_comments: + meta.comments = comments + meta.inline_comments = inline_comments + return HclDict(result.items(), meta=meta) + if options.with_comments: if comments: - result["__comments__"] = comments + result[COMMENTS_KEY] = comments if inline_comments: result[INLINE_COMMENTS_KEY] = inline_comments @@ -151,7 +159,11 @@ def serialize(self, options=SerializationOptions(), context=SerializationContext """Serialize to a nested dict with labels as keys.""" result = self._body.serialize(options) if options.explicit_blocks: - result.update({IS_BLOCK: True}) + meta = meta_of(result) + if meta is not None: + meta.is_block = True + else: + result.update({IS_BLOCK: True}) labels = self._labels for label in reversed(labels[1:]): diff --git a/hcl2/utils.py b/hcl2/utils.py index 6e79f007..08dc0977 100644 --- a/hcl2/utils.py +++ b/hcl2/utils.py @@ -31,6 +31,12 @@ class SerializationOptions: # Add __is_block__ markers to distinguish blocks from plain objects. # Note: round-trip through from_dict/dumps is NOT supported WITHOUT this option. explicit_blocks: bool = True + # Carry the metadata keys beside the mapping instead of among its keys, as + # `HclDict.hcl_meta`. The in-band keys collide with any attribute a document + # happens to name `__is_block__`, `__comments__` or `__inline_comments__`; + # the sidecar cannot. Off by default because the keys are a documented part + # of the output shape, and because JSON cannot carry the sidecar. + metadata_sidecar: bool = False # Keep heredoc syntax (< Date: Tue, 1 Sep 2026 21:43:43 -0700 Subject: [PATCH 02/11] fix: copying an HclDict keeps its metadata `dict.copy` returns a plain `dict`, so an inherited copy dropped the sidecar and the block was then written as an object. `document.copy()` before modifying is ordinary enough that losing block metadata to it would be a trap, and the in-band form has no such edge -- its metadata is among the keys, so a copy carries it for free. `copy()`, `copy.copy`, `copy.deepcopy` and pickling all carry it now. `dict(hcl_dict)` deliberately does not: asking for a `dict` gives the mapping and nothing else. --- hcl2/meta.py | 35 ++++++++++++++++++++++++- test/unit/test_metadata_sidecar.py | 42 ++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/hcl2/meta.py b/hcl2/meta.py index d4575356..ff3d026d 100644 --- a/hcl2/meta.py +++ b/hcl2/meta.py @@ -13,8 +13,9 @@ what used to sit among them. """ +import copy as copy_module from dataclasses import dataclass, field -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Tuple @dataclass @@ -54,8 +55,40 @@ def __repr__(self) -> str: return super().__repr__() return f"{super().__repr__()} + {self.hcl_meta!r}" + def copy(self) -> "HclDict": + """Copy the mapping and the metadata together. + + `dict.copy` returns a plain `dict`, which would drop the sidecar -- + and `document = document.copy()` is ordinary enough that losing block + metadata to it would be a trap. The in-band form survives a copy + because its metadata is among the keys; this has to say so explicitly. + """ + return HclDict(self, meta=copy_module.copy(self.hcl_meta)) + + def __copy__(self) -> "HclDict": + """Same for `copy.copy`.""" + return self.copy() + + def __deepcopy__(self, memo: dict) -> "HclDict": + """Same for `copy.deepcopy`, metadata included.""" + duplicate = HclDict( + {key: copy_module.deepcopy(value, memo) for key, value in self.items()}, + meta=copy_module.deepcopy(self.hcl_meta, memo), + ) + memo[id(self)] = duplicate + return duplicate + + def __reduce__(self) -> Tuple[Any, ...]: + """Carry the metadata through pickling, which `dict` would not.""" + return (_rebuild, (dict(self), self.hcl_meta)) + def meta_of(value: Any) -> Optional[HclMeta]: """Return the metadata carried beside *value*, or None if it carries none.""" meta = getattr(value, "hcl_meta", None) return meta if isinstance(meta, HclMeta) else None + + +def _rebuild(items: Dict[str, Any], meta: HclMeta) -> HclDict: + """Reconstruct an `HclDict` from its pickled parts.""" + return HclDict(items, meta=meta) diff --git a/test/unit/test_metadata_sidecar.py b/test/unit/test_metadata_sidecar.py index f9f3afb5..dc3f9772 100644 --- a/test/unit/test_metadata_sidecar.py +++ b/test/unit/test_metadata_sidecar.py @@ -11,7 +11,9 @@ holds attributes and nothing else, so there is nothing to collide with. """ +import copy import json +import pickle from unittest import TestCase from hcl2.api import dumps, loads @@ -123,3 +125,43 @@ def test_metadata_still_arrives_in_band(self): body = loads('resource "a" "b" {\n x = 1\n}\n')["resource"][0]['"a"']['"b"'] self.assertTrue(body[IS_BLOCK]) self.assertIsNone(meta_of(body)) + + +class TestCopyingCarriesTheSidecar(TestCase): + """`document.copy()` is ordinary enough that losing metadata to it is a trap. + + `dict.copy` returns a plain `dict`, so an inherited copy would drop the + sidecar and the block would then be written as an object. The in-band form + survives a copy for free, because its metadata is among the keys; this has + to say so explicitly. + """ + + def setUp(self): + self.document = loads('resource "a" "b" {\n x = 1\n}\n', serialization_options=SIDECAR) + self.body = self.document["resource"][0]['"a"']['"b"'] + + def test_the_dict_method(self): + duplicate = self.body.copy() + self.assertIsInstance(duplicate, HclDict) + self.assertTrue(meta_of(duplicate).is_block) + + def test_copy_copy(self): + self.assertTrue(meta_of(copy.copy(self.body)).is_block) + + def test_copy_deepcopy(self): + duplicate = copy.deepcopy(self.body) + self.assertTrue(meta_of(duplicate).is_block) + self.assertIsNot(meta_of(duplicate), meta_of(self.body)) + + def test_pickle(self): + self.assertTrue(meta_of(pickle.loads(pickle.dumps(self.body))).is_block) + + def test_a_copied_document_still_writes_a_block(self): + copied = copy.deepcopy(self.document) + self.assertEqual(dumps(copied), dumps(self.document)) + + def test_dict_of_it_is_a_plain_dict(self): + # Deliberate: asking for a `dict` gives the mapping, nothing else. + plain = dict(self.body) + self.assertIsNone(meta_of(plain)) + self.assertEqual(plain, {"x": 1}) From 09de3fb94a76efcc87c48c8fbd6d2534f5616308 Mon Sep 17 00:00:00 2001 From: Tim Date: Tue, 1 Sep 2026 22:20:26 -0700 Subject: [PATCH 03/11] fix: three holes a code review found in the sidecar The option moved every positional argument. `SerializationOptions` is not `kw_only`, and the field went in among the block options, so `SerializationOptions(True, False, False, False, True, False, False, True, False)` meant something different before and after -- silently, with no exception. It is appended now, and a test pins the order. Object literals were not covered. Only `BodyRule` learned the sidecar, so `x = { __is_block__ = true, keep = 1 }` still tripped the in-band branch: the object was read as a block, and `dumps` emitted `x = keep = 1`, which is not HCL. An object literal carries no metadata of its own, but it has to say so in the same form a body does -- otherwise the option makes the collision worse than it was. `BlockView.to_dict` wrote the in-band comments key onto a dict carrying a sidecar. Nothing reserves that name there any more, so `dumps` emitted `__comments__ = [...]` as real HCL, which does not re-parse; and the merge read back an empty list, because the block's own comments had moved to the meta. Neither list was complete. It now writes to whichever form the dict is carrying. --- hcl2/query/blocks.py | 15 +++++- hcl2/rules/containers.py | 8 ++++ hcl2/utils.py | 16 ++++--- test/unit/test_metadata_sidecar.py | 75 ++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 8 deletions(-) diff --git a/hcl2/query/blocks.py b/hcl2/query/blocks.py index 269f2209..0142b234 100644 --- a/hcl2/query/blocks.py +++ b/hcl2/query/blocks.py @@ -3,6 +3,7 @@ from typing import Any, List, Optional from hcl2.const import COMMENTS_KEY +from hcl2.meta import meta_of from hcl2.query._base import NodeView, register_view from hcl2.rules.abstract import LarkElement from hcl2.rules.base import BlockRule @@ -72,8 +73,18 @@ def to_dict(self, options: Optional[SerializationOptions] = None) -> Any: ): # Place adjacent comments at the outer level of the block dict, # alongside the label keys — not drilled into the body dict. - existing = result.get(COMMENTS_KEY, []) - result[COMMENTS_KEY] = self._adjacent_comments + existing + # + # Whichever form the serializer used: writing the in-band key onto + # a dict carrying a sidecar would put it back among the attributes, + # where nothing reserves it any more, and `dumps` would emit it as + # real HCL. Reading `result.get(COMMENTS_KEY)` there would also + # find nothing, because the block's own comments are in the meta. + meta = meta_of(result) + if meta is not None: + meta.comments = self._adjacent_comments + meta.comments + else: + existing = result.get(COMMENTS_KEY, []) + result[COMMENTS_KEY] = self._adjacent_comments + existing return result def blocks(self, block_type: Optional[str] = None, *labels: str) -> List["NodeView"]: diff --git a/hcl2/rules/containers.py b/hcl2/rules/containers.py index 8b811ce8..25eb9d3d 100644 --- a/hcl2/rules/containers.py +++ b/hcl2/rules/containers.py @@ -2,6 +2,7 @@ from typing import Any, List, Optional, Tuple, Union +from hcl2.meta import HclDict from hcl2.rules.abstract import LarkRule from hcl2.rules.expressions import ExpressionRule from hcl2.rules.literal_rules import ( @@ -192,6 +193,13 @@ def serialize(self, options=SerializationOptions(), context=SerializationContext dict_result: dict = {} for element in self.elements: dict_result.update(element.serialize(options, context)) + if options.metadata_sidecar: + # An object literal has no metadata of its own, but it has to + # say so in the same form a body does. Left a plain dict, a key + # the document wrote as `__is_block__` reads back as the marker + # and the object is emitted as a block -- which is the very + # collision the option exists to remove. + return HclDict(dict_result) return dict_result with context.modify(inside_dollar_string=True): diff --git a/hcl2/utils.py b/hcl2/utils.py index 08dc0977..6d9a2a90 100644 --- a/hcl2/utils.py +++ b/hcl2/utils.py @@ -31,12 +31,6 @@ class SerializationOptions: # Add __is_block__ markers to distinguish blocks from plain objects. # Note: round-trip through from_dict/dumps is NOT supported WITHOUT this option. explicit_blocks: bool = True - # Carry the metadata keys beside the mapping instead of among its keys, as - # `HclDict.hcl_meta`. The in-band keys collide with any attribute a document - # happens to name `__is_block__`, `__comments__` or `__inline_comments__`; - # the sidecar cannot. Off by default because the keys are a documented part - # of the output shape, and because JSON cannot carry the sidecar. - metadata_sidecar: bool = False # Keep heredoc syntax (< str: + return dumps(loads(source, serialization_options=SIDECAR)) + + def test_each_reserved_name_survives_as_a_key(self): + for key in RESERVED: + with self.subTest(key=key): + written = self._round_trip(f"x = {{\n {key} = 99\n keep = 1\n}}\n") + self.assertIn(key, written) + self.assertIn("keep", written) + + def test_an_ordinary_object_is_unchanged(self): + self.assertEqual(self._round_trip("x = {\n a = 1\n}\n"), "x = {\n a = 1,\n}\n") + + +class TestTheOptionDidNotMoveTheOtherOnes(TestCase): + """`SerializationOptions` is not `kw_only`, so field order is a contract. + + Inserting the new field among the block options changed what every + positional argument after it meant -- silently, with no exception and no + test to catch it. It is appended instead. + """ + + def test_metadata_sidecar_is_last(self): + names = [f.name for f in dataclasses.fields(SerializationOptions)] + self.assertEqual(names[-1], "metadata_sidecar") + + def test_the_earlier_fields_keep_their_positions(self): + options = SerializationOptions(True, False, False, False, True, False, False, True, False) + self.assertFalse(options.force_operation_parentheses) + self.assertTrue(options.preserve_scientific_notation) + self.assertFalse(options.metadata_sidecar) + + +class TestTheQueryLayerWritesToTheSidecar(TestCase): + """`BlockView.to_dict` merges adjacent comments, and has to pick the form. + + Writing the in-band key onto a dict carrying a sidecar put it back among + the attributes, where nothing reserves it any more -- so `dumps` emitted + `__comments__ = [...]` as real HCL, which does not re-parse. Reading the + in-band key there also found nothing, because the block's own comments had + moved to the meta, so neither list was complete. + """ + + SOURCE = '# lead comment\nterraform {\n required_version = ">= 1.0"\n}\n' + + def _to_dict(self, options): + from hcl2.query import DocumentView + + return DocumentView.parse(self.SOURCE).blocks("terraform")[0].to_dict(options=options) + + def test_the_comment_lands_in_the_meta(self): + body = self._to_dict(SerializationOptions(metadata_sidecar=True, with_comments=True)) + self.assertEqual(meta_of(body).comments, [{"value": "lead comment"}]) + self.assertNotIn(COMMENTS_KEY, body) + + def test_the_block_still_writes_as_a_block(self): + body = self._to_dict(SerializationOptions(metadata_sidecar=True, with_comments=True)) + self.assertEqual(dumps({"terraform": [body]}), 'terraform {\n required_version = ">= 1.0"\n}\n') + + def test_the_in_band_form_is_unchanged(self): + body = self._to_dict(SerializationOptions(with_comments=True)) + self.assertEqual(body[COMMENTS_KEY], [{"value": "lead comment"}]) From 5557bfd662f8c0bd86b0df85ca715529f6ca1029 Mon Sep 17 00:00:00 2001 From: Tim Date: Tue, 1 Sep 2026 22:35:51 -0700 Subject: [PATCH 04/11] fix: no key name is reserved, including `meta` The constructor took keyword items, which reserved one: `HclDict(**{ "meta": "prod"})` swallowed the attribute and stored a string where the metadata goes, and `repr` then raised `AttributeError` on it. `meta` is a real attribute name in real configs -- Nomad meta stanzas, provider meta blocks -- so the one class whose purpose is that no key name is reserved was quietly reserving that one. It takes the mapping positionally now, and refuses a `meta=` that is not an `HclMeta` with a message saying how to store the key. `body | {...}` and `{...} | body` keep the metadata. `dict.__or__` returns a plain dict, so the idiomatic non-mutating edit would have dropped the sidecar and the block would then have been written as an object. `{**body}` cannot be helped -- unpacking always builds a plain dict and there is no hook for it -- so a test states that rather than leaving it to be found. `HclDict`, `HclMeta` and `meta_of` are exported from `hcl2`, which the CHANGELOG already implied by making the type part of the contract. --- CHANGELOG.md | 2 +- hcl2/__init__.py | 14 +++++--- hcl2/meta.py | 35 ++++++++++++++++-- test/unit/test_metadata_sidecar.py | 58 ++++++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85e8751f..53075785 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added -- `SerializationOptions.metadata_sidecar`, which carries `__is_block__`, `__comments__` and `__inline_comments__` beside the mapping rather than among its keys. HCL reserves none of those names, so a document may declare an attribute called any of them -- and in-band one of the two has to lose: on read the marker overwrites the attribute, on write the deserializer drops it, and by then the dict holds a single value with no way to tell which happened. With the option set, `loads` returns an `HclDict`, a `dict` subclass whose `hcl_meta` holds the three, so the mapping contains attributes and nothing else. `dumps` accepts either form, including a hand-built dict using the old keys. Off by default: the keys are a documented part of the output shape, and JSON cannot carry a sidecar. ([#331](https://github.com/amplify-education/python-hcl2/issues/331)) +- `SerializationOptions.metadata_sidecar`, which carries `__is_block__`, `__comments__` and `__inline_comments__` beside the mapping rather than among its keys. HCL reserves none of those names, so a document may declare an attribute called any of them -- and in-band one of the two has to lose: on read the marker overwrites the attribute, on write the deserializer drops it, and by then the dict holds a single value with no way to tell which happened. With the option set, `loads` returns an `HclDict`, a `dict` subclass whose `hcl_meta` holds the three, so the mapping contains attributes and nothing else. `dumps` accepts either form, including a hand-built dict using the old keys. Off by default: the keys are a documented part of the output shape, and JSON cannot carry a sidecar. `HclDict`, `HclMeta` and `meta_of` are exported from `hcl2`. Copying, merging with `|` and pickling carry the metadata; `dict(d)` and `{**d}` deliberately do not, since asking for a `dict` gives the mapping and nothing else. ([#331](https://github.com/amplify-education/python-hcl2/issues/331)) ## \[8.1.3\] - 2026-08-26 diff --git a/hcl2/__init__.py b/hcl2/__init__.py index 4bbdcd7e..14152251 100644 --- a/hcl2/__init__.py +++ b/hcl2/__init__.py @@ -24,27 +24,31 @@ from .builder import Builder from .deserializer import DeserializerOptions from .formatter import FormatterOptions +from .meta import HclDict, HclMeta, meta_of from .rules.base import StartRule from .utils import SerializationOptions __all__ = [ + "Builder", + "DeserializerOptions", "dump", "dumps", + "FormatterOptions", "from_dict", "from_json", + "HclDict", + "HclMeta", "load", "loads", + "meta_of", "parse", "parse_to_tree", "parses", "parses_to_tree", "query", "reconstruct", + "SerializationOptions", "serialize", - "transform", - "Builder", - "DeserializerOptions", - "FormatterOptions", "StartRule", - "SerializationOptions", + "transform", ] diff --git a/hcl2/meta.py b/hcl2/meta.py index ff3d026d..c8809e65 100644 --- a/hcl2/meta.py +++ b/hcl2/meta.py @@ -45,8 +45,21 @@ class HclDict(Dict[str, Any]): __slots__ = ("hcl_meta",) - def __init__(self, *args: Any, meta: Optional[HclMeta] = None, **kwargs: Any) -> None: - super().__init__(*args, **kwargs) + def __init__(self, *args: Any, meta: Optional[HclMeta] = None) -> None: + """Build from a mapping, with the metadata passed separately. + + No `**kwargs`: this is the one class whose whole point is that no key + name is reserved, and taking keyword items would reserve `meta` -- + `HclDict(**{"meta": "prod"})` would swallow the attribute and store a + string where the metadata goes. `meta` is a real name in real configs. + Pass the mapping positionally, as `dict` also allows. + """ + super().__init__(*args) + if meta is not None and not isinstance(meta, HclMeta): + raise TypeError( + "HclDict(meta=...) takes an HclMeta; to store a key called " + f"'meta', pass the mapping positionally: HclDict({{'meta': {meta!r}}})" + ) self.hcl_meta = meta if meta is not None else HclMeta() def __repr__(self) -> str: @@ -82,6 +95,24 @@ def __reduce__(self) -> Tuple[Any, ...]: """Carry the metadata through pickling, which `dict` would not.""" return (_rebuild, (dict(self), self.hcl_meta)) + def __or__(self, other: Any) -> "HclDict": + """Merge, keeping this side's metadata. + + `dict.__or__` returns a plain `dict`, so `body | {"size": ...}` -- the + idiomatic non-mutating edit -- would drop the sidecar and the block + would then be written as an object. `{**body, ...}` cannot be helped: + unpacking always builds a plain `dict`, and there is no hook for it. + """ + merged = HclDict(self, meta=copy_module.copy(self.hcl_meta)) + merged.update(other) + return merged + + def __ror__(self, other: Any) -> "HclDict": + """Same from the left, keeping this side's metadata.""" + merged = HclDict(other, meta=copy_module.copy(self.hcl_meta)) + merged.update(self) + return merged + def meta_of(value: Any) -> Optional[HclMeta]: """Return the metadata carried beside *value*, or None if it carries none.""" diff --git a/test/unit/test_metadata_sidecar.py b/test/unit/test_metadata_sidecar.py index fca6a337..343a86da 100644 --- a/test/unit/test_metadata_sidecar.py +++ b/test/unit/test_metadata_sidecar.py @@ -240,3 +240,61 @@ def test_the_block_still_writes_as_a_block(self): def test_the_in_band_form_is_unchanged(self): body = self._to_dict(SerializationOptions(with_comments=True)) self.assertEqual(body[COMMENTS_KEY], [{"value": "lead comment"}]) + + +class TestTheTypeIsPartOfThePublicSurface(TestCase): + """The CHANGELOG makes `HclDict` part of the contract, so it has to be reachable.""" + + def test_it_is_exported(self): + import hcl2 + + self.assertIs(hcl2.HclDict, HclDict) + self.assertIs(hcl2.HclMeta, HclMeta) + self.assertIs(hcl2.meta_of, meta_of) + + +class TestNoKeyNameIsReserved(TestCase): + """Including `meta`, which the constructor would otherwise have taken. + + `meta` is a real attribute name in real configs -- Nomad `meta` stanzas, + provider `meta` blocks -- and a class whose purpose is that no key name is + reserved cannot quietly reserve one. Taking keyword items would have: + `HclDict(**{"meta": "prod"})` swallowed the attribute and stored a string + where the metadata goes, and `repr` then raised `AttributeError`. + """ + + def test_a_key_called_meta_is_kept(self): + body = HclDict({"meta": '"prod"', "ami": '"a"'}, meta=HclMeta(is_block=True)) + self.assertEqual(body["meta"], '"prod"') + self.assertTrue(meta_of(body).is_block) + + def test_a_document_declaring_it_round_trips(self): + written = dumps(loads('block "a" {\n meta = "prod"\n}\n', serialization_options=SIDECAR)) + self.assertIn("meta", written) + + def test_passing_something_else_as_meta_is_refused(self): + with self.assertRaises(TypeError): + HclDict({"a": 1}, meta="prod") + + +class TestMergingKeepsTheSidecar(TestCase): + """`body | {...}` is the idiomatic non-mutating edit.""" + + def setUp(self): + self.body = loads('resource "aws_instance" "web" {\n ami = "a"\n}\n', serialization_options=SIDECAR)[ + "resource" + ][0]['"aws_instance"']['"web"'] + + def test_or_keeps_it(self): + merged = self.body | {"size": '"t2.micro"'} + self.assertTrue(meta_of(merged).is_block) + self.assertEqual(merged["size"], '"t2.micro"') + + def test_ror_keeps_it(self): + merged = {"first": 1} | self.body + self.assertTrue(meta_of(merged).is_block) + + def test_unpacking_cannot_keep_it(self): + # `{**body}` always builds a plain dict and there is no hook for it. + # Stated rather than left to be discovered. + self.assertIsNone(meta_of({**self.body})) From f8ece568a0c2fc06c121d09241a61cf8e39199bf Mon Sep 17 00:00:00 2001 From: Tim Date: Tue, 1 Sep 2026 22:37:33 -0700 Subject: [PATCH 05/11] chore: record the deliberate narrowing of __or__ `dict.__or__` is declared to return `dict`; these always return an `HclDict`, which mypy reads as an incompatible override. The ignore says which of the two it is. --- hcl2/meta.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/hcl2/meta.py b/hcl2/meta.py index c8809e65..1c324bcf 100644 --- a/hcl2/meta.py +++ b/hcl2/meta.py @@ -95,9 +95,13 @@ def __reduce__(self) -> Tuple[Any, ...]: """Carry the metadata through pickling, which `dict` would not.""" return (_rebuild, (dict(self), self.hcl_meta)) - def __or__(self, other: Any) -> "HclDict": + def __or__(self, other: Any) -> "HclDict": # type: ignore[override] """Merge, keeping this side's metadata. + Narrower than `dict.__or__`, which is declared to return `dict` for any + mapping: this always returns an `HclDict`, so the ignore records a + deliberate narrowing rather than a mismatch. + `dict.__or__` returns a plain `dict`, so `body | {"size": ...}` -- the idiomatic non-mutating edit -- would drop the sidecar and the block would then be written as an object. `{**body, ...}` cannot be helped: @@ -107,7 +111,7 @@ def __or__(self, other: Any) -> "HclDict": merged.update(other) return merged - def __ror__(self, other: Any) -> "HclDict": + def __ror__(self, other: Any) -> "HclDict": # type: ignore[override] """Same from the left, keeping this side's metadata.""" merged = HclDict(other, meta=copy_module.copy(self.hcl_meta)) merged.update(self) From 1cb3804c364bb3e15b9e4c366fe2d09f134cbc9c Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 2 Sep 2026 11:33:24 -0700 Subject: [PATCH 06/11] fix: register the deepcopy duplicate before copying into it HclDict.__deepcopy__ built the copy from a comprehension over the items and only then wrote memo[id(self)]. A mapping holding a reference back to itself therefore reached __deepcopy__ again with nothing recorded, and the descent ran to RecursionError -- while copy.deepcopy of the plain dict it subclasses handles the same shape and preserves the cycle. The duplicate now goes into the memo empty, before its metadata or any of its children are copied, which is what copy._deepcopy_dict does. Keys are copied as well as values, for the same parity. --- hcl2/meta.py | 18 +++++++++---- test/unit/test_metadata_sidecar.py | 43 ++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/hcl2/meta.py b/hcl2/meta.py index 1c324bcf..89c1ccee 100644 --- a/hcl2/meta.py +++ b/hcl2/meta.py @@ -83,12 +83,20 @@ def __copy__(self) -> "HclDict": return self.copy() def __deepcopy__(self, memo: dict) -> "HclDict": - """Same for `copy.deepcopy`, metadata included.""" - duplicate = HclDict( - {key: copy_module.deepcopy(value, memo) for key, value in self.items()}, - meta=copy_module.deepcopy(self.hcl_meta, memo), - ) + """Same for `copy.deepcopy`, metadata included. + + The duplicate is recorded in *memo* before anything inside it is + copied. A mapping may hold a reference back to itself, and copying + the children first means the recursion reaches this dict again with + nothing recorded, which does not terminate. `dict` registers its own + copy first for that reason; a subclass that did not would make a + cyclic document worse than the plain mapping it replaces. + """ + duplicate = HclDict() memo[id(self)] = duplicate + duplicate.hcl_meta = copy_module.deepcopy(self.hcl_meta, memo) + for key, value in self.items(): + duplicate[copy_module.deepcopy(key, memo)] = copy_module.deepcopy(value, memo) return duplicate def __reduce__(self) -> Tuple[Any, ...]: diff --git a/test/unit/test_metadata_sidecar.py b/test/unit/test_metadata_sidecar.py index 343a86da..adb12b4c 100644 --- a/test/unit/test_metadata_sidecar.py +++ b/test/unit/test_metadata_sidecar.py @@ -298,3 +298,46 @@ def test_unpacking_cannot_keep_it(self): # `{**body}` always builds a plain dict and there is no hook for it. # Stated rather than left to be discovered. self.assertIsNone(meta_of({**self.body})) + + +class TestDeepcopyHandlesACycle(TestCase): + """`dict` copies a self-referencing mapping; a subclass that did not would + make cyclic structures worse than the mapping it replaces. + + `copy.deepcopy` passes a memo so that a value reached twice is copied once. + Registering the duplicate in it has to happen before the children are + copied: a child holding a reference back to this dict otherwise arrives + with nothing memoised, and the descent does not terminate. + """ + + def test_a_self_reference_is_copied_rather_than_recursed(self): + body = HclDict({"x": 1}, meta=HclMeta(is_block=True)) + body["self"] = body + + duplicate = copy.deepcopy(body) + + self.assertIsNot(duplicate, body) + self.assertIs(duplicate["self"], duplicate) + self.assertEqual(duplicate["x"], 1) + self.assertTrue(meta_of(duplicate).is_block) + + def test_two_dicts_referring_to_each_other(self): + first = HclDict({"name": "first"}, meta=HclMeta(is_block=True)) + second = HclDict({"name": "second"}) + first["other"] = second + second["other"] = first + + duplicate = copy.deepcopy(first) + + self.assertIs(duplicate["other"]["other"], duplicate) + self.assertEqual(duplicate["other"]["name"], "second") + self.assertTrue(meta_of(duplicate).is_block) + + def test_a_dict_reached_twice_is_copied_once(self): + shared = HclDict({"n": 1}) + body = HclDict({"a": shared, "b": shared}, meta=HclMeta(is_block=True)) + + duplicate = copy.deepcopy(body) + + self.assertIs(duplicate["a"], duplicate["b"]) + self.assertIsNot(duplicate["a"], shared) From 5244630473b4acab7de5a47176ca9c6c353b0bb1 Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 23 Sep 2026 11:26:13 -0700 Subject: [PATCH 07/11] fix: pickle a cyclic HclDict, refuse non-dicts in |, own copied metadata Pickling passed dict(self) to the constructor, so a dict holding itself recursed until it failed; the items now go in the reduce tuple's dict-items slot, after the empty dict is memoised. `|` accepted a list of pairs where dict raises TypeError, and now returns NotImplemented for anything that is not a dict. A copy, and a merge, shared the metadata's comment lists with the original, so appending to one edited both. --- hcl2/meta.py | 37 +++++++++++++------ test/unit/test_metadata_sidecar.py | 57 ++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 10 deletions(-) diff --git a/hcl2/meta.py b/hcl2/meta.py index 89c1ccee..514b3c72 100644 --- a/hcl2/meta.py +++ b/hcl2/meta.py @@ -30,6 +30,15 @@ def is_empty(self) -> bool: """Whether there is nothing here worth carrying.""" return not (self.is_block or self.comments or self.inline_comments) + def copy(self) -> "HclMeta": + """A copy with lists of its own. + + `copy.copy` would share them, so appending a comment to a copied + body's metadata would add it to the original's too. The comment dicts + themselves are shared, as a shallow copy of a `dict` shares its values. + """ + return HclMeta(self.is_block, list(self.comments), list(self.inline_comments)) + class HclDict(Dict[str, Any]): """A dict whose HCL metadata lives on the object rather than among the keys. @@ -76,7 +85,7 @@ def copy(self) -> "HclDict": metadata to it would be a trap. The in-band form survives a copy because its metadata is among the keys; this has to say so explicitly. """ - return HclDict(self, meta=copy_module.copy(self.hcl_meta)) + return HclDict(self, meta=self.hcl_meta.copy()) def __copy__(self) -> "HclDict": """Same for `copy.copy`.""" @@ -100,8 +109,17 @@ def __deepcopy__(self, memo: dict) -> "HclDict": return duplicate def __reduce__(self) -> Tuple[Any, ...]: - """Carry the metadata through pickling, which `dict` would not.""" - return (_rebuild, (dict(self), self.hcl_meta)) + """Carry the metadata through pickling, which `dict` would not. + + The items go in the reduce tuple's dict-items slot rather than as a + constructor argument, so the empty `HclDict` is built and memoised + before any of them is pickled. A mapping may hold a reference back to + itself, and passing `dict(self)` to the constructor pickles that + reference before there is anything to point it at, which does not + terminate -- `dict` pickles a cycle, so this has to as well. The + metadata is slot state, restored by the default `__setstate__`. + """ + return (HclDict, (), (None, {"hcl_meta": self.hcl_meta}), None, iter(self.items())) def __or__(self, other: Any) -> "HclDict": # type: ignore[override] """Merge, keeping this side's metadata. @@ -115,13 +133,17 @@ def __or__(self, other: Any) -> "HclDict": # type: ignore[override] would then be written as an object. `{**body, ...}` cannot be helped: unpacking always builds a plain `dict`, and there is no hook for it. """ - merged = HclDict(self, meta=copy_module.copy(self.hcl_meta)) + if not isinstance(other, dict): + return NotImplemented + merged = HclDict(self, meta=self.hcl_meta.copy()) merged.update(other) return merged def __ror__(self, other: Any) -> "HclDict": # type: ignore[override] """Same from the left, keeping this side's metadata.""" - merged = HclDict(other, meta=copy_module.copy(self.hcl_meta)) + if not isinstance(other, dict): + return NotImplemented + merged = HclDict(other, meta=self.hcl_meta.copy()) merged.update(self) return merged @@ -130,8 +152,3 @@ def meta_of(value: Any) -> Optional[HclMeta]: """Return the metadata carried beside *value*, or None if it carries none.""" meta = getattr(value, "hcl_meta", None) return meta if isinstance(meta, HclMeta) else None - - -def _rebuild(items: Dict[str, Any], meta: HclMeta) -> HclDict: - """Reconstruct an `HclDict` from its pickled parts.""" - return HclDict(items, meta=meta) diff --git a/test/unit/test_metadata_sidecar.py b/test/unit/test_metadata_sidecar.py index adb12b4c..a9a00d2f 100644 --- a/test/unit/test_metadata_sidecar.py +++ b/test/unit/test_metadata_sidecar.py @@ -341,3 +341,60 @@ def test_a_dict_reached_twice_is_copied_once(self): self.assertIs(duplicate["a"], duplicate["b"]) self.assertIsNot(duplicate["a"], shared) + + +class TestPickleHandlesACycle(TestCase): + """Pickling has the same obligation as `copy.deepcopy`: `dict` survives a + self-reference, so the subclass has to as well.""" + + def test_a_self_reference_round_trips(self): + body = HclDict({"x": 1}, meta=HclMeta(is_block=True, comments=[{"value": "c"}])) + body["self"] = body + + restored = pickle.loads(pickle.dumps(body)) + + self.assertIs(restored["self"], restored) + self.assertEqual(restored["x"], 1) + self.assertEqual(meta_of(restored), meta_of(body)) + + def test_every_protocol(self): + body = HclDict({"x": 1}, meta=HclMeta(is_block=True)) + for protocol in range(pickle.HIGHEST_PROTOCOL + 1): + with self.subTest(protocol=protocol): + restored = pickle.loads(pickle.dumps(body, protocol=protocol)) + self.assertIsInstance(restored, HclDict) + self.assertEqual(restored, body) + self.assertTrue(meta_of(restored).is_block) + + +class TestMergingRefusesWhatDictRefuses(TestCase): + """`dict | x` is a TypeError unless `x` is a dict; the subclass must not be + more permissive than the type it stands in for.""" + + def test_or_with_a_list_of_pairs(self): + with self.assertRaises(TypeError): + HclDict({"a": 1}) | [("b", 2)] # pylint: disable=expression-not-assigned + + def test_ror_with_a_list_of_pairs(self): + with self.assertRaises(TypeError): + [("b", 2)] | HclDict({"a": 1}) # pylint: disable=expression-not-assigned + + +class TestACopyOwnsItsMetadata(TestCase): + """Editing the copy's comments must not edit the original's.""" + + def setUp(self): + self.body = HclDict({"x": 1}, meta=HclMeta(is_block=True, comments=[{"value": "c"}])) + + def test_each_way_of_copying(self): + for name, duplicate in ( + ("copy()", self.body.copy()), + ("copy.copy", copy.copy(self.body)), + ("|", self.body | {}), + ("ror", {} | self.body), + ): + with self.subTest(name=name): + meta_of(duplicate).comments.append({"value": "new"}) + meta_of(duplicate).inline_comments.append({"value": "new"}) + self.assertEqual(meta_of(self.body).comments, [{"value": "c"}]) + self.assertEqual(meta_of(self.body).inline_comments, []) From 0ec91da38f23fd1a6d9ec88d78f58f5ea0bf3a6b Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 23 Sep 2026 11:45:10 -0700 Subject: [PATCH 08/11] fix: every query view writes to the sidecar, and the meta holds a span Under metadata_sidecar, BlockView.to_dict reached the sidecar only for an unlabelled block. A labelled one returns a plain {label: body} wrapper, and AttributeView.to_dict a plain {name: value}, so their adjacent comments went in-band as __comments__ -- which dumps then wrote as real HCL -- and an attribute spelled __is_block__ read back as the marker. Both views now return an HclDict under the option, with the comments in its metadata. HclMeta gains start_line and end_line, so the with_meta span can travel beside a body like the other metadata instead of among its keys. --- CHANGELOG.md | 2 +- hcl2/meta.py | 36 +++++++++++- hcl2/query/attributes.py | 10 +++- hcl2/query/blocks.py | 4 +- test/unit/test_metadata_sidecar.py | 92 ++++++++++++++++++++++++++++++ 5 files changed, 139 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17a4984d..76641972 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Python 3.14 is now tested and declared as supported. No source changes were needed; the full suite passes on 3.14 as-is. -- `SerializationOptions.metadata_sidecar`, which carries `__is_block__`, `__comments__` and `__inline_comments__` beside the mapping rather than among its keys. HCL reserves none of those names, so a document may declare an attribute called any of them -- and in-band one of the two has to lose: on read the marker overwrites the attribute, on write the deserializer drops it, and by then the dict holds a single value with no way to tell which happened. With the option set, `loads` returns an `HclDict`, a `dict` subclass whose `hcl_meta` holds the three, so the mapping contains attributes and nothing else. `dumps` accepts either form, including a hand-built dict using the old keys. Off by default: the keys are a documented part of the output shape, and JSON cannot carry a sidecar. `HclDict`, `HclMeta` and `meta_of` are exported from `hcl2`. Copying, merging with `|` and pickling carry the metadata; `dict(d)` and `{**d}` deliberately do not, since asking for a `dict` gives the mapping and nothing else. ([#331](https://github.com/amplify-education/python-hcl2/issues/331)) +- `SerializationOptions.metadata_sidecar`, which carries `__is_block__`, `__comments__` and `__inline_comments__` beside the mapping rather than among its keys. HCL reserves none of those names, so a document may declare an attribute called any of them -- and in-band one of the two has to lose: on read the marker overwrites the attribute, on write the deserializer drops it, and by then the dict holds a single value with no way to tell which happened. With the option set, `loads` returns an `HclDict`, a `dict` subclass whose `hcl_meta` holds the three, so the mapping contains attributes and nothing else. `dumps` accepts either form, including a hand-built dict using the old keys. Off by default: the keys are a documented part of the output shape, and JSON cannot carry a sidecar. `HclDict`, `HclMeta` and `meta_of` are exported from `hcl2`. The query views follow the option too: `to_dict` on a block or an attribute view returns an `HclDict`, with any adjacent comments in its metadata. Copying, merging with `|` and pickling carry the metadata; `dict(d)` and `{**d}` deliberately do not, since asking for a `dict` gives the mapping and nothing else. ([#331](https://github.com/amplify-education/python-hcl2/issues/331)) ### Changed diff --git a/hcl2/meta.py b/hcl2/meta.py index 514b3c72..226a6808 100644 --- a/hcl2/meta.py +++ b/hcl2/meta.py @@ -25,10 +25,21 @@ class HclMeta: is_block: bool = False comments: List[dict] = field(default_factory=list) inline_comments: List[dict] = field(default_factory=list) + # The span `with_meta` reports, 1-based and inclusive. None when the + # option is off or the tree carries no positions -- a tree built by the + # deserializer has none, and that is "no span", not line zero. + start_line: Optional[int] = None + end_line: Optional[int] = None def is_empty(self) -> bool: """Whether there is nothing here worth carrying.""" - return not (self.is_block or self.comments or self.inline_comments) + return not ( + self.is_block + or self.comments + or self.inline_comments + or self.start_line is not None + or self.end_line is not None + ) def copy(self) -> "HclMeta": """A copy with lists of its own. @@ -37,7 +48,13 @@ def copy(self) -> "HclMeta": body's metadata would add it to the original's too. The comment dicts themselves are shared, as a shallow copy of a `dict` shares its values. """ - return HclMeta(self.is_block, list(self.comments), list(self.inline_comments)) + return HclMeta( + self.is_block, + list(self.comments), + list(self.inline_comments), + self.start_line, + self.end_line, + ) class HclDict(Dict[str, Any]): @@ -148,6 +165,21 @@ def __ror__(self, other: Any) -> "HclDict": # type: ignore[override] return merged +def as_sidecar_dict(value: Any) -> Any: + """Return *value* as an `HclDict` if it is a plain dict, else unchanged. + + A view's `to_dict` can return a dict the serializer never built as a body: + the `{label: body}` wrapper around a labelled block, or the `{name: value}` + of an attribute. Under `metadata_sidecar` those have to be `HclDict`s too. + Left plain, anything attached to them goes back in-band, and a key the + document spelled `__is_block__` reads back as the marker -- the collision + the option exists to remove. + """ + if isinstance(value, dict) and meta_of(value) is None: + return HclDict(value) + return value + + def meta_of(value: Any) -> Optional[HclMeta]: """Return the metadata carried beside *value*, or None if it carries none.""" meta = getattr(value, "hcl_meta", None) diff --git a/hcl2/query/attributes.py b/hcl2/query/attributes.py index 567bb037..9ed0baa9 100644 --- a/hcl2/query/attributes.py +++ b/hcl2/query/attributes.py @@ -2,6 +2,8 @@ from typing import Any, List, Optional +from hcl2.const import COMMENTS_KEY +from hcl2.meta import as_sidecar_dict, meta_of from hcl2.query._base import NodeView, register_view, view_for from hcl2.rules.abstract import LarkElement from hcl2.rules.base import AttributeRule @@ -41,11 +43,17 @@ def value_node(self) -> "NodeView": def to_dict(self, options: Optional[SerializationOptions] = None) -> Any: """Serialize, merging adjacent comments from the parent body.""" result = super().to_dict(options=options) + if options is not None and options.metadata_sidecar: + result = as_sidecar_dict(result) if ( self._adjacent_comments and options is not None and options.with_comments and isinstance(result, dict) ): - result["__comments__"] = self._adjacent_comments + meta = meta_of(result) + if meta is not None: + meta.comments = self._adjacent_comments + meta.comments + else: + result[COMMENTS_KEY] = self._adjacent_comments return result diff --git a/hcl2/query/blocks.py b/hcl2/query/blocks.py index 0142b234..4d05b04f 100644 --- a/hcl2/query/blocks.py +++ b/hcl2/query/blocks.py @@ -3,7 +3,7 @@ from typing import Any, List, Optional from hcl2.const import COMMENTS_KEY -from hcl2.meta import meta_of +from hcl2.meta import as_sidecar_dict, meta_of from hcl2.query._base import NodeView, register_view from hcl2.rules.abstract import LarkElement from hcl2.rules.base import BlockRule @@ -65,6 +65,8 @@ def body(self) -> "NodeView": def to_dict(self, options: Optional[SerializationOptions] = None) -> Any: """Serialize, merging adjacent comments from the parent body.""" result = super().to_dict(options=options) + if options is not None and options.metadata_sidecar: + result = as_sidecar_dict(result) if ( self._adjacent_comments and options is not None diff --git a/test/unit/test_metadata_sidecar.py b/test/unit/test_metadata_sidecar.py index a9a00d2f..7d4294b7 100644 --- a/test/unit/test_metadata_sidecar.py +++ b/test/unit/test_metadata_sidecar.py @@ -242,6 +242,58 @@ def test_the_in_band_form_is_unchanged(self): self.assertEqual(body[COMMENTS_KEY], [{"value": "lead comment"}]) +class TestEveryViewWritesToTheSidecar(TestCase): + """A labelled block and an attribute have to pick the form as well. + + A labelled block serializes to a plain `{label: body}` wrapper, and an + attribute to a plain `{name: value}`, so neither carried a sidecar and the + adjacent comments went in-band onto them -- the one key the option exists + to keep out of the mapping, and one `dumps` then wrote as `__comments__`. + The comments land at the same level the in-band form puts them, on the + dict the view returns, so `meta_of(view.to_dict(...))` finds them there + whichever node the view is over. + """ + + SIDECAR_COMMENTS = SerializationOptions(metadata_sidecar=True, with_comments=True) + + def _document(self, source): + from hcl2.query import DocumentView + + return DocumentView.parse(source) + + def test_a_labelled_block(self): + block = self._document('# lead\nresource "a" "b" {\n x = 1\n}\n').blocks("resource")[0] + result = block.to_dict(options=self.SIDECAR_COMMENTS) + self.assertNotIn(COMMENTS_KEY, result) + self.assertEqual(meta_of(result).comments, [{"value": "lead"}]) + self.assertEqual(dumps({"resource": [result]}), 'resource "a" "b" {\n x = 1\n}\n') + + def test_the_in_band_labelled_block_is_unchanged(self): + block = self._document('# lead\nresource "a" "b" {\n x = 1\n}\n').blocks("resource")[0] + result = block.to_dict(options=SerializationOptions(with_comments=True)) + self.assertEqual(result[COMMENTS_KEY], [{"value": "lead"}]) + self.assertIsNone(meta_of(result)) + + def test_an_attribute(self): + attribute = self._document("# lead\nx = 1\n").attributes("x")[0] + result = attribute.to_dict(options=self.SIDECAR_COMMENTS) + self.assertEqual(result, {"x": 1}) + self.assertEqual(meta_of(result).comments, [{"value": "lead"}]) + + def test_the_in_band_attribute_is_unchanged(self): + attribute = self._document("# lead\nx = 1\n").attributes("x")[0] + result = attribute.to_dict(options=SerializationOptions(with_comments=True)) + self.assertEqual(result, {"x": 1, COMMENTS_KEY: [{"value": "lead"}]}) + + def test_an_attribute_named_like_the_marker_stays_an_attribute(self): + # Returned as a plain dict, `{"__is_block__": true}` would read back + # as the marker; the sidecar form is what keeps it an attribute. + attribute = self._document("__is_block__ = true\n").attributes()[0] + result = attribute.to_dict(options=SerializationOptions(metadata_sidecar=True)) + self.assertIsNotNone(meta_of(result)) + self.assertEqual(dumps(result), "__is_block__ = true\n") + + class TestTheTypeIsPartOfThePublicSurface(TestCase): """The CHANGELOG makes `HclDict` part of the contract, so it has to be reachable.""" @@ -398,3 +450,43 @@ def test_each_way_of_copying(self): meta_of(duplicate).inline_comments.append({"value": "new"}) self.assertEqual(meta_of(self.body).comments, [{"value": "c"}]) self.assertEqual(meta_of(self.body).inline_comments, []) + + +class TestTheMetaHasRoomForLineNumbers(TestCase): + """`with_meta` line spans belong in the sidecar too. + + Written in-band into an `HclDict`, `__start_line__` and `__end_line__` sit + among the attributes, where the sidecar reserves nothing, so `dumps` writes + them out as real HCL. The meta carries them instead, beside the other + things that are not attributes of the body. + """ + + def test_absent_by_default(self): + meta = HclMeta() + self.assertIsNone(meta.start_line) + self.assertIsNone(meta.end_line) + self.assertTrue(meta.is_empty()) + + def test_a_span_alone_is_worth_carrying(self): + self.assertFalse(HclMeta(start_line=1, end_line=3).is_empty()) + + def test_the_earlier_fields_keep_their_positions(self): + meta = HclMeta(True, [{"value": "c"}], []) + self.assertTrue(meta.is_block) + self.assertEqual(meta.comments, [{"value": "c"}]) + + def test_every_copy_carries_the_span(self): + body = HclDict({"x": 1}, meta=HclMeta(is_block=True, start_line=4, end_line=9)) + for name, duplicate in ( + ("copy()", body.copy()), + ("copy.copy", copy.copy(body)), + ("deepcopy", copy.deepcopy(body)), + ("pickle", pickle.loads(pickle.dumps(body))), + ("|", body | {}), + ): + with self.subTest(name=name): + self.assertEqual((meta_of(duplicate).start_line, meta_of(duplicate).end_line), (4, 9)) + + def test_the_span_is_not_written(self): + body = HclDict({"x": 1}, meta=HclMeta(is_block=True, start_line=1, end_line=3)) + self.assertEqual(dumps({"b": [body]}), "b {\n x = 1\n}\n") From f0b541e552c54184ba3ee6ccbabc864a12c63764 Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 23 Sep 2026 11:45:10 -0700 Subject: [PATCH 09/11] docs: document metadata_sidecar, HclDict, HclMeta and meta_of The option table, the module map and the public API list in CLAUDE.md, and a section in the advanced API reference. --- CLAUDE.md | 4 +++- docs/01_getting_started.md | 1 + docs/03_advanced_api.md | 23 +++++++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5084697f..501a1f72 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -25,6 +25,7 @@ The **Direct** pipeline (`parse_to_tree` → `transform` → `to_lark` → `reco | `hcl2/builder.py` | Programmatic HCL document construction | | `hcl2/walk.py` | Generic tree-walking primitives for the LarkElement IR tree | | `hcl2/utils.py` | `SerializationOptions`, `SerializationContext`, string helpers | +| `hcl2/meta.py` | `HclDict`, `HclMeta`, `meta_of` — metadata carried beside a body under `metadata_sidecar` | | `hcl2/const.py` | Constants: `IS_BLOCK`, `COMMENTS_KEY`, `INLINE_COMMENTS_KEY` | | `hcl2/cli/helpers.py` | File/directory/stdin conversion helpers | | `hcl2/cli/hcl_to_json.py` | `hcl2tojson` entry point | @@ -75,11 +76,12 @@ Follows the `json` module convention. All option parameters are keyword-only. - `dump/dumps` — Python dict → HCL2 text - `query` — HCL2 text/file → `DocumentView` for structured queries - Intermediate stages: `parse/parses`, `parse_to_tree/parses_to_tree`, `transform`, `serialize`, `from_dict`, `from_json`, `reconstruct` +- Metadata sidecar (`hcl2/meta.py`, exported from `hcl2`): `HclDict` (a `dict` whose `hcl_meta` holds the block marker and comments), `HclMeta`, and `meta_of(value)`, which returns the metadata or `None` ### Option Dataclasses **`SerializationOptions`** (LarkElement → dict): -`with_comments`, `with_meta`, `wrap_objects`, `wrap_tuples`, `explicit_blocks`, `preserve_heredocs`, `force_operation_parentheses`, `preserve_scientific_notation`, `strip_string_quotes` +`with_comments`, `with_meta`, `wrap_objects`, `wrap_tuples`, `explicit_blocks`, `preserve_heredocs`, `force_operation_parentheses`, `preserve_scientific_notation`, `strip_string_quotes`, `metadata_sidecar` **`DeserializerOptions`** (dict → LarkElement): `heredocs_to_strings`, `strings_to_heredocs`, `object_elements_colon`, `object_elements_trailing_comma` diff --git a/docs/01_getting_started.md b/docs/01_getting_started.md index 431fc06b..32799352 100644 --- a/docs/01_getting_started.md +++ b/docs/01_getting_started.md @@ -78,6 +78,7 @@ data = loads(text, serialization_options=SerializationOptions( | `force_operation_parentheses` | `bool` | `False` | Force parentheses around all operations | | `preserve_scientific_notation` | `bool` | `True` | Keep scientific notation as-is | | `strip_string_quotes` | `bool` | `False` | Yield string *values* rather than source text: remove surrounding quotes (e.g. `"hello"` instead of `'"hello"'`) and resolve escape sequences (`"a\nb"` becomes a real newline). String literals inside expressions keep their quotes, so `upper("x")` stays `'${upper("x")}'`. **Breaks JSON->HCL2 deserialization and reconstruction.** | +| `metadata_sidecar` | `bool` | `False` | Carry `__is_block__`, `__comments__` and `__inline_comments__` beside each body instead of among its keys, so a document attribute with one of those names survives. Bodies, objects and query results come back as `HclDict`, a `dict` subclass; read the metadata with `hcl2.meta_of(value)` (see [Advanced API](03_advanced_api.md#metadata-sidecar)). `dumps` accepts either form. JSON cannot carry the sidecar, so `json.dumps` of the result writes the attributes only. | ### Comment Format diff --git a/docs/03_advanced_api.md b/docs/03_advanced_api.md index d8cc5b54..c2431fa9 100644 --- a/docs/03_advanced_api.md +++ b/docs/03_advanced_api.md @@ -45,6 +45,29 @@ from hcl2 import SerializationOptions data = hcl2.serialize(tree, serialization_options=SerializationOptions(with_meta=True)) ``` +### Metadata sidecar + +By default the serializer reports what it knows about a body -- that it is a +block and its comments -- as `__is_block__`, `__comments__` and +`__inline_comments__` keys among the attributes. HCL reserves none of those names, so a document that +declares one loses either the attribute or the metadata. With +`metadata_sidecar=True` each body is an `HclDict` instead: a `dict` holding the +attributes and nothing else, with the metadata on `hcl_meta`. + +```python +from hcl2 import HclDict, meta_of, SerializationOptions + +data = hcl2.loads(text, serialization_options=SerializationOptions(metadata_sidecar=True)) +body = data["resource"][0]['"aws_instance"']['"web"'] +meta_of(body).is_block # True +meta_of(body).comments # [{"value": "..."}] +meta_of({"plain": "dict"}) # None +``` + +`copy()`, `copy.copy`, `copy.deepcopy`, pickling and `|` keep the metadata; +`dict(body)` and `{**body}` give the attributes alone. `dumps` accepts either +form, including a hand-built dict using the in-band keys. + ### from_dict / from_json — Python dict or JSON to LarkElement tree ```python From e3a279db7e5bfcc92dc7097765d89c99152231fc Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 23 Sep 2026 12:41:31 -0700 Subject: [PATCH 10/11] fix: label levels carry a sidecar too, so a label cannot pose as metadata Under metadata_sidecar a labelled block's {label: body} levels were plain dicts, which the deserializer reads in-band, so an identifier label spelled __comments__ or __is_block__ was taken for metadata and dropped with the body it names. Each level is now an HclDict. --- CHANGELOG.md | 2 +- hcl2/rules/base.py | 6 ++++ test/unit/test_metadata_sidecar.py | 54 ++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76641972..6e1df08b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Python 3.14 is now tested and declared as supported. No source changes were needed; the full suite passes on 3.14 as-is. -- `SerializationOptions.metadata_sidecar`, which carries `__is_block__`, `__comments__` and `__inline_comments__` beside the mapping rather than among its keys. HCL reserves none of those names, so a document may declare an attribute called any of them -- and in-band one of the two has to lose: on read the marker overwrites the attribute, on write the deserializer drops it, and by then the dict holds a single value with no way to tell which happened. With the option set, `loads` returns an `HclDict`, a `dict` subclass whose `hcl_meta` holds the three, so the mapping contains attributes and nothing else. `dumps` accepts either form, including a hand-built dict using the old keys. Off by default: the keys are a documented part of the output shape, and JSON cannot carry a sidecar. `HclDict`, `HclMeta` and `meta_of` are exported from `hcl2`. The query views follow the option too: `to_dict` on a block or an attribute view returns an `HclDict`, with any adjacent comments in its metadata. Copying, merging with `|` and pickling carry the metadata; `dict(d)` and `{**d}` deliberately do not, since asking for a `dict` gives the mapping and nothing else. ([#331](https://github.com/amplify-education/python-hcl2/issues/331)) +- `SerializationOptions.metadata_sidecar`, which carries `__is_block__`, `__comments__` and `__inline_comments__` beside the mapping rather than among its keys. HCL reserves none of those names, so a document may declare an attribute called any of them -- and in-band one of the two has to lose: on read the marker overwrites the attribute, on write the deserializer drops it, and by then the dict holds a single value with no way to tell which happened. With the option set, `loads` returns an `HclDict`, a `dict` subclass whose `hcl_meta` holds the three, so the mapping contains attributes and nothing else. `dumps` accepts either form, including a hand-built dict using the old keys. Off by default: the keys are a documented part of the output shape, and JSON cannot carry a sidecar. `HclDict`, `HclMeta` and `meta_of` are exported from `hcl2`. The `{label: body}` levels around a labelled block are `HclDict`s as well, so a label spelled like one of the three names is not taken for metadata. The query views follow the option too: `to_dict` on a block or an attribute view returns an `HclDict`, with any adjacent comments in its metadata. Copying, merging with `|` and pickling carry the metadata; `dict(d)` and `{**d}` deliberately do not, since asking for a `dict` gives the mapping and nothing else. ([#331](https://github.com/amplify-education/python-hcl2/issues/331)) ### Changed diff --git a/hcl2/rules/base.py b/hcl2/rules/base.py index 367a83f1..aeea4960 100644 --- a/hcl2/rules/base.py +++ b/hcl2/rules/base.py @@ -168,5 +168,11 @@ def serialize(self, options=SerializationOptions(), context=SerializationContext labels = self._labels for label in reversed(labels[1:]): result = {label.serialize(options): result} + if options.metadata_sidecar: + # A label level is a mapping too. Left plain, the deserializer + # reads it in-band, and a label spelled `__comments__` or + # `__is_block__` is taken for metadata and dropped with the + # body it names. + result = HclDict(result) return result diff --git a/test/unit/test_metadata_sidecar.py b/test/unit/test_metadata_sidecar.py index 7d4294b7..ca38456c 100644 --- a/test/unit/test_metadata_sidecar.py +++ b/test/unit/test_metadata_sidecar.py @@ -490,3 +490,57 @@ def test_every_copy_carries_the_span(self): def test_the_span_is_not_written(self): body = HclDict({"x": 1}, meta=HclMeta(is_block=True, start_line=1, end_line=3)) self.assertEqual(dumps({"b": [body]}), "b {\n x = 1\n}\n") + + +class TestALabelNamedLikeMetadataSurvives(TestCase): + """A label level is a mapping the sidecar has to cover as well. + + `BlockRule` wrapped each label in a plain `{label: body}` dict, which the + deserializer reads in-band, so an identifier label spelled `__comments__` + or `__is_block__` was taken for metadata and dropped with its whole body. + """ + + def test_each_reserved_name_as_a_label(self): + for name in (IS_BLOCK, COMMENTS_KEY, INLINE_COMMENTS_KEY): + with self.subTest(label=name): + source = f"b {name} {{\n x = 1\n}}\n" + self.assertEqual(dumps(loads(source, serialization_options=SIDECAR)), source) + + def test_a_label_level_is_an_hcl_dict(self): + level = loads('b "l" {\n x = 1\n}\n', serialization_options=SIDECAR)["b"][0] + self.assertIsInstance(level, HclDict) + self.assertFalse(meta_of(level).is_block) + + def test_the_in_band_form_is_unchanged(self): + level = loads('b "l" {\n x = 1\n}\n')["b"][0] + self.assertIs(type(level), dict) + + +class _Sub(HclDict): + __slots__ = () + + +class TestASubclassKeepsItsType(TestCase): + """Copying or pickling a subclass gives back that subclass, as `dict` does.""" + + def setUp(self): + self.value = _Sub({"a": 1}, meta=HclMeta(is_block=True, comments=[{"value": "c"}])) + + def test_every_way_of_duplicating(self): + for name, duplicate in ( + ("copy()", self.value.copy()), + ("copy.copy", copy.copy(self.value)), + ("copy.deepcopy", copy.deepcopy(self.value)), + ("pickle", pickle.loads(pickle.dumps(self.value))), + ("|", self.value | {"b": 2}), + ("ror", {"b": 2} | self.value), + ): + with self.subTest(name=name): + self.assertIs(type(duplicate), _Sub) + self.assertTrue(meta_of(duplicate).is_block) + + def test_a_cyclic_subclass_still_pickles(self): + self.value["self"] = self.value + restored = pickle.loads(pickle.dumps(self.value)) + self.assertIs(type(restored), _Sub) + self.assertIs(restored["self"], restored) From 701c4741a061586b4b99832aa5a1034fe4d7c428 Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 23 Sep 2026 12:41:31 -0700 Subject: [PATCH 11/11] fix: copying or pickling an HclDict subclass keeps the subclass copy(), copy.copy, copy.deepcopy, pickling and | built HclDict by name, so a subclass came back as the base class, where dict keeps its type. --- hcl2/meta.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hcl2/meta.py b/hcl2/meta.py index 226a6808..e39cc0b2 100644 --- a/hcl2/meta.py +++ b/hcl2/meta.py @@ -102,7 +102,7 @@ def copy(self) -> "HclDict": metadata to it would be a trap. The in-band form survives a copy because its metadata is among the keys; this has to say so explicitly. """ - return HclDict(self, meta=self.hcl_meta.copy()) + return type(self)(self, meta=self.hcl_meta.copy()) def __copy__(self) -> "HclDict": """Same for `copy.copy`.""" @@ -118,7 +118,7 @@ def __deepcopy__(self, memo: dict) -> "HclDict": copy first for that reason; a subclass that did not would make a cyclic document worse than the plain mapping it replaces. """ - duplicate = HclDict() + duplicate = type(self)() memo[id(self)] = duplicate duplicate.hcl_meta = copy_module.deepcopy(self.hcl_meta, memo) for key, value in self.items(): @@ -136,7 +136,7 @@ def __reduce__(self) -> Tuple[Any, ...]: terminate -- `dict` pickles a cycle, so this has to as well. The metadata is slot state, restored by the default `__setstate__`. """ - return (HclDict, (), (None, {"hcl_meta": self.hcl_meta}), None, iter(self.items())) + return (type(self), (), (None, {"hcl_meta": self.hcl_meta}), None, iter(self.items())) def __or__(self, other: Any) -> "HclDict": # type: ignore[override] """Merge, keeping this side's metadata. @@ -152,7 +152,7 @@ def __or__(self, other: Any) -> "HclDict": # type: ignore[override] """ if not isinstance(other, dict): return NotImplemented - merged = HclDict(self, meta=self.hcl_meta.copy()) + merged = type(self)(self, meta=self.hcl_meta.copy()) merged.update(other) return merged @@ -160,7 +160,7 @@ def __ror__(self, other: Any) -> "HclDict": # type: ignore[override] """Same from the left, keeping this side's metadata.""" if not isinstance(other, dict): return NotImplemented - merged = HclDict(other, meta=self.hcl_meta.copy()) + merged = type(self)(other, meta=self.hcl_meta.copy()) merged.update(self) return merged