From 9d68aac76db76bad44f6b17ada7c4464a871a331 Mon Sep 17 00:00:00 2001 From: Kaoru0822 Date: Fri, 30 Oct 2026 01:01:21 -0700 Subject: [PATCH] fix(javascript): stop reporting self-defaulted receivers as module loads (#8109) The CommonJS loader search treated any repeated state on its own ancestor chain as an unknown loader. A self-default such as `session.messages = session.messages || []` forms that cycle, because a member read inside the assigned value was attributed to the later call's read site and selected the same write again. Each `push` on the value was then reported as an unresolved `require` load, and graph coverage became partial. Record a member read inside a flowed value as a depth-1 captured selection at its own position, as member mutations already do for flowed origins. Treat a revisited state as adding no source; the cycle's other entries are still expanded once each. Remove the superseded ancestor tracking. Fixes #8102 Co-authored-by: Claude Opus 5.5 --- .../javascriptSemanticRequireOrigin.ts | 27 +++--- .../javascriptSelfDefaultReceivers.test.ts | 97 +++++++++++++++++++ 2 files changed, 110 insertions(+), 14 deletions(-) create mode 100644 tests/acceptance/applications/javascriptSelfDefaultReceivers.test.ts diff --git a/src/domain/javascript/javascriptSemanticRequireOrigin.ts b/src/domain/javascript/javascriptSemanticRequireOrigin.ts index 426caf95f3..1e924ab607 100644 --- a/src/domain/javascript/javascriptSemanticRequireOrigin.ts +++ b/src/domain/javascript/javascriptSemanticRequireOrigin.ts @@ -970,16 +970,22 @@ const inheritLoaderSourceContext = ( ), })); +// A later read site selects only the remaining path. This member selects its +// own key when it is evaluated, so `o.k = o.k || d` reads o.k before the write. const loaderMemberSource = ( current: ModuleObjectSource, member: t.MemberExpression | t.OptionalMemberExpression, ): ModuleObjectSource => ({ node: member.object, readAt: current.readAt ?? member, - capturedSelections: semanticShiftCapturedSelections( - current.capturedSelections, - 1, - ), + capturedSelections: + current.readAt === undefined + ? semanticShiftCapturedSelections(current.capturedSelections, 1) + : semanticNormalizeCapturedSelections([ + { at: member, depth: 1 }, + ...(semanticShiftCapturedSelections(current.capturedSelections, 1) ?? + []), + ]), path: [ semanticStaticPropertyKey(member.property, member.computed), ...current.path, @@ -1056,13 +1062,10 @@ const hasCommonJsModuleSource = ( readAt?: t.Node, ): boolean => { type Work = ModuleObjectSource & { - readonly ancestors: ReadonlySet; /** Await occurs after this many requested outer property selections. */ readonly awaitDepths: readonly number[]; }; - const pending: Work[] = [ - { node, path: [], readAt, ancestors: new Set(), awaitDepths: [] }, - ]; + const pending: Work[] = [{ node, path: [], readAt, awaitDepths: [] }]; // Keep depths separate and join finite key alternatives at each source read. const selections = new SemanticAliasSelections(() => { state.valueResourceLimits.add("alias-selections"); @@ -1108,13 +1111,10 @@ const hasCommonJsModuleSource = ( identity = {}; context.identities.set(pathKey, identity); } - if (next.ancestors.has(identity)) { - if (includeUnknown) return true; - continue; - } + // A revisited state adds no source. A cycle's other entries are expanded + // once each, so a cycle is not an unknown loader. if (examined.has(identity)) continue; examined.add(identity); - const ancestors = new Set([...next.ancestors, identity]); const enqueueAt = ( depths: readonly number[], ...sources: ModuleObjectSource[] @@ -1122,7 +1122,6 @@ const hasCommonJsModuleSource = ( pending.push( ...inheritLoaderSourceContext(next, sources).map((source) => ({ ...source, - ancestors, awaitDepths: depths, })), ); diff --git a/tests/acceptance/applications/javascriptSelfDefaultReceivers.test.ts b/tests/acceptance/applications/javascriptSelfDefaultReceivers.test.ts new file mode 100644 index 0000000000..8ff71baa47 --- /dev/null +++ b/tests/acceptance/applications/javascriptSelfDefaultReceivers.test.ts @@ -0,0 +1,97 @@ +import { expect } from "vitest"; + +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; +import { + analyzeJavaScriptDirectory, + runNodeJsonControl, +} from "../../support/cli/javascriptApplication.js"; +import { writeFixtureFiles } from "../../support/javascriptApplicationFixture.js"; + +const requireEdges = ( + graph: Awaited>["graph"], +) => + graph.edges.flatMap(({ relation, properties }) => + relation === "imports" && properties.kind === "require" + ? [ + { + specifier: properties.specifier, + expression: properties.expression, + resolved_path: properties.resolved_path, + }, + ] + : [], + ); + +cliTest.for(["CLI", "stdio MCP"] as const)( + "keeps self-defaulted receivers out of the module loader graph through %s", + async (surface, { cli }) => { + const members = await createTestTempDirectory("rea-self-default-members-"); + await writeFixtureFiles(members, { + "main.cjs": `function message(session, text) { + session.messages = session.messages || []; + session.messages.push(text); +} +const session = {}; +message(session, "first"); +message(session, "second"); +const local = {}; +local.items = local.items || []; +local.items.push("item"); +function trimmed(value) { + value = value || " fallback "; + return value.trim(); +} +let load = require; +load = load || require; +console.log(JSON.stringify([session.messages, local.items, trimmed(), load("./dep.cjs")]));`, + "dep.cjs": 'module.exports = "dep";', + }); + expect(await runNodeJsonControl(members, "main.cjs")).toEqual([ + ["first", "second"], + ["item"], + "fallback", + "dep", + ]); + const memberResult = await analyzeJavaScriptDirectory( + surface, + cli, + members, + ); + expect(requireEdges(memberResult.graph)).toEqual([ + { specifier: "./dep.cjs", expression: null, resolved_path: "dep.cjs" }, + ]); + expect(memberResult.graph.coverage.status).toBe("complete"); + + // A binding read in its own initializer is not a loader, while a native + // delegate that survives its self-default remains a possible load. + const bindings = await createTestTempDirectory( + "rea-self-default-bindings-", + ); + await writeFixtureFiles(bindings, { + "queue.cjs": `var queue = queue || []; +queue.push(["track", "load"]); +console.log(JSON.stringify(queue));`, + "delegate.cjs": `const box = { load: module.require }; +box.load = box.load || null; +console.log(JSON.stringify(box.load.call(module, "./dep.cjs")));`, + "dep.cjs": 'module.exports = "dep";', + }); + expect(await runNodeJsonControl(bindings, "queue.cjs")).toEqual([ + ["track", "load"], + ]); + expect(await runNodeJsonControl(bindings, "delegate.cjs")).toBe("dep"); + const bindingResult = await analyzeJavaScriptDirectory( + surface, + cli, + bindings, + ); + expect(requireEdges(bindingResult.graph)).toEqual([ + { + specifier: null, + expression: 'box.load.call(module, "./dep.cjs")', + resolved_path: null, + }, + ]); + }, +);