diff --git a/CHANGELOG.md b/CHANGELOG.md index 812a0c84..54ed0ddf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,15 @@ With a Go test target the line is a `run_task(slot:"test", target:…, run:…)` call whose filter `run_task` accepts, with the rest counted. Tests that only sit in the package are never named, and a stale index names none. +- **`context_for_task` says when a seed's callers are complete.** The index now + keeps the call resolver's outcome for every recorded Go call (topology schema + 8, so the index rebuilds once), and each Go function or method seed, and each + top declaration of a file seed, gets a caller line counted from the index, not + from the walk: either "complete (every recorded call of X resolves)", or the + exact unresolved calls of the name that may still be callers, with their files + and lines, so an agent checks those instead of searching the tree. Checked + against the Go type checker's static callers on this repository and on + golang.org/x/tools: no "complete" claim missed a caller. - **The Go call graph resolves method calls on local variables of evident type.** `x.M()` now links to the method of `x`'s type when the enclosing function declares `x` once as `x := T{}`, `&T{}`, `new(T)`, `var x T`, `x := y.(T)`, or @@ -251,6 +260,13 @@ ### Fixed +- **The Go call graph no longer guesses between two same-named declarations in + a file, and links generic calls.** The extractor's same-file call edges matched + by name alone, so with two methods called `Close` in one file, a call of either + became an edge to whichever was declared last, at full confidence; such calls + now get no edge and are reported as open. A call with explicit type arguments, + `F[T](x)`, had no same-file edge at all. + - **A revoked resume credential no longer breaks continuity for good.** A `plumb serve` proxy keeps the resume credential it was given and presents it when a replacement serve takes over the conversation. If the daemon refused it diff --git a/docs/tools.md b/docs/tools.md index f4b2c8b6..92d0dad1 100644 --- a/docs/tools.md +++ b/docs/tools.md @@ -1072,6 +1072,22 @@ marked "not a resolved caller"; an empty, short or cut caller list is never reported as "no callers", and a walk that was cut says so before it says anything about the index. +**Caller completeness (Go).** The index keeps the call resolver's outcome for +every recorded call, so for each Go function or method seed, and each top +declaration of a file seed, the pack states its callers against the index rather +than the walk: `callers of X: complete (every recorded call of X resolves): N in +the index, M listed here` when no call of the name is left open, or `N in the +index, M listed here; K unresolved call(s) of X may also be it: a.go:12 +(receiver), …` naming the open calls that could be this declaration (a receiver +whose type no declaration states, a bare name in the same package or in a file +that dot-imports one, a name the calling function also declares in some block, a +same-file call where the file declares the name twice, a call outside any +function), so the caller checks those sites rather than the whole tree. Calls +only: a function used as a value, or a method called through an interface, is +not a call of it, and the pack says so once. Checked against the Go type +checker's static callers on this repository and on golang.org/x/tools, no +"complete" claim missed a caller. + **Same-package callers.** The Go call resolver links bare same-package calls, calls on a method's own receiver and calls on a typed parameter, so those callers arrive as resolved `e2` edges. A test caller is one of them, and at most 12 test diff --git a/internal/tools/context_census.go b/internal/tools/context_census.go new file mode 100644 index 00000000..cdbd4649 --- /dev/null +++ b/internal/tools/context_census.go @@ -0,0 +1,213 @@ +package tools + +import ( + "context" + "fmt" + "path" + "slices" + "strings" + "unicode" + + "github.com/plumbkit/plumb/internal/topology" +) + +// context_census.go — whether a seed's caller list is the whole list. +// +// A ranked caller list never lets a reader stop: "3 callers, more may exist" sends +// an agent back to grep for the name, and the pack is paid for twice. The index +// records every call expression and, per call, what the resolver made of it, so it +// can say which calls of the seed's name are still open. None open means every +// recorded call of the name resolved, to this declaration or another, and the +// list is complete as far as calls go; some open means exactly those sites are what +// is left to check, and the pack names them. +// +// What it cannot see is said with the claim: a function used as a value, and a +// method called through an interface, are not calls of the declaration. + +// contextCensusSites is how many open sites a seed's line names. +const contextCensusSites = 6 + +// census fills c from the index's per-call outcomes for a Go function or method +// seed. It leaves c alone for anything else, or when the index cannot answer, and +// the older wording stands. +func (e *expander) census(ctx context.Context, n topology.Node, c *callerNote) { + // Asked afresh rather than through the walk's cache: a walk cut by its deadline + // caches "not admitted" for the nodes it never reached. + if n.Language != "go" || !e.store.DerivedCallsAdmittedFor(ctx, n.ID) { + return + } + open, _, err := e.store.OpenCallSites(ctx, n.Name, "go", 0) + if err != nil { + e.fail(ctx, err) + return + } + res, err := e.store.ImpactFrom(ctx, n, contextTraversalOpts(true, []string{string(topology.EdgeCalls)})) + if err != nil { + e.fail(ctx, err) + return + } + var ok bool + if c.Indexed, c.IndexedOutside, ok = callerCounts(res.DependedOnBy, n.ID, e.inScope); !ok { + return + } + c.Census, c.Method, c.Name = true, n.Kind == topology.KindMethod, n.Name + c.Unclaimable = e.censusDoubt(ctx) + seed := censusSeed{method: c.Method, exported: isExportedName(n.Name), dir: path.Dir(n.Path), pkg: e.packageName(ctx, n.Path)} + for _, o := range open { + if !couldCall(o, seed) { + continue + } + if !e.inScope(o.Path) { + c.OpenOutside++ // counted, never named: the scope leaves the file out + continue + } + noteOpen(c, fmt.Sprintf("%s:%d (%s)", o.Path, o.Line, o.Kind)) + } +} + +// inScope is the scope filter without its side effect of recording an exclusion: +// a census counts what lies outside, it does not list it. +func (e *expander) inScope(p string) bool { + if plumbStateReason(p) != "" { + return false + } + ok, _ := e.scope.allows(p, corpusOfPath(p)) + return ok +} + +// censusDoubt says why no caller list may be called complete however the census +// comes out, or "" when nothing stands in the way. A call in a file the index did +// not record is invisible to it: the index is behind the files (not fresh), or a +// Go file failed to extract (a parse error mid-edit, a file over the size cap). +func (e *expander) censusDoubt(ctx context.Context) string { + if f := indexFreshness(e.store.Health()); f != "fresh" { + return "the index is " + f + } + n, err := e.store.UnreadFiles(ctx, ".go") + switch { + case err != nil: + return "the index could not say which Go files it failed to read" + case n > 0: + return fmt.Sprintf("%d Go file(s) could not be read by the index, so their calls are not recorded", n) + } + return "" +} + +// packageName is a Go file's package clause, or "" when the index has none. +func (e *expander) packageName(ctx context.Context, p string) string { + n, _ := e.packageOf(ctx, p) + return n.Name +} + +// callerCounts counts a declaration's resolved callers in an inward +// neighbourhood, inside and outside the scope. ok is false when the +// neighbourhood was cut at the per-hop ceiling: a cut list gives no count to state. +func callerCounts(nb *topology.Neighbourhood, self int64, inScope func(string) bool) (in, out int, ok bool) { + if nb == nil { + return 0, 0, true + } + if nb.Truncated { + return 0, 0, false + } + for _, m := range nb.Nodes { + switch { + case m.ID == self: + case inScope(m.Path): + in++ + default: + out++ + } + } + return in, out, true +} + +// noteOpen records one open call at its place. Two calls on one line are one place +// to look, and at most contextCensusSites places are named; OpenShown counts the +// calls the named places hold, so "and k more" counts only the rest. +func noteOpen(c *callerNote, at string) { + c.Open++ + if !slices.Contains(c.OpenAt, at) { + if len(c.OpenAt) == contextCensusSites { + return + } + c.OpenAt = append(c.OpenAt, at) + } + c.OpenShown++ +} + +// censusSeed is what couldCall needs to know of the declaration. +type censusSeed struct { + method, exported bool + dir, pkg string // its directory and package clause +} + +// couldCall reports whether an open call of the seed's name could be a call of +// the seed. A bare call can be a function only, from the function's own package +// or, for an exported function, from a file that dot-imports a package; a +// receiver or unmatched qualified call can be a method only; an external call can +// be an exported function whose package clause its qualifier names (the import +// path may simply have failed to map onto the directory); an unexported name is +// callable from its own package only. Anything else could be either. It errs +// towards open: an open site that is not a call of the seed costs a reader a look, +// a missed one makes "complete" false. +func couldCall(o topology.OpenCall, seed censusSeed) bool { + samePkg := path.Dir(o.Path) == seed.dir + if !seed.exported && !samePkg { + return false + } + switch o.Kind { + case "bare", "local": + return !seed.method && (samePkg || o.DotImport) + case "receiver", "unmatched": + return seed.method + case "external": + return !seed.method && seed.pkg != "" && o.Qualifier == seed.pkg + } + return true +} + +func isExportedName(name string) bool { + for _, r := range name { + return unicode.IsUpper(r) + } + return false +} + +// censusText words a seed's caller list with what the census found. Its counts +// are the index's, not the walk's, so a cut walk cannot shrink them; the caveat +// every census line shares (labelCensus) is said once, after them. "Complete" needs +// no open call of the name anywhere, inside the scope or outside it, and nothing +// that would hide one (censusDoubt). +func (c callerNote) censusText() string { + here := fmt.Sprintf("%d in the index, %d listed here%s", c.Indexed, c.Resolved, c.matched()) + if c.IndexedOutside > 0 { + here += fmt.Sprintf(" (and %d outside the scope)", c.IndexedOutside) + } + outside := "" + if c.OpenOutside > 0 { + outside = fmt.Sprintf("; %d unresolved call(s) of %s outside the scope are not listed", c.OpenOutside, c.Name) + } + if c.Open == 0 { + switch { + case c.Unclaimable != "": + return fmt.Sprintf("callers of %s: %s%s; not claimed complete: %s", c.Selector, here, outside, c.Unclaimable) + case c.OpenOutside > 0: + return fmt.Sprintf("callers of %s: %s%s, so the list is not complete", c.Selector, here, outside) + } + rest := "" + if c.Indexed > c.Resolved { + rest = "; call_hierarchy lists the rest" + } + return fmt.Sprintf("callers of %s: complete (every recorded call of %s resolves): %s%s", c.Selector, c.Name, here, rest) + } + more := "" + if c.Open > c.OpenShown { + more = fmt.Sprintf(" and %d more", c.Open-c.OpenShown) + } + doubt := "" + if c.Unclaimable != "" { + doubt = "; and " + c.Unclaimable + } + return fmt.Sprintf("callers of %s: %s; %d unresolved call(s) of %s may also be it: %s%s%s%s. Check these, not the whole tree", + c.Selector, here, c.Open, c.Name, strings.Join(c.OpenAt, ", "), more, outside, doubt) +} diff --git a/internal/tools/context_census_test.go b/internal/tools/context_census_test.go new file mode 100644 index 00000000..11a21b40 --- /dev/null +++ b/internal/tools/context_census_test.go @@ -0,0 +1,181 @@ +package tools + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/plumbkit/plumb/internal/topology" +) + +// TestCouldCall pins which open calls of a name could be calls of a declaration +// in internal/lib. Each false row is a call a "complete" claim may ignore; each +// true row is one it must not. +func TestCouldCall(t *testing.T) { + const dir = "internal/lib" + call := func(p, kind string, dot bool) topology.OpenCall { + return topology.OpenCall{Path: p, Kind: kind, DotImport: dot} + } + for _, c := range []struct { + name string + o topology.OpenCall + method, exported bool + want bool + }{ + {"bare call of a function, own package", call("internal/lib/a.go", "bare", false), false, true, true}, + {"bare call of a function, another package", call("internal/app/a.go", "bare", false), false, true, false}, + {"bare call through a dot import", call("internal/app/a.go", "bare", true), false, true, true}, + {"dot import cannot reach an unexported name", call("internal/app/a.go", "bare", true), false, false, false}, + {"bare call is never a method", call("internal/lib/a.go", "bare", false), true, true, false}, + {"block-scoped local may still be the function", call("internal/lib/a.go", "local", false), false, false, true}, + {"receiver call of a method", call("internal/app/a.go", "receiver", false), true, true, true}, + {"receiver call is never a function", call("internal/app/a.go", "receiver", false), false, true, false}, + {"unexported method from another package", call("internal/app/a.go", "receiver", false), true, false, false}, + {"package value's method (unmatched)", call("internal/app/a.go", "unmatched", false), true, true, true}, + {"top-level call could be either", call("internal/app/a.go", "top-level", false), false, true, true}, + {"same-file ambiguity could be either", call("internal/lib/a.go", "same-file", false), true, false, true}, + {"unread call could be either", call("internal/app/a.go", "unresolved", false), true, true, true}, + {"stale outcome could be either", call("internal/app/a.go", "stale", false), true, true, true}, + {"external call naming the seed's package", topology.OpenCall{Path: "internal/app/a.go", Kind: "external", Qualifier: "lib"}, false, true, true}, + {"external call naming another package", topology.OpenCall{Path: "internal/app/a.go", Kind: "external", Qualifier: "fmt"}, false, true, false}, + {"external call is never a method", topology.OpenCall{Path: "internal/app/a.go", Kind: "external", Qualifier: "lib"}, true, true, false}, + } { + if got := couldCall(c.o, censusSeed{method: c.method, exported: c.exported, dir: dir, pkg: "lib"}); got != c.want { + t.Errorf("%s: couldCall = %v, want %v", c.name, got, c.want) + } + } +} + +// TestContextForTask_CensusCoversAFileSeedsDeclarations: agents mostly pass files, +// so a file seed's top declarations get the census too, with the shared caveat +// said once. Control: the open calls of Total are named, not claimed complete. +func TestContextForTask_CensusCoversAFileSeedsDeclarations(t *testing.T) { + s := newShop(t) + out, err := s.run(t, map[string]any{"files": []string{"cart/cart.go"}, "max_bytes": 32000}) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, "callers of (*Cart).Total: 0 in the index") || + !strings.Contains(out, "unresolved call(s) of Total may also be it: api/checkout.go:13 (receiver)") { + t.Errorf("a file seed's declaration has no census line:\n%s", out) + } + if n := strings.Count(out, labelCensus); n != 1 { + t.Errorf("the census caveat appears %d times, want once:\n%s", n, out) + } + if strings.Contains(out, "callers of (*Cart).Total: complete") { + t.Errorf("open calls of Total were claimed complete:\n%s", out) + } +} + +// TestCallerCounts: a neighbourhood cut at the per-hop ceiling gives no count at +// all, never a short one; the declaration itself (recursion) is not its caller; +// callers the scope leaves out are counted apart. +func TestCallerCounts(t *testing.T) { + in := func(p string) bool { return strings.HasPrefix(p, "a/") } + nb := &topology.Neighbourhood{Nodes: []topology.Node{{ID: 1, Path: "a/x.go"}, {ID: 2, Path: "a/y.go"}, {ID: 3, Path: "b/z.go"}, {ID: 9, Path: "a/self.go"}}} + if i, o, ok := callerCounts(nb, 9, in); !ok || i != 2 || o != 1 { + t.Errorf("callerCounts = %d in, %d out, ok %v; want 2, 1, true", i, o, ok) + } + nb.Truncated = true + if _, _, ok := callerCounts(nb, 9, in); ok { + t.Error("a truncated neighbourhood gave a count") + } + if i, o, ok := callerCounts(nil, 9, in); !ok || i+o != 0 { + t.Errorf("no neighbourhood: %d, %d, %v; want 0, 0, true", i, o, ok) + } +} + +// TestNoteOpen: two calls on one line are one place, the named places +// stop at the cap, and "and k more" counts only the calls no named place holds. +func TestNoteOpen(t *testing.T) { + var c callerNote + noteOpen(&c, "a.go:1 (receiver)") + noteOpen(&c, "a.go:1 (receiver)") + for i := range contextCensusSites { + noteOpen(&c, fmt.Sprintf("b.go:%d (receiver)", i+2)) + } + if c.Open != contextCensusSites+2 || len(c.OpenAt) != contextCensusSites || c.OpenShown != contextCensusSites+1 { + t.Errorf("Open %d, places %d, shown %d; want %d, %d, %d", c.Open, len(c.OpenAt), c.OpenShown, contextCensusSites+2, contextCensusSites, contextCensusSites+1) + } + c.Census, c.Name, c.Selector = true, "M", "(*T).M" + if txt := c.censusText(); !strings.Contains(txt, " and 1 more") || strings.Count(txt, "a.go:1") != 1 { + t.Errorf("a place is listed twice, or the rest is miscounted: %s", txt) + } +} + +const censusModule = "module example.com/c\n\ngo 1.22\n" + +// TestContextForTask_CensusRespectsTheScope (review B1): an open call the scope +// leaves out is counted, never named, and the list is not called complete while +// one exists. Control: without the scope the call is named. +func TestContextForTask_CensusRespectsTheScope(t *testing.T) { + s := newModuleWorkspace(t, map[string]string{ + "go.mod": censusModule, + "p/s.go": "package p\n\n// S is seeded.\ntype S struct{}\n\n// Seed is called from q through an interface value.\nfunc (s *S) Seed() {}\n", + "q/use.go": "package q\n\n// Use calls Seed on a value no declaration types.\nfunc Use(x interface{ Seed() }) {\n\tx.Seed()\n}\n", + }) + args := map[string]any{"symbols": []string{"p/s.go#S.Seed"}} + whole, err := s.run(t, args) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(whole, "1 unresolved call(s) of Seed may also be it: q/use.go:5 (receiver)") { + t.Fatalf("control: the open call is not named without a scope:\n%s", whole) + } + args["within"] = []string{"p"} + scoped, err := s.run(t, args) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(scoped, "1 unresolved call(s) of Seed outside the scope are not listed, so the list is not complete") || + strings.Contains(scoped, "callers of (*S).Seed: complete") || strings.Contains(scoped, "q/use.go") { + t.Errorf("an open call outside the scope must be counted, not named, and must stop \"complete\":\n%s", scoped) + } +} + +// TestContextForTask_CensusNeedsEveryGoFileRead (review S1): a Go file the index +// failed to read (here an unreadable one) hides its calls, so no list is called +// complete while one exists. Control: the same workspace without it says complete. +// A file with a syntax error is not such a file: the Go extractor records what it +// parses, and its calls with it. A file over the size cap is never recorded at all, +// which only the caveat (labelCensus: "in the files the index holds") covers. +func TestContextForTask_CensusNeedsEveryGoFileRead(t *testing.T) { + files := map[string]string{ + "go.mod": censusModule, + "p/lone.go": "package p\n\n// Lone is called once, from Caller.\nfunc Lone() int { return 1 }\n", + "p/other.go": "package p\n\n// Caller calls Lone.\nfunc Caller() int { return Lone() }\n", + } + open := func(files map[string]string, unreadable string) shopTool { + root := t.TempDir() + writeFiles(t, root, files) + if unreadable != "" { + p := filepath.Join(root, filepath.FromSlash(unreadable)) + if err := os.Chmod(p, 0); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(p, 0o644) }) + if _, err := os.ReadFile(p); err == nil { + t.Skip("running as a user who can read a mode-0 file (root): the case cannot be built") + } + } + return newShopTool(t, openContextStore(t, root, 2), root) + } + args := map[string]any{"symbols": []string{"p/lone.go#Lone"}} + clean, err := open(files, "").run(t, args) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(clean, "callers of Lone: complete (every recorded call of Lone resolves): 1 in the index") { + t.Fatalf("control: a fully read workspace should be complete:\n%s", clean) + } + files["p/hidden.go"] = "package p\n\n// Hidden calls Lone, but the index cannot read it.\nfunc Hidden() int { return Lone() }\n" + big, err := open(files, "p/hidden.go").run(t, args) + if err != nil { + t.Fatal(err) + } + if strings.Contains(big, "callers of Lone: complete") || !strings.Contains(big, "not claimed complete: 1 Go file(s) could not be read") { + t.Errorf("an unread Go file must stop \"complete\":\n%s", big) + } +} diff --git a/internal/tools/context_expand_scope_test.go b/internal/tools/context_expand_scope_test.go index b185e5fd..0d2b35ad 100644 --- a/internal/tools/context_expand_scope_test.go +++ b/internal/tools/context_expand_scope_test.go @@ -225,8 +225,10 @@ func TestContextForTask_NodeCapIsSixtyAndTheCutIsDeterministic(t *testing.T) { if !strings.Contains(out, "20 further candidate(s) beyond the 60-node, depth-2 cap were not kept") { t.Errorf("the cut is not disclosed:\n%s", out) } - if !strings.Contains(out, "callers of Run: none reached before the walk was cut") || strings.Contains(out, "none resolved in the index") { - t.Errorf("a capped walk must say it was cut, not speak of callers as if the index had been asked in full:\n%s", out) + // The caller line's counts are the index's own (the census), so a cut walk + // cannot shrink them: Run is called nowhere, and every call of its name resolves. + if !strings.Contains(out, "callers of Run: complete (every recorded call of Run resolves): 0 in the index, 0 listed here") { + t.Errorf("the caller line must state the index's census, whatever the walk listed:\n%s", out) } } diff --git a/internal/tools/context_expand_test.go b/internal/tools/context_expand_test.go index 617825ff..48faf94e 100644 --- a/internal/tools/context_expand_test.go +++ b/internal/tools/context_expand_test.go @@ -201,8 +201,12 @@ func TestContextForTask_C01_CalleesAreDerivedEvidenceAndTheGraphIsDisclosed(t *t t.Errorf("the Go-only, syntactic call graph is not disclosed:\n%s", out) } requireNeverNoCallers(t, out) - if !strings.Contains(out, "callers of (*Cart).Total: none resolved in the index") { - t.Errorf("an empty caller list is not worded as an absence of resolution:\n%s", out) + // An empty resolved list with open calls of the name is never "complete": it + // names the calls that may be callers, so the reader checks those and no more. + if !strings.Contains(out, "callers of (*Cart).Total: 0 in the index, 0 listed here") || + !strings.Contains(out, "5 unresolved call(s) of Total may also be it: api/checkout.go:13 (receiver)") || + strings.Contains(out, "callers of (*Cart).Total: complete") { + t.Errorf("an empty caller list with open calls must name them and not claim completeness:\n%s", out) } for _, r := range pack.Related { if r.Node.Path == "store/ledger.go" { diff --git a/internal/tools/context_gap.go b/internal/tools/context_gap.go index 3b6a3bbc..ba3b28b7 100644 --- a/internal/tools/context_gap.go +++ b/internal/tools/context_gap.go @@ -39,6 +39,7 @@ const ( labelIndexFailing = "index unavailable or failing: no relationship or test-impact claims; body from a disk snapshot" labelStructural = "structural only: relationships come from the topology index; no language server was consulted" labelStale = "file changed since indexing: relationships may be stale" + labelCensus = "caller completeness counts calls in the files the index holds only: a function used as a value, or a method called through an interface, is not a call of it" ) func callableKind(k topology.NodeKind) bool { @@ -208,6 +209,26 @@ type callerNote struct { Selector string Resolved int NameMatched int // same-package name-match callers listed (heuristic, never counted as resolved) + + // Census is set when the index's per-call outcomes were read for the seed's name + // (Go only): Indexed is its resolved callers in the index, Open how many recorded + // calls of the name the resolver left open that could be it, and OpenAt where + // the first of them are. + Census bool + Method bool + Name string + Indexed int // resolved callers the index holds in the scope, however many the walk listed + // IndexedOutside and OpenOutside are the resolved callers and open calls the + // scope leaves out: counted, never named. + IndexedOutside int + OpenOutside int + // Unclaimable says why the list may not be called complete whatever the count + // (censusDoubt), "" when nothing stands in the way. + Unclaimable string + Open int + OpenAt []string // distinct places, at most contextCensusSites + // OpenShown is how many of the Open calls the places in OpenAt hold. + OpenShown int } // languageNote is what the call graph covers for one language in the pack. @@ -257,7 +278,19 @@ func (e *expander) summarise(ctx context.Context, related []contextRelated) cont for _, s := range e.seeds { note(s.node) if callableKind(s.node.Kind) { - x.Callers = append(x.Callers, e.callerNote(s, related)) + c := e.callerNote(s, related) + e.census(ctx, s.node, &c) + x.Callers = append(x.Callers, c) + } + } + // A file seed's top declarations are what the caller will edit, so their caller + // lists get the census too; the file seed itself has no caller list to word. + for _, r := range e.kept { + if r.Root && !r.Withheld && callableKind(r.Node.Kind) { + c := e.callerNote(hopNode{node: r.Node, sel: nodeSelector(r.Node)}, related) + if e.census(ctx, r.Node, &c); c.Census { + x.Callers = append(x.Callers, c) + } } } for _, r := range related { @@ -312,8 +345,13 @@ func (p *contextPack) relationGaps() []string { if x.NameMatches > 0 { gaps = append(gaps, labelSamePackage) } + census := false for _, c := range x.Callers { gaps = append(gaps, c.text(x.Stats.capped())) + census = census || c.Census + } + if census { + gaps = append(gaps, labelCensus) } if x.Stats.MembersListed { gaps = append(gaps, labelMembers) @@ -335,6 +373,9 @@ func languageGap(l languageNote) string { // the index holds: an empty list from a walk that stopped early reports where the // walk got to, not what the index knows. func (c callerNote) text(capped bool) string { + if c.Census { + return c.censusText() + } switch { case capped && c.Resolved == 0: return fmt.Sprintf("callers of %s: none reached before the walk was cut (a cap, the deadline or an index error stopped it), so the index was not fully asked; this is not proof of no callers", c.Selector) diff --git a/internal/tools/context_lsp_test.go b/internal/tools/context_lsp_test.go index 1929590b..c9c06891 100644 --- a/internal/tools/context_lsp_test.go +++ b/internal/tools/context_lsp_test.go @@ -224,7 +224,7 @@ func TestContextForTask_LSP_AnAnsweringServerAddsAndConfirmsEdgesAtEvidence4(t * "related (", "e4 language-server edge, e3 extractor edge", " [e4 lsp] Checkout — api/checkout.go:11 function; caller of (*Cart).Total (lsp 1.0)", " [e4 lsp] Apply — pricing/discount.go:19 function; callee of (*Cart).Total (lsp 1.0)", - "callers of (*Cart).Total: 1 resolved and listed", + "callers of (*Cart).Total: 0 in the index, 1 listed here", } { if !strings.Contains(out, want) { t.Errorf("output lacks %q:\n%s", want, out) diff --git a/internal/tools/context_review_test.go b/internal/tools/context_review_test.go index a7048d0e..8e1493b2 100644 --- a/internal/tools/context_review_test.go +++ b/internal/tools/context_review_test.go @@ -297,9 +297,9 @@ func TestContextForTask_ASensitiveCallerIsNotAGapSubject(t *testing.T) { } } -// NIT 1: an empty caller list from a walk that was cut reports the cut, not what the -// index holds. Positive control: the same seed, uncut, says none were resolved in the -// index. +// NIT 1, under the census: a walk that was cut still states the index's own caller +// counts, which the cut cannot shrink, and the cut itself is disclosed beside them. +// Control: the same seed uncut gives the same caller line. func TestContextForTask_ACutWalkDoesNotClaimTheIndexHasNoCallers(t *testing.T) { s := newShop(t) args := map[string]any{"symbols": []string{"cart/cart.go#Cart.Total"}} @@ -307,16 +307,20 @@ func TestContextForTask_ACutWalkDoesNotClaimTheIndexHasNoCallers(t *testing.T) { if err != nil { t.Fatal(err) } - if !strings.Contains(whole, "callers of (*Cart).Total: none resolved in the index") { - t.Fatalf("control: the uncut walk should speak of the index:\n%s", whole) + const line = "callers of (*Cart).Total: 0 in the index, 0 listed here" + if !strings.Contains(whole, line) { + t.Fatalf("control: the uncut walk should state the census:\n%s", whole) } s.collector.deadline = time.Nanosecond cut, err := s.run(t, args) if err != nil { t.Fatal(err) } - if !strings.Contains(cut, "callers of (*Cart).Total: none reached before the walk was cut") || strings.Contains(cut, "none resolved in the index") { - t.Errorf("a cut walk must say it was cut, and not describe the index:\n%s", cut) + if !strings.Contains(cut, line) || !strings.Contains(cut, "unresolved call(s) of Total may also be it") { + t.Errorf("a cut walk must still state the index's census and the open calls:\n%s", cut) + } + if strings.Contains(cut, "callers of (*Cart).Total: complete") { + t.Errorf("a cut walk must not turn open calls into a complete list:\n%s", cut) } } diff --git a/internal/tools/context_samepkg_test.go b/internal/tools/context_samepkg_test.go index db3968df..19dd1306 100644 --- a/internal/tools/context_samepkg_test.go +++ b/internal/tools/context_samepkg_test.go @@ -153,7 +153,7 @@ func TestSamePackage_AMethodIsCalledByNameFromSiblingFiles(t *testing.T) { for _, want := range []string{ "[e1 heuristic] Savings — p/b.go:4 function; caller of (*Calc).Apply (same-package name match) — ", "TestApply — p/c_test.go:5 test; caller of (*Calc).Apply (same-package name match, test)", - "callers of (*Calc).Apply: none resolved in the index, plus 2 same-package name match(es) (e1); this is not proof of no callers", + "callers of (*Calc).Apply: 0 in the index, 0 listed here, plus 2 same-package name match(es) (e1); 3 unresolved call(s) of Apply may also be it: p/b.go:6 (receiver), p/c_test.go:5 (receiver), p/external_test.go:11 (receiver)", labelSamePackage, } { if !strings.Contains(out, want) { @@ -313,7 +313,7 @@ func TestSamePackage_AMethodNeedsReceiverEvidenceInTheCaller(t *testing.T) { t.Fatal(err) } // The count beside the resolved callers is of callers, not of candidates. - if want := "callers of (*A).Run: 2 resolved and listed, plus 5 same-package name match(es) (e1)"; !strings.Contains(out, want) { + if want := "callers of (*A).Run: 2 in the index, 2 listed here, plus 5 same-package name match(es) (e1); 11 unresolved call(s) of Run"; !strings.Contains(out, want) { t.Errorf("output lacks %q:\n%s", want, out) } requireNeverNoCallers(t, out) @@ -453,6 +453,12 @@ func makeIndexPredateLocalTier(t *testing.T, root string) int { t.Fatal(err) } removed, _ := res.RowsAffected() + // That build left those calls open, so their recorded outcome is the receiver + // bucket, not resolved: 0 and 1 are topology's persisted resolveBucket values + // (pinned by topology's TestOpenCallSites). + if _, err := db.Exec(`UPDATE topology_call_sites SET outcome = 1 WHERE outcome = 0 AND receiver_type NOT IN ('', ?)`, topology.ReceiverAmbiguous); err != nil { + t.Fatal(err) + } var left, census int if err := db.QueryRow(`SELECT count(*) `+localEdges, sourceCallResolver, topology.ReceiverAmbiguous).Scan(&left); err != nil { t.Fatal(err) @@ -513,7 +519,7 @@ func TestSamePackage_AMethodOnAnEvidentLocalIsResolvedOnlyOnAnIndexThatRanTheLoc if err != nil { t.Fatal(err) } - if want := "callers of (*Calc).Apply: 4 resolved and listed, plus 1 same-package name match(es) (e1)"; !strings.Contains(out, want) { + if want := "callers of (*Calc).Apply: 4 in the index, 4 listed here, plus 1 same-package name match(es) (e1); 2 unresolved call(s) of Apply may also be it: p/twice.go:7 (receiver), p/twice.go:9 (receiver)"; !strings.Contains(out, want) { t.Errorf("output lacks %q:\n%s", want, out) } @@ -531,7 +537,7 @@ func TestSamePackage_AMethodOnAnEvidentLocalIsResolvedOnlyOnAnIndexThatRanTheLoc if err != nil { t.Fatal(err) } - if want := "callers of (*Calc).Apply: 2 resolved and listed, plus 3 same-package name match(es) (e1)"; !strings.Contains(out, want) { + if want := "callers of (*Calc).Apply: 2 in the index, 2 listed here, plus 3 same-package name match(es) (e1); 4 unresolved call(s) of Apply"; !strings.Contains(out, want) { t.Errorf("output lacks %q:\n%s", want, out) } } diff --git a/internal/topology/callsites_open_test.go b/internal/topology/callsites_open_test.go new file mode 100644 index 00000000..c945f13e --- /dev/null +++ b/internal/topology/callsites_open_test.go @@ -0,0 +1,125 @@ +package topology + +import ( + "context" + "path/filepath" + "slices" + "testing" +) + +// TestOpenCallSites pins which calls of a name stay open after a resolution pass: +// each open kind with a site that produces it, and the controls that must not be +// open (a resolved call, a same-file call of a name its file declares once). +func TestOpenCallSites(t *testing.T) { + db, err := openDB(filepath.Join(t.TempDir(), "open.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { db.Close() }) + node := func(file int64, p string, n Node) int64 { + n.Language = "go" + return insertTestNode(t, db, file, p, n) + } + goFile := func(p, pkg string) int64 { + id := insertLangFile(t, db, p, "go") + node(id, p, Node{Kind: KindPackage, Name: pkg}) + return id + } + site := func(file, enclosing int64, qualifier any, callee string) { + t.Helper() + if _, err := db.Exec(`INSERT INTO topology_call_sites(file_id, enclosing_id, language, site_kind, callee, qualifier) + VALUES (?,?,?,?,?,?)`, file, enclosing, "go", string(CallSiteCall), callee, qualifier); err != nil { + t.Fatal(err) + } + } + const lib, twin, app, dot = "internal/lib/lib.go", "internal/lib/twin.go", "internal/app/app.go", "internal/app/dot.go" + libFile := goFile(lib, "lib") + node(libFile, lib, Node{Kind: KindFunction, Name: "Do", Signature: "func Do()"}) + node(libFile, lib, Node{Kind: KindMethod, Name: "Run", Qualified: "(*T).Run", Signature: "func (t *T) Run()"}) + unique := node(libFile, lib, Node{Kind: KindFunction, Name: "unique", Signature: "func unique()"}) + libCaller := node(libFile, lib, Node{Kind: KindFunction, Name: "caller", Signature: "func caller()"}) + site(libFile, libCaller, nil, "unique") // control: same file, declared once, with the extractor's edge + if _, err := db.Exec(`INSERT INTO topology_edges(from_id, to_id, kind, confidence, source) VALUES (?,?,?,?,?)`, + libCaller, unique, string(EdgeCalls), 1.0, "extractor"); err != nil { + t.Fatal(err) + } + node(libFile, lib, Node{Kind: KindFunction, Name: "Gone", Signature: "func Gone()"}) + + twinFile := goFile(twin, "lib") + node(twinFile, twin, Node{Kind: KindFunction, Name: "Close", Signature: "func Close()"}) + node(twinFile, twin, Node{Kind: KindMethod, Name: "Close", Qualified: "(B).Close", Signature: "func (b B) Close()"}) + twinCaller := node(twinFile, twin, Node{Kind: KindFunction, Name: "shut", Signature: "func shut()"}) + site(twinFile, twinCaller, nil, "Close") // same file, but the file declares Close twice + + appFile := goFile(app, "app") + node(appFile, app, Node{Kind: KindImport, Name: "lib", Qualified: "example.com/m/internal/lib"}) + node(appFile, app, Node{Kind: KindImport, Name: "fmt", Qualified: "fmt"}) + appCaller := node(appFile, app, Node{Kind: KindFunction, Name: "main", Signature: "func main()"}) + site(appFile, appCaller, "lib", "Do") // control: resolves through the import + site(appFile, appCaller, "x", "Run") // a receiver no declaration types + site(appFile, appCaller, nil, "Do") // bare, and app declares no Do + site(appFile, appCaller, QualifierLocal, "Do") // bare, and main also declares a Do + site(appFile, appCaller, "lib", "Gone") // resolves now; its edge is deleted below, as a later scoped pass leaves it + site(appFile, appCaller, "fmt", "Println") // external: fmt names no indexed directory + + dotFile := goFile(dot, "app") + node(dotFile, dot, Node{Kind: KindImport, Name: ".", Qualified: "example.com/m/internal/lib"}) + dotCaller := node(dotFile, dot, Node{Kind: KindFunction, Name: "viaDot", Signature: "func viaDot()"}) + site(dotFile, dotCaller, nil, "Do") // bare in a file that dot-imports a package + + if err := (&Indexer{db: db}).resolveCalls(context.Background()); err != nil { + t.Fatal(err) + } + // A later pass re-indexes lib.go and Gone goes; app.go is unchanged, so its + // site keeps "resolved" and only the edge is gone. + if res, err := db.Exec(`DELETE FROM topology_edges WHERE to_id IN (SELECT id FROM topology_nodes WHERE name = 'Gone')`); err != nil { + t.Fatal(err) + } else if n, _ := res.RowsAffected(); n != 1 { + t.Fatalf("control: lib.Gone had %d edges to remove, want 1", n) + } + s := &Store{db: db} + ctx := context.Background() + kinds := func(name string) []string { + t.Helper() + open, total, err := s.OpenCallSites(ctx, name, "go", 0) + if err != nil { + t.Fatal(err) + } + if total != len(open) { + t.Fatalf("%s: total %d, listed %d with no limit", name, total, len(open)) + } + out := make([]string, 0, len(open)) + for _, o := range open { + k := o.Path + " " + o.Kind + if o.DotImport { + k += " dot" + } + out = append(out, k) + } + slices.Sort(out) + return out + } + for name, want := range map[string][]string{ + "Do": {app + " bare", app + " local", dot + " bare dot"}, + "Run": {app + " receiver"}, + "Close": {twin + " same-file"}, + "unique": nil, + "Gone": {app + " stale"}, + "Println": {app + " external"}, + } { + if got := kinds(name); !slices.Equal(got, want) { + t.Errorf("open calls of %s = %q, want %q", name, got, want) + } + } + var resolved int + if err := db.QueryRow(`SELECT outcome FROM topology_call_sites WHERE callee = 'Do' AND qualifier = 'lib'`).Scan(&resolved); err != nil { + t.Fatal(err) + } + if resolveBucket(resolved) != bucketResolved { + t.Errorf("lib.Do's recorded outcome = %d, want resolved (%d)", resolved, bucketResolved) + } + open, total, err := s.OpenCallSites(ctx, "Do", "go", 1) + if err != nil || total != 3 || len(open) != 1 { + t.Errorf("limit 1: listed %d of total %d (err %v), want 1 of 3", len(open), total, err) + } +} diff --git a/internal/topology/callsites_query.go b/internal/topology/callsites_query.go index 56842747..032287b0 100644 --- a/internal/topology/callsites_query.go +++ b/internal/topology/callsites_query.go @@ -184,6 +184,117 @@ func fillPackages(ctx context.Context, db *sql.DB, sites []CallSiteRef) error { return nil } +// OpenCall is a recorded call of a name that no edge accounts for exactly: the +// resolver tied it to nothing, tied it to a declaration with no caller to hang an +// edge on, or left it to an intra-file pass that cannot tell two declarations of +// the name apart. These are the calls of the name a reader still has to look at. +type OpenCall struct { + Path string + Line int + Qualifier string // "" for a bare call + // Kind says why the call is open: "receiver" (a method call on a value whose + // type no declaration states), "unmatched" (a package-qualified name that + // declares no such function), "bare" (a bare name with no function of that + // name in its package), "local" (a bare name the calling function also + // declares, which may or may not shadow it where the call is), "same-file" + // (its file declares the name more than once), "top-level" (resolved, but + // outside any declaration), "external" (a package-qualified call whose import + // names no indexed directory: usually another module, but a package the index + // failed to map would land here too), "stale" (resolved by an earlier pass, + // but the edge it produced is gone) or "unresolved" (no pass has read it). + Kind string + // DotImport is set when the call's file dot-imports a package, so a bare name + // may be another package's exported function. + DotImport bool +} + +// OpenCallSites returns the recorded calls named name, in the language's files, +// that no edge accounts for exactly, ordered by path then position, at most limit +// of them; total counts them all. A resolved, repeated or same-file call is not +// open while the edge it stands for exists (a same-file call needs its file to +// declare the name once). An external call is returned for the caller to judge +// by its qualifier. +func (s *Store) OpenCallSites(ctx context.Context, name, language string, limit int) (open []OpenCall, total int, err error) { + // A call counts as accounted for only while the edge it produced still stands: + // a scoped pass re-reads only the changed files' sites, so a caller elsewhere + // keeps "resolved" after its target is deleted or renamed, with no edge left. A + // same-file call of the enclosing declaration's own name is recursion, which no + // pass draws an edge for and no census counts as a caller. + rows, err := s.db.QueryContext(ctx, ` + SELECT f.path, cs.start_line, IFNULL(cs.qualifier, ''), cs.outcome, + EXISTS (SELECT 1 FROM topology_nodes i WHERE i.file_id = cs.file_id AND i.kind = ? AND i.name = '.'), + cs.outcome IN (?, ?, ?) AND (cs.enclosing_id IS NULL OR NOT EXISTS ( + SELECT 1 FROM topology_edges e JOIN topology_nodes t ON t.id = e.to_id + WHERE e.from_id = cs.enclosing_id AND e.kind = ? AND t.name = cs.callee)) + AND NOT (cs.outcome = ? AND EXISTS ( + SELECT 1 FROM topology_nodes en WHERE en.id = cs.enclosing_id AND en.name = cs.callee)), + cs.outcome = ? AND (SELECT COUNT(*) FROM topology_nodes d + WHERE d.file_id = cs.file_id AND d.name = cs.callee AND d.kind IN (?, ?, ?)) > 1 + FROM topology_call_sites cs + JOIN topology_files f ON f.id = cs.file_id + WHERE cs.language = ? AND cs.callee = ? AND cs.site_kind = ? + AND (cs.outcome IN (?, ?, ?, ?, ?, -1) + OR (cs.outcome = ? AND (SELECT COUNT(*) FROM topology_nodes d + WHERE d.file_id = cs.file_id AND d.name = cs.callee AND d.kind IN (?, ?, ?)) > 1) + OR (cs.outcome IN (?, ?, ?) AND (cs.enclosing_id IS NULL OR NOT EXISTS ( + SELECT 1 FROM topology_edges e JOIN topology_nodes t ON t.id = e.to_id + WHERE e.from_id = cs.enclosing_id AND e.kind = ? AND t.name = cs.callee)) + AND NOT (cs.outcome = ? AND EXISTS ( + SELECT 1 FROM topology_nodes en WHERE en.id = cs.enclosing_id AND en.name = cs.callee)))) + ORDER BY f.path, cs.start_byte`, + string(KindImport), + int(bucketResolved), int(bucketRepeat), int(bucketSameFile), string(EdgeCalls), int(bucketSameFile), + int(bucketSameFile), string(KindFunction), string(KindMethod), string(KindTest), + language, name, string(CallSiteCall), + int(bucketReceiver), int(bucketUnmatched), int(bucketBareUnmatched), int(bucketNoCaller), int(bucketExternal), + int(bucketSameFile), string(KindFunction), string(KindMethod), string(KindTest), + int(bucketResolved), int(bucketRepeat), int(bucketSameFile), string(EdgeCalls), int(bucketSameFile)) + if err != nil { + return nil, 0, fmt.Errorf("topology: open call sites: %w", err) + } + defer rows.Close() + for rows.Next() { + var c OpenCall + var outcome int + var dangling, ambiguous bool + if err := rows.Scan(&c.Path, &c.Line, &c.Qualifier, &outcome, &c.DotImport, &dangling, &ambiguous); err != nil { + return nil, 0, fmt.Errorf("topology: open call sites: %w", err) + } + c.Kind = openKind(resolveBucket(outcome)) + if dangling && !ambiguous { + c.Kind = "stale" + } + if c.Qualifier == QualifierLocal { + // The extractor marks a name local for the whole function, not the block + // that declares it, so the call may still be the package's function. + c.Kind, c.Qualifier = "local", "" + } + total++ + if limit <= 0 || len(open) < limit { + open = append(open, c) + } + } + return open, total, rows.Err() +} + +func openKind(b resolveBucket) string { + switch b { + case bucketReceiver: + return "receiver" + case bucketUnmatched: + return "unmatched" + case bucketBareUnmatched: + return "bare" + case bucketSameFile: + return "same-file" + case bucketExternal: + return "external" + case bucketNoCaller: + return "top-level" + } + return "unresolved" +} + // marks is n comma-separated placeholders. func marks(n int) string { return strings.TrimSuffix(strings.Repeat("?,", n), ",") } @@ -214,3 +325,15 @@ func declsNamedInDir(ctx context.Context, db *sql.DB, dir, language, name string } return out, rows.Err() } + +// UnreadFiles counts the indexed files with the extension whose last extraction +// failed (a parse error, a file over the size cap): their calls are not recorded, +// so no census can speak for them. +func (s *Store) UnreadFiles(ctx context.Context, ext string) (int, error) { + var n int + err := s.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM topology_files WHERE error_msg != '' AND path LIKE ?`, "%"+ext).Scan(&n) + if err != nil { + return 0, fmt.Errorf("topology: unread files: %w", err) + } + return n, nil +} diff --git a/internal/topology/db.go b/internal/topology/db.go index a79fda3b..603b1dc8 100644 --- a/internal/topology/db.go +++ b/internal/topology/db.go @@ -113,10 +113,15 @@ CREATE TABLE IF NOT EXISTS topology_call_sites ( -- Go only: what the enclosing function's own declarations say about the -- qualifier when it is a local variable (CallSite.ReceiverType). '' when -- nothing is evident. - receiver_type TEXT NOT NULL DEFAULT '' + receiver_type TEXT NOT NULL DEFAULT '', + -- The call resolver's outcome for the site (resolveBucket), written by every + -- pass that reads it; -1 until a pass has. It is what lets a reader say that + -- every call of a name resolved, rather than only list the ones that did. + outcome INTEGER NOT NULL DEFAULT -1 ); CREATE INDEX IF NOT EXISTS idx_tcs_file ON topology_call_sites(file_id); CREATE INDEX IF NOT EXISTS idx_tcs_lookup ON topology_call_sites(language, site_kind, qualifier); +CREATE INDEX IF NOT EXISTS idx_tcs_callee ON topology_call_sites(language, callee); -- Opt-in semantic-search cache: one row per (embedding model, content hash). -- Keyed by content hash (not node id) so it survives re-indexing and shares a @@ -174,7 +179,10 @@ CREATE TABLE IF NOT EXISTS topology_embeddings ( // 7 — topology_call_sites.receiver_type: the type a Go call's local receiver was // declared with, or that its qualifier is declared more than once. A site // written before it has none, and an unchanged file is never re-extracted. -const SchemaVersion = 7 +// 8 — topology_call_sites.outcome, the resolver's per-site outcome, and an index +// on (language, callee). CREATE IF NOT EXISTS cannot add a column to an +// existing table, so the table is recreated. +const SchemaVersion = 8 // topologyTables are the topology tables/virtual tables, listed so the version // gate can DROP them in dependency order (children before parents). The diff --git a/internal/topology/extractors/golang/extractor.go b/internal/topology/extractors/golang/extractor.go index 028285f2..37e2bdca 100644 --- a/internal/topology/extractors/golang/extractor.go +++ b/internal/topology/extractors/golang/extractor.go @@ -113,14 +113,26 @@ func fileCallEdges(funcEntries []funcEntry, nodes []topology.Node) []topology.Ed return edges } +// buildNameIndex maps each callable name the file declares once to its node. A +// name declared twice (two receivers' methods, a function and a method) is left +// out: the pass matches by name alone, so it cannot tell which one a call means, +// and an edge to either would be a guess stated at confidence 1.0. Such calls +// stay open for the census (topology.OpenCallSites) instead. func buildNameIndex(nodes []topology.Node) map[string]int64 { m := make(map[string]int64, len(nodes)) + dup := map[string]bool{} for i, n := range nodes { switch n.Kind { case topology.KindFunction, topology.KindMethod, topology.KindTest: + if _, seen := m[n.Name]; seen { + dup[n.Name] = true + } m[n.Name] = int64(i) } } + for name := range dup { + delete(m, name) + } return m } @@ -153,12 +165,21 @@ func callEdgesFor(fn *ast.FuncDecl, fromIdx int64, nameToIdx map[string]int64) [ return edges } +// calleeIdent is the called name, seen through parentheses and explicit type +// arguments: `F[T](x)` calls F. The resolver files a same-file generic call as the +// extractor's (bucketSameFile), so a call this missed had no edge at all. func calleeIdent(expr ast.Expr) string { switch e := expr.(type) { case *ast.Ident: return e.Name case *ast.SelectorExpr: return e.Sel.Name + case *ast.ParenExpr: + return calleeIdent(e.X) + case *ast.IndexExpr: + return calleeIdent(e.X) + case *ast.IndexListExpr: + return calleeIdent(e.X) default: return "" } diff --git a/internal/topology/extractors/golang/extractor_calls_census_test.go b/internal/topology/extractors/golang/extractor_calls_census_test.go new file mode 100644 index 00000000..97695e0e --- /dev/null +++ b/internal/topology/extractors/golang/extractor_calls_census_test.go @@ -0,0 +1,82 @@ +package golang + +import ( + "context" + "strings" + "testing" + + "github.com/plumbkit/plumb/internal/topology" +) + +// callEdgesByName extracts src and returns its intra-file call edges as +// "caller->callee" by declaration name and receiver. +func callEdgesByName(t *testing.T, src string) map[string]bool { + t.Helper() + nodes, edges, err := New().Extract(context.Background(), "p.go", []byte(src)) + if err != nil { + t.Fatal(err) + } + out := map[string]bool{} + for _, e := range edges { + if e.Kind == topology.EdgeCalls { + out[nodes[e.FromID].Qualified+"->"+nodes[e.ToID].Qualified] = true + } + } + return out +} + +// TestExtract_CallEdges_GenericInstantiation: `F[T](x)` calls F. The resolver files +// a same-file generic call as this pass's, so a miss here left the call with no +// edge at all, and the census called F's callers complete without it. +func TestExtract_CallEdges_GenericInstantiation(t *testing.T) { + got := callEdgesByName(t, `package p + +func Get[T any]() (v T) { return } + +func Pair[K comparable, V any]() {} + +func use() { + _ = Get[int]() + Pair[string, int]() +} + +func paren() { _ = (Get[string])() } +`) + for _, want := range []string{"use->Get", "use->Pair", "paren->Get"} { + if !got[want] { + t.Errorf("missing intra-file edge %s; got %v", want, got) + } + } +} + +// TestExtract_CallEdges_AmbiguousNameIsNotGuessed: the pass matches by name alone, +// so when the file declares a name twice it cannot tell which one `a.Close()` +// means and emits no edge, rather than one to whichever was declared last. A name +// declared once in the same file is the control. +func TestExtract_CallEdges_AmbiguousNameIsNotGuessed(t *testing.T) { + got := callEdgesByName(t, `package p + +type A struct{} + +func (A) Close() {} + +type B struct{} + +func (B) Close() {} + +func helper() {} + +func shut(a A) { + a.Close() + helper() +} +`) + for e := range got { + if strings.HasPrefix(e, "shut->") && strings.HasSuffix(e, "Close") { + t.Errorf("an edge to one of two same-named declarations was guessed: %s", e) + } + } + if !got["shut->helper"] { + t.Errorf("control: shut->helper (declared once) is missing; got %v", got) + } +} diff --git a/internal/topology/indexer_calls.go b/internal/topology/indexer_calls.go index c24593f0..db262051 100644 --- a/internal/topology/indexer_calls.go +++ b/internal/topology/indexer_calls.go @@ -95,6 +95,7 @@ type callResolution struct { callSites int // qualifiedSites is the subset carrying a qualifier. qualifiedSites int + outcomes []siteOutcome // each site's bucket, held until the pass writes them } // resolveCalls turns recorded call sites into cross-file `calls` edges, for @@ -223,6 +224,10 @@ func resolveLanguageCallsScoped(ctx context.Context, tx *sql.Tx, lang string, sc if err := insertResolvedEdges(tx, edges); err != nil { return r, err } + if err := writeSiteOutcomes(ctx, tx, r.outcomes); err != nil { + return r, err + } + r.outcomes = nil return r, nil } @@ -265,7 +270,7 @@ type resolvedEdge struct { } func collectResolvedEdges(ctx context.Context, tx *sql.Tx, lang string, t resolverTables, r *callResolution, scoped bool) ([]resolvedEdge, error) { - query := `SELECT cs.file_id, IFNULL(cs.enclosing_id, 0), cs.callee, cs.qualifier, f.path, cs.receiver_type + query := `SELECT cs.id, cs.file_id, IFNULL(cs.enclosing_id, 0), cs.callee, cs.qualifier, f.path, cs.receiver_type FROM topology_call_sites cs JOIN topology_files f ON f.id = cs.file_id WHERE cs.language = ? AND cs.site_kind = ?` @@ -280,10 +285,10 @@ func collectResolvedEdges(ctx context.Context, tx *sql.Tx, lang string, t resolv seen := map[[2]int64]bool{} var edges []resolvedEdge for rows.Next() { - var fileID, enclosing int64 + var siteID, fileID, enclosing int64 var callee, callerPath, receiverType string var qualifier sql.NullString - if err := rows.Scan(&fileID, &enclosing, &callee, &qualifier, &callerPath, &receiverType); err != nil { + if err := rows.Scan(&siteID, &fileID, &enclosing, &callee, &qualifier, &callerPath, &receiverType); err != nil { return nil, fmt.Errorf("topology: resolve calls: scan site: %w", err) } if qualifier.Valid && qualifier.String != QualifierLocal { @@ -300,6 +305,7 @@ func collectResolvedEdges(ctx context.Context, tx *sql.Tx, lang string, t resolv bucket, emitted = qualifiedOutcome(t, fileID, enclosing, callee, qualifier.String, seen) } r.countBucket(bucket) + r.outcomes = append(r.outcomes, siteOutcome{id: siteID, bucket: bucket}) if bucket != bucketResolved { continue } @@ -373,6 +379,7 @@ func insertResolvedEdges(tx *sql.Tx, edges []resolvedEdge) error { return nil } +// resolveBucket is a site's outcome, persisted in topology_call_sites.outcome: append, never renumber. type resolveBucket int const ( diff --git a/internal/topology/indexer_calls_outcome.go b/internal/topology/indexer_calls_outcome.go new file mode 100644 index 00000000..a3cd95eb --- /dev/null +++ b/internal/topology/indexer_calls_outcome.go @@ -0,0 +1,38 @@ +package topology + +import ( + "context" + "database/sql" + "fmt" +) + +// indexer_calls_outcome.go — each call site's resolver outcome, kept on its row so +// a reader can tell the calls of a name that resolved from the ones still open +// (callsites_query.go, OpenCallSites). + +// siteOutcome is one site's bucket, recorded on the site after the pass. +type siteOutcome struct { + id int64 + bucket resolveBucket +} + +// writeSiteOutcomes stores each site's outcome on its row, after the scan has +// closed, so a reader can ask which calls of a name are still open +// (OpenCallSites). A scoped pass writes only the sites it read, as it writes only +// their edges. +func writeSiteOutcomes(ctx context.Context, tx *sql.Tx, outcomes []siteOutcome) error { + if len(outcomes) == 0 { + return nil + } + stmt, err := tx.PrepareContext(ctx, `UPDATE topology_call_sites SET outcome = ? WHERE id = ?`) + if err != nil { + return fmt.Errorf("topology: resolve calls: prepare outcome: %w", err) + } + defer stmt.Close() + for _, o := range outcomes { + if _, err := stmt.ExecContext(ctx, int(o.bucket), o.id); err != nil { + return fmt.Errorf("topology: resolve calls: outcome: %w", err) + } + } + return nil +}