From 1265fc66b04a6cd9802f7572c8c2d9fcd4b6383a Mon Sep 17 00:00:00 2001 From: Tim Date: Tue, 1 Sep 2026 21:19:54 -0700 Subject: [PATCH 1/6] 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 2/6] 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 3/6] 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 4/6] 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 5/6] 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 6/6] 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)