Merge pull request #695 from kaoru0822-kitauji/fix/reference-parser-recovery

fix(reference): retain recovered parser diagnostics

src/domain/referenceSourceImportParsing.ts

@@ -291,8 +291,7 @@ const parseWithBabel = (
   language: string | null,
 ): {
   ast: File | undefined;
-  parseState: ReferenceSourceImportParseState;
-  reason?: string;
+  reasons: readonly string[];
 } => {
   const plugins: ParserPlugin[] = ["jsx"];
   if (language === "TypeScript" || language === "TSX") {
@@ -316,12 +315,11 @@ const parseWithBabel = (
       errorRecovery: true,
       plugins,
     });
-    return { ast, parseState: "parsed" };
+    return { ast, reasons: ast.errors.map((error) => error.message) };
   } catch (cause: unknown) {
     return {
       ast: undefined,
-      parseState: "unknown",
-      reason: cause instanceof Error ? cause.message : "Parse failed",
+      reasons: [cause instanceof Error ? cause.message : "Parse failed"],
     };
   }
 };
@@ -336,18 +334,17 @@ export const parseReferenceSourceImports = (
     return { relationships: [], parse_failures: [] };
 
   const source = new TextDecoder("utf-8", { fatal: false }).decode(bytes);
-  const { ast, parseState, reason } = parseWithBabel(path, source, language);
+  const { ast, reasons } = parseWithBabel(path, source, language);
+  const parseFailures = reasons.map((reason) => ({
+    path,
+    parser: "babel",
+    reason,
+  }));
 
-  if (ast === undefined || parseState === "unknown") {
+  if (ast === undefined) {
     return {
       relationships: [],
-      parse_failures: [
-        {
-          path,
-          parser: "babel",
-          reason: reason ?? "Unknown parse failure",
-        },
-      ],
+      parse_failures: parseFailures,
     };
   }
 
@@ -356,7 +353,13 @@ export const parseReferenceSourceImports = (
   extractRequireAndDynamicImports(ast.program.body, path, relationships);
 
   return {
-    relationships,
-    parse_failures: [],
+    relationships: relationships.map((relationship) => ({
+      ...relationship,
+      parse_state:
+        parseFailures.length > 0 && relationship.parse_state === "parsed"
+          ? "partial"
+          : relationship.parse_state,
+    })),
+    parse_failures: parseFailures,
   };
 };

src/domain/referenceSourceImportRecovery.test.ts

@@ -0,0 +1,150 @@
+import { describe, expect, it } from "vitest";
+
+import { parseReferenceSourceImports } from "./referenceSourceImportParsing.js";
+
+const recoverySource = [
+  "const missing;",
+  "let duplicate;",
+  "let duplicate;",
+  "let duplicate;",
+].join("\n");
+
+const recoveryReasons = [
+  "Missing initializer in const declaration. (1:13)",
+  "Identifier 'duplicate' has already been declared. (3:4)",
+  "Identifier 'duplicate' has already been declared. (4:4)",
+];
+
+const moduleSource = [
+  'import "./static.js";',
+  'export { value } from "./named.js";',
+  'export * from "./all.js";',
+  'require("./common.cjs");',
+  'require.resolve("node:fs");',
+  'import("./lazy.js");',
+  "import(moduleName);",
+].join("\n");
+
+const moduleRelationships = [
+  ["./static.js", "imports", "internal", "parsed"],
+  ["./named.js", "imports", "internal", "parsed"],
+  ["./all.js", "imports", "internal", "parsed"],
+  ["./common.cjs", "requires", "internal", "parsed"],
+  ["node:fs", "requires", "external", "parsed"],
+  ["./lazy.js", "imports", "internal", "parsed"],
+  ["<dynamic-import>", "imports", "unknown", "partial"],
+].map(([to, kind, resolution, parse_state]) => ({
+  to,
+  kind,
+  resolution,
+  parse_state,
+}));
+
+const parseSource = (path: string, source: string, language: string | null) =>
+  parseReferenceSourceImports(path, new TextEncoder().encode(source), language);
+
+describe.each([
+  ["main.js", "JavaScript"],
+  ["main.jsx", "JSX"],
+  ["main.ts", "TypeScript"],
+  ["main.tsx", "TSX"],
+])("recovered reference imports in %s", (path, language) => {
+  it("retains every diagnostic and marks recovered relationships partial", () => {
+    const result = parseSource(
+      path,
+      `${recoverySource}\n${moduleSource}`,
+      language,
+    );
+
+    expect(result.parse_failures).toEqual(
+      recoveryReasons.map((reason) => ({ path, parser: "babel", reason })),
+    );
+    expect(result.relationships).toEqual(
+      moduleRelationships.map((relationship) => ({
+        ...relationship,
+        from_path: path,
+        parse_state: "partial",
+      })),
+    );
+  });
+
+  it("preserves valid and computed relationships without diagnostics", () => {
+    expect(parseSource(path, moduleSource, language)).toEqual({
+      relationships: moduleRelationships.map((relationship) => ({
+        ...relationship,
+        from_path: path,
+      })),
+      parse_failures: [],
+    });
+  });
+
+  it("reports recovery diagnostics when there are no relationships", () => {
+    expect(parseSource(path, recoverySource, language)).toEqual({
+      relationships: [],
+      parse_failures: recoveryReasons.map((reason) => ({
+        path,
+        parser: "babel",
+        reason,
+      })),
+    });
+  });
+
+  it("returns no relationships when parsing throws", () => {
+    expect(
+      parseSource(path, 'import "./before.js";\nconst = ;', language),
+    ).toEqual({
+      relationships: [],
+      parse_failures: [
+        { path, parser: "babel", reason: "Unexpected token (2:6)" },
+      ],
+    });
+  });
+});
+
+it.each(["main.ts", "types.d.ts", "types.d.mts", "types.d.cts"])(
+  "marks recovered TypeScript declarations partial in %s",
+  (path) => {
+    const result = parseSource(
+      path,
+      [
+        "let duplicate: number;",
+        "let duplicate: number;",
+        'import dep = require("node:path");',
+        'declare module "ambient-package" {}',
+      ].join("\n"),
+      "TypeScript",
+    );
+    expect(result).toEqual({
+      relationships: [
+        {
+          from_path: path,
+          to: "node:path",
+          kind: "requires",
+          resolution: "external",
+          parse_state: "partial",
+        },
+        {
+          from_path: path,
+          to: "ambient-package",
+          kind: "declares-module",
+          resolution: "unknown",
+          parse_state: "partial",
+        },
+      ],
+      parse_failures: [
+        {
+          path,
+          parser: "babel",
+          reason: "Identifier 'duplicate' has already been declared. (2:4)",
+        },
+      ],
+    });
+  },
+);
+
+it("keeps unsupported source languages outside Babel parsing", () => {
+  expect(parseSource("main.py", recoverySource, "Python")).toEqual({
+    relationships: [],
+    parse_failures: [],
+  });
+});

tests/boundary/filesystem/referenceSourceRecovery.test.ts

@@ -0,0 +1,143 @@
+import { createHash } from "node:crypto";
+import { mkdir, writeFile } from "node:fs/promises";
+import { join } from "node:path";
+
+import { expect, it } from "vitest";
+
+import {
+  importReferenceSource,
+  normalizeHistoricalSourceParseFailures,
+} from "../../../src/application/ReferenceSourceImport.js";
+import { historicalSourceGraphSchema } from "../../../src/domain/referenceSourceGraph.js";
+import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
+
+it("preserves recovered diagnostics and file evidence through filesystem import", async () => {
+  const root = await createTestTempDirectory("rea-reference-recovery-");
+  const sources = {
+    "fatal.js": 'import "./valid";\nconst = ;',
+    "recovered.js": [
+      "const missing;",
+      "let duplicate;",
+      "let duplicate;",
+      "let duplicate;",
+      'import "./valid";',
+      'require("node:fs");',
+      "import(moduleName);",
+    ].join("\n"),
+    "recovered.ts": [
+      "let duplicate: number;",
+      "let duplicate: number;",
+      'import dep = require("./valid");',
+      'declare module "ambient-package" {}',
+    ].join("\n"),
+    "valid.js": 'export * from "node:path";\n',
+  };
+  await mkdir(join(root, "src"));
+  await Promise.all(
+    Object.entries(sources).map(([path, source]) =>
+      writeFile(join(root, "src", path), source),
+    ),
+  );
+
+  const result = await importReferenceSource({
+    root,
+    caller: "reference-recovery-test",
+    policy: { secretPatterns: [] },
+  });
+  expect(result.ok).toBe(true);
+  if (!result.ok) return;
+  const graph = result.value;
+  expect(historicalSourceGraphSchema.safeParse(graph).success).toBe(true);
+  expect(graph.entries.map(({ path }) => path)).toEqual([
+    "src",
+    ...Object.keys(sources).map((path) => `src/${path}`),
+  ]);
+  for (const [path, source] of Object.entries(sources)) {
+    expect(graph.entries).toContainEqual(
+      expect.objectContaining({
+        path: `src/${path}`,
+        kind: "file",
+        size: Buffer.byteLength(source),
+        sha256: createHash("sha256").update(source).digest("hex"),
+        content_state: "hashed",
+        limitations: [],
+      }),
+    );
+  }
+
+  const failures = [
+    { path: "src/fatal.js", parser: "babel", reason: "Unexpected token (2:6)" },
+    {
+      path: "src/recovered.js",
+      parser: "babel",
+      reason: "Identifier 'duplicate' has already been declared. (3:4)",
+    },
+    {
+      path: "src/recovered.js",
+      parser: "babel",
+      reason: "Identifier 'duplicate' has already been declared. (4:4)",
+    },
+    {
+      path: "src/recovered.js",
+      parser: "babel",
+      reason: "Missing initializer in const declaration. (1:13)",
+    },
+    {
+      path: "src/recovered.ts",
+      parser: "babel",
+      reason: "Identifier 'duplicate' has already been declared. (2:4)",
+    },
+  ];
+  expect(graph.parse_failures).toEqual(failures);
+  expect(
+    normalizeHistoricalSourceParseFailures(
+      [...graph.parse_failures].reverse().concat(graph.parse_failures),
+    ),
+  ).toEqual(failures);
+  expect(graph.relationships).toEqual([
+    {
+      from_path: "src/recovered.js",
+      to: "<dynamic-import>",
+      kind: "imports",
+      resolution: "unknown",
+      parse_state: "partial",
+    },
+    {
+      from_path: "src/recovered.js",
+      to: "node:fs",
+      kind: "requires",
+      resolution: "external",
+      parse_state: "partial",
+    },
+    {
+      from_path: "src/recovered.js",
+      to: "src/valid.js",
+      kind: "imports",
+      resolution: "internal",
+      parse_state: "partial",
+    },
+    {
+      from_path: "src/recovered.ts",
+      to: "ambient-package",
+      kind: "declares-module",
+      resolution: "unknown",
+      parse_state: "partial",
+    },
+    {
+      from_path: "src/recovered.ts",
+      to: "src/valid.js",
+      kind: "requires",
+      resolution: "internal",
+      parse_state: "partial",
+    },
+    {
+      from_path: "src/valid.js",
+      to: "node:path",
+      kind: "imports",
+      resolution: "external",
+      parse_state: "parsed",
+    },
+  ]);
+  // The reader's pathname-race advisory already makes inventory partial.
+  expect(graph.inventory_state).toBe("partial");
+});