From 58c2256770102ef92130a99f601e3310f6d7bb18 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 18:51:43 +0000 Subject: [PATCH 1/5] feat(modgraph): the module graph of a Go module, with no Go toolchain A go.mod lists the modules of a build, but not which module imports which. Without the edges, a caller such as vet cannot tell that no code imports a module, or that no other module can call a vulnerable one. modgraph.Build reads the import blocks of the Go files on the disk, with go/parser in ImportsOnly mode. It starts at the packages of the main module, its tests too, and reads each package that an import reaches, as go list -deps does. It runs no go command, opens no network connection and needs no CGO. The graph may hold an edge that a build does not have, and it never misses one that a build has: - A file counts unless its build constraint can never hold, with every tag free except ignore. - The resolver tests the modules whose path is a prefix of an import, longest first. A module with no source may hold the package, so it gets an edge too. A module path is tested before the standard library. - A module with no source and only a go.mod is open, because its go.mod need not list each module that its code imports. Its require lines stay as evidence. - A module with no source and no go.mod, an import that no module holds, a file that does not parse and the end of the file budget make a module open. Once a reached node is open, each node is reached. - A main go.mod before go 1.17 gives ErrUnpruned. modgraph.Discover finds each source as the go command builds it: a replace target, then vendor at the version of go.mod, then the module cache, then the .mod file of the cache. It reads a local replacement only inside LocalRoot. Build caches the imports of each package of the module cache in the store, by module, version, directory and go.sum hash, and never caches a package that it could not read in full. TestBuildHoldsEachEdgeOfGoList runs go list -deps -test for four platforms and checks that the graph holds each module edge. On this repository and on safedep/vet, with and without vendor, the graph holds each edge and no reached node is open. A graph with the golang.org/x edges removed fails it with 630 errors. examples/modgraph prints the graph of a module, and docs/modgraph.md describes the rules. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PFQbqPqd9xn1fT6hBsyoUm Signed-off-by: Claude --- README.md | 3 + docs/modgraph.md | 105 +++++++ examples/modgraph/main.go | 81 +++++ examples/modgraph/main_test.go | 34 ++ modgraph/cache.go | 83 +++++ modgraph/discover.go | 199 ++++++++++++ modgraph/discover_test.go | 133 ++++++++ modgraph/files.go | 231 ++++++++++++++ modgraph/graph.go | 554 +++++++++++++++++++++++++++++++++ modgraph/graph_test.go | 474 ++++++++++++++++++++++++++++ modgraph/oracle_test.go | 180 +++++++++++ modgraph/resolve.go | 107 +++++++ 12 files changed, 2184 insertions(+) create mode 100644 docs/modgraph.md create mode 100644 examples/modgraph/main.go create mode 100644 examples/modgraph/main_test.go create mode 100644 modgraph/cache.go create mode 100644 modgraph/discover.go create mode 100644 modgraph/discover_test.go create mode 100644 modgraph/files.go create mode 100644 modgraph/graph.go create mode 100644 modgraph/graph_test.go create mode 100644 modgraph/oracle_test.go create mode 100644 modgraph/resolve.go diff --git a/README.md b/README.md index b1c43f58..8d76b42c 100644 --- a/README.md +++ b/README.md @@ -13,3 +13,6 @@ See [docs/detection.md](./docs/detection.md) for the APIs that security detectio See [docs/projectgraph.md](./docs/projectgraph.md) for the project graph: the call graph of a whole project across its files and into its dependencies, with the index, link, roots, reach and summary steps. + +See [docs/modgraph.md](./docs/modgraph.md) for the module graph of a Go module: which module +imports which, from the code on the disk, with no Go toolchain. diff --git a/docs/modgraph.md b/docs/modgraph.md new file mode 100644 index 00000000..64f6d9c3 --- /dev/null +++ b/docs/modgraph.md @@ -0,0 +1,105 @@ +# Module graph + +`modgraph` builds the module graph of a Go module: which module of the build +list imports which. A `go.mod` lists the modules of a build, but not the +edges between them. A caller needs the edges to tell that no code imports a +module, or that no other module can call one. + +The package reads the code on the disk. It runs no `go` command, opens no +network connection and builds with `CGO_ENABLED=0`. It parses each Go file +with `go/parser` in `ImportsOnly` mode, so it reads the build constraint and +the import block of a file and stops. + +## The rule + +The graph may hold an edge that a build does not have. It never misses an +edge that a build has. A caller can then trust the absence of a path: no +build of any platform or tag set can reach a module that the graph does not +reach. + +So a module whose imports `Build` cannot read is **open**: it may import any +module, and a path that meets it reaches each module. + +| State | The module | Edges | +| --- | --- | --- | +| `imports` | Has source. `Build` read each package of it that the build reaches | The imports of those packages | +| `requires` | Has no source, only its `go.mod` | Its require lines, as evidence only. A `go.mod` need not list each module that its code imports, because the main `go.mod` can give it as `// indirect`. So the node is open | +| `open` | Has nothing that `Build` can read, or an import that `Build` cannot resolve | None | + +## The walk + +1. `Build` reads each package of the main module, its test files too. It + skips `testdata`, `vendor`, a directory or file whose name starts with + `_` or `.`, and a directory with a `go.mod` of its own. +2. For each import, the resolver takes the modules of the build list whose + path is a prefix of the import path, longest first. A module with source + holds the package when it has the directory. A module with no source may + hold it, so it gets an edge too, and the test goes on to a shorter prefix. + A path that no module holds is the standard library when its first + element has no dot. Else its importer is open. +3. The walk reads each package that an import reaches, as `go list -deps` + does. A package of a dependency that no import reaches adds nothing. Test + files of a dependency never count. +4. A file counts unless its build constraint can never hold, with every tag + free except `ignore`. A `//go:build` line decides, else the `// +build` + lines. A file of `//go:build purego` counts, because a project can build + with `-tags purego`. A GOOS or GOARCH suffix of the file name never drops + a file. +5. Once a reached node is open, each node counts as reached, and a module + that the walk read no package of becomes open. + +A main `go.mod` with a `go` directive below 1.17, or with none, gives +`ErrUnpruned`. Before module graph pruning, `go.mod` need not list each +module of the build. + +## Use + +```go +sources, err := modgraph.Discover(modgraph.DiscoverOptions{ + Dir: projectDir, // the directory of the main go.mod + ModuleCache: moduleCache, // GOMODCACHE; the caller finds it + LocalRoot: scanRoot, // a local replace directive is read only inside it +}) +g, err := modgraph.Build(ctx, modgraph.Input{ + MainFS: os.DirFS(projectDir), + Modules: sources, + Store: store, // optional cache of the packages of the module cache +}) +g.Reaches(g.Main.Path, "github.com/x/y") +``` + +`Discover` finds each module as the go command builds it: the target of a +`replace` directive, then `vendor` when `vendor/modules.txt` lists the +module at its version, then the module cache, then the `.mod` file of the +module cache. A caller that finds the sources in another way can pass its +own `[]Source`. `Module.Path` is the path that import statements use, also +for a replaced module. + +`examples/modgraph` prints the graph of a module. Run `go mod download` +first: a module with no source on the disk is open, and one open node that +the build reaches makes each module reached. + +## Cache + +`Input.Store` caches the imports of each package of an immutable source, +the module cache, by module, version, directory and `go.sum` hash. A +`vendor` directory and a local replacement can change, so they are read on +each run. `Build` stores no entry for a package that it could not read in +full. + +## Oracle test + +`TestBuildHoldsEachEdgeOfGoList` runs `go list -deps -test` on this +repository for four platforms and checks that the graph holds each module +edge, or that the edge leaves an open node. `MODGRAPH_ORACLE_DIRS` adds more +projects, and a project with `vendor/modules.txt` reads its dependencies +from `vendor`. The test needs the `go` command and a module cache, and skips +with neither. It sets `GOPROXY=off`, so it downloads nothing. + +## Not covered + +- `go.work` workspaces. A caller keeps no graph for a workspace. +- A file larger than 16 MiB whose first 64 KiB does not parse makes its + module open. +- A file budget, 200,000 files by default, caps a run. Past it the module + that is next is open, and `Stats.Truncated` is set. diff --git a/examples/modgraph/main.go b/examples/modgraph/main.go new file mode 100644 index 00000000..1c55820e --- /dev/null +++ b/examples/modgraph/main.go @@ -0,0 +1,81 @@ +// Command modgraph prints the module graph of a Go module: each module of +// the build list, what Build knows of its imports, and the edges between +// them. It finds the source of each module with modgraph.Discover in the +// vendor directory and in the module cache, which -modcache names, or +// $GOMODCACHE, or $GOPATH/pkg/mod. It runs no go command and downloads +// nothing. Run go mod download first for a graph with no open node. +// +// go run ./examples/modgraph -dir ../vet +// go run ./examples/modgraph -dir ../vet -json +package main + +import ( + "cmp" + "context" + "encoding/json" + "flag" + "fmt" + "io" + "os" + "path/filepath" + + "github.com/safedep/code/modgraph" +) + +func main() { + dir := flag.String("dir", ".", "the directory of the main go.mod") + modcache := flag.String("modcache", "", "the module cache, else $GOMODCACHE, else $GOPATH/pkg/mod, else $HOME/go/pkg/mod") + asJSON := flag.Bool("json", false, "print the graph as JSON") + flag.Parse() + if err := run(os.Stdout, *dir, *modcache, *asJSON); err != nil { + fmt.Fprintln(os.Stderr, "modgraph:", err) + os.Exit(1) + } +} + +func run(w io.Writer, dir, modcache string, asJSON bool) error { + sources, err := modgraph.Discover(modgraph.DiscoverOptions{Dir: dir, ModuleCache: moduleCache(modcache), LocalRoot: dir}) + if err != nil { + return err + } + g, err := modgraph.Build(context.Background(), modgraph.Input{MainFS: os.DirFS(dir), Modules: sources}) + if err != nil { + return err + } + if asJSON { + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + return enc.Encode(g) + } + for _, n := range g.Nodes { + reached := "unreached" + if n.Reached { + reached = "reached" + } + if _, err := fmt.Fprintf(w, "%-9s %-9s %s %s\n", reached, n.State, n.Module, n.Reason); err != nil { + return err + } + } + for _, e := range g.Edges { + if _, err := fmt.Fprintf(w, "%s -> %s (%s, %s:%d)\n", e.From.Path, e.To.Path, e.Kind, e.Evidence.File, e.Evidence.Line); err != nil { + return err + } + } + _, err = fmt.Fprintf(w, "%d files, %d packages\n", g.Stats.Files, g.Stats.Packages) + return err +} + +// moduleCache finds the module cache as the go command does, with no go +// command. +func moduleCache(flagValue string) string { + if c := cmp.Or(flagValue, os.Getenv("GOMODCACHE")); c != "" { + return c + } + if p := filepath.SplitList(os.Getenv("GOPATH")); len(p) > 0 && p[0] != "" { + return filepath.Join(p[0], "pkg", "mod") + } + if home, err := os.UserHomeDir(); err == nil { + return filepath.Join(home, "go", "pkg", "mod") + } + return "" +} diff --git a/examples/modgraph/main_test.go b/examples/modgraph/main_test.go new file mode 100644 index 00000000..fb6674ed --- /dev/null +++ b/examples/modgraph/main_test.go @@ -0,0 +1,34 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRunPrintsTheGraph(t *testing.T) { + dir := t.TempDir() + cache := t.TempDir() + for name, data := range map[string]string{ + filepath.Join(dir, "go.mod"): "module example.com/app\n\ngo 1.22\n\nrequire example.com/dep v1.0.0\n", + filepath.Join(dir, "main.go"): "package main\n\nimport _ \"example.com/dep\"\n", + filepath.Join(cache, "example.com/dep@v1.0.0/d.go"): "package dep\n\nimport _ \"fmt\"\n", + } { + require.NoError(t, os.MkdirAll(filepath.Dir(name), 0o755)) + require.NoError(t, os.WriteFile(name, []byte(data), 0o600)) + } + + var out strings.Builder + require.NoError(t, run(&out, dir, cache, false)) + assert.Contains(t, out.String(), "reached imports example.com/dep@v1.0.0") + assert.Contains(t, out.String(), "example.com/app -> example.com/dep (imports, main.go:3)") + assert.Contains(t, out.String(), "example.com/dep -> std (imports, d.go:3)") + + out.Reset() + require.NoError(t, run(&out, dir, cache, true)) + assert.Contains(t, out.String(), `"Path": "example.com/dep"`) +} diff --git a/modgraph/cache.go b/modgraph/cache.go new file mode 100644 index 00000000..7edfb8ae --- /dev/null +++ b/modgraph/cache.go @@ -0,0 +1,83 @@ +package modgraph + +import ( + "encoding/json" + "errors" + "fmt" + + "github.com/safedep/code/fileindex" + "github.com/safedep/code/fileindex/store" +) + +// cachePlugin is the plugin name of the artifacts of Build. +const cachePlugin = "modgraph" + +// errCache marks a failure of the store. It ends Build, because it says +// nothing about the module. +var errCache = errors.New("modgraph: cache") + +// cacheEntry is the artifact of one package: the imports of its Go files +// that count. Build stores an entry only after it read each file of the +// package with no error, so an entry never stands for a part of a package. +type cacheEntry struct { + Imports []importRef `json:"imports"` +} + +// cacheKey is the key of a package of an immutable source. Its path is the +// unit and the directory, as "go:golang.org/x/net@v0.30.0//html". The go.sum +// hash, when the caller has it, separates two copies of one version with +// other bytes. +func cacheKey(ref pkgRef) store.ArtifactKey { + m := ref.mod.src.Module + unit := fileindex.PackageUnit{Ecosystem: "go", Name: m.Path, Version: m.Version}.String() + return store.ArtifactKey{ + Key: fileindex.Key{Path: fileindex.UnitPath(unit, ref.dir), ContentHash: ref.mod.src.Sum}, + Plugin: cachePlugin, + Version: Version, + } +} + +// cacheable reports whether the imports of a package can come from, and go +// to, the cache. +func (b *builder) cacheable(ref pkgRef) bool { + return b.in.Store != nil && !ref.mod.main && ref.mod.src.Immutable && ref.mod.src.Module.Version != "" +} + +// cached returns the imports of a package from the cache. +func (b *builder) cached(ref pkgRef) ([]importRef, bool, error) { + if !b.cacheable(ref) { + return nil, false, nil + } + k := cacheKey(ref) + found, err := b.in.Store.GetArtifacts(b.ctx, []store.ArtifactKey{k}) + if err != nil { + return nil, false, fmt.Errorf("%w: read %s: %w", errCache, k.Path, err) + } + data, ok := found[k] + if !ok { + return nil, false, nil + } + var e cacheEntry + if err := json.Unmarshal(data, &e); err != nil { + // A bad entry is a miss. The read that follows writes a good one. + return nil, false, nil + } + b.stats.Packages++ + b.stats.CachedPackages++ + return e.Imports, true, nil +} + +// cache stores the imports of a package that Build read whole. +func (b *builder) cache(ref pkgRef, imps []importRef) error { + if !b.cacheable(ref) { + return nil + } + data, err := json.Marshal(cacheEntry{Imports: imps}) + if err != nil { + return fmt.Errorf("encode the cache entry of %s: %w", ref.dir, err) + } + if err := b.in.Store.PutArtifacts(b.ctx, []store.Artifact{{Key: cacheKey(ref), Data: data}}); err != nil { + return fmt.Errorf("%w: write %s: %w", errCache, ref.dir, err) + } + return nil +} diff --git a/modgraph/discover.go b/modgraph/discover.go new file mode 100644 index 00000000..cd124c25 --- /dev/null +++ b/modgraph/discover.go @@ -0,0 +1,199 @@ +package modgraph + +import ( + "bufio" + "bytes" + "fmt" + "io/fs" + "os" + "path/filepath" + "slices" + "strings" + + "golang.org/x/mod/modfile" + gomodule "golang.org/x/mod/module" +) + +// DiscoverOptions says where Discover looks for the source of each module. +type DiscoverOptions struct { + // Dir is the directory of the main go.mod. + Dir string + // ModuleCache is the module cache of the go command, as GOMODCACHE, or "" + // for none. The caller finds it, so Discover reads no environment. + ModuleCache string + // LocalRoot bounds a replace directive with a local path: Discover reads + // a replacement only inside it, as the root of the scan. "" reads none. + LocalRoot string +} + +// Discover finds the source of each module of the build list of a main +// module, as the go command builds it: the replacement of a replace +// directive, then the vendor directory when vendor/modules.txt lists the +// module at its version, then the module cache. A module with no source +// gets the go.mod of the module cache, when the cache has it. Discover never +// downloads, and a module that it cannot find has no source and no go.mod, +// so Build makes it open. +func Discover(opts DiscoverOptions) ([]Source, error) { + data, err := os.ReadFile(filepath.Join(opts.Dir, "go.mod")) + if err != nil { + return nil, fmt.Errorf("modgraph: read go.mod: %w", err) + } + // A lax parse drops the replace directives, and a replaced module read + // from the module cache is other code. So go.mod must parse in full. + f, err := modfile.Parse("go.mod", data, nil) + if err != nil { + return nil, fmt.Errorf("modgraph: parse go.mod: %w", err) + } + vendored := readVendorList(filepath.Join(opts.Dir, "vendor", "modules.txt")) + sums := readSums(filepath.Join(opts.Dir, "go.sum")) + var out []Source + for _, r := range f.Require { + s := Source{Module: Module{Path: r.Mod.Path, Version: r.Mod.Version}} + src := replacement(f, r.Mod) + switch { + case src.Version == "": + s.FS, s.GoMod = localSource(opts, src.Path) + case vendored[r.Mod.Path] == src: + s.FS = os.DirFS(filepath.Join(opts.Dir, "vendor", filepath.FromSlash(r.Mod.Path))) + default: + s.FS, s.GoMod = cacheSource(opts.ModuleCache, src) + s.Immutable = s.FS != nil + s.Sum = sums[src] + } + out = append(out, s) + } + return out, nil +} + +// replacement returns the module that the go command builds for a module of +// the build list: the target of the replace directive of that version, else +// of a replace directive of every version, else the module itself. +func replacement(f *modfile.File, m gomodule.Version) gomodule.Version { + var all *modfile.Replace + for _, r := range f.Replace { + switch { + case r.Old.Path != m.Path: + case r.Old.Version == m.Version: + return r.New + case r.Old.Version == "": + all = r + } + } + if all != nil { + return all.New + } + return m +} + +// localSource returns the source of a replacement with a local path, inside +// LocalRoot only. +func localSource(opts DiscoverOptions, p string) (fs.FS, []byte) { + if opts.LocalRoot == "" { + return nil, nil + } + dir := filepath.FromSlash(p) + if !filepath.IsAbs(dir) { + dir = filepath.Join(opts.Dir, dir) + } + root, err := filepath.Abs(opts.LocalRoot) + if err != nil { + return nil, nil + } + dir, err = filepath.Abs(dir) + if err != nil { + return nil, nil + } + if rel, err := filepath.Rel(root, dir); err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return nil, nil + } + if info, err := os.Stat(dir); err != nil || !info.IsDir() { + return nil, nil + } + return os.DirFS(dir), readOptional(filepath.Join(dir, "go.mod")) +} + +// cacheSource returns the extracted source of a module version in the +// module cache, and its go.mod from cache/download. The escaping of the go +// command keeps a path or a version from leaving the cache. +func cacheSource(cache string, m gomodule.Version) (fs.FS, []byte) { + if cache == "" { + return nil, nil + } + escPath, err := gomodule.EscapePath(m.Path) + if err != nil { + return nil, nil + } + escVersion, err := gomodule.EscapeVersion(m.Version) + if err != nil { + return nil, nil + } + gomod := readOptional(filepath.Join(cache, "cache", "download", filepath.FromSlash(escPath), "@v", escVersion+".mod")) + dir := filepath.Join(cache, filepath.FromSlash(escPath)+"@"+escVersion) + if info, err := os.Stat(dir); err != nil || !info.IsDir() { + return nil, gomod + } + return os.DirFS(dir), gomod +} + +// readOptional reads a file that may be absent. A file that Discover cannot +// read counts as absent, so Build makes its module open, which keeps each +// edge. +func readOptional(name string) []byte { + data, err := os.ReadFile(name) + if err != nil { + return nil + } + return data +} + +// readVendorList reads the modules that vendor/modules.txt lists, each with +// the module that the vendor directory holds for it: the module itself, or +// the target of its replace. A missing file lists none. +func readVendorList(name string) map[string]gomodule.Version { + data, err := os.ReadFile(name) + if err != nil { + return nil + } + out := map[string]gomodule.Version{} + sc := bufio.NewScanner(bytes.NewReader(data)) + for sc.Scan() { + // A module line is "# path version", or + // "# path [version] => target [version]". + fields := strings.Fields(sc.Text()) + if len(fields) < 3 || fields[0] != "#" { + continue + } + path, rest := fields[1], fields[2:] + if i := slices.Index(rest, "=>"); i >= 0 { + target := rest[i+1:] + switch len(target) { + case 1: + out[path] = gomodule.Version{Path: target[0]} + case 2: + out[path] = gomodule.Version{Path: target[0], Version: target[1]} + } + continue + } + out[path] = gomodule.Version{Path: path, Version: rest[0]} + } + return out +} + +// readSums reads the h1 hash of each module version in go.sum. A go.mod +// hash line is not the hash of the source, so it does not count. +func readSums(name string) map[gomodule.Version]string { + data, err := os.ReadFile(name) + if err != nil { + return nil + } + out := map[gomodule.Version]string{} + sc := bufio.NewScanner(bytes.NewReader(data)) + for sc.Scan() { + fields := strings.Fields(sc.Text()) + if len(fields) != 3 || strings.HasSuffix(fields[1], "/go.mod") { + continue + } + out[gomodule.Version{Path: fields[0], Version: fields[1]}] = fields[2] + } + return out +} diff --git a/modgraph/discover_test.go b/modgraph/discover_test.go new file mode 100644 index 00000000..d00aa216 --- /dev/null +++ b/modgraph/discover_test.go @@ -0,0 +1,133 @@ +package modgraph + +import ( + "io/fs" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func writeFiles(t *testing.T, root string, files map[string]string) { + t.Helper() + for name, data := range files { + p := filepath.Join(root, filepath.FromSlash(name)) + require.NoError(t, os.MkdirAll(filepath.Dir(p), 0o755)) + require.NoError(t, os.WriteFile(p, []byte(data), 0o600)) + } +} + +func sourceOf(t *testing.T, sources []Source, path string) Source { + t.Helper() + for _, s := range sources { + if s.Module.Path == path { + return s + } + } + require.Failf(t, "no source", "%s", path) + return Source{} +} + +func hasFile(t *testing.T, fsys fs.FS, name string) bool { + t.Helper() + require.NotNil(t, fsys) + _, err := fs.Stat(fsys, name) + return err == nil +} + +func TestDiscoverFindsEachSourceAsTheGoCommandBuildsIt(t *testing.T) { + root := t.TempDir() + cache := t.TempDir() + writeFiles(t, root, map[string]string{ + "app/go.mod": `module example.com/app + +go 1.22 + +require ( + example.com/cached v1.0.0 + example.com/modonly v1.0.0 + example.com/missing v1.0.0 + github.com/Upper/case v1.2.0 + example.com/vendored v1.1.0 + example.com/local v1.0.0 + example.com/outside v1.0.0 + example.com/forked v1.0.0 + example.com/exact v1.0.0 +) + +replace example.com/local => ../lib +replace example.com/outside => ../../elsewhere +replace example.com/forked => example.com/fork v2.0.0 +replace example.com/exact v0.9.0 => example.com/never v1.0.0 +`, + "app/go.sum": "example.com/cached v1.0.0 h1:source=\nexample.com/cached v1.0.0/go.mod h1:gomod=\n", + "app/vendor/modules.txt": "# example.com/vendored v1.1.0\n## explicit; go 1.21\nexample.com/vendored\n", + "app/vendor/example.com/vendored/v.go": "package vendored\n", + "lib/go.mod": "module example.com/local\n\ngo 1.22\n", + "lib/l.go": "package local\n", + }) + writeFiles(t, cache, map[string]string{ + "example.com/cached@v1.0.0/c.go": "package cached\n", + "cache/download/example.com/cached/@v/v1.0.0.mod": "module example.com/cached\n", + "cache/download/example.com/modonly/@v/v1.0.0.mod": "module example.com/modonly\n", + "github.com/!upper/case@v1.2.0/u.go": "package case\n", + "example.com/fork@v2.0.0/f.go": "package fork\n", + "example.com/exact@v1.0.0/e.go": "package exact\n", + }) + + sources, err := Discover(DiscoverOptions{Dir: filepath.Join(root, "app"), ModuleCache: cache, LocalRoot: root}) + require.NoError(t, err) + require.Len(t, sources, 9) + + cached := sourceOf(t, sources, "example.com/cached") + assert.True(t, hasFile(t, cached.FS, "c.go")) + assert.True(t, cached.Immutable) + assert.Equal(t, "h1:source=", cached.Sum, "the hash of the source, not of its go.mod") + assert.Equal(t, "module example.com/cached\n", string(cached.GoMod)) + + modonly := sourceOf(t, sources, "example.com/modonly") + assert.Nil(t, modonly.FS) + assert.Equal(t, "module example.com/modonly\n", string(modonly.GoMod)) + + missing := sourceOf(t, sources, "example.com/missing") + assert.Nil(t, missing.FS) + assert.Nil(t, missing.GoMod) + + assert.True(t, hasFile(t, sourceOf(t, sources, "github.com/Upper/case").FS, "u.go"), "the module cache escapes upper case") + + vendored := sourceOf(t, sources, "example.com/vendored") + assert.True(t, hasFile(t, vendored.FS, "v.go")) + assert.False(t, vendored.Immutable, "a vendor directory can change") + + local := sourceOf(t, sources, "example.com/local") + assert.True(t, hasFile(t, local.FS, "l.go")) + assert.False(t, local.Immutable) + assert.Equal(t, Module{Path: "example.com/local", Version: "v1.0.0"}, local.Module, "the node keeps the path that imports use") + + assert.Nil(t, sourceOf(t, sources, "example.com/outside").FS, "a local replacement outside the root is not read") + + forked := sourceOf(t, sources, "example.com/forked") + assert.True(t, hasFile(t, forked.FS, "f.go")) + assert.Equal(t, "example.com/forked", forked.Module.Path) + + assert.True(t, hasFile(t, sourceOf(t, sources, "example.com/exact").FS, "e.go"), "a replace of another version does not apply") +} + +func TestDiscoverReadsAVendorDirectoryOnlyAtTheVersionOfGoMod(t *testing.T) { + root := t.TempDir() + writeFiles(t, root, map[string]string{ + "go.mod": "module example.com/app\n\ngo 1.22\n\nrequire example.com/v v1.2.0\n", + "vendor/modules.txt": "# example.com/v v1.1.0\nexample.com/v\n", + "vendor/example.com/v/v.go": "package v\n", + }) + sources, err := Discover(DiscoverOptions{Dir: root}) + require.NoError(t, err) + assert.Nil(t, sources[0].FS, "a vendor copy of another version proves nothing") +} + +func TestDiscoverNeedsAGoMod(t *testing.T) { + _, err := Discover(DiscoverOptions{Dir: t.TempDir()}) + assert.ErrorContains(t, err, "read go.mod") +} diff --git a/modgraph/files.go b/modgraph/files.go new file mode 100644 index 00000000..042f15a4 --- /dev/null +++ b/modgraph/files.go @@ -0,0 +1,231 @@ +package modgraph + +import ( + "errors" + "fmt" + "go/ast" + "go/build/constraint" + "go/parser" + "io" + "io/fs" + "path" + "strconv" + "strings" +) + +const ( + // headSize is the part of a Go file that Build reads first. The import + // block of a Go file is near its top, so the head almost always holds it. + headSize = 64 << 10 + // maxFileSize caps a Go file that Build reads whole, when its head does + // not parse. A larger file makes its module open. + maxFileSize = 16 << 20 + // maxConstraintTags caps the tags of a build constraint that Build tests + // one by one. A constraint with more tags counts as one that can hold. + maxConstraintTags = 16 +) + +var errBudget = errors.New("the file budget ran out") + +// importRef is one import of a Go file. +type importRef struct { + Path string `json:"path"` + // File is the path of the file, relative to the root of its module. + File string `json:"file"` + Line int `json:"line"` + // Test marks an import of a _test.go file. Only the main module reads + // its test files. + Test bool `json:"test,omitempty"` +} + +// skippedDir reports whether the go command skips a directory of a module: +// testdata, vendor, and a name that starts with _ or a dot. +func skippedDir(name string) bool { + return name == "testdata" || name == "vendor" || strings.HasPrefix(name, "_") || strings.HasPrefix(name, ".") +} + +// countedFile reports whether the go command can build a file of a package +// for an importer, by its name: a .go file whose name does not start with _ +// or a dot. A _test.go file counts only for the main module, whose tests the +// build of the project runs. A GOOS or GOARCH suffix of the name never drops +// a file, because the graph holds the edges of each platform. +func countedFile(name string, tests bool) bool { + if path.Ext(name) != ".go" || strings.HasPrefix(name, "_") || strings.HasPrefix(name, ".") { + return false + } + return tests || !strings.HasSuffix(name, "_test.go") +} + +// packageImports returns the imports of the Go files of one package. It +// reads the cache first for an immutable source. +func (b *builder) packageImports(ref pkgRef) ([]importRef, error) { + if imps, ok, err := b.cached(ref); err != nil || ok { + return imps, err + } + entries, err := fs.ReadDir(ref.mod.src.FS, ref.dir) + if err != nil { + return nil, fmt.Errorf("read the directory %s: %w", ref.dir, err) + } + var imps []importRef + for _, e := range entries { + if !e.Type().IsRegular() || !countedFile(e.Name(), ref.mod.main) { + continue + } + if b.stats.Files >= b.maxFiles { + return nil, errBudget + } + b.stats.Files++ + file := path.Join(ref.dir, e.Name()) + fileImps, err := b.fileImports(ref.mod.src.FS, file) + if err != nil { + return nil, err + } + imps = append(imps, fileImps...) + } + b.stats.Packages++ + if err := b.cache(ref, imps); err != nil { + return nil, err + } + return imps, nil +} + +// fileImports returns the imports of one Go file, or none when its build +// constraint can never hold. +func (b *builder) fileImports(fsys fs.FS, file string) ([]importRef, error) { + f, err := b.parse(fsys, file) + if err != nil { + return nil, err + } + if excluded(f) { + return nil, nil + } + test := strings.HasSuffix(file, "_test.go") + out := make([]importRef, 0, len(f.Imports)) + for _, spec := range f.Imports { + p, err := strconv.Unquote(spec.Path.Value) + if err != nil { + return nil, fmt.Errorf("%s: import %s: %w", file, spec.Path.Value, err) + } + out = append(out, importRef{Path: p, File: file, Line: b.fset.Position(spec.Pos()).Line, Test: test}) + } + return out, nil +} + +// parse reads the package clause, the comments before it and the import +// block of a Go file. It parses the head of the file first, and the whole +// file when the head does not parse. +func (b *builder) parse(fsys fs.FS, file string) (_ *ast.File, err error) { + fh, err := fsys.Open(file) + if err != nil { + return nil, fmt.Errorf("open %s: %w", file, err) + } + defer func() { err = errors.Join(err, fh.Close()) }() + head := make([]byte, headSize) + n, err := io.ReadFull(fh, head) + if err != nil && !errors.Is(err, io.ErrUnexpectedEOF) && !errors.Is(err, io.EOF) { + return nil, fmt.Errorf("read %s: %w", file, err) + } + whole := n < headSize + src := head[:n] + f, perr := parser.ParseFile(b.fset, file, src, parser.ImportsOnly|parser.ParseComments) + if perr == nil || whole { + if perr != nil { + return nil, fmt.Errorf("%s does not parse: %w", file, perr) + } + return f, nil + } + rest, err := io.ReadAll(io.LimitReader(fh, maxFileSize-headSize+1)) + if err != nil { + return nil, fmt.Errorf("read %s: %w", file, err) + } + if len(src)+len(rest) > maxFileSize { + return nil, fmt.Errorf("%s is larger than %d bytes and its head does not parse", file, maxFileSize) + } + f, perr = parser.ParseFile(b.fset, file, append(src, rest...), parser.ImportsOnly|parser.ParseComments) + if perr != nil { + return nil, fmt.Errorf("%s does not parse: %w", file, perr) + } + return f, nil +} + +// excluded reports whether no build reads a file: its build constraint can +// never hold, with each tag free except ignore, which no build sets. A +// //go:build line decides when the file has one. Else each // +build line +// must hold, as the go command reads them. The go command reads a +// constraint only in the comments before the blank line that ends them, so +// a line of the doc comment of the package does not count. A constraint that +// does not parse, or that has more than maxConstraintTags tags, leaves the +// file in, so the graph loses no edge. +func excluded(f *ast.File) bool { + var plus []constraint.Expr + for _, group := range f.Comments { + if group.End() >= f.Package { + break + } + if group == f.Doc { + continue + } + for _, c := range group.List { + switch { + case constraint.IsGoBuild(c.Text): + expr, err := constraint.Parse(c.Text) + return err == nil && !satisfiable(expr) + case constraint.IsPlusBuild(c.Text): + expr, err := constraint.Parse(c.Text) + if err != nil { + return false + } + plus = append(plus, expr) + } + } + } + if len(plus) == 0 { + return false + } + all := plus[0] + for _, e := range plus[1:] { + all = &constraint.AndExpr{X: all, Y: e} + } + return !satisfiable(all) +} + +// satisfiable reports whether some set of tags makes a constraint hold, +// with ignore false. +func satisfiable(expr constraint.Expr) bool { + var tags []string + seen := map[string]bool{} + collectTags(expr, func(tag string) { + if tag != "ignore" && !seen[tag] { + seen[tag] = true + tags = append(tags, tag) + } + }) + if len(tags) > maxConstraintTags { + return true + } + for set := 0; set < 1< 0 { + p := queue[0] + queue = queue[1:] + if p == to && p != from { + return true + } + if n, ok := g.Node(p); ok && n.Open() { + return true + } + for _, q := range next[p] { + if q == to { + return true + } + if !seen[q] { + seen[q] = true + queue = append(queue, q) + } + } + } + return false +} + +// Build reads the module graph of the main module in MainFS. It returns +// ErrUnpruned for a main go.mod before module graph pruning. +func Build(ctx context.Context, in Input) (*Graph, error) { + if in.MainFS == nil { + return nil, errors.New("modgraph: no main module") + } + data, err := fs.ReadFile(in.MainFS, "go.mod") + if err != nil { + return nil, fmt.Errorf("modgraph: read the go.mod of the main module: %w", err) + } + mf, err := modfile.ParseLax("go.mod", data, nil) + if err != nil { + return nil, fmt.Errorf("modgraph: parse the go.mod of the main module: %w", err) + } + if mf.Module == nil || mf.Module.Mod.Path == "" { + return nil, errors.New("modgraph: the go.mod of the main module has no module directive") + } + if mf.Go == nil || version.Compare("go"+mf.Go.Version, "go1.17") < 0 { + return nil, ErrUnpruned + } + b, err := newBuilder(ctx, in, mf.Module.Mod.Path) + if err != nil { + return nil, err + } + return b.run() +} + +// module is the state of one node during a build. +type module struct { + src Source + main bool + node Node + // walked is true when Build read a package of the module, so its edges + // are those of the packages that the build reaches. + walked bool +} + +type pkgRef struct { + mod *module + dir string +} + +type edgeKey struct{ from, to string } + +type builder struct { + ctx context.Context + in Input + maxFiles int + fset *token.FileSet + + main *module + std *module + byPath map[string]*module + // paths holds the module paths of the build list and of the main module, + // longest first, for the resolver. + paths []string + + queue []pkgRef + seen map[pkgRef]bool + edges map[edgeKey]*Edge + stats Stats +} + +func newBuilder(ctx context.Context, in Input, mainPath string) (*builder, error) { + b := &builder{ + ctx: ctx, + in: in, + maxFiles: in.MaxFiles, + fset: token.NewFileSet(), + byPath: map[string]*module{}, + seen: map[pkgRef]bool{}, + edges: map[edgeKey]*Edge{}, + } + if b.maxFiles <= 0 { + b.maxFiles = defaultMaxFiles + } + b.main = &module{src: Source{Module: Module{Path: mainPath}, FS: in.MainFS}, main: true, node: Node{Module: Module{Path: mainPath}, State: StateImports}} + b.byPath[mainPath] = b.main + for _, s := range in.Modules { + p := s.Module.Path + switch { + case p == "": + return nil, errors.New("modgraph: a module of the build list has no path") + case p == StdPath: + return nil, fmt.Errorf("modgraph: the build list holds the reserved path %q", StdPath) + case b.byPath[p] != nil: + return nil, fmt.Errorf("modgraph: the build list holds %s twice", p) + } + m := &module{src: s, node: Node{Module: s.Module}} + switch { + case s.FS != nil: + m.node.State = StateImports + case s.GoMod != nil: + m.node.State, m.node.Reason = StateRequires, "no source, so only the require lines of its go.mod give edges" + default: + m.node.State, m.node.Reason = StateOpen, "no source and no go.mod" + } + b.byPath[p] = m + } + for p := range b.byPath { + b.paths = append(b.paths, p) + } + slices.SortFunc(b.paths, func(x, y string) int { + if len(x) != len(y) { + return len(y) - len(x) + } + return strings.Compare(x, y) + }) + return b, nil +} + +func (b *builder) run() (*Graph, error) { + if err := b.walkMain(); err != nil { + return nil, err + } + for len(b.queue) > 0 { + if err := b.ctx.Err(); err != nil { + return nil, err + } + ref := b.queue[0] + b.queue = b.queue[1:] + if err := b.walkPackage(ref); err != nil { + return nil, err + } + } + b.finish() + return b.graph(), nil +} + +// walkMain reads each package of the main module, with its test files. +func (b *builder) walkMain() error { + b.main.walked = true + return fs.WalkDir(b.in.MainFS, ".", func(dir string, d fs.DirEntry, err error) error { + if err != nil { + b.setOpen(b.main, fmt.Sprintf("read %s: %v", dir, err)) + return fs.SkipDir + } + if !d.IsDir() { + return nil + } + if dir != "." && (skippedDir(d.Name()) || nestedModule(b.in.MainFS, dir)) { + return fs.SkipDir + } + if err := b.ctx.Err(); err != nil { + return err + } + return b.readAndLink(pkgRef{mod: b.main, dir: dir}) + }) +} + +func (b *builder) walkPackage(ref pkgRef) error { + if ref.mod.node.State == StateOpen { + return nil + } + ref.mod.walked = true + return b.readAndLink(ref) +} + +func (b *builder) readAndLink(ref pkgRef) error { + imps, err := b.packageImports(ref) + switch { + case errors.Is(err, errBudget): + b.stats.Truncated = true + b.setOpen(ref.mod, "the file budget ran out") + return nil + case errors.Is(err, errCache): + return err + case err != nil && b.ctx.Err() != nil: + return b.ctx.Err() + case err != nil: + b.setOpen(ref.mod, err.Error()) + return nil + } + for _, imp := range imps { + b.link(ref.mod, imp) + } + return nil +} + +// link adds the edges of one import of a module, and queues the package +// that it names. +func (b *builder) link(from *module, imp importRef) { + switch { + case imp.Path == "C": + return + case imp.Path == "" || strings.HasPrefix(imp.Path, ".") || strings.HasPrefix(imp.Path, "/"): + b.setOpen(from, fmt.Sprintf("%s imports the path %q, which the build cannot resolve", imp.File, imp.Path)) + return + } + cands := b.resolve(imp.Path) + if len(cands) == 0 { + if isStdPath(imp.Path) && !b.prefixed(imp.Path) { + b.addEdge(from, b.stdNode(), imp, StateImports) + return + } + b.setOpen(from, fmt.Sprintf("%s imports %s, which no module of the build list holds", imp.File, imp.Path)) + return + } + for _, c := range cands { + if c.mod != from { + b.addEdge(from, c.mod, imp, StateImports) + } + if c.provides { + b.enqueue(pkgRef{mod: c.mod, dir: c.dir}) + } + } +} + +func (b *builder) enqueue(ref pkgRef) { + if ref.mod.main || b.seen[ref] { + return + } + b.seen[ref] = true + b.queue = append(b.queue, ref) +} + +func (b *builder) addEdge(from, to *module, imp importRef, kind State) { + k := edgeKey{from.node.Module.Path, to.node.Module.Path} + if e, ok := b.edges[k]; ok { + if !imp.Test { + e.TestOnly = false + } + return + } + b.edges[k] = &Edge{ + From: from.node.Module, To: to.node.Module, Kind: kind, + TestOnly: from.main && imp.Test, + Evidence: Evidence{File: imp.File, Line: imp.Line}, + } +} + +func (b *builder) stdNode() *module { + if b.std == nil { + b.std = &module{node: Node{Module: Module{Path: StdPath}, State: StateImports}, walked: true} + b.byPath[StdPath] = b.std + } + return b.std +} + +func (b *builder) setOpen(m *module, reason string) { + if m.node.State == StateOpen { + return + } + m.node.State, m.node.Reason = StateOpen, reason +} + +// finish adds the require edges of each reached module with no source, then +// sets Reached. Once a reached node is open, each node counts as reached, +// and a module that no walk read becomes open, because its edges are not +// known. +func (b *builder) finish() { + expanded := map[*module]bool{} + for { + reached := b.reachable() + added := false + for _, p := range sortedKeys(b.byPath) { + m := b.byPath[p] + if reached[m] && m.node.State == StateRequires && !expanded[m] { + expanded[m] = true + b.addRequires(m) + added = true + } + } + if !added { + anyOpen := false + for m := range reached { + anyOpen = anyOpen || m.node.Open() + } + for _, m := range b.byPath { + m.node.Reached = reached[m] || anyOpen + if anyOpen && !m.walked && m.node.State == StateImports { + b.setOpen(m, "only an open node reaches it, so the build read none of its packages") + } + } + return + } + } +} + +// addRequires adds an edge from a module with no source to each module of +// the build list that its go.mod requires. +func (b *builder) addRequires(m *module) { + f, err := modfile.ParseLax("go.mod", m.src.GoMod, nil) + if err != nil { + b.setOpen(m, fmt.Sprintf("its go.mod does not parse: %v", err)) + return + } + for _, r := range f.Require { + to := b.byPath[r.Mod.Path] + if to == nil || to == m || to.main { + continue + } + line := 0 + if r.Syntax != nil { + line = r.Syntax.Start.Line + } + b.addEdge(m, to, importRef{File: "go.mod", Line: line}, StateRequires) + } +} + +// reachable returns the modules that the edges reach from the main module. +func (b *builder) reachable() map[*module]bool { + next := map[string][]string{} + for k := range b.edges { + next[k.from] = append(next[k.from], k.to) + } + seen := map[*module]bool{b.main: true} + queue := []*module{b.main} + for len(queue) > 0 { + m := queue[0] + queue = queue[1:] + for _, p := range next[m.node.Module.Path] { + if to := b.byPath[p]; to != nil && !seen[to] { + seen[to] = true + queue = append(queue, to) + } + } + } + return seen +} + +func (b *builder) graph() *Graph { + g := &Graph{Version: Version, Main: b.main.node.Module, Stats: b.stats} + for _, p := range sortedKeys(b.byPath) { + g.Nodes = append(g.Nodes, b.byPath[p].node) + } + for _, e := range b.edges { + g.Edges = append(g.Edges, *e) + } + slices.SortFunc(g.Edges, func(x, y Edge) int { + if c := strings.Compare(x.From.Path, y.From.Path); c != 0 { + return c + } + return strings.Compare(x.To.Path, y.To.Path) + }) + return g +} + +func sortedKeys[V any](m map[string]V) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + slices.Sort(keys) + return keys +} diff --git a/modgraph/graph_test.go b/modgraph/graph_test.go new file mode 100644 index 00000000..e3556574 --- /dev/null +++ b/modgraph/graph_test.go @@ -0,0 +1,474 @@ +package modgraph + +import ( + "context" + "fmt" + "slices" + "strings" + "testing" + "testing/fstest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/safedep/code/fileindex/store" + "github.com/safedep/code/fileindex/store/memstore" +) + +const mainPath = "example.com/app" + +// goSrc returns a Go file of a package with the imports. +func goSrc(pkg string, imports ...string) *fstest.MapFile { + var b strings.Builder + fmt.Fprintf(&b, "package %s\n\n", pkg) + for _, imp := range imports { + fmt.Fprintf(&b, "import _ %q\n", imp) + } + return &fstest.MapFile{Data: []byte(b.String())} +} + +func mainFS(files fstest.MapFS) fstest.MapFS { + return withGoMod(files, "module "+mainPath+"\n\ngo 1.22\n") +} + +func withGoMod(files fstest.MapFS, gomod string) fstest.MapFS { + out := fstest.MapFS{"go.mod": {Data: []byte(gomod)}} + for k, v := range files { + out[k] = v + } + return out +} + +func source(path string, files fstest.MapFS) Source { + return Source{Module: Module{Path: path, Version: "v1.0.0"}, FS: files} +} + +func build(t *testing.T, in Input) *Graph { + t.Helper() + g, err := Build(context.Background(), in) + require.NoError(t, err) + return g +} + +// edgeList writes each edge as "from -> to" with a mark for a test-only +// edge or a require edge. +func edgeList(g *Graph) []string { + var out []string + for _, e := range g.Edges { + s := e.From.Path + " -> " + e.To.Path + if e.TestOnly { + s += " (test)" + } + if e.Kind == StateRequires { + s += " (requires)" + } + out = append(out, s) + } + return out +} + +func node(t *testing.T, g *Graph, path string) Node { + t.Helper() + n, ok := g.Node(path) + require.True(t, ok, "no node %s", path) + return n +} + +func TestBuildFollowsTheImportsOfTheReachedPackages(t *testing.T) { + g := build(t, Input{ + MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "fmt", "example.com/a/x")}), + Modules: []Source{ + source("example.com/a", fstest.MapFS{ + "x/x.go": goSrc("x", "example.com/b"), + "y/y.go": goSrc("y", "example.com/c"), + }), + source("example.com/b", fstest.MapFS{"b.go": goSrc("b", "net/http")}), + source("example.com/c", fstest.MapFS{"c.go": goSrc("c")}), + }, + }) + + assert.Equal(t, []string{ + "example.com/a -> example.com/b", + "example.com/app -> example.com/a", + "example.com/app -> std", + "example.com/b -> std", + }, edgeList(g), "the package y of a is not reached, so its import of c gives no edge") + assert.True(t, node(t, g, "example.com/b").Reached) + assert.False(t, node(t, g, "example.com/c").Reached) + assert.Equal(t, StateImports, node(t, g, "example.com/c").State) + assert.Equal(t, Evidence{File: "x/x.go", Line: 3}, g.Edges[0].Evidence) + assert.Equal(t, Version, g.Version) + assert.Equal(t, Module{Path: mainPath}, g.Main) +} + +func TestBuildReadsEachPackageOfTheMainModule(t *testing.T) { + g := build(t, Input{ + MainFS: mainFS(fstest.MapFS{ + "main.go": goSrc("main"), + "third_party/forked/y/wrap.go": goSrc("y", "example.com/y"), + "vendor/example.com/z/z.go": goSrc("z", "example.com/z"), + "testdata/t.go": goSrc("t", "example.com/z"), + "_old/o.go": goSrc("o", "example.com/z"), + ".hidden/h.go": goSrc("h", "example.com/z"), + "_skip.go": goSrc("main", "example.com/z"), + "tools/go.mod": {Data: []byte("module example.com/app/tools\n\ngo 1.22\n")}, + "tools/tools.go": goSrc("tools", "example.com/z"), + }), + Modules: []Source{ + source("example.com/y", fstest.MapFS{"y.go": goSrc("y")}), + source("example.com/z", fstest.MapFS{"z.go": goSrc("z")}), + }, + }) + + assert.Equal(t, []string{"example.com/app -> example.com/y"}, edgeList(g)) + assert.False(t, node(t, g, "example.com/z").Reached) +} + +func TestBuildMarksTheTestOnlyEdgesOfTheMainModule(t *testing.T) { + g := build(t, Input{ + MainFS: mainFS(fstest.MapFS{ + "a.go": goSrc("app", "example.com/both"), + "a_test.go": goSrc("app", "example.com/test", "example.com/both"), + }), + Modules: []Source{ + source("example.com/both", fstest.MapFS{"b.go": goSrc("both")}), + source("example.com/test", fstest.MapFS{"t.go": goSrc("test"), "t_test.go": goSrc("test", "example.com/onlytests")}), + source("example.com/onlytests", fstest.MapFS{"o.go": goSrc("onlytests")}), + }, + }) + + assert.Equal(t, []string{ + "example.com/app -> example.com/both", + "example.com/app -> example.com/test (test)", + }, edgeList(g), "the test files of a dependency never count") + assert.True(t, node(t, g, "example.com/test").Reached) + assert.False(t, node(t, g, "example.com/onlytests").Reached) +} + +func TestBuildResolvesAnImportPath(t *testing.T) { + cases := []struct { + name string + imp string + modules []Source + want []string + }{ + { + name: "the longest module path that has the directory", + imp: "github.com/a/b/c/d", + modules: []Source{ + source("github.com/a/b", fstest.MapFS{"c/d/d.go": goSrc("d")}), + source("github.com/a/b/c", fstest.MapFS{"c.go": goSrc("c")}), + }, + want: []string{"example.com/app -> github.com/a/b"}, + }, + { + name: "a longer module with no source may hold the package", + imp: "github.com/a/b/c/d", + modules: []Source{ + source("github.com/a/b", fstest.MapFS{"c/d/d.go": goSrc("d")}), + {Module: Module{Path: "github.com/a/b/c", Version: "v0.1.0"}}, + }, + want: []string{"example.com/app -> github.com/a/b", "example.com/app -> github.com/a/b/c"}, + }, + { + name: "a module path with no dot is not the standard library", + imp: "corp/lib/x", + modules: []Source{ + source("corp/lib", fstest.MapFS{"x/x.go": goSrc("x")}), + }, + want: []string{"example.com/app -> corp/lib"}, + }, + { + name: "a module root", + imp: "gopkg.in/yaml.v3", + modules: []Source{ + source("gopkg.in/yaml.v3", fstest.MapFS{"yaml.go": goSrc("yaml")}), + }, + want: []string{"example.com/app -> gopkg.in/yaml.v3"}, + }, + { + name: "a nested module of a dependency is not its package", + imp: "github.com/a/b/sub", + modules: []Source{ + source("github.com/a/b", fstest.MapFS{"sub/go.mod": {Data: []byte("module github.com/a/b/sub\n")}, "sub/s.go": goSrc("sub")}), + source("github.com/a/b/sub", fstest.MapFS{"s.go": goSrc("sub")}), + }, + want: []string{"example.com/app -> github.com/a/b/sub"}, + }, + { + name: "cgo", + imp: "C", + modules: nil, + want: nil, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + g := build(t, Input{MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", tc.imp)}), Modules: tc.modules}) + assert.Equal(t, tc.want, edgeList(g)) + assert.Equal(t, StateImports, node(t, g, mainPath).State) + }) + } +} + +func TestBuildOpensAModuleThatImportsWhatItCannotResolve(t *testing.T) { + cases := []struct { + name string + imp string + }{ + {"a path of no module", "example.com/missing/x"}, + {"a package directory that its module does not have", "example.com/a/missing"}, + {"a relative path", "./local"}, + {"a directory of a module nested in the dependency", "example.com/a/sub"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + g := build(t, Input{ + MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a")}), + Modules: []Source{ + source("example.com/a", fstest.MapFS{ + "a.go": goSrc("a", tc.imp), + "sub/go.mod": {Data: []byte("module example.com/a/sub\n")}, + "sub/s.go": goSrc("sub"), + }), + source("example.com/unused", fstest.MapFS{"u.go": goSrc("unused")}), + }, + }) + a := node(t, g, "example.com/a") + assert.Equal(t, StateOpen, a.State) + assert.Contains(t, a.Reason, tc.imp) + unused := node(t, g, "example.com/unused") + assert.True(t, unused.Reached, "an open node can reach each module") + assert.Equal(t, StateOpen, unused.State, "no walk read its packages") + assert.True(t, g.Reaches(mainPath, "example.com/unused")) + }) + } +} + +func TestBuildReadsTheGoModOfAModuleWithNoSource(t *testing.T) { + g := build(t, Input{ + MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a")}), + Modules: []Source{ + {Module: Module{Path: "example.com/a", Version: "v1.0.0"}, GoMod: []byte("module example.com/a\n\ngo 1.21\n\nrequire (\n\texample.com/b v1.0.0\n\texample.com/outside v1.0.0\n)\n")}, + source("example.com/b", fstest.MapFS{"b.go": goSrc("b")}), + source("example.com/c", fstest.MapFS{"c.go": goSrc("c")}), + }, + }) + + assert.Equal(t, []string{ + "example.com/a -> example.com/b (requires)", + "example.com/app -> example.com/a", + }, edgeList(g), "a require of a module outside the build list gives no edge") + a := node(t, g, "example.com/a") + assert.Equal(t, StateRequires, a.State) + assert.True(t, a.Open(), "go.mod need not list each module that the code imports") + assert.True(t, node(t, g, "example.com/c").Reached) + assert.Equal(t, Evidence{File: "go.mod", Line: 6}, g.Edges[0].Evidence) +} + +func TestBuildOpensAModuleWithNoSourceAndNoGoMod(t *testing.T) { + g := build(t, Input{ + MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a")}), + Modules: []Source{{Module: Module{Path: "example.com/a", Version: "v1.0.0"}}}, + }) + a := node(t, g, "example.com/a") + assert.Equal(t, StateOpen, a.State) + assert.True(t, a.Reached) +} + +func TestBuildKeepsAFileUnlessItsConstraintCanNeverHold(t *testing.T) { + cases := []struct { + name, file, src string + counts bool + }{ + {"no constraint", "a.go", "package a\n\nimport _ \"example.com/dep\"\n", true}, + {"ignore", "a.go", "//go:build ignore\n\npackage a\n\nimport _ \"example.com/dep\"\n", false}, + {"ignore with a platform", "a.go", "//go:build ignore && linux\n\npackage a\n\nimport _ \"example.com/dep\"\n", false}, + {"a tag that the go command never sets", "a.go", "//go:build purego\n\npackage a\n\nimport _ \"example.com/dep\"\n", true}, + {"a contradiction", "a.go", "//go:build linux && !linux\n\npackage a\n\nimport _ \"example.com/dep\"\n", false}, + {"a file name suffix never drops a file", "a_windows.go", "//go:build !windows\n\npackage a\n\nimport _ \"example.com/dep\"\n", true}, + {"a +build line", "a.go", "// +build ignore\n\npackage a\n\nimport _ \"example.com/dep\"\n", false}, + {"a +build line with no space", "a.go", "//+build ignore\n\npackage a\n\nimport _ \"example.com/dep\"\n", false}, + {"+build lines that each hold", "a.go", "// +build linux darwin\n// +build amd64\n\npackage a\n\nimport _ \"example.com/dep\"\n", true}, + {"+build lines that cannot hold together", "a.go", "// +build linux\n// +build !linux\n\npackage a\n\nimport _ \"example.com/dep\"\n", false}, + {"a //go:build line wins over a +build line", "a.go", "//go:build linux\n// +build ignore\n\npackage a\n\nimport _ \"example.com/dep\"\n", true}, + {"a +build line of the package doc comment", "a.go", "// +build ignore\npackage a\n\nimport _ \"example.com/dep\"\n", true}, + {"a line of the package doc comment", "a.go", "//go:build ignore\npackage a\n\nimport _ \"example.com/dep\"\n", true}, + {"a constraint that does not parse", "a.go", "//go:build linux &&\n\npackage a\n\nimport _ \"example.com/dep\"\n", true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + g := build(t, Input{ + MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a")}), + Modules: []Source{ + source("example.com/a", fstest.MapFS{tc.file: {Data: []byte(tc.src)}}), + source("example.com/dep", fstest.MapFS{"d.go": goSrc("dep")}), + }, + }) + assert.Equal(t, tc.counts, slices.Contains(edgeList(g), "example.com/a -> example.com/dep")) + }) + } +} + +func TestSatisfiableCountsAConstraintWithManyTags(t *testing.T) { + g := build(t, Input{ + MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a")}), + Modules: []Source{ + source("example.com/a", fstest.MapFS{"a.go": {Data: []byte("//go:build ignore && (t1 || t2 || t3 || t4 || t5 || t6 || t7 || t8 || t9 || t10 || t11 || t12 || t13 || t14 || t15 || t16 || t17)\n\npackage a\n\nimport _ \"example.com/dep\"\n")}}), + source("example.com/dep", fstest.MapFS{"d.go": goSrc("dep")}), + }, + }) + assert.Contains(t, edgeList(g), "example.com/a -> example.com/dep", "past the tag cap the file counts") +} + +func TestBuildReadsAnImportBlockPastTheHead(t *testing.T) { + long := "// " + strings.Repeat("x", headSize) + "\n\npackage a\n\nimport _ \"example.com/dep\"\n" + g := build(t, Input{ + MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a")}), + Modules: []Source{ + source("example.com/a", fstest.MapFS{"a.go": {Data: []byte(long)}}), + source("example.com/dep", fstest.MapFS{"d.go": goSrc("dep")}), + }, + }) + assert.Contains(t, edgeList(g), "example.com/a -> example.com/dep") + assert.Equal(t, StateImports, node(t, g, "example.com/a").State) +} + +func TestBuildOpensAModuleWithAFileThatDoesNotParse(t *testing.T) { + g := build(t, Input{ + MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a")}), + Modules: []Source{ + source("example.com/a", fstest.MapFS{"a.go": {Data: []byte("package a\n\nimport (\n")}}), + }, + }) + a := node(t, g, "example.com/a") + assert.Equal(t, StateOpen, a.State) + assert.Contains(t, a.Reason, "does not parse") +} + +func TestBuildNeedsAPrunedGoMod(t *testing.T) { + cases := []struct { + name, gomod string + err error + }{ + {"go 1.16", "module " + mainPath + "\n\ngo 1.16\n", ErrUnpruned}, + {"no go directive", "module " + mainPath + "\n", ErrUnpruned}, + {"go 1.17", "module " + mainPath + "\n\ngo 1.17\n", nil}, + {"go 1.25.1", "module " + mainPath + "\n\ngo 1.25.1\n", nil}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := Build(context.Background(), Input{MainFS: withGoMod(fstest.MapFS{"main.go": goSrc("main")}, tc.gomod)}) + if tc.err == nil { + assert.NoError(t, err) + } else { + assert.ErrorIs(t, err, tc.err) + } + }) + } +} + +func TestBuildRejectsABadInput(t *testing.T) { + cases := []struct { + name string + in Input + msg string + }{ + {"no main module", Input{}, "no main module"}, + {"no go.mod", Input{MainFS: fstest.MapFS{"main.go": goSrc("main")}}, "read the go.mod"}, + {"no module directive", Input{MainFS: withGoMod(nil, "go 1.22\n")}, "no module directive"}, + {"a module twice", Input{MainFS: mainFS(nil), Modules: []Source{source("example.com/a", nil), source("example.com/a", nil)}}, "twice"}, + {"the reserved std path", Input{MainFS: mainFS(nil), Modules: []Source{source(StdPath, nil)}}, "reserved"}, + {"a module with no path", Input{MainFS: mainFS(nil), Modules: []Source{{}}}, "no path"}, + {"the main module in the build list", Input{MainFS: mainFS(nil), Modules: []Source{source(mainPath, nil)}}, "twice"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := Build(context.Background(), tc.in) + require.Error(t, err) + assert.Contains(t, err.Error(), tc.msg) + }) + } +} + +func TestBuildOpensAModulePastTheFileBudget(t *testing.T) { + g := build(t, Input{ + MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a")}), + Modules: []Source{ + source("example.com/a", fstest.MapFS{"a.go": goSrc("a")}), + }, + MaxFiles: 1, + }) + assert.True(t, g.Stats.Truncated) + assert.Equal(t, StateOpen, node(t, g, "example.com/a").State) + assert.Equal(t, 1, g.Stats.Files) +} + +func TestBuildDoesNotDependOnTheOrderOfTheBuildList(t *testing.T) { + mods := []Source{ + source("example.com/a", fstest.MapFS{"a.go": goSrc("a", "example.com/b", "example.com/c")}), + source("example.com/b", fstest.MapFS{"b.go": goSrc("b", "example.com/c")}), + source("example.com/c", fstest.MapFS{"c.go": goSrc("c")}), + } + in := Input{MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a", "example.com/b")}), Modules: mods} + want := build(t, in) + slices.Reverse(in.Modules) + assert.Equal(t, want, build(t, in)) +} + +func TestBuildCachesThePackagesOfAnImmutableSource(t *testing.T) { + ctx := context.Background() + st := memstore.New() + immutable := func(s Source) Source { s.Immutable = true; return s } + in := Input{ + MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a", "example.com/v")}), + Modules: []Source{ + immutable(source("example.com/a", fstest.MapFS{"a.go": goSrc("a", "example.com/b")})), + immutable(source("example.com/b", fstest.MapFS{"b.go": goSrc("b")})), + source("example.com/v", fstest.MapFS{"v.go": goSrc("v")}), + }, + Store: st, + } + first := build(t, in) + assert.Equal(t, 4, first.Stats.Files) + assert.Equal(t, 0, first.Stats.CachedPackages) + + second := build(t, in) + assert.Equal(t, 2, second.Stats.Files, "the main module and the mutable source are read again") + assert.Equal(t, 2, second.Stats.CachedPackages) + assert.Equal(t, first.Edges, second.Edges) + + in.Modules[0].Sum = "h1:other" + third := build(t, in) + assert.Equal(t, 1, third.Stats.CachedPackages, "another go.sum hash is another entry") + + found, err := st.GetArtifacts(ctx, []store.ArtifactKey{cacheKey(pkgRef{mod: &module{src: in.Modules[2]}, dir: "."})}) + require.NoError(t, err) + assert.Empty(t, found, "a mutable source has no entry") +} + +func TestBuildCachesNoPackageThatFailed(t *testing.T) { + st := memstore.New() + bad := Source{Module: Module{Path: "example.com/a", Version: "v1.0.0"}, FS: fstest.MapFS{"a.go": {Data: []byte("package a\nimport (\n")}}, Immutable: true} + in := Input{MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a")}), Modules: []Source{bad}, Store: st} + build(t, in) + g := build(t, in) + assert.Equal(t, 0, g.Stats.CachedPackages) + assert.Equal(t, StateOpen, node(t, g, "example.com/a").State) +} + +func TestGraphReaches(t *testing.T) { + g := build(t, Input{ + MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a")}), + Modules: []Source{ + source("example.com/a", fstest.MapFS{"a.go": goSrc("a", "example.com/b")}), + source("example.com/b", fstest.MapFS{"b.go": goSrc("b")}), + source("example.com/c", fstest.MapFS{"c.go": goSrc("c", "example.com/b")}), + }, + }) + assert.True(t, g.Reaches(mainPath, "example.com/b")) + assert.True(t, g.Reaches("example.com/a", "example.com/b")) + assert.False(t, g.Reaches("example.com/b", "example.com/a")) + assert.False(t, g.Reaches(mainPath, "example.com/c")) + assert.False(t, g.Reaches("example.com/c", "example.com/b"), "the build reads no package of a module that nothing reaches") +} diff --git a/modgraph/oracle_test.go b/modgraph/oracle_test.go new file mode 100644 index 00000000..2abdc683 --- /dev/null +++ b/modgraph/oracle_test.go @@ -0,0 +1,180 @@ +package modgraph + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "io" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/safedep/code/fileindex/store/memstore" +) + +// The oracle test compares the graph with the module edges of go list. Each +// edge that go list gives must be in the graph, unless the node that it +// leaves is open, and the node that it reaches must be reached. go list +// runs for each platform pair below, with the test packages of the main +// module. The test runs on this repository. MODGRAPH_ORACLE_DIRS adds more +// projects, as a list of directories that filepath.SplitList reads. A +// project with vendor/modules.txt reads its dependencies from vendor. The +// test needs the go command and a module cache that holds the build list, +// and skips with neither. It never downloads: GOPROXY is off. + +var oraclePlatforms = [][2]string{ + {"linux", "amd64"}, + {"darwin", "arm64"}, + {"windows", "amd64"}, + {"js", "wasm"}, +} + +func TestBuildHoldsEachEdgeOfGoList(t *testing.T) { + if testing.Short() { + t.Skip("the oracle test runs the go command") + } + if _, err := exec.LookPath("go"); err != nil { + t.Skip("no go command") + } + root, err := filepath.Abs("..") + require.NoError(t, err) + dirs := []string{root} + for _, d := range filepath.SplitList(os.Getenv("MODGRAPH_ORACLE_DIRS")) { + if d != "" { + dirs = append(dirs, d) + } + } + for _, dir := range dirs { + t.Run(filepath.Base(dir), func(t *testing.T) { + sources, ok := oracleSources(t, dir) + if !ok { + t.Skip("the module cache does not hold the build list") + } + st := memstore.New() + start := time.Now() + g, err := Build(context.Background(), Input{MainFS: os.DirFS(dir), Modules: sources, Store: st}) + require.NoError(t, err) + cold := time.Since(start) + start = time.Now() + warm, err := Build(context.Background(), Input{MainFS: os.DirFS(dir), Modules: sources, Store: st}) + require.NoError(t, err) + assert.Equal(t, g.Edges, warm.Edges, "the cache gives the same graph") + t.Logf("Build: %d files, %d packages, %v; with the cache: %d files, %d cached packages, %v", + g.Stats.Files, g.Stats.Packages, cold.Round(time.Millisecond), warm.Stats.Files, warm.Stats.CachedPackages, time.Since(start).Round(time.Millisecond)) + for _, n := range g.Nodes { + if n.Open() && n.Reached && !strings.HasPrefix(n.Reason, "only an open node") { + t.Logf("open: %s: %s", n.Module.Path, n.Reason) + } + } + edges := map[[2]string]bool{} + for _, e := range g.Edges { + edges[[2]string{e.From.Path, e.To.Path}] = true + } + for _, p := range oraclePlatforms { + listed := goListEdges(t, dir, p[0], p[1]) + t.Logf("%s/%s: go list has %d module edges, the graph %d, with %d nodes", p[0], p[1], len(listed), len(g.Edges), len(g.Nodes)) + for e := range listed { + from, _ := g.Node(e[0]) + to, ok := g.Node(e[1]) + assert.True(t, ok && to.Reached, "%s/%s: go list reaches %s, the graph does not", p[0], p[1], e[1]) + assert.True(t, edges[e] || from.Open(), "%s/%s: go list has the edge %s -> %s", p[0], p[1], e[0], e[1]) + } + } + + // With no source, each node holds only its go.mod, and each module + // that go list reaches must still be reached. + for i := range sources { + sources[i].FS = nil + } + g, err = Build(context.Background(), Input{MainFS: os.DirFS(dir), Modules: sources}) + require.NoError(t, err) + for e := range goListEdges(t, dir, "linux", "amd64") { + to, ok := g.Node(e[1]) + assert.True(t, ok && to.Reached, "go.mod only: go list reaches %s, the graph does not", e[1]) + } + }) + } +} + +// oracleSources finds the sources with Discover and the module cache of +// the go command. +func oracleSources(t *testing.T, dir string) ([]Source, bool) { + t.Helper() + out, err := goCommand(dir, nil, "env", "GOMODCACHE") + if err != nil { + t.Logf("go env: %v", err) + return nil, false + } + sources, err := Discover(DiscoverOptions{Dir: dir, ModuleCache: strings.TrimSpace(string(out)), LocalRoot: dir}) + require.NoError(t, err) + return sources, true +} + +type listPackage struct { + ImportPath string + Standard bool + Module *struct{ Path string } + Imports []string +} + +// goListEdges returns the module edges of the packages that go list -deps +// -test gives for a platform. +func goListEdges(t *testing.T, dir, goos, goarch string) map[[2]string]bool { + t.Helper() + out, err := goCommand(dir, []string{"GOOS=" + goos, "GOARCH=" + goarch, "CGO_ENABLED=1"}, "list", "-e", "-deps", "-test", "-json", "./...") + require.NoError(t, err, "go list -deps") + var pkgs []listPackage + dec := json.NewDecoder(bytes.NewReader(out)) + for { + var p listPackage + if err := dec.Decode(&p); errors.Is(err, io.EOF) { + break + } else { + require.NoError(t, err) + } + pkgs = append(pkgs, p) + } + moduleOf := map[string]string{} + for _, p := range pkgs { + switch { + case p.Standard: + moduleOf[p.ImportPath] = StdPath + case p.Module != nil: + moduleOf[p.ImportPath] = p.Module.Path + } + } + edges := map[[2]string]bool{} + for _, p := range pkgs { + from, ok := moduleOf[p.ImportPath] + if !ok || from == StdPath { + continue + } + for _, imp := range p.Imports { + if to, ok := moduleOf[imp]; ok && to != from && imp != "C" { + edges[[2]string{from, to}] = true + } + } + } + return edges +} + +func goCommand(dir string, env []string, args ...string) ([]byte, error) { + cmd := exec.Command("go", args...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), "GOPROXY=off", "GOFLAGS=", "GOWORK=off") + cmd.Env = append(cmd.Env, env...) + var stderr strings.Builder + cmd.Stderr = &stderr + out, err := cmd.Output() + if err != nil { + return nil, errors.Join(err, errors.New(stderr.String())) + } + return out, nil +} diff --git a/modgraph/resolve.go b/modgraph/resolve.go new file mode 100644 index 00000000..0e006e63 --- /dev/null +++ b/modgraph/resolve.go @@ -0,0 +1,107 @@ +package modgraph + +import ( + "io/fs" + "path" + "strings" +) + +// candidate is a module that can hold the package of an import path. +type candidate struct { + mod *module + // dir is the directory of the package inside the module, "." for its + // root. + dir string + // provides is true when the source of the module has the package, so + // the walk reads it. + provides bool +} + +// resolve returns the modules that can hold the package of an import path. +// The go command takes the module of the build list whose path is the +// longest prefix of the import path and that has the package directory. A +// module with no source may have the directory, so it is a candidate, and the +// test goes on to the next prefix. The first module with source that has the +// directory ends the list. A list with no module that has or may have the +// directory is empty. +func (b *builder) resolve(importPath string) []candidate { + var out []candidate + for _, p := range b.paths { + dir, ok := under(importPath, p) + if !ok { + continue + } + m := b.byPath[p] + if m.src.FS == nil { + out = append(out, candidate{mod: m, dir: dir}) + continue + } + if hasPackageDir(m.src.FS, dir) { + return append(out, candidate{mod: m, dir: dir, provides: true}) + } + } + return out +} + +// prefixed reports whether a module of the build list or the main module has +// a path that is a prefix of the import path. Such a path is never a package +// of the standard library, even with no dot in its first element, as the +// path corp/lib of a replaced module. +func (b *builder) prefixed(importPath string) bool { + for _, p := range b.paths { + if _, ok := under(importPath, p); ok { + return true + } + } + return false +} + +// under returns the path of a package below a module path, "." for the +// module root, at a "/" boundary. +func under(importPath, modPath string) (string, bool) { + switch { + case importPath == modPath: + return ".", true + case strings.HasPrefix(importPath, modPath+"/"): + return importPath[len(modPath)+1:], true + } + return "", false +} + +// isStdPath reports whether an import path can name a package of the +// standard library: its first element has no dot. +func isStdPath(importPath string) bool { + first, _, _ := strings.Cut(importPath, "/") + return first != "" && !strings.Contains(first, ".") +} + +// hasPackageDir reports whether the source of a module has the directory of +// a package that an importer can build. A directory that the go command +// skips, or that belongs to a module nested in this one, is not a package of +// the module. +func hasPackageDir(fsys fs.FS, dir string) bool { + if dir != "." { + for _, part := range strings.Split(dir, "/") { + if skippedDir(part) { + return false + } + } + if nestedModule(fsys, dir) { + return false + } + } + info, err := fs.Stat(fsys, dir) + return err == nil && info.IsDir() +} + +// nestedModule reports whether a directory below the root of a module, or +// one of its parents below the root, holds a go.mod, so it is a module of its +// own. +func nestedModule(fsys fs.FS, dir string) bool { + for d := dir; d != "."; d = path.Dir(d) { + if _, err := fs.Stat(fsys, path.Join(d, "go.mod")); err == nil { + return true + } + } + return false +} From 772861531b294d59e5cbec044b24401bf47766b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 19:09:42 +0000 Subject: [PATCH 2/5] fix(modgraph): close the missed edges that the code review found A Fable review proved five inputs where Build missed an edge of a real build, and left the node at imports. Each one now has a test. - A link to a .go file did not count. The go command reads it. Build now reads a link to a regular file as a file of its package. - A package of the main module behind a link to a directory was dropped, because the walk of the main module does not follow links and enqueue refused each main package. The walk now marks the packages that it read, and an import queues any other one. - A 64 KiB head that ends inside a comment between two imports, or inside the word import, parsed and was taken as the whole import block. Build now takes a head only when a token that is not import ends inside it after the last import. Else it reads the whole file. - A // +build line after a /* */ comment excluded a file that the go command builds. excluded is now go/build parseFileHeader and shouldBuild. A //go:build line in the doc comment now counts, as it does for the go command. - The cache keyed a package by the build list module, so a fork that replaces a module could read the imports of the original. The key is now Source.Origin, the module that the source holds, and Discover sets it. Other changes from the review: - An import of a testdata, _ or . directory reaches a package, as the go command builds it. Only vendor and a nested module are refused. - Discover reads vendor before a local replacement, as vendor mode does, and resolves links before it checks LocalRoot. - Node.Walked is new. Graph.Path and Graph.Reaches take a set of start modules, treat a module that Build did not walk as open, and share one index. - Stats.Packages counts only directories with a Go file. The evidence of an edge that a test file gave first moves to the import that clears TestOnly. IsStdPath comes from projectgraph/modules. - The oracle test requires the node of each edge of go list. With the head check, a cold Build reads safedep/vet in 1.2 s, as before. The oracle still finds each edge of go list on this repository, safedep/vet and safedep/control-tower. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PFQbqPqd9xn1fT6hBsyoUm Signed-off-by: Claude --- docs/modgraph.md | 55 +++++---- modgraph/cache.go | 14 ++- modgraph/discover.go | 17 ++- modgraph/discover_test.go | 29 +++++ modgraph/files.go | 231 ++++++++++++++++++++++++++------------ modgraph/graph.go | 117 +++++++++++++------ modgraph/graph_test.go | 127 +++++++++++++++++++-- modgraph/oracle_test.go | 3 +- modgraph/resolve.go | 22 +--- 9 files changed, 451 insertions(+), 164 deletions(-) diff --git a/docs/modgraph.md b/docs/modgraph.md index 64f6d9c3..fc95c456 100644 --- a/docs/modgraph.md +++ b/docs/modgraph.md @@ -7,8 +7,10 @@ module, or that no other module can call one. The package reads the code on the disk. It runs no `go` command, opens no network connection and builds with `CGO_ENABLED=0`. It parses each Go file -with `go/parser` in `ImportsOnly` mode, so it reads the build constraint and -the import block of a file and stops. +with `go/parser` in `ImportsOnly` mode. It reads the first 64 KiB of a file, +and reads the whole file when that head does not show the end of the import +block: a token after the last import that is not `import`, and that ends +inside the head. ## The rule @@ -28,9 +30,10 @@ module, and a path that meets it reaches each module. ## The walk -1. `Build` reads each package of the main module, its test files too. It - skips `testdata`, `vendor`, a directory or file whose name starts with - `_` or `.`, and a directory with a `go.mod` of its own. +1. `Build` reads each package of the main module, its test files too, as + `./...` names them. It skips `testdata`, `vendor`, a directory or file + whose name starts with `_` or `.`, and a directory with a `go.mod` of its + own. A link to a file is a file of its package. 2. For each import, the resolver takes the modules of the build list whose path is a prefix of the import path, longest first. A module with source holds the package when it has the directory. A module with no source may @@ -38,13 +41,16 @@ module, and a path that meets it reaches each module. A path that no module holds is the standard library when its first element has no dot. Else its importer is open. 3. The walk reads each package that an import reaches, as `go list -deps` - does. A package of a dependency that no import reaches adds nothing. Test - files of a dependency never count. + does, a `testdata` package or a package behind a link to a directory too. + A package of a dependency that no import reaches adds nothing. Test files + of a dependency never count. 4. A file counts unless its build constraint can never hold, with every tag - free except `ignore`. A `//go:build` line decides, else the `// +build` - lines. A file of `//go:build purego` counts, because a project can build - with `-tags purego`. A GOOS or GOARCH suffix of the file name never drops - a file. + free except `ignore`. `Build` reads the constraint as `go/build` does: a + `//go:build` line in the leading comments decides, else each `// +build` + line before the last blank line of the leading `//` comments. A file of + `//go:build purego` counts, because a project can build with + `-tags purego`. A GOOS or GOARCH suffix of the file name never drops a + file. 5. Once a reached node is open, each node counts as reached, and a module that the walk read no package of becomes open. @@ -65,15 +71,24 @@ g, err := modgraph.Build(ctx, modgraph.Input{ Modules: sources, Store: store, // optional cache of the packages of the module cache }) -g.Reaches(g.Main.Path, "github.com/x/y") +path := g.Path([]string{g.Main.Path}, "github.com/x/y") // nil when no path reaches it ``` -`Discover` finds each module as the go command builds it: the target of a -`replace` directive, then `vendor` when `vendor/modules.txt` lists the -module at its version, then the module cache, then the `.mod` file of the -module cache. A caller that finds the sources in another way can pass its -own `[]Source`. `Module.Path` is the path that import statements use, also -for a replaced module. +`Graph.Path` and `Graph.Reaches` take a set of start modules. A path that +meets an open node, or a module that `Build` did not walk (`Node.Walked` is +false), can reach each module: the path ends at that node. + +`Discover` finds each module as the go command builds it: `vendor` when +`vendor/modules.txt` lists the module at its version, then the local +directory of a `replace` directive, inside `LocalRoot` after links are +resolved, then the module cache, then the `.mod` file of the module cache. +A caller that finds the sources in another way can pass its own +`[]Source`. `Module.Path` is the path that import statements use, also for +a replaced module, and `Source.Origin` is the module that the source holds. + +`Discover` parses the main `go.mod` in full, because a lax parse drops the +`replace` directives. A directive that `golang.org/x/mod` does not know +fails the parse, and the caller keeps no graph. `examples/modgraph` prints the graph of a module. Run `go mod download` first: a module with no source on the disk is open, and one open node that @@ -82,7 +97,9 @@ the build reaches makes each module reached. ## Cache `Input.Store` caches the imports of each package of an immutable source, -the module cache, by module, version, directory and `go.sum` hash. A +the module cache. The key is the module that the source holds +(`Source.Origin` for a replacement), its version, the directory and the +`go.sum` hash. A `vendor` directory and a local replacement can change, so they are read on each run. `Build` stores no entry for a package that it could not read in full. diff --git a/modgraph/cache.go b/modgraph/cache.go index 7edfb8ae..15b58143 100644 --- a/modgraph/cache.go +++ b/modgraph/cache.go @@ -28,7 +28,10 @@ type cacheEntry struct { // hash, when the caller has it, separates two copies of one version with // other bytes. func cacheKey(ref pkgRef) store.ArtifactKey { - m := ref.mod.src.Module + m := ref.mod.src.Origin + if m.Path == "" { + m = ref.mod.src.Module + } unit := fileindex.PackageUnit{Ecosystem: "go", Name: m.Path, Version: m.Version}.String() return store.ArtifactKey{ Key: fileindex.Key{Path: fileindex.UnitPath(unit, ref.dir), ContentHash: ref.mod.src.Sum}, @@ -40,7 +43,14 @@ func cacheKey(ref pkgRef) store.ArtifactKey { // cacheable reports whether the imports of a package can come from, and go // to, the cache. func (b *builder) cacheable(ref pkgRef) bool { - return b.in.Store != nil && !ref.mod.main && ref.mod.src.Immutable && ref.mod.src.Module.Version != "" + if b.in.Store == nil || ref.mod.main || !ref.mod.src.Immutable { + return false + } + origin := ref.mod.src.Origin + if origin.Path == "" { + origin = ref.mod.src.Module + } + return origin.Version != "" } // cached returns the imports of a package from the cache. diff --git a/modgraph/discover.go b/modgraph/discover.go index cd124c25..42854177 100644 --- a/modgraph/discover.go +++ b/modgraph/discover.go @@ -50,11 +50,16 @@ func Discover(opts DiscoverOptions) ([]Source, error) { for _, r := range f.Require { s := Source{Module: Module{Path: r.Mod.Path, Version: r.Mod.Version}} src := replacement(f, r.Mod) + if src != r.Mod { + s.Origin = Module{Path: src.Path, Version: src.Version} + } + // In vendor mode the go command reads each module from vendor, a + // local replacement too. switch { - case src.Version == "": - s.FS, s.GoMod = localSource(opts, src.Path) case vendored[r.Mod.Path] == src: s.FS = os.DirFS(filepath.Join(opts.Dir, "vendor", filepath.FromSlash(r.Mod.Path))) + case src.Version == "": + s.FS, s.GoMod = localSource(opts, src.Path) default: s.FS, s.GoMod = cacheSource(opts.ModuleCache, src) s.Immutable = s.FS != nil @@ -86,7 +91,8 @@ func replacement(f *modfile.File, m gomodule.Version) gomodule.Version { } // localSource returns the source of a replacement with a local path, inside -// LocalRoot only. +// LocalRoot only. Both paths are resolved through links first, so a link +// cannot lead the read out of LocalRoot. func localSource(opts DiscoverOptions, p string) (fs.FS, []byte) { if opts.LocalRoot == "" { return nil, nil @@ -95,12 +101,11 @@ func localSource(opts DiscoverOptions, p string) (fs.FS, []byte) { if !filepath.IsAbs(dir) { dir = filepath.Join(opts.Dir, dir) } - root, err := filepath.Abs(opts.LocalRoot) + root, err := filepath.EvalSymlinks(opts.LocalRoot) if err != nil { return nil, nil } - dir, err = filepath.Abs(dir) - if err != nil { + if dir, err = filepath.EvalSymlinks(dir); err != nil { return nil, nil } if rel, err := filepath.Rel(root, dir); err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { diff --git a/modgraph/discover_test.go b/modgraph/discover_test.go index d00aa216..d88ecebf 100644 --- a/modgraph/discover_test.go +++ b/modgraph/discover_test.go @@ -111,6 +111,8 @@ replace example.com/exact v0.9.0 => example.com/never v1.0.0 forked := sourceOf(t, sources, "example.com/forked") assert.True(t, hasFile(t, forked.FS, "f.go")) assert.Equal(t, "example.com/forked", forked.Module.Path) + assert.Equal(t, Module{Path: "example.com/fork", Version: "v2.0.0"}, forked.Origin, "the cache keys the fork by its own module") + assert.Equal(t, Module{}, cached.Origin) assert.True(t, hasFile(t, sourceOf(t, sources, "example.com/exact").FS, "e.go"), "a replace of another version does not apply") } @@ -127,6 +129,33 @@ func TestDiscoverReadsAVendorDirectoryOnlyAtTheVersionOfGoMod(t *testing.T) { assert.Nil(t, sources[0].FS, "a vendor copy of another version proves nothing") } +func TestDiscoverReadsALocalReplacementFromVendorInVendorMode(t *testing.T) { + root := t.TempDir() + writeFiles(t, root, map[string]string{ + "app/go.mod": "module example.com/app\n\ngo 1.22\n\nrequire example.com/local v1.0.0\n\nreplace example.com/local => ../lib\n", + "app/vendor/modules.txt": "# example.com/local v1.0.0 => ../lib\nexample.com/local\n", + "app/vendor/example.com/local/v.go": "package local\n", + "lib/l.go": "package local\n", + }) + sources, err := Discover(DiscoverOptions{Dir: filepath.Join(root, "app"), LocalRoot: root}) + require.NoError(t, err) + assert.True(t, hasFile(t, sources[0].FS, "v.go")) + assert.False(t, hasFile(t, sources[0].FS, "l.go")) +} + +func TestDiscoverDoesNotFollowALinkOutOfTheLocalRoot(t *testing.T) { + root := t.TempDir() + outside := t.TempDir() + writeFiles(t, root, map[string]string{ + "app/go.mod": "module example.com/app\n\ngo 1.22\n\nrequire example.com/local v1.0.0\n\nreplace example.com/local => ./lib\n", + }) + writeFiles(t, outside, map[string]string{"l.go": "package local\n"}) + require.NoError(t, os.Symlink(outside, filepath.Join(root, "app", "lib"))) + sources, err := Discover(DiscoverOptions{Dir: filepath.Join(root, "app"), LocalRoot: root}) + require.NoError(t, err) + assert.Nil(t, sources[0].FS) +} + func TestDiscoverNeedsAGoMod(t *testing.T) { _, err := Discover(DiscoverOptions{Dir: t.TempDir()}) assert.ErrorContains(t, err, "read go.mod") diff --git a/modgraph/files.go b/modgraph/files.go index 042f15a4..ee3323c6 100644 --- a/modgraph/files.go +++ b/modgraph/files.go @@ -1,11 +1,14 @@ package modgraph import ( + "bytes" "errors" "fmt" "go/ast" "go/build/constraint" "go/parser" + "go/scanner" + "go/token" "io" "io/fs" "path" @@ -18,8 +21,9 @@ const ( // block of a Go file is near its top, so the head almost always holds it. headSize = 64 << 10 // maxFileSize caps a Go file that Build reads whole, when its head does - // not parse. A larger file makes its module open. - maxFileSize = 16 << 20 + // not show the end of the import block. A larger file makes its module + // open. + maxFileSize = 32 << 20 // maxConstraintTags caps the tags of a build constraint that Build tests // one by one. A constraint with more tags counts as one that can hold. maxConstraintTags = 16 @@ -67,22 +71,32 @@ func (b *builder) packageImports(ref pkgRef) ([]importRef, error) { return nil, fmt.Errorf("read the directory %s: %w", ref.dir, err) } var imps []importRef + files := 0 for _, e := range entries { - if !e.Type().IsRegular() || !countedFile(e.Name(), ref.mod.main) { + if e.IsDir() || !countedFile(e.Name(), ref.mod.main) { continue } + file := path.Join(ref.dir, e.Name()) + // The go command reads a link to a file, as a file of the package. + if !e.Type().IsRegular() { + if info, err := fs.Stat(ref.mod.src.FS, file); err != nil || !info.Mode().IsRegular() { + continue + } + } if b.stats.Files >= b.maxFiles { return nil, errBudget } b.stats.Files++ - file := path.Join(ref.dir, e.Name()) + files++ fileImps, err := b.fileImports(ref.mod.src.FS, file) if err != nil { return nil, err } imps = append(imps, fileImps...) } - b.stats.Packages++ + if files > 0 { + b.stats.Packages++ + } if err := b.cache(ref, imps); err != nil { return nil, err } @@ -90,13 +104,34 @@ func (b *builder) packageImports(ref pkgRef) ([]importRef, error) { } // fileImports returns the imports of one Go file, or none when its build -// constraint can never hold. -func (b *builder) fileImports(fsys fs.FS, file string) ([]importRef, error) { - f, err := b.parse(fsys, file) +// constraint can never hold. It reads the head of the file first, and the +// whole file when the head does not show the end of the import block. +func (b *builder) fileImports(fsys fs.FS, file string) (_ []importRef, err error) { + fh, err := fsys.Open(file) if err != nil { - return nil, err + return nil, fmt.Errorf("open %s: %w", file, err) + } + defer func() { err = errors.Join(err, fh.Close()) }() + src, err := io.ReadAll(io.LimitReader(fh, headSize)) + if err != nil { + return nil, fmt.Errorf("read %s: %w", file, err) } - if excluded(f) { + f, err := parser.ParseFile(b.fset, file, src, parser.ImportsOnly) + if len(src) == headSize && (err != nil || !importsEnd(b.fset, f, src)) { + rest, rerr := io.ReadAll(io.LimitReader(fh, maxFileSize-headSize+1)) + if rerr != nil { + return nil, fmt.Errorf("read %s: %w", file, rerr) + } + if len(src)+len(rest) > maxFileSize { + return nil, fmt.Errorf("%s is larger than %d bytes", file, maxFileSize) + } + src = append(src, rest...) + f, err = parser.ParseFile(b.fset, file, src, parser.ImportsOnly) + } + if err != nil { + return nil, fmt.Errorf("%s does not parse: %w", file, err) + } + if excluded(src) { return nil, nil } test := strings.HasSuffix(file, "_test.go") @@ -111,82 +146,136 @@ func (b *builder) fileImports(fsys fs.FS, file string) ([]importRef, error) { return out, nil } -// parse reads the package clause, the comments before it and the import -// block of a Go file. It parses the head of the file first, and the whole -// file when the head does not parse. -func (b *builder) parse(fsys fs.FS, file string) (_ *ast.File, err error) { - fh, err := fsys.Open(file) - if err != nil { - return nil, fmt.Errorf("open %s: %w", file, err) - } - defer func() { err = errors.Join(err, fh.Close()) }() - head := make([]byte, headSize) - n, err := io.ReadFull(fh, head) - if err != nil && !errors.Is(err, io.ErrUnexpectedEOF) && !errors.Is(err, io.EOF) { - return nil, fmt.Errorf("read %s: %w", file, err) +// importsEnd reports whether a head of a Go file holds the end of its import +// block: after the last import declaration, a token that is not import and +// that ends before the end of the head. A head can end inside a comment +// between two import declarations, or inside the word import, and still +// parse, so a parse of the head alone proves nothing. +func importsEnd(fset *token.FileSet, f *ast.File, head []byte) bool { + end := f.Name.End() + if n := len(f.Decls); n > 0 { + end = f.Decls[n-1].End() } - whole := n < headSize - src := head[:n] - f, perr := parser.ParseFile(b.fset, file, src, parser.ImportsOnly|parser.ParseComments) - if perr == nil || whole { - if perr != nil { - return nil, fmt.Errorf("%s does not parse: %w", file, perr) + off := fset.File(f.Package).Offset(end) + rest := head[off:] + tf := token.NewFileSet().AddFile("", -1, len(rest)) + var sc scanner.Scanner + bad := false + sc.Init(tf, rest, func(token.Position, string) { bad = true }, 0) + for { + pos, tok, lit := sc.Scan() + switch { + case bad || tok == token.EOF || tok == token.ILLEGAL || tok == token.IMPORT: + return false + case tok == token.SEMICOLON && lit == "\n": + continue } - return f, nil + size := len(lit) + if size == 0 { + size = len(tok.String()) + } + return tf.Offset(pos)+size < len(rest) } - rest, err := io.ReadAll(io.LimitReader(fh, maxFileSize-headSize+1)) - if err != nil { - return nil, fmt.Errorf("read %s: %w", file, err) +} + +// excluded reports whether no build reads a file: its build constraint can +// never hold, with each tag free except ignore, which no build sets. It +// reads the constraint as the go command does, in go/build shouldBuild: a +// //go:build line in the leading comments decides, and else each // +build +// line before the last blank line of the leading // comments must hold. A +// constraint that does not parse, two //go:build lines, and a constraint +// with more than maxConstraintTags tags leave the file in, so the graph +// loses no edge. +func excluded(src []byte) bool { + header, goBuild, ok := fileHeader(src) + if !ok { + return false } - if len(src)+len(rest) > maxFileSize { - return nil, fmt.Errorf("%s is larger than %d bytes and its head does not parse", file, maxFileSize) + if goBuild != "" { + expr, err := constraint.Parse(goBuild) + return err == nil && !satisfiable(expr) } - f, perr = parser.ParseFile(b.fset, file, append(src, rest...), parser.ImportsOnly|parser.ParseComments) - if perr != nil { - return nil, fmt.Errorf("%s does not parse: %w", file, perr) + var all constraint.Expr + for line := range strings.Lines(string(header)) { + line = strings.TrimSpace(line) + if !constraint.IsPlusBuild(line) { + continue + } + expr, err := constraint.Parse(line) + if err != nil { + continue + } + if all == nil { + all = expr + } else { + all = &constraint.AndExpr{X: all, Y: expr} + } } - return f, nil + return all != nil && !satisfiable(all) } -// excluded reports whether no build reads a file: its build constraint can -// never hold, with each tag free except ignore, which no build sets. A -// //go:build line decides when the file has one. Else each // +build line -// must hold, as the go command reads them. The go command reads a -// constraint only in the comments before the blank line that ends them, so -// a line of the doc comment of the package does not count. A constraint that -// does not parse, or that has more than maxConstraintTags tags, leaves the -// file in, so the graph loses no edge. -func excluded(f *ast.File) bool { - var plus []constraint.Expr - for _, group := range f.Comments { - if group.End() >= f.Package { - break - } - if group == f.Doc { - continue +// fileHeader is go/build parseFileHeader. It returns the part of the file +// up to the last blank line of the leading run of // comments and blank +// lines, where a // +build line counts, and the //go:build line of the +// leading comments. ok is false for a file with two //go:build lines. +func fileHeader(content []byte) (header []byte, goBuild string, ok bool) { + end := 0 + p := content + ended := false + inSlashStar := false +Lines: + for len(p) > 0 { + line := p + if i := bytes.IndexByte(line, '\n'); i >= 0 { + line, p = line[:i], p[i+1:] + } else { + p = p[len(p):] + } + line = bytes.TrimSpace(line) + if len(line) == 0 && !ended { + end = len(content) - len(p) + continue Lines + } + if !bytes.HasPrefix(line, []byte("//")) { + ended = true + } + if !inSlashStar && isGoBuildComment(line) { + if goBuild != "" { + return nil, "", false + } + goBuild = string(line) } - for _, c := range group.List { - switch { - case constraint.IsGoBuild(c.Text): - expr, err := constraint.Parse(c.Text) - return err == nil && !satisfiable(expr) - case constraint.IsPlusBuild(c.Text): - expr, err := constraint.Parse(c.Text) - if err != nil { - return false + Comments: + for len(line) > 0 { + if inSlashStar { + if i := bytes.Index(line, []byte("*/")); i >= 0 { + inSlashStar = false + line = bytes.TrimSpace(line[i+2:]) + continue Comments } - plus = append(plus, expr) + continue Lines + } + if bytes.HasPrefix(line, []byte("//")) { + continue Lines + } + if bytes.HasPrefix(line, []byte("/*")) { + inSlashStar = true + line = bytes.TrimSpace(line[2:]) + continue Comments } + break Lines } } - if len(plus) == 0 { + return content[:end], goBuild, true +} + +// isGoBuildComment is go/build isGoBuildComment. +func isGoBuildComment(line []byte) bool { + rest, ok := bytes.CutPrefix(line, []byte("//go:build")) + if !ok { return false } - all := plus[0] - for _, e := range plus[1:] { - all = &constraint.AndExpr{X: all, Y: e} - } - return !satisfiable(all) + return len(rest) == 0 || len(bytes.TrimSpace(rest)) < len(rest) } // satisfiable reports whether some set of tags makes a constraint hold, diff --git a/modgraph/graph.go b/modgraph/graph.go index 9c5b87c9..36725cb9 100644 --- a/modgraph/graph.go +++ b/modgraph/graph.go @@ -18,12 +18,15 @@ import ( "go/token" "go/version" "io/fs" + "maps" "slices" "strings" + "sync" "golang.org/x/mod/modfile" "github.com/safedep/code/fileindex/store" + "github.com/safedep/code/projectgraph/modules" ) // Version changes when Build gives another graph for the same input. @@ -69,7 +72,12 @@ type Source struct { // as a directory of the module cache. Build caches what it reads of an // immutable source. A vendor directory can change, so it is not immutable. Immutable bool - // Sum is the h1 hash of the module in go.sum, or "". It is a part of the + // Origin is the module version whose code FS holds, when it is not + // Module: the target of a replace directive. The cache keys a package + // by Origin, so a fork never serves the imports of the module it + // replaces. + Origin Module + // Sum is the h1 hash of Origin in go.sum, or "". It is a part of the // cache key, so a module version that a private proxy serves again with // other bytes gets a new entry. Sum string @@ -114,6 +122,9 @@ type Node struct { Reached bool // Reason says why the node is not StateImports. Reason string + // Walked is true when Build read a package of the module. The edges of + // a module that Build did not walk are not known. + Walked bool } // Open reports whether the node can import a module that it has no edge to. @@ -164,46 +175,79 @@ type Graph struct { // Edges is sorted by the path of From, then of To. Edges []Edge Stats Stats + + index sync.Once + next map[string][]string + nodeIndex map[string]int +} + +func (g *Graph) build() { + g.index.Do(func() { + g.next = map[string][]string{} + for _, e := range g.Edges { + g.next[e.From.Path] = append(g.next[e.From.Path], e.To.Path) + } + g.nodeIndex = make(map[string]int, len(g.Nodes)) + for i, n := range g.Nodes { + g.nodeIndex[n.Module.Path] = i + } + }) } // Node returns the node of a module path. func (g *Graph) Node(path string) (Node, bool) { - i, ok := slices.BinarySearchFunc(g.Nodes, path, func(n Node, p string) int { return strings.Compare(n.Module.Path, p) }) + g.build() + i, ok := g.nodeIndex[path] if !ok { return Node{}, false } return g.Nodes[i], true } -// Reaches reports whether a path of edges from one module can reach -// another. A path that meets an open node can reach each module. -func (g *Graph) Reaches(from, to string) bool { - next := map[string][]string{} - for _, e := range g.Edges { - next[e.From.Path] = append(next[e.From.Path], e.To.Path) +// Reaches reports whether a path from one of the modules can reach a +// module. See Path. +func (g *Graph) Reaches(from []string, to string) bool { return g.Path(from, to) != nil } + +// Path returns a shortest path of module paths from one of the modules to +// a module, or nil. A path that meets a node whose edges are not known, an +// open node or a module that Build did not walk, can reach each module: +// the path ends at that node, and to follows it. A module is not on a path +// from itself. +func (g *Graph) Path(from []string, to string) []string { + g.build() + prev := map[string]string{} + var queue []string + for _, f := range from { + if _, ok := prev[f]; !ok && f != to { + prev[f] = "" + queue = append(queue, f) + } + } + back := func(p string) []string { + var out []string + for ; p != ""; p = prev[p] { + out = append(out, p) + } + slices.Reverse(out) + return out } - seen := map[string]bool{from: true} - queue := []string{from} for len(queue) > 0 { p := queue[0] queue = queue[1:] - if p == to && p != from { - return true + if n, ok := g.Node(p); !ok || n.Open() || !n.Walked { + return append(back(p), to) } - if n, ok := g.Node(p); ok && n.Open() { - return true - } - for _, q := range next[p] { + for _, q := range g.next[p] { if q == to { - return true + return append(back(p), to) } - if !seen[q] { - seen[q] = true + if _, seen := prev[q]; !seen { + prev[q] = p queue = append(queue, q) } } } - return false + return nil } // Build reads the module graph of the main module in MainFS. It returns @@ -335,7 +379,9 @@ func (b *builder) run() (*Graph, error) { return b.graph(), nil } -// walkMain reads each package of the main module, with its test files. +// walkMain reads each package of the main module, with its test files. The +// walk does not follow a link to a directory, so a package that an import +// reaches through a link is queued as any other. func (b *builder) walkMain() error { b.main.walked = true return fs.WalkDir(b.in.MainFS, ".", func(dir string, d fs.DirEntry, err error) error { @@ -352,7 +398,9 @@ func (b *builder) walkMain() error { if err := b.ctx.Err(); err != nil { return err } - return b.readAndLink(pkgRef{mod: b.main, dir: dir}) + ref := pkgRef{mod: b.main, dir: dir} + b.seen[ref] = true + return b.readAndLink(ref) }) } @@ -397,7 +445,7 @@ func (b *builder) link(from *module, imp importRef) { } cands := b.resolve(imp.Path) if len(cands) == 0 { - if isStdPath(imp.Path) && !b.prefixed(imp.Path) { + if modules.IsStdPath(imp.Path) && !b.prefixed(imp.Path) { b.addEdge(from, b.stdNode(), imp, StateImports) return } @@ -415,7 +463,7 @@ func (b *builder) link(from *module, imp importRef) { } func (b *builder) enqueue(ref pkgRef) { - if ref.mod.main || b.seen[ref] { + if b.seen[ref] { return } b.seen[ref] = true @@ -425,8 +473,9 @@ func (b *builder) enqueue(ref pkgRef) { func (b *builder) addEdge(from, to *module, imp importRef, kind State) { k := edgeKey{from.node.Module.Path, to.node.Module.Path} if e, ok := b.edges[k]; ok { - if !imp.Test { + if e.TestOnly && !imp.Test { e.TestOnly = false + e.Evidence = Evidence{File: imp.File, Line: imp.Line} } return } @@ -461,7 +510,7 @@ func (b *builder) finish() { for { reached := b.reachable() added := false - for _, p := range sortedKeys(b.byPath) { + for _, p := range slices.Sorted(maps.Keys(b.byPath)) { m := b.byPath[p] if reached[m] && m.node.State == StateRequires && !expanded[m] { expanded[m] = true @@ -529,8 +578,11 @@ func (b *builder) reachable() map[*module]bool { func (b *builder) graph() *Graph { g := &Graph{Version: Version, Main: b.main.node.Module, Stats: b.stats} - for _, p := range sortedKeys(b.byPath) { - g.Nodes = append(g.Nodes, b.byPath[p].node) + for _, p := range slices.Sorted(maps.Keys(b.byPath)) { + m := b.byPath[p] + n := m.node + n.Walked = m.walked + g.Nodes = append(g.Nodes, n) } for _, e := range b.edges { g.Edges = append(g.Edges, *e) @@ -543,12 +595,3 @@ func (b *builder) graph() *Graph { }) return g } - -func sortedKeys[V any](m map[string]V) []string { - keys := make([]string, 0, len(m)) - for k := range m { - keys = append(keys, k) - } - slices.Sort(keys) - return keys -} diff --git a/modgraph/graph_test.go b/modgraph/graph_test.go index e3556574..16aa040e 100644 --- a/modgraph/graph_test.go +++ b/modgraph/graph_test.go @@ -3,6 +3,10 @@ package modgraph import ( "context" "fmt" + "go/parser" + "go/token" + "os" + "path/filepath" "slices" "strings" "testing" @@ -240,7 +244,7 @@ func TestBuildOpensAModuleThatImportsWhatItCannotResolve(t *testing.T) { unused := node(t, g, "example.com/unused") assert.True(t, unused.Reached, "an open node can reach each module") assert.Equal(t, StateOpen, unused.State, "no walk read its packages") - assert.True(t, g.Reaches(mainPath, "example.com/unused")) + assert.True(t, g.Reaches([]string{mainPath}, "example.com/unused")) }) } } @@ -293,7 +297,11 @@ func TestBuildKeepsAFileUnlessItsConstraintCanNeverHold(t *testing.T) { {"+build lines that cannot hold together", "a.go", "// +build linux\n// +build !linux\n\npackage a\n\nimport _ \"example.com/dep\"\n", false}, {"a //go:build line wins over a +build line", "a.go", "//go:build linux\n// +build ignore\n\npackage a\n\nimport _ \"example.com/dep\"\n", true}, {"a +build line of the package doc comment", "a.go", "// +build ignore\npackage a\n\nimport _ \"example.com/dep\"\n", true}, - {"a line of the package doc comment", "a.go", "//go:build ignore\npackage a\n\nimport _ \"example.com/dep\"\n", true}, + {"a //go:build line of the package doc comment", "a.go", "//go:build ignore\npackage a\n\nimport _ \"example.com/dep\"\n", false}, + {"a +build line after a block comment", "a.go", "/* license */\n\n// +build ignore\n\npackage a\n\nimport _ \"example.com/dep\"\n", true}, + {"a +build line before a block comment", "a.go", "// +build ignore\n/* x */\n\npackage a\n\nimport _ \"example.com/dep\"\n", true}, + {"a //go:build line after a block comment", "a.go", "/* license */\n//go:build ignore\n\npackage a\n\nimport _ \"example.com/dep\"\n", false}, + {"two //go:build lines", "a.go", "//go:build ignore\n//go:build ignore\n\npackage a\n\nimport _ \"example.com/dep\"\n", true}, {"a constraint that does not parse", "a.go", "//go:build linux &&\n\npackage a\n\nimport _ \"example.com/dep\"\n", true}, } for _, tc := range cases { @@ -321,17 +329,53 @@ func TestSatisfiableCountsAConstraintWithManyTags(t *testing.T) { assert.Contains(t, edgeList(g), "example.com/a -> example.com/dep", "past the tag cap the file counts") } -func TestBuildReadsAnImportBlockPastTheHead(t *testing.T) { - long := "// " + strings.Repeat("x", headSize) + "\n\npackage a\n\nimport _ \"example.com/dep\"\n" +func TestBuildReadsEachImportOfALongFile(t *testing.T) { + long := "package a\n\nimport _ \"example.com/first\"\n\n// " + strings.Repeat("x", 70<<10) + "\n\nimport _ \"example.com/dep\"\n" g := build(t, Input{ MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a")}), Modules: []Source{ source("example.com/a", fstest.MapFS{"a.go": {Data: []byte(long)}}), + source("example.com/first", fstest.MapFS{"f.go": goSrc("first")}), source("example.com/dep", fstest.MapFS{"d.go": goSrc("dep")}), }, }) - assert.Contains(t, edgeList(g), "example.com/a -> example.com/dep") - assert.Equal(t, StateImports, node(t, g, "example.com/a").State) + assert.Contains(t, edgeList(g), "example.com/a -> example.com/dep", "an import after a long comment counts") + assert.Contains(t, edgeList(g), "example.com/a -> example.com/first") +} + +func TestBuildReadsTheLinksOfTheMainModule(t *testing.T) { + root := t.TempDir() + writeFiles(t, root, map[string]string{ + "app/go.mod": "module " + mainPath + "\n\ngo 1.22\n", + "app/main.go": "package main\n\nimport _ \"" + mainPath + "/linked\"\n", + "shared/s.go": "package real\n\nimport _ \"example.com/viafile\"\n", + "outside/o.go": "package linked\n\nimport _ \"example.com/viadir\"\n", + "app/real/keep.go": "package real\n", + }) + require.NoError(t, os.Symlink(filepath.Join(root, "shared", "s.go"), filepath.Join(root, "app", "real", "link.go"))) + require.NoError(t, os.Symlink(filepath.Join(root, "outside"), filepath.Join(root, "app", "linked"))) + g := build(t, Input{ + MainFS: os.DirFS(filepath.Join(root, "app")), + Modules: []Source{ + source("example.com/viafile", fstest.MapFS{"v.go": goSrc("viafile")}), + source("example.com/viadir", fstest.MapFS{"v.go": goSrc("viadir")}), + }, + }) + assert.Contains(t, edgeList(g), mainPath+" -> example.com/viafile", "a link to a file is a file of the package") + assert.Contains(t, edgeList(g), mainPath+" -> example.com/viadir", "a package that an import reaches through a link to a directory is read") + assert.Equal(t, StateImports, node(t, g, mainPath).State) +} + +func TestBuildReadsATestdataPackageThatAnImportNames(t *testing.T) { + g := build(t, Input{ + MainFS: mainFS(fstest.MapFS{ + "main.go": goSrc("main", mainPath+"/testdata/fixture"), + "testdata/fixture/f.go": goSrc("fixture", "example.com/dep"), + }), + Modules: []Source{source("example.com/dep", fstest.MapFS{"d.go": goSrc("dep")})}, + }) + assert.Equal(t, []string{mainPath + " -> example.com/dep"}, edgeList(g)) + assert.Equal(t, StateImports, node(t, g, mainPath).State) } func TestBuildOpensAModuleWithAFileThatDoesNotParse(t *testing.T) { @@ -447,6 +491,18 @@ func TestBuildCachesThePackagesOfAnImmutableSource(t *testing.T) { assert.Empty(t, found, "a mutable source has no entry") } +func TestBuildCachesAReplacementByItsOwnModule(t *testing.T) { + st := memstore.New() + original := Source{Module: Module{Path: "example.com/a", Version: "v1.0.0"}, FS: fstest.MapFS{"a.go": goSrc("a")}, Immutable: true} + fork := Source{Module: Module{Path: "example.com/a", Version: "v1.0.0"}, Origin: Module{Path: "example.com/fork", Version: "v1.0.0"}, FS: fstest.MapFS{"a.go": goSrc("a", "example.com/dep")}, Immutable: true} + dep := source("example.com/dep", fstest.MapFS{"d.go": goSrc("dep")}) + main := mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a")}) + + build(t, Input{MainFS: main, Modules: []Source{original, dep}, Store: st}) + g := build(t, Input{MainFS: main, Modules: []Source{fork, dep}, Store: st}) + assert.Contains(t, edgeList(g), "example.com/a -> example.com/dep", "the fork is not served the imports of the original") +} + func TestBuildCachesNoPackageThatFailed(t *testing.T) { st := memstore.New() bad := Source{Module: Module{Path: "example.com/a", Version: "v1.0.0"}, FS: fstest.MapFS{"a.go": {Data: []byte("package a\nimport (\n")}}, Immutable: true} @@ -457,7 +513,7 @@ func TestBuildCachesNoPackageThatFailed(t *testing.T) { assert.Equal(t, StateOpen, node(t, g, "example.com/a").State) } -func TestGraphReaches(t *testing.T) { +func TestGraphPath(t *testing.T) { g := build(t, Input{ MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a")}), Modules: []Source{ @@ -466,9 +522,56 @@ func TestGraphReaches(t *testing.T) { source("example.com/c", fstest.MapFS{"c.go": goSrc("c", "example.com/b")}), }, }) - assert.True(t, g.Reaches(mainPath, "example.com/b")) - assert.True(t, g.Reaches("example.com/a", "example.com/b")) - assert.False(t, g.Reaches("example.com/b", "example.com/a")) - assert.False(t, g.Reaches(mainPath, "example.com/c")) - assert.False(t, g.Reaches("example.com/c", "example.com/b"), "the build reads no package of a module that nothing reaches") + assert.Equal(t, []string{mainPath, "example.com/a", "example.com/b"}, g.Path([]string{mainPath}, "example.com/b")) + assert.True(t, g.Reaches([]string{"example.com/a"}, "example.com/b")) + assert.False(t, g.Reaches([]string{"example.com/b"}, "example.com/a")) + assert.False(t, g.Reaches([]string{mainPath}, "example.com/c")) + assert.False(t, g.Reaches([]string{"example.com/b"}, "example.com/b"), "a module is not on a path from itself") + assert.Equal(t, []string{"example.com/c", "example.com/b"}, g.Path([]string{"example.com/c"}, "example.com/b"), + "Build read no package of c, so its edges are not known and it can reach each module") + assert.False(t, node(t, g, "example.com/c").Walked) + assert.True(t, node(t, g, "example.com/a").Walked) +} + +func TestImportsEndNeedsAWholeTokenAfterTheImports(t *testing.T) { + cases := []struct { + name string + head string + want bool + }{ + {"a declaration after the imports", "package a\n\nimport _ \"x\"\n\nfunc f() {}\n", true}, + {"a cut inside a comment", "package a\n\nimport _ \"x\"\n\n// comment", false}, + {"a cut inside the word import", "package a\n\nimport _ \"x\"\n\nimp", false}, + {"a cut after the word import, which does not parse", "package a\n\nimport _ \"x\"\n\nimport", false}, + {"a cut inside a word that is not import", "package a\n\nimport _ \"x\"\n\nfun", false}, + {"no import and a declaration", "package a\n\nvar v = 1\n", true}, + {"a cut inside a block comment, which does not parse", "package a\n\nimport _ \"x\"\n\n/* open", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "a.go", tc.head, parser.ImportsOnly) + if strings.Contains(tc.name, "does not parse") { + assert.Error(t, err, "Build reads the whole file") + return + } + require.NoError(t, err) + assert.Equal(t, tc.want, importsEnd(fset, f, []byte(tc.head))) + }) + } +} + +func TestBuildReadsAnImportThatTheHeadCuts(t *testing.T) { + prefix := "package a\n\nimport _ \"example.com/first\"\n// " + src := prefix + strings.Repeat("x", headSize-3-len(prefix)-1) + "\nimport _ \"example.com/dep\"\n" + require.Equal(t, "imp", src[headSize-3:headSize], "the head ends inside the word import") + g := build(t, Input{ + MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a")}), + Modules: []Source{ + source("example.com/a", fstest.MapFS{"a.go": {Data: []byte(src)}}), + source("example.com/first", fstest.MapFS{"f.go": goSrc("first")}), + source("example.com/dep", fstest.MapFS{"d.go": goSrc("dep")}), + }, + }) + assert.Contains(t, edgeList(g), "example.com/a -> example.com/dep") } diff --git a/modgraph/oracle_test.go b/modgraph/oracle_test.go index 2abdc683..9a6bea2d 100644 --- a/modgraph/oracle_test.go +++ b/modgraph/oracle_test.go @@ -81,7 +81,8 @@ func TestBuildHoldsEachEdgeOfGoList(t *testing.T) { listed := goListEdges(t, dir, p[0], p[1]) t.Logf("%s/%s: go list has %d module edges, the graph %d, with %d nodes", p[0], p[1], len(listed), len(g.Edges), len(g.Nodes)) for e := range listed { - from, _ := g.Node(e[0]) + from, ok := g.Node(e[0]) + require.True(t, ok, "the graph has no node %s", e[0]) to, ok := g.Node(e[1]) assert.True(t, ok && to.Reached, "%s/%s: go list reaches %s, the graph does not", p[0], p[1], e[1]) assert.True(t, edges[e] || from.Open(), "%s/%s: go list has the edge %s -> %s", p[0], p[1], e[0], e[1]) diff --git a/modgraph/resolve.go b/modgraph/resolve.go index 0e006e63..03a39afb 100644 --- a/modgraph/resolve.go +++ b/modgraph/resolve.go @@ -3,6 +3,7 @@ package modgraph import ( "io/fs" "path" + "slices" "strings" ) @@ -68,25 +69,14 @@ func under(importPath, modPath string) (string, bool) { return "", false } -// isStdPath reports whether an import path can name a package of the -// standard library: its first element has no dot. -func isStdPath(importPath string) bool { - first, _, _ := strings.Cut(importPath, "/") - return first != "" && !strings.Contains(first, ".") -} - // hasPackageDir reports whether the source of a module has the directory of -// a package that an importer can build. A directory that the go command -// skips, or that belongs to a module nested in this one, is not a package of -// the module. +// a package that an importer can build. An import can name a testdata +// directory or one whose name starts with _ or a dot, and the go command +// builds it, though ./... skips it. A vendor directory, and a directory of a +// module nested in this one, is not a package of the module. func hasPackageDir(fsys fs.FS, dir string) bool { if dir != "." { - for _, part := range strings.Split(dir, "/") { - if skippedDir(part) { - return false - } - } - if nestedModule(fsys, dir) { + if slices.Contains(strings.Split(dir, "/"), "vendor") || nestedModule(fsys, dir) { return false } } From 91de70bd031c4f5f1853f12458f02ce578502f82 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 19:24:45 +0000 Subject: [PATCH 3/5] fix(modgraph): resolve by Go files and sweep once a node is open The resolver took a module whose package directory had no Go file. The go command takes the next shorter prefix in that case. The resolver now needs an entry that ends in .go and is a file or a link to a file, as IsDirWithGoFiles does. An open node can import any package of the build list. Build now reads each package of each module with source once a reached node is open. Before this, a module with one read package hid the edges of its other packages. The sweep skips a package that does not parse or that imports a module outside the build list, because no build of the project reads it. The resolver looks up the prefixes of the import path in a map. Build of vet takes 0.9 s, down from 1.7 s. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PFQbqPqd9xn1fT6hBsyoUm Signed-off-by: Claude --- docs/modgraph.md | 21 ++++-- modgraph/files.go | 10 ++- modgraph/graph.go | 157 ++++++++++++++++++++++++++++------------ modgraph/graph_test.go | 57 ++++++++++++++- modgraph/oracle_test.go | 2 +- modgraph/resolve.go | 109 +++++++++++++++++++--------- 6 files changed, 268 insertions(+), 88 deletions(-) diff --git a/docs/modgraph.md b/docs/modgraph.md index fc95c456..26af2232 100644 --- a/docs/modgraph.md +++ b/docs/modgraph.md @@ -36,8 +36,10 @@ module, and a path that meets it reaches each module. own. A link to a file is a file of its package. 2. For each import, the resolver takes the modules of the build list whose path is a prefix of the import path, longest first. A module with source - holds the package when it has the directory. A module with no source may - hold it, so it gets an edge too, and the test goes on to a shorter prefix. + holds the package when its directory has an entry whose name ends in + `.go` and that is a file or a link to a file, as the go command decides + it. A module with no source may hold it, so it gets an edge too, and the + test goes on to a shorter prefix. A path that no module holds is the standard library when its first element has no dot. Else its importer is open. 3. The walk reads each package that an import reaches, as `go list -deps` @@ -51,8 +53,17 @@ module, and a path that meets it reaches each module. `//go:build purego` counts, because a project can build with `-tags purego`. A GOOS or GOARCH suffix of the file name never drops a file. -5. Once a reached node is open, each node counts as reached, and a module - that the walk read no package of becomes open. +5. Once a reached node is open, it can import any package of the build + list. So each node counts as reached, and `Build` reads each directory + of each module with source that an import can name, except `vendor` and + a nested module. Else a module of one read package would hide the edges + of its other packages. A module with a link to a directory is then open, + because the walk does not follow the link. A module zip holds no links. + This read skips a package that does not parse, or that imports a path + that no module of the build list holds. No build of the project reads + such a package, because `go mod tidy` lists in the main `go.mod` each + module that a package of the build imports. In the walk from the main + module, such a package makes its module open. A main `go.mod` with a `go` directive below 1.17, or with none, gives `ErrUnpruned`. Before module graph pruning, `go.mod` need not list each @@ -116,7 +127,7 @@ with neither. It sets `GOPROXY=off`, so it downloads nothing. ## Not covered - `go.work` workspaces. A caller keeps no graph for a workspace. -- A file larger than 16 MiB whose first 64 KiB does not parse makes its +- A file larger than 32 MiB whose first 64 KiB does not parse makes its module open. - A file budget, 200,000 files by default, caps a run. Past it the module that is next is open, and `Stats.Truncated` is set. diff --git a/modgraph/files.go b/modgraph/files.go index ee3323c6..57966120 100644 --- a/modgraph/files.go +++ b/modgraph/files.go @@ -60,6 +60,12 @@ func countedFile(name string, tests bool) bool { return tests || !strings.HasSuffix(name, "_test.go") } +// unbuildable is the error of a package that the go command cannot build, +// such as a package with a file that does not parse. +type unbuildable struct{ error } + +func (e unbuildable) Unwrap() error { return e.error } + // packageImports returns the imports of the Go files of one package. It // reads the cache first for an immutable source. func (b *builder) packageImports(ref pkgRef) ([]importRef, error) { @@ -129,7 +135,7 @@ func (b *builder) fileImports(fsys fs.FS, file string) (_ []importRef, err error f, err = parser.ParseFile(b.fset, file, src, parser.ImportsOnly) } if err != nil { - return nil, fmt.Errorf("%s does not parse: %w", file, err) + return nil, unbuildable{fmt.Errorf("%s does not parse: %w", file, err)} } if excluded(src) { return nil, nil @@ -139,7 +145,7 @@ func (b *builder) fileImports(fsys fs.FS, file string) (_ []importRef, err error for _, spec := range f.Imports { p, err := strconv.Unquote(spec.Path.Value) if err != nil { - return nil, fmt.Errorf("%s: import %s: %w", file, spec.Path.Value, err) + return nil, unbuildable{fmt.Errorf("%s: import %s: %w", file, spec.Path.Value, err)} } out = append(out, importRef{Path: p, File: file, Line: b.fset.Position(spec.Pos()).Line, Test: test}) } diff --git a/modgraph/graph.go b/modgraph/graph.go index 36725cb9..32ee771b 100644 --- a/modgraph/graph.go +++ b/modgraph/graph.go @@ -285,6 +285,8 @@ type module struct { // walked is true when Build read a package of the module, so its edges // are those of the packages that the build reaches. walked bool + // expanded is true when Build added the require edges of its go.mod. + expanded bool } type pkgRef struct { @@ -303,12 +305,11 @@ type builder struct { main *module std *module byPath map[string]*module - // paths holds the module paths of the build list and of the main module, - // longest first, for the resolver. - paths []string queue []pkgRef seen map[pkgRef]bool + // sweep is true once walkAll reads each package of the build list. + sweep bool edges map[edgeKey]*Edge stats Stats } @@ -349,15 +350,6 @@ func newBuilder(ctx context.Context, in Input, mainPath string) (*builder, error } b.byPath[p] = m } - for p := range b.byPath { - b.paths = append(b.paths, p) - } - slices.SortFunc(b.paths, func(x, y string) int { - if len(x) != len(y) { - return len(y) - len(x) - } - return strings.Compare(x, y) - }) return b, nil } @@ -365,18 +357,27 @@ func (b *builder) run() (*Graph, error) { if err := b.walkMain(); err != nil { return nil, err } + if err := b.drain(); err != nil { + return nil, err + } + if err := b.finish(); err != nil { + return nil, err + } + return b.graph(), nil +} + +func (b *builder) drain() error { for len(b.queue) > 0 { if err := b.ctx.Err(); err != nil { - return nil, err + return err } ref := b.queue[0] b.queue = b.queue[1:] if err := b.walkPackage(ref); err != nil { - return nil, err + return err } } - b.finish() - return b.graph(), nil + return nil } // walkMain reads each package of the main module, with its test files. The @@ -423,6 +424,9 @@ func (b *builder) readAndLink(ref pkgRef) error { return err case err != nil && b.ctx.Err() != nil: return b.ctx.Err() + case errors.As(err, new(unbuildable)): + b.cannotBuild(ref.mod, err.Error()) + return nil case err != nil: b.setOpen(ref.mod, err.Error()) return nil @@ -440,7 +444,7 @@ func (b *builder) link(from *module, imp importRef) { case imp.Path == "C": return case imp.Path == "" || strings.HasPrefix(imp.Path, ".") || strings.HasPrefix(imp.Path, "/"): - b.setOpen(from, fmt.Sprintf("%s imports the path %q, which the build cannot resolve", imp.File, imp.Path)) + b.cannotBuild(from, fmt.Sprintf("%s imports the path %q, which the build cannot resolve", imp.File, imp.Path)) return } cands := b.resolve(imp.Path) @@ -449,7 +453,7 @@ func (b *builder) link(from *module, imp importRef) { b.addEdge(from, b.stdNode(), imp, StateImports) return } - b.setOpen(from, fmt.Sprintf("%s imports %s, which no module of the build list holds", imp.File, imp.Path)) + b.cannotBuild(from, fmt.Sprintf("%s imports %s, which no module of the build list holds", imp.File, imp.Path)) return } for _, c := range cands { @@ -494,6 +498,18 @@ func (b *builder) stdNode() *module { return b.std } +// cannotBuild records a package that the go command cannot build. In the +// walk from the main module its module becomes open, because the build of +// the project reads the package, and Build cannot tell its imports. In the +// sweep no build reads the package: go mod tidy lists in the main go.mod +// each module that a package of the build imports, and the package does not +// parse or imports a path outside that list. So it adds nothing. +func (b *builder) cannotBuild(m *module, reason string) { + if !b.sweep { + b.setOpen(m, reason) + } +} + func (b *builder) setOpen(m *module, reason string) { if m.node.State == StateOpen { return @@ -502,41 +518,90 @@ func (b *builder) setOpen(m *module, reason string) { } // finish adds the require edges of each reached module with no source, then -// sets Reached. Once a reached node is open, each node counts as reached, -// and a module that no walk read becomes open, because its edges are not -// known. -func (b *builder) finish() { - expanded := map[*module]bool{} - for { - reached := b.reachable() - added := false - for _, p := range slices.Sorted(maps.Keys(b.byPath)) { - m := b.byPath[p] - if reached[m] && m.node.State == StateRequires && !expanded[m] { - expanded[m] = true - b.addRequires(m) - added = true - } +// sets Reached. Once a reached node is open, it can import any package of the +// build list. So each node counts as reached, and Build reads each package of +// each module with source, else a module of one read package would hide the +// edges of the others. +func (b *builder) finish() error { + reached := b.reachable() + for b.addRequires(reached) { + reached = b.reachable() + } + anyOpen := false + for m := range reached { + anyOpen = anyOpen || m.node.Open() + } + if !anyOpen { + for _, m := range b.byPath { + m.node.Reached = reached[m] } - if !added { - anyOpen := false - for m := range reached { - anyOpen = anyOpen || m.node.Open() - } - for _, m := range b.byPath { - m.node.Reached = reached[m] || anyOpen - if anyOpen && !m.walked && m.node.State == StateImports { - b.setOpen(m, "only an open node reaches it, so the build read none of its packages") + return nil + } + if err := b.walkAll(); err != nil { + return err + } + all := map[*module]bool{} + for _, m := range b.byPath { + m.node.Reached = true + all[m] = true + } + b.addRequires(all) + return nil +} + +// walkAll reads each directory of each module with source that an import can +// name, as a sweep. A test file of a dependency still never counts. The walk +// does not follow a link to a directory, so a module with one is open. +func (b *builder) walkAll() error { + b.sweep = true + for _, p := range slices.Sorted(maps.Keys(b.byPath)) { + m := b.byPath[p] + if m.main || m.src.FS == nil || m.node.State == StateOpen { + continue + } + err := fs.WalkDir(m.src.FS, ".", func(dir string, d fs.DirEntry, err error) error { + switch { + case err != nil: + b.setOpen(m, fmt.Sprintf("read %s: %v", dir, err)) + return fs.SkipAll + case d.Type()&fs.ModeSymlink != 0: + if info, err := fs.Stat(m.src.FS, dir); err != nil || info.IsDir() { + b.setOpen(m, fmt.Sprintf("%s is a link that can lead to a package, and the walk does not follow it", dir)) + return fs.SkipAll } + return nil + case !d.IsDir(): + return nil + case dir != "." && (d.Name() == "vendor" || strings.HasPrefix(d.Name(), ".") || nestedModule(m.src.FS, dir)): + return fs.SkipDir } - return + b.enqueue(pkgRef{mod: m, dir: dir}) + return nil + }) + if err != nil { + return err + } + } + return b.drain() +} + +// addRequires adds an edge from each module of the set with no source to +// each module of the build list that its go.mod requires. It reports whether +// it read a go.mod that it had not read. +func (b *builder) addRequires(set map[*module]bool) bool { + added := false + for _, p := range slices.Sorted(maps.Keys(b.byPath)) { + m := b.byPath[p] + if set[m] && m.node.State == StateRequires && !m.expanded { + m.expanded = true + b.addRequiresOf(m) + added = true } } + return added } -// addRequires adds an edge from a module with no source to each module of -// the build list that its go.mod requires. -func (b *builder) addRequires(m *module) { +func (b *builder) addRequiresOf(m *module) { f, err := modfile.ParseLax("go.mod", m.src.GoMod, nil) if err != nil { b.setOpen(m, fmt.Sprintf("its go.mod does not parse: %v", err)) diff --git a/modgraph/graph_test.go b/modgraph/graph_test.go index 16aa040e..349a7905 100644 --- a/modgraph/graph_test.go +++ b/modgraph/graph_test.go @@ -199,6 +199,15 @@ func TestBuildResolvesAnImportPath(t *testing.T) { }, want: []string{"example.com/app -> github.com/a/b/sub"}, }, + { + name: "a longer module whose directory has no Go file", + imp: "example.com/a/sub/pkg", + modules: []Source{ + source("example.com/a", fstest.MapFS{"sub/pkg/p.go": goSrc("pkg")}), + source("example.com/a/sub", fstest.MapFS{"pkg/readme.txt": {Data: []byte("assets")}, "pkg/data.go/x.txt": {Data: []byte("a directory")}}), + }, + want: []string{"example.com/app -> example.com/a"}, + }, { name: "cgo", imp: "C", @@ -243,12 +252,58 @@ func TestBuildOpensAModuleThatImportsWhatItCannotResolve(t *testing.T) { assert.Contains(t, a.Reason, tc.imp) unused := node(t, g, "example.com/unused") assert.True(t, unused.Reached, "an open node can reach each module") - assert.Equal(t, StateOpen, unused.State, "no walk read its packages") + assert.Equal(t, StateImports, unused.State, "Build reads each package that the open node can import") + assert.True(t, unused.Walked) assert.True(t, g.Reaches([]string{mainPath}, "example.com/unused")) }) } } +func TestBuildReadsEachPackageOnceAReachedNodeIsOpen(t *testing.T) { + g := build(t, Input{ + MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a/first", "example.com/gone")}), + Modules: []Source{ + source("example.com/a", fstest.MapFS{ + "first/f.go": goSrc("first"), + "second/s.go": goSrc("second", "example.com/b"), + "second/s_test.go": goSrc("second", "example.com/c"), + "vendor/v/v.go": goSrc("v", "example.com/c"), + "testdata/bad.go": {Data: []byte("package bad\nimport (\n")}, + "stray/s.go": goSrc("stray", "example.com/outside/x"), + }), + source("example.com/b", fstest.MapFS{"b.go": goSrc("b")}), + source("example.com/c", fstest.MapFS{"c.go": goSrc("c")}), + {Module: Module{Path: "example.com/gone", Version: "v1.0.0"}}, + }, + }) + assert.True(t, node(t, g, "example.com/gone").Open()) + assert.Equal(t, StateImports, node(t, g, "example.com/a").State, + "no build reads a package that does not parse or that imports a module outside the build list") + assert.Equal(t, []string{"example.com/a", "example.com/b"}, g.Path([]string{"example.com/a"}, "example.com/b"), + "the open node can import example.com/a/second, which no walk from the main module reaches") + assert.False(t, g.Reaches([]string{"example.com/a"}, "example.com/c"), "a test file and a vendor directory of a dependency never count") + assert.True(t, node(t, g, "example.com/c").Reached) +} + +func TestBuildOpensAModuleWithALinkToADirectoryOnceAReachedNodeIsOpen(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "go.mod"), []byte("module example.com/a\n"), 0o600)) + require.NoError(t, os.Mkdir(filepath.Join(dir, "real"), 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "real", "r.go"), []byte("package real\n"), 0o600)) + require.NoError(t, os.Symlink("real", filepath.Join(dir, "linked"))) + + g := build(t, Input{ + MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/gone")}), + Modules: []Source{ + {Module: Module{Path: "example.com/a", Version: "v1.0.0"}, FS: os.DirFS(dir)}, + {Module: Module{Path: "example.com/gone", Version: "v1.0.0"}}, + }, + }) + a := node(t, g, "example.com/a") + assert.Equal(t, StateOpen, a.State) + assert.Contains(t, a.Reason, "linked") +} + func TestBuildReadsTheGoModOfAModuleWithNoSource(t *testing.T) { g := build(t, Input{ MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a")}), diff --git a/modgraph/oracle_test.go b/modgraph/oracle_test.go index 9a6bea2d..4d5eebdf 100644 --- a/modgraph/oracle_test.go +++ b/modgraph/oracle_test.go @@ -69,7 +69,7 @@ func TestBuildHoldsEachEdgeOfGoList(t *testing.T) { t.Logf("Build: %d files, %d packages, %v; with the cache: %d files, %d cached packages, %v", g.Stats.Files, g.Stats.Packages, cold.Round(time.Millisecond), warm.Stats.Files, warm.Stats.CachedPackages, time.Since(start).Round(time.Millisecond)) for _, n := range g.Nodes { - if n.Open() && n.Reached && !strings.HasPrefix(n.Reason, "only an open node") { + if n.Open() && n.Reached { t.Logf("open: %s: %s", n.Module.Path, n.Reason) } } diff --git a/modgraph/resolve.go b/modgraph/resolve.go index 03a39afb..cc50645f 100644 --- a/modgraph/resolve.go +++ b/modgraph/resolve.go @@ -1,7 +1,9 @@ package modgraph import ( + "errors" "io/fs" + "iter" "path" "slices" "strings" @@ -20,24 +22,23 @@ type candidate struct { // resolve returns the modules that can hold the package of an import path. // The go command takes the module of the build list whose path is the -// longest prefix of the import path and that has the package directory. A -// module with no source may have the directory, so it is a candidate, and the -// test goes on to the next prefix. The first module with source that has the -// directory ends the list. A list with no module that has or may have the -// directory is empty. +// longest prefix of the import path and that has a Go file in the package +// directory. A module with no source may have the file, so it is a candidate, +// and the test goes on to the next prefix. So is a module whose directory +// Build cannot read. The first module with source that has the file ends the +// list. A list with no module that has or may have the file is empty. func (b *builder) resolve(importPath string) []candidate { var out []candidate - for _, p := range b.paths { - dir, ok := under(importPath, p) - if !ok { - continue - } - m := b.byPath[p] + for m, dir := range b.prefixes(importPath) { if m.src.FS == nil { out = append(out, candidate{mod: m, dir: dir}) continue } - if hasPackageDir(m.src.FS, dir) { + has, err := hasPackage(m.src.FS, dir) + switch { + case err != nil: + out = append(out, candidate{mod: m, dir: dir, provides: true}) + case has: return append(out, candidate{mod: m, dir: dir, provides: true}) } } @@ -49,39 +50,81 @@ func (b *builder) resolve(importPath string) []candidate { // of the standard library, even with no dot in its first element, as the // path corp/lib of a replaced module. func (b *builder) prefixed(importPath string) bool { - for _, p := range b.paths { - if _, ok := under(importPath, p); ok { - return true - } + for range b.prefixes(importPath) { + return true } return false } -// under returns the path of a package below a module path, "." for the -// module root, at a "/" boundary. -func under(importPath, modPath string) (string, bool) { - switch { - case importPath == modPath: - return ".", true - case strings.HasPrefix(importPath, modPath+"/"): - return importPath[len(modPath)+1:], true +// prefixes yields each module of the build list, and the main module, whose +// path is the import path or a prefix of it at a "/" boundary, longest +// first. It yields the directory of the package in the module with it, "." +// for the module root. +func (b *builder) prefixes(importPath string) iter.Seq2[*module, string] { + return func(yield func(*module, string) bool) { + for p := importPath; ; { + if m := b.byPath[p]; m != nil && m != b.std { + dir := "." + if p != importPath { + dir = importPath[len(p)+1:] + } + if !yield(m, dir) { + return + } + } + i := strings.LastIndexByte(p, '/') + if i < 0 { + return + } + p = p[:i] + } } - return "", false } -// hasPackageDir reports whether the source of a module has the directory of -// a package that an importer can build. An import can name a testdata -// directory or one whose name starts with _ or a dot, and the go command -// builds it, though ./... skips it. A vendor directory, and a directory of a -// module nested in this one, is not a package of the module. -func hasPackageDir(fsys fs.FS, dir string) bool { +// hasPackage reports whether the source of a module holds a package in a +// directory, as the go command decides it: the directory has an entry whose +// name ends in .go and that is a file or a link to a file. A build +// constraint or a name that starts with _ does not matter here. An import can +// name a testdata directory or one whose name starts with _ or a dot, and the +// go command builds it, though ./... skips it. A vendor directory, and a +// directory of a module nested in this one, is not a package of the module. +// It returns an error when it cannot tell. +func hasPackage(fsys fs.FS, dir string) (bool, error) { if dir != "." { if slices.Contains(strings.Split(dir, "/"), "vendor") || nestedModule(fsys, dir) { - return false + return false, nil } } info, err := fs.Stat(fsys, dir) - return err == nil && info.IsDir() + switch { + case errors.Is(err, fs.ErrNotExist): + return false, nil + case err != nil: + return false, err + case !info.IsDir(): + return false, nil + } + entries, err := fs.ReadDir(fsys, dir) + if err != nil { + return false, err + } + var firstErr error + for _, e := range entries { + if e.IsDir() || !strings.HasSuffix(e.Name(), ".go") { + continue + } + if e.Type().IsRegular() { + return true, nil + } + info, err := fs.Stat(fsys, path.Join(dir, e.Name())) + if err == nil && info.Mode().IsRegular() { + return true, nil + } + if err != nil && firstErr == nil { + firstErr = err + } + } + return false, firstErr } // nestedModule reports whether a directory below the root of a module, or From e35a246b96d545fd8a3a4f6895d9dc897ec32808 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 19:30:31 +0000 Subject: [PATCH 4/5] fix(modgraph): confine local links and bound constraint tests A link inside a local replacement could lead Build out of LocalRoot. Each read of a local replacement now resolves its links again. A link that leads out of LocalRoot makes the module open. A dependency of many files with unsatisfiable constraints of 16 tags cost 2^16 tests per file. One Build now tests at most 4,194,304 tag sets. Past that, each constraint counts as one that can hold. The oracle test now fails when an edge of go list leaves a module with no source, and when go list resolves no module for an import of this repository. Both cases let the test pass with nothing compared. The docs example checks each error. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PFQbqPqd9xn1fT6hBsyoUm Signed-off-by: Claude --- docs/modgraph.md | 12 +++++++++ modgraph/discover.go | 39 ++++++++++++++++++++++------ modgraph/discover_test.go | 46 +++++++++++++++++++++++++++++++++ modgraph/files.go | 53 ++++++++++++++++++++++++--------------- modgraph/graph.go | 21 +++++++++------- modgraph/graph_test.go | 13 ++++++++++ modgraph/oracle_test.go | 33 +++++++++++++++++++----- 7 files changed, 175 insertions(+), 42 deletions(-) diff --git a/docs/modgraph.md b/docs/modgraph.md index 26af2232..a31d5910 100644 --- a/docs/modgraph.md +++ b/docs/modgraph.md @@ -77,11 +77,17 @@ sources, err := modgraph.Discover(modgraph.DiscoverOptions{ ModuleCache: moduleCache, // GOMODCACHE; the caller finds it LocalRoot: scanRoot, // a local replace directive is read only inside it }) +if err != nil { + return err +} g, err := modgraph.Build(ctx, modgraph.Input{ MainFS: os.DirFS(projectDir), Modules: sources, Store: store, // optional cache of the packages of the module cache }) +if err != nil { + return err +} path := g.Path([]string{g.Main.Path}, "github.com/x/y") // nil when no path reaches it ``` @@ -93,6 +99,8 @@ false), can reach each module: the path ends at that node. `vendor/modules.txt` lists the module at its version, then the local directory of a `replace` directive, inside `LocalRoot` after links are resolved, then the module cache, then the `.mod` file of the module cache. +Each read of a local replacement resolves its links again, and a link that +leads out of `LocalRoot` makes the module open. A caller that finds the sources in another way can pass its own `[]Source`. `Module.Path` is the path that import statements use, also for a replaced module, and `Source.Origin` is the module that the source holds. @@ -131,3 +139,7 @@ with neither. It sets `GOPROXY=off`, so it downloads nothing. module open. - A file budget, 200,000 files by default, caps a run. Past it the module that is next is open, and `Stats.Truncated` is set. +- `Build` tests a build constraint one tag set at a time. A constraint of + more than 16 tags counts as one that can hold. After 4,194,304 tested + sets in one run, each constraint counts as one that can hold. So a + dependency of many such files cannot make a run slow. diff --git a/modgraph/discover.go b/modgraph/discover.go index 42854177..48ecdf2b 100644 --- a/modgraph/discover.go +++ b/modgraph/discover.go @@ -3,6 +3,7 @@ package modgraph import ( "bufio" "bytes" + "errors" "fmt" "io/fs" "os" @@ -91,8 +92,9 @@ func replacement(f *modfile.File, m gomodule.Version) gomodule.Version { } // localSource returns the source of a replacement with a local path, inside -// LocalRoot only. Both paths are resolved through links first, so a link -// cannot lead the read out of LocalRoot. +// LocalRoot only. Both paths are resolved through links first, and each read +// of the source resolves its links again, so a link cannot lead the read out +// of LocalRoot. func localSource(opts DiscoverOptions, p string) (fs.FS, []byte) { if opts.LocalRoot == "" { return nil, nil @@ -105,16 +107,39 @@ func localSource(opts DiscoverOptions, p string) (fs.FS, []byte) { if err != nil { return nil, nil } - if dir, err = filepath.EvalSymlinks(dir); err != nil { - return nil, nil - } - if rel, err := filepath.Rel(root, dir); err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + if dir, err = filepath.EvalSymlinks(dir); err != nil || !within(root, dir) { return nil, nil } if info, err := os.Stat(dir); err != nil || !info.IsDir() { return nil, nil } - return os.DirFS(dir), readOptional(filepath.Join(dir, "go.mod")) + return rootedFS{dir: dir, root: root}, readOptional(filepath.Join(dir, "go.mod")) +} + +var errOutsideRoot = errors.New("a link leads out of the local root") + +// rootedFS reads a directory as os.DirFS does, and refuses a name whose +// links resolve outside root. +type rootedFS struct{ dir, root string } + +func (r rootedFS) Open(name string) (fs.File, error) { + if !fs.ValidPath(name) { + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrInvalid} + } + resolved, err := filepath.EvalSymlinks(filepath.Join(r.dir, filepath.FromSlash(name))) + if err != nil { + return nil, &fs.PathError{Op: "open", Path: name, Err: err} + } + if !within(r.root, resolved) { + return nil, &fs.PathError{Op: "open", Path: name, Err: errOutsideRoot} + } + return os.Open(resolved) +} + +// within reports whether a resolved path is root or below it. +func within(root, p string) bool { + rel, err := filepath.Rel(root, p) + return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) } // cacheSource returns the extracted source of a module version in the diff --git a/modgraph/discover_test.go b/modgraph/discover_test.go index d88ecebf..37899db5 100644 --- a/modgraph/discover_test.go +++ b/modgraph/discover_test.go @@ -1,6 +1,7 @@ package modgraph import ( + "context" "io/fs" "os" "path/filepath" @@ -156,6 +157,51 @@ func TestDiscoverDoesNotFollowALinkOutOfTheLocalRoot(t *testing.T) { assert.Nil(t, sources[0].FS) } +func TestDiscoverConfinesEachReadOfALocalReplacement(t *testing.T) { + cases := []struct { + name string + imp string + link func(t *testing.T, lib, outside string) + wantOpen bool + }{ + {"a link to a file outside the root", "example.com/local", func(t *testing.T, lib, outside string) { + require.NoError(t, os.Symlink(filepath.Join(outside, "o.go"), filepath.Join(lib, "o.go"))) + }, true}, + {"a link to a package outside the root", "example.com/local/sub", func(t *testing.T, lib, outside string) { + require.NoError(t, os.Symlink(outside, filepath.Join(lib, "sub"))) + }, true}, + {"a link inside the root", "example.com/local", func(t *testing.T, lib, _ string) { + require.NoError(t, os.Symlink(filepath.Join(lib, "real.go"), filepath.Join(lib, "o.go"))) + }, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + root := t.TempDir() + outside := t.TempDir() + writeFiles(t, root, map[string]string{ + "app/go.mod": "module example.com/app\n\ngo 1.22\n\nrequire example.com/local v1.0.0\n\nreplace example.com/local => ./lib\n", + "app/main.go": "package main\n\nimport _ \"" + tc.imp + "\"\n", + "app/lib/go.mod": "module example.com/local\n", + "app/lib/real.go": "package local\n", + }) + writeFiles(t, outside, map[string]string{"o.go": "package local\n\nimport _ \"example.com/secret\"\n"}) + tc.link(t, filepath.Join(root, "app", "lib"), outside) + + dir := filepath.Join(root, "app") + sources, err := Discover(DiscoverOptions{Dir: dir, LocalRoot: root}) + require.NoError(t, err) + require.NotNil(t, sources[0].FS) + g, err := Build(context.Background(), Input{MainFS: os.DirFS(dir), Modules: sources}) + require.NoError(t, err) + n, ok := g.Node("example.com/local") + require.True(t, ok) + assert.Equal(t, tc.wantOpen, n.State == StateOpen, n.Reason) + assert.Equal(t, StateImports, node(t, g, "example.com/app").State) + assert.NotContains(t, n.Reason, "example.com/secret", "Build reads no file outside the root") + }) + } +} + func TestDiscoverNeedsAGoMod(t *testing.T) { _, err := Discover(DiscoverOptions{Dir: t.TempDir()}) assert.ErrorContains(t, err, "read go.mod") diff --git a/modgraph/files.go b/modgraph/files.go index 57966120..916b0479 100644 --- a/modgraph/files.go +++ b/modgraph/files.go @@ -27,6 +27,12 @@ const ( // maxConstraintTags caps the tags of a build constraint that Build tests // one by one. A constraint with more tags counts as one that can hold. maxConstraintTags = 16 + // maxConstraintSets caps the tag sets that one Build tests, over each + // constraint of each file. Without it a module of many files with + // unsatisfiable constraints of maxConstraintTags tags each costs 2^16 + // tests per file. Past the cap each constraint counts as one that can + // hold. + maxConstraintSets = 1 << 22 ) var errBudget = errors.New("the file budget ran out") @@ -83,9 +89,17 @@ func (b *builder) packageImports(ref pkgRef) ([]importRef, error) { continue } file := path.Join(ref.dir, e.Name()) - // The go command reads a link to a file, as a file of the package. + // The go command reads a link to a file, as a file of the package. A + // link that leads nowhere holds no file. Another error, such as a + // link that leads out of the local root, leaves the imports unknown. if !e.Type().IsRegular() { - if info, err := fs.Stat(ref.mod.src.FS, file); err != nil || !info.Mode().IsRegular() { + info, err := fs.Stat(ref.mod.src.FS, file) + switch { + case errors.Is(err, fs.ErrNotExist): + continue + case err != nil: + return nil, fmt.Errorf("stat %s: %w", file, err) + case !info.Mode().IsRegular(): continue } } @@ -137,7 +151,7 @@ func (b *builder) fileImports(fsys fs.FS, file string) (_ []importRef, err error if err != nil { return nil, unbuildable{fmt.Errorf("%s does not parse: %w", file, err)} } - if excluded(src) { + if excluded(src, &b.constraintSets) { return nil, nil } test := strings.HasSuffix(file, "_test.go") @@ -191,15 +205,15 @@ func importsEnd(fset *token.FileSet, f *ast.File, head []byte) bool { // line before the last blank line of the leading // comments must hold. A // constraint that does not parse, two //go:build lines, and a constraint // with more than maxConstraintTags tags leave the file in, so the graph -// loses no edge. -func excluded(src []byte) bool { +// loses no edge. So does a constraint once the budget of tag sets runs out. +func excluded(src []byte, budget *int) bool { header, goBuild, ok := fileHeader(src) if !ok { return false } if goBuild != "" { expr, err := constraint.Parse(goBuild) - return err == nil && !satisfiable(expr) + return err == nil && !satisfiable(expr, budget) } var all constraint.Expr for line := range strings.Lines(string(header)) { @@ -217,7 +231,7 @@ func excluded(src []byte) bool { all = &constraint.AndExpr{X: all, Y: expr} } } - return all != nil && !satisfiable(all) + return all != nil && !satisfiable(all, budget) } // fileHeader is go/build parseFileHeader. It returns the part of the file @@ -285,25 +299,24 @@ func isGoBuildComment(line []byte) bool { } // satisfiable reports whether some set of tags makes a constraint hold, -// with ignore false. -func satisfiable(expr constraint.Expr) bool { - var tags []string - seen := map[string]bool{} +// with ignore false. Each set that it tests takes one unit of the budget, +// and an empty budget reports true. +func satisfiable(expr constraint.Expr, budget *int) bool { + bit := map[string]int{} collectTags(expr, func(tag string) { - if tag != "ignore" && !seen[tag] { - seen[tag] = true - tags = append(tags, tag) + if _, ok := bit[tag]; !ok && tag != "ignore" { + bit[tag] = len(bit) } }) - if len(tags) > maxConstraintTags { + if len(bit) > maxConstraintTags { return true } - for set := 0; set < 1< example.com/dep", "past the tag cap the file counts") } +func TestSatisfiableStopsAtTheBudget(t *testing.T) { + expr, err := constraint.Parse("//go:build a && b && c && !(a || b || c)") + require.NoError(t, err) + budget := 1 << 10 + assert.False(t, satisfiable(expr, &budget)) + assert.Equal(t, 1<<10-8, budget, "each of the 8 sets takes one unit") + + budget = 3 + assert.True(t, satisfiable(expr, &budget), "an empty budget keeps the file") + assert.Zero(t, budget) +} + func TestBuildReadsEachImportOfALongFile(t *testing.T) { long := "package a\n\nimport _ \"example.com/first\"\n\n// " + strings.Repeat("x", 70<<10) + "\n\nimport _ \"example.com/dep\"\n" g := build(t, Input{ diff --git a/modgraph/oracle_test.go b/modgraph/oracle_test.go index 4d5eebdf..e6e4e4e4 100644 --- a/modgraph/oracle_test.go +++ b/modgraph/oracle_test.go @@ -77,12 +77,24 @@ func TestBuildHoldsEachEdgeOfGoList(t *testing.T) { for _, e := range g.Edges { edges[[2]string{e.From.Path, e.To.Path}] = true } + // An open node passes each edge, so an edge of go list must leave a + // module that has source, or the test compares nothing. + sourced := map[string]bool{g.Main.Path: true} + for _, s := range sources { + sourced[s.Module.Path] = s.FS != nil + } for _, p := range oraclePlatforms { - listed := goListEdges(t, dir, p[0], p[1]) + listed, dropped := goListEdges(t, dir, p[0], p[1]) t.Logf("%s/%s: go list has %d module edges, the graph %d, with %d nodes", p[0], p[1], len(listed), len(g.Edges), len(g.Nodes)) + if dir == root { + assert.Empty(t, dropped, "%s/%s: go list resolves each import of this repository", p[0], p[1]) + } else if len(dropped) > 0 { + t.Logf("%s/%s: go list resolves no module for %d imports, such as %s", p[0], p[1], len(dropped), dropped[0]) + } for e := range listed { from, ok := g.Node(e[0]) require.True(t, ok, "the graph has no node %s", e[0]) + require.True(t, sourced[e[0]], "go list reads %s, and Discover finds no source of it", e[0]) to, ok := g.Node(e[1]) assert.True(t, ok && to.Reached, "%s/%s: go list reaches %s, the graph does not", p[0], p[1], e[1]) assert.True(t, edges[e] || from.Open(), "%s/%s: go list has the edge %s -> %s", p[0], p[1], e[0], e[1]) @@ -96,7 +108,8 @@ func TestBuildHoldsEachEdgeOfGoList(t *testing.T) { } g, err = Build(context.Background(), Input{MainFS: os.DirFS(dir), Modules: sources}) require.NoError(t, err) - for e := range goListEdges(t, dir, "linux", "amd64") { + listed, _ := goListEdges(t, dir, "linux", "amd64") + for e := range listed { to, ok := g.Node(e[1]) assert.True(t, ok && to.Reached, "go.mod only: go list reaches %s, the graph does not", e[1]) } @@ -126,8 +139,10 @@ type listPackage struct { } // goListEdges returns the module edges of the packages that go list -deps -// -test gives for a platform. -func goListEdges(t *testing.T, dir, goos, goarch string) map[[2]string]bool { +// -test gives for a platform, and each import that go list resolves to no +// module. go list -e gives such an import with no package of a module, so +// it leaves no edge to compare. +func goListEdges(t *testing.T, dir, goos, goarch string) (map[[2]string]bool, []string) { t.Helper() out, err := goCommand(dir, []string{"GOOS=" + goos, "GOARCH=" + goarch, "CGO_ENABLED=1"}, "list", "-e", "-deps", "-test", "-json", "./...") require.NoError(t, err, "go list -deps") @@ -152,18 +167,24 @@ func goListEdges(t *testing.T, dir, goos, goarch string) map[[2]string]bool { } } edges := map[[2]string]bool{} + var dropped []string for _, p := range pkgs { from, ok := moduleOf[p.ImportPath] if !ok || from == StdPath { continue } for _, imp := range p.Imports { - if to, ok := moduleOf[imp]; ok && to != from && imp != "C" { + to, ok := moduleOf[imp] + switch { + case imp == "C": + case !ok: + dropped = append(dropped, p.ImportPath+" -> "+imp) + case to != from: edges[[2]string{from, to}] = true } } } - return edges + return edges, dropped } func goCommand(dir string, env []string, args ...string) ([]byte, error) { From 3f674e04b1e07023dabbdb784a33d5380d484fcd Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 19:34:17 +0000 Subject: [PATCH 5/5] fix(modgraph): miss a cache entry with no imports, refuse module std A cache entry of {} decoded as a package with no imports, so Build skipped the source and lost its edges. An entry with no imports field is now a miss, and Build writes [] for a package with no imports. A main module with the path std took the key of the standard library node. Build now refuses it, as it refuses std in the build list. The oracle test also checks that the module that an edge leaves is reached, and that the cache gives the same nodes. The example test decodes the JSON graph. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PFQbqPqd9xn1fT6hBsyoUm Signed-off-by: Claude --- examples/modgraph/main_test.go | 12 +++++++++++- modgraph/cache.go | 13 +++++++++---- modgraph/graph.go | 3 +++ modgraph/graph_test.go | 28 ++++++++++++++++++++++++++++ modgraph/oracle_test.go | 2 ++ 5 files changed, 53 insertions(+), 5 deletions(-) diff --git a/examples/modgraph/main_test.go b/examples/modgraph/main_test.go index fb6674ed..2dfcf468 100644 --- a/examples/modgraph/main_test.go +++ b/examples/modgraph/main_test.go @@ -1,6 +1,7 @@ package main import ( + "encoding/json" "os" "path/filepath" "strings" @@ -8,6 +9,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/safedep/code/modgraph" ) func TestRunPrintsTheGraph(t *testing.T) { @@ -30,5 +33,12 @@ func TestRunPrintsTheGraph(t *testing.T) { out.Reset() require.NoError(t, run(&out, dir, cache, true)) - assert.Contains(t, out.String(), `"Path": "example.com/dep"`) + var g modgraph.Graph + require.NoError(t, json.Unmarshal([]byte(out.String()), &g)) + dep, ok := g.Node("example.com/dep") + require.True(t, ok) + assert.True(t, dep.Reached) + assert.Equal(t, modgraph.StateImports, dep.State) + assert.True(t, g.Reaches([]string{"example.com/app"}, modgraph.StdPath)) + assert.Equal(t, []string{"example.com/app", "example.com/dep"}, g.Path([]string{"example.com/app"}, "example.com/dep")) } diff --git a/modgraph/cache.go b/modgraph/cache.go index 15b58143..83c48b18 100644 --- a/modgraph/cache.go +++ b/modgraph/cache.go @@ -20,7 +20,9 @@ var errCache = errors.New("modgraph: cache") // that count. Build stores an entry only after it read each file of the // package with no error, so an entry never stands for a part of a package. type cacheEntry struct { - Imports []importRef `json:"imports"` + // Imports is a pointer, so an entry with no imports field is a miss, not + // a package with no imports. + Imports *[]importRef `json:"imports"` } // cacheKey is the key of a package of an immutable source. Its path is the @@ -68,13 +70,13 @@ func (b *builder) cached(ref pkgRef) ([]importRef, bool, error) { return nil, false, nil } var e cacheEntry - if err := json.Unmarshal(data, &e); err != nil { + if err := json.Unmarshal(data, &e); err != nil || e.Imports == nil { // A bad entry is a miss. The read that follows writes a good one. return nil, false, nil } b.stats.Packages++ b.stats.CachedPackages++ - return e.Imports, true, nil + return *e.Imports, true, nil } // cache stores the imports of a package that Build read whole. @@ -82,7 +84,10 @@ func (b *builder) cache(ref pkgRef, imps []importRef) error { if !b.cacheable(ref) { return nil } - data, err := json.Marshal(cacheEntry{Imports: imps}) + if imps == nil { + imps = []importRef{} + } + data, err := json.Marshal(cacheEntry{Imports: &imps}) if err != nil { return fmt.Errorf("encode the cache entry of %s: %w", ref.dir, err) } diff --git a/modgraph/graph.go b/modgraph/graph.go index fe7448f3..da9fe07c 100644 --- a/modgraph/graph.go +++ b/modgraph/graph.go @@ -267,6 +267,9 @@ func Build(ctx context.Context, in Input) (*Graph, error) { if mf.Module == nil || mf.Module.Mod.Path == "" { return nil, errors.New("modgraph: the go.mod of the main module has no module directive") } + if mf.Module.Mod.Path == StdPath { + return nil, fmt.Errorf("modgraph: the main module holds the reserved path %q", StdPath) + } if mf.Go == nil || version.Compare("go"+mf.Go.Version, "go1.17") < 0 { return nil, ErrUnpruned } diff --git a/modgraph/graph_test.go b/modgraph/graph_test.go index a4e43149..acf2b662 100644 --- a/modgraph/graph_test.go +++ b/modgraph/graph_test.go @@ -491,6 +491,7 @@ func TestBuildRejectsABadInput(t *testing.T) { {"no module directive", Input{MainFS: withGoMod(nil, "go 1.22\n")}, "no module directive"}, {"a module twice", Input{MainFS: mainFS(nil), Modules: []Source{source("example.com/a", nil), source("example.com/a", nil)}}, "twice"}, {"the reserved std path", Input{MainFS: mainFS(nil), Modules: []Source{source(StdPath, nil)}}, "reserved"}, + {"the reserved std path as the main module", Input{MainFS: withGoMod(nil, "module std\n\ngo 1.22\n")}, "reserved"}, {"a module with no path", Input{MainFS: mainFS(nil), Modules: []Source{{}}}, "no path"}, {"the main module in the build list", Input{MainFS: mainFS(nil), Modules: []Source{source(mainPath, nil)}}, "twice"}, } @@ -571,6 +572,33 @@ func TestBuildCachesAReplacementByItsOwnModule(t *testing.T) { assert.Contains(t, edgeList(g), "example.com/a -> example.com/dep", "the fork is not served the imports of the original") } +func TestBuildReadsTheSourceForACacheEntryWithNoImports(t *testing.T) { + st := memstore.New() + a := Source{Module: Module{Path: "example.com/a", Version: "v1.0.0"}, FS: fstest.MapFS{"a.go": goSrc("a", "example.com/dep")}, Immutable: true} + in := Input{ + MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a")}), + Modules: []Source{a, source("example.com/dep", fstest.MapFS{"d.go": goSrc("dep")})}, + Store: st, + } + key := cacheKey(pkgRef{mod: &module{src: a}, dir: "."}) + require.NoError(t, st.PutArtifacts(context.Background(), []store.Artifact{{Key: key, Data: []byte("{}")}})) + + g := build(t, in) + assert.Contains(t, edgeList(g), "example.com/a -> example.com/dep") + assert.Equal(t, 0, g.Stats.CachedPackages) +} + +func TestBuildCachesAPackageWithNoImports(t *testing.T) { + st := memstore.New() + in := Input{ + MainFS: mainFS(fstest.MapFS{"main.go": goSrc("main", "example.com/a")}), + Modules: []Source{{Module: Module{Path: "example.com/a", Version: "v1.0.0"}, FS: fstest.MapFS{"a.go": goSrc("a")}, Immutable: true}}, + Store: st, + } + build(t, in) + assert.Equal(t, 1, build(t, in).Stats.CachedPackages) +} + func TestBuildCachesNoPackageThatFailed(t *testing.T) { st := memstore.New() bad := Source{Module: Module{Path: "example.com/a", Version: "v1.0.0"}, FS: fstest.MapFS{"a.go": {Data: []byte("package a\nimport (\n")}}, Immutable: true} diff --git a/modgraph/oracle_test.go b/modgraph/oracle_test.go index e6e4e4e4..42aad3de 100644 --- a/modgraph/oracle_test.go +++ b/modgraph/oracle_test.go @@ -66,6 +66,7 @@ func TestBuildHoldsEachEdgeOfGoList(t *testing.T) { warm, err := Build(context.Background(), Input{MainFS: os.DirFS(dir), Modules: sources, Store: st}) require.NoError(t, err) assert.Equal(t, g.Edges, warm.Edges, "the cache gives the same graph") + assert.Equal(t, g.Nodes, warm.Nodes, "the cache gives the same nodes") t.Logf("Build: %d files, %d packages, %v; with the cache: %d files, %d cached packages, %v", g.Stats.Files, g.Stats.Packages, cold.Round(time.Millisecond), warm.Stats.Files, warm.Stats.CachedPackages, time.Since(start).Round(time.Millisecond)) for _, n := range g.Nodes { @@ -95,6 +96,7 @@ func TestBuildHoldsEachEdgeOfGoList(t *testing.T) { from, ok := g.Node(e[0]) require.True(t, ok, "the graph has no node %s", e[0]) require.True(t, sourced[e[0]], "go list reads %s, and Discover finds no source of it", e[0]) + assert.True(t, from.Reached, "%s/%s: go list reaches %s, the graph does not", p[0], p[1], e[0]) to, ok := g.Node(e[1]) assert.True(t, ok && to.Reached, "%s/%s: go list reaches %s, the graph does not", p[0], p[1], e[1]) assert.True(t, edges[e] || from.Open(), "%s/%s: go list has the edge %s -> %s", p[0], p[1], e[0], e[1])