From 40b76b74a0053324d683823ef7ad70f5b29f0dc9 Mon Sep 17 00:00:00 2001
From: Brian McGee <brian@bmcgee.ie>
Date: Fri, 26 Apr 2024 10:27:11 +0100
Subject: [PATCH] feat: ensure deterministic application of formatters

Signed-off-by: Brian McGee <brian@bmcgee.ie>
---
 cli/format.go       | 13 ++++++++-
 cli/format_test.go  | 64 ++++++++++++++++++++++++++++++++++++++++++++-
 format/formatter.go |  2 +-
 nix/formatters.nix  | 13 +++++++++
 4 files changed, 89 insertions(+), 3 deletions(-)

diff --git a/cli/format.go b/cli/format.go
index 873e5a2..6f0b356 100644
--- a/cli/format.go
+++ b/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)
diff --git a/cli/format_test.go b/cli/format_test.go
index 9356c97..df02288 100644
--- a/cli/format_test.go
+++ b/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
+		}
+	}
+}
diff --git a/format/formatter.go b/format/formatter.go
index ae2e759..e2ec9e3 100644
--- a/format/formatter.go
+++ b/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)
diff --git a/nix/formatters.nix b/nix/formatters.nix
index 5bce9a5..8454d9d 100644
--- a/nix/formatters.nix
+++ b/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
+    '';
+  })
 ]