fix(javascript): preserve literal CommonJS path punctuation (#576)

src/application/ElectronBoundaryAnalysis.ts

@@ -169,6 +169,7 @@ export const summarizeElectronBoundaries = (
         context: "module-specifier",
         files,
         kind: "native-addon",
+        moduleKind: finding.module_kind,
       }),
     ).length,
   };
@@ -201,6 +202,7 @@ interface ResolveToFileInput {
   readonly context: "module-specifier" | "filesystem-expression";
   readonly files: ReadonlyMap<string, JavaScriptArtifactFile>;
   readonly kind?: JavaScriptArtifactFile["kind"];
+  readonly moduleKind?: "import" | "require";
 }
 
 const resolvesToFile = (input: ResolveToFileInput): boolean => {
@@ -209,6 +211,7 @@ const resolvesToFile = (input: ResolveToFileInput): boolean => {
     sourcePath: input.file.path,
     context: input.context,
     files: input.files,
+    ...(input.moduleKind === undefined ? {} : { moduleKind: input.moduleKind }),
   }).resolved_path;
   return (
     resolved !== null &&

src/application/ElectronBoundaryGraphNative.ts

@@ -42,6 +42,7 @@ const addNativeBinding = (input: NativeBindingInput): void => {
     value.specifier,
     file.path,
     context.filesByPath,
+    value.module_kind,
   );
   const addon = resolved === null ? undefined : context.fileNodes.get(resolved);
   const binding = context.accumulator.addNode({

src/application/JavaScriptArtifactPathResolution.ts

@@ -113,7 +113,8 @@ const contextualCandidate = (
 ): string | ArtifactPathResolution => {
   const { context } = input;
   const declared =
-    context === "module-specifier" || context === "html-reference"
+    context === "html-reference" ||
+    (context === "module-specifier" && input.moduleKind !== "require")
       ? stripQueryAndFragment(input.declaredPath)
       : input.declaredPath;
   if (context === "html-reference") return htmlCandidate(input);

src/application/JavaScriptModuleSpecifierPaths.test.ts

@@ -0,0 +1,77 @@
+import { describe, expect, it } from "vitest";
+
+import type { JavaScriptArtifactFile } from "./JavaScriptArtifactFiles.js";
+import { resolveArtifactPathByContext } from "./JavaScriptArtifactPathResolution.js";
+
+const filesFor = (paths: readonly string[]) =>
+  new Map<string, JavaScriptArtifactFile>(
+    ["app/main.js", ...paths].map((path) => [
+      path,
+      {
+        path,
+        container_sha256: "a".repeat(64),
+        sha256: "b".repeat(64),
+        bytes: 0,
+        inventory_artifact_id: `artifact-${path}`,
+        kind: "javascript",
+        unpacked: false,
+        text: { included: true, value: "" },
+      },
+    ]),
+  );
+
+const cases = ["#", "?"].flatMap((punctuation) =>
+  (["require", "import", undefined] as const).flatMap((moduleKind) =>
+    (["literal", "stripped", "both"] as const).map((layout) => ({
+      punctuation,
+      moduleKind,
+      layout,
+    })),
+  ),
+);
+
+describe("module specifier punctuation", () => {
+  it.each(cases)(
+    "resolves $moduleKind with $punctuation and $layout files",
+    ({ punctuation, moduleKind, layout }) => {
+      const literal = `app/dep.cjs${punctuation}literal.cjs`;
+      const stripped = "app/dep.cjs";
+      const files = filesFor([
+        ...(layout !== "stripped" ? [literal] : []),
+        ...(layout !== "literal" ? [stripped] : []),
+      ]);
+      const expected = moduleKind === "require" ? literal : stripped;
+      const result = resolveArtifactPathByContext({
+        declaredPath: `./dep.cjs${punctuation}literal.cjs`,
+        sourcePath: "app/main.js",
+        context: "module-specifier",
+        files,
+        ...(moduleKind === undefined ? {} : { moduleKind }),
+      });
+      expect(result).toMatchObject({
+        resolution_status: files.has(expected) ? "resolved" : "not-found",
+        resolved_path: files.has(expected) ? expected : null,
+      });
+    },
+  );
+
+  it.each(["#", "?"])(
+    "keeps HTML URL suffix handling for %s",
+    (punctuation) => {
+      expect(
+        resolveArtifactPathByContext({
+          declaredPath: `./dep.cjs${punctuation}literal.cjs`,
+          sourcePath: "app/main.js",
+          context: "html-reference",
+          files: filesFor([
+            "app/dep.cjs",
+            `app/dep.cjs${punctuation}literal.cjs`,
+          ]),
+        }),
+      ).toMatchObject({
+        resolution_status: "resolved",
+        resolved_path: "app/dep.cjs",
+      });
+    },
+  );
+});

src/domain/electronNativeLoaderPaths.test.ts

@@ -0,0 +1,51 @@
+import { describe, expect, it } from "vitest";
+
+import { analyzeJavaScriptStaticSource } from "./javascriptStaticAnalysis.js";
+
+describe("native addon loader provenance", () => {
+  it("distinguishes CommonJS and ESM re-exports", () => {
+    const analysis = analyzeJavaScriptStaticSource(`
+      const required = require("./addon.node#literal.node");
+      module.exports = require("./addon.node#literal.node");
+      import imported from "./addon.node#literal.node";
+      export { default } from "./addon.node#literal.node";
+    `);
+    expect(analysis.electron.native_addon_bindings).toHaveLength(4);
+    expect(analysis.electron.native_addon_bindings).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({
+          binding_kind: "require",
+          module_kind: "require",
+        }),
+        expect.objectContaining({
+          binding_kind: "import",
+          module_kind: "import",
+        }),
+        expect.objectContaining({
+          binding_kind: "re-export",
+          module_kind: "require",
+        }),
+        expect.objectContaining({
+          binding_kind: "re-export",
+          module_kind: "import",
+        }),
+      ]),
+    );
+  });
+
+  it.each(["#", "?"])(
+    "classifies require filenames literally for %s",
+    (punctuation) => {
+      const analysis = analyzeJavaScriptStaticSource(`
+      const script = require("./addon.node${punctuation}helper.js");
+      import native from "./addon.node${punctuation}helper.js";
+    `);
+      expect(analysis.electron.native_addon_bindings).toEqual([
+        expect.objectContaining({
+          binding_kind: "import",
+          module_kind: "import",
+        }),
+      ]);
+    },
+  );
+});

src/domain/electronStaticAnalysisNative.ts

@@ -17,6 +17,7 @@ interface NativeBindingInput {
   readonly node: t.Node;
   readonly specifier: string;
   readonly kind: ElectronNativeAddonBindingFinding["binding_kind"];
+  readonly moduleKind: ElectronNativeAddonBindingFinding["module_kind"];
   readonly members: readonly string[];
 }
 
@@ -36,7 +37,7 @@ const inspectImport = (
   node: t.ImportDeclaration,
   context: JavaScriptFindingContext,
 ): void => {
-  if (!isNativeSpecifier(node.source.value)) return;
+  if (!isNativeSpecifier(node.source.value, "import")) return;
   const members = node.specifiers.map((specifier) => {
     if (t.isImportDefaultSpecifier(specifier)) return "default";
     if (t.isImportNamespaceSpecifier(specifier)) return "*";
@@ -47,6 +48,7 @@ const inspectImport = (
     node,
     specifier: node.source.value,
     kind: "import",
+    moduleKind: "import",
     members,
   });
 };
@@ -58,7 +60,7 @@ const inspectNamedExport = (
   if (
     node.source === null ||
     node.source === undefined ||
-    !isNativeSpecifier(node.source.value)
+    !isNativeSpecifier(node.source.value, "import")
   )
     return;
   const members = node.specifiers.map((specifier) => {
@@ -70,6 +72,7 @@ const inspectNamedExport = (
     node,
     specifier: node.source.value,
     kind: "re-export",
+    moduleKind: "import",
     members,
   });
 };
@@ -78,13 +81,17 @@ const inspectExportAll = (
   node: t.ExportAllDeclaration,
   context: JavaScriptFindingContext,
 ): void => {
-  if (node.source === undefined || !isNativeSpecifier(node.source.value))
+  if (
+    node.source === undefined ||
+    !isNativeSpecifier(node.source.value, "import")
+  )
     return;
   addBinding({
     context,
     node,
     specifier: node.source.value,
     kind: "re-export",
+    moduleKind: "import",
     members: ["*"],
   });
 };
@@ -100,6 +107,7 @@ const inspectRequireBinding = (
     node,
     specifier: required.specifier,
     kind: "require",
+    moduleKind: "require",
     members:
       required.member === null ? bindingMembers(node.id) : [required.member],
   });
@@ -116,6 +124,7 @@ const inspectReExport = (
     node,
     specifier: required.specifier,
     kind: "re-export",
+    moduleKind: "require",
     members: [required.member ?? exportedMember(node.left) ?? "*"],
   });
 };
@@ -133,6 +142,7 @@ const addBinding = (input: NativeBindingInput): void => {
     value: {
       specifier,
       binding_kind: kind,
+      module_kind: input.moduleKind,
       members,
       module_key: null,
       location: range(node),
@@ -162,7 +172,7 @@ const nativeRequire = (
   )
     return undefined;
   const specifier = stringValue(argumentNode(node.arguments[0]));
-  return specifier !== undefined && isNativeSpecifier(specifier)
+  return specifier !== undefined && isNativeSpecifier(specifier, "require")
     ? { specifier, member: null }
     : undefined;
 };
@@ -193,7 +203,13 @@ const exportedMember = (node: t.Node): string | undefined => {
   return undefined;
 };
 
-const isNativeSpecifier = (specifier: string): boolean => {
-  const path = specifier.split("#", 1)[0]?.split("?", 1)[0] ?? "";
+const isNativeSpecifier = (
+  specifier: string,
+  moduleKind: ElectronNativeAddonBindingFinding["module_kind"],
+): boolean => {
+  const path =
+    moduleKind === "require"
+      ? specifier
+      : (specifier.split("#", 1)[0]?.split("?", 1)[0] ?? "");
   return path.toLowerCase().endsWith(".node");
 };

src/domain/electronStaticAnalysisTypes.ts

@@ -162,6 +162,7 @@ export type ElectronUtilityProcessFinding = ElectronUtilityProcessFindingState &
 export interface ElectronNativeAddonBindingFinding {
   readonly specifier: string;
   readonly binding_kind: "import" | "require" | "re-export";
+  readonly module_kind: "import" | "require";
   readonly members: readonly string[];
   readonly module_key: string | null;
   readonly location: JavaScriptSourceRange;

tests/boundary/filesystem/javascriptRequireLiteralPaths.test.ts

@@ -0,0 +1,133 @@
+import { writeFile } from "node:fs/promises";
+import { createRequire } from "node:module";
+import { join, relative, sep } from "node:path";
+
+import { describe, expect, it } from "vitest";
+
+import { analyzeJavaScriptApplication } from "../../../src/application/JavaScriptApplicationService.js";
+import { parseApplicationGraphEvidence } from "../../../src/application/JavaScriptApplicationEvidenceGraph.js";
+import { javascriptApplicationAnalysisResultSchema } from "../../../src/domain/javascriptApplicationAnalysis.js";
+import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
+
+const analyze = async (root: string) => {
+  const result = await analyzeJavaScriptApplication({
+    input_path: root,
+    format: "directory",
+  });
+  if (!result.ok) throw result.error;
+  const authenticated = parseApplicationGraphEvidence(result.value);
+  return {
+    graph: authenticated.graph,
+    result: javascriptApplicationAnalysisResultSchema.parse(
+      authenticated.evidence.normalized_result,
+    ),
+  };
+};
+
+describe("literal CommonJS paths in application graphs", () => {
+  it("agrees with Node for both require projections while preserving ESM suffixes", async () => {
+    const root = await createTestTempDirectory("rea-literal-require-");
+    const specifier = "./dep.cjs#literal.cjs";
+    await Promise.all([
+      writeFile(
+        join(root, "main.cjs"),
+        `const value = require(${JSON.stringify(specifier)}); module.exports = value;`,
+      ),
+      writeFile(
+        join(root, "imports.mjs"),
+        `import value from ${JSON.stringify(specifier)}; export { value };`,
+      ),
+      writeFile(join(root, "dep.cjs"), "module.exports = 'stripped';"),
+      writeFile(
+        join(root, "dep.cjs#literal.cjs"),
+        "module.exports = 'literal';",
+      ),
+    ]);
+    const expected = relative(
+      root,
+      createRequire(join(root, "main.cjs")).resolve(specifier),
+    )
+      .split(sep)
+      .join("/");
+    expect(expected).toBe("dep.cjs#literal.cjs");
+    const { graph } = await analyze(root);
+    const properties = graph.edges
+      .filter((edge) => edge.properties.specifier === specifier)
+      .map((edge) => edge.properties);
+    expect(properties).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({
+          module_link_kind: "require",
+          resolved_path: expected,
+        }),
+        expect.objectContaining({ kind: "require", resolved_path: expected }),
+        expect.objectContaining({
+          module_link_kind: "import",
+          resolved_path: "dep.cjs",
+        }),
+        expect.objectContaining({
+          kind: "static-import",
+          resolved_path: "dep.cjs",
+        }),
+      ]),
+    );
+    expect(graph.coverage.status).toBe("complete");
+  });
+
+  it.each(["literal", "stripped", "both"] as const)(
+    "keeps native graph/count projections aligned with %s files",
+    async (layout) => {
+      const root = await createTestTempDirectory("rea-literal-native-");
+      const specifier = "./addon.node#literal.node";
+      await Promise.all([
+        writeFile(
+          join(root, "main.cjs"),
+          `const addon = require(${JSON.stringify(specifier)}); module.exports = require(${JSON.stringify(specifier)}); throw new Error('must not execute');`,
+        ),
+        writeFile(
+          join(root, "imports.mjs"),
+          `import addon from ${JSON.stringify(specifier)}; export { default } from ${JSON.stringify(specifier)}; throw new Error('must not execute');`,
+        ),
+        ...(layout !== "stripped"
+          ? [
+              writeFile(
+                join(root, "addon.node#literal.node"),
+                Buffer.from([0, 1, 2, 3]),
+              ),
+            ]
+          : []),
+        ...(layout !== "literal"
+          ? [writeFile(join(root, "addon.node"), Buffer.from([0, 4, 5, 6]))]
+          : []),
+      ]);
+      const { graph, result } = await analyze(root);
+      const bindings = graph.nodes
+        .filter(({ kind }) => kind === "native-export")
+        .flatMap(({ observations }) => observations)
+        .map((observation) => ({
+          source:
+            observation.evidence.location.available &&
+            observation.evidence.location.value.kind === "source-range"
+              ? observation.evidence.location.value.source
+              : null,
+          binding: observation.properties.binding_kind,
+          path: observation.properties.resolved_path,
+        }));
+      const required = layout !== "stripped" ? "addon.node#literal.node" : null;
+      const imported = layout !== "literal" ? "addon.node" : null;
+      expect(bindings).toHaveLength(4);
+      expect(bindings).toEqual(
+        expect.arrayContaining([
+          { source: "main.cjs", binding: "require", path: required },
+          { source: "main.cjs", binding: "re-export", path: required },
+          { source: "imports.mjs", binding: "import", path: imported },
+          { source: "imports.mjs", binding: "re-export", path: imported },
+        ]),
+      );
+      expect(result.summary).toMatchObject({
+        native_addon_bindings: 4,
+        resolved_native_addon_bindings: layout === "both" ? 4 : 2,
+      });
+    },
+  );
+});