From fc249fdfcf0afb2225fc4df711552bfa745ade86 Mon Sep 17 00:00:00 2001 From: Giancarlo Erra Date: Tue, 28 Apr 2026 00:20:45 +0100 Subject: [PATCH] fix(graph): make C# namespace resolution deterministic `buildCsNamespaceMap` previously iterated `fileSet` in fs.readdir() traversal order, which POSIX does not guarantee. Combined with the `candidates[0]` selection in `resolveImport`, this meant a `using` directive could resolve to different files on different machines or runs. Sort the `.cs` paths lexically before scanning so candidate lists are stable. Adds a regression test that builds the map twice from sets populated in different orders and asserts both produce the same candidate sequence and the same first candidate. Found by CodeRabbit on PR #34. --- src/services/graph-resolution.ts | 16 ++++++++-- tests/unit/graph-resolution.test.ts | 47 +++++++++++++++++++++++++---- 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/src/services/graph-resolution.ts b/src/services/graph-resolution.ts index fe05068..b0cd97c 100644 --- a/src/services/graph-resolution.ts +++ b/src/services/graph-resolution.ts @@ -60,12 +60,17 @@ export function buildJvmSuffixMap(fileSet: Set): Map { * introduced in C# 10) and builds: * * key: "App.Services" - * value: ["src/Services/UserService.cs", "src/Services/OrderService.cs"] + * value: ["src/Services/OrderService.cs", "src/Services/UserService.cs"] * * Used to resolve `using App.Services;` to the candidate files that * contribute to that namespace. Without this, every C# `using` resolved * to `null` and C# projects produced an empty file-import graph. * + * Files are processed in lexicographic order so the resulting candidate + * lists are deterministic across machines and runs. This matters because + * multi-file namespaces resolve to `candidates[0]` in `resolveImport`, + * and a stable "first" file is required for reproducible graphs. + * * Cost: O(n) reads at graph-build time (negligible vs. AST parsing). Files * with no `namespace` declaration are silently skipped. Read failures are * swallowed since this is best-effort. @@ -80,8 +85,13 @@ export function buildCsNamespaceMap( // so commented lines (`// namespace X.Y`) are not matched. const namespaceRegex = /^namespace\s+([\w.]+)/gm; - for (const f of fileSet) { - if (path.extname(f).toLowerCase() !== ".cs") continue; + // `fileSet` reflects fs.readdir() traversal order, which POSIX does not + // guarantee. Sort .cs paths lexically so candidate lists are stable. + const csFiles = [...fileSet] + .filter((f) => path.extname(f).toLowerCase() === ".cs") + .sort(); + + for (const f of csFiles) { let source: string; try { source = readFileSync(path.join(projectPath, f), "utf-8"); diff --git a/tests/unit/graph-resolution.test.ts b/tests/unit/graph-resolution.test.ts index eb43a00..bc463e0 100644 --- a/tests/unit/graph-resolution.test.ts +++ b/tests/unit/graph-resolution.test.ts @@ -754,9 +754,10 @@ describe("graph-resolution", () => { csNamespaceMap, ); - // Multi-file namespaces resolve to the first registered file. Multi-file - // fan-out is a known follow-up. - expect(result).toBe("Services/UserService.cs"); + // Multi-file namespaces resolve to the first registered file. Files are + // visited in lexicographic order, so OrderService.cs precedes + // UserService.cs. Multi-file fan-out is a known follow-up. + expect(result).toBe("Services/OrderService.cs"); }); it("returns null for unknown namespaces even with a populated map", () => { @@ -804,15 +805,49 @@ describe("graph-resolution", () => { // ── buildCsNamespaceMap ─────────────────────────────────────────────── describe("buildCsNamespaceMap", () => { - it("indexes block-scoped namespace declarations", () => { + it("indexes block-scoped namespace declarations in lexicographic order", () => { project = createTempProject({ "Models/User.cs": "namespace MyApp.Models { public class User {} }", "Models/Order.cs": "namespace MyApp.Models { public class Order {} }", }); const map = buildCsNamespaceMap(project.fileSet, project.root); - const files = map.get("MyApp.Models") ?? []; - expect(files.sort()).toEqual(["Models/Order.cs", "Models/User.cs"]); + // Files are sorted lexically, so Order.cs comes before User.cs. + expect(map.get("MyApp.Models")).toEqual([ + "Models/Order.cs", + "Models/User.cs", + ]); + }); + + it("returns the same candidate order regardless of fileSet insertion order", () => { + // Build two projects on the same physical layout but feed buildCsNamespaceMap + // a Set populated in two different orders, mimicking how fs.readdir() can + // hand back entries in arbitrary order across filesystems. + project = createTempProject({ + "Services/UserService.cs": + "namespace MyApp.Services { public class UserService {} }", + "Services/OrderService.cs": + "namespace MyApp.Services { public class OrderService {} }", + "Services/AccountService.cs": + "namespace MyApp.Services { public class AccountService {} }", + }); + + const forward = new Set([ + "Services/AccountService.cs", + "Services/OrderService.cs", + "Services/UserService.cs", + ]); + const reverse = new Set([ + "Services/UserService.cs", + "Services/OrderService.cs", + "Services/AccountService.cs", + ]); + + const a = buildCsNamespaceMap(forward, project.root); + const b = buildCsNamespaceMap(reverse, project.root); + + expect(a.get("MyApp.Services")).toEqual(b.get("MyApp.Services")); + expect(a.get("MyApp.Services")?.[0]).toBe("Services/AccountService.cs"); }); it("indexes file-scoped namespace declarations (C# 10+)", () => {