This uses the `printer` package to normalise HCL files. It's inspired by
github.com/fatih/hclfmt - but it differs in that it's intended to be more
generic, re-usable, and follow the semantics of `gofmt` or `go fmt`.
I intend to utilise this in Terraform by implementing `terraform fmt`
sub-command which will normalise all files in the current working directory
(like `go fmt ./..`) or contents over STDIN (with `terraform fmt -` that
could be used by editor plugins). So that Terraform users can benefit from
linting without installing another package/binary.
I hope that by placing most of the logic in the HCL package it should be
easy to implement sub-commands in other projects that also use HCL.
Some notes about the implementation:
- the significant difference from `gofmt` is that STDIN/STDOUT are passed in
and errors aren't logged/written directly to STDERR, which gives consumers
(e.g. Terraform) the ability to set appropriate exit codes
- I chose to use inline fixtures instead of files because there were a
number of times where I needed to reference the contents and group them
together with diff output
- it seemed simplest to construct the expected outputs by looping over the
relevant fixtures and building up a string/byte slice, hope it isn't too
confusing to read
- the test failure reporting is kind of rough because the outputs are so
large, but I didn't want to add another diff function
- I chose to have a separate test for sub-directories rather than making it
the default in the fixtures so that I didn't need to add additional logic
to the fixture rendering
- the fixtures are sorted by filename before any of the tests runs so that
they match the order that they are read from disk by `filepath.Walk()`
Several of the esoteric syntax errors we encounter in Terraform have
bubbled up errors from the decoder. Since all these errors have a Node
in context, they can report their position which makes them _much_ more
helpful to the user, even if the error message itself is confusing.
I tried to move around PosError somewhere but got stuck finding a good
spot for it that doesn't have either a silly name or an import cycle, so
for now I'm just cross referencing into the parser package to reference
the error type.
A user testing an RC release of Terraform 0.6.7 found the following
string unexpectedly caused a syntax error:
"echo ${var.region}${element(split(",",var.zones),0)}"
This fixes the error and covers it with a test.
Before the parser rewrite, HCL would silently convert `\"` within
interpolation braces to `"`.
After the conversion, this became a syntax error.
We've found several instances of Terraform configs in the wild using
this syntax. It results in a hard "syntax error" message during config
parsing. While avoiding the extra escape on double quotes within
interpolations is definitely preferred, the UX of the syntax error feels
harsh enough to be worth inserting this backwards compatibility for now,
leaving us the option of deprecating it with a warning down the line.
With the previous Walk function it's not easy to rewrite the node as we
don't have any kind of reference to the parent. If we want to rewrite a
given AST, we have to manually traverse it as Walk is not usable. To
allow us rewriting the AST we change the signature of the function
passed to Walk. It'll allow us to rewrite the AST and return back.
Internally Walk() overrides the returned AST.
This idea was also talked here:
https://groups.google.com/forum/#!topic/golang-nuts/cRZQV36IckM
extensively.
