command: "terraform fmt" must fail if file has invalid syntax

Since the HCL formatter only works with tokens, it can in principle be
called with any input and produce some output. However, when given invalid
syntax it will tend to produce nonsensical results that may drastically
change the input file and be hard for the user to undo.

Since there's no strong reason to try to format an invalid or incomplete
file, we'll instead try parsing first and fail if parsing does not
complete successfully.

Since we talk directly to the HCL API here this is only a _syntax_ check,
and so it can be applied to files that are invalid in other ways as far
as Terraform is concerned, such as using unsupported top-level block types,
resource types that don't exist, etc.
This commit is contained in:
Martin Atkins 2019-01-17 10:44:37 -08:00
parent a10f9a18cf
commit b0a43cab84
2 changed files with 47 additions and 6 deletions

View File

@ -11,11 +11,12 @@ import (
"path/filepath" "path/filepath"
"strings" "strings"
"github.com/hashicorp/terraform/configs" "github.com/hashicorp/hcl2/hcl"
"github.com/hashicorp/hcl2/hcl/hclsyntax"
"github.com/hashicorp/hcl2/hclwrite"
"github.com/mitchellh/cli" "github.com/mitchellh/cli"
"github.com/hashicorp/hcl2/hclwrite" "github.com/hashicorp/terraform/configs"
"github.com/hashicorp/terraform/tfdiags" "github.com/hashicorp/terraform/tfdiags"
) )
@ -140,6 +141,15 @@ func (c *FmtCommand) processFile(path string, r io.Reader, w io.Writer, isStdout
return diags return diags
} }
// File must be parseable as HCL native syntax before we'll try to format
// it. If not, the formatter is likely to make drastic changes that would
// be hard for the user to undo.
_, syntaxDiags := hclsyntax.ParseConfig(src, path, hcl.Pos{Line: 1, Column: 1})
if syntaxDiags.HasErrors() {
diags = diags.Append(syntaxDiags)
return diags
}
result := hclwrite.Format(src) result := hclwrite.Format(src)
if !bytes.Equal(src, result) { if !bytes.Equal(src, result) {

View File

@ -12,7 +12,7 @@ import (
"github.com/mitchellh/cli" "github.com/mitchellh/cli"
) )
func TestFmt_errorReporting(t *testing.T) { func TestFmt_nonexist(t *testing.T) {
tempDir := fmtFixtureWriteDir(t) tempDir := fmtFixtureWriteDir(t)
ui := new(cli.MockUi) ui := new(cli.MockUi)
@ -23,8 +23,8 @@ func TestFmt_errorReporting(t *testing.T) {
}, },
} }
dummy_file := filepath.Join(tempDir, "doesnotexist") missingDir := filepath.Join(tempDir, "doesnotexist")
args := []string{dummy_file} args := []string{missingDir}
if code := c.Run(args); code != 2 { if code := c.Run(args); code != 2 {
t.Fatalf("wrong exit code. errors: \n%s", ui.ErrorWriter.String()) t.Fatalf("wrong exit code. errors: \n%s", ui.ErrorWriter.String())
} }
@ -35,6 +35,37 @@ func TestFmt_errorReporting(t *testing.T) {
} }
} }
func TestFmt_syntaxError(t *testing.T) {
tempDir := testTempDir(t)
invalidSrc := `
a = 1 +
`
err := ioutil.WriteFile(filepath.Join(tempDir, "invalid.tf"), []byte(invalidSrc), 0644)
if err != nil {
t.Fatal(err)
}
ui := new(cli.MockUi)
c := &FmtCommand{
Meta: Meta{
testingOverrides: metaOverridesForProvider(testProvider()),
Ui: ui,
},
}
args := []string{tempDir}
if code := c.Run(args); code != 2 {
t.Fatalf("wrong exit code. errors: \n%s", ui.ErrorWriter.String())
}
expected := "Invalid expression"
if actual := ui.ErrorWriter.String(); !strings.Contains(actual, expected) {
t.Fatalf("expected:\n%s\n\nto include: %q", actual, expected)
}
}
func TestFmt_tooManyArgs(t *testing.T) { func TestFmt_tooManyArgs(t *testing.T) {
ui := new(cli.MockUi) ui := new(cli.MockUi)
c := &FmtCommand{ c := &FmtCommand{