Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions internal/cachecov/coverage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ var versionCoverage = map[int][]string{
97: {"TestMatchAnyGlob_MidPatternDoublestar", "TestGolden"}, // Directory-scoped Ruby test globs: production *_ab_test.rb no longer deleted from the graph
98: {"TestKtComplexity_ScalingLoopDepth_ConstantRangeDiscounted", "TestKtComplexity_ScalingLoopDepth_VariableRangeNotDiscounted", "TestKtComplexity_ScalingLoopDepth_ConstantIteratorReceiverDiscounted", "TestKtComplexity_ScalingLoopDepth_VariableIteratorReceiverNotDiscounted", "TestKtComplexity_ScalingLoopDepth_ConstantOuterScalingInner", "TestKtComplexity_ScalingLoopDepth_ConstantInnerScalingOuter", "TestKtComplexity_ScalingLoopDepth_InfiniteLoopDiscounted", "TestKtComplexity_ScalingLoopDepth_ConditionalWhileNotDiscounted", "TestKtComplexity_ScalingLoopDepth_AbsentWithoutLoops", "TestKtComplexity_LoopCountAndCyclomaticUnchangedByBounding", "TestGolden"}, // Kotlin bounded-loop discounting: scaling_loop_depth joins the Go/Python/TS convention
99: {"TestKtComplexity_CallsInScalingLoop_ConstantExcluded", "TestKtComplexity_CallsInScalingLoop_InfiniteLoopCallsRetained", "TestKtComplexity_CallsInScalingLoop_ScalingRetained", "TestKtComplexity_CallsInScalingLoop_MixedLoops", "TestKtComplexity_CallsInScalingLoop_AbsentWithoutLoopCalls", "TestExtract_CallsInScalingLoop_InfiniteLoopCallsRetained", "TestExtract_CallsInScalingLoop_PresentButEmptyWhenAllBounded", "TestExtract_CallsInScalingLoop_AbsentWithoutLoopCalls", "TestPyComplexity_CallsInScalingLoop_InfiniteLoopCallsRetained", "TestPyComplexity_CallsInScalingLoop_PresentButEmptyWhenAllBounded", "TestPyComplexity_CallsInScalingLoop_AbsentWithoutLoopCalls", "TestTsComplexity_CallsInScalingLoop_InfiniteLoopCallsRetained", "TestTsComplexity_CallsInScalingLoop_PresentButEmptyWhenAllBounded", "TestTsComplexity_CallsInScalingLoop_AbsentWithoutLoopCalls", "TestGolden"}, // calls_in_scaling_loop = repeated (not merely scaling) loops, emitted even when empty
100: {"TestExtractTestRefs_InPackageCallResolvesToProductionSymbol", "TestExtractTestRefs_ExternalTestPackageResolvesThroughImport", "TestExtractTestRefs_SkipsBuiltinsAndEmitsNoSymbols", "TestExtractTestRefs_IgnoresFilesItDoesNotOwn", "TestExtractTestRefs_ReferenceFreeFileYieldsNoFact", "TestDefaultTestGlobsCoverGoAndStayIgnored", "TestGolden"}, // Go test_ref facts: both gates, so a function called only from its _test.go is no longer high-confidence dead
}

func TestCacheVersionCoverage(t *testing.T) {
Expand Down
7 changes: 6 additions & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,12 @@ func Default() *Config {
// include test symbols — but the engine collects them separately for
// reference-only extraction so the dead-code detector can see that a
// production symbol is exercised by a test and not mis-report it as dead.
TestGlobs: []string{"**/spec/**/*_spec.rb", "**/test/**/*_test.rb"},
// A glob here without an extractor implementing plugin.TestRefExtractor is a
// no-op (engine.runTestRefExtractors skips non-implementers), so extend this
// list only alongside the matching extractor. Go's bare suffix is correct:
// the toolchain defines any *_test.go as a test file, so — unlike Ruby's
// _test.rb (v97) — no production file can collide with it.
TestGlobs: []string{"**/*_test.go", "**/spec/**/*_spec.rb", "**/test/**/*_test.rb"},
Extractors: []string{"cpp", "go", "grpc", "java", "kotlin", "openapi", "php", "python", "typescript", "swift", "ruby"},
Explainers: []string{"cycles", "layers", "crossrepo", "coverage", "unused-routes", "god-class", "hotspots", "dependency-depth", "exported-surface", "complexity-outliers"},
Renderers: []string{"llm_context"},
Expand Down
28 changes: 28 additions & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,31 @@ func TestDefaultIgnoresPythonEnvs(t *testing.T) {
}
}
}

// TestDefaultTestGlobsCoverGoAndStayIgnored pins both halves of the test-ref
// contract for Go (GAP-GO-06, v100).
//
// A test file's references survive only if it is BOTH ignored for normal indexing
// AND matched by a TestGlob — engine.walkRepo collects an ignored file for
// reference-only extraction only when matchesTestGlob says so. Adding a glob to
// one list and not the other silently drops the file (ignored, never recovered)
// or indexes test symbols as production code. config.go states the invariant in a
// comment; this asserts it.
//
// Go's bare-suffix form is correct and must stay: unlike Ruby — where
// "**/*_test.rb" swallowed a production job named ..._ab_test.rb and had to become
// directory-scoped (v97) — the Go toolchain DEFINES any *_test.go as a test file,
// so no production file can collide.
func TestDefaultTestGlobsCoverGoAndStayIgnored(t *testing.T) {
cfg := Default()

if !contains(cfg.TestGlobs, "**/*_test.go") {
t.Errorf("Default().TestGlobs missing %q — Go test files are ignored but never recovered", "**/*_test.go")
}

for _, g := range cfg.TestGlobs {
if !contains(cfg.Ignore, g) {
t.Errorf("TestGlob %q is not in Default().Ignore; a test glob that is not ignored indexes test symbols as production code", g)
}
}
}
13 changes: 12 additions & 1 deletion internal/engine/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,18 @@ import (
// calls_in_loop. The second bug masked the first: fixing only the emptiness would have
// deleted true-positive N+1 findings on infinite loops. Cached Go/Python/TypeScript/Kotlin
// snapshots must re-extract.
const cacheVersion = "v99"
// v100: the Go extractor implements plugin.TestRefExtractor and config.Default().TestGlobs
// gained "**/*_test.go", so a production function whose only caller is its own _test.go is
// no longer reported as high-confidence dead code. Both gates were needed: the glob alone
// is a no-op (runTestRefExtractors skips non-implementers), and the interface alone never
// sees the files (walkRepo collects an ignored file only when it matches a test glob). Go
// was the sharpest case of GAP-XL-02 — _test.go is ignored under BOTH shipped configs and a
// plain function's orphan tier is `high`. Test refs are resolved with the production
// resolvers (flattenSelector/collectLocalTypes/resolveChain), so a reference from a test is
// spelled exactly as one from production and inherits goBuiltins filtering: a package-level
// `min` shadowing the Go 1.21 builtin is still not credited, from either side. The file set
// reaching the extractors changes, so cached snapshots must re-extract.
const cacheVersion = "v100"

// extractorCache holds per-extractor facts keyed by a content hash of the files
// the extractor depends on. It is loaded from disk at the start of a snapshot and
Expand Down
4 changes: 4 additions & 0 deletions internal/engine/testdata/golden/go_sample.facts.jsonl
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,12 @@
{"kind":"module","name":"pkg/b","file":"pkg/b","repo":"go_sample","props":{"language":"go","package":"b"}}
{"kind":"symbol","name":"..main","file":"main.go","line":10,"repo":"go_sample","props":{"cyclomatic":1,"exported":false,"language":"go","symbol_kind":"function"},"relations":[{"kind":"calls","target":"pkg/a.Alpha"},{"kind":"calls","target":"pkg/b.Beta"},{"kind":"declares","target":"."}]}
{"kind":"symbol","name":"pkg/a.Alpha","file":"pkg/a/a.go","line":7,"repo":"go_sample","props":{"cyclomatic":1,"exported":true,"language":"go","symbol_kind":"function"},"relations":[{"kind":"calls","target":"pkg/b.Beta"},{"kind":"declares","target":"pkg/a"}]}
{"kind":"symbol","name":"pkg/a.Gamma","file":"pkg/a/a.go","line":43,"repo":"go_sample","props":{"cyclomatic":1,"exported":true,"language":"go","symbol_kind":"function"},"relations":[{"kind":"declares","target":"pkg/a"}]}
{"kind":"symbol","name":"pkg/a.GetPath","file":"pkg/a/a.go","line":15,"repo":"go_sample","props":{"calls_in_loop":["pkg/a.getByID"],"calls_in_scaling_loop":["pkg/a.getByID"],"cyclomatic":2,"exported":true,"language":"go","loop_count":1,"loop_depth":1,"scaling_loop_depth":0,"symbol_kind":"function"},"relations":[{"kind":"calls","target":"pkg/a.getByID"},{"kind":"declares","target":"pkg/a"}]}
{"kind":"symbol","name":"pkg/a.Seed","file":"pkg/a/a.go","line":24,"repo":"go_sample","props":{"calls_in_loop":["pkg/a.setup"],"calls_in_scaling_loop":[],"cyclomatic":2,"exported":true,"language":"go","loop_count":1,"loop_depth":1,"scaling_loop_depth":0,"symbol_kind":"function"},"relations":[{"kind":"calls","target":"pkg/a.setup"},{"kind":"declares","target":"pkg/a"}]}
{"kind":"symbol","name":"pkg/a.getByID","file":"pkg/a/a.go","line":30,"repo":"go_sample","props":{"cyclomatic":1,"exported":false,"language":"go","symbol_kind":"function"},"relations":[{"kind":"declares","target":"pkg/a"}]}
{"kind":"symbol","name":"pkg/a.helper","file":"pkg/a/a.go","line":37,"repo":"go_sample","props":{"cyclomatic":1,"exported":false,"language":"go","symbol_kind":"function"},"relations":[{"kind":"declares","target":"pkg/a"}]}
{"kind":"symbol","name":"pkg/a.setup","file":"pkg/a/a.go","line":31,"repo":"go_sample","props":{"cyclomatic":1,"exported":false,"language":"go","symbol_kind":"function"},"relations":[{"kind":"declares","target":"pkg/a"}]}
{"kind":"symbol","name":"pkg/b.Beta","file":"pkg/b/b.go","line":6,"repo":"go_sample","props":{"cyclomatic":1,"exported":true,"language":"go","symbol_kind":"function"},"relations":[{"kind":"calls","target":"pkg/a.Alpha"},{"kind":"declares","target":"pkg/b"}]}
{"kind":"test_ref","name":"pkg/a/a_ext_test.go","file":"pkg/a/a_ext_test.go","repo":"go_sample","props":{"language":"go"},"relations":[{"kind":"calls","target":"pkg/a.Gamma"},{"kind":"calls","target":"t.Log"}]}
{"kind":"test_ref","name":"pkg/a/a_test.go","file":"pkg/a/a_test.go","repo":"go_sample","props":{"language":"go"},"relations":[{"kind":"calls","target":"pkg/a.helper"},{"kind":"calls","target":"t.Fatalf"}]}
12 changes: 12 additions & 0 deletions internal/engine/testdata/repos/go_sample/pkg/a/a.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,15 @@ func Seed() {

func getByID(id int) {}
func setup(c string) {}

// helper has no production caller. Its only reference is from a_test.go, which is
// ignored for indexing and recovered as a test_ref fact — so the graph must still
// carry an incoming edge for it. Before v100 no such fact existed for Go and the
// symbol looked dead. Pins GAP-GO-06 for the in-package test idiom. (v100)
func helper(n int) int { return n * 2 }

// Gamma is the same case for the OTHER Go test idiom: its only reference is from
// a_ext_test.go (`package a_test`), which reaches it through an import alias rather
// than unqualified. Deliberately not Alpha — Alpha's dependent count is asserted by
// TestE2E_ImpactAnalysis, and a test_ref counts as a dependent (see GAP-XL-15). (v100)
func Gamma() {}
15 changes: 15 additions & 0 deletions internal/engine/testdata/repos/go_sample/pkg/a/a_ext_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package a_test

import (
"testing"

"example.com/gosample/pkg/a"
)

// The external-test-package idiom: `package a_test` imports the package under test,
// so the reference is qualified by an import alias and must resolve through
// buildFileImports to the same canonical name ("pkg/a.Gamma"). (v100)
func TestGammaFromExternalPackage(t *testing.T) {
a.Gamma()
t.Log("called")
}
12 changes: 12 additions & 0 deletions internal/engine/testdata/repos/go_sample/pkg/a/a_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package a

import "testing"

// The in-package idiom: the production function is called unqualified, so the
// test-ref pass must resolve it against the file's own package dir ("pkg/a.helper").
// This is the shape behind golf's NewRateLimiter false positive. (v100)
func TestHelperDoubles(t *testing.T) {
if helper(2) != 4 {
t.Fatalf("helper(2) != 4")
}
}
160 changes: 160 additions & 0 deletions internal/extractors/goextractor/testrefs.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
package goextractor

import (
"context"
"go/ast"
"go/parser"
"go/token"
"log"
"os"
"path/filepath"
"strings"

"github.com/enola-labs/enola/internal/facts"
"github.com/enola-labs/enola/internal/parallel"
)

// isGoTestFile reports whether a repo-relative path is a Go test file. The Go
// toolchain defines the suffix, so this needs no directory scoping — a production
// file cannot legally be named *_test.go and still compile into the package.
func isGoTestFile(relFile string) bool { return strings.HasSuffix(relFile, "_test.go") }

// ExtractTestRefs implements plugin.TestRefExtractor. It parses *_test.go files for
// the SOLE purpose of capturing their outbound references into production code,
// emitting one facts.KindTestRef fact per file that carries only RelCalls edges —
// no symbols. Test functions therefore never become dead-code candidates, and no
// symbol/module/route explainer is affected, while the dead-code detector can see
// that a production function is exercised by a test and not mis-report it as dead.
//
// The engine hands every TestGlob match to every TestRefExtractor whose repo it
// detected, scoped by plugin.FileOwner when the extractor implements it.
// GoExtractor deliberately does not: FileOwner is what opts an extractor into the
// incremental cache (see its doc comment), and implementing it here would both
// enable Go caching and pull .go files out of computeExtractorKeys' shared
// partition, changing the shared hash that keys EVERY other extractor. So the
// filter lives here instead — Ruby's ExtractTestRefs filters internally too.
func (e *GoExtractor) ExtractTestRefs(ctx context.Context, repoPath string, files []string) ([]facts.Fact, error) {
var goFiles []string
for _, relFile := range files {
if isGoTestFile(relFile) {
goFiles = append(goFiles, relFile)
}
}
if len(goFiles) == 0 {
return nil, nil
}

modulePath := readModulePath(repoPath)
perFile := parallel.MapFiles(ctx, goFiles, func(relFile string) []facts.Fact {
src, err := os.ReadFile(filepath.Join(repoPath, relFile))
if err != nil {
log.Printf("[go-extractor] error reading test file %s: %v", relFile, err)
return nil
}
return refsFromGoTest(src, relFile, modulePath)
})

var out []facts.Fact
for _, ff := range perFile {
out = append(out, ff...)
}
return out, nil
}

// refsFromGoTest parses one Go test file and returns a single reference-only fact
// carrying the production symbols it calls, or nil when it references nothing.
//
// Call targets are resolved with the PRODUCTION resolvers (flattenSelector,
// collectLocalTypes, resolveChain), so a reference from a test is spelled exactly
// as the same reference from production code would be, and the dead-code detector
// needs no special case. That also inherits goBuiltins filtering, so len/make/min
// never become phantom targets.
//
// Only call expressions yield targets — matching analyzeBody, which likewise
// ignores composite literals. A type constructed only as `Foo{}` from a test is
// therefore still reported dead, but so is one constructed only that way from
// production code: that blind spot is pre-existing and not specific to tests.
func refsFromGoTest(src []byte, relFile, modulePath string) []facts.Fact {
fset := token.NewFileSet()
f, err := parser.ParseFile(fset, relFile, src, parser.SkipObjectResolution)
if err != nil {
log.Printf("[go-extractor] error parsing test file %s: %v", relFile, err)
return nil
}

base := resolveCtx{
pkgDir: filepath.Dir(relFile),
modulePath: modulePath,
// pkgNames is deliberately nil. It exists to recover a declared package name
// that differs from its directory base ("go-auth" → package auth), which
// needs a view of every parsed package — and this pass sees only test files.
// Worse, a test file's own package name carries a _test suffix
// (`package svc_test`), so feeding these in would alias the import under
// test as "svc_test" and break the very idiom this exists to resolve.
// buildFileImports then falls back to the import path's base, exactly as the
// production pass does for any package it did not parse.
imports: buildFileImports(f, modulePath, nil),
}

seen := make(map[string]bool)
var rels []facts.Relation
add := func(target string) {
if target == "" || seen[target] {
return
}
seen[target] = true
rels = append(rels, facts.Relation{Kind: facts.RelCalls, Target: target})
}
collectCalls := func(n ast.Node, ctx resolveCtx) {
ast.Inspect(n, func(node ast.Node) bool {
if call, ok := node.(*ast.CallExpr); ok {
if chain := flattenSelector(call.Fun); chain != nil {
add(resolveChain(chain, ctx))
}
}
return true
})
}

for _, decl := range f.Decls {
switch d := decl.(type) {
case *ast.FuncDecl:
if d.Body == nil {
continue
}
ctx := base
if d.Recv != nil && len(d.Recv.List) > 0 {
field := d.Recv.List[0]
ctx.recvType = typeExprToString(field.Type)
if len(field.Names) > 0 {
ctx.recvVar = field.Names[0].Name
}
}
ctx.localTypes = collectLocalTypes(d.Body, ctx)
collectCalls(d.Body, ctx)
case *ast.GenDecl:
// File-scope initializers (`var _ = Register(handler)`) reference
// production code with no enclosing function to attribute them to.
for _, spec := range d.Specs {
vs, ok := spec.(*ast.ValueSpec)
if !ok {
continue
}
for _, v := range vs.Values {
collectCalls(v, base)
}
}
}
}

if len(rels) == 0 {
return nil
}
return []facts.Fact{{
Kind: facts.KindTestRef,
Name: relFile,
File: relFile,
Props: map[string]any{"language": "go"},
Relations: rels,
}}
}
Loading
Loading