From 438f2dab7e2c268ca3460c1902ab49dc328ce5d0 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Wed, 8 Jul 2026 07:26:25 -0600 Subject: [PATCH] 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 --- .../src/extractors/javascript.rs | 121 ++++++++++++++++-- src/extractors/javascript.ts | 66 +++++++--- ...ssue-1901-array-pattern-definition.test.ts | 103 +++++++++++++++ tests/parsers/javascript.test.ts | 29 ++++- 4 files changed, 285 insertions(+), 34 deletions(-) create mode 100644 tests/integration/issue-1901-array-pattern-definition.test.ts diff --git a/crates/codegraph-core/src/extractors/javascript.rs b/crates/codegraph-core/src/extractors/javascript.rs index 58839901..576cbbfd 100644 --- a/crates/codegraph-core/src/extractors/javascript.rs +++ b/crates/codegraph-core/src/extractors/javascript.rs @@ -1534,14 +1534,11 @@ fn handle_var_decl(node: &Node, source: &[u8], symbols: &mut FileSymbols) { } } } - } else if is_const - && (name_n.kind() == "identifier" || name_n.kind() == "array_pattern") - && !in_function_scope - { + } else if is_const && name_n.kind() == "identifier" && !in_function_scope { // Any other initializer shape becomes a "constant" Definition, regardless of // complexity (call/member/parenthesized expressions, etc.) — mirroring how // function declarations are captured regardless of body complexity, and the - // WASM/TS extractor's unconditional identifier + array_pattern branches (#1819). + // WASM/TS extractor's unconditional identifier branch (#1819). symbols.definitions.push(Definition { name: node_text(&name_n, source).to_string(), kind: "constant".to_string(), @@ -1555,10 +1552,14 @@ fn handle_var_decl(node: &Node, source: &[u8], symbols: &mut FileSymbols) { // Phase 8.3f: extract function/arrow properties from object literals and seed // typeMap composite keys so that this.method() inside Object.defineProperty // accessor functions can resolve them. - if value_n.kind() == "object" && name_n.kind() == "identifier" { + if value_n.kind() == "object" { let var_name = node_text(&name_n, source); extract_object_literal_functions(&value_n, source, var_name, symbols); } + } else if is_const && name_n.kind() == "array_pattern" && !in_function_scope { + // Array destructuring: `const [x, y] = ...` — one constant Definition per + // bound identifier (#1901). Scope guard mirrors the object_pattern branch above. + extract_array_pattern_bindings(&name_n, source, start_line(node), end_line(node), &mut symbols.definitions); } else if !is_const && value_n.kind() == "object" && name_n.kind() == "identifier" && !in_function_scope { // `let`/`var` object literals get no "constant" definition of their own (mirrors // WASM extractLetVarObjLiteralDeclarators) but still need their function/method @@ -3027,6 +3028,80 @@ fn extract_destructured_bindings( } } +/// Extract a per-element "constant" Definition from each bound identifier in +/// an array-destructuring pattern (`const [a, b] = fn()`) — the array-pattern +/// counterpart to `extract_destructured_bindings`'s per-property handling of +/// object patterns (#1773). Each bound name becomes its own resolvable node, +/// superseding the prior single-node-named-by-raw-pattern-text approach +/// (`[a, b]` as one unresolvable node), which was never a real identifier and +/// could never be a call target (#1901). Mirrors the TS extractor's +/// `extractArrayPatternBindings`. +fn extract_array_pattern_bindings( + pattern: &Node, + source: &[u8], + line: u32, + end_line: u32, + definitions: &mut Vec, +) { + for i in 0..pattern.child_count() { + let Some(child) = pattern.child(i) else { continue }; + match child.kind() { + "identifier" => { + definitions.push(Definition { + name: node_text(&child, source).to_string(), + kind: "constant".to_string(), + line, + end_line: Some(end_line), + decorators: None, + complexity: None, + cfg: None, + children: None, + }); + } + "assignment_pattern" => { + if let Some(left) = child.child_by_field_name("left") { + if left.kind() == "identifier" { + definitions.push(Definition { + name: node_text(&left, source).to_string(), + kind: "constant".to_string(), + line, + end_line: Some(end_line), + decorators: None, + complexity: None, + cfg: None, + children: None, + }); + } + } + } + "rest_pattern" | "rest_element" => { + // The identifier is at child index 1 (index 0 is the `...` + // token itself) — mirroring extract_js_parameters' own + // rest_pattern handling, which scans all children for the + // identifier rather than assuming a fixed index. + for j in 0..child.child_count() { + if let Some(inner) = child.child(j) { + if inner.kind() == "identifier" { + definitions.push(Definition { + name: node_text(&inner, source).to_string(), + kind: "constant".to_string(), + line, + end_line: Some(end_line), + decorators: None, + complexity: None, + cfg: None, + children: None, + }); + break; + } + } + } + } + _ => {} + } + } +} + /// Mirrors `extractReceiverName` in src/extractors/javascript.ts: normalize a /// call receiver node to a resolvable name. Inline-new (`new Foo().method()`) /// and single-paren-wrapped new (`(new Foo()).method()`) yield the constructor @@ -4653,14 +4728,34 @@ mod tests { #[test] fn extracts_const_array_pattern_with_call_expression_initializer() { // Parity with the identifier case above: array-pattern names must also - // be discoverable regardless of initializer complexity. + // be discoverable regardless of initializer complexity — one + // definition per bound identifier (#1901), not a single node named + // by the raw pattern text. let s = parse_js("const [a, b] = computePair();"); - let def = s - .definitions - .iter() - .find(|d| d.name == "[a, b]") - .unwrap_or_else(|| panic!("[a, b] should be extracted as a definition")); - assert_eq!(def.kind, "constant"); + for name in ["a", "b"] { + let def = s + .definitions + .iter() + .find(|d| d.name == name) + .unwrap_or_else(|| panic!("{name} should be extracted as a definition")); + assert_eq!(def.kind, "constant"); + } + assert!(!s.definitions.iter().any(|d| d.name == "[a, b]")); + } + + #[test] + fn extracts_array_pattern_default_and_rest_bindings_as_own_definitions() { + // #1901: array-pattern default-value and rest bindings each become + // their own "constant" Definition, matching the plain-identifier case. + let s = parse_js("const [a = 1, ...rest] = computeList();"); + for name in ["a", "rest"] { + let def = s + .definitions + .iter() + .find(|d| d.name == name) + .unwrap_or_else(|| panic!("{name} should be extracted as a definition")); + assert_eq!(def.kind, "constant"); + } } #[test] diff --git a/src/extractors/javascript.ts b/src/extractors/javascript.ts index 8f7b4ad8..1bed7656 100644 --- a/src/extractors/javascript.ts +++ b/src/extractors/javascript.ts @@ -827,14 +827,13 @@ function extractDestructuredDeclarators( if (binding) cjsRequireBindings.push(binding); } } else if (nameN && nameN.type === 'array_pattern') { - // `const [x, y] = ...` — emit a single constant node whose name is the - // full array pattern text (e.g. `[x, y]`), matching native engine behaviour. - definitions.push({ - name: nameN.text, - kind: 'constant', - line: nodeStartLine(declNode), - endLine: nodeEndLine(declNode), - }); + // `const [x, y] = ...` — one constant Definition per bound identifier (#1901). + extractArrayPatternBindings( + nameN, + nodeStartLine(declNode), + nodeEndLine(declNode), + definitions, + ); } } } @@ -1378,6 +1377,45 @@ function extractDestructuredBindings( } } +/** + * Extract a per-element `constant` Definition from each bound identifier in an + * array-destructuring pattern (`const [a, b] = fn()`) — the array-pattern + * counterpart to `extractDestructuredBindings`'s per-property handling of + * object patterns (#1773). Each bound name becomes its own resolvable node + * (e.g. `a()`, `b()` calls can resolve to `a`/`b` directly), superseding the + * prior single-node-named-by-raw-pattern-text approach (`[a, b]` as one + * unresolvable node), which was never a real identifier and could never be a + * call target (#1901). + */ +function extractArrayPatternBindings( + pattern: TreeSitterNode, + line: number, + endLine: number, + definitions: Definition[], +): void { + for (let i = 0; i < pattern.childCount; i++) { + const child = pattern.child(i); + if (!child) continue; + if (child.type === 'identifier') { + // [a, b] — plain positional binding + definitions.push({ name: child.text, kind: 'constant', line, endLine }); + } else if (child.type === 'assignment_pattern') { + // [a = defaultValue] — the bound name is the left-hand identifier + const left = child.childForFieldName('left'); + if (left && left.type === 'identifier') { + definitions.push({ name: left.text, kind: 'constant', line, endLine }); + } + } else if (child.type === 'rest_pattern' || child.type === 'rest_element') { + // [...rest] — the identifier is at child index 1 (index 0 is the `...` + // token itself), matching extractParameters' own rest_pattern handling. + const inner = child.child(1) || child.childForFieldName('name'); + if (inner && inner.type === 'identifier') { + definitions.push({ name: inner.text, kind: 'constant', line, endLine }); + } + } + } +} + function handleVariableDecl(node: TreeSitterNode, ctx: ExtractorOutput): void { const isConst = node.text.startsWith('const '); for (let i = 0; i < node.childCount; i++) { @@ -1431,15 +1469,9 @@ function handleVariableDeclarator( } else if (isConst && nameN.type === 'object_pattern' && !hasFunctionScopeAncestor(node)) { handleConstObjectPatternAssignment(node, nameN, valueN, ctx); } else if (isConst && nameN.type === 'array_pattern' && !hasFunctionScopeAncestor(node)) { - // Array destructuring: `const [x, y] = ...` — emit a single constant node - // whose name is the full array pattern text (e.g. `[x, y]`), matching - // native engine behaviour. Scope guard mirrors the object_pattern branch above. - ctx.definitions.push({ - name: nameN.text, - kind: 'constant', - line: nodeStartLine(node), - endLine: nodeEndLine(node), - }); + // Array destructuring: `const [x, y] = ...` — one constant Definition per + // bound identifier (#1901). Scope guard mirrors the object_pattern branch above. + extractArrayPatternBindings(nameN, nodeStartLine(node), nodeEndLine(node), ctx.definitions); } } diff --git a/tests/integration/issue-1901-array-pattern-definition.test.ts b/tests/integration/issue-1901-array-pattern-definition.test.ts new file mode 100644 index 00000000..f1e23cca --- /dev/null +++ b/tests/integration/issue-1901-array-pattern-definition.test.ts @@ -0,0 +1,103 @@ +/** + * Integration test for #1901: engine parity for `const [a, b] = fn();` + * top-level array-pattern destructuring. + * + * Root cause: the native Rust extractor (`crates/codegraph-core/src/ + * extractors/javascript.rs`, `handle_var_decl`) emitted no `Definition` at + * all for array-pattern name nodes, while the WASM/TS extractor + * (`src/extractors/javascript.ts`, both the walk path in + * `handleVariableDeclarator` and the query path in + * `extractDestructuredDeclarators`) emitted a single `Definition` whose + * `name` was the raw pattern source text (e.g. `"[a, b]"`) — not a real + * identifier, and never itself a valid call target. + * + * Fix: both engines now emit one `constant`-kind `Definition` per bound + * identifier (`a`, `b`, ...), mirroring how object-pattern destructuring + * already works per-property (#1773) — via `extractArrayPatternBindings` + * (WASM/TS) and `extract_array_pattern_bindings` (native). + */ + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import Database from 'better-sqlite3'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { buildGraph } from '../../src/domain/graph/builder.js'; +import { isNativeAvailable } from '../../src/infrastructure/native.js'; + +const FIXTURE = { + 'sample.js': ` +function getArr() { return [1, 2]; } +const [a, b] = getArr(); +console.log(a, b); + +function withDefaultsAndRest() { return [1, 2, 3]; } +const [c = 0, ...rest] = withDefaultsAndRest(); +console.log(c, rest); +`, +}; + +function readNode(dbPath: string, file: string, name: string) { + const db = new Database(dbPath, { readonly: true }); + try { + return db.prepare('SELECT name, kind FROM nodes WHERE file = ? AND name = ?').get(file, name) as + | { name: string; kind: string } + | undefined; + } finally { + db.close(); + } +} + +function expectPerElementBindings(dbPath: string) { + for (const name of ['a', 'b', 'c', 'rest']) { + const node = readNode(dbPath, 'sample.js', name); + expect(node, `${name} node not found`).toBeDefined(); + expect(node!.kind, `${name} must be kind constant`).toBe('constant'); + } + // The old single-node-named-by-raw-pattern-text approach must be gone. + expect(readNode(dbPath, 'sample.js', '[a, b]')).toBeUndefined(); + expect(readNode(dbPath, 'sample.js', '[c = 0, ...rest]')).toBeUndefined(); +} + +describe('array-pattern destructuring Definition extraction (#1901) — WASM', () => { + let tmpDir: string; + + beforeAll(async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1901-wasm-')); + for (const [rel, content] of Object.entries(FIXTURE)) { + fs.writeFileSync(path.join(tmpDir, rel), content); + } + await buildGraph(tmpDir, { engine: 'wasm', incremental: false, skipRegistry: true }); + }); + + afterAll(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('extracts one constant definition per bound identifier', () => { + expectPerElementBindings(path.join(tmpDir, '.codegraph', 'graph.db')); + }); +}); + +describe.skipIf(!isNativeAvailable())( + 'array-pattern destructuring Definition extraction (#1901) — native', + () => { + let tmpDir: string; + + beforeAll(async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1901-native-')); + for (const [rel, content] of Object.entries(FIXTURE)) { + fs.writeFileSync(path.join(tmpDir, rel), content); + } + await buildGraph(tmpDir, { engine: 'native', incremental: false, skipRegistry: true }); + }, 60_000); + + afterAll(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('extracts one constant definition per bound identifier (previously emitted none)', () => { + expectPerElementBindings(path.join(tmpDir, '.codegraph', 'graph.db')); + }); + }, +); diff --git a/tests/parsers/javascript.test.ts b/tests/parsers/javascript.test.ts index a17a92f4..93a4b601 100644 --- a/tests/parsers/javascript.test.ts +++ b/tests/parsers/javascript.test.ts @@ -2341,16 +2341,34 @@ function runDemo(users: string[]): void { }); }); - describe('array destructuring constant extraction (#1471)', () => { - it('extracts const array pattern as a single constant node', () => { + describe('array destructuring constant extraction (#1471, #1901)', () => { + it('extracts one constant definition per bound identifier in a const array pattern', () => { + // Per-element extraction (#1901) supersedes the prior single-node + // ("[x, y]" as one unresolvable name) approach — `[x, y]` was never a + // real identifier and could never be a call target. const symbols = parseJS(`const [x, y] = new Set([() => {}, () => {}]);`); expect(symbols.definitions).toContainEqual( - expect.objectContaining({ name: '[x, y]', kind: 'constant' }), + expect.objectContaining({ name: 'x', kind: 'constant' }), + ); + expect(symbols.definitions).toContainEqual( + expect.objectContaining({ name: 'y', kind: 'constant' }), + ); + expect(symbols.definitions.every((d) => d.name !== '[x, y]')).toBe(true); + }); + + it('extracts the default-value binding and the rest binding as their own constants', () => { + const symbols = parseJS(`const [a = 1, ...rest] = computeList();`); + expect(symbols.definitions).toContainEqual( + expect.objectContaining({ name: 'a', kind: 'constant' }), + ); + expect(symbols.definitions).toContainEqual( + expect.objectContaining({ name: 'rest', kind: 'constant' }), ); }); it('does not extract let or var array destructuring', () => { const symbols = parseJS(`let [a, b] = [1, 2];`); + expect(symbols.definitions.every((d) => d.name !== 'a' && d.name !== 'b')).toBe(true); expect(symbols.definitions.every((d) => d.name !== '[a, b]')).toBe(true); }); }); @@ -2496,7 +2514,10 @@ function runDemo(users: string[]): void { it('extracts a const array pattern with a call-expression initializer (parity with identifier case)', () => { const symbols = parseJS(`const [a, b] = computePair();`); expect(symbols.definitions).toContainEqual( - expect.objectContaining({ name: '[a, b]', kind: 'constant' }), + expect.objectContaining({ name: 'a', kind: 'constant' }), + ); + expect(symbols.definitions).toContainEqual( + expect.objectContaining({ name: 'b', kind: 'constant' }), ); }); });