Fix cachedGitignore cache-key correctness

Address PR #215 review (thread on internal/fix/fix.go:65): the
previous "drop the Abs error fallback" simplification was wrong —
filepath.Abs returns "" on failure (not the input string), so on
the rare error path every relative dir would collide on the empty
cache key and share one matcher across unrelated directories. My
preceding comment also misdescribed Abs's error semantics.

Fix the right way: don't normalize the cache key at all.
lint.NewGitignoreMatcher does its own filepath.Abs internally to
root the matcher, so the cache key only needs to be deterministic
across calls within a Fix run, which prepareFile already
guarantees by passing the same form (filepath.Dir(path) or
f.RootDir). Use the dir string verbatim and update the comment to
match what the code actually does.

Add TestFixer_CachedGitignore_DistinctKeys documenting the cache
contract: distinct inputs yield distinct matchers, repeated input
hits the cache, empty-string input is its own entry rather than
aliasing with everything else.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: jeduden

internal/fix/fix.go

@@ -42,27 +42,26 @@ type Fixer struct {
 }
 
 // cachedGitignore returns a GitignoreMatcher for the given directory,
-// creating and caching it on first use. Mirrors engine.Runner so the
-// fix path's lint.File values give catalog (and any other rule that
-// calls f.GetGitignore()) the same matcher the check path would.
+// creating and caching it on first use so the matcher tree is walked
+// once per (Fixer, dir). Mirrors engine.Runner so the fix path's
+// lint.File values give catalog (and any other rule that calls
+// f.GetGitignore()) the same matcher the check path would.
 //
-// filepath.Abs is allowed to error (it can fail when the process
-// can't read its current directory); on that path it returns the
-// input string unchanged, which is still a usable cache key — the
-// only consequence is that two callers passing the same relative
-// path from different working directories would share a cache entry,
-// which is acceptable for the fix pipeline that always passes either
-// filepath.Dir(path) or f.RootDir.
+// The cache is keyed on the raw dir string. lint.NewGitignoreMatcher
+// canonicalizes the path internally before walking, so the matcher
+// itself is correctly rooted regardless of whether dir was passed
+// absolute or relative; the cache key just has to be deterministic
+// across calls within a Fix run, which prepareFile guarantees by
+// always passing the same form (filepath.Dir(path) or f.RootDir).
 func (f *Fixer) cachedGitignore(dir string) *lint.GitignoreMatcher {
 	if f.gitignoreCache == nil {
 		f.gitignoreCache = make(map[string]*lint.GitignoreMatcher)
 	}
-	absDir, _ := filepath.Abs(dir)
-	if m, ok := f.gitignoreCache[absDir]; ok {
+	if m, ok := f.gitignoreCache[dir]; ok {
 		return m
 	}
-	m := lint.NewGitignoreMatcher(absDir)
-	f.gitignoreCache[absDir] = m
+	m := lint.NewGitignoreMatcher(dir)
+	f.gitignoreCache[dir] = m
 	return m
 }
 

internal/fix/fix_generated_ranges_test.go

@@ -161,6 +161,38 @@ func TestFix_FilesHaveGitignoreFunc(t *testing.T) {
 	}
 }
 
+// TestFixer_CachedGitignore_DistinctKeys directly exercises the
+// cache contract: distinct inputs must yield distinct matchers, and
+// a repeat input must hit the cache. The previous implementation
+// canonicalized via filepath.Abs and silently swallowed Abs's error
+// return, which would collapse every relative path to the empty
+// string ("") cache key on Abs failure (e.g., unreadable cwd) and
+// share one matcher across unrelated directories.
+func TestFixer_CachedGitignore_DistinctKeys(t *testing.T) {
+	fixer := &Fixer{}
+
+	a1 := fixer.cachedGitignore("/tmp/aaa")
+	b := fixer.cachedGitignore("/tmp/bbb")
+	a2 := fixer.cachedGitignore("/tmp/aaa")
+	empty1 := fixer.cachedGitignore("")
+	empty2 := fixer.cachedGitignore("")
+
+	require.NotNil(t, a1)
+	require.NotNil(t, b)
+	require.NotNil(t, empty1)
+
+	assert.NotSame(t, a1, b,
+		"different directories must produce different matchers; the previous "+
+			"Abs-no-fallback impl shared one cache entry across unrelated dirs "+
+			"on the Abs-error path")
+	assert.Same(t, a1, a2,
+		"repeated input must hit the cache and return the same matcher pointer")
+	assert.Same(t, empty1, empty2,
+		"empty-string input is its own cache entry, not aliased with /tmp/aaa")
+	assert.NotSame(t, a1, empty1,
+		"empty-string input must not collide with /tmp/aaa")
+}
+
 // TestFix_FilesHaveGitignoreFuncWithRootDir covers the prepareFile
 // branch where Fixer.RootDir is set (gitignore matcher is anchored at
 // the root rather than the file's directory). Without a test that