morluto/rea

Fork 0

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

[PR #2709] [CLOSED] fix(javascript): keep the exact empty CommonJS export name #5333

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2709
Author: @XiaoBinGan
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: fix/javascript-empty-export-name

📝 Commits (10+)

c49c88c fix(javascript): keep the exact empty CommonJS export name
ea7e7dd fix(javascript): accept an exact empty export name across the shape contract
1385e5e fix(process): report no remaining time after a provider deadline fires
70f1fe2 Merge captured main into PR #2709
95099f4 Merge remote-tracking branch 'origin/main' into fix/javascript-empty-export-name
505d10f Merge remote-tracking branch 'origin/main' into fix/javascript-empty-export-name
90f4c39 Merge remote-tracking branch 'origin/main' into fix/javascript-empty-export-name
cfbb486 Merge branch 'main' into fix/javascript-empty-export-name
289573a Merge remote-tracking branch 'origin/main' into fix/javascript-empty-export-name
44a1767 Merge remote-tracking branch 'origin/main' into fix/javascript-empty-export-name

📊 Changes

4 files changed (+35 additions, -2 deletions)

View changed files

📝 src/domain/javascript/javascriptExportShapeComparisonSchemas.test.ts (+23 -0)
📝 src/domain/javascript/javascriptExportShapeComparisonSchemas.ts (+5 -0)
📝 src/domain/javascript/javascriptSemanticAnalysis.empty_keys.test.ts (+2 -0)
📝 src/domain/javascript/javascriptSemanticProjection.ts (+5 -2)

📄 Description

semanticStaticPropertyKey deliberately preserves the empty string as an exact property key, but commonJsExportName treats that same value as a missing key.

Summary

exports[key] = … / module.exports[key] = … with a key that is not statically readable assigns an export whose name is unknown, and the projection reports the wildcard * rather than the variable name, so it never claims a real export. That intent is stated in the comment at javascriptSemanticProjection.ts and it is the right one.

The implementation spells it with key || "*" and key || "default", so the empty string takes the same branch as an unreadable key. But "" is a legal, exactly known property key: the callee semanticStaticPropertyKey distinguishes null ("cannot read this key") from "" ("the key is exactly the empty string"), and the sibling empty-key handling for request field names in javascriptSemanticAnalysis.empty_keys.test.ts already retains "" as an exact name.

Two consequences, both wrong:

exports[''] = 1 is reported as the wildcard *, i.e. an unknowable export name, when the name is exactly known.
module.exports[''] = 2 is reported as default, claiming a default export the module does not have — the exact outcome the comment two lines above says must never happen.

Problem and expected behavior

For exports[''] = 1; module.exports[''] = 2; exports['ok'] = 3; exports[key] = 4;, the produced moduleLinks exported names are:

current:  [ "*", "default", "ok", "*" ]
expected: [ "",  "",        "ok", "*" ]

The last entry is the genuine unknown-key case and keeps the wildcard. The first two are statically known names and must be reported verbatim.

Change and scope

commonJsExportName tests the key for null alone in both branches, so "" falls through to the exact key. key === null already covers every unknowable case, so no behaviour is lost: the wildcard for a dynamic key, and default for an exports-shaped member that is not a property access, are both unchanged. No new helper, no alias, no second representation.

Contract and boundary impact

Semantic owner and earliest changed stage: the projected CommonJS export name during JavaScript semantic analysis, at the point the AST node is classified. The parser, the module graph, the bundler-edge projection and every other producer are untouched.
CLI and MCP/tool-catalog contract: no tool name, command, input field, output schema or error code changes. A module link for an empty export name gains the exact name it always had in the source instead of a fabricated one.
Provider, target-format or platform compatibility: none — platform-independent AST projection.
Evidence, artifact, provenance or reconstruction contract: one moduleLinks entry per assignment is unchanged in count and identity; only its exportedName becomes truthful. No Evidence value is added or removed.
Process execution, authorization, cleanup or containment impact: none.
Generated metadata, package or installation impact: none.

Evidence and regression coverage

A new case in javascriptSemanticAnalysis.empty_keys.test.ts, "retains the exact empty CommonJS export name", asserts the four exported names above. It sits in the file that already pins "" as an exact name for request fields, so the two empty-key owners are covered together.

I re-ran it myself on macOS 26.5.1 arm64 with Node.js 24.18.0 and npm 11.16.0, based on bb54082:

npm run test:focused -- src/domain/javascript/javascriptSemanticAnalysis.empty_keys.test.ts: 10 passed with the change.
Reverting only javascriptSemanticProjection.ts to bb54082 fails exactly the new case with AssertionError: expected [ '*', 'default', 'ok', '*' ] to deeply equal [ '', '', 'ok', '*' ], 1 failed and 9 passed.

Unavailable coverage: no Windows or Linux run and no real bundler output — the regression input is a source string parsed by the repository's own analyzer. The change does not touch platform-conditional 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/2709 **Author:** [@XiaoBinGan](https://github.com/XiaoBinGan) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/javascript-empty-export-name` --- ### 📝 Commits (10+) - [`c49c88c`](https://github.com/morluto/rea/commit/c49c88c317d09d4b9f5a473fcc3dadf04b08e9bb) fix(javascript): keep the exact empty CommonJS export name - [`ea7e7dd`](https://github.com/morluto/rea/commit/ea7e7ddfa8f230d23f7eb7939cd38792a8705b35) fix(javascript): accept an exact empty export name across the shape contract - [`1385e5e`](https://github.com/morluto/rea/commit/1385e5e6fa8911f0a954d11bab4ef88d24d5341f) fix(process): report no remaining time after a provider deadline fires - [`70f1fe2`](https://github.com/morluto/rea/commit/70f1fe20585496373942e0f9fbd399becbe8c6ed) Merge captured main into PR #2709 - [`95099f4`](https://github.com/morluto/rea/commit/95099f44529985be0e3af86f9f1f04b0c7153084) Merge remote-tracking branch 'origin/main' into fix/javascript-empty-export-name - [`505d10f`](https://github.com/morluto/rea/commit/505d10fef0b57bedda160c89bcf2827f61c649f1) Merge remote-tracking branch 'origin/main' into fix/javascript-empty-export-name - [`90f4c39`](https://github.com/morluto/rea/commit/90f4c397dcd08d084360429172dac3bc9ad7859e) Merge remote-tracking branch 'origin/main' into fix/javascript-empty-export-name - [`cfbb486`](https://github.com/morluto/rea/commit/cfbb48687bfc6692c9d20f2e36f5f3c77f064379) Merge branch 'main' into fix/javascript-empty-export-name - [`289573a`](https://github.com/morluto/rea/commit/289573a60f1d996a5fa51fc74074930bf7eccf73) Merge remote-tracking branch 'origin/main' into fix/javascript-empty-export-name - [`44a1767`](https://github.com/morluto/rea/commit/44a1767d78281a51306b13985d80a106580d3eaa) Merge remote-tracking branch 'origin/main' into fix/javascript-empty-export-name ### 📊 Changes **4 files changed** (+35 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptExportShapeComparisonSchemas.test.ts` (+23 -0) 📝 `src/domain/javascript/javascriptExportShapeComparisonSchemas.ts` (+5 -0) 📝 `src/domain/javascript/javascriptSemanticAnalysis.empty_keys.test.ts` (+2 -0) 📝 `src/domain/javascript/javascriptSemanticProjection.ts` (+5 -2) </details> ### 📄 Description `semanticStaticPropertyKey` deliberately preserves the empty string as an exact property key, but `commonJsExportName` treats that same value as a missing key. ## Summary `exports[key] = …` / `module.exports[key] = …` with a key that is not statically readable assigns an export whose name is unknown, and the projection reports the wildcard `*` rather than the variable name, so it never claims a real export. That intent is stated in the comment at [`javascriptSemanticProjection.ts`](https://github.com/morluto/rea/blob/bb54082/src/domain/javascript/javascriptSemanticProjection.ts) and it is the right one. The implementation spells it with `key || "*"` and `key || "default"`, so the *empty string* takes the same branch as an unreadable key. But `""` is a legal, exactly known property key: the callee `semanticStaticPropertyKey` distinguishes `null` ("cannot read this key") from `""` ("the key is exactly the empty string"), and the sibling empty-key handling for request field names in `javascriptSemanticAnalysis.empty_keys.test.ts` already retains `""` as an exact name. Two consequences, both wrong: - `exports[''] = 1` is reported as the wildcard `*`, i.e. an unknowable export name, when the name is exactly known. - `module.exports[''] = 2` is reported as `default`, claiming a default export the module does not have — the exact outcome the comment two lines above says must never happen. ## Problem and expected behavior For `exports[''] = 1; module.exports[''] = 2; exports['ok'] = 3; exports[key] = 4;`, the produced `moduleLinks` exported names are: ``` current: [ "*", "default", "ok", "*" ] expected: [ "", "", "ok", "*" ] ``` The last entry is the genuine unknown-key case and keeps the wildcard. The first two are statically known names and must be reported verbatim. ## Change and scope [`commonJsExportName`](https://github.com/XiaoBinGan/rea/blob/HEAD/src/domain/javascript/javascriptSemanticProjection.ts) tests the key for `null` alone in both branches, so `""` falls through to the exact key. `key === null` already covers every unknowable case, so no behaviour is lost: the wildcard for a dynamic key, and `default` for an `exports`-shaped member that is not a property access, are both unchanged. No new helper, no alias, no second representation. ## Contract and boundary impact - **Semantic owner and earliest changed stage:** the projected CommonJS export name during JavaScript semantic analysis, at the point the AST node is classified. The parser, the module graph, the bundler-edge projection and every other producer are untouched. - **CLI and MCP/tool-catalog contract:** no tool name, command, input field, output schema or error code changes. A module link for an empty export name gains the exact name it always had in the source instead of a fabricated one. - **Provider, target-format or platform compatibility:** none — platform-independent AST projection. - **Evidence, artifact, provenance or reconstruction contract:** one `moduleLinks` entry per assignment is unchanged in count and identity; only its `exportedName` becomes truthful. No Evidence value is added or removed. - **Process execution, authorization, cleanup or containment impact:** none. - **Generated metadata, package or installation impact:** none. ## Evidence and regression coverage A new case in [`javascriptSemanticAnalysis.empty_keys.test.ts`](https://github.com/XiaoBinGan/rea/blob/HEAD/src/domain/javascript/javascriptSemanticAnalysis.empty_keys.test.ts), "retains the exact empty CommonJS export name", asserts the four exported names above. It sits in the file that already pins `""` as an exact name for request fields, so the two empty-key owners are covered together. I re-ran it myself on macOS 26.5.1 arm64 with Node.js 24.18.0 and npm 11.16.0, based on `bb54082`: - `npm run test:focused -- src/domain/javascript/javascriptSemanticAnalysis.empty_keys.test.ts`: 10 passed with the change. - Reverting only `javascriptSemanticProjection.ts` to `bb54082` fails exactly the new case with `AssertionError: expected [ '*', 'default', 'ok', '*' ] to deeply equal [ '', '', 'ok', '*' ]`, 1 failed and 9 passed. Unavailable coverage: no Windows or Linux run and no real bundler output — the regression input is a source string parsed by the repository's own analyzer. The change does not touch platform-conditional 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:05:48 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[PR #5358] [CLOSED] fix(go): read metadata when PE virtual size is zero #6894

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#5333

No description provided.

Rows
Columns