From c54265517754f4d67684e5810c87cbba0644ae0d Mon Sep 17 00:00:00 2001 From: Brian McGee Date: Wed, 10 Jan 2024 14:28:20 +0000 Subject: [PATCH] feat: separate formatter config from formatter Signed-off-by: Brian McGee --- internal/cli/format.go | 44 ++++++++++++++++++----------------- internal/cli/format_test.go | 20 ++++++++-------- internal/format/config.go | 2 +- internal/format/format.go | 46 +++++++++++++++++++++++++------------ 4 files changed, 65 insertions(+), 47 deletions(-) diff --git a/internal/cli/format.go b/internal/cli/format.go index 4e2e5f1..e3be08a 100644 --- a/internal/cli/format.go +++ b/internal/cli/format.go @@ -64,22 +64,23 @@ func (f *Format) Run() error { } } + formatters := make(map[string]*format.Formatter) + // detect dependency cycles and broken dependencies - for name, formatter := range cfg.Formatters { - if formatter.Before != "" { + // todo dependency cycle detection + for name, config := range cfg.Formatters { + before := config.Before + if before != "" { // check child formatter exists - _, ok := cfg.Formatters[formatter.Before] + _, ok := cfg.Formatters[before] if !ok { - return fmt.Errorf( - "formatter %v is before %v but config for %v was not found", - name, formatter.Before, formatter.Before, - ) + return fmt.Errorf("formatter %v is before %v but config for %v was not found", name, before, before) } } } // init formatters - for name, formatter := range cfg.Formatters { + for name, config := range cfg.Formatters { if !includeFormatter(name) { // remove this formatter delete(cfg.Formatters, name) @@ -87,23 +88,24 @@ func (f *Format) Run() error { continue } - err = formatter.Init(name, globalExcludes) + formatter, err := format.NewFormatter(name, config, globalExcludes) if errors.Is(err, format.ErrFormatterNotFound) && Cli.AllowMissingFormatter { l.Debugf("formatter not found: %v", name) - // remove this formatter - delete(cfg.Formatters, name) + continue } else if err != nil { return fmt.Errorf("%w: failed to initialise formatter: %v", err, name) } + + formatters[name] = formatter } // iterate the initialised formatters configuring parent/child relationships - for _, formatter := range cfg.Formatters { - if formatter.Before != "" { - child, ok := cfg.Formatters[formatter.Before] + for _, formatter := range formatters { + if formatter.Before() != "" { + child, ok := formatters[formatter.Before()] if !ok { // formatter has been filtered out by the user - formatter.Before = "" + formatter.ResetBefore() continue } formatter.SetChild(child) @@ -111,7 +113,7 @@ func (f *Format) Run() error { } } - if err = cache.Open(Cli.TreeRoot, Cli.ClearCache, cfg.Formatters); err != nil { + if err = cache.Open(Cli.TreeRoot, Cli.ClearCache, formatters); err != nil { return err } @@ -124,8 +126,8 @@ func (f *Format) Run() error { eg, ctx := errgroup.WithContext(ctx) // start the formatters - for name := range cfg.Formatters { - formatter := cfg.Formatters[name] + for name := range formatters { + formatter := formatters[name] eg.Go(func() error { return formatter.Run(ctx) }) @@ -180,7 +182,7 @@ func (f *Format) Run() error { eg.Go(func() error { // pass paths to each formatter for path := range pathsCh { - for _, formatter := range cfg.Formatters { + for _, formatter := range formatters { if formatter.Wants(path) { formatter.Put(path) } @@ -188,7 +190,7 @@ func (f *Format) Run() error { } // indicate no more paths for each formatter - for _, formatter := range cfg.Formatters { + for _, formatter := range formatters { if formatter.Parent() != nil { // this formatter is not a root, it will be closed by a parent continue @@ -197,7 +199,7 @@ func (f *Format) Run() error { } // await completion - for _, formatter := range cfg.Formatters { + for _, formatter := range formatters { formatter.AwaitCompletion() } diff --git a/internal/cli/format_test.go b/internal/cli/format_test.go index 8081e87..4f4378b 100644 --- a/internal/cli/format_test.go +++ b/internal/cli/format_test.go @@ -20,7 +20,7 @@ func TestAllowMissingFormatter(t *testing.T) { configPath := tempDir + "/treefmt.toml" test.WriteConfig(t, configPath, format.Config{ - Formatters: map[string]*format.Formatter{ + Formatters: map[string]*format.FormatterConfig{ "foo-fmt": { Command: "foo-fmt", }, @@ -41,7 +41,7 @@ func TestSpecifyingFormatters(t *testing.T) { configPath := tempDir + "/treefmt.toml" test.WriteConfig(t, configPath, format.Config{ - Formatters: map[string]*format.Formatter{ + Formatters: map[string]*format.FormatterConfig{ "elm": { Command: "echo", Includes: []string{"*.elm"}, @@ -90,7 +90,7 @@ func TestIncludesAndExcludes(t *testing.T) { // test without any excludes config := format.Config{ - Formatters: map[string]*format.Formatter{ + Formatters: map[string]*format.FormatterConfig{ "echo": { Command: "echo", Includes: []string{"*"}, @@ -162,7 +162,7 @@ func TestCache(t *testing.T) { // test without any excludes config := format.Config{ - Formatters: map[string]*format.Formatter{ + Formatters: map[string]*format.FormatterConfig{ "echo": { Command: "echo", Includes: []string{"*"}, @@ -197,7 +197,7 @@ func TestChangeWorkingDirectory(t *testing.T) { // test without any excludes config := format.Config{ - Formatters: map[string]*format.Formatter{ + Formatters: map[string]*format.FormatterConfig{ "echo": { Command: "echo", Includes: []string{"*"}, @@ -222,7 +222,7 @@ func TestFailOnChange(t *testing.T) { // test without any excludes config := format.Config{ - Formatters: map[string]*format.Formatter{ + Formatters: map[string]*format.FormatterConfig{ "echo": { Command: "echo", Includes: []string{"*"}, @@ -258,7 +258,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) { // start with 2 formatters config := format.Config{ - Formatters: map[string]*format.Formatter{ + Formatters: map[string]*format.FormatterConfig{ "python": { Command: "black", Includes: []string{"*.py"}, @@ -302,7 +302,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) { as.Contains(string(out), "0 files changed") // add go formatter - config.Formatters["go"] = &format.Formatter{ + config.Formatters["go"] = &format.FormatterConfig{ Command: "gofmt", Options: []string{"-w"}, Includes: []string{"*.go"}, @@ -353,7 +353,7 @@ func TestOrderingFormatters(t *testing.T) { // missing child test.WriteConfig(t, configPath, format.Config{ - Formatters: map[string]*format.Formatter{ + Formatters: map[string]*format.FormatterConfig{ "hs-a": { Command: "echo", Includes: []string{"*.hs"}, @@ -367,7 +367,7 @@ func TestOrderingFormatters(t *testing.T) { // multiple roots test.WriteConfig(t, configPath, format.Config{ - Formatters: map[string]*format.Formatter{ + Formatters: map[string]*format.FormatterConfig{ "hs-a": { Command: "echo", Includes: []string{"*.hs"}, diff --git a/internal/format/config.go b/internal/format/config.go index 29b127d..e404d52 100644 --- a/internal/format/config.go +++ b/internal/format/config.go @@ -8,7 +8,7 @@ type Config struct { // Excludes is an optional list of glob patterns used to exclude certain files from all formatters. Excludes []string } - Formatters map[string]*Formatter `toml:"formatter"` + Formatters map[string]*FormatterConfig `toml:"formatter"` } // ReadConfigFile reads from path and unmarshals toml into a Config instance. diff --git a/internal/format/format.go b/internal/format/format.go index 79d8b60..76a9fb1 100644 --- a/internal/format/format.go +++ b/internal/format/format.go @@ -14,8 +14,7 @@ import ( // ErrFormatterNotFound is returned when the Command for a Formatter is not available. var ErrFormatterNotFound = errors.New("formatter not found") -// Formatter represents a command which should be applied to a filesystem. -type Formatter struct { +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. @@ -26,11 +25,17 @@ type Formatter struct { Excludes []string // Before is the name of another formatter which must process a path after this one Before string +} + +// Formatter represents a command which should be applied to a filesystem. +type Formatter struct { + name string + config *FormatterConfig - name string log *log.Logger executable string // path to the executable described by Command + before string parent *Formatter child *Formatter @@ -49,24 +54,35 @@ type Formatter struct { batchSize int } +func (f *Formatter) Before() string { + return f.before +} + +func (f *Formatter) ResetBefore() { + f.before = "" +} + // Executable returns the path to the executable defined by Command func (f *Formatter) Executable() string { return f.executable } -// Init is used to initialise internal state before this Formatter is ready to accept paths. -func (f *Formatter) Init(name string, globalExcludes []glob.Glob) error { +// NewFormatter is used to create a new Formatter. +func NewFormatter(name string, config *FormatterConfig, globalExcludes []glob.Glob) (*Formatter, error) { var err error + f := Formatter{} // capture the name from the config file f.name = name + f.config = config + f.before = config.Before // test if the formatter is available - executable, err := exec.LookPath(f.Command) + executable, err := exec.LookPath(config.Command) if errors.Is(err, exec.ErrNotFound) { - return ErrFormatterNotFound + return nil, ErrFormatterNotFound } else if err != nil { - return err + return nil, err } f.executable = executable @@ -77,18 +93,18 @@ func (f *Formatter) Init(name string, globalExcludes []glob.Glob) error { f.inboxCh = make(chan string, f.batchSize) f.completedCh = make(chan interface{}, 1) - f.includes, err = CompileGlobs(f.Includes) + f.includes, err = CompileGlobs(config.Includes) if err != nil { - return fmt.Errorf("%w: formatter '%v' includes", err, f.name) + return nil, fmt.Errorf("%w: formatter '%v' includes", err, f.name) } - f.excludes, err = CompileGlobs(f.Excludes) + f.excludes, err = CompileGlobs(config.Excludes) if err != nil { - return fmt.Errorf("%w: formatter '%v' excludes", err, f.name) + return nil, fmt.Errorf("%w: formatter '%v' excludes", err, f.name) } f.excludes = append(f.excludes, globalExcludes...) - return nil + return &f, nil } func (f *Formatter) SetParent(formatter *Formatter) { @@ -177,7 +193,7 @@ func (f *Formatter) apply(ctx context.Context) error { } // construct args, starting with config - args := f.Options + args := f.config.Options // append each file path for _, path := range f.batch { @@ -186,7 +202,7 @@ func (f *Formatter) apply(ctx context.Context) error { // execute start := time.Now() - cmd := exec.CommandContext(ctx, f.Command, args...) + cmd := exec.CommandContext(ctx, f.config.Command, args...) if out, err := cmd.CombinedOutput(); err != nil { f.log.Debugf("\n%v", string(out))