diff --git a/CHANGELOG.md b/CHANGELOG.md index 871dfffd1..b9e7a486b 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 000000000..24b805b84 --- /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 eec327f9d..0eab81d41 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 d3a514448..8f43536a6 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 22889804e..16678aa5e 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