From c79a515c41531a3b21cf6253a8d99b01e0222ae0 Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Fri, 30 Oct 2026 00:38:21 -0500 Subject: [PATCH] fix(go): a package-level var or const named in uppercase is exported Both extractors created a Go var/const node from its var_spec or const_spec without exportedness: the TS Go branch passed no isExported (the declaration-level value is read off the var_declaration, which has no name), and the kernel's extract_variable set none. Every such node was unexported, so findGoPackageMember, which looks up pkg.Name among the package's exported declarations, skipped it: a call through `var Hook = func...` from another package linked to nothing, though the same call inside the package linked to the variable. Both engines now mark the node exported by its own name's case, read from the spec through goExtractor.isExported / is_exported, as functions, methods and types already were. Validation: - __tests__/go-package-value-exported.test.ts, 16 tests (extraction on the kernel and wasm, LF and CRLF; cross-package calls on both paths): 12 fail on the base main, 6 with only the TS half (the kernel checks), all pass with the fix. - A/B, main vs fix, kernel path: gin 108 var/const flags, 0 edges; prometheus 625 flags, 0 edges; etcd 806 flags and 12 new calls, all right (e2e.GetVersionFromBinary x11 and model.NonDeterministicModel, both `var X = func(...)`); flask and express byte-identical. The fix's kernel and wasm graphs are byte-identical on all three Go repos, and the wasm A/B matches. - kernel-parity --lang go: gin 99/99, prometheus 755/755, etcd 1065/1105 with 40 deferred on main too; 0 diffs. - etcd index time, median of 3 interleaved: 13.4 s main, 12.6 s fix. Reimplements PR #1521 by ericquan8. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/go-package-value-exported.test.ts | 211 ++++++++++++++++++++ codegraph-kernel/src/go.rs | 8 +- src/extraction/languages/go.ts | 6 +- src/extraction/tree-sitter.ts | 5 + 5 files changed, 227 insertions(+), 4 deletions(-) create mode 100644 __tests__/go-package-value-exported.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 871dfffd10..b9e7a486b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -275,6 +275,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Go, a struct now links to the types its fields are declared with, like etcd's `PutResponse` holding a `*ResponseHeader` or gin's `Context` holding a `HandlersChain`, so impact on `ResponseHeader` or `HandlersChain` reaches every struct that holds one and the code that uses those structs, and `codegraph_node` lists a struct's field types. Before, a struct linked only to the types it embeds. A field written through another package, like `*pb.ResponseHeader`, links to that package's type, one from outside your project, like `context.Context`, links to nothing, the types an embedded generic is filled in with, like the `Pod` in client-go's `listers.ResourceIndexer[*v1.Pod]`, link too, and a struct that holds itself, like a list node's `next *node`, doesn't link to itself. `codegraph_explore` still answers a question about a flow with the code that runs it, not with the structs that merely hold one of its types. Re-index Go projects after upgrading. - In Go, an unexported name used in another file of its package, like prometheus's `memSeries` in `tsdb/head_append.go`, now links to that package's own declaration when a package nested inside it, like `tsdb/agent`, declares the same name. Before, such a parameter, result, struct field, composite literal or call linked to nothing. A call or a function value that the surrounding function binds itself, like a test's `init := func(…)` followed by `init(t)`, or a local `volumeSpec` variable passed on, no longer links to the package's declaration of that name. A name bound in one `case` of a `switch` or `select`, or a parameter named inside a function-typed result like `func() func(run string)`, is not such a binding anywhere else, so a call there still links. Thanks @inth3shadows. (#2580) Re-index Go projects after upgrading. - In Go, a method whose name starts with an uppercase letter, like `func (w *writer) Close()`, is now marked exported, as a function of that name already was. Before, every Go method was marked unexported, so dead-code results listed exported methods that code outside the project may call. Re-index Go projects after upgrading. Thanks @danusha2345. (#1734) +- In Go, a package-level variable or constant whose name starts with an uppercase letter, like `var Hook = func() int {…}` or `const Limit = 3`, alone or inside a `var ( … )` or `const ( … )` group, is now marked exported, as a function or type of that name already was. A call from another package through such a variable, like etcd's `e2e.GetVersionFromBinary(path)`, now links to it; before, it linked to nothing, though the same call from inside the package did. Re-index Go projects after upgrading. Thanks @ericquan8. (#1521) - In Go, when a function uses a package function as a value, like `apply(run)`, and also has a local or parameter of the same name somewhere, like a `run := 2` inside an `if` or a `case`, a loop variable, or a closure's parameter, each use is now read on its own: a use the local covers links to nothing, and a use outside it links to the package function. Before, only the function's first use was looked at, so a local used first hid every later use of the package function. Re-index Go projects after upgrading. Thanks @inth3shadows for the report. (#2596) - In Go, a method an interface declares, like etcd's `KV.Put(ctx context.Context, key, val string, opts ...OpOption) (*PutResponse, error)`, now links to the types its parameters and results name, so impact on `PutResponse` or `OpOption` reaches the interface method and every caller that goes through the interface. Before, an interface's methods linked to no types at all. Re-index Go projects after upgrading. - In Go, a package imported under an explicit name, like `v3 "go.etcd.io/etcd/client/v3"`, now owns that name even when another import's path ends in it, like `"github.com/cheggaaa/pb/v3"` (package `pb`) in etcd's benchmark tool. Before, whichever of the two came first took every `v3.` name, so `v3.Client` and `v3.Op` could link to nothing. Re-index Go projects after upgrading. diff --git a/__tests__/go-package-value-exported.test.ts b/__tests__/go-package-value-exported.test.ts new file mode 100644 index 0000000000..24b805b84e --- /dev/null +++ b/__tests__/go-package-value-exported.test.ts @@ -0,0 +1,211 @@ +/** + * A Go package-level variable or constant is exported by its own name's case, + * as a function or a type is: + * + * var Hook = func() int { return 1 } // exported + * var ( + * GroupedHook = func() int { return 2 } // exported + * grouped = 0 // package-local + * ) + * const Limit = 3 // exported + * + * Both extractors read exportedness off the declaration node, which has no + * name (the names sit on its `var_spec` / `const_spec` children), so every + * var and const was unexported. Another package's `a.Hook()` then found no + * exported `Hook` in package `a` and linked to nothing, though a call from + * inside the package linked to the variable. + * + * Runs against the native kernel (when built) and the wasm extractor, which + * must agree. + */ +import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; +import { extractFromSource } from '../src/extraction'; +import { initGrammars, loadGrammarsForLanguages } from '../src/extraction/grammars'; +import { tryKernelExtract, resetKernelForTests } from '../src/extraction/kernel'; +import type { ExtractionResult } from '../src/types'; + +const KERNEL_PATH = path.join( + __dirname, + '..', + 'codegraph-kernel', + 'prebuilds', + `${process.platform}-${process.arch}`, + 'codegraph-kernel.node' +); +const kernelAvailable = fs.existsSync(KERNEL_PATH) || process.env.CODEGRAPH_KERNEL_EXPECT === '1'; + +const PACKAGE_A = `package a + +// Hook is a package-level function value. +var Hook = func() int { return 1 } + +var ( + // GroupedHook sits in a var group. + GroupedHook = func() int { return 2 } + grouped = 0 +) + +var hidden = func() int { return 3 } + +var Typed, second int + +const Limit = 3 + +const ( + ModeA = iota + modeB +) + +type Client struct{} + +func (c *Client) Do() int { return 4 } + +func helper() int { return Hook() + hidden() + grouped } +`; + +const PACKAGE_B = `package b + +import "example.com/app/a" + +func Use() int { + return a.Hook() + a.GroupedHook() + a.Limit + a.hidden() +} +`; + +const ENV_KEYS = ['CODEGRAPH_KERNEL', 'CODEGRAPH_KERNEL_LANGS'] as const; + +describe('Go package-level variables and constants are exported by their name', () => { + let savedEnv: Record = {}; + + beforeAll(async () => { + await initGrammars(); + await loadGrammarsForLanguages(['go']); + }); + + beforeEach(() => { + savedEnv = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]])); + resetKernelForTests(); + }); + + afterEach(() => { + for (const k of ENV_KEYS) { + if (savedEnv[k] === undefined) delete process.env[k]; + else process.env[k] = savedEnv[k]; + } + resetKernelForTests(); + }); + + function extract(backend: 'kernel' | 'wasm', source: string): ExtractionResult { + if (backend === 'wasm') { + process.env.CODEGRAPH_KERNEL = '0'; + return extractFromSource('a/a.go', source, 'go'); + } + delete process.env.CODEGRAPH_KERNEL; + process.env.CODEGRAPH_KERNEL_LANGS = 'all'; + const result = tryKernelExtract('a/a.go', source, 'go'); + expect(result, 'kernel extraction').not.toBeNull(); + return result!; + } + + const backends = kernelAvailable ? (['kernel', 'wasm'] as const) : (['wasm'] as const); + + for (const crlf of [false, true]) { + it.each(backends)(`%s${crlf ? ' (CRLF)' : ''}`, (backend) => { + const result = extract(backend, crlf ? PACKAGE_A.replace(/\n/g, '\r\n') : PACKAGE_A); + const exported = Object.fromEntries( + result.nodes + .filter((n) => n.kind === 'variable' || n.kind === 'constant') + .map((n) => [`${n.kind} ${n.name}`, n.isExported]) + ); + // Every spec's node says, as a boolean, whether its name is exported. + expect(exported).toEqual({ + 'variable Hook': true, + 'variable GroupedHook': true, + 'variable grouped': false, + 'variable hidden': false, + 'variable Typed': true, + 'constant Limit': true, + 'constant ModeA': true, + 'constant modeB': false, + }); + // Functions, methods and types keep theirs. + const flag = (kind: string, name: string) => result.nodes.find((n) => n.kind === kind && n.name === name)?.isExported; + expect(flag('struct', 'Client')).toBe(true); + expect(flag('method', 'Do')).toBe(true); + expect(flag('function', 'helper')).toBe(false); + }); + } +}); + +describe.each([ + ['default', 'LF'], + ['wasm', 'LF'], + ['default', 'CRLF'], + ['wasm', 'CRLF'], +])('a call through another package reaches its exported variable (%s, %s)', (backend, eol) => { + let root = ''; + let cg: CodeGraph | undefined; + let kernel: string | undefined; + + beforeAll(async () => { + kernel = process.env.CODEGRAPH_KERNEL; + if (backend === 'wasm') process.env.CODEGRAPH_KERNEL = '0'; + else delete process.env.CODEGRAPH_KERNEL; + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-go-value-exported-')); + const files: Record = { + 'go.mod': 'module example.com/app\n\ngo 1.21\n', + 'a/a.go': PACKAGE_A, + 'b/b.go': PACKAGE_B, + }; + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), eol === 'CRLF' ? content.replace(/\n/g, '\r\n') : content); + } + cg = await CodeGraph.init(root, { index: true }); + }, 60_000); + + afterAll(() => { + cg?.destroy(); + if (root) fs.rmSync(root, { recursive: true, force: true }); + if (kernel === undefined) delete process.env.CODEGRAPH_KERNEL; + else process.env.CODEGRAPH_KERNEL = kernel; + }); + + const nodeNamed = (file: string, name: string) => { + const nodes = cg!.getNodesInFile(file).filter((n) => n.name === name && n.kind !== 'import'); + expect(nodes, `${name} in ${file}`).toHaveLength(1); + return nodes[0]!; + }; + + /** `kind file:name` of each call out of `name` in `file`, sorted. */ + const callsFrom = (file: string, name: string) => + cg!.getOutgoingEdgesFrom([nodeNamed(file, name).id]) + .filter((e) => e.kind === 'calls') + .map((e) => `${e.kind} ${cg!.getNode(e.target)!.filePath}:${cg!.getNode(e.target)!.name}`) + .sort(); + + it('a.Hook() and a.GroupedHook() call the variables package a exports', () => { + expect(callsFrom('b/b.go', 'Use')).toEqual([ + 'calls a/a.go:GroupedHook', + 'calls a/a.go:Hook', + ]); + }); + + it('an unexported variable stays inside its package', () => { + const unresolved = cg!.getUnresolvedReferencesFrom(nodeNamed('b/b.go', 'Use').id) + .filter((r) => r.referenceKind === 'calls') + .map((r) => r.referenceName); + expect(unresolved).toEqual(['a.hidden']); + }); + + it('calls inside the package still reach its variables', () => { + expect(callsFrom('a/a.go', 'helper')).toEqual([ + 'calls a/a.go:Hook', + 'calls a/a.go:hidden', + ]); + }); +}); diff --git a/codegraph-kernel/src/go.rs b/codegraph-kernel/src/go.rs index eec327f9d9..0eab81d41d 100644 --- a/codegraph-kernel/src/go.rs +++ b/codegraph-kernel/src/go.rs @@ -341,7 +341,8 @@ impl<'t> Walker<'t> { Some(sig) } - /// goExtractor.isExported: uppercase first letter of the name field. + /// goExtractor.isExported: uppercase first letter of the name field (a + /// var_spec / const_spec's first name included). fn is_exported(&self, node: Node) -> bool { if let Some(name_node) = node.child_by_field_name("name") { let text = self.text(name_node); @@ -722,11 +723,14 @@ impl<'t> Walker<'t> { // own doc comment beside it in the parentheses; one without // keeps the group's. let doc = self.docstring_of(spec).or_else(|| docstring.clone()); + // Exported by its own name's case, read from the spec (the + // declaration around it has no name). + let is_exported = Some(self.is_exported(spec)); var_row = self.create_node( if is_const_decl { "constant" } else { "variable" }, &name, spec, - Extra { docstring: doc, signature, ..Extra::default() }, + Extra { docstring: doc, signature, is_exported, ..Extra::default() }, ); } } diff --git a/src/extraction/languages/go.ts b/src/extraction/languages/go.ts index d3a5144481..8f43536a66 100644 --- a/src/extraction/languages/go.ts +++ b/src/extraction/languages/go.ts @@ -216,8 +216,10 @@ export const goExtractor: LanguageExtractor = { }, isExported: (node, source) => { // Go: a symbol is exported when its identifier starts with an uppercase letter. - // Look at the `name` field directly (works for function_declaration, - // method_declaration, type_spec, and var_spec / const_spec via extractor flow). + // Look at the `name` field directly: function_declaration, + // method_declaration, type_spec, and a var_spec / const_spec, whose first + // name is the one the extractor gives a node (never the declaration + // around a spec, which has no name). const nameNode = getChildByField(node, 'name'); if (nameNode) { const text = getNodeText(nameNode, source); diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 22889804ed..16678aa5eb 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -4149,9 +4149,14 @@ export class TreeSitterExtractor { // A member of a `const ( … )` or `var ( … )` group has its own doc // comment beside it in the parentheses; one without keeps the group's. + // Exported by its own name's case, read from the spec: the + // declaration around it has no name, so `var Hook = …` and + // `const Limit = 3` read as unexported and `a.Hook()` from another + // package linked to nothing. varNode = this.createNode(node.type === 'const_declaration' ? 'constant' : 'variable', name, spec, { docstring: this.docstringFor(spec) ?? docstring, signature: initSignature, + isExported: this.extractor.isExported?.(spec, this.source), }); } // Walk the initializer so composite literals and calls in a