Merge pull request #140 from HexmosTech/feat/rename-aware-blast-graph

Show pre-rename callers in the blast-radius dependency graph

Author: lovestaco

blastradius/blastradius.go

@@ -154,6 +154,12 @@ type CallerRef struct {
 	// Path holds the intermediate qualified names between the symbol and
 	// this caller (see CallerContribution.Path).
 	Path []string
+	// PreRename marks a caller reached via the symbol's pre-rename name
+	// rather than the graph's CALLS edges: it references the old name and
+	// breaks until it's migrated. CALLS fan-in can never find these - the
+	// index reflects the post-rename tree, where the old name has no node
+	// and every edge pointing at it was dropped as unresolvable.
+	PreRename bool
 }
 
 // SymbolContribution describes one touched symbol's part of a hunk's score.
@@ -182,7 +188,14 @@ type SymbolContribution struct {
 	TransitiveCount int
 	// Callers is the full caller list (only for "calls"), sorted by depth
 	// then name, for showing exactly which transitive calls contributed.
+	// A renamed symbol additionally carries its pre-rename callers here,
+	// flagged with CallerRef.PreRename - see RenamedFrom.
 	Callers []CallerRef
+	// RenamedFrom is the pre-rename name when this symbol's own declaration
+	// was renamed in the diff, empty otherwise. It explains why the entries
+	// in Callers flagged PreRename are there: they still reference this
+	// name, so they break until they're migrated.
+	RenamedFrom string
 	// ImpactedPackages are the distinct packages/directories this symbol's
 	// influence reaches: for "calls", the packages its callers live in; for
 	// "text-references", the directories search_code found matches in.
@@ -632,6 +645,17 @@ func ScoreHunks(ctx context.Context, project string, hunks []Hunk, opts ...Optio
 		impactedPackages := sortedUnique(packages)
 
 		rename, renamed := renameByQN[qn]
+		// A renamed symbol's CALLS fan-in above only sees callers already
+		// migrated to the new name - usually none, which is why the graph
+		// looked empty for exactly the highest-risk case. The pre-rename
+		// callers are the ones about to break, so append them (display and
+		// visualization only; they contribute no points - the rename's score
+		// stays entirely in renameOldNameSignal below).
+		if renamed {
+			preRename := oldNameCallers(ctx, c, rename, qn)
+			callers = append(callers, preRename...)
+			callers = append(callers, expandPreRenameCallers(ctx, c, preRename, o.Score, qn)...)
+		}
 		callerReachDetail := fmt.Sprintf("%d direct + %d transitive caller(s), up to %d hops", direct, transitive, maxDepth)
 		if renamed {
 			callerReachDetail = fmt.Sprintf("%d direct + %d transitive caller(s) already migrated to the new name %q, up to %d hops", direct, transitive, rename.NewName, maxDepth)
@@ -706,6 +730,7 @@ func ScoreHunks(ctx context.Context, project string, hunks []Hunk, opts ...Optio
 			DirectCount:      direct,
 			TransitiveCount:  transitive,
 			Callers:          callers,
+			RenamedFrom:      rename.OldName, // zero value when !renamed
 			ImpactedPackages: impactedPackages,
 		}
 	}
@@ -818,11 +843,24 @@ func ScoreHunks(ctx context.Context, project string, hunks []Hunk, opts ...Optio
 				}
 			}
 
+			// A renamed type has no call graph of its own (that's what makes
+			// it a "text-references" symbol), but the places still using its
+			// old name are real, locatable nodes - so a renamed struct gets a
+			// dependency graph here where it previously had nothing to show.
+			var callers []CallerRef
+			if renamed {
+				preRename := oldNameCallers(ctx, c, rename, qn)
+				callers = append(callers, preRename...)
+				callers = append(callers, expandPreRenameCallers(ctx, c, preRename, o.Score, qn)...)
+			}
+
 			contribByQN[qn] = SymbolContribution{
 				Method:            "text-references",
 				Signals:           signals,
 				BlastRadiusRaw:    sumSignalPoints(signals, blastRadiusCategories),
 				DirectCount:       refs,
+				Callers:           callers,
+				RenamedFrom:       rename.OldName, // zero value when !renamed
 				ImpactedPackages:  usage.Directories,
 				MethodBlastRadius: methodBlastRadius,
 			}

blastradius/client/client.go

@@ -158,6 +158,64 @@ func (c *Client) SearchCodeUsage(ctx context.Context, pattern string) (*CodeUsag
 	return &CodeUsage{TotalMatches: result.TotalGrepMatches, Directories: dirs}, nil
 }
 
+// CodeSymbolMatch is one textual hit resolved to the graph node that encloses
+// it - search_code's "compact" mode maps each grep match back to the symbol
+// whose source range contains it, which is what makes it usable as a caller
+// list for names that have no node of their own (see SearchCodeSymbols).
+type CodeSymbolMatch struct {
+	QualifiedName string `json:"qualified_name"`
+	Name          string `json:"node"`
+	Label         string `json:"label"`
+	File          string `json:"file"`
+	StartLine     int    `json:"start_line"`
+	// MatchLines are the 1-based line numbers inside this symbol where the
+	// pattern actually occurred.
+	MatchLines []int `json:"match_lines"`
+}
+
+// CodeSymbolMatches is the result of SearchCodeSymbols.
+type CodeSymbolMatches struct {
+	// Matches are the enclosing symbols, deduplicated by the tool itself
+	// (one entry per symbol, however many lines inside it matched).
+	Matches []CodeSymbolMatch
+	// TotalMatches is the raw grep-style hit count, the same number
+	// SearchCodeUsage reports - unaffected by the enrichment limit.
+	TotalMatches int
+	// Truncated reports that the tool found more enclosing symbols than the
+	// limit allowed it to enrich, so Matches is a partial list.
+	Truncated bool
+}
+
+// SearchCodeSymbols runs `cli search_code --mode compact` for pattern and
+// returns the graph symbols enclosing each match. Unlike SearchCodeUsage
+// (which only counts hits), this identifies *who* references the name - the
+// only way to find references to a name the graph has no node for, e.g. a
+// symbol's pre-rename name after the tree was reindexed. limit <= 0 leaves
+// the tool's own default in place.
+func (c *Client) SearchCodeSymbols(ctx context.Context, pattern string, limit int) (*CodeSymbolMatches, error) {
+	args := []string{"--pattern", pattern, "--mode", "compact"}
+	if limit > 0 {
+		args = append(args, "--limit", strconv.Itoa(limit))
+	}
+	out, err := c.run(ctx, "search_code", args...)
+	if err != nil {
+		return nil, err
+	}
+	var result struct {
+		Results          []CodeSymbolMatch `json:"results"`
+		TotalGrepMatches int               `json:"total_grep_matches"`
+		TotalResults     int               `json:"total_results"`
+	}
+	if err := json.Unmarshal(out, &result); err != nil {
+		return nil, fmt.Errorf("blastradius/client: parsing search_code output: %w", err)
+	}
+	return &CodeSymbolMatches{
+		Matches:      result.Results,
+		TotalMatches: result.TotalGrepMatches,
+		Truncated:    result.TotalResults > len(result.Results),
+	}, nil
+}
+
 // ArchitectureEntryPoint is one entry in get_architecture's "entry_points"
 // aspect - a real, cross-language entry point (main functions, extension
 // activate/deactivate hooks, script mains), not just the is_entry_point

blastradius/rename.go

@@ -5,9 +5,11 @@ import (
 	"fmt"
 	"math"
 	"regexp"
+	"sort"
 	"strings"
 
 	"github.com/HexmosTech/blastradius/client"
+	"github.com/HexmosTech/blastradius/score"
 	"github.com/HexmosTech/blastradius/symbols"
 )
 
@@ -115,33 +117,172 @@ func matchSingleIdentSwap(oldLine, newLine, symName string) *DeclRename {
 
 // renameOldNameSignal runs a best-effort text search for rn.OldName and
 // returns a Signal counting how many places in the repo still reference the
-// pre-rename name - i.e. call sites/references that are about to break until
-// they're migrated to rn.NewName. Weighted 1.5x relative to the plain
-// sqrt(refs) used for an ordinary text-reference count elsewhere in this
-// package, since this is live, unmigrated breakage risk right now, not just
-// historical usage. Unlike the ordinary text-reference count, no "-1 for the
-// symbol's own definition" adjustment is applied: the definition itself was
-// renamed away, so every remaining match under the old name is an external
-// reference.
+// pre-rename name after the declaration moved to rn.NewName. Weighted 1.5x
+// relative to the plain sqrt(refs) used for an ordinary text-reference count
+// elsewhere in this package, since an unmigrated reference is a live risk
+// right now, not just historical usage. Unlike the ordinary text-reference
+// count, no "-1 for the symbol's own definition" adjustment is applied: the
+// definition itself was renamed away, so every remaining match under the old
+// name is an external reference.
+//
+// The wording states only what was measured. search_code is grep-based, so a
+// match can equally be a real call, a comment, or a string literal - calling
+// these references "about to break" asserted a compile failure that nothing
+// here verifies. Points are unchanged; this is phrasing only.
 func renameOldNameSignal(ctx context.Context, c *client.Client, rn DeclRename) Signal {
 	usage, err := c.SearchCodeUsage(ctx, rn.OldName)
 	if err != nil {
 		// Distinguish "checked, found nothing" from "couldn't check" - the
 		// latter must not silently read as 0 points/0 references, which
-		// would falsely assert the rename is safe when it simply wasn't
-		// verified.
+		// would falsely assert the rename is fully migrated when it simply
+		// wasn't verified.
 		return Signal{
-			Name:     "Callers of old name (about to break)",
-			Detail:   fmt.Sprintf("could not check references to the pre-rename name %q: %v", rn.OldName, err),
+			Name:     "Still uses the old name",
+			Detail:   fmt.Sprintf("could not check references to the old name %q: %v", rn.OldName, err),
 			Points:   0,
 			Category: "graph",
 		}
 	}
 	refs := usage.TotalMatches
 	return Signal{
-		Name:     "Callers of old name (about to break)",
-		Detail:   fmt.Sprintf("%d reference(s) to the pre-rename name %q still exist and will break until migrated to %q", refs, rn.OldName, rn.NewName),
+		Name:     "Still uses the old name",
+		Detail:   fmt.Sprintf("%d reference(s) to %q remain after the rename to %q", refs, rn.OldName, rn.NewName),
 		Points:   1.5 * math.Sqrt(float64(refs)),
 		Category: "graph",
 	}
 }
+
+// oldNameCallerLimit caps how many enclosing symbols search_code is asked to
+// resolve for a pre-rename name. Generous enough that a normal rename's call
+// sites all fit, bounded so a rename of a very common word (e.g. "get") can't
+// turn one hunk into a thousand-node graph.
+const oldNameCallerLimit = 60
+
+// preRenameCallerLabels are the node labels that can actually contain a
+// reference that breaks. search_code resolves a match to whatever node encloses
+// it, which includes documentation and structural nodes (Section, File, Folder,
+// Package, ...) - a README mentioning the old name is not a caller, and listing
+// it as one would misrepresent the breakage.
+var preRenameCallerLabels = map[string]bool{
+	"Function": true,
+	"Method":   true,
+	"Class":    true,
+	"Struct":   true,
+	"Variable": true,
+	"Route":    true,
+}
+
+// oldNameCallers finds the symbols that still reference rn.OldName - the call
+// sites that break until they're migrated to rn.NewName - and returns them as
+// depth-1 CallerRefs flagged PreRename.
+//
+// These cannot come from CALLS fan-in: the graph is indexed against the
+// post-rename tree, so the old name has no node and every edge that pointed at
+// it was dropped as unresolvable. search_code's compact mode is the way in - it
+// resolves each textual hit to the graph node enclosing it, which is exactly
+// the caller we want.
+//
+// Best-effort by the same rule as renameOldNameSignal's error path: a failed
+// lookup returns no callers rather than failing the report. It contributes no
+// points either way - scoring stays entirely with renameOldNameSignal.
+func oldNameCallers(ctx context.Context, c *client.Client, rn DeclRename, selfQN string) []CallerRef {
+	matches, err := c.SearchCodeSymbols(ctx, rn.OldName, oldNameCallerLimit)
+	if err != nil {
+		return nil
+	}
+	return preRenameCallersFrom(matches.Matches, selfQN)
+}
+
+// preRenameCallersFrom is oldNameCallers' pure half: turn search_code's
+// enclosing-symbol matches into a deduplicated, name-sorted caller list.
+func preRenameCallersFrom(matches []client.CodeSymbolMatch, selfQN string) []CallerRef {
+	var callers []CallerRef
+	seen := make(map[string]bool)
+	for _, m := range matches {
+		// The renamed symbol's own declaration is not one of its callers.
+		// It can still match: a method body referencing its own old name,
+		// or a stale doc comment above the new declaration.
+		if m.QualifiedName == "" || m.QualifiedName == selfQN || seen[m.QualifiedName] {
+			continue
+		}
+		if !preRenameCallerLabels[m.Label] {
+			continue
+		}
+		seen[m.QualifiedName] = true
+		callers = append(callers, CallerRef{
+			QualifiedName: m.QualifiedName,
+			Depth:         1,
+			Weight:        1,
+			PreRename:     true,
+		})
+	}
+	sort.Slice(callers, func(i, j int) bool { return callers[i].QualifiedName < callers[j].QualifiedName })
+	return callers
+}
+
+// expandPreRenameCallers adds the transitive fan-in of direct pre-rename
+// callers. Unlike the old name itself, those callers are ordinary present-day
+// graph nodes, so score.FanIn can walk them - which is what gives a rename's
+// sunburst/flamegraph the same depth structure as any other symbol's instead of
+// a single flat ring.
+//
+// Each discovered caller comes back one hop further from the renamed symbol
+// than it is from the direct caller, reached *through* that direct caller, so
+// its Depth is shifted by 1 and its Path prefixed accordingly. Anything already
+// present as a direct pre-rename caller is left alone - the shallower depth is
+// the truthful one.
+func expandPreRenameCallers(ctx context.Context, q score.GraphQuerier, direct []CallerRef, cfg score.Config, selfQN string) []CallerRef {
+	if len(direct) == 0 {
+		return nil
+	}
+	directQN := make([]string, 0, len(direct))
+	seen := make(map[string]bool, len(direct))
+	for _, c := range direct {
+		directQN = append(directQN, c.QualifiedName)
+		seen[c.QualifiedName] = true
+	}
+	// One fewer hop than a normal walk: the direct pre-rename callers are
+	// already one hop out from the renamed symbol, so the budget left for
+	// their own callers is MaxDepth-1.
+	expandCfg := cfg
+	expandCfg.MaxDepth = cfg.MaxDepth - 1
+	if expandCfg.MaxDepth < 1 {
+		return nil
+	}
+	scores, err := score.FanIn(ctx, q, directQN, expandCfg)
+	if err != nil {
+		return nil // best-effort: the direct callers alone are still worth showing
+	}
+
+	best := make(map[string]CallerRef)
+	for viaQN, ss := range scores {
+		for _, cc := range ss.Callers {
+			qn := cc.QualifiedName
+			if qn == "" || qn == selfQN || seen[qn] {
+				continue
+			}
+			depth := cc.Depth + 1
+			if existing, ok := best[qn]; ok && existing.Depth <= depth {
+				continue
+			}
+			best[qn] = CallerRef{
+				QualifiedName: qn,
+				Depth:         depth,
+				Weight:        cc.Weight * cfg.Decay,
+				Path:          append([]string{viaQN}, cc.Path...),
+				PreRename:     true,
+			}
+		}
+	}
+	expanded := make([]CallerRef, 0, len(best))
+	for _, c := range best {
+		expanded = append(expanded, c)
+	}
+	sort.Slice(expanded, func(i, j int) bool {
+		if expanded[i].Depth != expanded[j].Depth {
+			return expanded[i].Depth < expanded[j].Depth
+		}
+		return expanded[i].QualifiedName < expanded[j].QualifiedName
+	})
+	return expanded
+}