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.

Sign up for GitHub

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

chore: optimize the format by using files from config tree. #1962

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

chore: optimize the format by using files from config tree. #1962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions cmd/terramate/cli/cli.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@ import (
"github.com/terramate-io/terramate/hcl/ast"
"github.com/terramate-io/terramate/hcl/eval"
"github.com/terramate-io/terramate/hcl/fmt"
"github.com/terramate-io/terramate/hcl/fmt/fs"
"github.com/terramate-io/terramate/hcl/info"
"github.com/terramate-io/terramate/modvendor/download"
"github.com/terramate-io/terramate/printer"

"github.com/terramate-io/terramate/safeguard"
"github.com/terramate-io/terramate/tg"
"github.com/terramate-io/terramate/versions"
Expand Down Expand Up @@ -1695,11 +1697,11 @@ func (c *cli) format() {
fatalWithDetailf(errors.E("--check conflicts with --detailed-exit-code"), "Invalid args")
}

var results []fmt.FormatResult
var results []fs.FormatResult
switch len(c.parsedArgs.Fmt.Files) {
case 0:
var err error
results, err = fmt.FormatTree(c.wd())
results, err = fs.FormatTree(c.cfg(), c.wd2())
if err != nil {
fatalWithDetailf(err, "formatting directory %s", c.wd())
}
Expand Down Expand Up @@ -1730,7 +1732,7 @@ func (c *cli) format() {
fallthrough
default:
var err error
results, err = fmt.FormatFiles(c.wd(), c.parsedArgs.Fmt.Files)
results, err = fs.FormatFiles(c.wd(), c.parsedArgs.Fmt.Files)
if err != nil {
fatalWithDetailf(err, "formatting files")
}
Expand Down Expand Up @@ -2404,6 +2406,7 @@ func (c *cli) gitSafeguardRemoteEnabled() bool {
}

func (c *cli) wd() string { return c.prj.wd }
func (c *cli) wd2() prj.Path { return prj.PrjAbsPath(c.rootdir(), c.wd()) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not more closely to what the function does?

Suggested change
func (c *cli) wd2() prj.Path { return prj.PrjAbsPath(c.rootdir(), c.wd()) }
func (c *cli) absPathWD() prj.Path { return prj.PrjAbsPath(c.rootdir(), c.wd()) }

func (c *cli) rootdir() string { return c.prj.rootdir }
func (c *cli) cfg() *config.Root { return c.prj.root }
func (c *cli) baseRef() string { return c.prj.baseRef }
Expand Down
2 changes: 2 additions & 0 deletions config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ type Tree struct {
TerramateFiles []string
OtherFiles []string
TmGenFiles []string
ChildrenDirs []string

// Children is a map of configuration dir names to tree nodes.
Children map[string]*Tree
Expand Down Expand Up @@ -492,6 +493,7 @@ func loadTree(parentTree *Tree, cfgdir string, rootcfg *hcl.Config) (_ *Tree, er
tree.TerramateFiles = filesResult.TmFiles
tree.OtherFiles = filesResult.OtherFiles
tree.TmGenFiles = filesResult.TmGenFiles
tree.ChildrenDirs = filesResult.Dirs
tree.Parent = parentTree
parentTree.Children[filepath.Base(cfgdir)] = tree

Expand Down
3 changes: 2 additions & 1 deletion e2etests/core/fmt_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/madlambda/spells/assert"
. "github.com/terramate-io/terramate/e2etests/internal/runner"
"github.com/terramate-io/terramate/hcl/fmt"
"github.com/terramate-io/terramate/hcl/fmt/fs"
. "github.com/terramate-io/terramate/test/hclwrite/hclutils"
"github.com/terramate-io/terramate/test/sandbox"
)
Expand Down Expand Up @@ -258,7 +259,7 @@ func TestFmtFiles(t *testing.T) {
files: []string{"non-existent.tm"},
want: want{
res: RunExpected{
StderrRegex: string(fmt.ErrReadFile),
StderrRegex: string(fs.ErrReadFile),
Status: 1,
},
},
Expand Down
139 changes: 0 additions & 139 deletions hcl/fmt/fmt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,16 @@ package fmt

import (
"fmt"
"os"
"path/filepath"
"sort"

"github.com/rs/zerolog/log"
"github.com/terramate-io/hcl/v2"
"github.com/terramate-io/hcl/v2/hclsyntax"
"github.com/terramate-io/hcl/v2/hclwrite"
"github.com/terramate-io/terramate/errors"
"github.com/terramate-io/terramate/fs"
)

// ErrHCLSyntax is the error kind for syntax errors.
const ErrHCLSyntax errors.Kind = "HCL syntax error"

// ErrReadFile is the error kind for any error related to reading the file content.
const ErrReadFile errors.Kind = "failed to read file"

// FormatResult represents the result of a formatting operation.
type FormatResult struct {
path string
formatted string
}

// FormatMultiline will format the given source code.
// It enforces lists to be formatted as multiline, where each
// element on the list resides on its own line followed by a comma.
Expand All @@ -54,131 +40,6 @@ func Format(src, filename string) (string, error) {
return string(hclwrite.Format(parsed.Bytes())), nil
}

// FormatTree will format all Terramate configuration files
// in the given tree starting at the given dir. It will recursively
// navigate on sub directories. Directories starting with "." are ignored.
//
// Only Terramate configuration files will be formatted.
//
// Files that are already formatted are ignored. If all files are formatted
// this function returns an empty result.
//
// All files will be left untouched. To save the formatted result on disk you
// can use FormatResult.Save for each FormatResult.
func FormatTree(dir string) ([]FormatResult, error) {
logger := log.With().
Str("action", "FormatTree").
Str("dir", dir).
Logger()

// TODO(i4k): use files from the config tree.
res, err := fs.ListTerramateFiles(dir)
if err != nil {
return nil, errors.E(errFormatTree, err)
}
for _, fname := range res.OtherFiles {
if fname == ".tmskip" {
logger.Debug().Msg("skip file found: skipping whole subtree")
return nil, nil
}
}

files := append([]string{}, res.TmFiles...)
files = append(files, res.TmGenFiles...)

sort.Strings(files)

errs := errors.L()
results, err := FormatFiles(dir, files)

errs.Append(err)

for _, d := range res.Dirs {
subres, err := FormatTree(filepath.Join(dir, d))
if err != nil {
errs.Append(err)
continue
}
results = append(results, subres...)
}

if err := errs.AsError(); err != nil {
return nil, err
}
sort.Slice(results, func(i, j int) bool {
return results[i].path < results[j].path
})
return results, nil
}

// FormatFiles will format all the provided Terramate paths.
// Only Terramate configuration files can be reliably formatted with this function.
// If HCL files for a different tool is provided, the result is unpredictable.
//
// Note: The provided file paths can be absolute or relative. If relative, ensure
// working directory is corrected adjusted. The special `-` filename is treated as a
// normal filename, then if it needs to be interpreted as `stdin` this needs to be
// handled separately by the caller.
//
// Files that are already formatted are ignored. If all files are formatted
// this function returns an empty result.
//
// All files will be left untouched. To save the formatted result on disk you
// can use FormatResult.Save for each FormatResult.
func FormatFiles(basedir string, files []string) ([]FormatResult, error) {
results := []FormatResult{}
errs := errors.L()

for _, file := range files {
fname := file
if !filepath.IsAbs(file) {
fname = filepath.Join(basedir, file)
}
fileContents, err := os.ReadFile(fname)
if err != nil {
errs.Append(errors.E(ErrReadFile, err))
continue
}
currentCode := string(fileContents)
formatted, err := Format(currentCode, fname)
if err != nil {
errs.Append(err)
continue
}
if currentCode == formatted {
continue
}
results = append(results, FormatResult{
path: fname,
formatted: formatted,
})
}
if err := errs.AsError(); err != nil {
return nil, err
}
return results, nil
}

// Save will save the formatted result on the original file, replacing
// its original contents.
func (f FormatResult) Save() error {
return os.WriteFile(f.path, []byte(f.formatted), 0644)
}

// Path is the absolute path of the original file.
func (f FormatResult) Path() string {
return f.path
}

// Formatted is the contents of the original file after formatting.
func (f FormatResult) Formatted() string {
return f.formatted
}

const (
errFormatTree errors.Kind = "formatting tree"
)

func fmtBody(body *hclwrite.Body) {
attrs := body.Attributes()
for name, attr := range attrs {
Expand Down
Loading
Loading