mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-24 19:30:10 +08:00
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 6bd217a2 (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>
276 lines
8.1 KiB
TypeScript
276 lines
8.1 KiB
TypeScript
/**
|
|
* A Go function used as a value is recorded once per function, at its first
|
|
* use (#756). Since #2658 a use that a parameter or local of the same name
|
|
* shadows links to nothing, which is right for that use. But when the first
|
|
* use in a function is the shadowed one and a later use, outside the local's
|
|
* scope, is the package function, the later use had no reference left: the
|
|
* function lost its `references` edge to the package function (#2596).
|
|
*
|
|
* A function that binds the name itself now records one use per scope (a
|
|
* block, a statement's implicit block, a case clause), so the later use keeps
|
|
* its own reference. Within one scope a local declared before the first use
|
|
* covers every later use as well, so the first use still speaks for the
|
|
* scope. A function that binds no such local keeps its single reference, and
|
|
* so does one whose own parameter has the name: that covers the whole body.
|
|
*/
|
|
import { describe, it, expect, beforeAll, beforeEach, afterEach, afterAll } 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 GO_MOD = 'module example.com/app\n\ngo 1.22\n';
|
|
|
|
/** Line numbers are pinned by the assertions below: keep the layout. */
|
|
const MAIN_GO = `package main
|
|
|
|
func run(a, b int) int { return a + b }
|
|
|
|
func apply(f any) {}
|
|
|
|
func pick(f any) any { return f }
|
|
|
|
func shadowThenUse(x int) {
|
|
if x > 0 {
|
|
run := 2
|
|
_ = run
|
|
}
|
|
apply(run)
|
|
}
|
|
|
|
func closureThenUse() {
|
|
g := func(run int) { _ = run }
|
|
g(1)
|
|
apply(run)
|
|
}
|
|
|
|
func caseThenUse(x int) {
|
|
switch x {
|
|
case 1:
|
|
run := 3
|
|
_ = run
|
|
case 2:
|
|
apply(run)
|
|
}
|
|
}
|
|
|
|
func headerThenUse() {
|
|
if run := pick(nil); run != nil {
|
|
_ = run
|
|
}
|
|
apply(run)
|
|
}
|
|
|
|
func rangeThenUse(xs []int) {
|
|
for _, run := range xs {
|
|
_ = run
|
|
}
|
|
apply(run)
|
|
}
|
|
|
|
func useThenShadow(x int) {
|
|
apply(run)
|
|
if x > 0 {
|
|
run := 2
|
|
_ = run
|
|
}
|
|
apply(run)
|
|
}
|
|
|
|
func allShadowed(run func(int, int) int) {
|
|
apply(run)
|
|
if true {
|
|
apply(run)
|
|
}
|
|
}
|
|
|
|
func noBinding(x int) {
|
|
apply(run)
|
|
if x > 0 {
|
|
apply(run)
|
|
}
|
|
}
|
|
|
|
func paramValue(run func(int, int) int) {
|
|
apply(run)
|
|
}
|
|
|
|
func localValue() {
|
|
run := 1
|
|
_ = run
|
|
}
|
|
|
|
func main() {}
|
|
|
|
var literalThenUse = func(x int) {
|
|
if x > 0 {
|
|
run := 2
|
|
_ = run
|
|
}
|
|
apply(run)
|
|
}
|
|
`;
|
|
|
|
/** `<function> L<line>` for every function_ref ref to `run`, in source order. */
|
|
function runRefs(result: ExtractionResult): string[] {
|
|
const byId = new Map(result.nodes.map((n) => [n.id, n]));
|
|
return result.unresolvedReferences
|
|
.filter((r) => r.referenceKind === 'function_ref' && r.referenceName === 'run')
|
|
.map((r) => `${byId.get(r.fromNodeId)?.name} L${r.line}`);
|
|
}
|
|
|
|
const ENV_KEYS = ['CODEGRAPH_KERNEL', 'CODEGRAPH_KERNEL_LANGS'] as const;
|
|
|
|
describe('a Go function value keeps its reference past a shadowing local (#2596)', () => {
|
|
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('main.go', source, 'go');
|
|
}
|
|
delete process.env.CODEGRAPH_KERNEL;
|
|
process.env.CODEGRAPH_KERNEL_LANGS = 'all';
|
|
const result = tryKernelExtract('main.go', source, 'go');
|
|
expect(result, 'kernel extraction of main.go').not.toBeNull();
|
|
return result!;
|
|
}
|
|
|
|
const backends = kernelAvailable ? (['kernel', 'wasm'] as const) : (['wasm'] as const);
|
|
|
|
for (const crlf of [false, true]) {
|
|
const source = crlf ? MAIN_GO.replace(/\n/g, '\r\n') : MAIN_GO;
|
|
const label = crlf ? ' (CRLF)' : '';
|
|
|
|
it.each(backends)(`records a use per scope only where the function binds the name: %s${label}`, (backend) => {
|
|
expect(runRefs(extract(backend, source))).toEqual([
|
|
// Binds `run` in a scope of its own: one use per scope.
|
|
'shadowThenUse L12',
|
|
'shadowThenUse L14',
|
|
'closureThenUse L18',
|
|
'closureThenUse L20',
|
|
'caseThenUse L27',
|
|
'caseThenUse L29',
|
|
'headerThenUse L35',
|
|
'headerThenUse L37',
|
|
'rangeThenUse L42',
|
|
'rangeThenUse L44',
|
|
// The function body's two uses are one scope: the first speaks for both.
|
|
'useThenShadow L48',
|
|
'useThenShadow L51',
|
|
// A parameter of the function itself covers its whole body: one use.
|
|
'allShadowed L57',
|
|
// Binds no `run`: one use per function, as before.
|
|
'noBinding L64',
|
|
'paramValue L71',
|
|
'localValue L76',
|
|
// A func literal at file scope is its own unit.
|
|
'literalThenUse L84',
|
|
'literalThenUse L86',
|
|
]);
|
|
});
|
|
}
|
|
|
|
it.runIf(kernelAvailable)('the kernel and the wasm extractor record the same references', () => {
|
|
const kernel = extract('kernel', MAIN_GO);
|
|
const wasm = extract('wasm', MAIN_GO);
|
|
const refs = (r: ExtractionResult) =>
|
|
r.unresolvedReferences.map((u) => `${u.referenceKind} ${u.referenceName} ${u.line}:${u.column}`);
|
|
expect(refs(kernel)).toEqual(refs(wasm));
|
|
});
|
|
});
|
|
|
|
describe('the references a function keeps to the package function (#2596)', () => {
|
|
let tempDir: string;
|
|
let cg: CodeGraph | null = null;
|
|
|
|
beforeAll(async () => {
|
|
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-2596-'));
|
|
fs.writeFileSync(path.join(tempDir, 'go.mod'), GO_MOD);
|
|
fs.writeFileSync(path.join(tempDir, 'main.go'), MAIN_GO);
|
|
cg = await CodeGraph.init(tempDir, { index: true });
|
|
cg.resolveReferences();
|
|
}, 120_000);
|
|
|
|
afterAll(() => {
|
|
cg?.close();
|
|
cg = null;
|
|
if (tempDir) fs.rmSync(tempDir, { recursive: true, force: true });
|
|
});
|
|
|
|
/** `L<line>` of each `references` edge from `fromName` to the package `func run`. */
|
|
function refsToRun(fromName: string, kind: 'function' | 'variable' = 'function'): string[] {
|
|
const from = cg!.getNodesByKind(kind).find((n) => n.name === fromName);
|
|
expect(from, fromName).toBeDefined();
|
|
return cg!
|
|
.getOutgoingEdges(from!.id)
|
|
.filter((e) => e.kind === 'references')
|
|
.filter((e) => {
|
|
const target = cg!.getNode(e.target);
|
|
return target?.kind === 'function' && target.name === 'run';
|
|
})
|
|
.map((e) => `L${e.line}`)
|
|
.sort();
|
|
}
|
|
|
|
it.each([
|
|
['an inner block local', 'shadowThenUse', 'L14'],
|
|
['a func literal parameter', 'closureThenUse', 'L20'],
|
|
['a local in another case clause', 'caseThenUse', 'L29'],
|
|
['an if-header local', 'headerThenUse', 'L37'],
|
|
['a range variable', 'rangeThenUse', 'L44'],
|
|
])('a later use outside %s links to the package function', (_label, fn, line) => {
|
|
expect(refsToRun(fn)).toEqual([line]);
|
|
});
|
|
|
|
it('a later use in a func literal at file scope links to the package function', () => {
|
|
expect(refsToRun('literalThenUse', 'variable')).toEqual(['L86']);
|
|
});
|
|
|
|
it('a function whose first use is the package function keeps that one reference', () => {
|
|
expect(refsToRun('useThenShadow')).toEqual(['L48']);
|
|
});
|
|
|
|
it('a function that binds no local of the name keeps its single reference', () => {
|
|
expect(refsToRun('noBinding')).toEqual(['L64']);
|
|
});
|
|
|
|
it('no use the local shadows links to the package function', () => {
|
|
expect(refsToRun('allShadowed')).toEqual([]);
|
|
expect(refsToRun('paramValue')).toEqual([]);
|
|
expect(refsToRun('localValue')).toEqual([]);
|
|
});
|
|
});
|