feat: carry serializer metadata beside the mapping (#331) - #345
Draft
livingstaccato wants to merge 5 commits into
Draft
feat: carry serializer metadata beside the mapping (#331)#345livingstaccato wants to merge 5 commits into
livingstaccato wants to merge 5 commits into
Conversation
…#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.
Contributor
Author
|
Please hold off on merging this one for now — I want to do another review pass over it before it goes in. Opened as a draft for that reason; I will mark it ready and say so here once I am done. |
`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.
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.
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.
`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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #331.
What
__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:Neither is recoverable by the caller: by the time the dict exists, it holds a single value with nothing to say which of the two it is.
The shape
SerializationOptions(metadata_sidecar=True)makesloadsreturn anHclDict— adictsubclass whosehcl_metacarries the three. The mapping then holds attributes and nothing else, so there is nothing left to collide with.It is a
dict, deliberately: equality, iteration,len,json.dumpsand every other mapping operation behave exactly as before, so anything reading attributes does not notice.dumpsreads whichever form it is handed — a document loaded either way, or a dict built by hand with the old keys — so nothing that works today stops working.Off by default
Two reasons, both worth stating rather than leaving to be discovered.
The keys are a documented part of the v7 and v8 output shape, and
hcl2tojson --with-commentsputs them in JSON that other tools read. Changing that silently is not this PR's to do.And JSON cannot carry a sidecar:
json.dumpsof anHclDictyields the attributes alone. Anyone serializing to JSON wants the in-band form, and should keep getting it unless they ask otherwise.Scope
This covers the three keys that exist in 8.1.3. #291's
__start_line__and__end_line__are the same class of key and belong in the same sidecar; PR #333 adds them in-band today, and should rebase onto this if this lands first.The test that pins the in-band loss stays, marked as the behaviour the option exists to avoid, so the difference between the two modes stays visible rather than becoming folklore.
Merging
It touches the same code as #333 (
hcl2/deserializer.py and hcl2/rules/base.py). Whichever of those lands first, this one needs a rebase rather than a merge — the overlaps are real edits to the same methods, not adjacent lines, so resolving them by hand risks losing one of the two fixes. Say the word and I will rebase and re-run the suite.This pull request, and the investigation behind it, were produced by an AI assistant (Claude) working on behalf of the author. Every reproduction, test run and benchmark cited was executed rather than inferred, but please review with that provenance in mind.