From cb17828dd151d4fd01f760bbef7e25f54cbdc2fa Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 11 Nov 2026 18:23:15 +0800 Subject: [PATCH] fix(reference): retain proven loader calls and unresolved targets (#9920) * fix(reference): retain proven loader calls and unresolved targets * refactor(reference): remove unused ancestor lookup * fix(reference): drop loader calls that return a reassigned member An immediate call could still use a member's original require initializer because mutation facts were not built for a call-valued callee. A definite assignment that precedes the call now replaces that initializer. * fix(reference): select returned loaders from indexed read facts * fix(reference): refresh loader facts for wrapped returned members A sequence such as (0, box.load) still used the original require initializer. Follow sequence, conditional, and logical returns to the member before deciding whether a prior assignment replaced it. * test(reference): record unresolved CommonJS targets in source evidence unknown.cjs calls require with a non-literal target. The inventory now keeps that coverage limitation on the file entry, and the boundary fixture asserts it beside the resolved member relationships. * fix(reference): cover opaque apply lists and assigned returned members require.apply(null, args) has no normalized argument list, so the inventory now records an unresolved target instead of complete coverage. An assignment return such as f = box.load is followed to the member before a prior replacement is trusted. * fix(reference): keep self-defaulted module delegates loadable Timing a member read at a later call made `box.load = box.load || null` observe its own write and drop the native delegate. Reads that already fed an earlier assignment stay at the member. --------- Co-authored-by: Cursor Agent --- .../ReferenceSourceImportEntries.test.ts | 100 +++++++++++++++++- .../javascriptSemanticRequireCaptures.test.ts | 16 +++ .../javascriptSemanticRequireOrigin.ts | 46 +++++++- ...iptStaticAnalysisLoaderInvocations.test.ts | 38 +++++++ src/domain/referenceSourceImportParsing.ts | 24 ++++- src/domain/referenceSourcePathIdentity.ts | 13 --- src/domain/referenceSourceRequireBindings.ts | 37 +++---- .../referenceSourceComputedRequire.test.ts | 20 +++- .../referenceSourceLoaderConstructors.test.ts | 64 +++++++++++ 9 files changed, 317 insertions(+), 41 deletions(-) diff --git a/src/application/ReferenceSourceImportEntries.test.ts b/src/application/ReferenceSourceImportEntries.test.ts index 3bd85788f6..7351a1d72a 100644 --- a/src/application/ReferenceSourceImportEntries.test.ts +++ b/src/application/ReferenceSourceImportEntries.test.ts @@ -3,7 +3,10 @@ import { createHash } from "node:crypto"; import { expect, it } from "vitest"; import { parseReferenceSourceEntries } from "./ReferenceSourceImportEntries.js"; -import { historicalSourceLanguages } from "../domain/referenceSourceGraph.js"; +import { + historicalSourceInventoryHasPartialFacts, + historicalSourceLanguages, +} from "../domain/referenceSourceGraph.js"; import type { ReferenceSourceRead } from "../reference/ReferenceSourceReaderTypes.js"; it("projects an unsupported non-regular entry as unknown kind instead of file", () => { @@ -98,6 +101,101 @@ it("keeps source entries when a module has an empty specifier", () => { expect(parsed.limitations[0]).toMatch(/not recorded.*relationship/iu); }); +it("resolves immediately invoked loaders from their proven source bases", () => { + const files = [ + [ + "main.cjs", + '(() => require)()("./dep.cjs"); (() => () => {})()("./local.cjs");', + ], + [ + "main.mjs", + [ + 'import {createRequire} from "node:module";', + 'createRequire(new URL("./sub/base.cjs", import.meta.url))("./dep.cjs");', + 'createRequire(new URL("./sub/base.cjs", import.meta.url)).call(null, "./member.cjs");', + ].join("\n"), + ], + ["dep.cjs", ""], + ["sub/dep.cjs", ""], + ["member.cjs", ""], + ["sub/member.cjs", ""], + ] as const; + const parsed = parseReferenceSourceEntries( + { + root: "/selected/source", + entries: files.map(([path, content]) => ({ + status: "read", + kind: "file", + path, + bytes: Buffer.from(content), + size: Buffer.byteLength(content), + })), + bytesRead: files.reduce( + (sum, [, content]) => sum + Buffer.byteLength(content), + 0, + ), + limitations: [], + }, + [], + ); + expect( + parsed.relationships.filter(({ kind }) => kind === "requires"), + ).toEqual([ + { + from_path: "main.cjs", + to: "dep.cjs", + kind: "requires", + resolution: "internal", + parse_state: "parsed", + }, + { + from_path: "main.mjs", + to: "sub/dep.cjs", + kind: "requires", + resolution: "internal", + parse_state: "parsed", + }, + ]); +}); + +it.each([ + ["function load(name) { require(name); }", true], + ["function load(args) { require.apply(null, args); }", true], + ["function load(require, name) { require(name); }", false], + ["require(7); require();", false], +] as const)( + "reports unresolved CommonJS target coverage for %s", + (source, partial) => { + const bytes = Buffer.from(source); + const parsed = parseReferenceSourceEntries( + { + root: "/selected/source", + entries: [ + { + status: "read", + kind: "file", + path: "main.cjs", + bytes, + size: bytes.length, + }, + ], + bytesRead: bytes.length, + limitations: [], + }, + [], + ); + expect(parsed.relationships).toEqual([]); + expect( + historicalSourceInventoryHasPartialFacts({ + ...parsed, + parse_failures: parsed.parseFailures, + exclusions: [], + }), + ).toBe(partial); + expect(parsed.entries[0]?.limitations).toEqual(parsed.limitations); + }, +); + it("retains Kotlin source identity and language across the inventory projection", () => { const files = [ [ diff --git a/src/domain/javascript/javascriptSemanticRequireCaptures.test.ts b/src/domain/javascript/javascriptSemanticRequireCaptures.test.ts index 0edc713ada..8fd55286bb 100644 --- a/src/domain/javascript/javascriptSemanticRequireCaptures.test.ts +++ b/src/domain/javascript/javascriptSemanticRequireCaptures.test.ts @@ -2,6 +2,22 @@ import { expect, it } from "vitest"; import { analyzeJavaScriptStaticSource } from "./javascriptStaticAnalysis.js"; +it("keeps a self-defaulted module delegate as a possible load", () => { + expect( + analyzeJavaScriptStaticSource( + `const box = { load: module.require }; +box.load = box.load || null; +box.load.call(module, "./dep.cjs");`, + ).references, + ).toEqual([ + expect.objectContaining({ + kind: "require", + specifier: null, + operation: "load", + }), + ]); +}); + it.each([ "load('./dep.cjs')", "load.call(null,'./dep.cjs')", diff --git a/src/domain/javascript/javascriptSemanticRequireOrigin.ts b/src/domain/javascript/javascriptSemanticRequireOrigin.ts index 104b7ca1fb..5a2dccebaf 100644 --- a/src/domain/javascript/javascriptSemanticRequireOrigin.ts +++ b/src/domain/javascript/javascriptSemanticRequireOrigin.ts @@ -424,7 +424,7 @@ const selectedRequireSourcesAt = ( if (t.isMemberExpression(node) || t.isOptionalMemberExpression(node)) { pending.push( ...inheritLoaderSourceContext(current, [ - loaderMemberSource(current, node), + loaderMemberSource(current, node, state.parentsByNode), ]), ); } else if (t.isIdentifier(node)) { @@ -591,6 +591,9 @@ const selectedRequireSourcesAt = ( returned.references.map((reference) => ({ node: reference.node, path: reference.projection, + // The immediate invocation produces this value before the + // receiving call can apply opaque effects to it. + readAt: node, })), ), ); @@ -1087,6 +1090,7 @@ const inheritLoaderSourceContext = ( const loaderMemberSource = ( current: ModuleObjectSource, member: t.MemberExpression | t.OptionalMemberExpression, + parents: WeakMap, ): ModuleObjectSource => ({ node: member.object, readAt: current.readAt ?? member, @@ -1094,7 +1098,17 @@ const loaderMemberSource = ( current.readAt === undefined ? semanticShiftCapturedSelections(current.capturedSelections, 1) : semanticNormalizeCapturedSelections([ - { at: member, depth: 1 }, + { + // A later consumer must not time a read that already fed an + // earlier assignment. `box.load = box.load || null` reads + // `box.load` before that write. + at: + current.path.length === 0 && + !memberFeedsEarlierAssignment(member, current.readAt, parents) + ? current.readAt + : member, + depth: 1, + }, ...(semanticShiftCapturedSelections(current.capturedSelections, 1) ?? []), ]), @@ -1104,6 +1118,30 @@ const loaderMemberSource = ( ], }); +/** True when this member is an input of an assignment that already ran. */ +const memberFeedsEarlierAssignment = ( + member: t.Node, + readAt: t.Node, + parents: WeakMap, +): boolean => { + let assignment: t.AssignmentExpression | undefined; + let parent = parents.get(member); + while (parent !== undefined && parent !== readAt) { + if (t.isAssignmentExpression(parent)) { + let cursor: t.Node | undefined = member; + while (cursor !== undefined && cursor !== parent) { + const next = parents.get(cursor); + if (next === parent && parent.right === cursor) assignment = parent; + cursor = next; + } + } + // An assignment inside a function runs with that function, not before it. + if (t.isFunction(parent)) return false; + parent = parents.get(parent); + } + return assignment !== undefined; +}; + const projectedLoaderSources = ( current: ModuleObjectSource, sources: readonly ModuleObjectSource[], @@ -1282,7 +1320,9 @@ const hasCommonJsModuleSource = ( continue; } if (t.isMemberExpression(value) || t.isOptionalMemberExpression(value)) { - enqueue(loaderMemberSource({ ...next, path }, value)); + enqueue( + loaderMemberSource({ ...next, path }, value, state.parentsByNode), + ); continue; } if ( diff --git a/src/domain/javascript/javascriptStaticAnalysisLoaderInvocations.test.ts b/src/domain/javascript/javascriptStaticAnalysisLoaderInvocations.test.ts index 8fc8933cc2..b0fdc0da3c 100644 --- a/src/domain/javascript/javascriptStaticAnalysisLoaderInvocations.test.ts +++ b/src/domain/javascript/javascriptStaticAnalysisLoaderInvocations.test.ts @@ -95,6 +95,44 @@ it.each([ }, ); +it("does not keep a dependency when an immediate call returns a replaced member", () => { + const callsFor = (source: string) => { + const parsed = parseJavaScriptSource(source, "entry.cjs"); + if (parsed === null) throw new Error("Expected CommonJS fixture"); + return [...referenceSourceRequireCalls(parsed, "entry.cjs").values()].map( + ({ resolutionBase }) => resolutionBase, + ); + }; + expect( + callsFor( + 'const box = { load: require }; box.load = () => {}; (() => box.load)()("./dep.cjs");', + ), + ).toEqual([]); + expect( + callsFor('const box = { load: require }; (() => box.load)()("./dep.cjs");'), + ).toEqual(["source-file"]); + expect( + callsFor( + 'const box = { load: require }; box.load = () => {}; (() => (0, box.load))()("./dep.cjs");', + ), + ).toEqual([]); + expect( + callsFor( + 'const box = { load: require }; (() => (0, box.load))()("./dep.cjs");', + ), + ).toEqual(["source-file"]); + expect( + callsFor( + 'let f; const box = { load: require }; box.load = () => {}; (() => (f = box.load))()("./dep.cjs");', + ), + ).toEqual([]); + expect( + callsFor( + 'let f; const box = { load: require }; (() => (f = box.load))()("./dep.cjs");', + ), + ).toEqual(["source-file"]); +}); + const imported = "import {createRequire as make} from 'node:module';"; it.each([ ["call", 'null,"./valid.cjs"', 'null,"./invented.cjs"'], diff --git a/src/domain/referenceSourceImportParsing.ts b/src/domain/referenceSourceImportParsing.ts index caf218003b..84a16c6117 100644 --- a/src/domain/referenceSourceImportParsing.ts +++ b/src/domain/referenceSourceImportParsing.ts @@ -157,6 +157,7 @@ const extractImports = ( ReferenceSourceRequireCall >, ): void => { + let unresolvedRequireTarget = false; traverseJavaScriptAst(ast.program, { enter: (node) => { if ( @@ -197,8 +198,21 @@ const extractImports = ( ) { const requireCall = requireCalls.get(node); if (requireCall === undefined) return; - const specifier = exactStringModuleName(requireCall.arguments?.[0]); - if (specifier === undefined) return; + // Undefined arguments are an opaque normalized list, such as + // `require.apply(null, args)`. An empty list is an explicit call + // with no target. + if (requireCall.arguments === undefined) { + unresolvedRequireTarget = true; + return; + } + const target = requireCall.arguments[0]; + if (target === undefined) return; + const literal = readExactJavaScriptLiteral(target); + if (!literal.found) { + unresolvedRequireTarget = true; + return; + } + if (typeof literal.value !== "string") return; if ( requireCall.limitation !== undefined && !limitations.includes(requireCall.limitation) @@ -207,7 +221,7 @@ const extractImports = ( const uncertainResolution = requireCall.resolutionBase === "unknown"; appendRelationship(relationships, limitations, { fromPath: from_path, - to: specifier, + to: literal.value, kind: "requires", parseState: uncertainResolution ? "partial" : "parsed", ...(uncertainResolution ? { resolution: "unknown" } : {}), @@ -221,6 +235,10 @@ const extractImports = ( } }, }); + if (unresolvedRequireTarget) + limitations.push( + `A CommonJS module target in ${from_path} could not be determined statically.`, + ); }; const parseWithBabel = ( diff --git a/src/domain/referenceSourcePathIdentity.ts b/src/domain/referenceSourcePathIdentity.ts index d6473cff87..0946d0e30d 100644 --- a/src/domain/referenceSourcePathIdentity.ts +++ b/src/domain/referenceSourcePathIdentity.ts @@ -6,8 +6,6 @@ export interface ReferenceSourcePathLookup { readonly has: (path: string) => boolean; readonly resolve: (path: string) => string | undefined; readonly resolveCandidates: (paths: readonly string[]) => string | undefined; - /** Resolve a path to the closest recorded parent identity. */ - readonly resolveAncestor: (path: string) => string | undefined; } /** @@ -35,17 +33,6 @@ export const createReferenceSourcePathLookup = ( resolveCandidates: (candidates) => candidates.find((path) => exact.has(path)) ?? candidates.map(equivalent).find((path) => path !== undefined), - resolveAncestor: (path) => { - for ( - let candidate = path; - candidate !== "." && candidate !== "/"; - candidate = posix.dirname(candidate) - ) { - const found = resolve(candidate); - if (found !== undefined) return found; - } - return undefined; - }, }; }; diff --git a/src/domain/referenceSourceRequireBindings.ts b/src/domain/referenceSourceRequireBindings.ts index 0151475a4f..323dbd014a 100644 --- a/src/domain/referenceSourceRequireBindings.ts +++ b/src/domain/referenceSourceRequireBindings.ts @@ -75,9 +75,16 @@ export const referenceSourceRequireCalls = ( !t.isNewExpression(node) ) return; - if (callReceiverIdentifier(node.callee) === undefined) return; - let fact = semanticRequireLoaderAt(node, index.bindings, factoryBindings); const callee = unwrapJavaScriptExpression(node.callee).node; + if ( + !t.isCallExpression(callee) && + !t.isOptionalCallExpression(callee) && + !hasIdentifierReceiver(callee) + ) + return; + if (t.isCallExpression(callee) || t.isOptionalCallExpression(callee)) + completeSemanticSteps(index.indexMemberMutationFactsSteps()); + let fact = semanticRequireLoaderAt(node, index.bindings, factoryBindings); if ( fact === "create-require" || ((t.isMemberExpression(callee) || @@ -86,8 +93,6 @@ export const referenceSourceRequireCalls = ( semanticStaticPropertyKey(callee.property, callee.computed) ?? "", )) ) { - // Definitions identify a possible factory. Its mutable member reads - // need the shared write index before they can prove an association. completeSemanticSteps(index.indexMemberMutationFactsSteps()); fact = semanticRequireLoaderAt(node, index.bindings, factoryBindings); } @@ -153,12 +158,7 @@ export const referenceSourceRequireCalls = ( }); }, }); - if ( - candidates.some( - ({ call }) => - !t.isIdentifier(unwrapJavaScriptExpression(call.callee).node), - ) - ) + if (candidates.some(({ call }) => hasMemberCallee(call))) completeSemanticSteps(index.indexMemberMutationFactsSteps()); return new Map( candidates.map( @@ -169,8 +169,7 @@ export const referenceSourceRequireCalls = ( arguments: callArguments, ...(limitation === undefined ? {} : { limitation }), resolutionBase: - !t.isIdentifier(unwrapJavaScriptExpression(call.callee).node) && - hasPriorMemberMutation(index, call) + hasMemberCallee(call) && hasPriorMemberMutation(index, call) ? "unknown" : resolutionBase, }, @@ -220,15 +219,17 @@ const hasPriorMemberMutation = ( ); }; -const callReceiverIdentifier = ( - callee: t.Node | null | undefined, -): t.Identifier | undefined => { - if (!t.isNode(callee)) return undefined; - let expression = unwrapJavaScriptExpression(callee).node; +const hasMemberCallee = (call: ReferenceSourceRequireInvocation): boolean => { + const callee = unwrapJavaScriptExpression(call.callee).node; + return t.isMemberExpression(callee) || t.isOptionalMemberExpression(callee); +}; + +const hasIdentifierReceiver = (callee: t.Node): boolean => { + let expression = callee; while ( t.isMemberExpression(expression) || t.isOptionalMemberExpression(expression) ) expression = unwrapJavaScriptExpression(expression.object).node; - return t.isIdentifier(expression) ? expression : undefined; + return t.isIdentifier(expression); }; diff --git a/tests/boundary/filesystem/referenceSourceComputedRequire.test.ts b/tests/boundary/filesystem/referenceSourceComputedRequire.test.ts index 87ebb5d434..931ba7ceef 100644 --- a/tests/boundary/filesystem/referenceSourceComputedRequire.test.ts +++ b/tests/boundary/filesystem/referenceSourceComputedRequire.test.ts @@ -9,7 +9,11 @@ import { expect, it } from "vitest"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { importReferenceSource } from "../../support/referenceSourceResourceScope.js"; -const hashedSourceEntry = (path: string, source: string) => ({ +const hashedSourceEntry = ( + path: string, + source: string, + limitations: readonly string[] = [], +) => ({ path, kind: "file", sha256: createHash("sha256").update(source).digest("hex"), @@ -17,7 +21,7 @@ const hashedSourceEntry = (path: string, source: string) => ({ language: "JavaScript", classifications: ["source"], content_state: "hashed", - limitations: [], + limitations: [...limitations], }); const memberFixtures = [ @@ -173,5 +177,15 @@ it("distinguishes computed require member values from literal members in source ]); expect(graph.entries).toHaveLength(Object.keys(sources).length); for (const [path, source] of Object.entries(sources)) - expect(graph.entries).toContainEqual(hashedSourceEntry(path, source)); + expect(graph.entries).toContainEqual( + hashedSourceEntry( + path, + source, + path === "unknown.cjs" + ? [ + "A CommonJS module target in unknown.cjs could not be determined statically.", + ] + : [], + ), + ); }); diff --git a/tests/boundary/filesystem/referenceSourceLoaderConstructors.test.ts b/tests/boundary/filesystem/referenceSourceLoaderConstructors.test.ts index c8c49609af..7b454d03d7 100644 --- a/tests/boundary/filesystem/referenceSourceLoaderConstructors.test.ts +++ b/tests/boundary/filesystem/referenceSourceLoaderConstructors.test.ts @@ -118,3 +118,67 @@ it("retains constructor loader requests without inventing host authority", async ), ).toBe(false); }); + +it("indexes returned loader reads before publishing dependency edges", async () => { + const root = await createTestTempDirectory("rea-returned-loader-reads-"); + const sources = { + "unchanged.cjs": 'const box={load:require};(()=>box.load)()("./dep.cjs");', + "child-only.cjs": + 'const box={load:require};box.load.extra=1;(()=>box.load)()("./dep.cjs");', + "captured-before.cjs": + 'const box={load:require};const load=box.load;box.load=()=>{};(()=>load)()("./dep.cjs");', + "nested-unchanged.cjs": + 'const box={api:{load:require}};(()=>box.api.load)()("./dep.cjs");', + "overwritten.cjs": + 'const box={load:require};box.load=()=>{};(()=>box.load)()("./dep.cjs");', + "conditional.cjs": + 'const box={load:require};box.load=()=>{};(()=>(true?box.load:box.load))()("./dep.cjs");', + "sequence.cjs": + 'const box={load:require};box.load=()=>{};(()=>(0,box.load))()("./dep.cjs");', + "deleted.cjs": + 'const box={load:require};delete box.load;(()=>box.load)()("./dep.cjs");', + "nested-replaced.cjs": + 'const box={api:{load:require}};box.api.load=()=>{};(()=>box.api.load)()("./dep.cjs");', + "own-write.cjs": + 'const box={load:require};(()=>(box.load=()=>{},box.load))()("./dep.cjs");', + "local.cjs": 'const box={load:()=>{}};(()=>box.load)()("./dep.cjs");', + "direct-overwritten.cjs": + 'const box={load:require};box.load=()=>{};box.load("./dep.cjs");', + "dep.cjs": "module.exports={};", + }; + await Promise.all( + Object.entries(sources).map(([name, source]) => + writeFile(join(root, name), source), + ), + ); + const result = await importReferenceSource({ + root, + caller: "returned-loader-reads", + policy: { secretPatterns: [] }, + }); + if (!result.ok) throw result.error; + expect(result.value.parse_failures).toEqual([]); + expect(result.value.relationships).toEqual([ + ...["captured-before.cjs", "child-only.cjs"].map((from_path) => ({ + from_path, + to: "dep.cjs", + kind: "requires", + resolution: "internal", + parse_state: "parsed", + })), + { + from_path: "direct-overwritten.cjs", + to: "./dep.cjs", + kind: "requires", + resolution: "unknown", + parse_state: "partial", + }, + ...["nested-unchanged.cjs", "unchanged.cjs"].map((from_path) => ({ + from_path, + to: "dep.cjs", + kind: "requires", + resolution: "internal", + parse_state: "parsed", + })), + ]); +});