From dfacbbb683c3c271f08c934d483736a89041696e Mon Sep 17 00:00:00 2001 From: Dejan Menges Date: Fri, 12 Jun 2026 07:53:19 +0200 Subject: [PATCH 1/6] Improving traverse and find_path --- internal/extractors/goextractor/go.go | 7 +- internal/extractors/goextractor/go_test.go | 38 ++++ internal/facts/graph.go | 84 ++++++-- internal/facts/graph_test.go | 86 +++++++- internal/facts/model.go | 25 +-- internal/server/scopedquery.go | 223 ++++++++++++++++++++ internal/server/server.go | 224 +++++++++++++-------- internal/server/server_test.go | 175 +++++++++++++++- 8 files changed, 748 insertions(+), 114 deletions(-) create mode 100644 internal/server/scopedquery.go diff --git a/internal/extractors/goextractor/go.go b/internal/extractors/goextractor/go.go index 57d3574..11282e7 100644 --- a/internal/extractors/goextractor/go.go +++ b/internal/extractors/goextractor/go.go @@ -521,6 +521,11 @@ func collectFieldTypes(files []*ast.File, pkgDir, modulePath string, pkgNames ma // - 3+ elements: resolve root to a qualified "pkg.Type", walk intermediate fields via fieldTypes, produce qualifiedType.method // // Falls back to the raw joined string when resolution is not possible, so no call is dropped. +// +// Known limitation: calls through an interface value (e.g. iface.Method()) cannot be +// statically bound to a concrete implementation without type-flow analysis, so the +// resolved target may name an interface method that has no backing symbol fact. Such +// edges surface as "unresolved" nodes during traversal rather than concrete callees. func resolveChain(chain []string, ctx resolveCtx) string { switch len(chain) { case 0: @@ -549,7 +554,7 @@ func resolveChain(chain []string, ctx resolveCtx) string { // 3+ elements: attempt field-chain resolution. root := chain[0] var qualType string // "pkgDir.TypeName" or "importedPkg.TypeName" - var fieldStart int // index of the first intermediate field in chain + var fieldStart int // index of the first intermediate field in chain if root == ctx.recvVar && ctx.recvType != "" { qualType = ctx.pkgDir + "." + ctx.recvType diff --git a/internal/extractors/goextractor/go_test.go b/internal/extractors/goextractor/go_test.go index f1d325a..fb8c2d0 100644 --- a/internal/extractors/goextractor/go_test.go +++ b/internal/extractors/goextractor/go_test.go @@ -656,3 +656,41 @@ func TestDetect(t *testing.T) { t.Error("expected Detect=false for directory without go.mod") } } + +// TestExtract_StructAndMethodAreSeparateFacts documents the contract the graph's +// has_method synthesis relies on: a struct and each of its methods are emitted as +// distinct sibling facts ("pkg.Type" and "pkg.Type.Method"), with no edge between +// them at extraction time. If this ever changes, internal/facts.NewGraph's +// has_method third pass must be revisited. +func TestExtract_StructAndMethodAreSeparateFacts(t *testing.T) { + ff := extractAll(t, map[string]string{ + "pkg/handler.go": `package pkg + +type AuthHandler struct{} + +func (h *AuthHandler) Login() {} +`, + }) + + st, ok := findFact(ff, "pkg.AuthHandler") + if !ok { + t.Fatal("expected struct fact pkg.AuthHandler") + } + if st.Props["symbol_kind"] != facts.SymbolStruct { + t.Errorf("AuthHandler symbol_kind = %v, want struct", st.Props["symbol_kind"]) + } + // The struct must NOT carry an edge to its method (the graph synthesizes it). + for _, rel := range st.Relations { + if rel.Target == "pkg.AuthHandler.Login" { + t.Errorf("struct should not declare its method directly, found relation %+v", rel) + } + } + + m, ok := findFact(ff, "pkg.AuthHandler.Login") + if !ok { + t.Fatal("expected method fact pkg.AuthHandler.Login as a separate fact") + } + if m.Props["symbol_kind"] != facts.SymbolMethod { + t.Errorf("Login symbol_kind = %v, want method", m.Props["symbol_kind"]) + } +} diff --git a/internal/facts/graph.go b/internal/facts/graph.go index 35b2d04..cc2e70d 100644 --- a/internal/facts/graph.go +++ b/internal/facts/graph.go @@ -9,16 +9,16 @@ import ( // It is a derived index rebuilt from the Store's facts after each snapshot generation. type Graph struct { mu sync.RWMutex - forward map[string][]Edge // fact name → outgoing edges - reverse map[string][]Edge // fact name → incoming edges - facts []Fact // reference to the store's facts (for metadata lookups) - factIdx map[string]int // fact name → first index in facts slice - edgeSeen map[string]struct{} // deduplication: "source\x00kind\x00target" + forward map[string][]Edge // fact name → outgoing edges + reverse map[string][]Edge // fact name → incoming edges + facts []Fact // reference to the store's facts (for metadata lookups) + factIdx map[string]int // fact name → first index in facts slice + edgeSeen map[string]struct{} // deduplication: "source\x00kind\x00target" } // Edge represents a directed relationship between two facts. type Edge struct { - RelKind string // "imports", "calls", "declares", "implements", "depends_on" + RelKind string // "imports", "calls", "declares", "implements", "depends_on", "has_method" Target string // target fact name (forward) or source fact name (reverse) } @@ -36,6 +36,12 @@ type TraversalNode struct { File string `json:"file,omitempty"` Line int `json:"line,omitempty"` Depth int `json:"depth"` + // Unresolved marks a node whose name is the target of an edge but has no + // backing fact in the store. This happens for inferred call targets that + // could not be matched to a declared symbol (e.g. interface-method dispatch, + // or calls into packages that weren't analyzed). The edge is real; the + // destination symbol just isn't in the graph. + Unresolved bool `json:"unresolved,omitempty"` } // TraversalEdge is an edge traversed during traversal. @@ -55,12 +61,12 @@ type TraversalStats struct { // ImpactResult holds depth-bucketed impact analysis results. type ImpactResult struct { - Target string `json:"target"` - ByDepth map[int][]TraversalNode `json:"by_depth"` - Edges []TraversalEdge `json:"edges"` - Summary string `json:"summary"` - Stats TraversalStats `json:"stats"` - Forward *TraversalResult `json:"forward_dependencies,omitempty"` + Target string `json:"target"` + ByDepth map[int][]TraversalNode `json:"by_depth"` + Edges []TraversalEdge `json:"edges"` + Summary string `json:"summary"` + Stats TraversalStats `json:"stats"` + Forward *TraversalResult `json:"forward_dependencies,omitempty"` } // PathResult holds a shortest-path result. @@ -149,6 +155,25 @@ func NewGraph(ff []Fact) *Graph { } } + // Third pass: synthesize "has_method" edges linking an owner type symbol + // (struct/interface/class/type) to its method symbols. Extractors emit a + // method as a sibling fact named "." with no edge back to the + // owner, so forward traversal from a type would otherwise surface none of its + // methods (and transitively none of their calls). This is language-agnostic: + // any fact named "." gets wired to its owner. + for _, f := range ff { + if f.Kind != KindSymbol { + continue + } + sk, _ := f.Props["symbol_kind"].(string) + if sk != SymbolMethod && sk != SymbolFunc { + continue + } + if owner := g.methodOwner(f.Name); owner != "" { + g.addEdge(owner, RelHasMethod, f.Name) + } + } + // edgeSeen is only needed during construction; release it so the GC can // reclaim the O(edges × 3 strings) backing memory. g.edgeSeen = nil @@ -156,6 +181,30 @@ func NewGraph(ff []Fact) *Graph { return g } +// methodOwner returns the owner type name for a method fact name of the form +// ".", but only when is itself a known symbol fact whose +// symbol_kind is a type (struct/interface/class/type). Returns "" otherwise. +func (g *Graph) methodOwner(name string) string { + dot := strings.LastIndex(name, ".") + if dot <= 0 { + return "" + } + owner := name[:dot] + idx, ok := g.factIdx[owner] + if !ok || idx >= len(g.facts) { + return "" + } + of := g.facts[idx] + if of.Kind != KindSymbol { + return "" + } + switch sk, _ := of.Props["symbol_kind"].(string); sk { + case SymbolStruct, SymbolInterface, SymbolClass, SymbolType: + return owner + } + return "" +} + // Traverse performs a BFS traversal from the given start node. // direction is "forward" or "reverse". // relKinds filters to specific relation types (nil = all). @@ -302,7 +351,7 @@ func (g *Graph) FindPath(from, to string, relKinds []string, maxDepth int) PathR relSet := toSet(relKinds) type queueItem struct { - name string + name string depth int } @@ -428,8 +477,8 @@ func (g *Graph) ImpactSet(target string, maxDepth, maxNodes int, includeForward // the consumer emits the full import path (e.g. "github.com/x/go-auth/adapters.Handler.Login") // but the provider's facts use the repo-relative path (e.g. "adapters.Handler.Login"). // -// subpackage: "github.com/x/go-auth/adapters.Handler.Login" → "adapters.Handler.Login" -// root pkg: "github.com/x/go-auth.SecurityHeaders" → "..SecurityHeaders" +// subpackage: "github.com/x/go-auth/adapters.Handler.Login" → "adapters.Handler.Login" +// root pkg: "github.com/x/go-auth.SecurityHeaders" → "..SecurityHeaders" func normalizeExternalTarget(target string, modulePaths map[string]struct{}) string { for modulePath := range modulePaths { if !strings.HasPrefix(target, modulePath) { @@ -555,6 +604,11 @@ func (g *Graph) nodeFor(name string, depth int) TraversalNode { node.Kind = f.Kind node.File = f.File node.Line = f.Line + } else { + // No backing fact: this is a dangling edge target (e.g. an inferred call + // into an unanalyzed package or an interface method). Mark it honestly + // rather than emitting a silent kind-less node. + node.Unresolved = true } return node } diff --git a/internal/facts/graph_test.go b/internal/facts/graph_test.go index 6544a81..dd3dd45 100644 --- a/internal/facts/graph_test.go +++ b/internal/facts/graph_test.go @@ -651,7 +651,7 @@ func TestNormalizeExternalTarget(t *testing.T) { {"github.com/dejo1307/go-auth/adapters.Handler.Login", "adapters.Handler.Login"}, {"github.com/dejo1307/go-auth.SecurityHeaders", "..SecurityHeaders"}, {"github.com/other/lib/pkg.Type.Method", ""}, // no matching module - {"github.com/dejo1307/go-auth", ""}, // no separator after module path + {"github.com/dejo1307/go-auth", ""}, // no separator after module path } for _, tc := range cases { @@ -661,3 +661,87 @@ func TestNormalizeExternalTarget(t *testing.T) { } } } + +// buildTypeMethodStore models a Go type with a method that makes a call, plus a +// dangling call into an unanalyzed package. The struct and method are separate +// sibling facts with no edge between them, mirroring the goextractor output. +func buildTypeMethodStore() (*Graph, *Store) { + s := NewStore() + s.Add( + Fact{Kind: KindSymbol, Name: "auth.AuthHandler", File: "auth/handler.go", Line: 1, + Props: map[string]any{"symbol_kind": SymbolStruct}}, + Fact{Kind: KindSymbol, Name: "auth.AuthHandler.Login", File: "auth/handler.go", Line: 10, + Props: map[string]any{"symbol_kind": SymbolMethod}, + Relations: []Relation{ + {Kind: RelCalls, Target: "jwt.Sign"}, + {Kind: RelCalls, Target: "external.Unknown"}, // no backing fact + }}, + Fact{Kind: KindSymbol, Name: "jwt.Sign", File: "jwt/jwt.go", Line: 5, + Props: map[string]any{"symbol_kind": SymbolFunc}}, + ) + s.BuildGraph() + return s.Graph(), s +} + +func TestNewGraph_StructToMethodEdges(t *testing.T) { + g, _ := buildTypeMethodStore() + + var found bool + for _, e := range g.Forward()["auth.AuthHandler"] { + if e.RelKind == RelHasMethod && e.Target == "auth.AuthHandler.Login" { + found = true + } + } + if !found { + t.Fatalf("expected has_method edge auth.AuthHandler -> auth.AuthHandler.Login, got %+v", g.Forward()["auth.AuthHandler"]) + } + + // A package-level function whose owner ("jwt") is not a type must NOT get a + // has_method edge. + for _, e := range g.Forward()["jwt"] { + if e.RelKind == RelHasMethod { + t.Errorf("unexpected has_method edge from non-type owner: %+v", e) + } + } +} + +func TestTraverse_ForwardFromStructSurfacesMethodCalls(t *testing.T) { + g, _ := buildTypeMethodStore() + + result := g.Traverse("auth.AuthHandler", "forward", nil, nil, 5, 100) + + names := nodeNames(result.Nodes) + for _, want := range []string{"auth.AuthHandler.Login", "jwt.Sign"} { + if !contains(names, want) { + t.Errorf("forward traverse from struct missing %q; got %v", want, names) + } + } +} + +func TestTraverse_UnresolvedTargetMarked(t *testing.T) { + g, _ := buildTypeMethodStore() + + result := g.Traverse("auth.AuthHandler.Login", "forward", nil, nil, 5, 100) + + var sawUnresolved, sawResolved bool + for _, n := range result.Nodes { + switch n.Name { + case "external.Unknown": + sawUnresolved = true + if !n.Unresolved { + t.Error("external.Unknown should be marked Unresolved") + } + case "jwt.Sign": + sawResolved = true + if n.Unresolved { + t.Error("jwt.Sign is a real fact and must not be marked Unresolved") + } + } + } + if !sawUnresolved { + t.Error("expected an unresolved node external.Unknown in the result") + } + if !sawResolved { + t.Error("expected the resolved node jwt.Sign in the result") + } +} diff --git a/internal/facts/model.go b/internal/facts/model.go index d16e015..64afedc 100644 --- a/internal/facts/model.go +++ b/internal/facts/model.go @@ -36,6 +36,7 @@ const ( RelDependsOn = "depends_on" RelInstantiates = "instantiates" // Source constructs an instance of target via a constructor call. RelInjects = "injects" // Source declares target as a DI-injected constructor parameter. + RelHasMethod = "has_method" // Owner type (struct/interface/class) declares target as a method. Synthesized in NewGraph. ) // Symbol kind property values. @@ -69,9 +70,9 @@ type Evidence struct { // Artifact represents a generated output file. type Artifact struct { - Name string `json:"name"` // e.g. "llm_context.md" - Content []byte `json:"-"` // Raw content - Type string `json:"type"` // MIME type hint + Name string `json:"name"` // e.g. "llm_context.md" + Content []byte `json:"-"` // Raw content + Type string `json:"type"` // MIME type hint } // Snapshot holds the complete result of an analysis run. @@ -84,15 +85,15 @@ type Snapshot struct { // SnapshotMeta contains metadata about a snapshot generation run. type SnapshotMeta struct { - RepoPath string `json:"repo_path"` - GeneratedAt string `json:"generated_at"` - Duration string `json:"duration"` - Extractors []string `json:"extractors"` - Explainers []string `json:"explainers"` - Renderers []string `json:"renderers"` - FileHashes []FileHash `json:"file_hashes,omitempty"` - FactCount int `json:"fact_count"` - InsightCount int `json:"insight_count"` + RepoPath string `json:"repo_path"` + GeneratedAt string `json:"generated_at"` + Duration string `json:"duration"` + Extractors []string `json:"extractors"` + Explainers []string `json:"explainers"` + Renderers []string `json:"renderers"` + FileHashes []FileHash `json:"file_hashes,omitempty"` + FactCount int `json:"fact_count"` + InsightCount int `json:"insight_count"` } // FileHash tracks a file's content hash for incremental updates. diff --git a/internal/server/scopedquery.go b/internal/server/scopedquery.go new file mode 100644 index 0000000..0ac258b --- /dev/null +++ b/internal/server/scopedquery.go @@ -0,0 +1,223 @@ +package server + +import ( + "sort" + "strings" + + "github.com/enola-labs/enola/internal/facts" +) + +// scopedQuery is the parsed form of a node-resolution input that may carry +// scoping prefixes (repo:, kind:, file:) to disambiguate an otherwise-ambiguous +// substring term. A plain input with no recognized prefix yields a scopedQuery +// whose Term equals the input and whose scope fields are empty, preserving the +// legacy substring-match behavior. +type scopedQuery struct { + Repo string // repo: