diff --git a/internal/cli/format.go b/internal/cli/format.go index 9483a5b..4e2e5f1 100644 --- a/internal/cli/format.go +++ b/internal/cli/format.go @@ -64,6 +64,20 @@ func (f *Format) Run() error { } } + // detect dependency cycles and broken dependencies + for name, formatter := range cfg.Formatters { + if formatter.Before != "" { + // check child formatter exists + _, ok := cfg.Formatters[formatter.Before] + if !ok { + return fmt.Errorf( + "formatter %v is before %v but config for %v was not found", + name, formatter.Before, formatter.Before, + ) + } + } + } + // init formatters for name, formatter := range cfg.Formatters { if !includeFormatter(name) { @@ -83,14 +97,25 @@ func (f *Format) Run() error { } } - ctx = format.RegisterFormatters(ctx, cfg.Formatters) + // iterate the initialised formatters configuring parent/child relationships + for _, formatter := range cfg.Formatters { + if formatter.Before != "" { + child, ok := cfg.Formatters[formatter.Before] + if !ok { + // formatter has been filtered out by the user + formatter.Before = "" + continue + } + formatter.SetChild(child) + child.SetParent(formatter) + } + } if err = cache.Open(Cli.TreeRoot, Cli.ClearCache, cfg.Formatters); err != nil { return err } // - pendingCh := make(chan string, 1024) completedCh := make(chan string, 1024) ctx = format.SetCompletedChannel(ctx, completedCh) @@ -114,20 +139,13 @@ func (f *Format) Run() error { batchSize := 1024 batch := make([]string, 0, batchSize) - var pending, completed, changes int + var changes int LOOP: for { select { case <-ctx.Done(): return ctx.Err() - case _, ok := <-pendingCh: - if ok { - pending += 1 - } else if pending == completed { - break LOOP - } - case path, ok := <-completedCh: if !ok { break LOOP @@ -141,12 +159,6 @@ func (f *Format) Run() error { changes += count batch = batch[:0] } - - completed += 1 - - if completed == pending { - close(completedCh) - } } } @@ -166,26 +178,32 @@ func (f *Format) Run() error { }) eg.Go(func() error { - count := 0 - + // pass paths to each formatter for path := range pathsCh { for _, formatter := range cfg.Formatters { if formatter.Wants(path) { - pendingCh <- path - count += 1 formatter.Put(path) } } } + // indicate no more paths for each formatter for _, formatter := range cfg.Formatters { + if formatter.Parent() != nil { + // this formatter is not a root, it will be closed by a parent + continue + } formatter.Close() } - if count == 0 { - close(completedCh) + // await completion + for _, formatter := range cfg.Formatters { + formatter.AwaitCompletion() } + // indicate no more completion events + close(completedCh) + return nil }) diff --git a/internal/cli/format_test.go b/internal/cli/format_test.go index 4773007..8081e87 100644 --- a/internal/cli/format_test.go +++ b/internal/cli/format_test.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "os/exec" + "path" "testing" "git.numtide.com/numtide/treefmt/internal/test" @@ -343,3 +344,57 @@ func TestBustCacheOnFormatterChange(t *testing.T) { as.NoError(err) as.Contains(string(out), "0 files changed") } + +func TestOrderingFormatters(t *testing.T) { + as := require.New(t) + + tempDir := test.TempExamples(t) + configPath := path.Join(tempDir, "treefmt.toml") + + // missing child + test.WriteConfig(t, configPath, format.Config{ + Formatters: map[string]*format.Formatter{ + "hs-a": { + Command: "echo", + Includes: []string{"*.hs"}, + Before: "hs-b", + }, + }, + }) + + out, err := cmd(t, "--config-file", configPath, "--tree-root", tempDir) + 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.Formatter{ + "hs-a": { + Command: "echo", + Includes: []string{"*.hs"}, + Before: "hs-b", + }, + "hs-b": { + Command: "echo", + Includes: []string{"*.hs"}, + Before: "hs-c", + }, + "hs-c": { + Command: "echo", + Includes: []string{"*.hs"}, + }, + "py-a": { + Command: "echo", + Includes: []string{"*.py"}, + Before: "py-b", + }, + "py-b": { + Command: "echo", + Includes: []string{"*.py"}, + }, + }, + }) + + out, err = cmd(t, "--config-file", configPath, "--tree-root", tempDir) + as.NoError(err) + as.Contains(string(out), "8 files changed") +} diff --git a/internal/format/context.go b/internal/format/context.go index 7de916b..d1fb703 100644 --- a/internal/format/context.go +++ b/internal/format/context.go @@ -5,28 +5,17 @@ import ( ) const ( - formattersKey = "formatters" completedChKey = "completedCh" ) -// RegisterFormatters is used to set a map of formatters in the provided context. -func RegisterFormatters(ctx context.Context, formatters map[string]*Formatter) context.Context { - return context.WithValue(ctx, formattersKey, formatters) -} - -// GetFormatters is used to retrieve a formatters map from the provided context. -func GetFormatters(ctx context.Context) map[string]*Formatter { - return ctx.Value(formattersKey).(map[string]*Formatter) -} - // SetCompletedChannel is used to set a channel for indication processing completion in the provided context. func SetCompletedChannel(ctx context.Context, completedCh chan string) context.Context { return context.WithValue(ctx, completedChKey, completedCh) } -// MarkFormatComplete is used to indicate that all processing has finished for the provided path. +// MarkPathComplete is used to indicate that all processing has finished for the provided path. // This is done by adding the path to the completion channel which should have already been set using // SetCompletedChannel. -func MarkFormatComplete(ctx context.Context, path string) { +func MarkPathComplete(ctx context.Context, path string) { ctx.Value(completedChKey).(chan string) <- path } diff --git a/internal/format/format.go b/internal/format/format.go index 26f9a66..79d8b60 100644 --- a/internal/format/format.go +++ b/internal/format/format.go @@ -24,19 +24,26 @@ type Formatter struct { 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 name string log *log.Logger executable string // path to the executable described by Command + parent *Formatter + child *Formatter + // internal compiled versions of Includes and Excludes. includes []glob.Glob excludes []glob.Glob - // inbox is used to accept new paths for formatting. - inbox chan string + // inboxCh is used to accept new paths for formatting. + inboxCh chan string + // completedCh is used to wait for this formatter to finish all processing. + completedCh chan interface{} - // Entries from inbox are batched according to batchSize and stored in batch for processing when the batchSize has + // Entries from inboxCh are batched according to batchSize and stored in batch for processing when the batchSize has // been reached or Close is invoked. batch []string batchSize int @@ -66,9 +73,9 @@ func (f *Formatter) Init(name string, globalExcludes []glob.Glob) error { // initialise internal state f.log = log.WithPrefix("format | " + name) f.batchSize = 1024 - f.inbox = make(chan string, f.batchSize) - f.batch = make([]string, f.batchSize) - f.batch = f.batch[:0] + f.batch = make([]string, 0, f.batchSize) + f.inboxCh = make(chan string, f.batchSize) + f.completedCh = make(chan interface{}, 1) f.includes, err = CompileGlobs(f.Includes) if err != nil { @@ -84,6 +91,18 @@ func (f *Formatter) Init(name string, globalExcludes []glob.Glob) error { return nil } +func (f *Formatter) SetParent(formatter *Formatter) { + f.parent = formatter +} + +func (f *Formatter) Parent() *Formatter { + return f.parent +} + +func (f *Formatter) SetChild(formatter *Formatter) { + f.child = formatter +} + // Wants is used to test if a Formatter wants path based on it's configured Includes and Excludes patterns. // Returns true if the Formatter should be applied to path, false otherwise. func (f *Formatter) Wants(path string) bool { @@ -94,16 +113,26 @@ func (f *Formatter) Wants(path string) bool { return match } -// Put add path into this Formatter's inbox for processing. +// Put add path into this Formatter's inboxCh for processing. func (f *Formatter) Put(path string) { - f.inbox <- path + f.inboxCh <- path } // Run is the main processing loop for this Formatter. // It accepts a context which is used to lookup certain dependencies and for cancellation. func (f *Formatter) Run(ctx context.Context) (err error) { + defer func() { + if f.child != nil { + // indicate no further processing for the child formatter + f.child.Close() + } + + // indicate this formatter has finished processing + f.completedCh <- nil + }() + LOOP: - // keep processing until ctx has been cancelled or inbox has been closed + // keep processing until ctx has been cancelled or inboxCh has been closed for { select { @@ -112,8 +141,8 @@ LOOP: err = ctx.Err() break LOOP - case path, ok := <-f.inbox: - // check if the inbox has been closed + case path, ok := <-f.inboxCh: + // check if the inboxCh has been closed if !ok { break LOOP } @@ -167,9 +196,16 @@ func (f *Formatter) apply(ctx context.Context) error { f.log.Infof("%v files processed in %v", len(f.batch), time.Now().Sub(start)) - // mark each path in this batch as completed - for _, path := range f.batch { - MarkFormatComplete(ctx, path) + if f.child == nil { + // mark each path in this batch as completed + for _, path := range f.batch { + MarkPathComplete(ctx, path) + } + } else { + // otherwise forward each path onto the next formatter for processing + for _, path := range f.batch { + f.child.Put(path) + } } // reset batch @@ -180,5 +216,10 @@ func (f *Formatter) apply(ctx context.Context) error { // Close is used to indicate that a Formatter should process any remaining paths and then stop it's processing loop. func (f *Formatter) Close() { - close(f.inbox) + close(f.inboxCh) +} + +func (f *Formatter) AwaitCompletion() { + // todo support a timeout + <-f.completedCh }