From 3cbe494426f3a0602d5616c8e4e4663987b31d37 Mon Sep 17 00:00:00 2001 From: Brian McGee Date: Fri, 12 Jan 2024 20:12:30 +0000 Subject: [PATCH] feat: create config package Move all config related code into a config package. Signed-off-by: Brian McGee --- internal/cli/format.go | 26 +++--- internal/cli/format_test.go | 90 +++++++++++---------- internal/{format => config}/config.go | 8 +- internal/{format => config}/config_test.go | 4 +- internal/config/formatter.go | 14 ++++ internal/format/{format.go => formatter.go} | 28 +++---- internal/test/temp.go | 5 +- 7 files changed, 92 insertions(+), 83 deletions(-) rename internal/{format => config}/config.go (60%) rename internal/{format => config}/config_test.go (97%) create mode 100644 internal/config/formatter.go rename internal/format/{format.go => formatter.go} (85%) diff --git a/internal/cli/format.go b/internal/cli/format.go index a6df44b..253a413 100644 --- a/internal/cli/format.go +++ b/internal/cli/format.go @@ -10,6 +10,8 @@ import ( "syscall" "time" + "git.numtide.com/numtide/treefmt/internal/config" + "git.numtide.com/numtide/treefmt/internal/cache" "git.numtide.com/numtide/treefmt/internal/format" @@ -39,7 +41,7 @@ func (f *Format) Run() error { defer cancel() // read config - cfg, err := format.ReadConfigFile(Cli.ConfigFile) + cfg, err := config.ReadFile(Cli.ConfigFile) if err != nil { return fmt.Errorf("%w: failed to read config file", err) } @@ -68,8 +70,8 @@ func (f *Format) Run() error { formatters := make(map[string]*format.Formatter) // detect broken dependencies - for name, config := range cfg.Formatters { - before := config.Before + for name, formatterCfg := range cfg.Formatters { + before := formatterCfg.Before if before != "" { // check child formatter exists _, ok := cfg.Formatters[before] @@ -80,7 +82,7 @@ func (f *Format) Run() error { } // dependency cycle detection - for name, config := range cfg.Formatters { + for name, formatterCfg := range cfg.Formatters { var ok bool var history []string childName := name @@ -88,23 +90,23 @@ func (f *Format) Run() error { // add to history history = append(history, childName) - if config.Before == "" { + if formatterCfg.Before == "" { break - } else if config.Before == name { + } else if formatterCfg.Before == name { return fmt.Errorf("formatter cycle detected %v", strings.Join(history, " -> ")) } // load child config - childName = config.Before - config, ok = cfg.Formatters[config.Before] + childName = formatterCfg.Before + formatterCfg, ok = cfg.Formatters[formatterCfg.Before] if !ok { - return fmt.Errorf("formatter not found: %v", config.Before) + return fmt.Errorf("formatter not found: %v", formatterCfg.Before) } } } // init formatters - for name, config := range cfg.Formatters { + for name, formatterCfg := range cfg.Formatters { if !includeFormatter(name) { // remove this formatter delete(cfg.Formatters, name) @@ -112,8 +114,8 @@ func (f *Format) Run() error { continue } - formatter, err := format.NewFormatter(name, config, globalExcludes) - if errors.Is(err, format.ErrFormatterNotFound) && Cli.AllowMissingFormatter { + formatter, err := format.NewFormatter(name, formatterCfg, globalExcludes) + if errors.Is(err, format.ErrCommandNotFound) && Cli.AllowMissingFormatter { l.Debugf("formatter not found: %v", name) continue } else if err != nil { diff --git a/internal/cli/format_test.go b/internal/cli/format_test.go index e3c285e..de8bc55 100644 --- a/internal/cli/format_test.go +++ b/internal/cli/format_test.go @@ -8,6 +8,8 @@ import ( "path/filepath" "testing" + "git.numtide.com/numtide/treefmt/internal/config" + "git.numtide.com/numtide/treefmt/internal/test" "github.com/go-git/go-billy/v5/osfs" "github.com/go-git/go-git/v5" @@ -24,8 +26,8 @@ func TestAllowMissingFormatter(t *testing.T) { tempDir := t.TempDir() configPath := tempDir + "/treefmt.toml" - test.WriteConfig(t, configPath, format.Config{ - Formatters: map[string]*format.FormatterConfig{ + test.WriteConfig(t, configPath, config.Config{ + Formatters: map[string]*config.Formatter{ "foo-fmt": { Command: "foo-fmt", }, @@ -33,7 +35,7 @@ func TestAllowMissingFormatter(t *testing.T) { }) _, err := cmd(t, "--config-file", configPath, "--tree-root", tempDir) - as.ErrorIs(err, format.ErrFormatterNotFound) + as.ErrorIs(err, format.ErrCommandNotFound) _, err = cmd(t, "--config-file", configPath, "--tree-root", tempDir, "--allow-missing-formatter") as.NoError(err) @@ -45,8 +47,8 @@ func TestDependencyCycle(t *testing.T) { tempDir := t.TempDir() configPath := tempDir + "/treefmt.toml" - test.WriteConfig(t, configPath, format.Config{ - Formatters: map[string]*format.FormatterConfig{ + test.WriteConfig(t, configPath, config.Config{ + Formatters: map[string]*config.Formatter{ "a": {Command: "echo", Before: "b"}, "b": {Command: "echo", Before: "c"}, "c": {Command: "echo", Before: "a"}, @@ -66,8 +68,8 @@ func TestSpecifyingFormatters(t *testing.T) { tempDir := test.TempExamples(t) configPath := tempDir + "/treefmt.toml" - test.WriteConfig(t, configPath, format.Config{ - Formatters: map[string]*format.FormatterConfig{ + test.WriteConfig(t, configPath, config.Config{ + Formatters: map[string]*config.Formatter{ "elm": { Command: "echo", Includes: []string{"*.elm"}, @@ -115,8 +117,8 @@ func TestIncludesAndExcludes(t *testing.T) { configPath := tempDir + "/echo.toml" // test without any excludes - config := format.Config{ - Formatters: map[string]*format.FormatterConfig{ + cfg := config.Config{ + Formatters: map[string]*config.Formatter{ "echo": { Command: "echo", Includes: []string{"*"}, @@ -124,33 +126,33 @@ func TestIncludesAndExcludes(t *testing.T) { }, } - test.WriteConfig(t, configPath, config) + test.WriteConfig(t, configPath, cfg) out, err := cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) as.Contains(string(out), fmt.Sprintf("%d files changed", 29)) // globally exclude nix files - config.Global.Excludes = []string{"*.nix"} + cfg.Global.Excludes = []string{"*.nix"} - test.WriteConfig(t, configPath, config) + test.WriteConfig(t, configPath, cfg) out, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) as.Contains(string(out), fmt.Sprintf("%d files changed", 28)) // add haskell files to the global exclude - config.Global.Excludes = []string{"*.nix", "*.hs"} + cfg.Global.Excludes = []string{"*.nix", "*.hs"} - test.WriteConfig(t, configPath, config) + test.WriteConfig(t, configPath, cfg) out, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) as.Contains(string(out), fmt.Sprintf("%d files changed", 22)) - echo := config.Formatters["echo"] + echo := cfg.Formatters["echo"] // remove python files from the echo formatter echo.Excludes = []string{"*.py"} - test.WriteConfig(t, configPath, config) + test.WriteConfig(t, configPath, cfg) out, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) as.Contains(string(out), fmt.Sprintf("%d files changed", 20)) @@ -158,7 +160,7 @@ func TestIncludesAndExcludes(t *testing.T) { // remove go files from the echo formatter echo.Excludes = []string{"*.py", "*.go"} - test.WriteConfig(t, configPath, config) + test.WriteConfig(t, configPath, cfg) out, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) as.Contains(string(out), fmt.Sprintf("%d files changed", 19)) @@ -166,7 +168,7 @@ func TestIncludesAndExcludes(t *testing.T) { // adjust the includes for echo to only include elm files echo.Includes = []string{"*.elm"} - test.WriteConfig(t, configPath, config) + test.WriteConfig(t, configPath, cfg) out, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) as.Contains(string(out), fmt.Sprintf("%d files changed", 1)) @@ -174,7 +176,7 @@ func TestIncludesAndExcludes(t *testing.T) { // add js files to echo formatter echo.Includes = []string{"*.elm", "*.js"} - test.WriteConfig(t, configPath, config) + test.WriteConfig(t, configPath, cfg) out, err = cmd(t, "-c", "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) as.Contains(string(out), fmt.Sprintf("%d files changed", 2)) @@ -187,8 +189,8 @@ func TestCache(t *testing.T) { configPath := tempDir + "/echo.toml" // test without any excludes - config := format.Config{ - Formatters: map[string]*format.FormatterConfig{ + cfg := config.Config{ + Formatters: map[string]*config.Formatter{ "echo": { Command: "echo", Includes: []string{"*"}, @@ -196,7 +198,7 @@ func TestCache(t *testing.T) { }, } - test.WriteConfig(t, configPath, config) + test.WriteConfig(t, configPath, cfg) out, err := cmd(t, "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) as.Contains(string(out), fmt.Sprintf("%d files changed", 29)) @@ -222,8 +224,8 @@ func TestChangeWorkingDirectory(t *testing.T) { configPath := tempDir + "/treefmt.toml" // test without any excludes - config := format.Config{ - Formatters: map[string]*format.FormatterConfig{ + cfg := config.Config{ + Formatters: map[string]*config.Formatter{ "echo": { Command: "echo", Includes: []string{"*"}, @@ -231,7 +233,7 @@ func TestChangeWorkingDirectory(t *testing.T) { }, } - test.WriteConfig(t, configPath, config) + test.WriteConfig(t, configPath, cfg) // by default, we look for ./treefmt.toml and use the cwd for the tree root // this should fail if the working directory hasn't been changed first @@ -247,8 +249,8 @@ func TestFailOnChange(t *testing.T) { configPath := tempDir + "/echo.toml" // test without any excludes - config := format.Config{ - Formatters: map[string]*format.FormatterConfig{ + cfg := config.Config{ + Formatters: map[string]*config.Formatter{ "echo": { Command: "echo", Includes: []string{"*"}, @@ -256,7 +258,7 @@ func TestFailOnChange(t *testing.T) { }, } - test.WriteConfig(t, configPath, config) + test.WriteConfig(t, configPath, cfg) _, err := cmd(t, "--fail-on-change", "--config-file", configPath, "--tree-root", tempDir) as.ErrorIs(err, ErrFailOnChange) } @@ -283,8 +285,8 @@ func TestBustCacheOnFormatterChange(t *testing.T) { as.NoError(os.Setenv("PATH", binPath+":"+os.Getenv("PATH"))) // start with 2 formatters - config := format.Config{ - Formatters: map[string]*format.FormatterConfig{ + cfg := config.Config{ + Formatters: map[string]*config.Formatter{ "python": { Command: "black", Includes: []string{"*.py"}, @@ -297,7 +299,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) { }, } - test.WriteConfig(t, configPath, config) + test.WriteConfig(t, configPath, cfg) args := []string{"--config-file", configPath, "--tree-root", tempDir} out, err := cmd(t, args...) as.NoError(err) @@ -328,12 +330,12 @@ func TestBustCacheOnFormatterChange(t *testing.T) { as.Contains(string(out), "0 files changed") // add go formatter - config.Formatters["go"] = &format.FormatterConfig{ + cfg.Formatters["go"] = &config.Formatter{ Command: "gofmt", Options: []string{"-w"}, Includes: []string{"*.go"}, } - test.WriteConfig(t, configPath, config) + test.WriteConfig(t, configPath, cfg) out, err = cmd(t, args...) as.NoError(err) @@ -345,8 +347,8 @@ func TestBustCacheOnFormatterChange(t *testing.T) { as.Contains(string(out), "0 files changed") // remove python formatter - delete(config.Formatters, "python") - test.WriteConfig(t, configPath, config) + delete(cfg.Formatters, "python") + test.WriteConfig(t, configPath, cfg) out, err = cmd(t, args...) as.NoError(err) @@ -358,8 +360,8 @@ func TestBustCacheOnFormatterChange(t *testing.T) { as.Contains(string(out), "0 files changed") // remove elm formatter - delete(config.Formatters, "elm") - test.WriteConfig(t, configPath, config) + delete(cfg.Formatters, "elm") + test.WriteConfig(t, configPath, cfg) out, err = cmd(t, args...) as.NoError(err) @@ -378,15 +380,15 @@ func TestGitWorktree(t *testing.T) { configPath := filepath.Join(tempDir, "/treefmt.toml") // basic config - config := format.Config{ - Formatters: map[string]*format.FormatterConfig{ + cfg := config.Config{ + Formatters: map[string]*config.Formatter{ "echo": { Command: "echo", Includes: []string{"*"}, }, }, } - test.WriteConfig(t, configPath, config) + test.WriteConfig(t, configPath, cfg) // init a git repo repo, err := git.Init( @@ -433,8 +435,8 @@ func TestOrderingFormatters(t *testing.T) { configPath := path.Join(tempDir, "treefmt.toml") // missing child - test.WriteConfig(t, configPath, format.Config{ - Formatters: map[string]*format.FormatterConfig{ + test.WriteConfig(t, configPath, config.Config{ + Formatters: map[string]*config.Formatter{ "hs-a": { Command: "echo", Includes: []string{"*.hs"}, @@ -447,8 +449,8 @@ func TestOrderingFormatters(t *testing.T) { as.ErrorContains(err, "formatter hs-a is before hs-b but config for hs-b was not found") // multiple roots - test.WriteConfig(t, configPath, format.Config{ - Formatters: map[string]*format.FormatterConfig{ + test.WriteConfig(t, configPath, config.Config{ + Formatters: map[string]*config.Formatter{ "hs-a": { Command: "echo", Includes: []string{"*.hs"}, diff --git a/internal/format/config.go b/internal/config/config.go similarity index 60% rename from internal/format/config.go rename to internal/config/config.go index e404d52..e116b0f 100644 --- a/internal/format/config.go +++ b/internal/config/config.go @@ -1,4 +1,4 @@ -package format +package config import "github.com/BurntSushi/toml" @@ -8,11 +8,11 @@ type Config struct { // Excludes is an optional list of glob patterns used to exclude certain files from all formatters. Excludes []string } - Formatters map[string]*FormatterConfig `toml:"formatter"` + Formatters map[string]*Formatter `toml:"formatter"` } -// ReadConfigFile reads from path and unmarshals toml into a Config instance. -func ReadConfigFile(path string) (cfg *Config, err error) { +// ReadFile reads from path and unmarshals toml into a Config instance. +func ReadFile(path string) (cfg *Config, err error) { _, err = toml.DecodeFile(path, &cfg) return } diff --git a/internal/format/config_test.go b/internal/config/config_test.go similarity index 97% rename from internal/format/config_test.go rename to internal/config/config_test.go index 1650aaf..611fc04 100644 --- a/internal/format/config_test.go +++ b/internal/config/config_test.go @@ -1,4 +1,4 @@ -package format +package config import ( "testing" @@ -9,7 +9,7 @@ import ( func TestReadConfigFile(t *testing.T) { as := require.New(t) - cfg, err := ReadConfigFile("../../test/treefmt.toml") + cfg, err := ReadFile("../../test/treefmt.toml") as.NoError(err, "failed to read config file") as.NotNil(cfg) diff --git a/internal/config/formatter.go b/internal/config/formatter.go new file mode 100644 index 0000000..34f4983 --- /dev/null +++ b/internal/config/formatter.go @@ -0,0 +1,14 @@ +package config + +type Formatter struct { + // Command is the command invoke when applying this Formatter. + Command string + // Options are an optional list of args to be passed to Command. + Options []string + // Includes is a list of glob patterns used to determine whether this Formatter should be applied against a path. + Includes []string + // Excludes is an optional list of glob patterns used to exclude certain files from this Formatter. + Excludes []string + // Before is the name of another formatter which must process a path after this one + Before string +} diff --git a/internal/format/format.go b/internal/format/formatter.go similarity index 85% rename from internal/format/format.go rename to internal/format/formatter.go index 460e506..a2654dd 100644 --- a/internal/format/format.go +++ b/internal/format/formatter.go @@ -7,37 +7,27 @@ import ( "os/exec" "time" + "git.numtide.com/numtide/treefmt/internal/config" + "github.com/charmbracelet/log" "github.com/gobwas/glob" ) -// ErrFormatterNotFound is returned when the Command for a Formatter is not available. -var ErrFormatterNotFound = errors.New("formatter not found") - -type FormatterConfig struct { - // Command is the command invoke when applying this Formatter. - Command string - // Options are an optional list of args to be passed to Command. - Options []string - // Includes is a list of glob patterns used to determine whether this Formatter should be applied against a path. - Includes []string - // Excludes is an optional list of glob patterns used to exclude certain files from this Formatter. - Excludes []string - // Before is the name of another formatter which must process a path after this one - Before string -} +// ErrCommandNotFound is returned when the Command for a Formatter is not available. +var ErrCommandNotFound = errors.New("formatter command not found in PATH") // Formatter represents a command which should be applied to a filesystem. type Formatter struct { name string - config *FormatterConfig + config *config.Formatter log *log.Logger executable string // path to the executable described by Command before string - parent *Formatter + child *Formatter + parent *Formatter // internal compiled versions of Includes and Excludes. includes []glob.Glob @@ -68,7 +58,7 @@ func (f *Formatter) Executable() string { } // NewFormatter is used to create a new Formatter. -func NewFormatter(name string, config *FormatterConfig, globalExcludes []glob.Glob) (*Formatter, error) { +func NewFormatter(name string, config *config.Formatter, globalExcludes []glob.Glob) (*Formatter, error) { var err error f := Formatter{} @@ -80,7 +70,7 @@ func NewFormatter(name string, config *FormatterConfig, globalExcludes []glob.Gl // test if the formatter is available executable, err := exec.LookPath(config.Command) if errors.Is(err, exec.ErrNotFound) { - return nil, ErrFormatterNotFound + return nil, ErrCommandNotFound } else if err != nil { return nil, err } diff --git a/internal/test/temp.go b/internal/test/temp.go index 3e338ad..2324d1c 100644 --- a/internal/test/temp.go +++ b/internal/test/temp.go @@ -4,13 +4,14 @@ import ( "os" "testing" - "git.numtide.com/numtide/treefmt/internal/format" + "git.numtide.com/numtide/treefmt/internal/config" + "github.com/BurntSushi/toml" cp "github.com/otiai10/copy" "github.com/stretchr/testify/require" ) -func WriteConfig(t *testing.T, path string, cfg format.Config) { +func WriteConfig(t *testing.T, path string, cfg config.Config) { t.Helper() f, err := os.Create(path) if err != nil {