[PR #3063] [CLOSED] fix(javascript-semantic-calls): evaluate computed method keys in the enclosing callable #5587

New issue

Closed

opened 2026-10-31 01:07:47 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:07:47 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3063
Author: @adesir089-spec
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/computed-method-key-captures

📝 Commits (4)

1ac5678 fix(javascript-semantic-calls): evaluate computed method keys in the enclosing callable
caa4a5a fix(javascript-semantic): centralize computed-key callable ownership across collectors
6ae2565 Merge remote-tracking branch 'origin/main' into fix/computed-method-key-captures
3f3e98e Merge remote-tracking branch 'origin/main' into fix/computed-method-key-captures

📊 Changes

8 files changed (+245 additions, -107 deletions)

View changed files

📝 src/domain/javascript/javascriptSemanticAnalysis.calls.test.ts (+115 -0)
📝 src/domain/javascript/javascriptSemanticAsyncEffects.ts (+6 -10)
➕ src/domain/javascript/javascriptSemanticCallableStack.ts (+85 -0)
📝 src/domain/javascript/javascriptSemanticCalls.ts (+10 -50)
📝 src/domain/javascript/javascriptSemanticChildProcesses.ts (+11 -20)
📝 src/domain/javascript/javascriptSemanticDataEffectHelpers.ts (+7 -7)
📝 src/domain/javascript/javascriptSemanticObjects.ts (+5 -11)
📝 src/domain/javascript/javascriptSemanticPromises.ts (+6 -9)

📄 Description

Summary

Follow-up to #2573 / 896696b.

Computed object/class method keys were attributed to the method itself in the semantic call/capture pass. That produced false closure captures and wrong callerCallableIds for calls made inside a computed key. They are now attributed to the enclosing callable, matching JavaScript evaluation order and the scope analyzer.

Problem and expected behavior

collectJavaScriptSemanticCalls pushes an ObjectMethod/ClassMethod onto the callable stack when it enters the method node, before visiting the method's computed key. Every reference inside [key] was therefore owned by the method.

Smallest trigger (from the issue):

function outer() { const name = "m"; const obj = { [name]() { return 1; } }; class C { [name]() { return 2; } } }

closureCaptures reported two captures of name, owned by [computed@51] and [computed@87]. Expected: []. A computed method key is evaluated before entering the method's parameter and body environments, so neither method captures name. Likewise, in { [pick()]() {} } the call to pick belongs to the enclosing function, not the method.

Change and scope

While traversing the computed key of an ObjectMethod/ClassMethod, the method is set aside from the callable stack and restored when the key node exits. The node test is the same one the scope analyzer already uses for computed method keys (javascriptSemanticAnalysis.ts, scope traversal), so both passes now agree on where a key is evaluated. Callables nested inside a key are still pushed and popped normally. Not included: any change to scope construction, fingerprints, or key-name extraction.

Contribution scope: an ordinary fix within existing boundaries; maintainer scope decision not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: src/domain/javascript/javascriptSemanticCalls.ts, call/capture collection
Contribution scope fit and linked maintainer decision: ordinary bug fix; not applicable
CLI and MCP/tool-catalog contract: none (output shape unchanged; only the closureCaptures entries and callerCallableId/frontier owners for code inside computed method keys change)
Provider, bridge, target-format, or platform compatibility: none
Evidence, artifact, provenance, or reconstruction contract: none
Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable
Generated metadata, package, or installation impact: none

Expected effects: no change to target interaction; this is pure in-memory analysis of already-parsed source.

Evidence and regression coverage

Tests added or updated: evaluates computed method keys in the enclosing callable in javascriptSemanticAnalysis.calls.test.ts. It asserts that computed keys create no captures, that a method body reading the outer name still captures it, and that a call inside a key is owned by outer.
Base reproduction or other evidence: executed. With the source change reverted, the new test fails (1 failed | 11 passed); with the fix, 12/12 pass. The issue's exact repro (closureCaptures → []) passes with the fix.
User-visible CLI/MCP output (if applicable): not applicable
Remaining proof gaps: the full repository suite and the native/browser verifiers were not run (they need Ghidra, browsers, etc., not available on this machine); the change does not touch those paths.

Validation performed

npm run test:focused -- src/domain/javascript/javascriptSemanticAnalysis.calls.test.ts: 12 passed
npm run test:focused -- $(ls src/domain/javascript/*.test.ts): 57 files, 1,295 tests passed
npm run test:focused -- <5 other test files using the semantic/static analyzers>: 236 tests passed
npx tsc --noEmit: clean
oxlint and oxfmt --check on the changed files: clean
npm run verify:module-boundaries: clean

Environment: Windows 11, Node v24.21.0.

Compatibility, safety, and release

Breaking changes or migration steps: none
Real Hopper/Ghidra, browser, or OS coverage: not applicable
Package or release metadata impact: none
Security, privacy, process, or containment review: none needed (no I/O or execution changes)

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/3063 **Author:** [@adesir089-spec](https://github.com/adesir089-spec) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/computed-method-key-captures` --- ### 📝 Commits (4) - [`1ac5678`](https://github.com/morluto/rea/commit/1ac56780ad23c22ff3595c03029d8548855a5d4d) fix(javascript-semantic-calls): evaluate computed method keys in the enclosing callable - [`caa4a5a`](https://github.com/morluto/rea/commit/caa4a5a056291369c9a04107a44c9532308192bf) fix(javascript-semantic): centralize computed-key callable ownership across collectors - [`6ae2565`](https://github.com/morluto/rea/commit/6ae25656fa7377bcb750c5da3a8f04a82a4ee58d) Merge remote-tracking branch 'origin/main' into fix/computed-method-key-captures - [`3f3e98e`](https://github.com/morluto/rea/commit/3f3e98e547aec08314963b9e5b5e7894be7d9359) Merge remote-tracking branch 'origin/main' into fix/computed-method-key-captures ### 📊 Changes **8 files changed** (+245 additions, -107 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptSemanticAnalysis.calls.test.ts` (+115 -0) 📝 `src/domain/javascript/javascriptSemanticAsyncEffects.ts` (+6 -10) ➕ `src/domain/javascript/javascriptSemanticCallableStack.ts` (+85 -0) 📝 `src/domain/javascript/javascriptSemanticCalls.ts` (+10 -50) 📝 `src/domain/javascript/javascriptSemanticChildProcesses.ts` (+11 -20) 📝 `src/domain/javascript/javascriptSemanticDataEffectHelpers.ts` (+7 -7) 📝 `src/domain/javascript/javascriptSemanticObjects.ts` (+5 -11) 📝 `src/domain/javascript/javascriptSemanticPromises.ts` (+6 -9) </details> ### 📄 Description ## Summary Follow-up to #2573 / 896696b. Computed object/class method keys were attributed to the method itself in the semantic call/capture pass. That produced false closure captures and wrong `callerCallableId`s for calls made inside a computed key. They are now attributed to the enclosing callable, matching JavaScript evaluation order and the scope analyzer. ## Problem and expected behavior `collectJavaScriptSemanticCalls` pushes an `ObjectMethod`/`ClassMethod` onto the callable stack when it enters the method node, before visiting the method's computed key. Every reference inside `[key]` was therefore owned by the method. Smallest trigger (from the issue): ```js function outer() { const name = "m"; const obj = { [name]() { return 1; } }; class C { [name]() { return 2; } } } ``` `closureCaptures` reported two captures of `name`, owned by `[computed@51]` and `[computed@87]`. Expected: `[]`. A computed method key is evaluated before entering the method's parameter and body environments, so neither method captures `name`. Likewise, in `{ [pick()]() {} }` the call to `pick` belongs to the enclosing function, not the method. ## Change and scope While traversing the computed key of an `ObjectMethod`/`ClassMethod`, the method is set aside from the callable stack and restored when the key node exits. The node test is the same one the scope analyzer already uses for computed method keys (`javascriptSemanticAnalysis.ts`, scope traversal), so both passes now agree on where a key is evaluated. Callables nested inside a key are still pushed and popped normally. Not included: any change to scope construction, fingerprints, or key-name extraction. Contribution scope: an ordinary fix within existing boundaries; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/domain/javascript/javascriptSemanticCalls.ts`, call/capture collection - Contribution scope fit and linked maintainer decision: ordinary bug fix; not applicable - CLI and MCP/tool-catalog contract: none (output shape unchanged; only the `closureCaptures` entries and `callerCallableId`/frontier owners for code inside computed method keys change) - Provider, bridge, target-format, or platform compatibility: none - Evidence, artifact, provenance, or reconstruction contract: none - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable - Generated metadata, package, or installation impact: none Expected effects: no change to target interaction; this is pure in-memory analysis of already-parsed source. ## Evidence and regression coverage - Tests added or updated: `evaluates computed method keys in the enclosing callable` in `javascriptSemanticAnalysis.calls.test.ts`. It asserts that computed keys create no captures, that a method body reading the outer `name` still captures it, and that a call inside a key is owned by `outer`. - Base reproduction or other evidence: executed. With the source change reverted, the new test fails (1 failed | 11 passed); with the fix, 12/12 pass. The issue's exact repro (`closureCaptures` → `[]`) passes with the fix. - User-visible CLI/MCP output (if applicable): not applicable - Remaining proof gaps: the full repository suite and the native/browser verifiers were not run (they need Ghidra, browsers, etc., not available on this machine); the change does not touch those paths. ## Validation performed - `npm run test:focused -- src/domain/javascript/javascriptSemanticAnalysis.calls.test.ts`: 12 passed - `npm run test:focused -- $(ls src/domain/javascript/*.test.ts)`: 57 files, 1,295 tests passed - `npm run test:focused -- <5 other test files using the semantic/static analyzers>`: 236 tests passed - `npx tsc --noEmit`: clean - `oxlint` and `oxfmt --check` on the changed files: clean - `npm run verify:module-boundaries`: clean Environment: Windows 11, Node v24.21.0. ## Compatibility, safety, and release - Breaking changes or migration steps: none - Real Hopper/Ghidra, browser, or OS coverage: not applicable - Package or release metadata impact: none - Security, privacy, process, or containment review: none needed (no I/O or execution changes) ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:07:47 +00:00