Merge pull request #10457 from teddylear/feature/recursivefmt-2

Adding recursive flag to formatter to format subdirectories

command/build_test.go

@@ -389,7 +389,7 @@ func TestBuild(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			defer tt.cleanup(t)
 			run(t, tt.args, tt.expectedCode)
-			tt.fileCheck.verify(t)
+			tt.fileCheck.verify(t, "")
 		})
 	}
 }
@@ -732,7 +732,7 @@ func TestHCL2PostProcessorForceFlag(t *testing.T) {
 	if code := c.Run(args); code != 0 {
 		fatalCommand(t, c.Meta)
 	}
-	fCheck.verify(t)
+	fCheck.verify(t, "")
 
 	// Second build should override previous manifest
 	UUID, _ = uuid.GenerateUUID()
@@ -766,7 +766,7 @@ func TestHCL2PostProcessorForceFlag(t *testing.T) {
 	if code := c.Run(args); code != 0 {
 		fatalCommand(t, c.Meta)
 	}
-	fCheck.verify(t)
+	fCheck.verify(t, "")
 }
 
 func TestBuildCommand_HCLOnlyExceptOptions(t *testing.T) {
@@ -887,19 +887,19 @@ func (fc fileCheck) expectedFiles() []string {
 	return expected
 }
 
-func (fc fileCheck) verify(t *testing.T) {
+func (fc fileCheck) verify(t *testing.T, dir string) {
 	for _, f := range fc.expectedFiles() {
-		if !fileExists(f) {
+		if _, err := os.Stat(filepath.Join(dir, f)); err != nil {
-			t.Errorf("Expected to find %s", f)
+			t.Errorf("Expected to find %s: %v", f, err)
 		}
 	}
 	for _, f := range fc.notExpected {
-		if fileExists(f) {
+		if _, err := os.Stat(filepath.Join(dir, f)); err == nil {
 			t.Errorf("Expected to not find %s", f)
 		}
 	}
 	for file, expectedContent := range fc.expectedContent {
-		content, err := ioutil.ReadFile(file)
+		content, err := ioutil.ReadFile(filepath.Join(dir, file))
 		if err != nil {
 			t.Fatalf("ioutil.ReadFile: %v", err)
 		}
@@ -909,14 +909,6 @@ func (fc fileCheck) verify(t *testing.T) {
 	}
 }
 
-// fileExists returns true if the filename is found
-func fileExists(filename string) bool {
-	if _, err := os.Stat(filename); err == nil {
-		return true
-	}
-	return false
-}
-
 // testCoreConfigBuilder creates a packer CoreConfig that has a file builder
 // available. This allows us to test a builder that writes files to disk.
 func testCoreConfigBuilder(t *testing.T) *packer.CoreConfig {

command/cli.go

@@ -161,12 +161,12 @@ func (va *FormatArgs) AddFlagSets(flags *flag.FlagSet) {
 	flags.BoolVar(&va.Check, "check", false, "check if the input is formatted")
 	flags.BoolVar(&va.Diff, "diff", false, "display the diff of formatting changes")
 	flags.BoolVar(&va.Write, "write", true, "overwrite source files instead of writing to stdout")
-
+	flags.BoolVar(&va.Recursive, "recursive", false, "Also process files in subdirectories")
 	va.MetaArgs.AddFlagSets(flags)
 }
 
 // FormatArgs represents a parsed cli line for `packer fmt`
 type FormatArgs struct {
 	MetaArgs
-	Check, Diff, Write bool
+	Check, Diff, Write, Recursive bool
 }

command/exec_test.go

@@ -87,6 +87,8 @@ func TestHelperProcess(*testing.T) {
 	switch cmd {
 	case "console":
 		os.Exit((&ConsoleCommand{Meta: commandMeta()}).Run(args))
+	case "fmt":
+		os.Exit((&FormatCommand{Meta: commandMeta()}).Run(args))
 	case "inspect":
 		os.Exit((&InspectCommand{Meta: commandMeta()}).Run(args))
 	case "build":

command/fmt.go

@@ -51,6 +51,7 @@ func (c *FormatCommand) RunContext(ctx context.Context, cla *FormatArgs) int {
 		ShowDiff:  cla.Diff,
 		Write:     cla.Write,
 		Output:    os.Stdout,
+		Recursive: cla.Recursive,
 	}
 
 	bytesModified, diags := formatter.Format(cla.Path)
@@ -90,6 +91,8 @@ Options:
   -write=false  Don't write to source files
                 (always disabled if using -check)
 
+  -recursive     Also process files in subdirectories. By default, only the
+                 given directory (or current directory) is processed.
 `
 
 	return strings.TrimSpace(helpText)
@@ -108,5 +111,6 @@ func (*FormatCommand) AutocompleteFlags() complete.Flags {
 		"-check":     complete.PredictNothing,
 		"-diff":      complete.PredictNothing,
 		"-write":     complete.PredictNothing,
+		"-recursive": complete.PredictNothing,
 	}
 }

command/fmt_test.go

@@ -1,10 +1,15 @@
 package command
 
 import (
+	"bytes"
+	"fmt"
+	"io/ioutil"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
 	"github.com/stretchr/testify/assert"
 )
@@ -50,3 +55,122 @@ func TestFmt_unfomattedTemlateDirectory(t *testing.T) {
 		fatalCommand(t, c.Meta)
 	}
 }
+
+const (
+	unformattedHCL = `
+ami_filter_name ="amzn2-ami-hvm-*-x86_64-gp2"
+ami_filter_owners =[ "137112412989" ]
+
+`
+	formattedHCL = `
+ami_filter_name   = "amzn2-ami-hvm-*-x86_64-gp2"
+ami_filter_owners = ["137112412989"]
+
+`
+)
+
+func TestFmt_Recursive(t *testing.T) {
+
+	tests := []struct {
+		name                  string
+		formatArgs            []string // arguments passed to format
+		alreadyPresentContent map[string]string
+		fileCheck
+	}{
+		{
+			name:       "nested formats recursively",
+			formatArgs: []string{"-recursive=true"},
+			alreadyPresentContent: map[string]string{
+				"foo/bar/baz.pkr.hcl":         unformattedHCL,
+				"foo/bar/baz/woo.pkrvars.hcl": unformattedHCL,
+				"potato":                      unformattedHCL,
+				"foo/bar/potato":              unformattedHCL,
+				"bar.pkr.hcl":                 unformattedHCL,
+				"-":                           unformattedHCL,
+			},
+			fileCheck: fileCheck{
+				expectedContent: map[string]string{
+					"foo/bar/baz.pkr.hcl":         formattedHCL,
+					"foo/bar/baz/woo.pkrvars.hcl": formattedHCL,
+					"potato":                      unformattedHCL,
+					"foo/bar/potato":              unformattedHCL,
+					"bar.pkr.hcl":                 formattedHCL,
+					"-":                           unformattedHCL,
+				}},
+		},
+		{
+			name:       "nested no recursive format",
+			formatArgs: []string{},
+			alreadyPresentContent: map[string]string{
+				"foo/bar/baz.pkr.hcl":         unformattedHCL,
+				"foo/bar/baz/woo.pkrvars.hcl": unformattedHCL,
+				"bar.pkr.hcl":                 unformattedHCL,
+				"-":                           unformattedHCL,
+			},
+			fileCheck: fileCheck{
+				expectedContent: map[string]string{
+					"foo/bar/baz.pkr.hcl":         unformattedHCL,
+					"foo/bar/baz/woo.pkrvars.hcl": unformattedHCL,
+					"bar.pkr.hcl":                 formattedHCL,
+					"-":                           unformattedHCL,
+				}},
+		},
+	}
+
+	c := &FormatCommand{
+		Meta: testMeta(t),
+	}
+
+	testDir := "test-fixtures/fmt"
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tempDirectory := mustString(ioutil.TempDir(testDir, "test-dir-*"))
+			defer os.RemoveAll(tempDirectory)
+
+			createFiles(tempDirectory, tt.alreadyPresentContent)
+
+			testArgs := append(tt.formatArgs, tempDirectory)
+			if code := c.Run(testArgs); code != 0 {
+				ui := c.Meta.Ui.(*packersdk.BasicUi)
+				out := ui.Writer.(*bytes.Buffer)
+				err := ui.ErrorWriter.(*bytes.Buffer)
+				t.Fatalf(
+					"Bad exit code for test case: %s.\n\nStdout:\n\n%s\n\nStderr:\n\n%s",
+					tt.name,
+					out.String(),
+					err.String())
+			}
+
+			tt.fileCheck.verify(t, tempDirectory)
+		})
+	}
+}
+
+func Test_fmt_pipe(t *testing.T) {
+
+	tc := []struct {
+		piped    string
+		command  []string
+		env      []string
+		expected string
+	}{
+		{unformattedHCL, []string{"fmt", "-"}, nil, formattedHCL},
+	}
+
+	for _, tc := range tc {
+		t.Run(fmt.Sprintf("echo %q | packer %s", tc.piped, tc.command), func(t *testing.T) {
+			p := helperCommand(t, tc.command...)
+			p.Stdin = strings.NewReader(tc.piped)
+			p.Env = append(p.Env, tc.env...)
+			fmt.Println(fmt.Sprintf("Path: %s", p.Path))
+			bs, err := p.Output()
+			if err != nil {
+				t.Fatalf("Error occurred running command %v: %s", err, bs)
+			}
+			if diff := cmp.Diff(tc.expected, string(bs)); diff != "" {
+				t.Fatalf("Error in diff: %s", diff)
+			}
+		})
+	}
+}

command/utils_test.go

@@ -39,3 +39,11 @@ func (c *configDirSingleton) dir(key string) string {
 	c.dirs[key] = mustString(ioutil.TempDir("", "pkr-test-cfg-dir-"+key))
 	return c.dirs[key]
 }
+
+// fileExists returns true if the filename is found
+func fileExists(filename string) bool {
+	if _, err := os.Stat(filename); err == nil {
+		return true
+	}
+	return false
+}

hcl2template/formatter.go

@@ -7,6 +7,8 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"path/filepath"
+	"strings"
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/hclparse"
@@ -14,7 +16,7 @@ import (
 )
 
 type HCL2Formatter struct {
-	ShowDiff, Write bool
+	ShowDiff, Write, Recursive bool
 	Output                     io.Writer
 	parser                     *hclparse.Parser
 }
@@ -26,55 +28,77 @@ func NewHCL2Formatter() *HCL2Formatter {
 	}
 }
 
+func isHcl2FileOrVarFile(path string) bool {
+	if strings.HasSuffix(path, hcl2FileExt) || strings.HasSuffix(path, hcl2VarFileExt) {
+		return true
+	}
+	return false
+}
+
+func (f *HCL2Formatter) formatFile(path string, diags hcl.Diagnostics, bytesModified int) (int, hcl.Diagnostics) {
+	data, err := f.processFile(path)
+	if err != nil {
+		diags = append(diags, &hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  fmt.Sprintf("encountered an error while formatting %s", path),
+			Detail:   err.Error(),
+		})
+	}
+	bytesModified += len(data)
+	return bytesModified, diags
+}
+
 // Format all HCL2 files in path and return the total bytes formatted.
 // If any error is encountered, zero bytes will be returned.
 //
 // Path can be a directory or a file.
 func (f *HCL2Formatter) Format(path string) (int, hcl.Diagnostics) {
+	var diags hcl.Diagnostics
+	var bytesModified int
 
-	var allHclFiles []string
+	if path == "" {
-	var diags []*hcl.Diagnostic
-
-	if path == "-" {
-		allHclFiles = []string{"-"}
-	} else {
-		hclFiles, _, diags := GetHCL2Files(path, hcl2FileExt, hcl2JsonFileExt)
-		if diags.HasErrors() {
-			return 0, diags
-		}
-
-		hclVarFiles, _, diags := GetHCL2Files(path, hcl2VarFileExt, hcl2VarJsonFileExt)
-		if diags.HasErrors() {
-			return 0, diags
-		}
-
-		allHclFiles = append(hclFiles, hclVarFiles...)
-
-		if len(allHclFiles) == 0 {
 		diags = append(diags, &hcl.Diagnostic{
 			Severity: hcl.DiagError,
-				Summary:  fmt.Sprintf("Cannot tell whether %s contains HCL2 configuration data", path),
+			Summary:  "path is empty, cannot format",
+			Detail:   "path is empty, cannot format",
 		})
-
+		return bytesModified, diags
-			return 0, diags
-		}
 	}
 
 	if f.parser == nil {
 		f.parser = hclparse.NewParser()
 	}
 
-	var bytesModified int
+	if s, err := os.Stat(path); err != nil || !s.IsDir() {
-	for _, fn := range allHclFiles {
+		return f.formatFile(path, diags, bytesModified)
-		data, err := f.processFile(fn)
+	}
-		if err != nil {
+
-			diags = append(diags, &hcl.Diagnostic{
+	fileInfos, err := ioutil.ReadDir(path)
-				Severity: hcl.DiagError,
+	if err != nil {
-				Summary:  fmt.Sprintf("encountered an error while formatting %s", fn),
+		diag := &hcl.Diagnostic{
-				Detail:   err.Error(),
+			Severity: hcl.DiagError,
-			})
+			Summary:  "Cannot read hcl directory",
+			Detail:   err.Error(),
+		}
+		diags = append(diags, diag)
+		return bytesModified, diags
+	}
+
+	for _, fileInfo := range fileInfos {
+		filename := filepath.Join(path, fileInfo.Name())
+		if fileInfo.IsDir() {
+			if f.Recursive {
+				var tempDiags hcl.Diagnostics
+				var tempBytesModified int
+				tempBytesModified, tempDiags = f.Format(filename)
+				bytesModified += tempBytesModified
+				diags = diags.Extend(tempDiags)
+			}
+			continue
+		}
+		if isHcl2FileOrVarFile(filename) {
+			bytesModified, diags = f.formatFile(filename, diags, bytesModified)
 		}
-		bytesModified += len(data)
 	}
 
 	return bytesModified, diags
@@ -84,6 +108,7 @@ func (f *HCL2Formatter) Format(path string) (int, hcl.Diagnostics) {
 // overwriting the contents of the original when the f.Write is true; a diff of the changes
 // will be outputted if f.ShowDiff is true.
 func (f *HCL2Formatter) processFile(filename string) ([]byte, error) {
+
 	if f.Output == nil {
 		f.Output = os.Stdout
 	}

hcl2template/formatter_test.go

@@ -32,11 +32,9 @@ func TestHCL2Formatter_Format(t *testing.T) {
 		if diags.HasErrors() {
 			t.Fatalf("the call to Format failed unexpectedly %s", diags.Error())
 		}
-
 		if buf.String() != "" && tc.FormatExpected == false {
 			t.Errorf("Format(%q) should contain the name of the formatted file(s), but got %q", tc.Path, buf.String())
 		}
-
 	}
 }