mirror of
https://github.com/giancarloerra/socraticode.git
synced 2026-07-03 14:05:21 +02:00
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.
This commit is contained in:
@@ -60,12 +60,17 @@ export function buildJvmSuffixMap(fileSet: Set<string>): Map<string, string> {
|
||||
* 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");
|
||||
|
||||
@@ -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+)", () => {
|
||||
|
||||
Reference in New Issue
Block a user