Skip to content

Commit f4fd8c5

Browse files
committed
Better roundtriping of HCL
1 parent e4bf8a1 commit f4fd8c5

File tree

6 files changed

+159
-11
lines changed

6 files changed

+159
-11
lines changed

agents.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ Key methods:
135135
- Use the candidate_node style attribute to store style information for round-trip. Ask if this needs to be updated with new styles.
136136
- Use build tags for optional compilation
137137
- Add comprehensive tests
138+
- Run the specific encoder/decoder test (e.g. <format>_test.go) whenever you make ay changes to the encoder_<format> or decoder_<format>
138139
- Handle errors gracefully
139140
- Add the no build directive, like the xml encoder and decoder, that enables a minimal yq builds. e.g. `//go:build !yq_<format>`. Be sure to also update the build_small-yq.sh and build-tinygo-yq.sh to not include the new format.
140141

pkg/yqlib/candidate_node.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,9 @@ type CandidateNode struct {
9797
// (e.g. top level cross document merge). This property does not propagate to child nodes.
9898
EvaluateTogether bool
9999
IsMapKey bool
100+
// For formats like HCL and TOML: indicates that child entries should be emitted as separate blocks/tables
101+
// rather than consolidated into nested mappings (default behaviour)
102+
EncodeSeparate bool
100103
}
101104

102105
func (n *CandidateNode) CreateChild() *CandidateNode {
@@ -407,6 +410,8 @@ func (n *CandidateNode) doCopy(cloneContent bool) *CandidateNode {
407410

408411
EvaluateTogether: n.EvaluateTogether,
409412
IsMapKey: n.IsMapKey,
413+
414+
EncodeSeparate: n.EncodeSeparate,
410415
}
411416

412417
if cloneContent {

pkg/yqlib/decoder_hcl.go

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,14 @@ func (dec *hclDecoder) Decode() (*CandidateNode, error) {
161161
}
162162

163163
// process blocks
164+
// Count blocks by type at THIS level to detect multiple separate blocks
165+
blocksByType := make(map[string]int)
164166
for _, block := range body.Blocks {
165-
addBlockToMapping(root, block, dec.fileBytes)
167+
blocksByType[block.Type]++
168+
}
169+
170+
for _, block := range body.Blocks {
171+
addBlockToMapping(root, block, dec.fileBytes, blocksByType[block.Type] > 1)
166172
}
167173

168174
dec.documentIndex++
@@ -187,14 +193,23 @@ func hclBodyToNode(body *hclsyntax.Body, src []byte) *CandidateNode {
187193

188194
node.AddKeyValueChild(key, val)
189195
}
196+
197+
// Process nested blocks, counting blocks by type at THIS level
198+
// to detect which block types appear multiple times
199+
blocksByType := make(map[string]int)
200+
for _, block := range body.Blocks {
201+
blocksByType[block.Type]++
202+
}
203+
190204
for _, block := range body.Blocks {
191-
addBlockToMapping(node, block, src)
205+
addBlockToMapping(node, block, src, blocksByType[block.Type] > 1)
192206
}
193207
return node
194208
}
195209

196210
// addBlockToMapping nests block type and labels into the parent mapping, merging children.
197-
func addBlockToMapping(parent *CandidateNode, block *hclsyntax.Block, src []byte) {
211+
// isMultipleBlocksOfType indicates if there are multiple blocks of this type at THIS level
212+
func addBlockToMapping(parent *CandidateNode, block *hclsyntax.Block, src []byte, isMultipleBlocksOfType bool) {
198213
bodyNode := hclBodyToNode(block.Body, src)
199214
current := parent
200215

@@ -208,6 +223,11 @@ func addBlockToMapping(parent *CandidateNode, block *hclsyntax.Block, src []byte
208223
}
209224
if typeNode == nil {
210225
_, typeNode = current.AddKeyValueChild(createStringScalarNode(block.Type), &CandidateNode{Kind: MappingNode})
226+
// Mark the type node if there are multiple blocks of this type at this level
227+
// This tells the encoder to emit them as separate blocks rather than consolidating them
228+
if isMultipleBlocksOfType {
229+
typeNode.EncodeSeparate = true
230+
}
211231
}
212232
current = typeNode
213233

pkg/yqlib/doc/usage/hcl.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,3 +175,27 @@ message = "Hello, ${name}!"
175175
shouty_message = upper(message)
176176
```
177177

178+
## Roundtrip: Separate blocks with same name.
179+
Given a sample.hcl file of:
180+
```hcl
181+
resource "aws_instance" "web" {
182+
ami = "ami-12345"
183+
}
184+
resource "aws_instance" "db" {
185+
ami = "ami-67890"
186+
}
187+
```
188+
then
189+
```bash
190+
yq sample.hcl
191+
```
192+
will output
193+
```hcl
194+
resource "aws_instance" "web" {
195+
ami = "ami-12345"
196+
}
197+
resource "aws_instance" "db" {
198+
ami = "ami-67890"
199+
}
200+
```
201+

pkg/yqlib/encoder_hcl.go

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,13 @@ func (he *hclEncoder) encodeBlockIfMapping(body *hclwrite.Body, key string, valu
438438
return false
439439
}
440440

441+
// If EncodeSeparate is set, emit children as separate blocks regardless of label extraction
442+
if valueNode.EncodeSeparate {
443+
if handled, _ := he.encodeMappingChildrenAsBlocks(body, key, valueNode); handled {
444+
return true
445+
}
446+
}
447+
441448
// Try to extract block labels from a single-entry mapping chain
442449
if labels, bodyNode, ok := extractBlockLabels(valueNode); ok {
443450
if len(labels) > 1 && mappingChildrenAllMappings(bodyNode) {
@@ -519,17 +526,46 @@ func (he *hclEncoder) encodeMappingChildrenAsBlocks(body *hclwrite.Body, blockTy
519526
return false, nil
520527
}
521528

529+
// Only emit as separate blocks if EncodeSeparate is true
530+
// This allows the encoder to respect the original block structure preserved by the decoder
531+
if !valueNode.EncodeSeparate {
532+
return false, nil
533+
}
534+
522535
for i := 0; i < len(valueNode.Content); i += 2 {
523536
childKey := valueNode.Content[i].Value
524537
childVal := valueNode.Content[i+1]
525-
labels := []string{childKey}
526-
if extraLabels, bodyNode, ok := extractBlockLabels(childVal); ok {
527-
labels = append(labels, extraLabels...)
528-
childVal = bodyNode
529-
}
530-
block := body.AppendNewBlock(blockType, labels)
531-
if err := he.encodeNodeAttributes(block.Body(), childVal); err != nil {
532-
return true, err
538+
539+
// Check if this child also represents multiple blocks (all children are mappings)
540+
if mappingChildrenAllMappings(childVal) {
541+
// Recursively emit each grandchild as a separate block with extended labels
542+
for j := 0; j < len(childVal.Content); j += 2 {
543+
grandchildKey := childVal.Content[j].Value
544+
grandchildVal := childVal.Content[j+1]
545+
labels := []string{childKey, grandchildKey}
546+
547+
// Try to extract additional labels if this is a single-entry chain
548+
if extraLabels, bodyNode, ok := extractBlockLabels(grandchildVal); ok {
549+
labels = append(labels, extraLabels...)
550+
grandchildVal = bodyNode
551+
}
552+
553+
block := body.AppendNewBlock(blockType, labels)
554+
if err := he.encodeNodeAttributes(block.Body(), grandchildVal); err != nil {
555+
return true, err
556+
}
557+
}
558+
} else {
559+
// Single block with this child as label(s)
560+
labels := []string{childKey}
561+
if extraLabels, bodyNode, ok := extractBlockLabels(childVal); ok {
562+
labels = append(labels, extraLabels...)
563+
childVal = bodyNode
564+
}
565+
block := body.AppendNewBlock(blockType, labels)
566+
if err := he.encodeNodeAttributes(block.Body(), childVal); err != nil {
567+
return true, err
568+
}
533569
}
534570
}
535571

pkg/yqlib/hcl_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,68 @@ var hclFormatScenarios = []formatScenario{
345345
expected: "tags:\n - \"a\"\n - \"b\"\n",
346346
scenarioType: "decode",
347347
},
348+
{
349+
description: "roundtrip list of objects",
350+
skipDoc: true,
351+
input: `items = [{ name = "a", value = 1 }, { name = "b", value = 2 }]`,
352+
expected: "items = [{\n name = \"a\"\n value = 1\n }, {\n name = \"b\"\n value = 2\n}]\n",
353+
scenarioType: "roundtrip",
354+
},
355+
{
356+
description: "roundtrip nested blocks with same name",
357+
skipDoc: true,
358+
input: "database \"primary\" {\n host = \"localhost\"\n port = 5432\n}\ndatabase \"replica\" {\n host = \"replica.local\"\n port = 5433\n}",
359+
expected: "database \"primary\" {\n host = \"localhost\"\n port = 5432\n}\ndatabase \"replica\" {\n host = \"replica.local\"\n port = 5433\n}\n",
360+
scenarioType: "roundtrip",
361+
},
362+
{
363+
description: "roundtrip mixed nested structure",
364+
skipDoc: true,
365+
input: "servers \"web\" {\n addresses = [\"10.0.1.1\", \"10.0.1.2\"]\n port = 8080\n}",
366+
expected: "servers \"web\" {\n addresses = [\"10.0.1.1\", \"10.0.1.2\"]\n port = 8080\n}\n",
367+
scenarioType: "roundtrip",
368+
},
369+
{
370+
description: "roundtrip null value",
371+
skipDoc: true,
372+
input: `value = null`,
373+
expected: "value = null\n",
374+
scenarioType: "roundtrip",
375+
},
376+
{
377+
description: "roundtrip empty list",
378+
skipDoc: true,
379+
input: `items = []`,
380+
expected: "items = []\n",
381+
scenarioType: "roundtrip",
382+
},
383+
{
384+
description: "roundtrip empty object",
385+
skipDoc: true,
386+
input: `config = {}`,
387+
expected: "config = {}\n",
388+
scenarioType: "roundtrip",
389+
},
390+
{
391+
description: "Roundtrip: Separate blocks with same name.",
392+
input: "resource \"aws_instance\" \"web\" {\n ami = \"ami-12345\"\n}\nresource \"aws_instance\" \"db\" {\n ami = \"ami-67890\"\n}",
393+
expected: "resource \"aws_instance\" \"web\" {\n ami = \"ami-12345\"\n}\nresource \"aws_instance\" \"db\" {\n ami = \"ami-67890\"\n}\n",
394+
scenarioType: "roundtrip",
395+
},
396+
{
397+
description: "roundtrip deeply nested structure",
398+
skipDoc: true,
399+
input: "app \"database\" \"primary\" \"connection\" {\n host = \"db.local\"\n port = 5432\n}",
400+
expected: "app \"database\" \"primary\" \"connection\" {\n host = \"db.local\"\n port = 5432\n}\n",
401+
scenarioType: "roundtrip",
402+
},
403+
{
404+
description: "roundtrip with leading comments",
405+
skipDoc: true,
406+
input: "# Main config\nenabled = true\nport = 8080",
407+
expected: "# Main config\nenabled = true\nport = 8080\n",
408+
scenarioType: "roundtrip",
409+
},
348410
}
349411

350412
func testHclScenario(t *testing.T, s formatScenario) {

0 commit comments

Comments
 (0)