Skip to content

fix(go): an unexported name links its own package's declaration, and a local binding shadows the package - #2658

Merged
colbymchenry merged 3 commits into
mainfrom
claude/go-unexported-namesake
Oct 19, 2026
Merged

colbymchenry merged 3 commits into
mainfrom
claude/go-unexported-namesake

Conversation

@colbymchenry

Copy link
Copy Markdown
Owner

Go keeps an unexported name inside its package, yet prometheus tsdb's []*memSeries in head_append.go linked to nothing. The cause is a tie, settled by a veto:

  • Name matching scored head.go's memSeries and tsdb/agent's the same. Directory proximity counts shared leading directories, capped at 80 points, so a child package (or, in a deep tree, a sibling package) ties with the package's own directory.
  • The first candidate met, tsdb/agent's, then failed the visibility check, and the reference ended unresolved.
  • Parameters, results, struct fields, composite literals and calls all failed this way. Examples: prometheus's memSeries and its 1,185 sample{…} literals; kubernetes's simpleCache, pruner, client-go's per-version informer types (deploymentInformer{…} in apps/v1 against apps/v1beta1) and many test helpers.

Follow-up 1 from #2655, which found these failing from struct fields.

Fix

  • A Go candidate the reference cannot see is out of the pool before the ranking. matchByExactName already does this for C#, Java, PHP and Scala ("a visible namesake wins where the veto after it would drop the ref"). The rule is isGoNameVisible, now shared with isVisibleAcrossFiles' Go branch: an exported name is visible from anywhere, an unexported one only in its own directory.
  • A bare Go call or function value that the surrounding function binds itself is left unresolved. gateTargetKind checks isGoLocallyBound, which wraps the resolver's existing Go block-scope reader goLocalDecl.

Why the second rule is needed

  • With only the first rule, names with more namesakes than the ambiguity ceiling started resolving to their package's declaration, which is mostly right.
  • But it also exposed bare calls to a local closure named like a package function. Kubernetes had 80 of these new wrong links, for example client-go's init := func(t *testing.T) {…} then init(t) bound to func init(), which Go never lets a program name.
  • Go's scoping says a parameter or local shadows every declaration of its package from the end of its declaration on, so the gate applies at the seam every strategy passes through.
  • It also removes links that were wrong on main:
    • a test's var ( volumeSpec = volumeSpec(…) ) passed on later (the call inside the declaration keeps its link: the local isn't in scope there yet);
    • a loop's var ( transformer storagevalue.PrefixTransformer … );
    • a local errors map in kubernetes' e2e upgrade tests that linked to kubectl's errors method.

Validation

Before/after full indexes on main a944ec3, diffed per reference site.

repo added each judged dropped each judged metadata-only
gin 0 — 0 — 4
etcd 2 own-package calls 25 locally bound 1,356
prometheus 1,251 1,185 sample{…} + 55 refs (memSeries …) + 9 calls, own package; 2 method calls, right method 112 locally bound 1,996
kubernetes 670 543 calls + 120 literals + 4 refs, own package; 3 method calls, right method 542 locally bound 42,017
  • Added links: every bare name lands in its own package. The 5 calls written through a value are correct: db.Head().getOrCreate(…) is Head's own method, and ….cache.set(…) goes through envelopeTransformer's field cache *simpleCache to simpleCache.set.
  • Dropped links: an independent text check confirmed the local binding (a parameter, :=, var, range variable) for 646 of the 679. I read the remaining 19 kubernetes sites by hand; all are grouped var ( … ) declarations the text check doesn't model.
  • Metadata-only: the same edge to the same target, at exact-match confidence 0.9 instead of 0.7 (a few at framework 0.8 before), because the pool now holds only the package's own declaration. Nothing downstream changes: the viewer folds only edges below 0.6.
  • codegraph_explore on before/after indexes (8 flow queries on prometheus, 5 on kubernetes): kubernetes is byte-identical; prometheus renders the same symbols, with text-only differences on 6 queries.
  • The change is resolver-only, so the kernel binary is unchanged.

Tests

  • New __tests__/go-unexported-namesake.test.ts: an indexed module under kernel and wasm, LF and CRLF, with prometheus's shape (namesakes in tsdb/agent and tsdb/chunks). It covers:
    • a parameter, a result, a composite literal and a call reaching the package's own declarations;
    • a struct's fields;
    • the child package keeping its own;
    • a name only another package declares linking to nothing;
    • a local closure and a local init shadowing the package's newSeries and func init().
      Of its 20 tests, 12 fail on main; the 8 that pass are the controls.
  • Full suite on a944ec3 + this change (--maxWorkers=4 --minWorkers=1, kernel expected): 7,396 passed, 0 failed, 80 skipped. After merging fix(ts): a call on a known class goes through that class's own bases #2656, which also edits name-matcher.ts, the overlapping files were rerun: this test, ts-known-class-methods, resolution, cross-file-visibility, function-ref, go-struct-field-refs and go-type-position-kinds, 360 tests, all passing.

🤖 Generated with Claude Code

colbymchenry and others added 3 commits October 19, 2026 05:06
…a local binding shadows the package

Go keeps an unexported name inside its package, yet prometheus tsdb's
`[]*memSeries` in head_append.go linked to nothing. Name matching scored
head.go's `memSeries` and `tsdb/agent`'s the same, because directory
proximity counts shared leading directories (and caps at 80), so a child
or deep sibling package ties with the package's own directory. The first
one met, `tsdb/agent`'s, was then vetoed as invisible. Parameters,
results, struct fields, composite literals and calls all failed this way:
prometheus's `memSeries` and `sample{…}` (1,185 literals), and in
kubernetes client-go's per-version informer types and many test helpers.

- matchByExactName drops a Go candidate the reference cannot see before
  the ranking (isGoNameVisible, shared with isVisibleAcrossFiles' Go rule),
  as it already did for C#, Java, PHP and Scala, so the package's own
  declaration wins.
- That lets a name with more namesakes than the ambiguity ceiling resolve
  to its package's declaration. It also exposed bare calls to a local
  closure named like a package function: client-go's `init := func(…)`
  then `init(t)` bound to `func init()`. gateTargetKind now leaves a bare
  Go call or function value unresolved when the function around it binds
  the name itself (isGoLocallyBound: a parameter, `:=`, `var`), as Go's
  scoping says. The same rule drops existing wrong links: a test's
  `volumeSpec = volumeSpec(…)` var passed on, or a local `errors` map that
  linked to kubectl's `errors` method.

A/B on main a944ec3:
- gin: unchanged.
- etcd: +2 / -25.
- prometheus: +1251 / -112.
- kubernetes: +670 / -542.
Every added link is the package's own declaration, or the right method
for the 5 calls written through a value. Every dropped one is a name the
enclosing function binds (checked by text, the rest by hand). The other
changes are the same targets at confidence 0.9 instead of 0.7.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@colbymchenry
colbymchenry merged commit 6692e05 into main Oct 19, 2026
@colbymchenry
colbymchenry deleted the claude/go-unexported-namesake branch October 19, 2026 10:13
colbymchenry added a commit to inth3shadows/codegraph that referenced this pull request Oct 20, 2026
Main gained a gate for this while the PR was open (PR colbymchenry#2658's
isGoLocallyBound in gateTargetKind): a bare Go call or function value
the enclosing function binds is dropped after every strategy, frameworks
included. With this PR's scope fixes in place, every test here passes
with only that gate, so the earlier isGoShadowedBareName check is
removed rather than kept as a second copy of the same rule.

What the PR still changes is where a local's scope ends: a name a case
clause declares ends at the next clause of its switch or select, and a
func-typed result's parameter names are no longer read as the
function's own. On main those five cases lost a correct link to the
package function. The CHANGELOG bullet is folded into the one for the
gate, which already describes the shadowing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
colbymchenry added a commit that referenced this pull request Oct 22, 2026
#2776)

In Go, a function used as a value, like `apply(run)`, was recorded once
per function and name, at its first use. Since PR #2658 the resolver drops
a use that a local or parameter of that name shadows. When a function's
first use was the shadowed one and a later use, outside the local's scope,
was the package function, that later use had no reference left, and the
function lost its references edge to the package function (an inner-block
`run := 2`, a closure's `func(run int)`, a case clause, an if header or a
range variable, then `apply(run)`). Issue #2596 predicted this.

Fix, in both extractors (function-ref.ts with tree-sitter.ts's
flushFnRefCandidates, and the kernel's go.rs flush_fn_ref_candidates):
- A Go function that binds the name itself records one use per scope
  instead of one per function. Bindings are a func literal's parameters,
  :=, var, const, range and type-switch variables and a select case's
  value. Scopes are Go's: a block, an if/for/switch/select statement's
  implicit block and each case clause. Within one scope a local declared
  before the first use also covers every later use, so the first use
  still speaks for the scope.
- The function's own receiver, parameters and results keep a single
  reference: they cover the whole body.
- A func literal at file scope is its own unit.
- Every other function is unchanged. The scope and enclosing function are
  read from the tree only for names a function uses at least twice, after
  the name gate (reading them for every container made kernel extraction
  of prometheus 2.4x slower).

Validation:
- go-shadowed-function-value.test.ts: 14 cases, kernel and wasm, LF and
  CRLF. On main's sources and kernel 10 fail; with the fix 14 pass.
  Per-scope keys without the binding check, and dropping range
  variables, case clauses or func literal parameters, each fail 3.
- Kernel parity, Go: gin 99/99, etcd 1065/1105 (40 deferred, as main),
  prometheus 746/746, kubernetes 17779/17952 (173 deferred, as main),
  0 files with diffs.
- A/B against 6bd217a (fresh indexes, each arm with its own kernel):
  gin 0f09c3a9, etcd bf9cadf0, prometheus 017fcf2e and kubernetes
  006cdf87 keep identical nodes and edges. The only change is 0, 4, 13
  and 163 more function_ref rows, all failed: per-scope uses of a local
  (all 17 on etcd and prometheus and 36 on kubernetes read by hand).
  express and flask dump byte-identically.
- Cost: kernel extraction of prometheus 2.90 s vs 2.90 s, wasm 6.06 vs
  6.07 s; kubernetes 34.9 vs 34.9 s; prometheus index medians
  12.35 s / 49.1 s CPU vs 12.45 s / 48.3 s over 4 interleaved rounds.

Fixes #2596

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant