Skip to content

fix(extractors): extract rest/default bindings from destructured const decls - #2277

Merged
carlos-alm merged 2 commits into
mainfrom
fix/issue-2051-destructured-binding-rest-default
Aug 4, 2026
Merged

fix(extractors): extract rest/default bindings from destructured const decls#2277
carlos-alm merged 2 commits into
mainfrom
fix/issue-2051-destructured-binding-rest-default

Conversation

@carlos-alm

Copy link
Copy Markdown
Contributor

Summary

  • extractDestructuredBindings (TS/WASM) and its mirror extract_destructured_bindings (native) only recognized shorthand_property_identifier_pattern and pair_pattern children when creating constant-kind Definitions for a plain object-destructuring const declaration, so:
    • const { a, ...rest } = someValue; never got a Definition for rest.
    • const { a = 1 } = someValue; never got a Definition for a at all.
  • Verified present identically in both engines — a genuine parity-preserving gap, not a WASM-only bug.
  • Fixed by adding object_assignment_pattern (default value) and rest_pattern/rest_element branches to both engines, mirroring the equivalent fix already landed for extractDynamicImportNames/extract_dynamic_import_names in WASM dynamic-import destructure extraction misses rest/default-value bindings that native engine handles #1920 (PR fix(extractors): extract rest/default bindings from dynamic import() destructures #2052). The TS side reuses the existing shared extractRestPatternIdentifier helper; the Rust side reuses the existing extract_rest_identifier helper.
  • Both the walk-based and query-based paths call the same shared extractDestructuredBindings/extract_destructured_bindings function, so both are fixed by this one change.
  • Checked the array-pattern counterpart (extractArrayPatternBindings/extract_array_pattern_bindings) for the same class of bug per the issue's broader "destructured-binding" title — it already correctly handles both assignment_pattern (default) and rest_pattern/rest_element in both engines (added in Engine parity: native JS extractor emits no definition for const array-pattern destructuring #1901), so no change was needed there.

Repro (issue's own example)

const { a, ...rest } = someValue;
// before: definitions contains `a` but not `rest`
// after:  definitions contains both `a` and `rest`, kind 'constant'

const { a = 1 } = someValue;
// before: definitions does not contain `a` at all
// after:  definitions contains `a`, kind 'constant'

Confirmed via a real buildGraph() run against a small fixture on both engine: 'wasm' and engine: 'native' (native addon rebuilt + codesigned locally) — both produce identical constant definitions: a, d, rest, someValue.

Test plan

  • npx vitest run tests/parsers/javascript.test.ts — 291 passed (3 new, covering rest/default/mixed bindings in object-pattern destructured const declarations)
  • cargo test --lib javascript (crates/codegraph-core) — 216 passed (3 new, mirroring the TS cases)
  • Rebuilt and codesigned the native addon locally (npx napi build --platform --release + codesign --sign - --force) so the fix was exercised by the real native engine
  • npm test — full suite: 273 files, 4427 passed, 30 skipped, 2 todo, 0 failed
  • npm run lint — clean
  • node scripts/parity-compare.mjs --langs javascript,typescript,tsx,pts-javascript,dynamic-javascript,dynamic-typescript --hybrid — PARITY OK, all engines identical across all 6 JS/TS-related fixtures
  • npx vitest run tests/benchmarks/resolution/resolution-benchmark.test.ts — 206 passed, javascript/typescript/pts-javascript all at 100% precision/recall
  • codegraph diff-impact --staged -T — confirms the change is scoped to exactly extractDestructuredBindings, 6 transitive callers, all within the JS/TS extractor

Closes #2051

…t 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
@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR completes destructured const binding extraction for object rest elements, shorthand defaults, and renamed defaults while preserving native/WASM parity.

  • Adds rest and default-pattern handling to the TypeScript extractor.
  • Mirrors the behavior in the Rust native extractor.
  • Adds regression coverage for plain, mixed, rest, shorthand-default, and renamed-default bindings.

Confidence Score: 5/5

The PR appears safe to merge.

The previously reported renamed-default omission is addressed in both extraction engines, with the nested assignment pattern’s local identifier now emitted through the shared destructuring helpers; no blocking failure remains.

Important Files Changed

Filename Overview
src/extractors/javascript.ts Extends the shared object-destructuring helper to emit constant definitions for rest, shorthand-default, and renamed-default bindings.
crates/codegraph-core/src/extractors/javascript.rs Mirrors the TypeScript extraction behavior in the native engine and adds native regression coverage.
tests/parsers/javascript.test.ts Adds parser regression tests covering the newly supported object-destructuring binding forms.

Reviews (2): Last reviewed commit: "fix(extractors): extract renamed destruc..." | Re-trigger Greptile

) {
definitions.push({ name: value.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

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Codegraph Impact Analysis

1 functions changed6 callers affected across 1 files

  • extractDestructuredBindings in src/extractors/javascript.ts:1587 (6 transitive callers)

… 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
@carlos-alm

Copy link
Copy Markdown
Contributor Author

Good catch — fixed in 0fa8d57. extractDestructuredBindings/extract_destructured_bindings's pair_pattern/pair branch now also handles an assignment_pattern nested under value (const { key: local = fallback } = x), mirroring the identical branch already in extractDynamicImportNames/collect_object_pattern_names since #1824. Added regression tests in both engines confirming local gets a constant Definition and key does not.

@carlos-alm

Copy link
Copy Markdown
Contributor Author

@greptileai

@carlos-alm
carlos-alm merged commit 4025aff into main Aug 4, 2026
30 checks passed
@carlos-alm
carlos-alm deleted the fix/issue-2051-destructured-binding-rest-default branch August 4, 2026 14:07
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 4, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JS/TS destructured-binding definition extraction misses rest/default bindings in both engines

1 participant