mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-21 00:38:18 +08:00
fix(go): variables declared in a var ( … ) block are indexed (#2682)
tree-sitter-go wraps the specs of a var group in a var_spec_list node, unlike a const group's, which sit directly in the declaration. Both extractors read only the specs directly inside a declaration, so every variable declared in a var ( ... ) block was missing from the graph: no node, no call or construction recorded from its initializer, and nothing for code that reads it, or for a call made through it, to point at. Each member of a var group is now a variable, as one declared on its own already was, with its own doc comment or the group's, and its initializer walked under it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<string, string | undefined> = {};
|
||||
|
||||
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);
|
||||
}
|
||||
});
|
||||
}
|
||||
});
|
||||
@@ -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<Node<'t>> = 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" },
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user