diff --git a/crates/codegraph-core/src/extractors/javascript.rs b/crates/codegraph-core/src/extractors/javascript.rs index 576cbbfd..4469dd01 100644 --- a/crates/codegraph-core/src/extractors/javascript.rs +++ b/crates/codegraph-core/src/extractors/javascript.rs @@ -3719,11 +3719,18 @@ fn collect_array_pattern_names(pattern: &Node, source: &[u8]) -> Vec { names } -/// Extract the identifier from a rest/spread element (`...rest` → `rest`) +/// Extract the identifier from a rest/spread element (`...rest` → `rest`). +/// Scans all children for the `identifier` node rather than assuming a fixed +/// index — the `...` token itself is child 0, so indexing into a fixed slot +/// silently returns the wrong node and drops the binding entirely (#1920). +/// Mirrors `extract_array_pattern_bindings`'s own rest_pattern handling. fn extract_rest_identifier(rest_node: &Node, source: &[u8], names: &mut Vec) { - if let Some(inner) = rest_node.child(0) { - if inner.kind() == "identifier" { - names.push(node_text(&inner, source).to_string()); + for i in 0..rest_node.child_count() { + if let Some(inner) = rest_node.child(i) { + if inner.kind() == "identifier" { + names.push(node_text(&inner, source).to_string()); + break; + } } } } @@ -4993,6 +5000,53 @@ mod tests { assert!(dyn_imports[0].names.contains(&"buildDataflowEdges".to_string())); } + // Regression tests for #1920: `extract_rest_identifier` indexed into a + // fixed child slot (0) that is actually the `...` token, not the bound + // identifier, so rest elements in both object- and array-pattern + // destructures of a dynamic `import()` were silently dropped. + + #[test] + fn finds_dynamic_import_with_object_rest_destructuring() { + let s = parse_js("const { a, ...rest } = await import('./mod.js');"); + let dyn_imports: Vec<_> = s.imports.iter().filter(|i| i.dynamic_import == Some(true)).collect(); + assert_eq!(dyn_imports.len(), 1); + assert_eq!(dyn_imports[0].names, vec!["a".to_string(), "rest".to_string()]); + } + + #[test] + fn finds_dynamic_import_with_shorthand_default_destructuring() { + let s = parse_js("const { a = 1 } = await import('./mod.js');"); + let dyn_imports: Vec<_> = s.imports.iter().filter(|i| i.dynamic_import == Some(true)).collect(); + assert_eq!(dyn_imports.len(), 1); + assert_eq!(dyn_imports[0].names, vec!["a".to_string()]); + } + + #[test] + fn finds_dynamic_import_with_mixed_plain_renamed_default_and_rest_destructuring() { + let s = parse_js("const { a, b: alias, c = 1, ...rest } = await import('./mod.js');"); + let dyn_imports: Vec<_> = s.imports.iter().filter(|i| i.dynamic_import == Some(true)).collect(); + assert_eq!(dyn_imports.len(), 1); + assert_eq!( + dyn_imports[0].names, + vec!["a".to_string(), "alias".to_string(), "c".to_string(), "rest".to_string()] + ); + let renamed = dyn_imports[0] + .renamed_imports + .as_ref() + .expect("renamed_imports should be populated for the renamed specifier"); + assert_eq!(renamed.len(), 1); + assert_eq!(renamed[0].local, "alias"); + assert_eq!(renamed[0].imported, "b"); + } + + #[test] + fn finds_dynamic_import_with_array_rest_destructuring() { + let s = parse_js("const [a, ...rest] = await import('./mod.js');"); + let dyn_imports: Vec<_> = s.imports.iter().filter(|i| i.dynamic_import == Some(true)).collect(); + assert_eq!(dyn_imports.len(), 1); + assert_eq!(dyn_imports[0].names, vec!["a".to_string(), "rest".to_string()]); + } + #[test] fn extracts_callback_reference_in_router_use() { let s = parse_js("router.use(handleToken);"); diff --git a/src/extractors/javascript.ts b/src/extractors/javascript.ts index 1bed7656..1b9be8cc 100644 --- a/src/extractors/javascript.ts +++ b/src/extractors/javascript.ts @@ -4686,6 +4686,21 @@ const DYNAMIC_IMPORT_WRAPPER_TYPES = new Set([ 'as_expression', ]); +/** + * Extract the bound identifier from a `rest_pattern`/`rest_element` node + * (`...rest` → `rest`). Scans all children for the `identifier` node rather + * than assuming a fixed index — the `...` token itself is child 0, so + * indexing into a fixed slot silently returns the wrong node (#1920). + * Mirrors `extract_rest_identifier` in the native engine. + */ +function extractRestPatternIdentifier(restNode: TreeSitterNode): string | undefined { + for (let i = 0; i < restNode.childCount; i++) { + const child = restNode.child(i); + if (child?.type === 'identifier') return child.text; + } + return undefined; +} + /** * Extract destructured names from a dynamic import() call expression. * @@ -4694,6 +4709,8 @@ const DYNAMIC_IMPORT_WRAPPER_TYPES = new Set([ * const mod = await import('./foo.js') → ['mod'] * const { a } = (await import('./foo.js')) as { a: Fn } → ['a'] * const { a: b } = await import('./foo.js') → ['b'] + * const { a, ...rest } = await import('./foo.js') → ['a', 'rest'] + * const { a = 1 } = await import('./foo.js') → ['a'] * import('./foo.js') → [] (no names extractable) * * Walks up the AST from the call_expression — through any nesting of @@ -4760,6 +4777,18 @@ function extractDynamicImportNames( // the key so the specifier isn't dropped entirely. names.push(key.text); } + } else if (child.type === 'object_assignment_pattern') { + // { a = defaultValue } — plain shorthand binding with a default + // value; the bound name is the `left`-hand identifier (#1920). + const left = child.childForFieldName('left'); + if (left?.type === 'shorthand_property_identifier_pattern' || left?.type === 'identifier') { + names.push(left.text); + } + } else if (child.type === 'rest_pattern' || child.type === 'rest_element') { + // { a, ...rest } — the rest binding was silently dropped entirely + // before (#1920). + const inner = extractRestPatternIdentifier(child); + if (inner) names.push(inner); } } return names; @@ -4776,9 +4805,11 @@ function extractDynamicImportNames( for (let i = 0; i < nameNode.childCount; i++) { const child = nameNode.child(i)!; if (child.type === 'identifier') names.push(child.text); - else if (child.type === 'rest_pattern') { - const inner = child.child(0) || child.childForFieldName('name'); - if (inner && inner.type === 'identifier') names.push(inner.text); + else if (child.type === 'rest_pattern' || child.type === 'rest_element') { + // [a, ...rest] — child(0) is the `...` token, not the identifier + // (#1920); extractRestPatternIdentifier scans for the real one. + const inner = extractRestPatternIdentifier(child); + if (inner) names.push(inner); } } return names; diff --git a/tests/parsers/javascript.test.ts b/tests/parsers/javascript.test.ts index 93a4b601..cf4e9379 100644 --- a/tests/parsers/javascript.test.ts +++ b/tests/parsers/javascript.test.ts @@ -349,6 +349,38 @@ describe('JavaScript parser', () => { }); }); + describe('dynamic import() destructuring rest/default bindings (#1920)', () => { + // `extractDynamicImportNames`'s object_pattern branch only recognized + // shorthand_property_identifier_pattern and pair_pattern children, so a + // rest element (`...rest`) was silently dropped entirely and a shorthand + // default (`{ a = 1 }`) produced no name at all (#1920). + + it('extracts a rest binding alongside plain destructured names', () => { + const symbols = parseJS(`const { a, ...rest } = await import('./mod.js');`); + expect(symbols.imports).toHaveLength(1); + expect(symbols.imports[0].names).toEqual(['a', 'rest']); + }); + + it('extracts a shorthand default-value binding', () => { + const symbols = parseJS(`const { a = 1 } = await import('./mod.js');`); + expect(symbols.imports).toHaveLength(1); + expect(symbols.imports[0].names).toEqual(['a']); + }); + + it('extracts a mix of plain, renamed, default, and rest bindings', () => { + const symbols = parseJS(`const { a, b: alias, c = 1, ...rest } = await import('./mod.js');`); + expect(symbols.imports).toHaveLength(1); + expect(symbols.imports[0].names).toEqual(['a', 'alias', 'c', 'rest']); + expect(symbols.imports[0].renamedImports).toEqual([{ local: 'alias', imported: 'b' }]); + }); + + it('extracts a rest binding from an array-pattern destructure', () => { + const symbols = parseJS(`const [a, ...rest] = await import('./mod.js');`); + expect(symbols.imports).toHaveLength(1); + expect(symbols.imports[0].names).toEqual(['a', 'rest']); + }); + }); + it('extracts call expressions', () => { const symbols = parseJS(`import { foo } from './bar'; foo(); baz();`); expect(symbols.calls).toContainEqual(expect.objectContaining({ name: 'foo' }));