Skip to content

fix: emit block line numbers when with_meta is set (#291) - #333

Open
livingstaccato wants to merge 5 commits into
amplify-education:mainfrom
livingstaccato:fix/with-meta-block-line-numbers
Open

fix: emit block line numbers when with_meta is set (#291)#333
livingstaccato wants to merge 5 commits into
amplify-education:mainfrom
livingstaccato:fix/with-meta-block-line-numbers

Conversation

@livingstaccato

@livingstaccato livingstaccato commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Fixes #291.

What

SerializationOptions.with_meta produced no metadata. The option, the hcl2tojson --with-meta flag and the v8 migration guide's promise that the v7 keys are "still available" all survived the rewrite; the code that emitted them did not. v7 wrote the keys from RuleTransformer.block, and v8 moved block serialization into BlockRule.serialize without carrying them over, so the option was read nowhere in the package.

BlockRule.serialize now emits __start_line__ and __end_line__ alongside the body -- the same innermost dict the labels nest around, which is where v7 put them. The line numbers in the tests were checked against python-hcl2 7.3.1 on the same input, so they are v7's values rather than merely self-consistent.

A tree built by the deserializer carries an empty Meta, so it reports no keys rather than inventing zeros.

Also here

  • BlockView.start_line / .end_line, so a span can be read from the query API without serializing the block. with_meta puts the numbers in the output dict, which otherwise meant reaching them through the label nesting or through the rule's private _meta. Both are None for a deserialized tree. hq picks them up through its property accessors: hq 'resource[*] | .start_line' main.tf.
  • Tests pinning what happens to an attribute genuinely named like a metadata key. The keys travel in-band, so dumps(loads(...)) drops such an attribute and with_meta overwrites it -- as already happened for __is_block__ and __comments__ before these two keys existed. That collision is filed separately as __is_block__, __comments__ and the with_meta keys silently displace attributes of the same name #331, and this PR makes it worse rather than merely inheriting it: before, __start_line__ was an ordinary attribute that survived a round trip, and after, it does not -- on the default path, with with_meta off. The tests state that rather than defend it. __is_block__, __comments__ and the with_meta keys silently displace attributes of the same name #331 now has an out-of-band fix in progress, which is where the collision goes away for all five names; if that lands first this PR should rebase onto it instead of reserving the two names in-band.

Note for merge order

This branch and the one for #328 both touch hcl2/query/blocks.py on adjacent lines. Whichever lands second will want a keep-both resolution.

Merging

It touches the same code as #332 (hcl2/query/blocks.py), #345 (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. Please review with that provenance in mind.

…#291)

`SerializationOptions.with_meta` documented `__start_line__` and
`__end_line__` keys, `hcl2tojson` exposed it as `--with-meta`, and the v8
migration guide told readers the v7 keys were "still available". None of
that was true: v7 added the keys in `RuleTransformer.block`, the v8
rewrite moved block serialization into `BlockRule.serialize`, and the
option went unread anywhere in the package.

Emit them from `BlockRule.serialize`, on the same innermost dict the
labels nest around and next to `__is_block__` -- the position v7 used.
The output is byte-identical to 7.3.1's for the same input, nested
blocks included.

Two details the old implementation did not have to handle:

- A tree built by the deserializer carries an empty `Meta`, whose `.line`
  raises. Serializing one with `with_meta` set now skips the keys rather
  than inventing zeros.
- `dumps()` would otherwise write the metadata back out as HCL
  attributes, so both keys join the deserializer's reserved set.

The two existing tests only asserted that the option was accepted, which
is why nothing noticed; they now assert on the metadata itself.
`with_meta` puts `__start_line__` and `__end_line__` in the serialized
output, which is the right place for a dict but an awkward one for a
query: reading a block's span meant serializing the block and descending
past however many labels it has, or reaching into the rule's private
`_meta`. Both are what a downstream Terraform block reader ended up
doing.

Add `start_line` and `end_line` to `BlockView`. They report the same
numbers `with_meta` serializes -- asserted against it rather than
restated -- and are `None` for a tree built by the deserializer, which
carries no positions and whose empty `Meta` has no line attributes at
all.

`hq` picks them up through its existing property accessors, so
`hq 'resource[*] | .start_line' main.tf` works with no further wiring.
The keys travel in-band, in the same dict as the block's attributes,
which is where v7 put them. That means the deserializer cannot tell a
key it wrote from one the document declared, so `dumps(loads(...))`
drops an attribute genuinely named `__start_line__`, and `with_meta`
overwrites its value -- exactly as both already happened for
`__is_block__`, `__comments__` and `__inline_comments__`.

These tests state that rather than leave it to be discovered. Nothing
short of moving all five keys out of band would make the metadata
unambiguous, and that is a breaking change to the serialized shape, not
a fix belonging to this option.
@livingstaccato

livingstaccato commented Sep 1, 2026

Copy link
Copy Markdown
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.

@livingstaccato

livingstaccato commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Review pass done, so the hold above no longer applies — this is ready for review now.

Rebased on current main; GitHub reports it mergeable as it stands.

🤖 Drafted with Claude Code.

kkozik-amplify and others added 2 commits September 7, 2026 15:10
amplify-education#291)

The suite asserted one document's numbers. Adds the cases where line counting
actually goes wrong: a heredoc before and inside a block -- its body is a
single token spanning several physical lines, so anything counting tokens gets
every block after it wrong -- plus CRLF, block and hash comments, an inline
comment on the header, surrounding blank lines, four levels of nesting and a
single-line block. Each asserts that the reported span brackets the block in
the source rather than a hardcoded pair, so the cases stay readable, and that
`with_meta`'s keys agree with the query properties for all of them.

Adds the v7 spans for a richer document as a fixture -- no labels, one label,
two, and two levels of nesting -- taken from python-hcl2 7.3.1 rather than
from this implementation, so a change in how lines are counted reads as a
disagreement with the version `with_meta` promises to match. All seven agree
today. Also pins that an object-valued attribute gets no span, since an object
nests exactly like a labelless block's body.

Mutation-checked: dropping the `result.update(...)` fails 14, returning
`line` from `end_line` fails 25, and removing the empty-`Meta` guard errors.

Records in the CHANGELOG that the two names are now reserved, so an attribute
genuinely called either one stops surviving `dumps(loads(...))` on the default
path with `with_meta` off, where it used to. The pull request said so; the
entry did not. Points at amplify-education#331 for the out-of-band fix.

Updates the `hcl2/const.py` row in CLAUDE.md for the two new constants.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
livingstaccato added a commit to livingstaccato/python-hcl2 that referenced this pull request Sep 7, 2026
@livingstaccato

Copy link
Copy Markdown
Contributor Author

Brought up to date with current main (0f74596) — GitHub had this flagged as
conflicting, which was only CHANGELOG.md and the merge=union driver that the
web UI does not honour. This one conflicts with #332 and #345 (hcl2/query/blocks.py,
hcl2/rules/base.py) rather than with main.

Merge order. Eight of the twelve open PRs apply to main in sequence with no
conflict at all — verified by merging each and running the suite, not by
inspection:

#332 → #334 → #335 → #345 → #346 → #349 → #352 → #354

(1552 → 1563 → 1591 → 1627 → 1671 → 1696 → 1703 → 1711 passing, from a 1534
baseline; each adds its own tests.) The other four — #333, #348, #350, #351
conflict with siblings rather than with main, so no ordering avoids them.

A resolved integration of all twelve is on the fork if it is useful:
livingstaccato/python-hcl2:int/pyvider-hcl-9 — 1784 passing, ruff and mypy
clean. Not proposed as a PR; the twelve are more reviewable apart.

Drafted with the help of an LLM, working on behalf of the author.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hcl2tojson --with-meta flag accepted but produces no metadata

2 participants