allow: the ratchet is split in one place

cmd/refs/main.go

@@ -16,6 +16,7 @@ import (
 	"slices"
 	"strings"
 
+	"github.com/spin-stack/go-tools/internal/allow"
 	"github.com/spin-stack/go-tools/internal/refs"
 )
 
@@ -40,9 +41,9 @@ func taskExeFromEnv() string {
 }
 
 func run(root, allowPath string, pathspecs []string, taskExe string) error {
-	allow, err := refs.ReadAllow(allowPath)
+	allowed, err := allow.Read(allowPath)
 	if err != nil {
-		return err
+		return fmt.Errorf("refs: %w", err)
 	}
 	// A document either has something holding it to the tree or it is deleted, and this is
 	// what holds the ones named.
@@ -72,10 +73,10 @@ func run(root, allowPath string, pathspecs []string, taskExe string) error {
 	if err != nil {
 		return err
 	}
-	unexplained, stale := allow.Split(found)
+	unexplained, stale := allow.Split(allowed, found, refs.Finding.Key)
 
 	fmt.Printf("scanned: %d files    references that do not resolve: %d    explained (%s): %d\n",
-		len(files), len(found), allowPath, allow.Len())
+		len(files), len(found), allowPath, len(allowed))
 
 	if len(stale) > 0 {
 		fmt.Fprint(os.Stderr, "\nstale entries in the allowlist - nothing matches these any more, so the sentence\n"+

cmd/testquality/main.go

@@ -35,22 +35,15 @@ func main() {
 		os.Exit(2)
 	}
 
-	var unexplained []string
-	used := map[string]bool{}
-	for _, f := range findings {
-		if allowed[f.Key()] {
-			used[f.Key()] = true
-			continue
-		}
-		unexplained = append(unexplained, fmt.Sprintf("%s:%d: %s", relative(f.Pos.Filename), f.Pos.Line, f.Message))
+	unexplained, stale := allow.Split(allowed, findings, testquality.Finding.Key)
+	lines := make([]string, 0, len(unexplained))
+	for _, f := range unexplained {
+		lines = append(lines, fmt.Sprintf("%s:%d: %s", relative(f.Pos.Filename), f.Pos.Line, f.Message))
 	}
+	sort.Strings(lines)
 
-	// Stale exemptions fail too, or the list stops shrinking.
-	stale := allow.Stale(allowed, used)
-	sort.Strings(unexplained)
-
-	for _, u := range unexplained {
-		fmt.Fprintln(os.Stderr, u)
+	for _, l := range lines {
+		fmt.Fprintln(os.Stderr, l)
 	}
 	for _, s := range stale {
 		fmt.Fprintf(os.Stderr, "%s is in %s and is no longer a finding: remove the line\n", s, *allowPath)

internal/allow/allow.go

@@ -43,15 +43,27 @@ func Read(path string) (map[string]bool, error) {
 	return allowed, nil
 }
 
-// Stale is the keys of allowed that matched does not hold, sorted: an entry that excuses nothing
-// is a sentence rewritten or a test fixed, and a list that keeps it stops being a ratchet.
-func Stale(allowed, matched map[string]bool) []string {
-	var stale []string
-	for key := range allowed {
-		if !matched[key] {
-			stale = append(stale, key)
+// Split turns findings into the two halves the ratchet reports: what nothing in allowed explains,
+// in the order found, and the keys of allowed that explain nothing, sorted.
+//
+// The second half is the direction people forget. An entry that matches nothing means the
+// sentence was rewritten or the test fixed, and a list that keeps it goes on excusing something
+// nobody does - which is how an allowlist stops being a ratchet and becomes a place to put things.
+func Split[T any](allowed map[string]bool, found []T, key func(T) string) (unexplained []T, stale []string) {
+	matched := map[string]bool{}
+	for _, f := range found {
+		k := key(f)
+		if allowed[k] {
+			matched[k] = true
+			continue
+		}
+		unexplained = append(unexplained, f)
+	}
+	for k := range allowed {
+		if !matched[k] {
+			stale = append(stale, k)
 		}
 	}
 	sort.Strings(stale)
-	return stale
+	return unexplained, stale
 }

internal/allow/allow_test.go

@@ -50,9 +50,15 @@ func TestAnExceptionNeedsItsReasonAndItsFile(t *testing.T) {
 	}
 }
 
-func TestStaleIsWhatNothingMatched(t *testing.T) {
-	got := allow.Stale(map[string]bool{"b": true, "a": true, "c": true}, map[string]bool{"c": true})
-	if !slices.Equal(got, []string{"a", "b"}) {
-		t.Errorf("stale = %q", got)
+// Both halves of the ratchet: a finding nothing explains keeps its place, and an entry that
+// explains nothing is named, however many findings it would have explained.
+func TestSplitIsWhatNothingExplainsAndWhatExplainsNothing(t *testing.T) {
+	allowed := map[string]bool{"b": true, "a": true, "c": true}
+	unexplained, stale := allow.Split(allowed, []string{"z", "c", "y", "c"}, strings.ToLower)
+	if !slices.Equal(unexplained, []string{"z", "y"}) {
+		t.Errorf("unexplained = %q", unexplained)
+	}
+	if !slices.Equal(stale, []string{"a", "b"}) {
+		t.Errorf("stale = %q", stale)
 	}
 }

internal/refs/refs.go

@@ -35,7 +35,9 @@ type Finding struct {
 	Ref string
 }
 
-// Key is the allowlist's spelling of a finding.
+// Key is the allowlist's spelling of a finding: `<file><TAB><reference>`. An allowlist names the
+// references made on purpose to something that is not here - naming a thing to say it is gone, or
+// naming something in another repository.
 func (f Finding) Key() string { return f.File + "\t" + f.Ref }
 
 // TaskLister answers what `task` names resolve in a directory. It returns no error and no

internal/refs/refs_test.go

@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/spin-stack/go-tools/internal/allow"
 	"github.com/spin-stack/go-tools/internal/refs"
 )
 
@@ -212,15 +213,12 @@ func TestTheAllowlistTurnsBothWays(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	allow, err := refs.ReadAllow(allowPath)
+	allowed, err := allow.Read(allowPath)
 	if err != nil {
 		t.Fatalf("read allow: %v", err)
 	}
-	if allow.Len() != 2 {
-		t.Fatalf("the list holds %d entries, wanted 2", allow.Len())
-	}
 
-	unexplained, stale := allow.Split(check(t, root, onlyRoot("build")))
+	unexplained, stale := allow.Split(allowed, check(t, root, onlyRoot("build")), refs.Finding.Key)
 	wants(t, unexplained, "internal/audit")
 	if len(stale) != 1 || stale[0] != "CLAUDE.md\tinternal/ancient" {
 		t.Fatalf("stale entries are %q, wanted the one nothing matches", stale)