Skip to content
Merged
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
134 changes: 134 additions & 0 deletions crates/codegraph-core/src/extractors/javascript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3475,6 +3475,12 @@ fn handle_instanceof_value_ref(node: &Node, source: &[u8], calls: &mut Vec<Call>
/// destructured value (e.g. `const { dbPath } = workerData`). `constant`-kind
/// nodes remain fully resolvable as call targets — call-target resolution is
/// kind-agnostic — so callback-style destructured bindings still resolve.
///
/// Also handles a shorthand default value (`const { a = 1 } = value`, node
/// kind `object_assignment_pattern`) and a rest element (`const { a, ...rest }
/// = value`, node kind `rest_pattern`/`rest_element`) — both were previously
/// dropped entirely, the same class of bug fixed for dynamic-import
/// destructure extraction in #1920 (see `extract_rest_identifier`) (#2051).
/// Mirrors the TS extractor's `extractDestructuredBindings`.
fn extract_destructured_bindings(
pattern: &Node,
Expand Down Expand Up @@ -3519,9 +3525,77 @@ fn extract_destructured_bindings(
content_hash: None,
accessor_kind: None,
});
} else if value.kind() == "assignment_pattern" {
// { original: renamed = defaultValue } — the local
// binding is the assignment_pattern's left identifier
// (Greptile follow-up to #2051, mirrors the identical
// branch already in collect_object_pattern_names
// since #1824).
if let Some(left) = value.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,
bodyless: None,
content_hash: None,
accessor_kind: None,
});
}
}
}
}
}
"object_assignment_pattern" => {
// { a = defaultValue } — shorthand binding with a default
// value; the bound name is the left-hand identifier (#2051,
// mirrors #1920's fix to collect_object_pattern_names).
if let Some(left) = child.child_by_field_name("left") {
if left.kind() == "shorthand_property_identifier_pattern"
|| 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,
bodyless: None,
content_hash: None,
accessor_kind: None,
});
}
}
}
"rest_pattern" | "rest_element" => {
// { a, ...rest } — the rest binding was silently dropped
// entirely before (#2051, mirrors #1920).
let mut rest_names = Vec::new();
extract_rest_identifier(&child, source, &mut rest_names);
for name in rest_names {
definitions.push(Definition {
name,
kind: "constant".to_string(),
line,
end_line: Some(end_line),
decorators: None,
complexity: None,
cfg: None,
children: None,
bodyless: None,
content_hash: None,
accessor_kind: None,
});
}
}
_ => {}
}
}
Expand Down Expand Up @@ -6611,6 +6685,66 @@ mod tests {
assert!(!s.definitions.iter().any(|d| d.name == "original"), "should not use the original key");
}

// Regression tests for #2051: extract_destructured_bindings's object_pattern
// branch only recognized shorthand_property_identifier_pattern and
// pair_pattern children, so a rest element (`...rest`) never got a
// Definition at all and a shorthand default (`{ a = 1 }`) produced no
// Definition either — the same class of bug fixed for dynamic-import
// destructure extraction in #1920, but for the generic
// destructured-const-binding path used by any object destructure.

#[test]
fn extracts_constant_definition_for_rest_binding_alongside_plain_names() {
let s = parse_js("const { a, ...rest } = someValue;");
let names: Vec<&str> = s.definitions.iter().map(|d| d.name.as_str()).collect();
assert!(names.contains(&"a"), "should extract a definition");
assert!(names.contains(&"rest"), "should extract rest definition");
let rest = s.definitions.iter().find(|d| d.name == "rest").unwrap();
assert_eq!(rest.kind, "constant");
}

#[test]
fn extracts_constant_definition_for_shorthand_default_value_binding() {
let s = parse_js("const { a = 1 } = someValue;");
let def = s
.definitions
.iter()
.find(|d| d.name == "a")
.expect("should extract a definition for the default-valued binding");
assert_eq!(def.kind, "constant");
}

#[test]
fn extracts_mixed_plain_renamed_default_and_rest_destructured_bindings() {
let s = parse_js("const { a, b: alias, c = 1, ...rest } = someValue;");
for expected in ["a", "alias", "c", "rest"] {
let def = s
.definitions
.iter()
.find(|d| d.name == expected)
.unwrap_or_else(|| panic!("should extract {expected} definition"));
assert_eq!(def.kind, "constant");
}
assert!(!s.definitions.iter().any(|d| d.name == "b"), "should not use the original key");
}

#[test]
fn extracts_constant_definition_for_renamed_binding_with_default_value() {
// Greptile follow-up: { key: local = fallback } nests an
// assignment_pattern under pair_pattern's value field — a distinct
// shape from the plain shorthand default ({ a = 1 }) case above.
// Without this branch the pair_pattern handler rejected the nested
// assignment_pattern and `local` never got a Definition at all.
let s = parse_js("const { key: local = fallback } = someValue;");
let def = s
.definitions
.iter()
.find(|d| d.name == "local")
.expect("should extract local definition");
assert_eq!(def.kind, "constant");
assert!(!s.definitions.iter().any(|d| d.name == "key"), "should not use the original key");
}

/// Regression test for issue #1271: native engine missing receiver edges.
/// Uses the exact sample-project index.js content (CommonJS, constructor
/// inside a function body). The extractor must produce:
Expand Down
28 changes: 28 additions & 0 deletions src/extractors/javascript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1577,6 +1577,12 @@ function handleTypeAliasDecl(node: TreeSitterNode, ctx: ExtractorOutput): void {
* (`TOP_LEVEL_BINDING_KINDS` in call-resolver.ts) — so callback-style
* destructured bindings (`const { handleToken } = router; handleToken(req)`)
* still resolve correctly.
*
* Also handles a shorthand default value (`const { a = 1 } = value`, node
* type `object_assignment_pattern`) and a rest element (`const { a, ...rest }
* = value`, node type `rest_pattern`/`rest_element`) — both were previously
* dropped entirely, the same class of bug fixed for dynamic-import destructure
* extraction in #1920 (see `extractRestPatternIdentifier`) (#2051).
*/
function extractDestructuredBindings(
pattern: TreeSitterNode,
Expand All @@ -1601,7 +1607,29 @@ function extractDestructuredBindings(
(value.type === 'identifier' || value.type === 'shorthand_property_identifier_pattern')
) {
definitions.push({ name: value.text, kind: 'constant', line, endLine });
} else if (value?.type === 'assignment_pattern') {
// { original: renamed = defaultValue } — the local binding is the
// assignment_pattern's left-hand identifier (Greptile follow-up to
// #2051, mirrors the identical branch already in
// extractDynamicImportNames since #1824).
const left = value.childForFieldName('left');
if (left?.type === 'identifier') {
definitions.push({ name: left.text, kind: 'constant', line, endLine });
}
}
} else if (child.type === 'object_assignment_pattern') {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Renamed defaults remain unbound

When a top-level const uses a renamed default such as const { key: local = fallback } = source, the pair_pattern branch rejects its nested assignment_pattern, causing the local Definition to be omitted and downstream calls or references to remain unresolved.

Knowledge Base Used:

Fix in Claude Code

// { a = defaultValue } — shorthand binding with a default value; the
// bound name is the left-hand identifier (#2051, mirrors #1920's fix
// to extractDynamicImportNames).
const left = child.childForFieldName('left');
if (left?.type === 'shorthand_property_identifier_pattern' || left?.type === 'identifier') {
definitions.push({ name: left.text, kind: 'constant', line, endLine });
}
} else if (child.type === 'rest_pattern' || child.type === 'rest_element') {
// { a, ...rest } — the rest binding was silently dropped entirely
// before (#2051, mirrors #1920).
const inner = extractRestPatternIdentifier(child);
if (inner) definitions.push({ name: inner, kind: 'constant', line, endLine });
}
}
}
Expand Down
50 changes: 50 additions & 0 deletions tests/parsers/javascript.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1513,6 +1513,56 @@ function runDemo(reporter: Reporter, users: string[]): void {
);
});

describe('destructured const binding rest/default definitions (#2051)', () => {
// extractDestructuredBindings's object_pattern branch only recognized
// shorthand_property_identifier_pattern and pair_pattern children, so a
// rest element (`...rest`) never got a Definition at all and a shorthand
// default (`{ a = 1 }`) produced no Definition either — the same class of
// bug fixed for dynamic-import destructure extraction in #1920, but for
// the generic destructured-const-binding path used by any object
// destructure, not just dynamic imports.

it('extracts a constant Definition for a rest binding alongside plain names', () => {
const symbols = parseJS(`const { a, ...rest } = someValue;`);
expect(symbols.definitions).toContainEqual(
expect.objectContaining({ name: 'a', kind: 'constant' }),
);
expect(symbols.definitions).toContainEqual(
expect.objectContaining({ name: 'rest', kind: 'constant' }),
);
});

it('extracts a constant Definition for a shorthand default-value binding', () => {
const symbols = parseJS(`const { a = 1 } = someValue;`);
expect(symbols.definitions).toContainEqual(
expect.objectContaining({ name: 'a', kind: 'constant' }),
);
});

it('extracts a mix of plain, renamed, default, and rest bindings', () => {
const symbols = parseJS(`const { a, b: alias, c = 1, ...rest } = someValue;`);
for (const name of ['a', 'alias', 'c', 'rest']) {
expect(symbols.definitions).toContainEqual(
expect.objectContaining({ name, kind: 'constant' }),
);
}
expect(symbols.definitions).not.toContainEqual(expect.objectContaining({ name: 'b' }));
});

it('extracts a constant Definition for a renamed binding with a default value', () => {
// Greptile follow-up: { key: local = fallback } nests an
// assignment_pattern under pair_pattern's value field — a distinct
// shape from the plain shorthand default ({ a = 1 }) case above.
// Without this branch the pair_pattern handler rejected the nested
// assignment_pattern and `local` never got a Definition at all.
const symbols = parseJS(`const { key: local = fallback } = someValue;`);
expect(symbols.definitions).toContainEqual(
expect.objectContaining({ name: 'local', kind: 'constant' }),
);
expect(symbols.definitions).not.toContainEqual(expect.objectContaining({ name: 'key' }));
});
});

// let/var object-literal method definitions
it('extracts qualified definitions from var object-literal arrow functions', () => {
// `var x = { a: function() {} }` — native produces `x.a`, WASM must too.
Expand Down
Loading