diff --git a/internal/config/config.go b/internal/config/config.go index 483c70b..bd5a3f4 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -11,6 +11,7 @@ import ( type Config struct { Repo string `yaml:"repo"` Ignore []string `yaml:"ignore"` + TestGlobs []string `yaml:"test_globs"` Extractors []string `yaml:"extractors"` Explainers []string `yaml:"explainers"` Renderers []string `yaml:"renderers"` @@ -68,6 +69,12 @@ func Default() *Config { "**/Pods/**", "**/.gradle/**", }, + // TestGlobs identify test/spec files. They stay ignored for normal indexing + // (still listed in Ignore above) — production architecture facts must not + // 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.rb", "**/*_test.rb"}, Extractors: []string{"cpp", "go", "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"}, diff --git a/internal/engine/cache.go b/internal/engine/cache.go index b0b21f7..24ad291 100644 --- a/internal/engine/cache.go +++ b/internal/engine/cache.go @@ -31,7 +31,16 @@ import ( // v15: C/C++ salvage function-pointer refs from file-scope ERROR regions (macro-opened structs like MACHINE_START/DT_MACHINE_START ... MACHINE_END). // v16: extend that salvage to file-scope assignment_expression/field_expression fragments (machine_desc blocks parse that way when surrounded by other code). // v17: full-tree salvage of `.field = fn` macro-struct debris (machine_desc) regardless of where tree-sitter scatters it (skips function bodies). -const cacheVersion = "v17" +// v18: Ruby records custom class-body macro names + resolves self/self.class receivers as call edges (dead-code precision). KindTestRef facts index outbound refs from spec/test files. +// v19: Ruby captures call edges outside method bodies — class/module-body qualified & argument-position calls attach to the class fact; top-level/file-scope calls (fixtures, after_initialize blocks) attach to a new KindFileRef fact folded by the orphan collector. +// v20: Ruby call capture outside method bodies generalized to a per-scope pass (whole class/module body + top-level program), so assignment RHS and all non-`call` statements — e.g. `x = GlobalSetting.foo`, `CONST = { Proc.new { Group.bar } }` — are covered, not just bare call statements. +// v21: Ruby records the static prefix of interpolated symbols (`:"report_#{type}"` -> "report_") as a KindFileRef prop, so the dead-code detector treats dynamically dispatched (public_send/send) same-prefix methods as used. +// v22: Ruby records `super` as a call to the same-named ancestor method, and literal-symbol dispatch args (`obj.try(:foo)`, `send(:bar)`, `respond_to?(:baz)`) as calls to the named method. +// v23: Ruby captures no-arg calls on a chained receiver (ActiveRecord scope/class-method chains `Model.scope.final`, `assoc.class_method`, `x.class.method`; cheap attribute reads skipped) and indexes .rake/Rakefile files. +// v24: Ruby walks method default-parameter values for calls (`def f(x = self.class.foo)`) and records single-level predicate/bang calls (`viewer.rich?`, `x.save!`) which — unlike plain attribute reads — are unambiguously method invocations. +// v25: Ruby folds `delegate :a, :b, ..., to: X` method names as calls, and records calls on a bare-method (non-local identifier) receiver when the method name is scope-like (`some_relation.pluck_job_id`). +// v26: Ruby records scope-like (underscored) method calls on ANY identifier receiver, including local relation variables (`items = ...; items.preload_relations`). +const cacheVersion = "v26" // 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 diff --git a/internal/engine/engine.go b/internal/engine/engine.go index a314ae9..59a1156 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -23,6 +23,7 @@ import ( "github.com/enola-labs/enola/internal/facts" "github.com/enola-labs/enola/internal/linkers/crossrepo" "github.com/enola-labs/enola/internal/renderers" + "github.com/enola-labs/enola/pkg/plugin" ) // Engine orchestrates the snapshot generation pipeline. @@ -184,12 +185,12 @@ func (e *Engine) GenerateSnapshot(ctx context.Context, repoPath string, appendMo // 1. Walk repository and collect files tStage := time.Now() - files, err := e.walkRepo(absRepo) + files, testFiles, err := e.walkRepo(absRepo) if err != nil { return nil, fmt.Errorf("walking repo: %w", err) } tWalk = time.Since(tStage) - log.Printf("[engine] found %d files in %s", len(files), absRepo) + log.Printf("[engine] found %d files (%d test files) in %s", len(files), len(testFiles), absRepo) // 2. Compute file hashes (for snapshot metadata) tStage = time.Now() @@ -218,6 +219,11 @@ func (e *Engine) GenerateSnapshot(ctx context.Context, repoPath string, appendMo } } } + // Reference-only extraction over test/spec files. Runs every snapshot (not + // cached with the main extractors) and adds only KindTestRef facts, so a + // production symbol exercised solely by a test is not mis-reported as dead. + e.runTestRefExtractors(ctx, absRepo, testFiles) + tExtract = time.Since(tStage) newCount := e.store.Count() log.Printf("[engine] extracted %d facts using %d extractors", newCount, len(usedExtractors)) @@ -388,10 +394,12 @@ func (e *Engine) flagUnmatchedRoutes() { } } -// walkRepo collects all files in the repo, applying ignore patterns. -func (e *Engine) walkRepo(repoPath string) ([]string, error) { - var files []string - err := filepath.WalkDir(repoPath, func(path string, d fs.DirEntry, err error) error { +// walkRepo collects all files in the repo, applying ignore patterns. It returns +// the indexable source files plus, separately, the test/spec files matched by +// TestGlobs — those are excluded from normal indexing but collected for +// reference-only extraction (see runTestRefExtractors). +func (e *Engine) walkRepo(repoPath string) (files, testFiles []string, err error) { + err = filepath.WalkDir(repoPath, func(path string, d fs.DirEntry, err error) error { if err != nil { return err } @@ -406,6 +414,12 @@ func (e *Engine) walkRepo(repoPath string) ([]string, error) { if d.IsDir() { return filepath.SkipDir } + // An ignored FILE that is a test/spec is not indexed as production + // source, but is collected for reference-only extraction so a + // production symbol exercised only by a test does not look dead. + if e.matchesTestGlob(relPath) { + testFiles = append(testFiles, relPath) + } return nil } @@ -414,7 +428,49 @@ func (e *Engine) walkRepo(repoPath string) ([]string, error) { } return nil }) - return files, err + return files, testFiles, err +} + +// matchesTestGlob reports whether a repo-relative path matches any TestGlob. +func (e *Engine) matchesTestGlob(relPath string) bool { + return matchAnyGlob(filepath.ToSlash(relPath), e.cfg.TestGlobs) +} + +// matchAnyGlob reports whether a forward-slash path matches any of the patterns, +// mirroring the "**//**", trailing-"/**", and "**/" handling of +// isIgnored so test-glob matching stays consistent with ignore matching. +func matchAnyGlob(relPath string, patterns []string) bool { + for _, pattern := range patterns { + if strings.HasPrefix(pattern, "**/") && strings.HasSuffix(pattern, "/**") { + seg := strings.TrimSuffix(strings.TrimPrefix(pattern, "**/"), "/**") + if seg != "" && !strings.Contains(seg, "/") { + for _, part := range strings.Split(relPath, "/") { + if part == seg { + return true + } + } + } + } + if strings.HasSuffix(pattern, "/**") { + dirPrefix := strings.TrimSuffix(pattern, "/**") + if relPath == dirPrefix || strings.HasPrefix(relPath, dirPrefix+"/") { + return true + } + } + if m, err := filepath.Match(pattern, relPath); err == nil && m { + return true + } + if strings.HasPrefix(pattern, "**/") { + sub := strings.TrimPrefix(pattern, "**/") + if m, err := filepath.Match(sub, filepath.Base(relPath)); err == nil && m { + return true + } + if m, err := filepath.Match(sub, relPath); err == nil && m { + return true + } + } + } + return false } // isIgnored checks whether a path matches any ignore pattern. @@ -530,6 +586,47 @@ func (e *Engine) runExtractors(ctx context.Context, repoPath string, files []str return usedNames, nil } +// runTestRefExtractors runs reference-only extraction over the test/spec files +// for every enabled, detected extractor that implements plugin.TestRefExtractor. +// It scopes each extractor to the test files it owns and adds the resulting +// KindTestRef facts to the store. Errors are logged, not fatal. +func (e *Engine) runTestRefExtractors(ctx context.Context, repoPath string, testFiles []string) { + if len(testFiles) == 0 { + return + } + for _, ext := range e.extractors.All() { + if !e.cfg.IsExtractorEnabled(ext.Name()) { + continue + } + tr, ok := ext.(plugin.TestRefExtractor) + if !ok { + continue + } + if detected, err := ext.Detect(repoPath); err != nil || !detected { + continue + } + owned := testFiles + if fo, ok := ext.(plugin.FileOwner); ok { + owned = owned[:0:0] + for _, f := range testFiles { + if fo.OwnsFile(f) { + owned = append(owned, f) + } + } + } + if len(owned) == 0 { + continue + } + refFacts, err := tr.ExtractTestRefs(ctx, repoPath, owned) + if err != nil { + log.Printf("[engine] extractor %s test-ref error: %v", ext.Name(), err) + continue + } + e.store.Add(refFacts...) + log.Printf("[engine] extractor %s: emitted %d test-ref facts from %d files", ext.Name(), len(refFacts), len(owned)) + } +} + // runExplainers runs all enabled explainers. func (e *Engine) runExplainers(ctx context.Context) ([]facts.Insight, []string, error) { var allInsights []facts.Insight diff --git a/internal/extractors/rubyextractor/ruby.go b/internal/extractors/rubyextractor/ruby.go index 52f8c4f..11a0653 100644 --- a/internal/extractors/rubyextractor/ruby.go +++ b/internal/extractors/rubyextractor/ruby.go @@ -112,6 +112,35 @@ func (e *RubyExtractor) Extract(ctx context.Context, repoPath string, files []st return allFacts, nil } +// ExtractTestRefs implements plugin.TestRefExtractor. It parses test/spec 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 methods therefore never become dead-code candidates +// (which the orphans package explicitly excludes), and no symbol/module/route +// explainer is affected, while the dead-code detector can still see that a +// production symbol is exercised by a test and not mis-report it as dead. +func (e *RubyExtractor) ExtractTestRefs(ctx context.Context, repoPath string, files []string) ([]facts.Fact, error) { + var rbFiles []string + for _, relFile := range files { + if isRubyFile(relFile) { + rbFiles = append(rbFiles, relFile) + } + } + perFile := parallel.MapFiles(ctx, rbFiles, func(relFile string) []facts.Fact { + src, err := os.ReadFile(filepath.Join(repoPath, relFile)) + if err != nil { + log.Printf("[ruby-extractor] error reading test file %s: %v", relFile, err) + return nil + } + return extractTestRefsAST(src, relFile) + }) + var out []facts.Fact + for _, ff := range perFile { + out = append(out, ff...) + } + return out, nil +} + // --- Rails detection --- func detectRailsProject(repoPath string) bool { @@ -127,9 +156,15 @@ func detectRailsProject(repoPath string) bool { return false } -// isRubyFile returns true if the file has a .rb extension. +// isRubyFile returns true if the file is Ruby source: a .rb/.rake file or a +// Rakefile. Rake tasks are Ruby and call into app code (e.g. from lib/tasks/), +// so indexing them lets those calls resolve (dead-code precision). func isRubyFile(path string) bool { - return strings.HasSuffix(strings.ToLower(path), ".rb") + lower := strings.ToLower(path) + if strings.HasSuffix(lower, ".rb") || strings.HasSuffix(lower, ".rake") { + return true + } + return filepath.Base(path) == "Rakefile" } // OwnsFile implements plugin.FileOwner for incremental caching. diff --git a/internal/extractors/rubyextractor/ruby_ast.go b/internal/extractors/rubyextractor/ruby_ast.go index 9c9b624..69ce5fa 100644 --- a/internal/extractors/rubyextractor/ruby_ast.go +++ b/internal/extractors/rubyextractor/ruby_ast.go @@ -2,6 +2,7 @@ package rubyextractor import ( "path/filepath" + "sort" "strings" "github.com/enola-labs/enola/internal/facts" @@ -9,6 +10,17 @@ import ( ruby "github.com/tree-sitter/tree-sitter-ruby/bindings/go" ) +// sortedKeys returns the keys of a set in deterministic (sorted) order — used to +// emit stable prop values into facts. +func sortedKeys(m map[string]bool) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + sort.Strings(out) + return out +} + // extractFileAST parses a Ruby file with tree-sitter and emits architectural // facts. It replaces the former line-based regex scanner: every symbol, import, // mixin, constant, attr, ActiveRecord storage/association, and RelCalls edge the @@ -29,11 +41,58 @@ func extractFileAST(src []byte, relFile string, isRails, exportedByPackwerk bool dir: filepath.Dir(relFile), isRails: isRails, exportedByPackwerk: exportedByPackwerk, + fileRefIdx: -1, + } + root := tree.RootNode() + w.walkBody(root) + // Capture executable calls made at file scope (top-level assignment RHS, + // conditionals, fixture `Badge.foo(...)`, plugin `after_initialize` blocks) on + // the file-scope ref fact. walkForCalls returns at nested defs/classes, which + // get their own pass. + if owner := w.bodyCallOwner(); owner >= 0 { + w.walkForCalls(root, owner, map[string]bool{}, nil) + } + // Attach any dynamic-dispatch prefixes discovered anywhere in the file to the + // file-scope fact so the collector can mark same-prefix methods as used. + if len(w.dynamicPrefixes) > 0 { + idx := w.ensureFileRefFact() + w.out[idx].Props["dynamic_send_prefixes"] = sortedKeys(w.dynamicPrefixes) + } + // Drop the file-scope reference fact if it carries neither call edges nor + // dynamic-dispatch prefixes, so empty facts never reach the store. + if w.fileRefIdx >= 0 && len(w.out[w.fileRefIdx].Relations) == 0 && + w.out[w.fileRefIdx].Props["dynamic_send_prefixes"] == nil { + w.out = append(w.out[:w.fileRefIdx], w.out[w.fileRefIdx+1:]...) } - w.walkBody(tree.RootNode()) return w.out } +// ensureFileRefFact returns the index of this file's lazily-created file-scope +// reference fact (facts.KindFileRef), creating it on first use. +func (w *rubyWalker) ensureFileRefFact() int { + if w.fileRefIdx < 0 { + w.out = append(w.out, facts.Fact{ + Kind: facts.KindFileRef, + Name: w.relFile, + File: w.relFile, + Props: map[string]any{"language": "ruby"}, + }) + w.fileRefIdx = len(w.out) - 1 + } + return w.fileRefIdx +} + +// bodyCallOwner returns the index of the fact that class/module-body and +// top-level call edges should attach to: the enclosing class/module symbol fact +// when inside a type scope, otherwise (top level, or an eigenclass body whose +// symFactIdx is -1) the lazily-created file-scope reference fact. +func (w *rubyWalker) bodyCallOwner() int { + if s := w.cur(); s != nil && s.symFactIdx >= 0 { + return s.symFactIdx + } + return w.ensureFileRefFact() +} + // rubyScope tracks a class/module/eigenclass nesting level. type rubyScope struct { name string // simple (last) name; "" for an eigenclass (class << self) @@ -55,6 +114,16 @@ type rubyWalker struct { out []facts.Fact scopeStack []rubyScope + // fileRefIdx is the index into out of the lazily-created file-scope reference + // fact (facts.KindFileRef) that holds top-level call edges; -1 until first used. + fileRefIdx int + + // dynamicPrefixes accumulates the static prefixes of interpolated symbols + // (`:"report_#{type}"` -> "report_") seen anywhere in the file. They mark + // dynamic dispatch (public_send/send by computed name), letting the dead-code + // detector treat same-prefix methods as used. File-global; nil until first hit. + dynamicPrefixes map[string]bool + // Per-method complexity state, set up by handleMethod around walkForCalls. // metrics is nil outside a method body walk. loopDepth is the current loop // nesting depth; selfName/selfShort are the enclosing method's full and short @@ -216,9 +285,12 @@ func (w *rubyWalker) walkStatement(node *sitter.Node) { w.handleAssignment(node) case "call": w.handleBodyCall(node) - // Descend into a trailing do/brace block so declarations inside - // included/class_methods/concerning/configure blocks are captured (the - // former line-based scanner was block-agnostic). + // Executable call EDGES in this statement (macro args, qualified + // `Const.method` calls, calls inside blocks) are captured by the per-scope + // walkForCalls pass run in handleClass/handleModule/extractFileAST — not here + // — so assignments and every other statement kind are covered uniformly. + // This case still descends into a trailing do/brace block to capture nested + // DECLARATIONS (def/class/const inside included/class_methods/concerning blocks). if body := blockBody(node); body != nil { w.walkBody(body) } @@ -288,8 +360,11 @@ func (w *rubyWalker) handleModule(node *sitter.Node) { Relations: []facts.Relation{{Kind: facts.RelDeclares, Target: w.dir}}, }) - w.push(rubyScope{name: name, kind: "module", visibility: "public", symFactIdx: len(w.out) - 1}) + modIdx := len(w.out) - 1 + w.push(rubyScope{name: name, kind: "module", visibility: "public", symFactIdx: modIdx}) w.walkBody(body) + // Capture executable calls made directly in the module body (see handleClass). + w.walkForCalls(body, modIdx, map[string]bool{}, nil) w.pop() } @@ -347,7 +422,12 @@ func (w *rubyWalker) handleClass(node *sitter.Node) { w.push(rubyScope{name: name, kind: "class", visibility: "public", isModel: isModel, isSerializer: isSerializerBase(superclass), symFactIdx: clsIdx}) - w.walkBody(node.ChildByFieldName("body")) + body := node.ChildByFieldName("body") + w.walkBody(body) + // Capture executable calls made directly in the class body (assignment RHS, + // conditionals, hash/Proc literals, macro args) as uses of this class. + // walkForCalls returns at nested defs/classes, which get their own pass. + w.walkForCalls(body, clsIdx, map[string]bool{}, nil) w.pop() } @@ -355,7 +435,13 @@ func (w *rubyWalker) handleSingletonClass(node *sitter.Node) { // class << self — methods inside become class (singleton) methods. The // eigenclass entry carries no name and does not affect qualification. w.push(rubyScope{name: "", kind: "eigenclass", visibility: "public", symFactIdx: -1}) - w.walkBody(node.ChildByFieldName("body")) + body := node.ChildByFieldName("body") + w.walkBody(body) + // The eigenclass has no symbol fact (symFactIdx -1); attribute any executable + // calls in its body to the file-scope ref fact via bodyCallOwner. + if owner := w.bodyCallOwner(); owner >= 0 { + w.walkForCalls(body, owner, map[string]bool{}, nil) + } w.pop() } @@ -407,6 +493,10 @@ func (w *rubyWalker) handleMethod(node *sitter.Node, isClassMethod bool) { // updates the emitted fact. seen := make(map[string]bool) locals := collectLocals(node, w.src) + // Default parameter values (`def f(x = self.class.foo)`) contain real call + // references. Walk them with metrics off (params are not the body, so they must + // not affect the complexity score); seen is shared so body calls still dedup. + w.walkForCalls(node.ChildByFieldName("parameters"), ownerIdx, seen, locals) w.metrics = &rubyBodyMetrics{} w.loopDepth = 0 w.selfName = fullName @@ -469,6 +559,19 @@ func (w *rubyWalker) walkForCalls(node *sitter.Node, ownerIdx int, seen, locals switch node.Kind() { case "method", "singleton_method", "class", "module", "singleton_class": return + case "super": + // `super` invokes the same-named method in an ancestor (superclass or mixin), + // so it references that base method. Only meaningful inside a method body + // (metrics != nil), where selfShort is the enclosing method's bare name. + // Recurse afterwards to capture any calls in `super(args)`. + if w.metrics != nil && w.selfShort != "" { + w.addCall(ownerIdx, seen, w.selfShort) + w.recordCallMetrics(w.selfShort) + } + for i := uint(0); i < node.ChildCount(); i++ { + w.walkForCalls(node.Child(i), ownerIdx, seen, locals) + } + return case "while", "until", "for", "while_modifier", "until_modifier": // Syntactic loops: everything in the body runs per iteration. if w.metrics != nil { @@ -487,6 +590,16 @@ func (w *rubyWalker) walkForCalls(node *sitter.Node, ownerIdx int, seen, locals case "call": method := node.ChildByFieldName("method") recv := node.ChildByFieldName("receiver") + // Dynamic dispatch by LITERAL name — `obj.try(:foo)`, `send(:bar)`, + // `respond_to?(:baz)` — names the target method exactly, so record it as a + // call. Safe-nav `&.try` still exposes the `method` child. Distinct from an + // interpolated symbol (`:"report_#{x}"`), which is only a prefix hint. + if method != nil && rubyDispatchers[rubyText(method, w.src)] { + if nm := dispatchSymbolArg(node.ChildByFieldName("arguments"), w.src); nm != "" { + w.addCall(ownerIdx, seen, nm) + w.recordCallMetrics(nm) + } + } if target := w.callTarget(node); target != "" { w.addCall(ownerIdx, seen, target) w.recordCallMetrics(target) @@ -495,12 +608,33 @@ func (w *rubyWalker) walkForCalls(node *sitter.Node, ownerIdx int, seen, locals w.addCall(ownerIdx, seen, name) w.recordCallMetrics(name) } - } else if w.loopDepth > 0 && recv != nil && method != nil && method.Kind() == "identifier" { - // A no-arg instance call inside a loop that callTarget suppressed (e.g. - // the association read `u.posts` or a `record.reload`). It is not a graph - // edge, but its method name feeds the perf metric so the enterprise - // analyzer can flag lazy-loaded association / per-iteration I/O (N+1). - if name := rubyText(method, w.src); !rubyNonCalls[name] && !rubyCheapMethods[name] { + } else if recv != nil && method != nil && method.Kind() == "identifier" { + // A no-arg call on a receiver that callTarget suppressed. Bare target + // (no ".") -> no coupling impact. Skip keywords and common + // attribute/enumerable reads so a dead method sharing a name with + // `.name`/`.count`/`.first` isn't hidden. + name := rubyText(method, w.src) + switch { + case rubyNonCalls[name] || rubyCheapMethods[name]: + // keyword / cheap attribute-or-enumerable read — ignore + case recv.Kind() == "call" || strings.HasSuffix(name, "?") || strings.HasSuffix(name, "!") || + (recv.Kind() == "identifier" && strings.Contains(name, "_")): + // Chained receiver (ActiveRecord scope / class-method chains + // `Model.scope1.scope2.final`, `assoc.class_method`, `x.class.method`), + // a predicate/bang call on ANY receiver (`viewer.rich?`, `x.save!`), OR a + // call on an identifier receiver (a local relation var OR a bare method) + // whose name is scope/class-method-like (has `_`) — e.g. + // `items.preload_relations`, `some_relation.pluck_job_id`. All are + // unambiguously method calls (an attribute read never ends in `?`/`!`, + // and a snake_case multi-word name is a scope/class-method, not a plain + // attribute). Single-word reads (`user.email`, `x.name`) stay out. + w.addCall(ownerIdx, seen, name) + w.recordCallMetrics(name) + case w.loopDepth > 0: + // A no-arg single-level read inside a loop (the association read + // `u.posts` or `record.reload`). It is not a graph edge, but its method + // name feeds the perf metric so the enterprise analyzer can flag + // lazy-loaded association / per-iteration I/O (N+1). w.recordInLoopCall(name) } } @@ -559,12 +693,182 @@ func (w *rubyWalker) walkForCalls(node *sitter.Node, ownerIdx int, seen, locals w.addCall(ownerIdx, seen, name) } return + case "delimited_symbol": + // An interpolated symbol `:"report_#{type}"` — the literal name of a method + // dispatched dynamically (public_send/send by computed name), which is not a + // resolvable call edge. Record its static prefix so the dead-code detector can + // treat same-prefix methods as used. Fall through to recurse: the interpolation + // (`#{...}`) may itself contain real calls. + if p := dynamicSymbolPrefix(node, w.src); p != "" { + if w.dynamicPrefixes == nil { + w.dynamicPrefixes = map[string]bool{} + } + w.dynamicPrefixes[p] = true + } } for i := uint(0); i < node.ChildCount(); i++ { w.walkForCalls(node.Child(i), ownerIdx, seen, locals) } } +// dynamicSymbolPrefix returns the static literal prefix of an interpolated symbol +// node (`:"report_#{type}"` -> "report_"), or "" when the node is not an +// interpolated symbol or the prefix is not specific enough to be a useful dispatch +// hint. The prefix is the string_content preceding the FIRST interpolation; it +// qualifies only when at least one interpolation is present and the prefix is >= 4 +// chars ending in "_" (a word boundary), so generic 1-2 char stems don't over-match. +func dynamicSymbolPrefix(node *sitter.Node, src []byte) string { + var prefix strings.Builder + sawInterp := false + for i := uint(0); i < node.ChildCount(); i++ { + c := node.Child(i) + switch c.Kind() { + case "interpolation": + sawInterp = true + i = node.ChildCount() // stop at the first interpolation + case "string_content": + prefix.WriteString(rubyText(c, src)) + } + } + if !sawInterp { + return "" + } + p := prefix.String() + if len(p) >= 4 && strings.HasSuffix(p, "_") { + return p + } + return "" +} + +// rubyDispatchers are methods that invoke (or reference) another method named by +// their first argument: `obj.try(:foo)`, `send(:bar)`, `respond_to?(:baz)`, +// `method(:qux)`. When that argument is a LITERAL symbol/string the target method +// is statically known, so it is recorded as a call. +var rubyDispatchers = map[string]bool{ + "send": true, "public_send": true, "__send__": true, + "try": true, "try!": true, "respond_to?": true, + "method": true, "public_method": true, +} + +// dispatchSymbolArg returns the method name named by the first argument of a +// dispatcher call (`:foo` -> "foo", "foo" -> "foo"), or "" when the first argument +// is not a literal symbol / static string (e.g. a variable or interpolated value). +func dispatchSymbolArg(args *sitter.Node, src []byte) string { + if args == nil { + return "" + } + for i := uint(0); i < args.ChildCount(); i++ { + c := args.Child(i) + if !c.IsNamed() { + continue + } + switch c.Kind() { + case "simple_symbol": + return strings.TrimPrefix(rubyText(c, src), ":") + case "string": + // Static string only (no interpolation): the literal is the method name. + for j := uint(0); j < c.ChildCount(); j++ { + if c.Child(j).Kind() == "interpolation" { + return "" + } + } + return stringLiteralContent(c, src) + } + return "" // first positional arg is something else — not a literal name + } + return "" +} + +// stringLiteralContent returns the concatenated string_content of a string node. +func stringLiteralContent(node *sitter.Node, src []byte) string { + var b strings.Builder + for i := uint(0); i < node.ChildCount(); i++ { + if node.Child(i).Kind() == "string_content" { + b.WriteString(rubyText(node.Child(i), src)) + } + } + return b.String() +} + +// extractTestRefsAST parses a test/spec file and returns a single +// facts.KindTestRef fact whose RelCalls relations name the production symbols the +// test references. It reuses the production call-target conventions (callTarget + +// bare receiver-less calls + bare constants) but descends through every scope — +// a test file has no meaningful symbol surface of its own, and its references +// live inside describe/context/it blocks and example methods. Local-variable +// tracking is deliberately omitted (unlike walkForCalls): over-emitting a +// reference can only ever keep a production symbol alive, never falsely flag one +// dead, matching the orphan detector's conservative bias. Returns nil when the +// file references nothing. +func extractTestRefsAST(src []byte, relFile string) []facts.Fact { + parser := sitter.NewParser() + defer parser.Close() + if err := parser.SetLanguage(sitter.NewLanguage(ruby.Language())); err != nil { + return nil + } + tree := parser.Parse(src, nil) + defer tree.Close() + + w := &rubyWalker{src: src, relFile: relFile, dir: filepath.Dir(relFile), fileRefIdx: -1} + 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}) + } + w.walkTestRefs(tree.RootNode(), add) + if len(rels) == 0 { + return nil + } + return []facts.Fact{{ + Kind: facts.KindTestRef, + Name: relFile, + File: relFile, + Props: map[string]any{"language": "ruby"}, + Relations: rels, + }} +} + +// walkTestRefs recurses through ALL named nodes of a test file, emitting a +// RelCalls target for each qualified call, bare receiver-less call, and bare +// constant — the same conventions as walkForCalls, minus the loop/complexity +// metrics and the class/module/method early-returns (a spec's references live +// inside those bodies). +func (w *rubyWalker) walkTestRefs(node *sitter.Node, add func(string)) { + if node == nil || !node.IsNamed() { + return + } + switch node.Kind() { + case "call": + method := node.ChildByFieldName("method") + recv := node.ChildByFieldName("receiver") + if target := w.callTarget(node); target != "" { + add(target) + } else if recv == nil && method != nil && method.Kind() == "identifier" { + if name := rubyText(method, w.src); !rubyNonCalls[name] { + add(name) + } + } + case "identifier": + // A bare identifier: an arg-less method call or a local read. Emit unless a + // keyword/builtin; matching is conservative so over-emitting is safe. + if name := rubyText(node, w.src); name != "" && !rubyNonCalls[name] { + add(name) + } + case "constant", "scope_resolution": + if name := stripLeadingColons(rubyText(node, w.src)); name != "" && !rubyBuiltinConsts[name] { + add(name) + } + return // recorded whole; do not descend into scope_resolution segments + } + for i := uint(0); i < node.ChildCount(); i++ { + w.walkTestRefs(node.Child(i), add) + } +} + // addCall appends a deduplicated RelCalls edge to the owner fact. func (w *rubyWalker) addCall(ownerIdx int, seen map[string]bool, target string) { if target == "" || seen[target] { @@ -731,11 +1035,30 @@ func (w *rubyWalker) callTarget(node *sitter.Node) string { switch recv.Kind() { case "constant", "scope_resolution": return rubyText(recv, w.src) + "." + methodName + case "self": + // `self.foo` / `self.save!` — same-object dispatch. Emit the bare method + // name so the method is recorded as used (dead-code precision). Bare target + // (no "."), short-name matched, ignored by the coupling graph. `self.class` + // as a receiver-only expression emits nothing here; the OUTER call (handled + // in `case "call":` below) emits the real method. + if methodName == "class" { + return "" + } + return methodName case "identifier": if node.ChildByFieldName("arguments") != nil { return rubyText(recv, w.src) + "." + methodName } case "call": + // `self.class.perform_when_readonly?`: the inner call is `self.class` + // (receiver kind "self", method "class"). Emit the OUTER method name, bare. + // No args-gate: the receiver is provably the class, so the name is provably + // a method (unlike a lowercase-variable attribute read). + if innerRecv := recv.ChildByFieldName("receiver"); innerRecv != nil && innerRecv.Kind() == "self" { + if innerMethod := recv.ChildByFieldName("method"); innerMethod != nil && rubyText(innerMethod, w.src) == "class" { + return methodName + } + } // Chained call, e.g. Rails.logger.info(x): use the inner call's method // name as a pseudo-receiver when it is a lowercase identifier. inner := recv.ChildByFieldName("method") @@ -813,6 +1136,13 @@ func (w *rubyWalker) handleBodyCall(node *sitter.Node) { method := rubyText(node.ChildByFieldName("method"), w.src) args := node.ChildByFieldName("arguments") + // Note: the macro NAME itself (`requires_login`, `cluster_concurrency`) is + // recorded as a use of the enclosing class by the walkForCalls pass that + // walkStatement now runs over every body-level call — so it is not repeated + // here (that would emit a duplicate RelCalls edge). This function only folds in + // the DSL's *symbol arguments* (callback/serializer method names), which + // walkForCalls does not special-case. + // Rails callback/validation DSL references methods by symbol literal // (`before_action :authenticate_user!`, `validate :check`). Record each as a // RelCalls edge on the enclosing class/module so callback-only methods are @@ -826,6 +1156,20 @@ func (w *rubyWalker) handleBodyCall(node *sitter.Node) { return } + // `delegate :a, :b, ..., to: X` generates methods that call each named method on + // the target — a real reference. Fold the delegated method names in as calls on + // the enclosing class so they are not mis-reported as dead. The `to:`/`prefix:` + // keyword args are `pair` nodes, so symbolArgs (direct simple_symbol children + // only) skips them. + if method == "delegate" { + if cur := w.cur(); cur != nil && cur.symFactIdx >= 0 { + for _, name := range symbolArgs(args, w.src) { + w.addCallToFact(cur.symFactIdx, name) + } + } + return + } + // ActiveModel::Serializer attribute/association DSL: `attributes :a, :b`, // `attribute :c`, `has_one :user`, `has_many :posts`. Each declared name is // backed by a same-named method the serializer framework calls (when defined), diff --git a/internal/extractors/rubyextractor/ruby_test.go b/internal/extractors/rubyextractor/ruby_test.go index 15a8082..679f37a 100644 --- a/internal/extractors/rubyextractor/ruby_test.go +++ b/internal/extractors/rubyextractor/ruby_test.go @@ -824,3 +824,584 @@ dependencies: t.Fatal("missing root module fact (should be named 'root', not '.')") } } + +// testRefFact returns the single KindTestRef fact from extractTestRefsAST output. +func testRefFact(result []facts.Fact) (facts.Fact, bool) { + for _, f := range result { + if f.Kind == facts.KindTestRef { + return f, true + } + } + return facts.Fact{}, false +} + +// TestExtractFile_CustomClassMacroRecorded checks that a bare-receiver class-body +// call (a custom Rails DSL macro like `requires_login` / `cluster_concurrency`) +// records the MACRO NAME as a use on the enclosing class, so the base-class method +// backing the macro is not mis-reported as dead. The target must be bare (no "."), +// so it never pollutes the packwerk coupling graph. +func TestExtractFile_CustomClassMacroRecorded(t *testing.T) { + src := `class ReportsController < ApplicationController + requires_login except: [:index] + cluster_concurrency 1 +end +` + result := extractFileAST([]byte(src), "app/controllers/reports_controller.rb", true, true) + cls, ok := symbolsByName(result)["ReportsController"] + if !ok { + t.Fatal("missing class ReportsController") + } + for _, want := range []string{"requires_login", "cluster_concurrency"} { + if !hasCall(cls, want) { + t.Errorf("missing custom macro RelCalls -> %s; relations = %v", want, cls.Relations) + } + } + if hasCall(cls, "ReportsController.requires_login") { + t.Errorf("macro name must be bare, not a coupling form; relations = %v", cls.Relations) + } +} + +// TestExtractFile_StructuralMacrosNotRecordedAsCalls guards that structural +// keywords (require/private) are NOT recorded as macro call edges. +func TestExtractFile_StructuralMacrosNotRecordedAsCalls(t *testing.T) { + src := `class Thing + require "set" + private +end +` + result := extractFileAST([]byte(src), "app/models/thing.rb", false, true) + cls := symbolsByName(result)["Thing"] + for _, skip := range []string{"require", "private"} { + if hasCall(cls, skip) { + t.Errorf("structural keyword %s must not be a macro edge; relations = %v", skip, cls.Relations) + } + } +} + +// TestExtractFile_TopLevelMacroNoSymbolEdge checks a body call outside any class +// attaches to no symbol fact (there is no enclosing class to hang it on). +func TestExtractFile_TopLevelMacroNoSymbolEdge(t *testing.T) { + src := `configure_app foo: 1 +` + result := extractFileAST([]byte(src), "config/init.rb", false, true) + for _, f := range result { + if f.Kind == facts.KindSymbol && hasCall(f, "configure_app") { + t.Errorf("top-level macro must not attach to any symbol fact; fact = %v", f) + } + } +} + +// TestExtractFile_SelfAndSelfClassReceivers checks that `self.foo` and +// `self.class.bar` dispatch is resolved to the bare method name, while a bare +// `self.class` receiver does not emit a spurious `class` edge. +func TestExtractFile_SelfAndSelfClassReceivers(t *testing.T) { + src := `class Job + def run + do_it if self.class.perform_when_readonly? + self.reset + end +end +` + result := extractFileAST([]byte(src), "app/jobs/job.rb", false, true) + meth := symbolsByName(result)["Job#run"] + for _, want := range []string{"perform_when_readonly?", "reset"} { + if !hasCall(meth, want) { + t.Errorf("missing self-receiver RelCalls -> %s; relations = %v", want, meth.Relations) + } + } + if hasCall(meth, "class") { + t.Errorf("self.class receiver must not emit a bare 'class' edge; relations = %v", meth.Relations) + } + if hasCall(meth, "class.perform_when_readonly?") { + t.Errorf("self.class.X must resolve to a bare method, not class.X; relations = %v", meth.Relations) + } +} + +// TestExtractFile_SelfBangAndPredicatePreserved checks predicate/bang suffixes +// survive self-receiver resolution. +func TestExtractFile_SelfBangAndPredicatePreserved(t *testing.T) { + src := `class Model + def process + self.save! + return unless self.valid? + end +end +` + result := extractFileAST([]byte(src), "app/models/model.rb", false, true) + meth := symbolsByName(result)["Model#process"] + for _, want := range []string{"save!", "valid?"} { + if !hasCall(meth, want) { + t.Errorf("missing self predicate/bang RelCalls -> %s; relations = %v", want, meth.Relations) + } + } +} + +// TestExtractTestRefsAST checks the reference-only spec pass emits a single +// KindTestRef fact carrying the production symbols the spec exercises (qualified +// receivers fold to bare method via the collector's lastSeg), and NO symbol facts. +func TestExtractTestRefsAST(t *testing.T) { + src := `# frozen_string_literal: true +describe Badge do + it "computes ids" do + expect(Badge.trust_level_badge_ids).to eq([1, 2]) + GlobalSetting.reset_redis_config! + end +end +` + result := extractTestRefsAST([]byte(src), "spec/services/badge_granter_spec.rb") + for _, f := range result { + if f.Kind == facts.KindSymbol { + t.Fatalf("test-ref pass must not emit symbol facts; got %v", f) + } + } + ref, ok := testRefFact(result) + if !ok { + t.Fatal("missing KindTestRef fact") + } + for _, want := range []string{"Badge.trust_level_badge_ids", "GlobalSetting.reset_redis_config!"} { + if !hasCall(ref, want) { + t.Errorf("missing test ref RelCalls -> %s; relations = %v", want, ref.Relations) + } + } +} + +// fileRefFact returns the single KindFileRef fact from extractFileAST output, if any. +func fileRefFact(result []facts.Fact) (facts.Fact, bool) { + for _, f := range result { + if f.Kind == facts.KindFileRef { + return f, true + } + } + return facts.Fact{}, false +} + +// countCalls returns how many RelCalls relations on a fact target the given name. +func countCalls(f facts.Fact, target string) int { + n := 0 + for _, r := range f.Relations { + if r.Kind == facts.RelCalls && r.Target == target { + n++ + } + } + return n +} + +// TestExtractFile_ClassBodySplatArgMethodCall checks that a method call in the +// ARGUMENT of a class-body macro (`requires_login *show_methods`) is captured on +// the class fact — not just the macro name — so the arg method is not flagged dead. +func TestExtractFile_ClassBodySplatArgMethodCall(t *testing.T) { + src := `class TagsController < ApplicationController + requires_login *show_methods +end +` + result := extractFileAST([]byte(src), "app/controllers/tags_controller.rb", true, true) + cls := symbolsByName(result)["TagsController"] + for _, want := range []string{"requires_login", "show_methods"} { + if !hasCall(cls, want) { + t.Errorf("missing class-body RelCalls -> %s; relations = %v", want, cls.Relations) + } + } + if hasCall(cls, "TagsController.show_methods") { + t.Errorf("arg method must be bare, not a coupling form; relations = %v", cls.Relations) + } +} + +// TestExtractFile_ClassBodyQualifiedCall checks a qualified Const.method call in a +// class body attaches to the class fact (previously dropped by handleBodyCall). +func TestExtractFile_ClassBodyQualifiedCall(t *testing.T) { + src := `class Report + Badge.register(self) +end +` + result := extractFileAST([]byte(src), "app/models/report.rb", false, true) + cls := symbolsByName(result)["Report"] + if !hasCall(cls, "Badge.register") { + t.Errorf("missing class-body qualified RelCalls -> Badge.register; relations = %v", cls.Relations) + } +} + +// TestExtractFile_ClassBodyMacroNoDuplicate guards the Fix A / walkForCalls dedup: +// a class-body macro name is recorded exactly once on the class fact. +func TestExtractFile_ClassBodyMacroNoDuplicate(t *testing.T) { + src := `class Thing + requires_login except: [:index] +end +` + result := extractFileAST([]byte(src), "app/controllers/thing_controller.rb", true, true) + cls := symbolsByName(result)["Thing"] + if got := countCalls(cls, "requires_login"); got != 1 { + t.Errorf("requires_login recorded %d times, want exactly 1; relations = %v", got, cls.Relations) + } +} + +// TestExtractFile_TopLevelQualifiedCallOnFileRef checks a top-level (fixture-style) +// call is captured on a KindFileRef fact, leaks no symbol fact, and that a file with +// only ignorable top-level calls produces no file-ref fact. +func TestExtractFile_TopLevelQualifiedCallOnFileRef(t *testing.T) { + result := extractFileAST([]byte("Badge.like_badge_counts(1, 2)\n"), "db/fixtures/006_badges.rb", false, true) + fr, ok := fileRefFact(result) + if !ok { + t.Fatal("missing KindFileRef fact for a top-level call") + } + if !hasCall(fr, "Badge.like_badge_counts") { + t.Errorf("missing file-ref RelCalls -> Badge.like_badge_counts; relations = %v", fr.Relations) + } + for _, f := range result { + if f.Kind == facts.KindSymbol && hasCall(f, "Badge.like_badge_counts") { + t.Errorf("top-level call must not attach to a symbol fact; fact = %v", f) + } + } + // A file whose only top-level call is a suppressed keyword produces no file-ref. + empty := extractFileAST([]byte("require \"set\"\n"), "config/init.rb", false, true) + if _, ok := fileRefFact(empty); ok { + t.Error("file with no real top-level refs should not emit a KindFileRef fact") + } +} + +// TestExtractFile_TopLevelAfterInitializeBlock checks a call inside a top-level +// plugin block (after_initialize do ... end) is captured on the file-ref fact. +func TestExtractFile_TopLevelAfterInitializeBlock(t *testing.T) { + src := `after_initialize do + CategoryList.register_included_association(:foo) +end +` + result := extractFileAST([]byte(src), "plugins/foo/plugin.rb", true, true) + fr, ok := fileRefFact(result) + if !ok { + t.Fatal("missing KindFileRef fact for a top-level block call") + } + if !hasCall(fr, "CategoryList.register_included_association") { + t.Errorf("missing file-ref RelCalls -> CategoryList.register_included_association; relations = %v", fr.Relations) + } +} + +// TestExtractFile_TopLevelAssignmentRHS checks that a call on the RHS of a +// top-level assignment (a Rails initializer pattern) is captured on the file-ref +// fact — previously dropped because handleAssignment never walked the value. +func TestExtractFile_TopLevelAssignmentRHS(t *testing.T) { + src := `if Rails.configuration.multisite + assets_hostnames = GlobalSetting.cdn_hostnames +end +` + result := extractFileAST([]byte(src), "config/initializers/200-first_middlewares.rb", true, true) + fr, ok := fileRefFact(result) + if !ok { + t.Fatal("missing KindFileRef fact for a top-level assignment RHS call") + } + if !hasCall(fr, "GlobalSetting.cdn_hostnames") { + t.Errorf("missing file-ref RelCalls -> GlobalSetting.cdn_hostnames; relations = %v", fr.Relations) + } + for _, f := range result { + if f.Kind == facts.KindSymbol && hasCall(f, "GlobalSetting.cdn_hostnames") { + t.Errorf("top-level assignment RHS call must not attach to a symbol fact; fact = %v", f) + } + } +} + +// TestExtractFile_TopLevelSetterAssignmentRHS checks a call on the RHS of a +// top-level setter-assignment inside an if/else is captured. +func TestExtractFile_TopLevelSetterAssignmentRHS(t *testing.T) { + src := `if Rails.env.test? + MessageBus.configure(backend: :memory) +else + MessageBus.redis_config = GlobalSetting.message_bus_redis_config +end +` + result := extractFileAST([]byte(src), "config/initializers/004-message_bus.rb", true, true) + fr, ok := fileRefFact(result) + if !ok { + t.Fatal("missing KindFileRef fact") + } + if !hasCall(fr, "GlobalSetting.message_bus_redis_config") { + t.Errorf("missing file-ref RelCalls -> GlobalSetting.message_bus_redis_config; relations = %v", fr.Relations) + } +} + +// TestExtractFile_ClassBodyConstAssignmentProc checks that calls inside a Proc in +// a class-body constant assignment (Discourse's TYPE_FILTERS pattern) are captured +// on the class fact. +func TestExtractFile_ClassBodyConstAssignmentProc(t *testing.T) { + src := `class GroupsController < ApplicationController + TYPE_FILTERS = { + my: Proc.new { |groups, user| Group.member_of(groups, user) }, + owner: Proc.new { |groups, user| Group.owner_of(groups, user) }, + } +end +` + result := extractFileAST([]byte(src), "app/controllers/groups_controller.rb", true, true) + cls := symbolsByName(result)["GroupsController"] + for _, want := range []string{"Group.owner_of", "Group.member_of"} { + if !hasCall(cls, want) { + t.Errorf("missing class-body const-assignment Proc RelCalls -> %s; relations = %v", want, cls.Relations) + } + } +} + +// fileRefPrefixes returns the dynamic_send_prefixes prop of the KindFileRef fact. +func fileRefPrefixes(result []facts.Fact) []string { + for _, f := range result { + if f.Kind == facts.KindFileRef { + if raw, ok := f.Props["dynamic_send_prefixes"].([]string); ok { + return raw + } + } + } + return nil +} + +// TestExtractFile_InterpolatedSymbolPrefix checks that an interpolated symbol +// (`:"report_#{type}"`) — the mark of dynamic dispatch by computed name — records +// its static prefix on the file-scope fact. +func TestExtractFile_InterpolatedSymbolPrefix(t *testing.T) { + src := `class IncomingLinksReport + def self.find(type) + report_method = :"report_#{type}" + public_send(report_method, type) + end +end +` + result := extractFileAST([]byte(src), "app/models/incoming_links_report.rb", true, true) + got := fileRefPrefixes(result) + found := false + for _, p := range got { + if p == "report_" { + found = true + } + } + if !found { + t.Errorf("expected dynamic_send_prefixes to contain %q; got %v", "report_", got) + } +} + +// TestExtractFile_NoPrefixFromStaticSymbolOrString checks the heuristic does NOT +// fire for static symbols, too-short prefixes, or interpolated STRINGS (i18n keys). +func TestExtractFile_NoPrefixFromStaticSymbolOrString(t *testing.T) { + src := `class Thing + def go(type) + a = :report_foo # static symbol, no interpolation + b = :"m#{type}" # prefix too short / no underscore + c = I18n.t("reports.#{type}.x") # interpolated STRING, not a symbol + [a, b, c] + end +end +` + result := extractFileAST([]byte(src), "app/models/thing.rb", true, true) + if got := fileRefPrefixes(result); len(got) != 0 { + t.Errorf("expected no dynamic_send_prefixes, got %v", got) + } +} + +// TestExtractFile_SuperReferencesAncestor checks that a `super` call records a +// reference to the same-named ancestor method (the base an override delegates to). +func TestExtractFile_SuperReferencesAncestor(t *testing.T) { + src := `module EE + module IssuesFinder + def negatable_params + @negatable_params ||= super + [:weight] + end + end +end +` + result := extractFileAST([]byte(src), "ee/app/finders/ee/issues_finder.rb", true, true) + meth := symbolsByName(result)["EE::IssuesFinder#negatable_params"] + if !hasCall(meth, "negatable_params") { + t.Errorf("super should record a call to the same-named ancestor method; relations = %v", meth.Relations) + } +} + +// TestExtractFile_LiteralSymbolDispatch checks that a literal-symbol argument to a +// dispatcher (try/send/respond_to?) records a call to the named method, while a +// non-dispatcher method with a symbol arg (e.g. validates :name) does not. +func TestExtractFile_LiteralSymbolDispatch(t *testing.T) { + src := `class BaseField + def complexity(resolver) + ext = resolver&.try(:calculate_ext_conn_complexity) + v = @resolver_class.send(:requires_argument?) + validates :name + [ext, v] + end +end +` + result := extractFileAST([]byte(src), "app/graphql/types/base_field.rb", true, true) + meth := symbolsByName(result)["BaseField#complexity"] + for _, want := range []string{"calculate_ext_conn_complexity", "requires_argument?"} { + if !hasCall(meth, want) { + t.Errorf("dispatcher literal-symbol arg should record RelCalls -> %s; relations = %v", want, meth.Relations) + } + } + // `validates :name` is a DSL, not a dispatcher — `name` must not be a call here. + if hasCall(meth, "name") { + t.Errorf("non-dispatcher symbol arg must not be recorded as a call; relations = %v", meth.Relations) + } +} + +// TestExtractFile_ChainedNoArgCall checks that a no-arg method call at the end of +// a chain (ActiveRecord scope / class-method chains) is captured, while common +// attribute/enumerable reads and single-level reads are not. +func TestExtractFile_ChainedNoArgCall(t *testing.T) { + src := `class Worker + def run + DeployToken.active.with_owners.ordered_for_keyset_pagination + merge_request.merge_request_closing_issues.preload_issue + group_link.class.access_options + user.name + list.map.first + a.b.count + end +end +` + result := extractFileAST([]byte(src), "app/workers/worker.rb", true, true) + meth := symbolsByName(result)["Worker#run"] + for _, want := range []string{"ordered_for_keyset_pagination", "preload_issue", "access_options"} { + if !hasCall(meth, want) { + t.Errorf("chained no-arg call should record RelCalls -> %s; relations = %v", want, meth.Relations) + } + } + // Cheap chained reads (name/first/class are in rubyCheapMethods) and the + // single-level read (user.name — var receiver) must NOT be recorded. + for _, skip := range []string{"name", "first", "class"} { + if hasCall(meth, skip) { + t.Errorf("attribute/enumerable/single-level read %q must not be recorded as a chained call; relations = %v", skip, meth.Relations) + } + } +} + +// TestIsRubyFile_Rake checks that .rake files and Rakefile are treated as Ruby. +func TestIsRubyFile_Rake(t *testing.T) { + for _, p := range []string{"lib/tasks/gitlab/graphql_introspection.rake", "Rakefile"} { + if !isRubyFile(p) { + t.Errorf("isRubyFile(%q) = false, want true", p) + } + } + if isRubyFile("app/models/foo.py") { + t.Error("isRubyFile should be false for non-Ruby files") + } +} + +// TestExtractFile_RakeTaskCallCaptured checks a top-level call in a .rake file is +// recorded on the file-scope fact (so its target is not mis-reported as dead). +func TestExtractFile_RakeTaskCallCaptured(t *testing.T) { + src := `namespace :gitlab do + task introspection: :environment do + puts CachedIntrospectionQuery.query_string_no_deprecated + end +end +` + result := extractFileAST([]byte(src), "lib/tasks/gitlab/graphql_introspection.rake", true, true) + fr, ok := fileRefFact(result) + if !ok || !hasCall(fr, "CachedIntrospectionQuery.query_string_no_deprecated") { + t.Errorf("rake task call should be captured on the file-ref fact; result = %v", result) + } +} + +// TestExtractFile_DefaultParamCall checks a call in a method's default parameter +// value is recorded (previously only the body was walked). +func TestExtractFile_DefaultParamCall(t *testing.T) { + src := `class BuildTraceChunk + def unsafe_persist_data!(new_store = self.class.persistable_store) + new_store.to_s + end +end +` + result := extractFileAST([]byte(src), "app/models/ci/build_trace_chunk.rb", true, true) + meth := symbolsByName(result)["BuildTraceChunk#unsafe_persist_data!"] + if !hasCall(meth, "persistable_store") { + t.Errorf("default-parameter call should be recorded; relations = %v", meth.Relations) + } +} + +// TestExtractFile_PredicateBangSingleLevelCall checks that predicate/bang calls on +// a plain variable receiver are recorded (they are unambiguous method calls), while +// plain attribute reads and cheap predicates are not. +func TestExtractFile_PredicateBangSingleLevelCall(t *testing.T) { + src := `class BlobHelper + def show(viewer, record) + x = viewer.rich? + record.save! + y = viewer.present? + z = viewer.blob + [x, y, z] + end +end +` + result := extractFileAST([]byte(src), "app/helpers/blob_helper.rb", true, true) + meth := symbolsByName(result)["BlobHelper#show"] + for _, want := range []string{"rich?", "save!"} { + if !hasCall(meth, want) { + t.Errorf("single-level predicate/bang call should be recorded -> %s; relations = %v", want, meth.Relations) + } + } + for _, skip := range []string{"present?", "blob"} { + if hasCall(meth, skip) { + t.Errorf("cheap predicate / plain attribute read %q must not be recorded; relations = %v", skip, meth.Relations) + } + } +} + +// TestExtractFile_DelegateFold checks that `delegate :a, :b, ..., to: :class` +// records the delegated method names as calls on the enclosing class, while the +// `to:` keyword value is not recorded. +func TestExtractFile_DelegateFold(t *testing.T) { + src := `class BlobViewer + delegate :rich?, :simple?, :loading_partial_path, to: :class +end +` + result := extractFileAST([]byte(src), "app/models/blob_viewer/base.rb", true, true) + cls := symbolsByName(result)["BlobViewer"] + for _, want := range []string{"rich?", "simple?", "loading_partial_path"} { + if !hasCall(cls, want) { + t.Errorf("delegate should record RelCalls -> %s; relations = %v", want, cls.Relations) + } + } + for _, skip := range []string{"to", "class"} { + if hasCall(cls, skip) { + t.Errorf("delegate keyword %q must not be recorded; relations = %v", skip, cls.Relations) + } + } +} + +// TestExtractFile_BareMethodChainCall checks that a scope/class-method call on a +// bare-method (non-local identifier) receiver is captured when the name is +// scope-like (has `_`), while single-word attribute reads and local receivers are not. +func TestExtractFile_BareMethodChainCall(t *testing.T) { + src := `class Service + def run + ordered_relation_scope.pluck_job_id.uniq + current_user.email + rel = base_scope + rel.pluck_something + end +end +` + result := extractFileAST([]byte(src), "app/services/service.rb", true, true) + meth := symbolsByName(result)["Service#run"] + // Non-local bare method receiver AND local relation-variable receiver both record + // their underscored scope-like calls. + for _, want := range []string{"pluck_job_id", "pluck_something"} { + if !hasCall(meth, want) { + t.Errorf("scope-like call should record %s; relations = %v", want, meth.Relations) + } + } + // current_user.email: single-word (no underscore) -> not recorded. + if hasCall(meth, "email") { + t.Errorf("single-word attribute read email must not be recorded; relations = %v", meth.Relations) + } +} + +// TestExtractFile_LocalRelationScopeCall checks the exact GitLab shape: a scope +// method invoked on a local variable holding an AR relation. +func TestExtractFile_LocalRelationScopeCall(t *testing.T) { + src := `class FeatureFlagsFinder + def execute(preload: true) + items = feature_flags + items = items.preload_relations if preload + items.ordered + end +end +` + result := extractFileAST([]byte(src), "app/finders/feature_flags_finder.rb", true, true) + meth := symbolsByName(result)["FeatureFlagsFinder#execute"] + if !hasCall(meth, "preload_relations") { + t.Errorf("scope call on a local relation var should record preload_relations; relations = %v", meth.Relations) + } +} diff --git a/internal/facts/model.go b/internal/facts/model.go index 75c92e6..2f787b2 100644 --- a/internal/facts/model.go +++ b/internal/facts/model.go @@ -25,6 +25,20 @@ const ( KindStorage = "storage" KindDependency = "dependency" KindService = "service" // A whole repository, represented as a node in the cross-repo "graph of graphs". + // KindTestRef is a reference-only fact emitted from a test/spec file. It carries + // solely RelCalls relations naming the production symbols the test exercises + // (Name/File are the test file path). Test files are excluded from normal + // indexing, so their symbols never become facts; this kind lets the dead-code + // detector still see that a production symbol is referenced by a test, without + // any other explainer (which key off symbol/module/route facts) being affected. + KindTestRef = "test_ref" + // KindFileRef is a reference-only fact holding call edges made in file-scope + // (top-level) code — fixtures, initializers, and plugin registration blocks — + // that have no enclosing symbol to attach to. Like KindTestRef it carries solely + // RelCalls relations (Name/File are the source file path) and is consumed only by + // the dead-code detector, so top-level references mark a production symbol used + // without perturbing the coupling graph or any other explainer. + KindFileRef = "file_ref" ) // Relation kind constants. diff --git a/pkg/facts/facts.go b/pkg/facts/facts.go index 9989561..b371d33 100644 --- a/pkg/facts/facts.go +++ b/pkg/facts/facts.go @@ -29,6 +29,8 @@ const ( KindStorage = internal.KindStorage KindDependency = internal.KindDependency KindService = internal.KindService + KindTestRef = internal.KindTestRef + KindFileRef = internal.KindFileRef ) // Relation kind constants. diff --git a/pkg/plugin/plugin.go b/pkg/plugin/plugin.go index 46dc83c..4df3c44 100644 --- a/pkg/plugin/plugin.go +++ b/pkg/plugin/plugin.go @@ -28,6 +28,19 @@ type FileOwner interface { OwnsFile(relFile string) bool } +// TestRefExtractor is an optional interface an Extractor may implement to parse +// test/spec files for their outbound references into production code only. The +// engine calls it with the test files (matched by config.TestGlobs) that the +// extractor owns. Implementations must emit ONLY reference facts (facts.KindTestRef +// carrying RelCalls relations) and no symbol/module/route facts, so test code +// never becomes a dead-code candidate and no other explainer is affected. +// Extractors that do not implement it are simply skipped for test-ref extraction. +type TestRefExtractor interface { + // ExtractTestRefs parses the given repo-relative test files and returns + // reference-only facts (facts.KindTestRef). + ExtractTestRefs(ctx context.Context, repoPath string, files []string) ([]facts.Fact, error) +} + // Explainer analyzes facts and produces architectural insights. type Explainer interface { // Name returns the explainer identifier (e.g. "cycles", "layers").