From 851757835c6ec499bb790c1e819bbf584e2a837c Mon Sep 17 00:00:00 2001 From: Rene Turcios <95189389+CloudCorpRecords@users.noreply.github.com> Date: Wed, 4 Nov 2026 11:49:18 -0800 Subject: [PATCH] config: compare files against GOCACHE, not the user cache dir Dir skipped every file below os.UserCacheDir, taking it for a file in Go's build cache. That silently ignored the staticcheck.conf of any package living below the user cache directory (such as git worktrees kept in ~/.cache), as well as packages below paths that merely share a string prefix with it, and it missed the build cache whenever GOCACHE pointed elsewhere. Compare against the actual build cache instead: the GOCACHE environment variable, then `go env GOCACHE` (which also accounts for the go/env file), falling back to the default location below the user cache directory. Membership is now tested on a path boundary instead of a string prefix. Fixes #1847 Signed-off-by: Rene Turcios <95189389+CloudCorpRecords@users.noreply.github.com> --- config/config.go | 83 ++++++++++++++++++++++++-------- config/config_test.go | 108 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 19 deletions(-) create mode 100644 config/config_test.go diff --git a/config/config.go b/config/config.go index 1c47f699..6a2d79e3 100644 --- a/config/config.go +++ b/config/config.go @@ -6,9 +6,11 @@ import ( "go/ast" "go/token" "os" + "os/exec" "path/filepath" "reflect" "strings" + "sync" "github.com/BurntSushi/toml" "golang.org/x/tools/go/analysis" @@ -20,34 +22,77 @@ import ( // such directory could be determined, for example because all files // were located in Go's build cache. func Dir(files []string) string { - if len(files) == 0 { - return "" - } - cache, err := os.UserCacheDir() - if err != nil { - cache = "" - } - var path string + return dir(files, buildCacheDir()) +} + +// dir is like Dir, but takes the path of Go's build cache as an +// argument. Files located in the build cache get skipped, because +// they are generated files that don't belong to any user package. A +// buildCache of "" disables the skipping. +func dir(files []string, buildCache string) string { for _, p := range files { - // FIXME(dh): using strings.HasPrefix isn't technically - // correct, but it should be good enough for now. - if cache != "" && strings.HasPrefix(p, cache) { + if buildCache != "" && inDir(buildCache, p) { // File in the build cache of the standard Go build system continue } - path = p - break + return filepath.Dir(p) } - if path == "" { - // The package only consists of generated files. - return "" - } + // The package only consists of generated files. + return "" +} - dir := filepath.Dir(path) - return dir +// inDir reports whether path is located in dir, or in one of its +// subdirectories. +func inDir(dir, path string) bool { + rel, err := filepath.Rel(dir, path) + if err != nil { + return false + } + return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) } +// buildCacheDir returns the path of Go's build cache, that is, the +// value of GOCACHE. It returns "" if the build cache is turned off. +// +// We used to approximate the build cache with os.UserCacheDir, but +// that is wrong in both directions: it skips user packages that +// happen to live anywhere below the user cache directory, and it +// misses the build cache whenever GOCACHE is set to a directory +// outside of it. +var buildCacheDir = sync.OnceValue(func() string { + // The GOCACHE environment variable takes precedence. Note that + // cmd/go explicitly sets GOCACHE for its child processes. + if dir := os.Getenv("GOCACHE"); dir != "" { + if dir == "off" { + return "" + } + if filepath.IsAbs(dir) { + return filepath.Clean(dir) + } + // A relative GOCACHE would be resolved by cmd/go; fall + // through to asking it. + } + // Ask the go tool, which also accounts for GOCACHE set in the + // go/env file. + if out, err := exec.Command("go", "env", "GOCACHE").Output(); err == nil { + if dir := strings.TrimSpace(string(out)); dir != "" { + if dir == "off" { + return "" + } + if filepath.IsAbs(dir) { + return filepath.Clean(dir) + } + } + } + // Fall back to the default location of the build cache, below + // the user cache directory. + if cache, err := os.UserCacheDir(); err == nil { + return filepath.Join(cache, "go-build") + } + return "" +}) + func dirAST(files []*ast.File, fset *token.FileSet) string { names := make([]string, len(files)) for i, f := range files { diff --git a/config/config_test.go b/config/config_test.go new file mode 100644 index 00000000..f4270072 --- /dev/null +++ b/config/config_test.go @@ -0,0 +1,108 @@ +package config + +import ( + "path/filepath" + "testing" +) + +func TestDir(t *testing.T) { + // Layout used by the tests: + // + // root/ + // cache/ (stands in for os.UserCacheDir) + // go-build/ (the build cache, GOCACHE) + // repro/ (a user package below the user cache dir) + // cache-like/ (shares a string prefix with cache/) + root := filepath.Join(string(filepath.Separator), "root") + cache := filepath.Join(root, "cache") + buildCache := filepath.Join(cache, "go-build") + repro := filepath.Join(cache, "repro") + + tests := []struct { + name string + files []string + buildCache string + want string + }{ + { + name: "no files", + files: nil, + buildCache: buildCache, + want: "", + }, + { + name: "regular package", + files: []string{filepath.Join(root, "src", "x.go")}, + buildCache: buildCache, + want: filepath.Join(root, "src"), + }, + { + // Issue #1847: a package below the user cache directory + // must not be mistaken for a package in the build cache. + name: "package below user cache dir", + files: []string{filepath.Join(repro, "repro.go")}, + buildCache: buildCache, + want: repro, + }, + { + name: "package sharing prefix with user cache dir", + files: []string{filepath.Join(root, "cache-like", "repro", "repro.go")}, + buildCache: buildCache, + want: filepath.Join(root, "cache-like", "repro"), + }, + { + name: "package sharing prefix with build cache", + files: []string{filepath.Join(cache, "go-build-like", "repro.go")}, + buildCache: buildCache, + want: filepath.Join(cache, "go-build-like"), + }, + { + name: "package only in build cache", + files: []string{filepath.Join(buildCache, "ab", "ab123", "_testmain.go")}, + buildCache: buildCache, + want: "", + }, + { + name: "file directly in build cache", + files: []string{filepath.Join(buildCache, "x.go")}, + buildCache: buildCache, + want: "", + }, + { + name: "first file in build cache, second is real", + files: []string{ + filepath.Join(buildCache, "ab", "ab123", "_testmain.go"), + filepath.Join(repro, "repro.go"), + }, + buildCache: buildCache, + want: repro, + }, + { + name: "unknown build cache disables skipping", + files: []string{filepath.Join(buildCache, "ab", "x.go")}, + buildCache: "", + want: filepath.Join(buildCache, "ab"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := dir(tt.files, tt.buildCache); got != tt.want { + t.Errorf("dir(%q, %q) = %q, want %q", tt.files, tt.buildCache, got, tt.want) + } + }) + } +} + +func TestDirPublic(t *testing.T) { + // A package in a regular directory gets its directory, exercising + // the real build cache lookup behind Dir. + d := t.TempDir() + f := filepath.Join(d, "x.go") + if got := Dir([]string{f}); got != d { + t.Errorf("Dir(%q) = %q, want %q", f, got, d) + } + if got := Dir(nil); got != "" { + t.Errorf("Dir(nil) = %q, want %q", got, "") + } +}