feat: ensure deterministic application of formatters

Signed-off-by: Brian McGee <[email protected]>

Author: brianmcgee

cli/format.go

@@ -11,6 +11,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"slices"
+	"sort"
 	"strings"
 	"syscall"
 	"time"
@@ -84,8 +85,18 @@ func (f *Format) Run() (err error) {
 		}
 	}
 
+	// sort the formatter names so that, as we construct pipelines, we add formatters in a determinstic fashion. This
+	// ensures a deterministic order even when all priority values are the same e.g. 0
+
+	names := make([]string, 0, len(cfg.Formatters))
+	for name := range cfg.Formatters {
+		names = append(names, name)
+	}
+	sort.Strings(names)
+
 	// init formatters
-	for name, formatterCfg := range cfg.Formatters {
+	for _, name := range names {
+		formatterCfg := cfg.Formatters[name]
 		formatter, err := format.NewFormatter(name, Cli.TreeRoot, formatterCfg, globalExcludes)
 		if errors.Is(err, format.ErrCommandNotFound) && Cli.AllowMissingFormatter {
 			l.Debugf("formatter not found: %v", name)

cli/format_test.go

@@ -1,11 +1,13 @@
 package cli
 
 import (
+	"bufio"
 	"fmt"
 	"os"
 	"os/exec"
 	"path"
 	"path/filepath"
+	"regexp"
 	"testing"
 
 	config2 "git.numtide.com/numtide/treefmt/config"
@@ -23,7 +25,7 @@ import (
 func TestAllowMissingFormatter(t *testing.T) {
 	as := require.New(t)
 
-	tempDir := t.TempDir()
+	tempDir := test.TempExamples(t)
 	configPath := tempDir + "/treefmt.toml"
 
 	test.WriteConfig(t, configPath, config2.Config{
@@ -529,3 +531,63 @@ go/main.go
 	as.NoError(err)
 	as.Contains(string(out), fmt.Sprintf("%d files changed", 3))
 }
+
+func TestDeterministicOrderingInPipeline(t *testing.T) {
+	as := require.New(t)
+
+	tempDir := test.TempExamples(t)
+	configPath := tempDir + "/treefmt.toml"
+
+	test.WriteConfig(t, configPath, config2.Config{
+		Formatters: map[string]*config2.Formatter{
+			// a and b should execute in lexicographical order as they have default priority 0, with c last since it has
+			// priority 1
+			"fmt-a": {
+				Command:  "test-fmt",
+				Options:  []string{"fmt-a"},
+				Includes: []string{"*.py"},
+				Pipeline: "foo",
+			},
+			"fmt-b": {
+				Command:  "test-fmt",
+				Options:  []string{"fmt-b"},
+				Includes: []string{"*.py"},
+				Pipeline: "foo",
+			},
+			"fmt-c": {
+				Command:  "test-fmt",
+				Options:  []string{"fmt-c"},
+				Includes: []string{"*.py"},
+				Pipeline: "foo",
+				Priority: 1,
+			},
+		},
+	})
+
+	_, err := cmd(t, "-C", tempDir)
+	as.NoError(err)
+
+	matcher := regexp.MustCompile("^fmt-(.*)")
+
+	// check each affected file for the sequence of test statements which should be prepended to the end
+	sequence := []string{"fmt-a", "fmt-b", "fmt-c"}
+	paths := []string{"python/main.py", "python/virtualenv_proxy.py"}
+
+	for _, p := range paths {
+		file, err := os.Open(filepath.Join(tempDir, p))
+		as.NoError(err)
+		scanner := bufio.NewScanner(file)
+
+		idx := 0
+
+		for scanner.Scan() {
+			line := scanner.Text()
+			matches := matcher.FindAllString(line, -1)
+			if len(matches) != 1 {
+				continue
+			}
+			as.Equal(sequence[idx], matches[0])
+			idx += 1
+		}
+	}
+}

format/formatter.go

@@ -93,7 +93,7 @@ func (f *Formatter) Apply(ctx context.Context, paths []string, filter bool) erro
 	return nil
 }
 
-// Wants is used to test if a Formatter wants path based on it's configured Includes and Excludes patterns.
+// Wants is used to test if a Formatter wants a 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 {
 	match := !PathMatches(path, f.excludes) && PathMatches(path, f.includes)

nix/formatters.nix

@@ -16,4 +16,17 @@ with pkgs; [
   statix
   deadnix
   terraform
+  # util for unit testing
+  (pkgs.writeShellApplication {
+    name = "test-fmt";
+    text = ''
+      VALUE="$1"
+      shift
+
+      # append value to each file
+      for FILE in "$@"; do
+          echo "$VALUE" >> "$FILE"
+      done
+    '';
+  })
 ]