Skip to content

Commit 9206014

Browse files
authored
Improving ruby extractor (#58)
* Second round - Ruby callbacks and dynamic dispatch * Another pass on calls outside method bodies * Addressing true dynamic dispatch * Chained no-arg calls, .rake indexing * Default params, predicate/bang calls * Wrapping it up
1 parent d3f7215 commit 9206014

9 files changed

Lines changed: 1125 additions & 23 deletions

File tree

internal/config/config.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
type Config struct {
1212
Repo string `yaml:"repo"`
1313
Ignore []string `yaml:"ignore"`
14+
TestGlobs []string `yaml:"test_globs"`
1415
Extractors []string `yaml:"extractors"`
1516
Explainers []string `yaml:"explainers"`
1617
Renderers []string `yaml:"renderers"`
@@ -68,6 +69,12 @@ func Default() *Config {
6869
"**/Pods/**",
6970
"**/.gradle/**",
7071
},
72+
// TestGlobs identify test/spec files. They stay ignored for normal indexing
73+
// (still listed in Ignore above) — production architecture facts must not
74+
// include test symbols — but the engine collects them separately for
75+
// reference-only extraction so the dead-code detector can see that a
76+
// production symbol is exercised by a test and not mis-report it as dead.
77+
TestGlobs: []string{"**/*_spec.rb", "**/*_test.rb"},
7178
Extractors: []string{"cpp", "go", "java", "kotlin", "openapi", "php", "python", "typescript", "swift", "ruby"},
7279
Explainers: []string{"cycles", "layers", "crossrepo", "coverage", "unused-routes", "god-class", "hotspots", "dependency-depth", "exported-surface", "complexity-outliers"},
7380
Renderers: []string{"llm_context"},

internal/engine/cache.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,16 @@ import (
3131
// v15: C/C++ salvage function-pointer refs from file-scope ERROR regions (macro-opened structs like MACHINE_START/DT_MACHINE_START ... MACHINE_END).
3232
// v16: extend that salvage to file-scope assignment_expression/field_expression fragments (machine_desc blocks parse that way when surrounded by other code).
3333
// v17: full-tree salvage of `.field = fn` macro-struct debris (machine_desc) regardless of where tree-sitter scatters it (skips function bodies).
34-
const cacheVersion = "v17"
34+
// 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.
35+
// 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.
36+
// 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.
37+
// 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.
38+
// 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.
39+
// 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.
40+
// 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.
41+
// 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`).
42+
// v26: Ruby records scope-like (underscored) method calls on ANY identifier receiver, including local relation variables (`items = ...; items.preload_relations`).
43+
const cacheVersion = "v26"
3544

3645
// extractorCache holds per-extractor facts keyed by a content hash of the files
3746
// the extractor depends on. It is loaded from disk at the start of a snapshot and

internal/engine/engine.go

Lines changed: 104 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/enola-labs/enola/internal/facts"
2424
"github.com/enola-labs/enola/internal/linkers/crossrepo"
2525
"github.com/enola-labs/enola/internal/renderers"
26+
"github.com/enola-labs/enola/pkg/plugin"
2627
)
2728

2829
// Engine orchestrates the snapshot generation pipeline.
@@ -184,12 +185,12 @@ func (e *Engine) GenerateSnapshot(ctx context.Context, repoPath string, appendMo
184185

185186
// 1. Walk repository and collect files
186187
tStage := time.Now()
187-
files, err := e.walkRepo(absRepo)
188+
files, testFiles, err := e.walkRepo(absRepo)
188189
if err != nil {
189190
return nil, fmt.Errorf("walking repo: %w", err)
190191
}
191192
tWalk = time.Since(tStage)
192-
log.Printf("[engine] found %d files in %s", len(files), absRepo)
193+
log.Printf("[engine] found %d files (%d test files) in %s", len(files), len(testFiles), absRepo)
193194

194195
// 2. Compute file hashes (for snapshot metadata)
195196
tStage = time.Now()
@@ -218,6 +219,11 @@ func (e *Engine) GenerateSnapshot(ctx context.Context, repoPath string, appendMo
218219
}
219220
}
220221
}
222+
// Reference-only extraction over test/spec files. Runs every snapshot (not
223+
// cached with the main extractors) and adds only KindTestRef facts, so a
224+
// production symbol exercised solely by a test is not mis-reported as dead.
225+
e.runTestRefExtractors(ctx, absRepo, testFiles)
226+
221227
tExtract = time.Since(tStage)
222228
newCount := e.store.Count()
223229
log.Printf("[engine] extracted %d facts using %d extractors", newCount, len(usedExtractors))
@@ -388,10 +394,12 @@ func (e *Engine) flagUnmatchedRoutes() {
388394
}
389395
}
390396

391-
// walkRepo collects all files in the repo, applying ignore patterns.
392-
func (e *Engine) walkRepo(repoPath string) ([]string, error) {
393-
var files []string
394-
err := filepath.WalkDir(repoPath, func(path string, d fs.DirEntry, err error) error {
397+
// walkRepo collects all files in the repo, applying ignore patterns. It returns
398+
// the indexable source files plus, separately, the test/spec files matched by
399+
// TestGlobs — those are excluded from normal indexing but collected for
400+
// reference-only extraction (see runTestRefExtractors).
401+
func (e *Engine) walkRepo(repoPath string) (files, testFiles []string, err error) {
402+
err = filepath.WalkDir(repoPath, func(path string, d fs.DirEntry, err error) error {
395403
if err != nil {
396404
return err
397405
}
@@ -406,6 +414,12 @@ func (e *Engine) walkRepo(repoPath string) ([]string, error) {
406414
if d.IsDir() {
407415
return filepath.SkipDir
408416
}
417+
// An ignored FILE that is a test/spec is not indexed as production
418+
// source, but is collected for reference-only extraction so a
419+
// production symbol exercised only by a test does not look dead.
420+
if e.matchesTestGlob(relPath) {
421+
testFiles = append(testFiles, relPath)
422+
}
409423
return nil
410424
}
411425

@@ -414,7 +428,49 @@ func (e *Engine) walkRepo(repoPath string) ([]string, error) {
414428
}
415429
return nil
416430
})
417-
return files, err
431+
return files, testFiles, err
432+
}
433+
434+
// matchesTestGlob reports whether a repo-relative path matches any TestGlob.
435+
func (e *Engine) matchesTestGlob(relPath string) bool {
436+
return matchAnyGlob(filepath.ToSlash(relPath), e.cfg.TestGlobs)
437+
}
438+
439+
// matchAnyGlob reports whether a forward-slash path matches any of the patterns,
440+
// mirroring the "**/<name>/**", trailing-"/**", and "**/<glob>" handling of
441+
// isIgnored so test-glob matching stays consistent with ignore matching.
442+
func matchAnyGlob(relPath string, patterns []string) bool {
443+
for _, pattern := range patterns {
444+
if strings.HasPrefix(pattern, "**/") && strings.HasSuffix(pattern, "/**") {
445+
seg := strings.TrimSuffix(strings.TrimPrefix(pattern, "**/"), "/**")
446+
if seg != "" && !strings.Contains(seg, "/") {
447+
for _, part := range strings.Split(relPath, "/") {
448+
if part == seg {
449+
return true
450+
}
451+
}
452+
}
453+
}
454+
if strings.HasSuffix(pattern, "/**") {
455+
dirPrefix := strings.TrimSuffix(pattern, "/**")
456+
if relPath == dirPrefix || strings.HasPrefix(relPath, dirPrefix+"/") {
457+
return true
458+
}
459+
}
460+
if m, err := filepath.Match(pattern, relPath); err == nil && m {
461+
return true
462+
}
463+
if strings.HasPrefix(pattern, "**/") {
464+
sub := strings.TrimPrefix(pattern, "**/")
465+
if m, err := filepath.Match(sub, filepath.Base(relPath)); err == nil && m {
466+
return true
467+
}
468+
if m, err := filepath.Match(sub, relPath); err == nil && m {
469+
return true
470+
}
471+
}
472+
}
473+
return false
418474
}
419475

420476
// isIgnored checks whether a path matches any ignore pattern.
@@ -530,6 +586,47 @@ func (e *Engine) runExtractors(ctx context.Context, repoPath string, files []str
530586
return usedNames, nil
531587
}
532588

589+
// runTestRefExtractors runs reference-only extraction over the test/spec files
590+
// for every enabled, detected extractor that implements plugin.TestRefExtractor.
591+
// It scopes each extractor to the test files it owns and adds the resulting
592+
// KindTestRef facts to the store. Errors are logged, not fatal.
593+
func (e *Engine) runTestRefExtractors(ctx context.Context, repoPath string, testFiles []string) {
594+
if len(testFiles) == 0 {
595+
return
596+
}
597+
for _, ext := range e.extractors.All() {
598+
if !e.cfg.IsExtractorEnabled(ext.Name()) {
599+
continue
600+
}
601+
tr, ok := ext.(plugin.TestRefExtractor)
602+
if !ok {
603+
continue
604+
}
605+
if detected, err := ext.Detect(repoPath); err != nil || !detected {
606+
continue
607+
}
608+
owned := testFiles
609+
if fo, ok := ext.(plugin.FileOwner); ok {
610+
owned = owned[:0:0]
611+
for _, f := range testFiles {
612+
if fo.OwnsFile(f) {
613+
owned = append(owned, f)
614+
}
615+
}
616+
}
617+
if len(owned) == 0 {
618+
continue
619+
}
620+
refFacts, err := tr.ExtractTestRefs(ctx, repoPath, owned)
621+
if err != nil {
622+
log.Printf("[engine] extractor %s test-ref error: %v", ext.Name(), err)
623+
continue
624+
}
625+
e.store.Add(refFacts...)
626+
log.Printf("[engine] extractor %s: emitted %d test-ref facts from %d files", ext.Name(), len(refFacts), len(owned))
627+
}
628+
}
629+
533630
// runExplainers runs all enabled explainers.
534631
func (e *Engine) runExplainers(ctx context.Context) ([]facts.Insight, []string, error) {
535632
var allInsights []facts.Insight

internal/extractors/rubyextractor/ruby.go

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,35 @@ func (e *RubyExtractor) Extract(ctx context.Context, repoPath string, files []st
112112
return allFacts, nil
113113
}
114114

115+
// ExtractTestRefs implements plugin.TestRefExtractor. It parses test/spec files
116+
// for the SOLE purpose of capturing their outbound references into production
117+
// code, emitting one facts.KindTestRef fact per file that carries only RelCalls
118+
// edges — no symbols. Test methods therefore never become dead-code candidates
119+
// (which the orphans package explicitly excludes), and no symbol/module/route
120+
// explainer is affected, while the dead-code detector can still see that a
121+
// production symbol is exercised by a test and not mis-report it as dead.
122+
func (e *RubyExtractor) ExtractTestRefs(ctx context.Context, repoPath string, files []string) ([]facts.Fact, error) {
123+
var rbFiles []string
124+
for _, relFile := range files {
125+
if isRubyFile(relFile) {
126+
rbFiles = append(rbFiles, relFile)
127+
}
128+
}
129+
perFile := parallel.MapFiles(ctx, rbFiles, func(relFile string) []facts.Fact {
130+
src, err := os.ReadFile(filepath.Join(repoPath, relFile))
131+
if err != nil {
132+
log.Printf("[ruby-extractor] error reading test file %s: %v", relFile, err)
133+
return nil
134+
}
135+
return extractTestRefsAST(src, relFile)
136+
})
137+
var out []facts.Fact
138+
for _, ff := range perFile {
139+
out = append(out, ff...)
140+
}
141+
return out, nil
142+
}
143+
115144
// --- Rails detection ---
116145

117146
func detectRailsProject(repoPath string) bool {
@@ -127,9 +156,15 @@ func detectRailsProject(repoPath string) bool {
127156
return false
128157
}
129158

130-
// isRubyFile returns true if the file has a .rb extension.
159+
// isRubyFile returns true if the file is Ruby source: a .rb/.rake file or a
160+
// Rakefile. Rake tasks are Ruby and call into app code (e.g. from lib/tasks/),
161+
// so indexing them lets those calls resolve (dead-code precision).
131162
func isRubyFile(path string) bool {
132-
return strings.HasSuffix(strings.ToLower(path), ".rb")
163+
lower := strings.ToLower(path)
164+
if strings.HasSuffix(lower, ".rb") || strings.HasSuffix(lower, ".rake") {
165+
return true
166+
}
167+
return filepath.Base(path) == "Rakefile"
133168
}
134169

135170
// OwnsFile implements plugin.FileOwner for incremental caching.

0 commit comments

Comments
 (0)