mirror of
https://github.com/morluto/rea
synced 2026-10-25 22:08:11 -04:00
fix(javascript): omit self-referential static references with disclosure (#1157)
* fix(javascript): omit self-referential static references with disclosure
A bundled module that requires its own key (`r(50)` inside webpack
module 50) resolved, through the static-reference composer, to its own
module node. The resulting self-referential imports edge failed result
validation and lost the entire analysis ("Result schema rejected 1
issue ... (custom)"). This is the same invariant as #1146; #1152 fixed
the bundler async-chunk composer, and this is the remaining adjacent
composer.
Skip a reference whose resolved target is its own source, count it, and
disclose the omission the way #1152 discloses async-chunk references.
Sibling requires are kept.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* refactor(javascript): share the self-reference omission disclosure
The self-import (#1130), async-chunk (#1152) and static-reference
disclosures repeated one sentence template in three composers, and the
new test repeated the analyze-and-check sequence of its neighbour. That
put jscpd at its 2.5% threshold. Share the template through
selfReferenceOmissions with identical wording, and share one test helper
for the graph-wide self-reference invariants.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -7,7 +7,10 @@ import {
|
||||
import type { JavaScriptModuleArtifactAnalysis } from "./JavaScriptArtifactAnalysisTypes.js";
|
||||
import type { JavaScriptArtifactFileSet } from "../../domain/javascript/javascriptArtifactFiles.js";
|
||||
import { JavaScriptArtifactGraphAccumulator } from "./JavaScriptArtifactGraphAccumulator.js";
|
||||
import type { JavaScriptArtifactGraphContext } from "./JavaScriptArtifactGraphContext.js";
|
||||
import {
|
||||
selfReferenceOmissions,
|
||||
type JavaScriptArtifactGraphContext,
|
||||
} from "./JavaScriptArtifactGraphContext.js";
|
||||
import {
|
||||
addJavaScriptHtmlRoles,
|
||||
addJavaScriptSourceMapOriginals,
|
||||
@@ -90,7 +93,7 @@ const buildJavaScriptArtifactGraphInput = (
|
||||
addJavaScriptSourceModules(context);
|
||||
const bundlerLimitations = addJavaScriptBundlerNodes(context);
|
||||
const relationshipOmissions = addJavaScriptModuleRelationships(context);
|
||||
addJavaScriptStaticFindings(context);
|
||||
const findingLimitations = addJavaScriptStaticFindings(context);
|
||||
addElectronBoundaries(context);
|
||||
addJavaScriptHtmlRoles(context);
|
||||
addJavaScriptSourceMapOriginals(context);
|
||||
@@ -106,6 +109,7 @@ const buildJavaScriptArtifactGraphInput = (
|
||||
coverage,
|
||||
limitations: [
|
||||
...bundlerLimitations,
|
||||
...findingLimitations,
|
||||
...graphLimitations(context, coverage.status, relationshipOmissions),
|
||||
],
|
||||
};
|
||||
@@ -194,11 +198,11 @@ const graphLimitations = (
|
||||
);
|
||||
return [
|
||||
...context.analysis.limitations,
|
||||
...(relationshipOmissions.selfImports > 0
|
||||
? [
|
||||
`${String(relationshipOmissions.selfImports)} import ${relationshipOmissions.selfImports === 1 ? "specifier" : "specifiers"} resolved back to the importing module itself and ${relationshipOmissions.selfImports === 1 ? "was" : "were"} omitted; application graph edges cannot be self-referential.`,
|
||||
]
|
||||
: []),
|
||||
...selfReferenceOmissions(
|
||||
relationshipOmissions.selfImports,
|
||||
["import specifier", "import specifiers"],
|
||||
"the importing module",
|
||||
),
|
||||
"CommonJS and ESM binding relationships were recovered from inert syntax and resolved only within the inventoried artifact container.",
|
||||
"Webpack/Rspack factories were recovered from AST literals; REA did not invoke push handlers or bundle bootstrap code.",
|
||||
"Static imports, entrypoints, workers, endpoints, and storage relationships do not prove runtime execution.",
|
||||
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
chunkLookupKey,
|
||||
javascriptAnalysisCoverage,
|
||||
moduleLookupKey,
|
||||
selfReferenceOmissions,
|
||||
type JavaScriptArtifactGraphContext,
|
||||
type JavaScriptArtifactGraphCoverage,
|
||||
} from "./JavaScriptArtifactGraphContext.js";
|
||||
@@ -53,11 +54,11 @@ export const addJavaScriptBundlerNodes = (
|
||||
for (const record of records)
|
||||
selfAsyncChunks += addBundlerRuntimeEdges(context, record);
|
||||
}
|
||||
return selfAsyncChunks === 0
|
||||
? []
|
||||
: [
|
||||
`${String(selfAsyncChunks)} bundler async-chunk ${selfAsyncChunks === 1 ? "reference" : "references"} resolved back to the requesting chunk itself and ${selfAsyncChunks === 1 ? "was" : "were"} omitted; application graph edges cannot be self-referential.`,
|
||||
];
|
||||
return selfReferenceOmissions(
|
||||
selfAsyncChunks,
|
||||
["bundler async-chunk reference", "bundler async-chunk references"],
|
||||
"the requesting chunk",
|
||||
);
|
||||
};
|
||||
|
||||
interface BundlerProjectionInput {
|
||||
|
||||
@@ -306,6 +306,21 @@ export const linkElectronRoleToAsset = (
|
||||
});
|
||||
};
|
||||
|
||||
/**
|
||||
* Disclose relationships omitted because they resolved to their own source;
|
||||
* the application graph forbids self-referential edges.
|
||||
*/
|
||||
export const selfReferenceOmissions = (
|
||||
count: number,
|
||||
noun: readonly [singular: string, plural: string],
|
||||
owner: string,
|
||||
): string[] =>
|
||||
count === 0
|
||||
? []
|
||||
: [
|
||||
`${String(count)} ${count === 1 ? noun[0] : noun[1]} resolved back to ${owner} itself and ${count === 1 ? "was" : "were"} omitted; application graph edges cannot be self-referential.`,
|
||||
];
|
||||
|
||||
/** Deterministic key for one recovered module inside a bundle asset. */
|
||||
export const moduleLookupKey = (path: string, moduleKey: string): string =>
|
||||
`${path}\0${moduleKey}`;
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
linkElectronRoleToAsset,
|
||||
moduleLookupKey,
|
||||
resolveArtifactPath,
|
||||
selfReferenceOmissions,
|
||||
sourceNodeFor,
|
||||
type JavaScriptArtifactGraphContext,
|
||||
type JavaScriptArtifactGraphCoverage,
|
||||
@@ -42,14 +43,15 @@ interface ResolvedReference {
|
||||
/** Project imports, workers, roles, endpoints, storage, and source-map links. */
|
||||
export const addJavaScriptStaticFindings = (
|
||||
context: JavaScriptArtifactGraphContext,
|
||||
): void => {
|
||||
): readonly string[] => {
|
||||
const omissions = { selfReferences: 0 };
|
||||
for (const analyzed of context.analysis.files) {
|
||||
const { file, javascript } = analyzed;
|
||||
const asset = context.assetNodes.get(file.path);
|
||||
if (javascript === null || asset === undefined) continue;
|
||||
const coverage = javascriptAnalysisCoverage(javascript);
|
||||
for (const value of javascript.references)
|
||||
addReference(context, { file, asset, value, coverage });
|
||||
addReference(context, { file, asset, value, coverage }, omissions);
|
||||
for (const value of javascript.endpoints)
|
||||
addEndpoint(context, { file, asset, value, coverage });
|
||||
for (const value of javascript.storage)
|
||||
@@ -59,11 +61,17 @@ export const addJavaScriptStaticFindings = (
|
||||
for (const value of javascript.source_map_urls)
|
||||
addSourceMapEdge(context, { file, asset, value, coverage });
|
||||
}
|
||||
return selfReferenceOmissions(
|
||||
omissions.selfReferences,
|
||||
["static reference", "static references"],
|
||||
"the referencing module",
|
||||
);
|
||||
};
|
||||
|
||||
const addReference = (
|
||||
context: JavaScriptArtifactGraphContext,
|
||||
input: FindingInput<StaticReference>,
|
||||
omissions: { selfReferences: number },
|
||||
): void => {
|
||||
const source =
|
||||
sourceNodeFor(context, input.file.path, input.value.module_key) ??
|
||||
@@ -119,6 +127,13 @@ const addReference = (
|
||||
});
|
||||
return;
|
||||
}
|
||||
// A reference that resolves to its own source, such as a bundled module
|
||||
// requiring its own key, cannot become an edge: the application graph
|
||||
// forbids self-referential edges and one would fail the whole analysis.
|
||||
if (resolved?.node.node_id === source.node_id) {
|
||||
omissions.selfReferences += 1;
|
||||
return;
|
||||
}
|
||||
const target =
|
||||
resolved?.node ??
|
||||
unresolvedReferenceNode(context, input.file, input.value, input.coverage);
|
||||
|
||||
@@ -3,13 +3,39 @@ import { expect } from "vitest";
|
||||
import { parseEvidence } from "../../../src/domain/evidence.js";
|
||||
import { javascriptApplicationAnalysisResultSchema } from "../../../src/domain/javascript/javascriptApplicationAnalysis.js";
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
import { cliTest } from "../../support/cli/cliFixture.js";
|
||||
import { cliTest, type TestCli } from "../../support/cli/cliFixture.js";
|
||||
import {
|
||||
findChunkNode,
|
||||
findGraphEdge,
|
||||
writeFixtureFiles,
|
||||
} from "../../support/javascriptApplicationFixture.js";
|
||||
|
||||
/** Analyze one fixture and check the graph-wide self-reference invariants. */
|
||||
const analyzeBundle = async (
|
||||
cli: TestCli,
|
||||
prefix: string,
|
||||
files: Readonly<Record<string, string>>,
|
||||
) => {
|
||||
const root = await createTestTempDirectory(prefix);
|
||||
await writeFixtureFiles(root, files);
|
||||
const output = await cli.run({
|
||||
arguments: ["analyze-javascript-application", root, "--json"],
|
||||
});
|
||||
expect(output.exitCode).toBe(0);
|
||||
const result = javascriptApplicationAnalysisResultSchema.parse(
|
||||
parseEvidence(output.json).normalized_result,
|
||||
);
|
||||
expect(
|
||||
result.graph.edges.filter(
|
||||
(edge) => edge.source_node_id === edge.target_node_id,
|
||||
),
|
||||
).toEqual([]);
|
||||
expect(result.limitations).toEqual(
|
||||
expect.arrayContaining(result.graph.limitations),
|
||||
);
|
||||
return result.graph;
|
||||
};
|
||||
|
||||
for (const scenario of [
|
||||
{ runtime: "webpackChunkdemo", keys: [1], omitted: 1 },
|
||||
{ runtime: "rspackChunkdemo", keys: [1, 2, 3], omitted: 3 },
|
||||
@@ -17,8 +43,7 @@ for (const scenario of [
|
||||
cliTest(
|
||||
`preserves ${scenario.runtime} evidence when a chunk requests its own IDs`,
|
||||
async ({ cli }) => {
|
||||
const root = await createTestTempDirectory("rea-bundler-self-");
|
||||
await writeFixtureFiles(root, {
|
||||
const graph = await analyzeBundle(cli, "rea-bundler-self-", {
|
||||
"runtime.js": `
|
||||
(self.${scenario.runtime} = self.${scenario.runtime} || []).push([
|
||||
${JSON.stringify(scenario.keys)},
|
||||
@@ -32,19 +57,6 @@ for (const scenario of [
|
||||
]);
|
||||
`,
|
||||
});
|
||||
const output = await cli.run({
|
||||
arguments: ["analyze-javascript-application", root, "--json"],
|
||||
});
|
||||
expect(output.exitCode).toBe(0);
|
||||
const result = javascriptApplicationAnalysisResultSchema.parse(
|
||||
parseEvidence(output.json).normalized_result,
|
||||
);
|
||||
const { graph } = result;
|
||||
expect(
|
||||
graph.edges.filter(
|
||||
(edge) => edge.source_node_id === edge.target_node_id,
|
||||
),
|
||||
).toEqual([]);
|
||||
expect(graph.limitations).toContainEqual(
|
||||
expect.stringMatching(
|
||||
new RegExp(
|
||||
@@ -53,9 +65,6 @@ for (const scenario of [
|
||||
),
|
||||
),
|
||||
);
|
||||
expect(result.limitations).toEqual(
|
||||
expect.arrayContaining(graph.limitations),
|
||||
);
|
||||
const source = findChunkNode(graph, scenario.runtime, "1");
|
||||
expect(source?.observations[0]?.properties.async_chunk_keys).toEqual(
|
||||
expect.arrayContaining([...scenario.keys.map(String), "4", "99"]),
|
||||
@@ -89,3 +98,33 @@ for (const scenario of [
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
cliTest(
|
||||
"preserves bundle evidence when a module requires its own key",
|
||||
async ({ cli }) => {
|
||||
const graph = await analyzeBundle(cli, "rea-bundler-self-require-", {
|
||||
"runtime.js": `
|
||||
(self.webpackChunkdemo = self.webpackChunkdemo || []).push([
|
||||
[5], {50(e,t,r){r(50); r(51)}, 51(e,t,r){r.r(t)}}
|
||||
]);
|
||||
`,
|
||||
});
|
||||
expect(graph.limitations).toContainEqual(
|
||||
expect.stringMatching(
|
||||
/^1 static reference resolved back to the referencing module itself and was omitted;/u,
|
||||
),
|
||||
);
|
||||
expect(
|
||||
graph.edges.filter(
|
||||
({ relation, properties }) =>
|
||||
relation === "imports" &&
|
||||
properties.kind === "require" &&
|
||||
["50", "51"].includes(String(properties.specifier)),
|
||||
),
|
||||
).toEqual([
|
||||
expect.objectContaining({
|
||||
properties: expect.objectContaining({ specifier: "51" }),
|
||||
}),
|
||||
]);
|
||||
},
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user