reorder_attributes emits attribute pairs fused on one line for certain source layouts
- Dominant language
- Go
- Stars
- 60
- Forks
- 19
- Avg merge
- 2h 2m
- Merged PRs (30d)
- 4
Description
## Summary
`reorder_attributes` emits attributes via `body.AppendUnstructuredTokens(el.attr.BuildTokens(nil))` (`pkg/transform_reorder_attributes.go:299`). For certain attribute pairs the resulting tokens fuse onto one line — e.g. `source = "./submod" location = "westeurope"` — producing output that requires `terraform fmt` to clean up. `terraform fmt` does fix it, so this is **cosmetic only**; the output is not structurally invalid HCL.
Surfaced during governance-side verification of [PR #106](https://github.com/Azure/mapotf/pull/106) by the `Azure/avm-terraform-governance` team. Not a regression introduced by #106 — pre-existing behaviour in `reorder_attributes` made visible by their G4 module-input ordering experiment.
## Reproduction
Before:
```hcl
module "scrambled" {
tags = { env = "test" }
enable_telemetry = false
name = "rg-example"
location = "westeurope"
source = "./submod"
}
```
Transform:
```hcl
transform "reorder_attributes" "module_inputs" {
target_block_address = "module.scrambled"
head_attributes = ["source", "version", "for_each", "count", "providers"]
body_attributes = ["location", "name", "enable_telemetry", "tags"]
foot_attributes = ["depends_on"]
}
```
Actual output (note the fused first line):
```hcl
module "scrambled" {
source = "./submod" location = "westeurope"
name = "rg-example"
enable_telemetry = false
tags = { env = "test" }
}
```
Expected output:
```hcl
module "scrambled" {
source = "./submod"
location = "westeurope"
name = "rg-example"
enable_telemetry = false
tags = { env = "test" }
}
```
## Likely cause
`pkg/transform_reorder_attributes.go:275-302` `emitReorderElements`:
```go
if el.isNested {
body.AppendBlock(el.block)
} else {
body.AppendUnstructuredTokens(el.attr.BuildTokens(nil))
}
```
`hclwrite.Attribute.BuildTokens` returns the attribute's complete token stream including leading whitespace/indent that was captured at parse time. `AppendUnstructuredTokens` is a raw token splice — it doesn't insert newlines on attribute boundaries the way `SetAttributeRaw` does. When an attribute's parse-time leading whitespace was a single space (because it followed a `{` opener, say) it gets appended after the previous attribute's trailing tokens without a newline in between.
The `body.AppendNewline()` calls at `emitReorderElements:293` and the initial `body.AppendNewline()` at `Apply:92` mitigate the section-boundary case for blocks (`AppendBlock` does its own newline management), but the attribute-only path relies on `BuildTokens` always returning a trailing newline. For attributes that originally lacked one (e.g. last attribute of the source block before the closing `}`), it doesn't.
## Suggested fix candidates
**Option A (smallest)** — switch to `SetAttributeRaw` for attributes:
```go
if el.isNested {
body.AppendBlock(el.block)
} else {
body.SetAttributeRaw(el.attr.Name(), el.attr.Expr().BuildTokens(nil))
}
```
`SetAttributeRaw` handles newline insertion correctly. Caveat: drops any leading comments/whitespace originally attached to the attribute (`BuildTokens` preserves these, `SetAttributeRaw` doesn't). Need a test to confirm whether the comment-preservation requirement was real.
**Option B (preserves comments)** — append an explicit `Newline` token after each attribute when the last token isn't a newline:
```go
if el.isNested {
body.AppendBlock(el.block)
} else {
tokens := el.attr.BuildTokens(nil)
body.AppendUnstructuredTokens(tokens)
if len(tokens) > 0 && tokens[len(tokens)-1].Type != hclsyntax.TokenNewline {
body.AppendNewline()
}
}
```
Imports `hclsyntax`. More defensive.
## Severity
Cosmetic. `terraform fmt -recursive` cleans the file on next run, so pipelines that run `mapotf transform` + `terraform fmt` together (which is the typical AVM `make pre-commit` pattern) are unaffected. Standalone `mapotf transform` users without a follow-up `terraform fmt` see ugly output.
**Not v0.1.4 blocking.** Suggest target v0.1.5.
## Acceptance criteria
- Add a regression test in `pkg/transform_reorder_attributes_test.go` with the reproduction fixture above (or similar — last-source-attribute-promoted-to-head) asserting line-by-line output equality with the expected layout
- All existing `reorder_attributes` tests still pass
- Output remains `terraform fmt`-stable (running `terraform fmt` on the output produces zero diff)
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.