morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #3066] [CLOSED] fix(javascript): keep unconditional loop initializers exact #5590

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3066
Author: @CodesWithRobi
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/javascript-unconditional-loop-initializer

📝 Commits (1)

c46c3ee fix(javascript): keep unconditional loop initializers exact

📊 Changes

2 files changed (+25 additions, -0 deletions)

View changed files

📝 src/domain/javascript/electronStaticAnalysisIdentity.test.ts (+20 -0)
📝 src/domain/javascript/javascriptSemanticAnalysis.ts (+5 -0)

📄 Description

Summary

A var Electron binding whose only initializer sits in a for loop initializer or a do...while body is dropped, so the recovered BrowserWindow construction and its literal preload path are lost. The plain var form recovers them.

hasConditionalInitializer treated every loop ancestor as a condition that can skip the initializer. A for initializer runs before the first test, and a do body runs at least once, so declarations there always reach their initializer. This excludes only those two positions; loop bodies, if branches, switch cases, and try statements still make an initializer conditional.

Fixes #2648.

Problem and expected behavior

Smallest trigger: analyzeJavaScriptStaticSource('for (var electron = require("electron"); false;) {} new electron.BrowserWindow({});') reports no window, while the plain var electron = require("electron"); form recovers it. Expected: one finding per source, with the literal preload path retained.

Change and scope

One classification fix at the producing boundary (hasConditionalInitializer in src/domain/javascript/javascriptSemanticAnalysis.ts) plus a regression test. Not included: any change to other control-flow positions (loop bodies, branches, cases, try) or to the identity-recovery rule itself.

Contract and boundary impact

Semantic owner and earliest changed stage: unconditional-initializer classification in javascriptSemanticAnalysis.ts
Contribution scope fit and linked maintainer decision: standalone fix within existing JavaScript boundaries; not applicable
CLI and MCP/tool-catalog contract: none
Provider, bridge, target-format, or platform compatibility: none
Evidence, artifact, provenance, or reconstruction contract: none
Target selection, authority, upstream behavior, and cleanup details: none
Generated metadata, package, or installation impact: none

Expected effects

Read existing files or captured data.

Evidence and regression coverage

Tests added or updated: src/domain/javascript/electronStaticAnalysisIdentity.test.ts — a for initializer and a do body each recover one finding with their literal preload paths, while a while body and an if branch stay unresolved.
Base reproduction or other evidence: the new test returned [] before the change.
User-visible CLI/MCP output: recovered browser_windows findings for the two positions.
Remaining proof gaps: none; the source strings are inert analyzer input.

Validation performed

npm run test:local -- src/domain/javascript/electronStaticAnalysisIdentity.test.ts — 8 passed (the new test failed before the fix)
all 57 src/domain/javascript/*.test.ts — 1295 passed
npm run test:local -- src/domain/webBundleAnalyzer.test.ts src/application/javascript/JavaScriptArtifactPathResolution.test.ts src/application/javascript/JavaScriptSemanticGraphElectronProjection.test.ts — 176 passed
npm run test:focused -- tests/boundary/filesystem/javascript-applications/javascriptFileUrlPaths.test.ts tests/boundary/filesystem/javascript-applications/javascriptArtifactReconstruction.test.ts — 41 passed
npm run check:fast — pass

Compatibility, safety, and release

Breaking changes or migration steps: none
Real Hopper/Ghidra, browser, or OS coverage: not applicable; static source analysis
Package or release metadata impact: none
Security, privacy, process, or containment review: none

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. (recovered findings above)
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 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/3066 **Author:** [@CodesWithRobi](https://github.com/CodesWithRobi) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/javascript-unconditional-loop-initializer` --- ### 📝 Commits (1) - [`c46c3ee`](https://github.com/morluto/rea/commit/c46c3ee8691541da6c863770a7a0a91cd611681e) fix(javascript): keep unconditional loop initializers exact ### 📊 Changes **2 files changed** (+25 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/electronStaticAnalysisIdentity.test.ts` (+20 -0) 📝 `src/domain/javascript/javascriptSemanticAnalysis.ts` (+5 -0) </details> ### 📄 Description ## Summary A `var` Electron binding whose only initializer sits in a `for` loop initializer or a `do...while` body is dropped, so the recovered `BrowserWindow` construction and its literal preload path are lost. The plain `var` form recovers them. `hasConditionalInitializer` treated every loop ancestor as a condition that can skip the initializer. A `for` initializer runs before the first test, and a `do` body runs at least once, so declarations there always reach their initializer. This excludes only those two positions; loop bodies, `if` branches, `switch` cases, and `try` statements still make an initializer conditional. Fixes #2648. ## Problem and expected behavior Smallest trigger: `analyzeJavaScriptStaticSource('for (var electron = require("electron"); false;) {} new electron.BrowserWindow({});')` reports no window, while the plain `var electron = require("electron");` form recovers it. Expected: one finding per source, with the literal preload path retained. ## Change and scope One classification fix at the producing boundary (`hasConditionalInitializer` in `src/domain/javascript/javascriptSemanticAnalysis.ts`) plus a regression test. Not included: any change to other control-flow positions (loop bodies, branches, cases, `try`) or to the identity-recovery rule itself. ## Contract and boundary impact - Semantic owner and earliest changed stage: unconditional-initializer classification in `javascriptSemanticAnalysis.ts` - Contribution scope fit and linked maintainer decision: standalone fix within existing JavaScript boundaries; not applicable - CLI and MCP/tool-catalog contract: none - Provider, bridge, target-format, or platform compatibility: none - Evidence, artifact, provenance, or reconstruction contract: none - Target selection, authority, upstream behavior, and cleanup details: none - Generated metadata, package, or installation impact: none Expected effects - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: `src/domain/javascript/electronStaticAnalysisIdentity.test.ts` — a `for` initializer and a `do` body each recover one finding with their literal preload paths, while a `while` body and an `if` branch stay unresolved. - Base reproduction or other evidence: the new test returned `[]` before the change. - User-visible CLI/MCP output: recovered `browser_windows` findings for the two positions. - Remaining proof gaps: none; the source strings are inert analyzer input. ## Validation performed - `npm run test:local -- src/domain/javascript/electronStaticAnalysisIdentity.test.ts` — 8 passed (the new test failed before the fix) - all 57 `src/domain/javascript/*.test.ts` — 1295 passed - `npm run test:local -- src/domain/webBundleAnalyzer.test.ts src/application/javascript/JavaScriptArtifactPathResolution.test.ts src/application/javascript/JavaScriptSemanticGraphElectronProjection.test.ts` — 176 passed - `npm run test:focused -- tests/boundary/filesystem/javascript-applications/javascriptFileUrlPaths.test.ts tests/boundary/filesystem/javascript-applications/javascriptArtifactReconstruction.test.ts` — 41 passed - `npm run check:fast` — pass ## Compatibility, safety, and release - Breaking changes or migration steps: none - Real Hopper/Ghidra, browser, or OS coverage: not applicable; static source analysis - Package or release metadata impact: none - Security, privacy, process, or containment review: none ## 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. (recovered findings above) - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <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:48 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:11:00 +00:00

[GH-ISSUE #5653] [Tracker] New issue intake by area (October 2026) #3423

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#5590

No description provided.

Rows
Columns