From 23a0846d9a95f8de9ebdfe4e7425a5959288d4e3 Mon Sep 17 00:00:00 2001 From: Nick Sheth Date: Thu, 24 Sep 2026 14:59:18 -0700 Subject: [PATCH 1/2] fix(coverage): distinguish "never indexed" from "zero importers" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --importers reported "No files import X. Coverage: complete" for a file that was never part of the scanned file set at all (e.g. it lives under an excluded or hidden directory, or the path doesn't exist) — identical output to a file that was genuinely scanned and truly has no importers. The project-level coverage status (complete/partial/ unavailable) describes whether the *scan as a whole* succeeded; it says nothing about whether one particular queried file was in scope, so an authoritative, complete scan could still turn a blind spot on a single file into a confidently wrong answer. FileGraph gains KnownFiles, the set of files actually in the scanned inventory (populated from the same file list used to build the fuzzy import resolver), and an Indexed(path) query against it. ImportersReport gains NotIndexed (JSON: not_indexed, omitempty), defaulting to false so any report built without this field in mind — including existing callers/tests that construct FileGraph or ImportersReport by hand — keeps meaning what it always meant. buildImportersReportFromGraph sets it from fg.Indexed(file); coverage status/notes describing the overall scan are left untouched. renderImportersReportCLI now prints ": not indexed (under a hidden or excluded folder, or the path doesn't exist)" instead of "No files import " when NotIndexed is set — quoted here per CONTRIBUTING's output-standard review rule: sourced (names the file), precise (no hedge words), bounded (says outright that this isn't a confirmed zero-importers result), useful (tells the reader which two things to check). The token-budgeted blast-radius bundle renderer is unchanged; NotIndexed is still available on the report for any consumer that wants it (JSON output already surfaces it). Added TestBuildImportersReportFromGraphMarksNotIndexedFile (unit, FileGraph with a partial KnownFiles set) and TestImportersDistinguishesNotIndexedFromZeroImporters (end-to-end via the built binary: a node_modules/ file reads as not indexed, a real zero-importer file still reads as a plain negative). --- blast_radius.go | 20 ++++++++ blast_radius_fixes_test.go | 93 ++++++++++++++++++++++++++++++++++++++ scanner/filegraph.go | 21 +++++++++ scanner/types.go | 26 +++++++---- 4 files changed, 152 insertions(+), 8 deletions(-) diff --git a/blast_radius.go b/blast_radius.go index aa60def..7c88616 100644 --- a/blast_radius.go +++ b/blast_radius.go @@ -1367,6 +1367,16 @@ func renderImportersReportString(report scanner.ImportersReport) string { // this command and deserves an explanation when the answer is "none". func renderImportersReportCLI(w io.Writer, report scanner.ImportersReport) { if len(report.Importers) == 0 && len(report.HubImports) == 0 { + if report.NotIndexed { + // Distinct from "scanned and found zero importers": codemap never + // looked at this file, so a bare "No files import X" here would be + // indistinguishable from a genuine negative and read as a + // confident answer instead of a blind spot. + fmt.Fprintf(w, "%s: not indexed (under a hidden or excluded folder, or the path doesn't exist).\n", report.File) + fmt.Fprintln(w, " codemap never scanned this file, so this is not a confirmed zero-importers result.") + renderCoverage(w, report.CoverageStatus, report.CoverageNotes) + return + } fmt.Fprintf(w, "No files import %s.\n", report.File) if strings.EqualFold(filepath.Ext(report.File), ".go") { fmt.Fprintln(w, " Note: files in the same package never import each other (Go resolves") @@ -1448,6 +1458,15 @@ func buildImportersReportFromGraph(root, file string, fg *scanner.FileGraph) (sc importers := append([]string(nil), fg.Importers[file]...) imports := append([]string(nil), fg.Imports[file]...) + // NotIndexed distinguishes "codemap scanned this file and it genuinely + // has zero importers" from "codemap never looked at this file at all" + // (e.g. it lives under a hidden or excluded directory, or the path + // doesn't exist). CoverageStatus/CoverageNotes below still describe the + // *scan as a whole* (complete/partial/unavailable) and are left as-is: + // an authoritative, complete scan can still miss one particular file + // simply because that file was outside its scanned set, which is + // exactly the gap NotIndexed exists to surface without redefining what + // "complete" means for the project overall. report := scanner.ImportersReport{ Root: root, Mode: "importers", @@ -1456,6 +1475,7 @@ func buildImportersReportFromGraph(root, file string, fg *scanner.FileGraph) (sc Imports: imports, ImporterCount: len(importers), IsHub: fg.IsHub(file), + NotIndexed: !fg.Indexed(file), CoverageStatus: string(fg.Coverage.EffectiveStatus()), CoverageNotes: append([]string(nil), fg.Coverage.Notes...), } diff --git a/blast_radius_fixes_test.go b/blast_radius_fixes_test.go index 042591f..8ae67b1 100644 --- a/blast_radius_fixes_test.go +++ b/blast_radius_fixes_test.go @@ -279,6 +279,54 @@ func TestBuildImportersReportFromGraphCarriesCoverage(t *testing.T) { } } +// TestBuildImportersReportFromGraphMarksNotIndexedFile is the regression for +// the "false complete coverage" bug: a file that was never part of the +// scanned inventory (KnownFiles) must be reported as not indexed rather than +// as a confirmed zero-importers result, and that must be visible both to +// JSON/machine consumers (NotIndexed) and to a human running --importers +// (renderImportersReportCLI's message), while a file that genuinely was +// scanned and has zero importers must keep reading as a plain negative. +func TestBuildImportersReportFromGraphMarksNotIndexedFile(t *testing.T) { + fg := &scanner.FileGraph{ + Imports: map[string][]string{}, + Importers: map[string][]string{}, + KnownFiles: map[string]bool{"lonely.js": true}, + } + + unindexed, err := buildImportersReportFromGraph("/repo", "node_modules/dep.js", fg) + if err != nil { + t.Fatalf("buildImportersReportFromGraph() error: %v", err) + } + if !unindexed.NotIndexed { + t.Fatalf("report.NotIndexed = false, want true for a file absent from KnownFiles") + } + if got := renderImportersReportString(unindexed); strings.Contains(got, "No files import") { + t.Fatalf("bundle renderer must not claim a confirmed zero-importers result for an unindexed file:\n%s", got) + } + var buf strings.Builder + renderImportersReportCLI(&buf, unindexed) + cliOut := buf.String() + if !strings.Contains(cliOut, "not indexed") { + t.Fatalf("CLI output should say the file was not indexed, got:\n%s", cliOut) + } + if strings.Contains(cliOut, "No files import") { + t.Fatalf("CLI output must not read as a confirmed zero-importers result:\n%s", cliOut) + } + + indexed, err := buildImportersReportFromGraph("/repo", "lonely.js", fg) + if err != nil { + t.Fatalf("buildImportersReportFromGraph() error: %v", err) + } + if indexed.NotIndexed { + t.Fatalf("report.NotIndexed = true, want false for a file present in KnownFiles") + } + buf.Reset() + renderImportersReportCLI(&buf, indexed) + if !strings.Contains(buf.String(), "No files import lonely.js") { + t.Fatalf("a genuinely scanned, zero-importer file should read as a plain negative, got:\n%s", buf.String()) + } +} + func TestBuildImportersReportFromGraphRejectsFileOutsideRoot(t *testing.T) { root := t.TempDir() outside := t.TempDir() @@ -437,3 +485,48 @@ func TestCoverageNotesFromSources(t *testing.T) { t.Fatalf("coverageNotesFromSources() = %#v, want %#v", notes, want) } } + +// TestImportersDistinguishesNotIndexedFromZeroImporters is the end-to-end +// regression for issue #2 in the bug report: codemap answered "No files +// import X. Coverage: complete" for a file it had never actually indexed +// (here, one under node_modules/, which the walker has always excluded), +// indistinguishable from a genuine zero-importers result. It must now say +// the file was not indexed, while a real, scanned, zero-importer file keeps +// reading as a plain negative. +func TestImportersDistinguishesNotIndexedFromZeroImporters(t *testing.T) { + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, ".git"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(root, "node_modules"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "node_modules", "dep.js"), []byte("module.exports = {};\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "lonely.js"), []byte("module.exports = {};\n"), 0o644); err != nil { + t.Fatal(err) + } + + notIndexedOut, err := runRootOptionsBinary(root, "--importers", "node_modules/dep.js") + if err != nil { + t.Fatalf("codemap --importers node_modules/dep.js failed: %v\n%s", err, notIndexedOut) + } + if !strings.Contains(notIndexedOut, "not indexed") { + t.Fatalf("expected a not-indexed answer for an excluded file, got:\n%s", notIndexedOut) + } + if strings.Contains(notIndexedOut, "No files import") { + t.Fatalf("an unindexed file must not be reported as a confirmed zero-importers result:\n%s", notIndexedOut) + } + + zeroImportersOut, err := runRootOptionsBinary(root, "--importers", "lonely.js") + if err != nil { + t.Fatalf("codemap --importers lonely.js failed: %v\n%s", err, zeroImportersOut) + } + if !strings.Contains(zeroImportersOut, "No files import lonely.js") { + t.Fatalf("a genuinely scanned, zero-importer file should read as a plain negative, got:\n%s", zeroImportersOut) + } + if strings.Contains(zeroImportersOut, "not indexed") { + t.Fatalf("a genuinely scanned file must not be reported as not indexed:\n%s", zeroImportersOut) + } +} diff --git a/scanner/filegraph.go b/scanner/filegraph.go index 7e60f4c..dab0345 100644 --- a/scanner/filegraph.go +++ b/scanner/filegraph.go @@ -26,6 +26,23 @@ type FileGraph struct { PathAliases map[string][]string // TS/JS path aliases from tsconfig.json (e.g., "@modules/*" -> ["src/modules/*"]) BaseURL string // TS/JS baseUrl from tsconfig.json Coverage GraphCoverage + // KnownFiles is every file this graph actually scanned (post filters), + // keyed by slash-separated path relative to Root. A path absent here was + // never indexed at all — e.g. it lives under an excluded or hidden + // directory — which is a distinct, worse state than "indexed and found + // to have zero importers": see Indexed. + KnownFiles map[string]bool +} + +// Indexed reports whether path (slash-separated, relative to fg.Root) was +// part of this graph's scanned inventory. A false result means codemap never +// looked at the file at all, so an empty Importers/Imports entry for it is a +// coverage gap, not a confirmed zero. +func (fg *FileGraph) Indexed(path string) bool { + if fg == nil { + return false + } + return fg.KnownFiles[path] } // fileIndex provides fast lookup of files by various import-like keys @@ -132,6 +149,10 @@ func buildFileGraphFromAnalysesWithCargoMetadataAndFilters(ctx context.Context, } } } + fg.KnownFiles = make(map[string]bool, len(files)) + for _, file := range files { + fg.KnownFiles[filepath.ToSlash(file.Path)] = true + } languages := inspectFileLanguages(files) hasCUEAnalysis := analysisLanguages.hasCUE if languages.hasCUE && !hasCUEAnalysis { diff --git a/scanner/types.go b/scanner/types.go index ee834be..3871f2f 100644 --- a/scanner/types.go +++ b/scanner/types.go @@ -175,14 +175,24 @@ func newDepsProjectWithFilters(root string, files []FileAnalysis, externalDeps m // ImportersReport is the JSON output for --importers mode. type ImportersReport struct { - Root string `json:"root"` - Mode string `json:"mode"` - File string `json:"file"` - Importers []string `json:"importers"` - Imports []string `json:"imports,omitempty"` - HubImports []string `json:"hub_imports,omitempty"` - ImporterCount int `json:"importer_count"` - IsHub bool `json:"is_hub"` + Root string `json:"root"` + Mode string `json:"mode"` + File string `json:"file"` + Importers []string `json:"importers"` + Imports []string `json:"imports,omitempty"` + HubImports []string `json:"hub_imports,omitempty"` + ImporterCount int `json:"importer_count"` + IsHub bool `json:"is_hub"` + // NotIndexed is true when the queried file was never part of the scanned + // file set at all — e.g. it lives under a hidden or excluded directory, + // or the path doesn't exist — as opposed to being scanned and genuinely + // having zero importers. A zero-importer answer for an unindexed file is + // a coverage gap, not a confirmed negative, so callers should check this + // before treating an empty Importers list as meaningful. It defaults to + // false (assume indexed) so any report built without this field in mind — + // including plenty of existing callers and tests — keeps meaning what it + // always meant. + NotIndexed bool `json:"not_indexed,omitempty"` CoverageStatus string `json:"coverage_status"` CoverageNotes []string `json:"coverage_notes,omitempty"` } From 67b929953601121d392577bc652d7b9761d5fe56 Mon Sep 17 00:00:00 2001 From: Nick Sheth Date: Thu, 24 Sep 2026 15:11:18 -0700 Subject: [PATCH 2/2] fix(scanner): resolve a plain relative import whose target is gitignored MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Isolated the reported drop (adgen/reviewAdapters/helpers.js: a plain require('./reviewAdapters/helpers') showing no importers, while an identical copy at the repo root showed its real 16) by testing .gitignore and the file's read-only permission bit independently: a read-only, tracked file resolved correctly; only adding .gitignore for the target reproduced the drop. Permissions were not the trigger. Root cause: ScanFiles applies the project's .gitignore when building the file inventory, and that same inventory is what the import resolver's fileIndex is built from. So a file's presence for "does the project have this source file" and for "can an import land on this file" were the same gitignore-filtered set — a tracked file's ordinary, unambiguous require() of an untracked-but-present sibling had nowhere to resolve to, and silently dropped the edge. tryExactMatch (scanner/filegraph.go) gains a narrow fallback: when its normal exact-match lookup misses (the candidate has zero occurrences in the scanned inventory, as opposed to the ambiguous 2+ case already handled), it checks the candidate against disk directly. It only does this when the candidate would otherwise have passed the project's --only/--exclude filters and isn't under one of the scanner's own never-source directory names (IgnoredDirs: node_modules, vendor, testdata, ...) — both stay absolutely excluded, matching the existing TestFilteredImportResolutionTargets invariant, which is exactly the test this first caught: an initial version of the fallback resolved a file blocked by an explicit --exclude filter too, which it must not. Only .gitignore's blind spot is recovered. fileIndex gains absRoot and filters fields (both required to enable the fallback; existing callers that build an index from bare FileInfo values with no real directory, i.e. nearly every scanner test, pass "" and get the old behavior unchanged). FileGraph.KnownFiles is backfilled for any import target that ends up with a resolved importer, so a file recovered this way correctly reads as indexed rather than contradicting itself by claiming a resolved edge while also saying "not indexed." Added TestGitignoreOnlyRecoverable (fast, ast-grep-independent unit test of tryExactMatch's fallback and its --exclude/IgnoredDirs guards) and TestGitignoredExactRelativeImportTargetStillResolves (end-to-end via BuildFileGraph, reproducing Nick's fixture almost exactly, including the read-only permission bit, to confirm it's inert here). go test ./... passes, including TestFilteredImportResolutionTargets. --- scanner/filegraph.go | 89 ++++++++++++++++++++++-- scanner/filegraph_test.go | 110 ++++++++++++++++++++++++++++++ scanner/hotpath_benchmark_test.go | 2 +- 3 files changed, 196 insertions(+), 5 deletions(-) diff --git a/scanner/filegraph.go b/scanner/filegraph.go index dab0345..334d32f 100644 --- a/scanner/filegraph.go +++ b/scanner/filegraph.go @@ -53,6 +53,17 @@ type fileIndex struct { goPkgs map[string][]string // Go package path -> files cueModules []cueModuleInfo cuePackages map[string]string + // absRoot and filters back a narrow fallback in tryExactMatch: a file + // excluded from the scanned inventory purely by .gitignore can still be + // the exact, unambiguous target of a plain relative import from a file + // that *was* scanned. absRoot empty disables the fallback entirely + // (e.g. tests that build a fileIndex from bare FileInfo values with no + // real directory behind them). filters is still enforced in that + // fallback so it recovers gitignore's blind spot without also + // resurrecting a file the project or caller explicitly excluded via + // --only/--exclude or config.json, which must stay absolutely excluded. + absRoot string + filters Filters } // BuildFileGraph scans a project with explicit filters and builds its file @@ -180,7 +191,7 @@ func buildFileGraphFromAnalysesWithCargoMetadataAndFilters(ctx context.Context, } // Build file index for fast fuzzy matching - idx, err := buildFileIndexContext(ctx, files, fg.Module) + idx, err := buildFileIndexContext(ctx, files, fg.Module, absRoot, filters) if err != nil { return nil, err } @@ -282,6 +293,17 @@ func buildFileGraphFromAnalysesWithCargoMetadataAndFilters(ctx context.Context, resolved[symbolLevelImportLanguages["swift"]] = true } fg.Coverage.addSymbolLevelImportCoverage(languages.symbolLevel, resolved) + // A file resolved via tryExactMatch's on-disk fallback (above) is, by + // construction, real and present — it just wasn't part of the scanned + // inventory KnownFiles was built from (e.g. gitignored). Now that it has + // a proven importer, it should read as indexed rather than "not + // indexed": the whole point of the fallback is to turn a dropped edge + // into a correct one, and leaving KnownFiles stale would have the + // importers report claim the edge while simultaneously claiming the + // target was never looked at. + for imported := range fg.Importers { + fg.KnownFiles[filepath.ToSlash(imported)] = true + } fg.sortEdges() return fg, nil } @@ -405,19 +427,24 @@ func inspectAnalysisLanguages(analyses []FileAnalysis) analysisLanguageInventory return inventory } -// buildFileIndex creates a multi-key index for fast import resolution +// buildFileIndex creates a multi-key index for fast import resolution. It has +// no project root behind it (callers pass bare FileInfo values, often in +// tests), so the on-disk fallback in tryExactMatch stays disabled; use +// buildFileIndexContext directly to enable it. func buildFileIndex(files []FileInfo, goModule string) *fileIndex { - idx, _ := buildFileIndexContext(context.Background(), files, goModule) + idx, _ := buildFileIndexContext(context.Background(), files, goModule, "", Filters{}) return idx } -func buildFileIndexContext(ctx context.Context, files []FileInfo, goModule string) (*fileIndex, error) { +func buildFileIndexContext(ctx context.Context, files []FileInfo, goModule string, absRoot string, filters Filters) (*fileIndex, error) { directoryHint := min(len(files), 1024) idx := &fileIndex{ byExact: make(map[string]uint32, len(files)), bySuffix: make([]string, 0, len(files)), byDir: make(map[string][]string, directoryHint), goPkgs: make(map[string][]string, directoryHint), + absRoot: absRoot, + filters: filters, } goPackagePaths := make(map[string]string, directoryHint) @@ -814,9 +841,63 @@ func tryExactMatch(path string, idx *fileIndex, sourceLanguage string) []string } } + // A candidate absent from the scanned inventory (count 0, as opposed to + // the ambiguous count >= 2 case above, which is deliberately left + // unresolved) may still be a real, readable file that the scan excluded + // for an orthogonal reason — most commonly .gitignore, since ScanFiles + // applies the project's gitignore rules to build the inventory this + // index is drawn from. A plain relative require('./x') naming an exact, + // unambiguous path is confident evidence the file is genuinely part of + // the dependency graph even though it isn't part of the scanned file + // 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) { + return []string{path} + } + for _, candidate := range typescriptSourceCandidates(path, sourceLanguage) { + if idx.byExact[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) { + return []string{candidate} + } + } + } + return nil } +// gitignoreOnlyRecoverable reports whether relPath is missing from the +// scanned inventory *solely* because of .gitignore, as opposed to an +// explicit --only/--exclude filter, a config.json exclude, or one of the +// scanner's own never-source directory names (node_modules, vendor, +// testdata, ...) — all of which must stay absolutely excluded, unlike +// .gitignore's blind spot. ScanFiles' only exclusion mechanisms are those +// three plus .gitignore, so a real, regular file that clears the other two +// and is still absent can only be missing because of .gitignore. +func (idx *fileIndex) gitignoreOnlyRecoverable(relPath string) bool { + if !MatchesFilters(filepath.ToSlash(relPath), filepath.Ext(relPath), idx.filters.Only, idx.filters.Exclude) { + return false + } + for dir := filepath.ToSlash(filepath.Dir(relPath)); dir != "." && dir != "/" && dir != ""; dir = pathpkg.Dir(dir) { + if IgnoredDirs[pathpkg.Base(dir)] { + return false + } + } + return isRegularFile(idx.absRoot, relPath) +} + +// isRegularFile reports whether root-relative path exists on disk under +// absRoot as a regular (not directory, not symlink-to-directory) file. +func isRegularFile(absRoot, relPath string) bool { + info, err := os.Stat(filepath.Join(absRoot, filepath.FromSlash(relPath))) + return err == nil && info.Mode().IsRegular() +} + // typescriptEmitExtensions maps an emitted JavaScript extension to the // TypeScript sources that produce it, in the order the compiler prefers. // diff --git a/scanner/filegraph_test.go b/scanner/filegraph_test.go index 3524657..cc044ba 100644 --- a/scanner/filegraph_test.go +++ b/scanner/filegraph_test.go @@ -1047,3 +1047,113 @@ func TestFuzzyResolveCsharpNamespace(t *testing.T) { } }) } + +// TestGitignoreOnlyRecoverable is a fast, ast-grep-independent unit test for +// the exact-match on-disk fallback: a plain relative require/import naming a +// file that ScanFiles excluded purely by .gitignore must still resolve, +// while the same on-disk file must stay unresolvable when an explicit +// --exclude/--only filter or one of the scanner's own never-source +// directory names (IgnoredDirs) is *also* in play — those exclusions are +// deliberate and must not be reversed by this recovery path. +func TestGitignoreOnlyRecoverable(t *testing.T) { + root := t.TempDir() + write := func(rel, content string) { + t.Helper() + full := filepath.Join(root, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + write("pkg/adapters/helper.js", "module.exports = {}\n") + write("blocked/target.js", "module.exports = {}\n") + write("node_modules/vendored.js", "module.exports = {}\n") + write(".gitignore", "pkg/adapters/helper.js\nblocked/target.js\nnode_modules/\n") + + cache := NewGitIgnoreCache(root) + scanned, err := ScanFiles(context.Background(), root, cache, nil, []string{filepath.FromSlash("blocked/target.js")}) + if err != nil { + t.Fatalf("ScanFiles() error: %v", err) + } + for _, gitignored := range []string{"pkg/adapters/helper.js", "blocked/target.js", "node_modules/vendored.js"} { + for _, file := range scanned { + if filepath.ToSlash(file.Path) == gitignored { + t.Fatalf("fixture setup: %s should have been excluded from ScanFiles by .gitignore, got it in inventory", gitignored) + } + } + } + + idx, err := buildFileIndexContext(context.Background(), scanned, "", root, Filters{Exclude: []string{filepath.FromSlash("blocked/target.js")}}) + if err != nil { + t.Fatalf("buildFileIndexContext() error: %v", err) + } + + if got := tryExactMatch("pkg/adapters/helper", idx, "javascript"); len(got) != 1 || got[0] != filepath.FromSlash("pkg/adapters/helper.js") { + t.Errorf("gitignore-only exclusion should still resolve, got %v", got) + } + if got := tryExactMatch("blocked/target", idx, "javascript"); got != nil { + t.Errorf("an explicitly --exclude'd path must stay unresolved even though it's also gitignored and present on disk, got %v", got) + } + if got := tryExactMatch("node_modules/vendored", idx, "javascript"); got != nil { + t.Errorf("a path under an IgnoredDirs-named directory must stay unresolved even though it's present on disk, got %v", got) + } +} + +// TestGitignoredExactRelativeImportTargetStillResolves is the end-to-end +// regression for the reported bug (Nick's adgen/reviewAdapters/helpers.js +// case): a plain relative require('./x') whose target is gitignored had its +// importer edge silently dropped, indistinguishable from the target never +// having existed. Root cause, isolated by testing gitignore and read-only +// permissions independently: it was .gitignore excluding the target from +// ScanFiles' inventory (which the import-resolution index is built from), +// not the read-only permission bit — a read-only, non-gitignored copy +// resolved correctly, and only adding .gitignore reproduced the drop. +func TestGitignoredExactRelativeImportTargetStillResolves(t *testing.T) { + if !NewAstGrepAnalyzer().Available() { + t.Skip("ast-grep not available") + } + + root := t.TempDir() + files := map[string]string{ + "helpers.js": "module.exports = { hello: () => 'root-hi' }\n", + "adgen/src/services/quoteRotationService.js": "const helpers = require('./reviewAdapters/helpers');\nmodule.exports = { helpers };\n", + "adgen/src/services/reviewAdapters/helpers.js": "module.exports = { hello: () => 'hi' }\n", + ".gitignore": "adgen/src/services/reviewAdapters/helpers.js\n", + } + for path, content := range files { + full := filepath.Join(root, filepath.FromSlash(path)) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + // Read-only, matching the reported fixture exactly; TestGitignoreOnlyRecoverable + // already isolates that permissions alone are not the trigger. + if err := os.Chmod(filepath.Join(root, filepath.FromSlash("adgen/src/services/reviewAdapters/helpers.js")), 0o444); err != nil { + t.Fatal(err) + } + + graph, err := BuildFileGraph(context.Background(), root, Filters{}) + if err != nil { + t.Fatalf("BuildFileGraph() error: %v", err) + } + + target := filepath.FromSlash("adgen/src/services/reviewAdapters/helpers.js") + source := filepath.FromSlash("adgen/src/services/quoteRotationService.js") + got := graph.Importers[target] + if len(got) != 1 || got[0] != source { + t.Fatalf("importers of gitignored %s = %v, want exactly [%s]", target, got, source) + } + if !graph.Indexed(filepath.ToSlash(target)) { + t.Fatalf("%s has a resolved importer, so it must read as indexed, not as a coverage gap", target) + } + // The root copy is a different file at a different path; nothing should + // alias the gitignored copy's importer onto it. + if got := graph.Importers["helpers.js"]; len(got) != 0 { + t.Fatalf("root copy importers = %v, want none (the import names the adgen copy specifically)", got) + } +} diff --git a/scanner/hotpath_benchmark_test.go b/scanner/hotpath_benchmark_test.go index c99d44e..ceca2ab 100644 --- a/scanner/hotpath_benchmark_test.go +++ b/scanner/hotpath_benchmark_test.go @@ -95,7 +95,7 @@ func benchmarkBuildFileIndex(b *testing.B, files []FileInfo) { b.ResetTimer() for range b.N { var err error - benchmarkFileIndex, err = buildFileIndexContext(context.Background(), files, "example.com/bench") + benchmarkFileIndex, err = buildFileIndexContext(context.Background(), files, "example.com/bench", "", Filters{}) if err != nil { b.Fatal(err) }