diff --git a/CHANGELOG.md b/CHANGELOG.md index 58844f9a5..b226caacb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Go, a declaration's documentation is now just the comment written right above it, as `go doc` shows it. A section banner, a TODO or commented-out code that a blank line separates from a function, type, constant or variable is no longer added to the front of its documentation, or taken as its documentation when it has none of its own. Re-index Go projects after upgrading. - In Go, a comment directive written above a declaration, like `//go:embed static`, `//go:generate stringer -type=Kind` or `//nolint:errcheck`, is no longer part of its documentation, as `go doc` shows it, so a variable whose only comment is a `//go:embed` line no longer has that line as its description. Re-index Go projects after upgrading. - In Go, a constant declared inside a `const ( … )` group now keeps the doc comment written above it, like gin's `// ErrorTypeBind is used when Context.Bind() fails.`, so searching for what a constant means can find it. Before, every member of the group got the group's comment, or nothing when the group had none; a member without a comment of its own still gets the group's. Re-index Go projects after upgrading. +- In Go, variables declared in a `var ( … )` block, like gin's `binding.JSON` or prometheus's metric counters, are now indexed, with their own doc comments. Before, every variable in such a block was missing from the graph: the calls and types in its value were not recorded, so a struct used only there looked unused, and a call made through it, like `fPointPool.Get()` or a counter's `.Inc()`, was matched by its method's name alone and could link to an unrelated type's method. Re-index Go projects after upgrading. - Indexing a project that includes large bundled JavaScript files, such as a copy of pdf.js or d3, is fast again: since 1.6.2, resolving the calls in a JavaScript or TypeScript file re-read the file's text above each call, so a single bundled library could add many seconds to an index. The graph it builds is unchanged. Thanks @bompus for the report. (#2334) - Indexing a project that ships a large minified JavaScript file, such as the GraphiQL bundle inside go-ethereum, no longer stalls at "Resolving refs" when CodeGraph runs on Node 22, and linking such a file is faster on any Node version. The graph it builds is unchanged. - The time `codegraph init` and `codegraph index` print beside the node and edge counts now covers the whole run, resolving references and linking included, and `codegraph sync` reports its whole run the same way. Before, it counted only reading and parsing the files, which can be a small part of an index, so a slow index looked fast. Thanks @bompus for the report. (#2334) diff --git a/__tests__/fixtures/kernel-parity/torture.go b/__tests__/fixtures/kernel-parity/torture.go index e227ee457..eaea20f30 100644 --- a/__tests__/fixtures/kernel-parity/torture.go +++ b/__tests__/fixtures/kernel-parity/torture.go @@ -21,6 +21,13 @@ var handlerTable = map[string]func(int){ "recv": TargetCb, } +// Registries: the group's comment, for a member without its own. +var ( + // handlersByName is documented inside its group. + handlersByName = map[string]func(int){"recv": TargetCb} + DefaultWidget = NewRegistry() +) + // Widget is documented above its own type declaration. type Widget struct { *Base diff --git a/__tests__/go-var-groups.test.ts b/__tests__/go-var-groups.test.ts new file mode 100644 index 000000000..7f273337e --- /dev/null +++ b/__tests__/go-var-groups.test.ts @@ -0,0 +1,164 @@ +/** + * Variables declared in a Go `var ( … )` block were not indexed at all: + * + * var ( + * // ErrNoSides is returned for a shape without sides. + * ErrNoSides = errors.New("no sides") + * ErrTooMany = newError("too many sides") + * ) + * + * tree-sitter-go wraps a var group's specs in a `var_spec_list`, unlike a + * `const ( … )` group's, and both extractors read only the specs directly + * inside the declaration. So the variables had no nodes, a call in their + * initializers (`newError(…)`) was never recorded, and code reading them had + * nothing to point at. Each member is now a variable with its own doc comment + * (or the group's), its initializer walked under it, as a variable declared on + * its own already was. + * + * Runs against the native kernel (when built) and the wasm extractor, which + * must agree. + */ +import { describe, it, expect, beforeAll, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as path from 'path'; +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 SOURCE = `// Package shapes is documented. +package shapes + +import ( + "errors" + "fmt" +) + +// Errors returned by the shapes package. +var ( + // ErrNoSides is returned for a shape without sides. + ErrNoSides = errors.New("no sides") + ErrTooMany = newError("too many sides") +) + +var ( + registry = map[string]int{} + // DefaultName is documented inside its group. + DefaultName string +) + +// single is documented above its own declaration. +var single = 3 + +func newError(msg string) error { return fmt.Errorf("shapes: %s", msg) } + +func check(n int) error { + if n == 0 { + return ErrNoSides + } + return ErrTooMany +} +`; + +const ENV_KEYS = ['CODEGRAPH_KERNEL', 'CODEGRAPH_KERNEL_LANGS'] as const; + +describe('Go variables declared in a var ( … ) block', () => { + 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('shapes/shapes.go', source, 'go'); + } + delete process.env.CODEGRAPH_KERNEL; + process.env.CODEGRAPH_KERNEL_LANGS = 'all'; + const result = tryKernelExtract('shapes/shapes.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 ? SOURCE.replace(/\n/g, '\r\n') : SOURCE); + const byName = new Map(result.nodes.map((n) => [`${n.kind} ${n.name}`, n])); + const node = (kind: string, name: string) => { + const found = byName.get(`${kind} ${name}`); + expect(found, `${kind} ${name}`).toBeDefined(); + return found!; + }; + const file = result.nodes.find((n) => n.kind === 'file')!; + + // Every member of a var group is a variable the file contains, with its + // own doc comment, or the group's when it has none. + const docs = Object.fromEntries( + ['ErrNoSides', 'ErrTooMany', 'registry', 'DefaultName', 'single'].map((name) => [ + name, + node('variable', name).docstring ?? null, + ]) + ); + expect(docs).toEqual({ + ErrNoSides: 'ErrNoSides is returned for a shape without sides.', + ErrTooMany: 'Errors returned by the shapes package.', + registry: null, + DefaultName: 'DefaultName is documented inside its group.', + single: 'single is documented above its own declaration.', + }); + for (const name of ['ErrNoSides', 'ErrTooMany', 'registry', 'DefaultName']) { + const id = node('variable', name).id; + expect( + result.edges.some((e) => e.kind === 'contains' && e.source === file.id && e.target === id), + `file contains ${name}` + ).toBe(true); + } + + // A call in a member's initializer is that variable's. + const tooMany = node('variable', 'ErrTooMany').id; + expect( + result.unresolvedReferences.some( + (r) => r.fromNodeId === tooMany && r.referenceKind === 'calls' && r.referenceName === 'newError' + ), + 'ErrTooMany calls newError' + ).toBe(true); + + // Code reading a member in the same file references it. + const check = node('function', 'check').id; + for (const name of ['ErrNoSides', 'ErrTooMany']) { + const id = node('variable', name).id; + expect( + result.edges.some((e) => e.kind === 'references' && e.source === check && e.target === id), + `check references ${name}` + ).toBe(true); + } + }); + } +}); diff --git a/codegraph-kernel/src/go.rs b/codegraph-kernel/src/go.rs index d27198c6c..d5c2166b5 100644 --- a/codegraph-kernel/src/go.rs +++ b/codegraph-kernel/src/go.rs @@ -682,8 +682,18 @@ impl<'t> Walker<'t> { let docstring = self.docstring_of(node); let is_const_decl = node.kind() == "const_declaration"; + // A `var ( … )` group wraps its specs in a `var_spec_list`; a + // `const ( … )` group holds them directly. + let mut specs: Vec> = Vec::new(); for i in 0..node.named_child_count() { - let Some(spec) = node.named_child(i) else { continue }; + let Some(child) = node.named_child(i) else { continue }; + if child.kind() == "var_spec_list" { + specs.extend((0..child.named_child_count()).filter_map(|j| child.named_child(j))); + } else { + specs.push(child); + } + } + for spec in specs { if !matches!(spec.kind(), "var_spec" | "const_spec") { continue; } @@ -697,9 +707,9 @@ impl<'t> Walker<'t> { None }; let signature = value_node.map(|v| util::init_signature(self.text(v))); - // A member of a `const ( … )` group has its own doc - // comment beside it in the parentheses; one without keeps - // the group's. + // A member of a `const ( … )` or `var ( … )` group has its + // own doc comment beside it in the parentheses; one without + // keeps the group's. let doc = self.docstring_of(spec).or_else(|| docstring.clone()); var_row = self.create_node( if is_const_decl { "constant" } else { "variable" }, diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 2b7f0dfdb..0ee4595c8 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -3558,10 +3558,12 @@ export class TreeSitterExtractor { } } else if (this.language === 'go') { // Go: var_declaration, short_var_declaration, const_declaration - // These can have multiple identifiers on the left - const specs = node.namedChildren.filter(c => - c.type === 'var_spec' || c.type === 'const_spec' - ); + // These can have multiple identifiers on the left. A `var ( … )` group + // wraps its specs in a `var_spec_list`; a `const ( … )` group holds them + // directly. + const specs = node.namedChildren + .flatMap((c) => (c.type === 'var_spec_list' ? c.namedChildren : [c])) + .filter((c) => c.type === 'var_spec' || c.type === 'const_spec'); for (const spec of specs) { const nameNode = spec.namedChild(0); @@ -3572,8 +3574,8 @@ export class TreeSitterExtractor { const initValue = valueNode ? getNodeText(valueNode, this.source).slice(0, 100) : undefined; const initSignature = initValue ? `= ${initValue}${initValue.length >= 100 ? '...' : ''}` : undefined; - // A member of a `const ( … )` group has its own doc comment beside - // it in the parentheses; one without keeps the group's. + // A member of a `const ( … )` or `var ( … )` group has its own doc + // comment beside it in the parentheses; one without keeps the group's. varNode = this.createNode(node.type === 'const_declaration' ? 'constant' : 'variable', name, spec, { docstring: this.docstringFor(spec) ?? docstring, signature: initSignature,