Skip to content

fix(go): resolve package-qualified calls into the imported package - #764

Open
kunalgoesbyken wants to merge 1 commit into
trailhq:mainfrom
kunalgoesbyken:fix/go-package-qualified-calls-dropped
Open

kunalgoesbyken wants to merge 1 commit into
trailhq:mainfrom
kunalgoesbyken:fix/go-package-qualified-calls-dropped

Conversation

@kunalgoesbyken

Copy link
Copy Markdown
Contributor

Summary

Fixes #763

In Go, a call into another package is always written pkg.Fn(). calleeName reads it as a member call on a receiver named pkg. resolveRecvType finds no type for a package name, and resolveEdges drops member calls that have no recvType. So no call between packages ever got a calls edge, and graft callers / blast stopped at the package boundary.

Changes

  • extract.ts: collect each Go file's imports as local name → import path. The local name is the alias, or else the last path segment. When a selector call's receiver has no resolved type and matches one of those names, the call edge carries the import path as its specifier. Gleam module calls already work this way.
  • resolve.ts: a Go call with a specifier maps the import path to its in-repo package directory, using the module lookup that resolveGoImport already did, now split out as goImportDir. It then looks for the name among the functions in that directory's non-_test.go files. Exactly one match gives an extracted edge. Anything else drops: stdlib or third-party packages, no match, or several matches.

Receivers that already resolve to a type (receiver var, bound local, struct field) are untouched, because the package lookup only runs when resolveRecvType returns nothing. Lookup is limited to the imported package's directory, so a function with the same name in another package can't be picked.

With the issue's reproducer:

$ graft callers Helper            # before
Helper · function · pkg/util/util.go:L3-L3
  no indexed callers — ...

$ graft callers Helper            # after
Helper · function · pkg/util/util.go:L3-L3
  calls ← run (main.go:L7-L7)
      7: func run() { util.Helper() }

Out of scope

  • Dot-imports (import . "pkg") and calls through package-level function variables still get no edge.
  • If a package's name differs from the last segment of its directory and the import has no alias, the call still drops rather than guessing.
  • A local variable that shadows an import's name and has no inferred type is read as the package. Go code rarely does this, since the shadow hides the package for the rest of that scope.

Testing

On Windows 11 with Node 26:

  • Added a test to test/graph-go.test.ts. A module with pkg/util/{helper,format}.go, plus pkg/other/other.go that defines the same names. main.go calls util.Helper() (next to fmt.Println), and alt.go calls ut.Format() through an aliased import. It asserts that run calls exactly pkg/util/helper.go#Helper and alt calls exactly pkg/util/format.go#Format.
    • Before the fix it fails: actual: [], expected: ['pkg/util/helper.go#Helper'].
    • After the fix: test/graph-go.test.ts passes 7/7.
  • tsc -p tsconfig.json --noEmit is clean.
  • test/graph-*.test.ts, test/blast*.test.ts and test/graphrank.test.ts pass (424 pass, 0 fail).

@trailhq-graft

trailhq-graft Bot commented Nov 8, 2026 •

Copy link
Copy Markdown

🌱 graft blast radius

1 area changed → 4 areas can be affected. 8 dependent symbols, depth 2.
Tests: Import Resolution has tests the diff did not touch.
Tag: @anirudhkumar-nanonets — 4 of 5 areas · @shhdwi — 3 of 5 areas · @Faeeth — Symbol Overlap Detection

flowchart TB
  A0(("Dependency Graph Construction<br/>5 symbols"))
  A1(("Graph Engine<br/>1 symbol"))
  A2(("Pull Request Review<br/>1 symbol"))
  A3(("Symbol Overlap Detection<br/>1 symbol"))
  classDef reached fill:#D9EDF3,stroke:#3AA7C9,stroke-width:1.5px,color:#0E313C;
  class A0,A1,A2,A3 reached;
Loading
Can be affected Symbols Nearest hop Reached from
Dependency Graph Construction 5 src/graph/build.ts:L214-L483 buildGraph — calls, depth 1 Import Resolution
Graph Engine 1 src/engine.ts:L97-L108 graph — calls, depth 2 Import Resolution
Pull Request Review 1 src/app/review.ts:L45-L99 reviewPullRequest — calls, depth 2 Import Resolution
Symbol Overlap Detection 1 src/notes/edit-overlap.ts:L79-L94 symbolsIn — calls, depth 2 Import Resolution
Who knows this code — 3 people across 5 areas
Area Who knows it
Import Resolution · changed @shhdwi — 14 commits, last 12d ago · @anirudhkumar-nanonets — 10 commits, last yesterday
Dependency Graph Construction · affected @anirudhkumar-nanonets — 22 commits, last 6d ago · @shhdwi — 9 commits, last 12d ago
Graph Engine · affected @anirudhkumar-nanonets — 17 commits, last 26d ago
Pull Request Review · affected @anirudhkumar-nanonets — 3 commits, last 2mo ago
Symbol Overlap Detection · affected @Faeeth — 1 commit, last today · @shhdwi — 1 commit, last 16d ago

Ownership is git history over each area's own files, weighted towards recent work (120-day half-life). Merge commits and bots are dropped, and you are dropped from your own PR. A name with no @ has no GitHub handle in its commit email — tag them by hand, or add a .mailmap entry. A suggestion from history, not a CODEOWNERS rule.

All 8 dependent symbols, grouped by area

Dependency Graph Construction — 5 symbols in 5 files

  • src/graph/build.ts:L214-L483 — buildGraph (calls, depth 1)
    234: const rawEdges: RawEdge[] = [];
  • src/graph/check.ts:L59-L159 — checkGraph (calls, depth 2)
  • src/graph/container.ts:L153-L214 — extractContainer (calls, depth 2)
    155: const rawEdges: RawEdge[] = [];
  • src/graph/refresh-run.ts:L18-L21 — main (calls, depth 2)
  • src/graph/refresh.ts:L202-L225 — rebuild (calls, depth 2)

Graph Engine — 1 symbol in 1 file

  • src/engine.ts:L97-L108 — graph (calls, depth 2)

Pull Request Review — 1 symbol in 1 file

  • src/app/review.ts:L45-L99 — reviewPullRequest (calls, depth 2)

Symbol Overlap Detection — 1 symbol in 1 file

  • src/notes/edit-overlap.ts:L79-L94 — symbolsIn (calls, depth 2)
Test signal per changed area — 1 ⚠

Reached = a node under a test path has a resolved edge into the changed symbol. It undercounts anything called indirectly — through a CLI, a spawned process or a dynamic import — so read a low ratio as “look here”, never as a coverage gate.

  • ⚠ Import Resolution — 1 of 8 reached · 3 test files reach it, none changed here
    • not reached: extractTree, walk, visit, declare, visit, goImportDir, resolveGoImport
48 test suites also reference this code

58 symbols, kept out of the diagram and the table so they cannot crowd out the areas a reviewer has to look at.

  • test/ask-index.test.ts
  • test/ask.test.ts
  • test/claude-hooks.test.ts
  • test/claude-prompt-private.test.ts
  • test/container-extract.test.ts
  • test/context-only-dir.test.ts
  • test/context.test.ts
  • test/covers.test.ts
  • test/depth-grammars.test.ts
  • test/depth-parity.ts
  • test/generic-extract.test.ts
  • test/generic-node24-probe.ts
  • test/graph-bindings.test.ts
  • test/graph-cross-language.test.ts
  • test/graph-extensions.test.ts
  • test/graph-extract-dedup.test.ts
  • test/graph-incremental.test.ts
  • test/graph-invariants.test.ts
  • test/graph-java.test.ts
  • test/graph-languages.test.ts
  • …28 more

graft blast · origin/main...HEAD · depth 2 · 3 changed files

Open the interactive graph → — click an area to see its dependent symbols at file:line.

github-actions Bot added a commit that referenced this pull request Nov 8, 2026
@shhdwi

shhdwi commented Nov 8, 2026

Copy link
Copy Markdown
Collaborator

Thanks, this works: callers Helper now finds run in main.go, and aliased imports resolve too. One false edge before it can merge: when a local variable shadows the import name and its type is unknown, the call goes to the package. For example, config := load(); config.Get("x") gives use -> config/config.go#Get marked extracted, but the real target is Cfg.Get. Main has no edge there. Go code like user, err := user.Load(...) hits this a lot. Could you skip the package lookup when the name is declared locally in the function (params, :=, var), with a test? Also #766 just merged and appends a test at the end of test/graph-go.test.ts, so keep both when you rebase.

A Go call into another package is always `pkg.Fn()`. It was read as a
member call on a receiver named `pkg`, which has no type, so resolve
dropped it and no cross-package call ever got an edge. Map each import's
local name to its path, carry that path on the call, and resolve the name
among the non-test functions in the package's directory.

A parameter, `:=`, `var`, range or type-switch binding that shadows the
import name is not the package once it is in scope, so calls on it take
no package edge. The right side of `user, err := user.Load()` still does.
@kunalgoesbyken
kunalgoesbyken force-pushed the fix/go-package-qualified-calls-dropped branch from 52ceafd to 6b97f00 Compare November 8, 2026 12:46
@kunalgoesbyken

Copy link
Copy Markdown
Contributor Author

Thanks for catching that. Fixed: the package lookup now skips any name declared locally in the function (params, :=, var, plus range and type-switch bindings), with a test for each. A binding only counts once it is in scope, so in user, err := user.Load(...) the right-hand user.Load still resolves to the package, and later user.Foo() calls take no package edge. Rebased on main with #766's test kept alongside this one.

github-actions Bot added a commit that referenced this pull request Nov 8, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Go: calls through an imported package (util.Helper()) produce no call edge, so callers misses every cross-package caller

2 participants