From 9277b3f766f959ff3b31271e21d797a5fa913495 Mon Sep 17 00:00:00 2001 From: Jordan Coin Jackson Date: Sun, 11 Oct 2026 22:23:35 +0200 Subject: [PATCH] fix(scanner): file-graph keys are forward-slashed on every OS; test binary gets .exe on Windows Group C of #214 was a product bug: on Windows the walker and ast-grep emitted native paths, filepath.Clean/Join/Dir rebuilt probes with "\", and every import written with "/" missed the index, so every file reported no importers. One spelling now, scanner.keyPath (Clean then ToSlash), applied where paths enter (walker FileInfo.Path, ast-grep FileAnalysis.Path), where keys are stored (fileIndex, FileGraph.Imports and Importers, precomputed Cargo and Swift edges) and where probes are looked up (fileIndex.exact, fileIndex.dir, suffixMatches, tryExactMatch, normalizeImport). Resolvers keep using filepath for filesystem access. Group B followed: tests expect "/" and the graph now emits "/", so the Rust fallback and Cargo assertions hold on Windows. The MCP normalizeImporterFile returns the same slash key. Group A: TestMain builds codemap_test_binary.exe on Windows so the 34 end-to-end CLI tests can exec it. Refs #214 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01NRaBgKRAZMX9hjbkkefA7T --- main_test.go | 4 ++ mcp/main.go | 3 +- mcp/structured_contract_test.go | 7 ++-- scanner/astgrep.go | 1 + scanner/deps_test.go | 6 +-- scanner/filegraph.go | 72 ++++++++++++++++++++------------- scanner/filegraph_test.go | 30 ++++++++++++-- scanner/jsworkspace.go | 2 +- scanner/rustbuildscript.go | 2 +- scanner/rustgraph.go | 14 +++---- scanner/types.go | 26 ++++++++++++ scanner/walker.go | 2 +- 12 files changed, 122 insertions(+), 47 deletions(-) diff --git a/main_test.go b/main_test.go index f84d07f..41fb096 100644 --- a/main_test.go +++ b/main_test.go @@ -6,6 +6,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "strings" "testing" @@ -21,6 +22,9 @@ func TestMain(m *testing.M) { os.Exit(1) } codemapTestBinaryPath = filepath.Join(tmpDir, "codemap_test_binary") + if runtime.GOOS == "windows" { + codemapTestBinaryPath += ".exe" + } // Build the binary for integration tests cmd := exec.Command("go", "build", "-o", codemapTestBinaryPath, ".") diff --git a/mcp/main.go b/mcp/main.go index 86cf9d6..bb860bb 100644 --- a/mcp/main.go +++ b/mcp/main.go @@ -982,7 +982,8 @@ func normalizeImporterFile(root, file string) string { file = relative } } - return filepath.Clean(file) + // Graph keys are forward-slashed on every OS (scanner.keyPath). + return filepath.ToSlash(filepath.Clean(file)) } // notIndexedText is the MCP answer for a file the graph cannot vouch for. It diff --git a/mcp/structured_contract_test.go b/mcp/structured_contract_test.go index 84f5a92..58b2a2a 100644 --- a/mcp/structured_contract_test.go +++ b/mcp/structured_contract_test.go @@ -89,15 +89,16 @@ func TestStructuredImporterPreservesCLIReportFields(t *testing.T) { } } -func TestNormalizeImporterFileUsesNativeGraphKeys(t *testing.T) { +func TestNormalizeImporterFileUsesSlashGraphKeys(t *testing.T) { root := filepath.Join(string(filepath.Separator)+"repo", "project") - want := filepath.Join("pkg", "types", "types.go") + want := "pkg/types/types.go" for _, input := range []string{ "pkg/types/types.go", + filepath.Join("pkg", "types", "types.go"), filepath.Join(root, "pkg", "types", "types.go"), } { if got := normalizeImporterFile(root, input); got != want { - t.Fatalf("normalizeImporterFile(%q) = %q, want native key %q", input, got, want) + t.Fatalf("normalizeImporterFile(%q) = %q, want slash key %q", input, got, want) } } } diff --git a/scanner/astgrep.go b/scanner/astgrep.go index 5296f5d..ccd94f4 100644 --- a/scanner/astgrep.go +++ b/scanner/astgrep.go @@ -442,6 +442,7 @@ func (s *AstGrepScanner) scanDirectory(parent context.Context, root string) ([]F if relPath == "" { relPath = m.File } + relPath = filepath.ToSlash(relPath) if fileMap[relPath] == nil { lang := detectLangFromRuleID(m.RuleID) diff --git a/scanner/deps_test.go b/scanner/deps_test.go index 7e1efe2..c2ffcb7 100644 --- a/scanner/deps_test.go +++ b/scanner/deps_test.go @@ -536,9 +536,9 @@ func TestDepsNormalizeImport(t *testing.T) { want string }{ {name: "trims quotes", imp: "\"pkg/util\"", want: "pkg/util"}, - {name: "python dotted path", imp: "app.core.config", want: filepath.Join("app", "core", "config")}, - {name: "crate path", imp: "crate::net::http", want: filepath.Join("net", "http")}, - {name: "super path", imp: "super::service::api", want: filepath.Join("super", "service", "api")}, + {name: "python dotted path", imp: "app.core.config", want: "app/core/config"}, + {name: "crate path", imp: "crate::net::http", want: "net/http"}, + {name: "super path", imp: "super::service::api", want: "super/service/api"}, {name: "already slash path", imp: "src/util", want: "src/util"}, } diff --git a/scanner/filegraph.go b/scanner/filegraph.go index 59b7aa5..6aa5404 100644 --- a/scanner/filegraph.go +++ b/scanner/filegraph.go @@ -333,11 +333,12 @@ func buildFileGraphFromAnalysesWithCargoMetadataAndFilters(ctx context.Context, } if len(resolvedImports) > 0 { - fg.Imports[a.Path] = dedupe(resolvedImports) + key := keyPath(a.Path) + fg.Imports[key] = dedupe(slashPaths(resolvedImports)) // Build reverse map - for _, imported := range fg.Imports[a.Path] { - fg.Importers[imported] = append(fg.Importers[imported], a.Path) + for _, imported := range fg.Imports[key] { + fg.Importers[imported] = append(fg.Importers[imported], key) } } } @@ -401,7 +402,7 @@ func addSwiftTypeReferenceEdges(absRoot string, analyses []FileAnalysis, fg *Fil if len(name) < 3 { continue } - declaredBy[name] = append(declaredBy[name], a.Path) + declaredBy[name] = append(declaredBy[name], keyPath(a.Path)) } } if len(declaredBy) == 0 { @@ -417,9 +418,10 @@ func addSwiftTypeReferenceEdges(absRoot string, analyses []FileAnalysis, fg *Fil } seen := make(map[string]bool) var targets []string + from := keyPath(a.Path) for _, ident := range swiftIdentifier.FindAll(swiftImportLine.ReplaceAll(data, nil), -1) { for _, target := range declaredBy[string(ident)] { - if target == a.Path || seen[target] || slices.Contains(fg.Imports[a.Path], target) { + if target == from || seen[target] || slices.Contains(fg.Imports[from], target) { continue } seen[target] = true @@ -430,9 +432,9 @@ func addSwiftTypeReferenceEdges(absRoot string, analyses []FileAnalysis, fg *Fil continue } sort.Strings(targets) - fg.Imports[a.Path] = append(fg.Imports[a.Path], targets...) + fg.Imports[from] = append(fg.Imports[from], targets...) for _, target := range targets { - fg.Importers[target] = append(fg.Importers[target], a.Path) + fg.Importers[target] = append(fg.Importers[target], from) } } return true @@ -460,11 +462,12 @@ func (fg *FileGraph) sortEdges() { func applyPrecomputedFileEdges(fg *FileGraph, edges []fileEdge) { for _, edge := range edges { - if edge.from == "" || edge.to == "" || edge.from == edge.to { + from, to := keyPath(edge.from), keyPath(edge.to) + if from == "" || to == "" || from == to { continue } - fg.Imports[edge.from] = dedupe(append(fg.Imports[edge.from], edge.to)) - fg.Importers[edge.to] = dedupe(append(fg.Importers[edge.to], edge.from)) + fg.Imports[from] = dedupe(append(fg.Imports[from], to)) + fg.Importers[to] = dedupe(append(fg.Importers[to], from)) } } @@ -519,8 +522,8 @@ func buildFileIndexContext(ctx context.Context, files []FileInfo, goModule strin if err := ctx.Err(); err != nil { return nil, err } - path := f.Path - dir := filepath.Dir(path) + path := keyPath(f.Path) + dir := filepath.ToSlash(filepath.Dir(path)) if dir == "." { dir = "" } @@ -559,6 +562,19 @@ func buildFileIndexContext(ctx context.Context, files []FileInfo, goModule strin return idx, nil } +// exact reports how many indexed files have this path, whatever separator +// the probe was built with. +func (idx *fileIndex) exact(p string) uint32 { + return idx.byExact[keyPath(p)] +} + +// dir returns the files directly inside the directory, whatever separator +// the probe was built with. +func (idx *fileIndex) dir(p string) ([]string, bool) { + files, ok := idx.byDir[keyPath(p)] + return files, ok +} + func comparePathsReversed(left, right string) int { limit := min(len(left), len(right)) for i := 0; i < limit; i++ { @@ -584,7 +600,7 @@ func comparePathSuffix(path, suffix string) int { limit := min(len(path), prefixLen) for i := 0; i < limit; i++ { left := path[len(path)-1-i] - right := byte(filepath.Separator) + right := byte('/') if i < len(suffix) { right = suffix[len(suffix)-1-i] } @@ -605,10 +621,11 @@ func comparePathSuffix(path, suffix string) int { } func hasPathSuffix(path, suffix string) bool { - return len(path) > len(suffix) && path[len(path)-len(suffix)-1] == byte(filepath.Separator) && strings.HasSuffix(path, suffix) + return len(path) > len(suffix) && path[len(path)-len(suffix)-1] == '/' && strings.HasSuffix(path, suffix) } func (idx *fileIndex) suffixMatches(suffix string) []string { + suffix = keyPath(suffix) if suffix == "" { return nil } @@ -678,7 +695,7 @@ func fuzzyResolveWithWorkspace( } packagePath := strings.TrimPrefix(strings.TrimPrefix(imp, module.path), "/") packagePath = filepath.Join(module.root, filepath.FromSlash(packagePath)) - files := idx.byDir[packagePath] + files, _ := idx.dir(packagePath) return resolveCUEPackage(files, idx.cuePackages, selector) } @@ -797,16 +814,16 @@ func normalizeImport(imp string) string { // #include "b.h" and require_once 'b.php' became b/h and b/php, so every // same-directory C, C++ and PHP include resolved to nothing. if strings.Contains(imp, ".") && !strings.Contains(imp, "/") && !strings.HasPrefix(imp, ".") && extToLang[strings.ToLower(filepath.Ext(imp))] == "" { - imp = strings.ReplaceAll(imp, ".", string(filepath.Separator)) + imp = strings.ReplaceAll(imp, ".", "/") } // Rust :: to slashes: crate::foo::bar -> foo/bar if strings.HasPrefix(imp, "crate::") { imp = strings.TrimPrefix(imp, "crate::") - imp = strings.ReplaceAll(imp, "::", string(filepath.Separator)) + imp = strings.ReplaceAll(imp, "::", "/") } if strings.HasPrefix(imp, "super::") { - imp = strings.ReplaceAll(imp, "::", string(filepath.Separator)) + imp = strings.ReplaceAll(imp, "::", "/") } return imp @@ -892,7 +909,8 @@ func resolveRelative(imp, fromDir string, idx *fileIndex, sourceLanguage string) // tryExactMatch looks for exact path matches with common extensions. // Extension list derived from the canonical scanner registry. func tryExactMatch(path string, idx *fileIndex, sourceLanguage string) []string { - if idx.byExact[path] == 1 && languagesCompatible(sourceLanguage, DetectLanguage(path)) { + path = keyPath(path) + if idx.exact(path) == 1 && languagesCompatible(sourceLanguage, DetectLanguage(path)) { return []string{path} } // Under ESM and NodeNext, TypeScript requires the specifier to name the @@ -900,13 +918,13 @@ func tryExactMatch(path string, idx *fileIndex, sourceLanguage string) []string // only on-disk counterpart is helper.ts. The specifier already carries an // extension, so appending one below never matches and the edge was lost. for _, candidate := range typescriptSourceCandidates(path, sourceLanguage) { - if idx.byExact[candidate] == 1 && languagesCompatible(sourceLanguage, DetectLanguage(candidate)) { + if idx.exact(candidate) == 1 && languagesCompatible(sourceLanguage, DetectLanguage(candidate)) { return []string{candidate} } } for _, ext := range resolverExtensions[:len(resolverExtensions)-1] { candidate := path + ext - if idx.byExact[candidate] == 1 && languagesCompatible(sourceLanguage, DetectLanguage(candidate)) { + if idx.exact(candidate) == 1 && languagesCompatible(sourceLanguage, DetectLanguage(candidate)) { return []string{candidate} } } @@ -922,17 +940,17 @@ func tryExactMatch(path string, idx *fileIndex, sourceLanguage string) []string // set, so this on-disk check recovers exactly that edge without // widening the fuzzy/suffix resolution strategies to gitignored files. if idx.absRoot != "" { - if idx.byExact[path] == 0 && languagesCompatible(sourceLanguage, DetectLanguage(path)) && idx.gitignoreOnlyRecoverable(path) { + if idx.exact(path) == 0 && languagesCompatible(sourceLanguage, DetectLanguage(path)) && idx.gitignoreOnlyRecoverable(path) { return []string{path} } for _, candidate := range typescriptSourceCandidates(path, sourceLanguage) { - if idx.byExact[candidate] == 0 && languagesCompatible(sourceLanguage, DetectLanguage(candidate)) && idx.gitignoreOnlyRecoverable(candidate) { + if idx.exact(candidate) == 0 && languagesCompatible(sourceLanguage, DetectLanguage(candidate)) && idx.gitignoreOnlyRecoverable(candidate) { return []string{candidate} } } for _, ext := range resolverExtensions[:len(resolverExtensions)-1] { candidate := path + ext - if idx.byExact[candidate] == 0 && languagesCompatible(sourceLanguage, DetectLanguage(candidate)) && idx.gitignoreOnlyRecoverable(candidate) { + if idx.exact(candidate) == 0 && languagesCompatible(sourceLanguage, DetectLanguage(candidate)) && idx.gitignoreOnlyRecoverable(candidate) { return []string{candidate} } } @@ -1034,7 +1052,7 @@ func trySuffixMatch(normalized string, idx *fileIndex, sourceLanguage string) [] // (e.g. "MyApp/Models" tries "MyApp/Models" first, then "Models"). func tryDirMatch(path string, idx *fileIndex, sourceLanguage string) []string { // Try exact match first - if files, ok := idx.byDir[path]; ok { + if files, ok := idx.dir(path); ok { if compatible := compatibleFiles(sourceLanguage, files); len(compatible) > 0 { return compatible } @@ -1042,7 +1060,7 @@ func tryDirMatch(path string, idx *fileIndex, sourceLanguage string) []string { // Normalize path separators for cross-platform compatibility nativePath := filepath.FromSlash(path) if nativePath != path { - if files, ok := idx.byDir[nativePath]; ok { + if files, ok := idx.dir(nativePath); ok { if compatible := compatibleFiles(sourceLanguage, files); len(compatible) > 0 { return compatible } @@ -1053,7 +1071,7 @@ func tryDirMatch(path string, idx *fileIndex, sourceLanguage string) []string { parts := strings.Split(filepath.ToSlash(path), "/") for i := 1; i < len(parts); i++ { suffix := filepath.Join(parts[i:]...) - if files, ok := idx.byDir[suffix]; ok { + if files, ok := idx.dir(suffix); ok { if compatible := compatibleFiles(sourceLanguage, files); len(compatible) > 0 { return compatible } diff --git a/scanner/filegraph_test.go b/scanner/filegraph_test.go index 67a155c..aefc66b 100644 --- a/scanner/filegraph_test.go +++ b/scanner/filegraph_test.go @@ -180,12 +180,12 @@ func TestNormalizeImport(t *testing.T) { expected string }{ {name: "removes quotes", input: "\"pkg/util\"", expected: "pkg/util"}, - {name: "python dots to slashes", input: "app.core.config", expected: filepath.Join("app", "core", "config")}, + {name: "python dots to slashes", input: "app.core.config", expected: "app/core/config"}, {name: "relative dotted import unchanged", input: "../app.core", expected: "../app.core"}, {name: "c header keeps its extension", input: "\"b.h\"", expected: "b.h"}, {name: "php require keeps its extension", input: "'b.php'", expected: "b.php"}, - {name: "crate import converted", input: "crate::feature::parser", expected: filepath.Join("feature", "parser")}, - {name: "super import converted", input: "super::module::item", expected: filepath.Join("super", "module", "item")}, + {name: "crate import converted", input: "crate::feature::parser", expected: "feature/parser"}, + {name: "super import converted", input: "super::module::item", expected: "super/module/item"}, } for _, tt := range tests { @@ -1166,3 +1166,27 @@ func TestGitignoredExactRelativeImportTargetStillResolves(t *testing.T) { t.Fatalf("root copy importers = %v, want none (the import names the adgen copy specifically)", got) } } + +// A Windows walker hands the index native paths while every import is +// written with "/"; before keyPath the index key and the probe never agreed +// and every file reported no importers. FromSlash makes the probe native on +// Windows and leaves it alone elsewhere, so this passes on both and would +// have failed on Windows before the fix. +func TestIndexMatchesNativeProbeAgainstSlashKeys(t *testing.T) { + idx := buildFileIndex([]FileInfo{ + {Path: "core/src/lib.rs"}, + {Path: filepath.FromSlash("core/src/util.rs")}, + }, "") + for _, probe := range []string{"core/src/lib", filepath.FromSlash("core/src/lib")} { + got := tryExactMatch(probe, idx, "rust") + if len(got) != 1 || got[0] != "core/src/lib.rs" { + t.Fatalf("tryExactMatch(%q) = %v, want [core/src/lib.rs]", probe, got) + } + } + if got, ok := idx.dir(filepath.FromSlash("core/src")); !ok || len(got) != 2 || got[0] != "core/src/lib.rs" || got[1] != "core/src/util.rs" { + t.Fatalf("dir(core/src) = %v, %v; want both files with slash keys", got, ok) + } + if got := idx.suffixMatches(filepath.FromSlash("src/util.rs")); len(got) != 1 || got[0] != "core/src/util.rs" { + t.Fatalf("suffixMatches = %v, want [core/src/util.rs]", got) + } +} diff --git a/scanner/jsworkspace.go b/scanner/jsworkspace.go index f33d3b7..85dfd34 100644 --- a/scanner/jsworkspace.go +++ b/scanner/jsworkspace.go @@ -415,7 +415,7 @@ func resolveManifestTarget(root, target string, idx *fileIndex, sourceLanguage s if !ok { return nil } - if idx.byExact[localPath] == 1 && languagesCompatible(sourceLanguage, DetectLanguage(localPath)) { + if idx.exact(localPath) == 1 && languagesCompatible(sourceLanguage, DetectLanguage(localPath)) { return []string{localPath} } return nil diff --git a/scanner/rustbuildscript.go b/scanner/rustbuildscript.go index 3a68569..139b93f 100644 --- a/scanner/rustbuildscript.go +++ b/scanner/rustbuildscript.go @@ -42,7 +42,7 @@ func resolveRustBuildScriptInput(fromFile, input string, idx *fileIndex, workspa return "" } // Duplicate inventory entries make ownership ambiguous. - if idx.byExact[candidate] != 1 { + if idx.exact(candidate) != 1 { return "" } return candidate diff --git a/scanner/rustgraph.go b/scanner/rustgraph.go index fe422ac..95407de 100644 --- a/scanner/rustgraph.go +++ b/scanner/rustgraph.go @@ -516,7 +516,7 @@ func resolveRustInclude(root, declaringFile, literal string, idx *fileIndex) str if target == "" { return "" } - if idx.byExact[target] != 1 { + if idx.exact(target) != 1 { return "" } return target @@ -527,7 +527,7 @@ func resolveRustEmbeddedFile(root, declaringFile, literal string, idx *fileIndex if target == "" { return "" } - if idx.byExact[target] != 1 { + if idx.exact(target) != 1 { return "" } return target @@ -654,7 +654,7 @@ func resolveRustAskamaTemplate(root, fromFile, literal string, idx *fileIndex, w return "" } // Duplicate inventory entries make ownership ambiguous. - if idx.byExact[target] != 1 { + if idx.exact(target) != 1 { return "" } return target @@ -723,7 +723,7 @@ func resolveRustModule(name, fromFile string, idx *fileIndex, workspace *rustWor filepath.Join(dir, name+".rs"), filepath.Join(dir, name, "mod.rs"), } { - if idx.byExact[candidate] == 1 { + if idx.exact(candidate) == 1 { return candidate } } @@ -891,7 +891,7 @@ func resolveRustPathFromDirectory(base string, parts []string, idx *fileIndex) s for i := len(parts); i > 0; i-- { modulePath := filepath.Join(append([]string{base}, parts[:i]...)...) for _, candidate := range []string{modulePath + ".rs", filepath.Join(modulePath, "mod.rs")} { - if idx.byExact[candidate] == 1 { + if idx.exact(candidate) == 1 { return candidate } } @@ -975,7 +975,7 @@ func (index *rustWorkspaceIndex) targetForFile(path string, idx *fileIndex) (rus } func rustTargetIndexed(target rustTarget, idx *fileIndex) bool { - return idx.byExact[target.rootFile] == 1 + return idx.exact(target.rootFile) == 1 } func pathWithin(path, dir string) bool { @@ -1022,7 +1022,7 @@ func (index *rustWorkspaceIndex) targetContainsFile(target rustTarget, path stri modulePath := filepath.Join(append([]string{target.sourceDir}, parts[:i+1]...)...) var next string for _, candidate := range []string{modulePath + ".rs", filepath.Join(modulePath, "mod.rs")} { - if idx.byExact[candidate] == 1 { + if idx.exact(candidate) == 1 { if next != "" { return false } diff --git a/scanner/types.go b/scanner/types.go index 39c7a6b..767e674 100644 --- a/scanner/types.go +++ b/scanner/types.go @@ -371,6 +371,32 @@ var LangDisplay = map[string]string{ } // dedupe removes duplicate strings from a slice +// keyPath is the one spelling of a project-relative path that the file +// graph, the file index, and every resolver agree on: cleaned and +// forward-slashed. filepath.Clean, Join and Dir emit the native separator, +// so on Windows a key built by one of them never matched an import written +// with "/" and every file reported no importers. Normalise at the boundary, +// on both the stored key and the probe, and the rest of the code can keep +// using filepath for filesystem access. +func keyPath(p string) string { + if p == "" { + return "" + } + return filepath.ToSlash(filepath.Clean(p)) +} + +// slashPaths returns keyPath applied to every element. +func slashPaths(items []string) []string { + if len(items) == 0 { + return items + } + out := make([]string, len(items)) + for i, item := range items { + out[i] = keyPath(item) + } + return out +} + func dedupe(items []string) []string { seen := make(map[string]bool) result := make([]string, 0, len(items)) diff --git a/scanner/walker.go b/scanner/walker.go index a69bbf9..b6ea933 100644 --- a/scanner/walker.go +++ b/scanner/walker.go @@ -393,7 +393,7 @@ func ScanFiles(ctx context.Context, root string, cache *GitIgnoreCache, only []s } files = append(files, FileInfo{ - Path: relPath, + Path: filepath.ToSlash(relPath), Size: info.Size(), Ext: ext, })