Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 58 additions & 4 deletions crates/codegraph-core/src/extractors/javascript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3719,11 +3719,18 @@ fn collect_array_pattern_names(pattern: &Node, source: &[u8]) -> Vec<String> {
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<String>) {
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;
}
}
}
}
Expand Down Expand Up @@ -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);");
Expand Down
37 changes: 34 additions & 3 deletions src/extractors/javascript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
32 changes: 32 additions & 0 deletions tests/parsers/javascript.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' }));
Expand Down
Loading