Skip to content

Commit 438f2da

Browse files
committed
fix(extractors): emit per-element definitions for array-pattern destructuring
Both engines previously emitted a single "constant" Definition for a top-level const array pattern (const [a, b] = fn()) whose name was the raw pattern source text (e.g. "[a, b]") -- never a real identifier and never a valid call target. The native Rust extractor additionally emitted no Definition at all for array_pattern name nodes, a genuine engine-parity gap. Both engines now emit one "constant" Definition per bound identifier, mirroring how object-pattern destructuring already works per-property (#1773): extractArrayPatternBindings (WASM/TS) and extract_array_pattern_bindings (native), covering plain identifiers, default-value bindings, and rest bindings. Fixes #1901 Impact: 3 functions changed, 8 affected
1 parent 1d957a4 commit 438f2da

4 files changed

Lines changed: 285 additions & 34 deletions

File tree

crates/codegraph-core/src/extractors/javascript.rs

Lines changed: 108 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1534,14 +1534,11 @@ fn handle_var_decl(node: &Node, source: &[u8], symbols: &mut FileSymbols) {
15341534
}
15351535
}
15361536
}
1537-
} else if is_const
1538-
&& (name_n.kind() == "identifier" || name_n.kind() == "array_pattern")
1539-
&& !in_function_scope
1540-
{
1537+
} else if is_const && name_n.kind() == "identifier" && !in_function_scope {
15411538
// Any other initializer shape becomes a "constant" Definition, regardless of
15421539
// complexity (call/member/parenthesized expressions, etc.) — mirroring how
15431540
// function declarations are captured regardless of body complexity, and the
1544-
// WASM/TS extractor's unconditional identifier + array_pattern branches (#1819).
1541+
// WASM/TS extractor's unconditional identifier branch (#1819).
15451542
symbols.definitions.push(Definition {
15461543
name: node_text(&name_n, source).to_string(),
15471544
kind: "constant".to_string(),
@@ -1555,10 +1552,14 @@ fn handle_var_decl(node: &Node, source: &[u8], symbols: &mut FileSymbols) {
15551552
// Phase 8.3f: extract function/arrow properties from object literals and seed
15561553
// typeMap composite keys so that this.method() inside Object.defineProperty
15571554
// accessor functions can resolve them.
1558-
if value_n.kind() == "object" && name_n.kind() == "identifier" {
1555+
if value_n.kind() == "object" {
15591556
let var_name = node_text(&name_n, source);
15601557
extract_object_literal_functions(&value_n, source, var_name, symbols);
15611558
}
1559+
} else if is_const && name_n.kind() == "array_pattern" && !in_function_scope {
1560+
// Array destructuring: `const [x, y] = ...` — one constant Definition per
1561+
// bound identifier (#1901). Scope guard mirrors the object_pattern branch above.
1562+
extract_array_pattern_bindings(&name_n, source, start_line(node), end_line(node), &mut symbols.definitions);
15621563
} else if !is_const && value_n.kind() == "object" && name_n.kind() == "identifier" && !in_function_scope {
15631564
// `let`/`var` object literals get no "constant" definition of their own (mirrors
15641565
// WASM extractLetVarObjLiteralDeclarators) but still need their function/method
@@ -3027,6 +3028,80 @@ fn extract_destructured_bindings(
30273028
}
30283029
}
30293030

3031+
/// Extract a per-element "constant" Definition from each bound identifier in
3032+
/// an array-destructuring pattern (`const [a, b] = fn()`) — the array-pattern
3033+
/// counterpart to `extract_destructured_bindings`'s per-property handling of
3034+
/// object patterns (#1773). Each bound name becomes its own resolvable node,
3035+
/// superseding the prior single-node-named-by-raw-pattern-text approach
3036+
/// (`[a, b]` as one unresolvable node), which was never a real identifier and
3037+
/// could never be a call target (#1901). Mirrors the TS extractor's
3038+
/// `extractArrayPatternBindings`.
3039+
fn extract_array_pattern_bindings(
3040+
pattern: &Node,
3041+
source: &[u8],
3042+
line: u32,
3043+
end_line: u32,
3044+
definitions: &mut Vec<Definition>,
3045+
) {
3046+
for i in 0..pattern.child_count() {
3047+
let Some(child) = pattern.child(i) else { continue };
3048+
match child.kind() {
3049+
"identifier" => {
3050+
definitions.push(Definition {
3051+
name: node_text(&child, source).to_string(),
3052+
kind: "constant".to_string(),
3053+
line,
3054+
end_line: Some(end_line),
3055+
decorators: None,
3056+
complexity: None,
3057+
cfg: None,
3058+
children: None,
3059+
});
3060+
}
3061+
"assignment_pattern" => {
3062+
if let Some(left) = child.child_by_field_name("left") {
3063+
if left.kind() == "identifier" {
3064+
definitions.push(Definition {
3065+
name: node_text(&left, source).to_string(),
3066+
kind: "constant".to_string(),
3067+
line,
3068+
end_line: Some(end_line),
3069+
decorators: None,
3070+
complexity: None,
3071+
cfg: None,
3072+
children: None,
3073+
});
3074+
}
3075+
}
3076+
}
3077+
"rest_pattern" | "rest_element" => {
3078+
// The identifier is at child index 1 (index 0 is the `...`
3079+
// token itself) — mirroring extract_js_parameters' own
3080+
// rest_pattern handling, which scans all children for the
3081+
// identifier rather than assuming a fixed index.
3082+
for j in 0..child.child_count() {
3083+
if let Some(inner) = child.child(j) {
3084+
if inner.kind() == "identifier" {
3085+
definitions.push(Definition {
3086+
name: node_text(&inner, source).to_string(),
3087+
kind: "constant".to_string(),
3088+
line,
3089+
end_line: Some(end_line),
3090+
decorators: None,
3091+
complexity: None,
3092+
cfg: None,
3093+
children: None,
3094+
});
3095+
break;
3096+
}
3097+
}
3098+
}
3099+
}
3100+
_ => {}
3101+
}
3102+
}
3103+
}
3104+
30303105
/// Mirrors `extractReceiverName` in src/extractors/javascript.ts: normalize a
30313106
/// call receiver node to a resolvable name. Inline-new (`new Foo().method()`)
30323107
/// and single-paren-wrapped new (`(new Foo()).method()`) yield the constructor
@@ -4653,14 +4728,34 @@ mod tests {
46534728
#[test]
46544729
fn extracts_const_array_pattern_with_call_expression_initializer() {
46554730
// Parity with the identifier case above: array-pattern names must also
4656-
// be discoverable regardless of initializer complexity.
4731+
// be discoverable regardless of initializer complexity — one
4732+
// definition per bound identifier (#1901), not a single node named
4733+
// by the raw pattern text.
46574734
let s = parse_js("const [a, b] = computePair();");
4658-
let def = s
4659-
.definitions
4660-
.iter()
4661-
.find(|d| d.name == "[a, b]")
4662-
.unwrap_or_else(|| panic!("[a, b] should be extracted as a definition"));
4663-
assert_eq!(def.kind, "constant");
4735+
for name in ["a", "b"] {
4736+
let def = s
4737+
.definitions
4738+
.iter()
4739+
.find(|d| d.name == name)
4740+
.unwrap_or_else(|| panic!("{name} should be extracted as a definition"));
4741+
assert_eq!(def.kind, "constant");
4742+
}
4743+
assert!(!s.definitions.iter().any(|d| d.name == "[a, b]"));
4744+
}
4745+
4746+
#[test]
4747+
fn extracts_array_pattern_default_and_rest_bindings_as_own_definitions() {
4748+
// #1901: array-pattern default-value and rest bindings each become
4749+
// their own "constant" Definition, matching the plain-identifier case.
4750+
let s = parse_js("const [a = 1, ...rest] = computeList();");
4751+
for name in ["a", "rest"] {
4752+
let def = s
4753+
.definitions
4754+
.iter()
4755+
.find(|d| d.name == name)
4756+
.unwrap_or_else(|| panic!("{name} should be extracted as a definition"));
4757+
assert_eq!(def.kind, "constant");
4758+
}
46644759
}
46654760

46664761
#[test]

src/extractors/javascript.ts

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -827,14 +827,13 @@ function extractDestructuredDeclarators(
827827
if (binding) cjsRequireBindings.push(binding);
828828
}
829829
} else if (nameN && nameN.type === 'array_pattern') {
830-
// `const [x, y] = ...` — emit a single constant node whose name is the
831-
// full array pattern text (e.g. `[x, y]`), matching native engine behaviour.
832-
definitions.push({
833-
name: nameN.text,
834-
kind: 'constant',
835-
line: nodeStartLine(declNode),
836-
endLine: nodeEndLine(declNode),
837-
});
830+
// `const [x, y] = ...` — one constant Definition per bound identifier (#1901).
831+
extractArrayPatternBindings(
832+
nameN,
833+
nodeStartLine(declNode),
834+
nodeEndLine(declNode),
835+
definitions,
836+
);
838837
}
839838
}
840839
}
@@ -1378,6 +1377,45 @@ function extractDestructuredBindings(
13781377
}
13791378
}
13801379

1380+
/**
1381+
* Extract a per-element `constant` Definition from each bound identifier in an
1382+
* array-destructuring pattern (`const [a, b] = fn()`) — the array-pattern
1383+
* counterpart to `extractDestructuredBindings`'s per-property handling of
1384+
* object patterns (#1773). Each bound name becomes its own resolvable node
1385+
* (e.g. `a()`, `b()` calls can resolve to `a`/`b` directly), superseding the
1386+
* prior single-node-named-by-raw-pattern-text approach (`[a, b]` as one
1387+
* unresolvable node), which was never a real identifier and could never be a
1388+
* call target (#1901).
1389+
*/
1390+
function extractArrayPatternBindings(
1391+
pattern: TreeSitterNode,
1392+
line: number,
1393+
endLine: number,
1394+
definitions: Definition[],
1395+
): void {
1396+
for (let i = 0; i < pattern.childCount; i++) {
1397+
const child = pattern.child(i);
1398+
if (!child) continue;
1399+
if (child.type === 'identifier') {
1400+
// [a, b] — plain positional binding
1401+
definitions.push({ name: child.text, kind: 'constant', line, endLine });
1402+
} else if (child.type === 'assignment_pattern') {
1403+
// [a = defaultValue] — the bound name is the left-hand identifier
1404+
const left = child.childForFieldName('left');
1405+
if (left && left.type === 'identifier') {
1406+
definitions.push({ name: left.text, kind: 'constant', line, endLine });
1407+
}
1408+
} else if (child.type === 'rest_pattern' || child.type === 'rest_element') {
1409+
// [...rest] — the identifier is at child index 1 (index 0 is the `...`
1410+
// token itself), matching extractParameters' own rest_pattern handling.
1411+
const inner = child.child(1) || child.childForFieldName('name');
1412+
if (inner && inner.type === 'identifier') {
1413+
definitions.push({ name: inner.text, kind: 'constant', line, endLine });
1414+
}
1415+
}
1416+
}
1417+
}
1418+
13811419
function handleVariableDecl(node: TreeSitterNode, ctx: ExtractorOutput): void {
13821420
const isConst = node.text.startsWith('const ');
13831421
for (let i = 0; i < node.childCount; i++) {
@@ -1431,15 +1469,9 @@ function handleVariableDeclarator(
14311469
} else if (isConst && nameN.type === 'object_pattern' && !hasFunctionScopeAncestor(node)) {
14321470
handleConstObjectPatternAssignment(node, nameN, valueN, ctx);
14331471
} else if (isConst && nameN.type === 'array_pattern' && !hasFunctionScopeAncestor(node)) {
1434-
// Array destructuring: `const [x, y] = ...` — emit a single constant node
1435-
// whose name is the full array pattern text (e.g. `[x, y]`), matching
1436-
// native engine behaviour. Scope guard mirrors the object_pattern branch above.
1437-
ctx.definitions.push({
1438-
name: nameN.text,
1439-
kind: 'constant',
1440-
line: nodeStartLine(node),
1441-
endLine: nodeEndLine(node),
1442-
});
1472+
// Array destructuring: `const [x, y] = ...` — one constant Definition per
1473+
// bound identifier (#1901). Scope guard mirrors the object_pattern branch above.
1474+
extractArrayPatternBindings(nameN, nodeStartLine(node), nodeEndLine(node), ctx.definitions);
14431475
}
14441476
}
14451477

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/**
2+
* Integration test for #1901: engine parity for `const [a, b] = fn();`
3+
* top-level array-pattern destructuring.
4+
*
5+
* Root cause: the native Rust extractor (`crates/codegraph-core/src/
6+
* extractors/javascript.rs`, `handle_var_decl`) emitted no `Definition` at
7+
* all for array-pattern name nodes, while the WASM/TS extractor
8+
* (`src/extractors/javascript.ts`, both the walk path in
9+
* `handleVariableDeclarator` and the query path in
10+
* `extractDestructuredDeclarators`) emitted a single `Definition` whose
11+
* `name` was the raw pattern source text (e.g. `"[a, b]"`) — not a real
12+
* identifier, and never itself a valid call target.
13+
*
14+
* Fix: both engines now emit one `constant`-kind `Definition` per bound
15+
* identifier (`a`, `b`, ...), mirroring how object-pattern destructuring
16+
* already works per-property (#1773) — via `extractArrayPatternBindings`
17+
* (WASM/TS) and `extract_array_pattern_bindings` (native).
18+
*/
19+
20+
import fs from 'node:fs';
21+
import os from 'node:os';
22+
import path from 'node:path';
23+
import Database from 'better-sqlite3';
24+
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
25+
import { buildGraph } from '../../src/domain/graph/builder.js';
26+
import { isNativeAvailable } from '../../src/infrastructure/native.js';
27+
28+
const FIXTURE = {
29+
'sample.js': `
30+
function getArr() { return [1, 2]; }
31+
const [a, b] = getArr();
32+
console.log(a, b);
33+
34+
function withDefaultsAndRest() { return [1, 2, 3]; }
35+
const [c = 0, ...rest] = withDefaultsAndRest();
36+
console.log(c, rest);
37+
`,
38+
};
39+
40+
function readNode(dbPath: string, file: string, name: string) {
41+
const db = new Database(dbPath, { readonly: true });
42+
try {
43+
return db.prepare('SELECT name, kind FROM nodes WHERE file = ? AND name = ?').get(file, name) as
44+
| { name: string; kind: string }
45+
| undefined;
46+
} finally {
47+
db.close();
48+
}
49+
}
50+
51+
function expectPerElementBindings(dbPath: string) {
52+
for (const name of ['a', 'b', 'c', 'rest']) {
53+
const node = readNode(dbPath, 'sample.js', name);
54+
expect(node, `${name} node not found`).toBeDefined();
55+
expect(node!.kind, `${name} must be kind constant`).toBe('constant');
56+
}
57+
// The old single-node-named-by-raw-pattern-text approach must be gone.
58+
expect(readNode(dbPath, 'sample.js', '[a, b]')).toBeUndefined();
59+
expect(readNode(dbPath, 'sample.js', '[c = 0, ...rest]')).toBeUndefined();
60+
}
61+
62+
describe('array-pattern destructuring Definition extraction (#1901) — WASM', () => {
63+
let tmpDir: string;
64+
65+
beforeAll(async () => {
66+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1901-wasm-'));
67+
for (const [rel, content] of Object.entries(FIXTURE)) {
68+
fs.writeFileSync(path.join(tmpDir, rel), content);
69+
}
70+
await buildGraph(tmpDir, { engine: 'wasm', incremental: false, skipRegistry: true });
71+
});
72+
73+
afterAll(() => {
74+
fs.rmSync(tmpDir, { recursive: true, force: true });
75+
});
76+
77+
it('extracts one constant definition per bound identifier', () => {
78+
expectPerElementBindings(path.join(tmpDir, '.codegraph', 'graph.db'));
79+
});
80+
});
81+
82+
describe.skipIf(!isNativeAvailable())(
83+
'array-pattern destructuring Definition extraction (#1901) — native',
84+
() => {
85+
let tmpDir: string;
86+
87+
beforeAll(async () => {
88+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1901-native-'));
89+
for (const [rel, content] of Object.entries(FIXTURE)) {
90+
fs.writeFileSync(path.join(tmpDir, rel), content);
91+
}
92+
await buildGraph(tmpDir, { engine: 'native', incremental: false, skipRegistry: true });
93+
}, 60_000);
94+
95+
afterAll(() => {
96+
fs.rmSync(tmpDir, { recursive: true, force: true });
97+
});
98+
99+
it('extracts one constant definition per bound identifier (previously emitted none)', () => {
100+
expectPerElementBindings(path.join(tmpDir, '.codegraph', 'graph.db'));
101+
});
102+
},
103+
);

tests/parsers/javascript.test.ts

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2341,16 +2341,34 @@ function runDemo(users: string[]): void {
23412341
});
23422342
});
23432343

2344-
describe('array destructuring constant extraction (#1471)', () => {
2345-
it('extracts const array pattern as a single constant node', () => {
2344+
describe('array destructuring constant extraction (#1471, #1901)', () => {
2345+
it('extracts one constant definition per bound identifier in a const array pattern', () => {
2346+
// Per-element extraction (#1901) supersedes the prior single-node
2347+
// ("[x, y]" as one unresolvable name) approach — `[x, y]` was never a
2348+
// real identifier and could never be a call target.
23462349
const symbols = parseJS(`const [x, y] = new Set([() => {}, () => {}]);`);
23472350
expect(symbols.definitions).toContainEqual(
2348-
expect.objectContaining({ name: '[x, y]', kind: 'constant' }),
2351+
expect.objectContaining({ name: 'x', kind: 'constant' }),
2352+
);
2353+
expect(symbols.definitions).toContainEqual(
2354+
expect.objectContaining({ name: 'y', kind: 'constant' }),
2355+
);
2356+
expect(symbols.definitions.every((d) => d.name !== '[x, y]')).toBe(true);
2357+
});
2358+
2359+
it('extracts the default-value binding and the rest binding as their own constants', () => {
2360+
const symbols = parseJS(`const [a = 1, ...rest] = computeList();`);
2361+
expect(symbols.definitions).toContainEqual(
2362+
expect.objectContaining({ name: 'a', kind: 'constant' }),
2363+
);
2364+
expect(symbols.definitions).toContainEqual(
2365+
expect.objectContaining({ name: 'rest', kind: 'constant' }),
23492366
);
23502367
});
23512368

23522369
it('does not extract let or var array destructuring', () => {
23532370
const symbols = parseJS(`let [a, b] = [1, 2];`);
2371+
expect(symbols.definitions.every((d) => d.name !== 'a' && d.name !== 'b')).toBe(true);
23542372
expect(symbols.definitions.every((d) => d.name !== '[a, b]')).toBe(true);
23552373
});
23562374
});
@@ -2496,7 +2514,10 @@ function runDemo(users: string[]): void {
24962514
it('extracts a const array pattern with a call-expression initializer (parity with identifier case)', () => {
24972515
const symbols = parseJS(`const [a, b] = computePair();`);
24982516
expect(symbols.definitions).toContainEqual(
2499-
expect.objectContaining({ name: '[a, b]', kind: 'constant' }),
2517+
expect.objectContaining({ name: 'a', kind: 'constant' }),
2518+
);
2519+
expect(symbols.definitions).toContainEqual(
2520+
expect.objectContaining({ name: 'b', kind: 'constant' }),
25002521
);
25012522
});
25022523
});

0 commit comments

Comments
 (0)