Add new fmtcmd package

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()`
This commit is contained in:
Dan Carley 2016-01-06 21:53:58 +00:00
parent 197e8d3cf4
commit aee8c49b75
6 changed files with 597 additions and 0 deletions

164
hcl/fmtcmd/fmtcmd.go Normal file
View File

@ -0,0 +1,164 @@
// Derivative work from:
// - https://golang.org/src/cmd/gofmt/gofmt.go
// - https://github.com/fatih/hclfmt
package fmtcmd
import (
"bytes"
"errors"
"fmt"
"io"
"io/ioutil"
"os"
"os/exec"
"path/filepath"
"strings"
"github.com/hashicorp/hcl/hcl/printer"
)
var (
ErrWriteStdin = errors.New("cannot use write option with standard input")
)
type Options struct {
List bool // list files whose formatting differs
Write bool // write result to (source) file instead of stdout
Diff bool // display diffs instead of rewriting files
}
func isValidFile(f os.FileInfo, extensions []string) bool {
if !f.IsDir() && !strings.HasPrefix(f.Name(), ".") {
for _, ext := range extensions {
if strings.HasSuffix(f.Name(), "."+ext) {
return true
}
}
}
return false
}
// If in == nil, the source is the contents of the file with the given filename.
func processFile(filename string, in io.Reader, out io.Writer, stdin bool, opts Options) error {
if in == nil {
f, err := os.Open(filename)
if err != nil {
return err
}
defer f.Close()
in = f
}
src, err := ioutil.ReadAll(in)
if err != nil {
return err
}
res, err := printer.Format(src)
if err != nil {
return err
}
// Files should end with newlines
res = append(res, []byte("\n")...)
if !bytes.Equal(src, res) {
// formatting has changed
if opts.List {
fmt.Fprintln(out, filename)
}
if opts.Write {
err = ioutil.WriteFile(filename, res, 0644)
if err != nil {
return err
}
}
if opts.Diff {
data, err := diff(src, res)
if err != nil {
return fmt.Errorf("computing diff: %s", err)
}
fmt.Fprintf(out, "diff a/%s b/%s\n", filename, filename)
out.Write(data)
}
}
if !opts.List && !opts.Write && !opts.Diff {
_, err = out.Write(res)
}
return err
}
func walkDir(path string, extensions []string, stdout io.Writer, opts Options) error {
visitFile := func(path string, f os.FileInfo, err error) error {
if err == nil && isValidFile(f, extensions) {
err = processFile(path, nil, stdout, false, opts)
}
return err
}
return filepath.Walk(path, visitFile)
}
func Run(
paths, extensions []string,
stdin io.Reader,
stdout io.Writer,
opts Options,
) error {
if len(paths) == 0 {
if opts.Write {
return ErrWriteStdin
}
if err := processFile("<standard input>", stdin, stdout, true, opts); err != nil {
return err
}
return nil
}
for _, path := range paths {
switch dir, err := os.Stat(path); {
case err != nil:
return err
case dir.IsDir():
if err := walkDir(path, extensions, stdout, opts); err != nil {
return err
}
default:
if err := processFile(path, nil, stdout, false, opts); err != nil {
return err
}
}
}
return nil
}
func diff(b1, b2 []byte) (data []byte, err error) {
f1, err := ioutil.TempFile("", "")
if err != nil {
return
}
defer os.Remove(f1.Name())
defer f1.Close()
f2, err := ioutil.TempFile("", "")
if err != nil {
return
}
defer os.Remove(f2.Name())
defer f2.Close()
f1.Write(b1)
f2.Write(b2)
data, err = exec.Command("diff", "-u", f1.Name(), f2.Name()).CombinedOutput()
if len(data) > 0 {
// diff exits with a non-zero status when the files don't match.
// Ignore that failure as long as we get output.
err = nil
}
return
}

431
hcl/fmtcmd/fmtcmd_test.go Normal file
View File

@ -0,0 +1,431 @@
package fmtcmd
import (
"bytes"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"reflect"
"regexp"
"sort"
"syscall"
"testing"
)
var fixtureExtensions = []string{"hcl"}
func init() {
sort.Sort(ByFilename(fixtures))
}
func TestIsValidFile(t *testing.T) {
const fixtureDir = "./test-fixtures"
cases := []struct {
Path string
Expected bool
}{
{"good.hcl", true},
{".hidden.ignore", false},
{"file.ignore", false},
{"dir.ignore", false},
}
for _, tc := range cases {
file, err := os.Stat(filepath.Join(fixtureDir, tc.Path))
if err != nil {
t.Errorf("unexpected error: %s", err)
}
if res := isValidFile(file, fixtureExtensions); res != tc.Expected {
t.Errorf("want: %b, got: %b", tc.Expected, res)
}
}
}
func TestRunMultiplePaths(t *testing.T) {
path1, err := renderFixtures("")
if err != nil {
t.Errorf("unexpected error: %s", err)
}
defer os.RemoveAll(path1)
path2, err := renderFixtures("")
if err != nil {
t.Errorf("unexpected error: %s", err)
}
defer os.RemoveAll(path2)
var expectedOut bytes.Buffer
for _, path := range []string{path1, path2} {
for _, fixture := range fixtures {
if !bytes.Equal(fixture.golden, fixture.input) {
expectedOut.WriteString(filepath.Join(path, fixture.filename) + "\n")
}
}
}
_, stdout := mockIO()
err = Run(
[]string{path1, path2},
fixtureExtensions,
nil, stdout,
Options{
List: true,
},
)
if err != nil {
t.Errorf("unexpected error: %s", err)
}
if stdout.String() != expectedOut.String() {
t.Errorf("stdout want:\n%s\ngot:\n%s", expectedOut, stdout)
}
}
func TestRunSubDirectories(t *testing.T) {
pathParent, err := ioutil.TempDir("", "")
if err != nil {
t.Errorf("unexpected error: %s", err)
}
defer os.RemoveAll(pathParent)
path1, err := renderFixtures(pathParent)
if err != nil {
t.Errorf("unexpected error: %s", err)
}
path2, err := renderFixtures(pathParent)
if err != nil {
t.Errorf("unexpected error: %s", err)
}
paths := []string{path1, path2}
sort.Strings(paths)
var expectedOut bytes.Buffer
for _, path := range paths {
for _, fixture := range fixtures {
if !bytes.Equal(fixture.golden, fixture.input) {
expectedOut.WriteString(filepath.Join(path, fixture.filename) + "\n")
}
}
}
_, stdout := mockIO()
err = Run(
[]string{pathParent},
fixtureExtensions,
nil, stdout,
Options{
List: true,
},
)
if err != nil {
t.Errorf("unexpected error: %s", err)
}
if stdout.String() != expectedOut.String() {
t.Errorf("stdout want:\n%s\ngot:\n%s", expectedOut, stdout)
}
}
func TestRunStdin(t *testing.T) {
var expectedOut bytes.Buffer
for i, fixture := range fixtures {
if i != 0 {
expectedOut.WriteString("\n")
}
expectedOut.Write(fixture.golden)
}
stdin, stdout := mockIO()
for _, fixture := range fixtures {
stdin.Write(fixture.input)
}
err := Run(
[]string{},
fixtureExtensions,
stdin, stdout,
Options{},
)
if err != nil {
t.Errorf("unexpected error: %s", err)
}
if !bytes.Equal(stdout.Bytes(), expectedOut.Bytes()) {
t.Errorf("stdout want:\n%s\ngot:\n%s", expectedOut, stdout)
}
}
func TestRunStdinAndWrite(t *testing.T) {
var expectedOut = []byte{}
stdin, stdout := mockIO()
stdin.WriteString("")
err := Run(
[]string{}, []string{},
stdin, stdout,
Options{
Write: true,
},
)
if err != ErrWriteStdin {
t.Errorf("error want:\n%s\ngot:\n%s", ErrWriteStdin, err)
}
if !bytes.Equal(stdout.Bytes(), expectedOut) {
t.Errorf("stdout want:\n%s\ngot:\n%s", expectedOut, stdout)
}
}
func TestRunFileError(t *testing.T) {
path, err := ioutil.TempDir("", "")
if err != nil {
t.Errorf("unexpected error: %s", err)
}
defer os.RemoveAll(path)
filename := filepath.Join(path, "unreadable.hcl")
var expectedError = &os.PathError{
Op: "open",
Path: filename,
Err: syscall.EACCES,
}
err = ioutil.WriteFile(filename, []byte{}, 0000)
if err != nil {
t.Errorf("unexpected error: %s", err)
}
_, stdout := mockIO()
err = Run(
[]string{path},
fixtureExtensions,
nil, stdout,
Options{},
)
if !reflect.DeepEqual(err, expectedError) {
t.Errorf("error want: %#v, got: %#v", expectedError, err)
}
}
func TestRunNoOptions(t *testing.T) {
path, err := renderFixtures("")
if err != nil {
t.Errorf("unexpected error: %s", err)
}
defer os.RemoveAll(path)
var expectedOut bytes.Buffer
for _, fixture := range fixtures {
expectedOut.Write(fixture.golden)
}
_, stdout := mockIO()
err = Run(
[]string{path},
fixtureExtensions,
nil, stdout,
Options{},
)
if err != nil {
t.Errorf("unexpected error: %s", err)
}
if stdout.String() != expectedOut.String() {
t.Errorf("stdout want:\n%s\ngot:\n%s", expectedOut, stdout)
}
}
func TestRunList(t *testing.T) {
path, err := renderFixtures("")
if err != nil {
t.Errorf("unexpected error: %s", err)
}
defer os.RemoveAll(path)
var expectedOut bytes.Buffer
for _, fixture := range fixtures {
if !bytes.Equal(fixture.golden, fixture.input) {
expectedOut.WriteString(fmt.Sprintln(filepath.Join(path, fixture.filename)))
}
}
_, stdout := mockIO()
err = Run(
[]string{path},
fixtureExtensions,
nil, stdout,
Options{
List: true,
},
)
if err != nil {
t.Errorf("unexpected error: %s", err)
}
if stdout.String() != expectedOut.String() {
t.Errorf("stdout want:\n%s\ngot:\n%s", expectedOut, stdout)
}
}
func TestRunWrite(t *testing.T) {
path, err := renderFixtures("")
if err != nil {
t.Errorf("unexpected error: %s", err)
}
defer os.RemoveAll(path)
_, stdout := mockIO()
err = Run(
[]string{path},
fixtureExtensions,
nil, stdout,
Options{
Write: true,
},
)
if err != nil {
t.Errorf("unexpected error: %s", err)
}
for _, fixture := range fixtures {
res, err := ioutil.ReadFile(filepath.Join(path, fixture.filename))
if err != nil {
t.Errorf("unexpected error: %s", err)
}
if !bytes.Equal(res, fixture.golden) {
t.Errorf("file %q contents want:\n%s\ngot:\n%s", fixture.filename, fixture.golden, res)
}
}
}
func TestRunDiff(t *testing.T) {
path, err := renderFixtures("")
if err != nil {
t.Errorf("unexpected error: %s", err)
}
defer os.RemoveAll(path)
var expectedOut bytes.Buffer
for _, fixture := range fixtures {
if len(fixture.diff) > 0 {
expectedOut.WriteString(
regexp.QuoteMeta(
fmt.Sprintf("diff a/%s/%s b/%s/%s\n", path, fixture.filename, path, fixture.filename),
),
)
// Need to use regex to ignore datetimes in diff.
expectedOut.WriteString(`--- .+?\n`)
expectedOut.WriteString(`\+\+\+ .+?\n`)
expectedOut.WriteString(regexp.QuoteMeta(string(fixture.diff)))
}
}
_, stdout := mockIO()
err = Run(
[]string{path},
fixtureExtensions,
nil, stdout,
Options{
Diff: true,
},
)
if err != nil {
t.Errorf("unexpected error: %s", err)
}
if !regexp.MustCompile(expectedOut.String()).Match(stdout.Bytes()) {
t.Errorf("stdout want match:\n%s\ngot:\n%q", expectedOut, stdout)
}
}
func mockIO() (stdin, stdout *bytes.Buffer) {
return new(bytes.Buffer), new(bytes.Buffer)
}
type fixture struct {
filename string
input, golden, diff []byte
}
type ByFilename []fixture
func (s ByFilename) Len() int { return len(s) }
func (s ByFilename) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
func (s ByFilename) Less(i, j int) bool { return len(s[i].filename) > len(s[j].filename) }
var fixtures = []fixture{
{
"noop.hcl",
[]byte(`resource "aws_security_group" "firewall" {
count = 5
}
`),
[]byte(`resource "aws_security_group" "firewall" {
count = 5
}
`),
[]byte(``),
}, {
"align_equals.hcl",
[]byte(`variable "foo" {
default = "bar"
description = "bar"
}
`),
[]byte(`variable "foo" {
default = "bar"
description = "bar"
}
`),
[]byte(`@@ -1,4 +1,4 @@
variable "foo" {
- default = "bar"
+ default = "bar"
description = "bar"
}
`),
}, {
"indentation.hcl",
[]byte(`provider "aws" {
access_key = "foo"
secret_key = "bar"
}
`),
[]byte(`provider "aws" {
access_key = "foo"
secret_key = "bar"
}
`),
[]byte(`@@ -1,4 +1,4 @@
provider "aws" {
- access_key = "foo"
- secret_key = "bar"
+ access_key = "foo"
+ secret_key = "bar"
}
`),
},
}
// parent can be an empty string, in which case the system's default
// temporary directory will be used.
func renderFixtures(parent string) (path string, err error) {
path, err = ioutil.TempDir(parent, "")
if err != nil {
return "", err
}
for _, fixture := range fixtures {
err = ioutil.WriteFile(filepath.Join(path, fixture.filename), []byte(fixture.input), 0644)
if err != nil {
os.RemoveAll(path)
return "", err
}
}
return path, nil
}

View File

@ -0,0 +1 @@
invalid

View File

View File

@ -0,0 +1 @@
invalid

View File