From 120ae8cb5476250b858c391cee17bb5f2d16884d Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Tue, 4 Aug 2026 07:11:50 -0600 Subject: [PATCH 1/2] fix(extractors): extract rest/default bindings from destructured const decls extractDestructuredBindings (TS/WASM) and its mirror extract_destructured_bindings (native) only recognized shorthand_property_identifier_pattern and pair_pattern children when creating constant Definitions for a plain object-destructuring const declaration, so `const { a, ...rest } = x` never got a Definition for rest and `const { a = 1 } = x` never got one for a at all. Add object_assignment_pattern and rest_pattern/rest_element branches to both engines, mirroring the equivalent fix already landed for extractDynamicImportNames/extract_dynamic_import_names in #1920. The array-pattern counterpart (extractArrayPatternBindings/ extract_array_pattern_bindings) already handled both cases correctly in both engines, so no change was needed there. docs check acknowledged: internal extractor bug fix, no new CLI surface, language support, or architecture change -- README.md, CLAUDE.md, and ROADMAP.md are unaffected. Closes #2051 Impact: 1 functions changed, 6 affected --- .../src/extractors/javascript.rs | 94 +++++++++++++++++++ src/extractors/javascript.ts | 19 ++++ tests/parsers/javascript.test.ts | 37 ++++++++ 3 files changed, 150 insertions(+) diff --git a/crates/codegraph-core/src/extractors/javascript.rs b/crates/codegraph-core/src/extractors/javascript.rs index 8e17c035..81956223 100644 --- a/crates/codegraph-core/src/extractors/javascript.rs +++ b/crates/codegraph-core/src/extractors/javascript.rs @@ -3475,6 +3475,12 @@ fn handle_instanceof_value_ref(node: &Node, source: &[u8], calls: &mut Vec /// 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, @@ -3522,6 +3528,51 @@ fn extract_destructured_bindings( } } } + "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, + }); + } + } _ => {} } } @@ -6611,6 +6662,49 @@ 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"); + } + /// 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: diff --git a/src/extractors/javascript.ts b/src/extractors/javascript.ts index c0a43bfa..d5969c07 100644 --- a/src/extractors/javascript.ts +++ b/src/extractors/javascript.ts @@ -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, @@ -1602,6 +1608,19 @@ function extractDestructuredBindings( ) { definitions.push({ name: value.text, kind: 'constant', line, endLine }); } + } else if (child.type === '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 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 }); } } } diff --git a/tests/parsers/javascript.test.ts b/tests/parsers/javascript.test.ts index 9ee9c012..fcd53327 100644 --- a/tests/parsers/javascript.test.ts +++ b/tests/parsers/javascript.test.ts @@ -1513,6 +1513,43 @@ 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' })); + }); + }); + // 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. From 0fa8d572da0322015ddbfa7f222d1ca2a833473a Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Tue, 4 Aug 2026 07:39:53 -0600 Subject: [PATCH 2/2] fix(extractors): extract renamed destructured bindings with a default value Greptile follow-up to the #2051 fix: extractDestructuredBindings (TS/WASM) and extract_destructured_bindings (native) rejected an assignment_pattern nested under a pair_pattern's value field, so a renamed binding with a default value (const { key: local = fallback } = x) never got a Definition for local at all. Add the assignment_pattern branch to both engines' pair_pattern/pair handling, mirroring the identical branch already present in extractDynamicImportNames/collect_object_pattern_names since #1824. docs check acknowledged: internal extractor bug fix, no new CLI surface, language support, or architecture change -- README.md, CLAUDE.md, and ROADMAP.md are unaffected. Impact: 1 functions changed, 6 affected --- .../src/extractors/javascript.rs | 40 +++++++++++++++++++ src/extractors/javascript.ts | 9 +++++ tests/parsers/javascript.test.ts | 13 ++++++ 3 files changed, 62 insertions(+) diff --git a/crates/codegraph-core/src/extractors/javascript.rs b/crates/codegraph-core/src/extractors/javascript.rs index 81956223..85a3f602 100644 --- a/crates/codegraph-core/src/extractors/javascript.rs +++ b/crates/codegraph-core/src/extractors/javascript.rs @@ -3525,6 +3525,29 @@ 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, + }); + } + } } } } @@ -6705,6 +6728,23 @@ mod tests { 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: diff --git a/src/extractors/javascript.ts b/src/extractors/javascript.ts index d5969c07..9dffdb23 100644 --- a/src/extractors/javascript.ts +++ b/src/extractors/javascript.ts @@ -1607,6 +1607,15 @@ 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') { // { a = defaultValue } — shorthand binding with a default value; the diff --git a/tests/parsers/javascript.test.ts b/tests/parsers/javascript.test.ts index fcd53327..0feacf96 100644 --- a/tests/parsers/javascript.test.ts +++ b/tests/parsers/javascript.test.ts @@ -1548,6 +1548,19 @@ function runDemo(reporter: Reporter, users: string[]): void { } 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