support formatter ordering (#20)

Allows specifying the order in which formatters are applied.

Very simple for now, adding a `Before` field to the formatted config which allows the user to say that formatter `x` needs to be applied _before_ formatted `y`.

```toml
[formatter.statix]
command = "statix"
includes = ["*.nix"]
before = "deadnix"

[formatter.deadnix]
command = "statix"
includes = ["*.nix"]
```

Signed-off-by: Brian McGee <[email protected]>
Reviewed-on: https://git.numtide.com/numtide/treefmt/pulls/20
Reviewed-by: Jonas Chevalier <[email protected]>
Co-authored-by: Brian McGee <[email protected]>
Co-committed-by: Brian McGee <[email protected]>

Author: brianmcgee

internal/cli/format.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"os"
 	"os/signal"
+	"strings"
 	"syscall"
 	"time"
 
@@ -64,33 +65,83 @@ func (f *Format) Run() error {
 		}
 	}
 
+	formatters := make(map[string]*format.Formatter)
+
+	// detect broken dependencies
+	for name, config := range cfg.Formatters {
+		before := config.Before
+		if before != "" {
+			// check child formatter exists
+			_, ok := cfg.Formatters[before]
+			if !ok {
+				return fmt.Errorf("formatter %v is before %v but config for %v was not found", name, before, before)
+			}
+		}
+	}
+
+	// dependency cycle detection
+	for name, config := range cfg.Formatters {
+		var ok bool
+		var history []string
+		childName := name
+		for {
+			// add to history
+			history = append(history, childName)
+
+			if config.Before == "" {
+				break
+			} else if config.Before == name {
+				return fmt.Errorf("formatter cycle detected %v", strings.Join(history, " -> "))
+			}
+
+			// load child config
+			childName = config.Before
+			config, ok = cfg.Formatters[config.Before]
+			if !ok {
+				return fmt.Errorf("formatter not found: %v", config.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)
 			l.Debugf("formatter %v is not in formatter list %v, skipping", name, Cli.Formatters)
 			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
 	}
 
-	ctx = format.RegisterFormatters(ctx, cfg.Formatters)
+	// iterate the initialised formatters configuring parent/child relationships
+	for _, formatter := range formatters {
+		if formatter.Before() != "" {
+			child, ok := formatters[formatter.Before()]
+			if !ok {
+				// formatter has been filtered out by the user
+				formatter.ResetBefore()
+				continue
+			}
+			formatter.SetChild(child)
+			child.SetParent(formatter)
+		}
+	}
 
-	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
 	}
 
 	//
-	pendingCh := make(chan string, 1024)
 	completedCh := make(chan string, 1024)
 
 	ctx = format.SetCompletedChannel(ctx, completedCh)
@@ -99,8 +150,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)
 		})
@@ -114,20 +165,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 +185,6 @@ func (f *Format) Run() error {
 					changes += count
 					batch = batch[:0]
 				}
-
-				completed += 1
-
-				if completed == pending {
-					close(completedCh)
-				}
 			}
 		}
 
@@ -166,26 +204,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 {
+			for _, formatter := range formatters {
 				if formatter.Wants(path) {
-					pendingCh <- path
-					count += 1
 					formatter.Put(path)
 				}
 			}
 		}
 
-		for _, formatter := range cfg.Formatters {
+		// indicate no more paths for each formatter
+		for _, formatter := range 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 formatters {
+			formatter.AwaitCompletion()
 		}
 
+		// indicate no more completion events
+		close(completedCh)
+
 		return nil
 	})
 

internal/cli/format_test.go

@@ -25,7 +25,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",
 			},
@@ -39,14 +39,35 @@ func TestAllowMissingFormatter(t *testing.T) {
 	as.NoError(err)
 }
 
+func TestDependencyCycle(t *testing.T) {
+	as := require.New(t)
+
+	tempDir := t.TempDir()
+	configPath := tempDir + "/treefmt.toml"
+
+	test.WriteConfig(t, configPath, format.Config{
+		Formatters: map[string]*format.FormatterConfig{
+			"a": {Command: "echo", Before: "b"},
+			"b": {Command: "echo", Before: "c"},
+			"c": {Command: "echo", Before: "a"},
+			"d": {Command: "echo", Before: "e"},
+			"e": {Command: "echo", Before: "f"},
+			"f": {Command: "echo"},
+		},
+	})
+
+	_, err := cmd(t, "--config-file", configPath, "--tree-root", tempDir)
+	as.ErrorContains(err, "formatter cycle detected a -> b -> c")
+}
+
 func TestSpecifyingFormatters(t *testing.T) {
 	as := require.New(t)
 
 	tempDir := test.TempExamples(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"},
@@ -95,7 +116,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{"*"},
@@ -167,7 +188,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{"*"},
@@ -202,7 +223,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{"*"},
@@ -227,7 +248,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{"*"},
@@ -263,7 +284,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"},
@@ -307,7 +328,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"},
@@ -358,7 +379,7 @@ func TestGitWorktree(t *testing.T) {
 
 	// basic config
 	config := format.Config{
-		Formatters: map[string]*format.Formatter{
+		Formatters: map[string]*format.FormatterConfig{
 			"echo": {
 				Command:  "echo",
 				Includes: []string{"*"},
@@ -404,3 +425,57 @@ func TestGitWorktree(t *testing.T) {
 	as.NoError(err)
 	as.Contains(string(out), fmt.Sprintf("%d files changed", 55))
 }
+
+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.FormatterConfig{
+			"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.FormatterConfig{
+			"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")
+}

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.

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
 }