Skip to content

fix: unwrap computed string-literal keys in typeMap/prototype extraction sites#2022

Open
carlos-alm wants to merge 1 commit into
fix/issue-1882-thread-config-db-busytimeoutms-into-the-rustfrom
fix/issue-1884-computed-string-literal-keys-not-unwrapped-in
Open

fix: unwrap computed string-literal keys in typeMap/prototype extraction sites#2022
carlos-alm wants to merge 1 commit into
fix/issue-1882-thread-config-db-busytimeoutms-into-the-rustfrom
fix/issue-1884-computed-string-literal-keys-not-unwrapped-in

Conversation

@carlos-alm

Copy link
Copy Markdown
Contributor

Summary

#1764 fixed extractObjectLiteralFunctions's pair branch (and its Rust mirror) so a computed string-literal key like { ['foo']: () => {} } resolves to the plain qualified Definition.name (obj.foo) instead of the raw bracket/quote text (obj.['foo']). This issue covers the remaining sites found during that fix — same "only unwrap plain string-typed keys, fall back to raw text for computed_property_name" gap, but in functions that feed typeMap seeding and other adjacent extraction paths rather than Definition.name directly. Left uncorrected, they produce dangling/garbled typeMap keys and under-extracted bindings for the affected call patterns (correctness bug, not just cosmetic).

TypeScript (src/extractors/javascript.ts)

  • handleObjectLiteralTypeMap's pair branch — same string-only gap for typeMap seeding.
  • handleObjectLiteralTypeMap's method_definition branch — used raw nameNode.text unconditionally (would even mishandle [Symbol.iterator]-style keys).
  • handleDefinePropertyTypeMap's defineProperties pair-iteration branch.
  • seedProtoProperties's pair branch (Object.create({ ['foo']: fn })).
  • collectObjectRestParams's pair branch — previously blanket-skipped all computed keys (including resolvable string literals); now unwraps resolvable ones instead of under-extracting.
  • extractPrototypeObjectLiteral's method_definition branch — didn't call resolveMethodDefinitionName (cross-engine divergence: Rust's mirror already did).
  • extractPrototypeObjectLiteral's pair branch.

Extracted a shared resolvePairKeyName(keyNode) helper (mirrors the Rust resolve_pair_key_name added in #1764) and routed all of the above through it.

Rust (crates/codegraph-core/src/extractors/javascript.rs)

  • seed_object_create_entries, seed_descriptor_object, seed_objlit_type_map_entries, extract_js_prototype_object_literal — same string-only gap in their pair arms; now route through the existing resolve_pair_key_name helper.
  • collect_object_rest_params's pair arm had the identical blanket-skip-all-computed-keys bug as its TS counterpart (not explicitly listed in the issue body, but the same root cause in the mirrored function) — fixed the same way for engine parity.

Both engines verified to produce identical results on a manual repro exercising every fixed site (codegraph query <fn> -T shows identical caller edges under --engine wasm and --engine native).

Closes #1884

Test plan

  • npx vitest run tests/parsers/javascript.test.ts — 230 passed, including 13 new tests covering every fixed site (both the resolvable-computed-key and non-string-computed-key-skip cases)
  • cargo test --manifest-path crates/codegraph-core/Cargo.toml --lib — 577 passed (8 new), including mirrors of the same 13 TS cases
  • npm test — full suite: 3912 passed, 1 pre-existing unrelated flake (checkNoNewCycles cycle-node ordering, filed separately as Flaky cycle-node ordering in checkNoNewCycles full-suite run (tests/integration/check.test.ts) #1990, reproduces identically with this fix's files reverted)
  • npm run lint / npm run build — clean
  • Native addon rebuilt from this branch's Rust changes, codesigned, and swapped into the worktree's node_modules/@optave/codegraph-darwin-arm64 for a real end-to-end check (not npm-registry prebuilt) — --engine wasm and --engine native produce identical node/edge counts and caller sets for the repro

…type extraction sites

extractObjectLiteralFunctions' pair branch was fixed for this gap in #1764, but
several adjacent extraction sites in both engines still fell back to raw
bracket/quote text for computed_property_name keys instead of unwrapping
resolvable string literals, producing garbled typeMap/prototype entries like
obj.['foo'] and under-extracting object-rest-param bindings for computed keys.

Introduces a shared resolvePairKeyName (TS) / reuses the existing
resolve_pair_key_name (Rust, added in #1764) helper and routes these sites
through it:

- handleObjectLiteralTypeMap's pair and method_definition branches
- handleDefinePropertyTypeMap's defineProperties pair-iteration branch
- seedProtoProperties's pair branch (Object.create prototype literal)
- collectObjectRestParams's pair branch (now unwraps resolvable computed
  keys instead of blanket-skipping all computed keys)
- extractPrototypeObjectLiteral's method_definition and pair branches

Mirrors: seed_object_create_entries, seed_descriptor_object,
seed_objlit_type_map_entries, extract_js_prototype_object_literal, and
collect_object_rest_params in the Rust extractor.

Impact: 6 functions changed, 15 affected
@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extends the computed string-literal key fix from #1764 to the remaining extraction sites in both the TypeScript and Rust engines. Previously only extractObjectLiteralFunctions unwrapped ['foo']-style pair keys; seven other sites in handleObjectLiteralTypeMap, handleDefinePropertyTypeMap, seedProtoProperties, collectObjectRestParams, and extractPrototypeObjectLiteral still fell back to raw bracket/quote text, producing garbled or dangling typeMap entries.

  • TypeScript (src/extractors/javascript.ts): Extracts a new resolvePairKeyName(keyNode) helper (mirroring resolveMethodDefinitionName) and routes all seven affected pair/method_definition branches through it; the helper strips outer quotes from string nodes, delegates computed_property_name to the existing resolveComputedKeyName, and returns the raw text for plain identifiers.
  • Rust (crates/codegraph-core/src/extractors/javascript.rs): Routes four matching pair arms (seed_object_create_entries, seed_descriptor_object, seed_objlit_type_map_entries, extract_js_prototype_object_literal) and collect_object_rest_params through the existing resolve_pair_key_name helper, eliminating identical string-only gaps and achieving full TS/native engine parity.
  • Tests (tests/parsers/javascript.test.ts / Rust #[test]): 13 new test cases per engine cover every fixed site with both resolvable-computed-key and non-string-computed-key-skip scenarios.

Confidence Score: 5/5

Safe to merge — all changed sites are narrowly scoped to key-resolution logic with direct test coverage, and both engines produce identical outputs for the fixed patterns.

The fix is a clean mechanical substitution: ad-hoc inline key-stripping in seven pair/method_definition branches is replaced by a well-tested shared helper that already existed for the extractObjectLiteralFunctions path. Every changed site now guards on an empty-string result, the Rust and TypeScript helpers are structurally identical, and 13 new tests per engine cover both the resolvable-computed-key and non-string-computed-key-skip cases. The only behavioral change beyond the bug fix is that previously garbled typeMap keys (e.g. obj.[Symbol.iterator]) are now silently dropped rather than emitted, which is strictly correct. No unrelated paths are touched.

No files require special attention. All three changed files are well-structured and consistently handled.

Important Files Changed

Filename Overview
src/extractors/javascript.ts Introduces resolvePairKeyName helper and routes seven affected pair/method_definition branches through it; empty-key guards added at all call sites; logic is consistent with the pre-existing resolveMethodDefinitionName/resolveComputedKeyName pair.
crates/codegraph-core/src/extractors/javascript.rs Routes five pair arms through the existing resolve_pair_key_name helper, removing ad-hoc string-stripping and computed-key blanket-skips; collect_object_rest_params also fixed for parity with the TS mirror.
tests/parsers/javascript.test.ts Adds 13 new TS test cases covering all fixed sites; both positive (resolvable ['key']) and negative (non-string [Symbol.iterator]) paths are exercised. Confidence values in assertions match the producing function (0.9 for emitPrototypeMethod, 0.85 for typeMap seeding paths).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[pair node key field] --> B{keyNode.type?}
    B -->|string| C[Strip outer quotes]
    B -->|computed_property_name| D[resolveComputedKeyName]
    B -->|identifier or other| E[Return keyNode.text as-is]
    D --> F{inner child type?}
    F -->|string or string_fragment| G[Return stripped string content]
    F -->|other e.g. Symbol.x| H[Return empty string]
    C --> I{keyName empty?}
    G --> I
    E --> I
    H --> I
    I -->|yes| J[skip - no entry emitted]
    I -->|no| K[Emit qualified typeMap or definition entry]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[pair node key field] --> B{keyNode.type?}
    B -->|string| C[Strip outer quotes]
    B -->|computed_property_name| D[resolveComputedKeyName]
    B -->|identifier or other| E[Return keyNode.text as-is]
    D --> F{inner child type?}
    F -->|string or string_fragment| G[Return stripped string content]
    F -->|other e.g. Symbol.x| H[Return empty string]
    C --> I{keyName empty?}
    G --> I
    E --> I
    H --> I
    I -->|yes| J[skip - no entry emitted]
    I -->|no| K[Emit qualified typeMap or definition entry]
Loading

Reviews (1): Last reviewed commit: "fix(extractors): unwrap computed string-..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Codegraph Impact Analysis

6 functions changed15 callers affected across 1 files

  • resolvePairKeyName in src/extractors/javascript.ts:2207 (11 transitive callers)
  • handleObjectLiteralTypeMap in src/extractors/javascript.ts:2588 (3 transitive callers)
  • handleDefinePropertyTypeMap in src/extractors/javascript.ts:2804 (11 transitive callers)
  • seedProtoProperties in src/extractors/javascript.ts:2901 (3 transitive callers)
  • collectObjectRestParams in src/extractors/javascript.ts:3145 (11 transitive callers)
  • extractPrototypeObjectLiteral in src/extractors/javascript.ts:4636 (11 transitive callers)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant