morluto/rea

Fork 0

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

[PR #2921] [CLOSED] fix(javascript): ignore a require call that a local binding shadows #5494

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/shadowed-require

📝 Commits (3)

9327cdf fix(javascript): ignore a require call a local binding shadows
89fd409 fix(javascript): keep member and createRequire loader calls
9ed55f6 fix(javascript): retain proven CommonJS require loaders

📊 Changes

2 files changed (+173 additions, -2 deletions)

View changed files

➕ src/domain/javascript/javascriptStaticAnalysis.shadowed_require.test.ts (+79 -0)
📝 src/domain/javascript/javascriptStaticAnalysisCalls.ts (+94 -2)

📄 Description

Summary

Only treat a call named require as the CommonJS loader when the callee is an
unshadowed reference to it, so a locally bound require no longer invents an
imports edge.

Problem and expected behavior

analyze-javascript-application reports a resolved, high-confidence imports
relationship for a call to a local function parameter named require.
Lexical binding already establishes that the call is not the CommonJS loader, so
the graph must not carry that edge.

Minimal reproduction, as reported:

function run(require) {
  return require("./ghost.cjs");
}
console.log(run(() => "local"));

src/domain/javascript/javascriptStaticAnalysisCalls.ts decided this by name
alone:

if (name === "require" || name.endsWith(".require"))
  return { specifier: argumentValue(node.arguments[0]) };

Change and scope

requireCallSpecifier now receives whether the callee is an unshadowed loader
reference, computed with isUnmodifiedGlobalReference — the helper this file
already uses for worker constructors, and the same ownership rule the semantic
path applies in javascriptSemanticRequireOrigin.ts, whose contract is to
recover an unshadowed literal require origin.

Two lanes deliberately keep their behavior:

name.endsWith(".require") (member calls such as module.require) is a
property access, not a lexical binding.
moduleRequireName is the module factory's own require parameter
(javascriptStaticAnalysisBundler.ts), which is a real binding. A plain
CommonJS module's ambient require has no binding, so ordinary calls are
still detected.

Contract and boundary impact

The producing boundary keeps ownership of the decision. The semantic path
already refused a shadowed require, so both paths now agree instead of one of
them re-deriving the call from its name.

Evidence and regression coverage

src/domain/javascript/javascriptStaticAnalysis.shadowed_require.test.ts
reproduces the reported source and covers both directions:

Without the change the shadowed call is recorded (the ghost specifier is
present in the references); with it, it is absent.
const loaded = require("./real.cjs") is still recorded, so no legitimate
loader call is lost.

npm run test:focused -- src/domain/javascript/*.test.ts passes: 57 files,
1283 tests. npm run check:fast passes.

Unavailable coverage

The end-to-end analyze-javascript-application edge was not re-run as a
boundary test; the evidence is at the reference boundary that produces it
(JavaScriptModuleRelationships.ts selects references whose kind is require).
No real-provider verification is involved.

_{🔄 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/2921 **Author:** [@XiaoBinGan](https://github.com/XiaoBinGan) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/shadowed-require` --- ### 📝 Commits (3) - [`9327cdf`](https://github.com/morluto/rea/commit/9327cdf39f972f57bee12883944d9872c60cc8ca) fix(javascript): ignore a require call a local binding shadows - [`89fd409`](https://github.com/morluto/rea/commit/89fd4097c53ca7fb40f20dada5e6101d47b55856) fix(javascript): keep member and createRequire loader calls - [`9ed55f6`](https://github.com/morluto/rea/commit/9ed55f69cd4eb48e762e4ed8e6a545be9ea17ad1) fix(javascript): retain proven CommonJS require loaders ### 📊 Changes **2 files changed** (+173 additions, -2 deletions) <details> <summary>View changed files</summary> ➕ `src/domain/javascript/javascriptStaticAnalysis.shadowed_require.test.ts` (+79 -0) 📝 `src/domain/javascript/javascriptStaticAnalysisCalls.ts` (+94 -2) </details> ### 📄 Description ## Summary Only treat a call named `require` as the CommonJS loader when the callee is an unshadowed reference to it, so a locally bound `require` no longer invents an `imports` edge. ## Problem and expected behavior `analyze-javascript-application` reports a resolved, high-confidence `imports` relationship for a call to a **local function parameter** named `require`. Lexical binding already establishes that the call is not the CommonJS loader, so the graph must not carry that edge. Minimal reproduction, as reported: ```js function run(require) { return require("./ghost.cjs"); } console.log(run(() => "local")); ``` `src/domain/javascript/javascriptStaticAnalysisCalls.ts` decided this by name alone: ```ts if (name === "require" || name.endsWith(".require")) return { specifier: argumentValue(node.arguments[0]) }; ``` ## Change and scope `requireCallSpecifier` now receives whether the callee is an unshadowed loader reference, computed with `isUnmodifiedGlobalReference` — the helper this file already uses for worker constructors, and the same ownership rule the semantic path applies in `javascriptSemanticRequireOrigin.ts`, whose contract is to recover an *unshadowed* literal require origin. Two lanes deliberately keep their behavior: - `name.endsWith(".require")` (member calls such as `module.require`) is a property access, not a lexical binding. - `moduleRequireName` is the module factory's own require parameter (`javascriptStaticAnalysisBundler.ts`), which is a real binding. A plain CommonJS module's ambient `require` has no binding, so ordinary calls are still detected. ## Contract and boundary impact The producing boundary keeps ownership of the decision. The semantic path already refused a shadowed `require`, so both paths now agree instead of one of them re-deriving the call from its name. ## Evidence and regression coverage `src/domain/javascript/javascriptStaticAnalysis.shadowed_require.test.ts` reproduces the reported source and covers both directions: - Without the change the shadowed call is recorded (the `ghost` specifier is present in the references); with it, it is absent. - `const loaded = require("./real.cjs")` is still recorded, so no legitimate loader call is lost. `npm run test:focused -- src/domain/javascript/*.test.ts` passes: 57 files, 1283 tests. `npm run check:fast` passes. ## Unavailable coverage The end-to-end `analyze-javascript-application` edge was not re-run as a boundary test; the evidence is at the reference boundary that produces it (`JavaScriptModuleRelationships.ts` selects references whose kind is `require`). No real-provider verification is involved. --- <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:04 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns