Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add terramate fmt #351

Merged
merged 37 commits into from
May 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
70949a6
refactor: extract predicate to check tm files
katcipis May 13, 2022
331de56
test: add tests for ignored dotfiles on genhcl/file
katcipis May 13, 2022
df90f3f
refactor: use listTerramateFiles on parser
katcipis May 13, 2022
1adb846
fix: code generation ignores dotfiles
katcipis May 13, 2022
b0a045e
feat: add hcl.Format
katcipis May 13, 2022
015e69e
feat: add hcl.FormatFile
katcipis May 13, 2022
6a7e026
feat: add hcl.FormatTree
katcipis May 13, 2022
2f51327
feat: finish hcl.FormatTree
katcipis May 13, 2022
a6def35
chore: add test list
katcipis May 13, 2022
69b1eca
Merge branch 'main' of github.com:mineiros-io/terramate into katcipis…
katcipis May 16, 2022
8b9e99a
feat: add new assert function
katcipis May 16, 2022
45d1c60
chore: use new assert func on hcl test
katcipis May 16, 2022
7008447
feat: add proper validation on format
katcipis May 16, 2022
0d53a2f
Merge branch 'main' into katcipis-add-fmt
katcipis May 16, 2022
d21fec6
test: check that files are untouched
katcipis May 16, 2022
22c868b
feat: add method to save formatting result
katcipis May 16, 2022
1aea15f
refactor: rename FmtRes to FormatResult
katcipis May 16, 2022
7160f5c
test: check errors for all files
katcipis May 16, 2022
dff097f
feat: hcl.FormatTree returns all errors on all files
katcipis May 16, 2022
47d1f1f
refactor: make formatting result immutable
katcipis May 17, 2022
3911bd8
chore: improve docs
katcipis May 17, 2022
5cc555f
test: add CLI fmt first test
katcipis May 17, 2022
2bbf873
chore: add license info
katcipis May 17, 2022
622755b
feat: add fmt on CLI
katcipis May 17, 2022
22bf5cc
test: add tests for empty/formatted project
katcipis May 17, 2022
f22439e
feat: check code that is already formatted
katcipis May 17, 2022
a7e4405
refactor: remove unused sentinel
katcipis May 17, 2022
54fa336
test: add more tree tests
katcipis May 17, 2022
8bda361
refactor: improve test name
katcipis May 17, 2022
77e589f
test: add error handling tests for dirs/files access
katcipis May 17, 2022
0416c76
chore: fix typo
katcipis May 17, 2022
fbf01da
feat: use our hcl.Format on generate_hcl
katcipis May 18, 2022
41a8911
test: use new Format function on test.hclwrite
katcipis May 18, 2022
8e23219
Merge branch 'main' of github.com:mineiros-io/terramate into katcipis…
katcipis May 19, 2022
e2015f7
fix: use filepath.Separator for portability
katcipis May 19, 2022
0860d8b
test: add check for subdir for fmt
katcipis May 19, 2022
b9ab265
test: add formatting on subdirs tests
katcipis May 19, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 53 additions & 1 deletion cmd/terramate/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ type cliSpec struct {
DisableCheckGitUntracked bool `optional:"true" default:"false" help:"Disable git check for untracked files"`
DisableCheckGitUncommitted bool `optional:"true" default:"false" help:"Disable git check for uncommitted files"`

Fmt struct {
Check bool `help:"Lists unformatted files, exit with 0 if all is formatted, 1 otherwise"`
} `cmd:"" help:"List stacks"`

List struct {
Why bool `help:"Shows the reason why the stack has changed"`
} `cmd:"" help:"List stacks"`
Expand Down Expand Up @@ -322,6 +326,8 @@ func (c *cli) run() {
logger.Debug().Msg("Handle command.")

switch c.ctx.Command() {
case "fmt":
c.format()
case "list":
c.printStacks()
case "run":
Expand All @@ -343,7 +349,7 @@ func (c *cli) run() {
case "experimental run-order":
c.printRunOrder()
default:
log.Fatal().Msg("unexpected command sequence")
logger.Fatal().Msg("unexpected command sequence")
}
}

Expand Down Expand Up @@ -465,6 +471,52 @@ func (c *cli) listStacks(mgr *terramate.Manager, isChanged bool) (*terramate.Sta
return mgr.List()
}

func (c *cli) format() {
logger := log.With().
Str("workingDir", c.wd()).
Str("action", "format()").
Logger()

logger.Trace().Msg("formatting all files recursively")
results, err := hcl.FormatTree(c.wd())
if err != nil {
logger.Fatal().Err(err).Msg("formatting files")
}

logger.Trace().Msg("listing formatted files")
for _, res := range results {
path := strings.TrimPrefix(res.Path(), c.wd()+string(filepath.Separator))
c.log(path)
}

if c.parsedArgs.Fmt.Check {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

top

logger.Trace().Msg("checking if we have unformatted files")
if len(results) > 0 {
logger.Trace().Msg("we have unformatted files")
os.Exit(1)
}
logger.Trace().Msg("all files formatted, nothing else to do")
return
}

logger.Trace().Msg("saving formatted files")

errs := errors.L()
for _, res := range results {
logger := log.With().
Str("workingDir", c.wd()).
Str("filepath", res.Path()).
Str("action", "format()").
Logger()
logger.Trace().Msg("saving formatted file")
errs.Append(res.Save())
}

if err := errs.AsError(); err != nil {
logger.Fatal().Err(err).Msg("saving files")
}
}

func (c *cli) printStacks() {
logger := log.With().
Str("action", "printStacks()").
Expand Down
180 changes: 180 additions & 0 deletions cmd/terramate/e2etests/fmt_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
// Copyright 2022 Mineiros GmbH
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package e2etest

import (
"fmt"
"path/filepath"
"strings"
"testing"

"github.com/madlambda/spells/assert"
"github.com/mineiros-io/terramate/hcl"
"github.com/mineiros-io/terramate/test/sandbox"
)

func TestFormatRecursively(t *testing.T) {
const unformattedHCL = `
globals {
name = "name"
description = "desc"
test = true
}
`
formattedHCL, err := hcl.Format(unformattedHCL, "")
assert.NoError(t, err)

s := sandbox.New(t)
cli := newCLI(t, s.RootDir())

t.Run("checking succeeds when there is no Terramate files", func(t *testing.T) {
assertRunResult(t, cli.run("fmt", "--check"), runExpected{})
})

t.Run("formatting succeeds when there is no Terramate files", func(t *testing.T) {
assertRunResult(t, cli.run("fmt"), runExpected{})
})

sprintf := fmt.Sprintf
writeUnformattedFiles := func() {
s.BuildTree([]string{
sprintf("f:globals.tm:%s", unformattedHCL),
sprintf("f:another-stacks/globals.tm.hcl:%s", unformattedHCL),
sprintf("f:another-stacks/stack-1/globals.tm.hcl:%s", unformattedHCL),
sprintf("f:another-stacks/stack-2/globals.tm.hcl:%s", unformattedHCL),
sprintf("f:stacks/globals.tm:%s", unformattedHCL),
sprintf("f:stacks/stack-1/globals.tm:%s", unformattedHCL),
sprintf("f:stacks/stack-2/globals.tm:%s", unformattedHCL),
})
}

writeUnformattedFiles()

wantedFiles := []string{
"globals.tm",
"another-stacks/globals.tm.hcl",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add a test with --chdir into the generated tree to check if the file paths are expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks for catching this (actually I was aware the tests were lacking and went into rush mode anyway, but it is good investment to improve that now, it didn't even take that long 😆 )

"another-stacks/stack-1/globals.tm.hcl",
"another-stacks/stack-2/globals.tm.hcl",
"stacks/globals.tm",
"stacks/stack-1/globals.tm",
"stacks/stack-2/globals.tm",
}
filesListOutput := func(files []string) string {
return strings.Join(files, "\n") + "\n"
}
wantedFilesStr := filesListOutput(wantedFiles)

assertFileContents := func(t *testing.T, path string, want string) {
t.Helper()
got := s.RootEntry().ReadFile(path)
assert.EqualStrings(t, want, string(got))
}

assertWantedFilesContents := func(t *testing.T, want string) {
t.Helper()

for _, file := range wantedFiles {
assertFileContents(t, file, want)
}
}

t.Run("checking fails with unformatted files", func(t *testing.T) {
assertRunResult(t, cli.run("fmt", "--check"), runExpected{
Status: 1,
Stdout: wantedFilesStr,
})
assertWantedFilesContents(t, unformattedHCL)
})

t.Run("checking fails with unformatted files on subdirs", func(t *testing.T) {
subdir := filepath.Join(s.RootDir(), "another-stacks")
cli := newCLI(t, subdir)
assertRunResult(t, cli.run("fmt", "--check"), runExpected{
Status: 1,
Stdout: filesListOutput([]string{
"globals.tm.hcl",
"stack-1/globals.tm.hcl",
"stack-2/globals.tm.hcl",
}),
})
assertWantedFilesContents(t, unformattedHCL)
})

t.Run("update unformatted files in place", func(t *testing.T) {
assertRunResult(t, cli.run("fmt"), runExpected{
Stdout: wantedFilesStr,
})
assertWantedFilesContents(t, formattedHCL)
})

t.Run("checking succeeds when all files are formatted", func(t *testing.T) {
assertRunResult(t, cli.run("fmt", "--check"), runExpected{})
assertWantedFilesContents(t, formattedHCL)
})

t.Run("formatting succeeds when all files are formatted", func(t *testing.T) {
assertRunResult(t, cli.run("fmt"), runExpected{})
assertWantedFilesContents(t, formattedHCL)
})

t.Run("update unformatted files in subdirs", func(t *testing.T) {
writeUnformattedFiles()

anotherStacks := filepath.Join(s.RootDir(), "another-stacks")
cli := newCLI(t, anotherStacks)
assertRunResult(t, cli.run("fmt"), runExpected{
Stdout: filesListOutput([]string{
"globals.tm.hcl",
"stack-1/globals.tm.hcl",
"stack-2/globals.tm.hcl",
}),
})

assertFileContents(t, "another-stacks/globals.tm.hcl", formattedHCL)
assertFileContents(t, "another-stacks/stack-1/globals.tm.hcl", formattedHCL)
assertFileContents(t, "another-stacks/stack-2/globals.tm.hcl", formattedHCL)

assertFileContents(t, "globals.tm", unformattedHCL)
assertFileContents(t, "stacks/globals.tm", unformattedHCL)
assertFileContents(t, "stacks/stack-1/globals.tm", unformattedHCL)
assertFileContents(t, "stacks/stack-2/globals.tm", unformattedHCL)

stacks := filepath.Join(s.RootDir(), "stacks")
cli = newCLI(t, stacks)
assertRunResult(t, cli.run("fmt"), runExpected{
Stdout: filesListOutput([]string{
"globals.tm",
"stack-1/globals.tm",
"stack-2/globals.tm",
}),
})

assertFileContents(t, "another-stacks/globals.tm.hcl", formattedHCL)
assertFileContents(t, "another-stacks/stack-1/globals.tm.hcl", formattedHCL)
assertFileContents(t, "another-stacks/stack-2/globals.tm.hcl", formattedHCL)
assertFileContents(t, "stacks/globals.tm", formattedHCL)
assertFileContents(t, "stacks/stack-1/globals.tm", formattedHCL)
assertFileContents(t, "stacks/stack-2/globals.tm", formattedHCL)

assertFileContents(t, "globals.tm", unformattedHCL)

cli = newCLI(t, s.RootDir())
assertRunResult(t, cli.run("fmt"), runExpected{
Stdout: filesListOutput([]string{"globals.tm"}),
})

assertWantedFilesContents(t, formattedHCL)
})
}
15 changes: 9 additions & 6 deletions generate/genhcl/genhcl.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ type StackHCLs struct {
// about the origin of the generated code.
type HCL struct {
origin string
body []byte
body string
}

const (
Expand Down Expand Up @@ -139,16 +139,19 @@ func Load(rootdir string, sm stack.Metadata, globals stack.Globals) (StackHCLs,

gen := hclwrite.NewEmptyFile()
if err := hcl.CopyBody(gen.Body(), loadedHCL.block.Body, evalctx); err != nil {
return StackHCLs{}, errors.E(
ErrEval,
sm,
err,
return StackHCLs{}, errors.E(ErrEval, sm, err,
"failed to generate block %q", name,
)
}
formatted, err := hcl.Format(string(gen.Bytes()), loadedHCL.origin)
if err != nil {
return StackHCLs{}, errors.E(sm, err,
"failed to format generated code for block %q", name,
)
}
res.hcls[name] = HCL{
origin: loadedHCL.origin,
body: hclwrite.Format(gen.Bytes()),
body: formatted,
}
}

Expand Down
26 changes: 6 additions & 20 deletions hcl/eval/partial_fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ import (
"github.com/madlambda/spells/assert"
"github.com/rs/zerolog"
"github.com/zclconf/go-cty/cty"

tmhclwrite "github.com/mineiros-io/terramate/test/hclwrite"
)

func FuzzPartialEval(f *testing.F) {
Expand Down Expand Up @@ -92,13 +90,9 @@ func FuzzPartialEval(f *testing.F) {

const testattr = "attr"

cfg := hcldoc(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now hclwriter always produces valid HCL, so if you use it to build invalid HCL files it panics, seemed like the way to go since it is what we want on 99% of the tests, we use it to build some valid HCL file for testing. But here we generate possibly invalid HCLs and that would cause panics, so it seemed easier to just create the HCL with a string since it is a pretty straithforward attr = val.

expr(testattr, str),
)

cfgString := cfg.String()
cfg := fmt.Sprintf("%s = %s", testattr, str)
parser := hclparse.NewParser()
file, diags := parser.ParseHCL([]byte(cfgString), "fuzz")
file, diags := parser.ParseHCL([]byte(cfg), "fuzz")
if diags.HasErrors() {
return
}
Expand All @@ -108,7 +102,7 @@ func FuzzPartialEval(f *testing.F) {
parsedExpr := attr.Expr

exprRange := parsedExpr.Range()
exprBytes := cfgString[exprRange.Start.Byte:exprRange.End.Byte]
exprBytes := cfg[exprRange.Start.Byte:exprRange.End.Byte]

parsedTokens, diags := hclsyntax.LexExpression([]byte(exprBytes), "fuzz", hcl.Pos{})
if diags.HasErrors() {
Expand All @@ -123,9 +117,9 @@ func FuzzPartialEval(f *testing.F) {
engine := newPartialEvalEngine(want, ctx)
got, err := engine.Eval()

if strings.Contains(cfgString, "global") ||
strings.Contains(cfgString, "terramate") ||
strings.Contains(cfgString, "tm_") {
if strings.Contains(cfg, "global") ||
strings.Contains(cfg, "terramate") ||
strings.Contains(cfg, "tm_") {
// TODO(katcipis): Validate generated code properties when
// substitution is in play.
return
Expand Down Expand Up @@ -156,14 +150,6 @@ func tokensStr(t hclwrite.Tokens) string {
return "[" + strings.Join(tokensStrs, ",") + "]"
}

func hcldoc(builders ...tmhclwrite.BlockBuilder) *tmhclwrite.Block {
return tmhclwrite.BuildHCL(builders...)
}

func expr(name string, expr string) tmhclwrite.BlockBuilder {
return tmhclwrite.Expression(name, expr)
}

func init() {
zerolog.SetGlobalLevel(zerolog.Disabled)
}