Skip to content

Commit d6701fa

Browse files
authored
Adding benchmarks v1 (#34)
* Adding --explain and fixing Python exporters * Improving Ruby dependency resolving (requires and includes) * Closing static import gap in Java * Fixing Swift & Kotlin edge dependencies * Fixing resolving internal module coupling in Kotlin
1 parent 6ceef33 commit d6701fa

18 files changed

Lines changed: 2660 additions & 42 deletions

File tree

cmd/enola/main.go

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ import (
77
"os"
88
"path/filepath"
99

10+
"github.com/enola-labs/enola/internal/config"
1011
"github.com/enola-labs/enola/pkg/bootstrap"
12+
"github.com/enola-labs/enola/pkg/explain"
1113
)
1214

1315
func main() {
@@ -16,12 +18,24 @@ func main() {
1618
ctx := context.Background()
1719

1820
generateMode := false
21+
explainMode := false
1922
cfgPath := "mcp-arch.yaml"
23+
explainRepo := "" // optional positional repo path for --explain
24+
2025
for _, arg := range os.Args[1:] {
21-
if arg == "--generate" {
26+
switch arg {
27+
case "--generate":
2228
generateMode = true
23-
} else {
24-
cfgPath = arg
29+
case "--explain":
30+
explainMode = true
31+
default:
32+
// In --explain mode the positional argument is the repository path;
33+
// otherwise it is the config file path.
34+
if explainMode {
35+
explainRepo = arg
36+
} else {
37+
cfgPath = arg
38+
}
2539
}
2640
}
2741

@@ -32,6 +46,11 @@ func main() {
3246
log.Fatalf("failed to create engine: %v", err)
3347
}
3448

49+
if explainMode {
50+
runExplain(ctx, eng, cfg, explainRepo)
51+
os.Exit(0)
52+
}
53+
3554
if generateMode {
3655
repoPath, err := filepath.Abs(cfg.Repo)
3756
if err != nil {
@@ -68,3 +87,24 @@ func main() {
6887
log.Fatalf("server error: %v", err)
6988
}
7089
}
90+
91+
// runExplain indexes the given repository (defaulting to the configured repo)
92+
// and prints a human-readable statistical summary to stdout.
93+
func runExplain(ctx context.Context, eng *bootstrap.Engine, cfg *config.Config, repoArg string) {
94+
repo := repoArg
95+
if repo == "" {
96+
repo = cfg.Repo
97+
}
98+
repoPath, err := filepath.Abs(repo)
99+
if err != nil {
100+
log.Fatalf("failed to resolve repo path: %v", err)
101+
}
102+
103+
fmt.Fprintf(os.Stderr, "Analyzing %s …\n", repoPath)
104+
if _, err := eng.GenerateSnapshot(ctx, repoPath, false); err != nil {
105+
log.Fatalf("snapshot generation failed: %v", err)
106+
}
107+
108+
report := explain.Compute(eng)
109+
fmt.Print(report.Render())
110+
}

internal/extractors/javaextractor/java.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,21 @@ func resolveImport(f *facts.Fact, typeDir, packageDir map[string]string) {
152152
// Wildcard / package import (e.g. "com.example.foo").
153153
dir, ok = packageDir[imp]
154154
}
155+
if !ok {
156+
// Parent-FQN fallback for static-member imports
157+
// ("com.foo.Constants.MAX" -> declaring type "com.foo.Constants") and
158+
// imports of internal types we didn't index ("com.foo.Bar" -> package
159+
// "com.foo"). Skipped for wildcards, whose import string is already the
160+
// package — walking to the grandparent would mis-resolve. Only our own
161+
// types/packages are in the indices, so this never flags an external import.
162+
if wc, _ := f.Props["wildcard"].(bool); !wc {
163+
if parent := parentName(imp); parent != "" {
164+
if dir, ok = typeDir[parent]; !ok {
165+
dir, ok = packageDir[parent]
166+
}
167+
}
168+
}
169+
}
155170
if !ok {
156171
return // external dependency
157172
}

internal/extractors/javaextractor/java_ast.go

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -177,16 +177,27 @@ func (w *astWalker) handleImport(node *sitter.Node) {
177177
}
178178
importPath := nodeText(pathNode, w.src)
179179

180+
props := map[string]any{
181+
"language": "java",
182+
"import": importPath,
183+
"source": "external", // refined to "internal" in canonicalizeTargets
184+
}
185+
// Mark the import shape so resolveImport can apply the parent-FQN fallback to
186+
// static-member / un-indexed-type imports but NOT to wildcards (whose import
187+
// string is already the package — walking to the grandparent would mis-resolve).
188+
if isStatic {
189+
props["static"] = true
190+
}
191+
if isWildcard {
192+
props["wildcard"] = true
193+
}
194+
180195
w.out = append(w.out, facts.Fact{
181-
Kind: facts.KindDependency,
182-
Name: w.dir + " -> " + importPath,
183-
File: w.relFile,
184-
Line: int(node.StartPosition().Row) + 1,
185-
Props: map[string]any{
186-
"language": "java",
187-
"import": importPath,
188-
"source": "external", // refined to "internal" in canonicalizeTargets
189-
},
196+
Kind: facts.KindDependency,
197+
Name: w.dir + " -> " + importPath,
198+
File: w.relFile,
199+
Line: int(node.StartPosition().Row) + 1,
200+
Props: props,
190201
Relations: []facts.Relation{
191202
{Kind: facts.RelImports, Target: importPath},
192203
},
@@ -421,10 +432,10 @@ func (w *astWalker) handleField(node *sitter.Node, owner *facts.Fact) {
421432
}
422433
}
423434
w.out = append(w.out, facts.Fact{
424-
Kind: facts.KindSymbol,
425-
Name: w.canonicalName(w.qualify(name)),
426-
File: w.relFile,
427-
Line: int(c.StartPosition().Row) + 1,
435+
Kind: facts.KindSymbol,
436+
Name: w.canonicalName(w.qualify(name)),
437+
File: w.relFile,
438+
Line: int(c.StartPosition().Row) + 1,
428439
Props: props,
429440
Relations: []facts.Relation{
430441
{Kind: facts.RelDeclares, Target: w.dir},

internal/extractors/javaextractor/java_ast_test.go

Lines changed: 96 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,9 @@ public class Order {
124124

125125
func TestExtract_InterfaceEnumRecord(t *testing.T) {
126126
ff := extractAll(t, map[string]string{
127-
"a/Shape.java": "package a;\npublic interface Shape { double area(); }\n",
128-
"a/Color.java": "package a;\npublic enum Color { RED, GREEN, BLUE }\n",
129-
"a/Point.java": "package a;\npublic record Point(int x, int y) {}\n",
127+
"a/Shape.java": "package a;\npublic interface Shape { double area(); }\n",
128+
"a/Color.java": "package a;\npublic enum Color { RED, GREEN, BLUE }\n",
129+
"a/Point.java": "package a;\npublic record Point(int x, int y) {}\n",
130130
})
131131

132132
iface, _ := findFact(ff, "a.Shape")
@@ -201,6 +201,99 @@ public class Service {
201201
}
202202
}
203203

204+
// TestExtract_StaticImportResolvesInternal covers the parent-FQN fallback:
205+
// a static member import names the member, not the type, so the declaring type's
206+
// FQN is the parent of the import string.
207+
func TestExtract_StaticImportResolvesInternal(t *testing.T) {
208+
ff := extractAll(t, map[string]string{
209+
"app/svc/Service.java": `package app.svc;
210+
211+
import static app.data.Constants.MAX;
212+
213+
public class Service {
214+
int v = MAX;
215+
}
216+
`,
217+
"app/data/Constants.java": "package app.data;\npublic class Constants { public static final int MAX = 1; }\n",
218+
})
219+
220+
var ok bool
221+
for _, f := range factsByKind(ff, facts.KindDependency) {
222+
if f.Props["import"] == "app.data.Constants.MAX" {
223+
if f.Props["source"] == "internal" && hasRelation(f, facts.RelImports, "app/data") {
224+
ok = true
225+
}
226+
}
227+
}
228+
if !ok {
229+
t.Error("static import app.data.Constants.MAX should resolve internal to module app/data")
230+
}
231+
}
232+
233+
// TestExtract_UnindexedTypeResolvesViaPackage covers the second fallback branch:
234+
// an imported type we didn't index as a top-level class still resolves to its
235+
// package's module dir, because the package is internal.
236+
func TestExtract_UnindexedTypeResolvesViaPackage(t *testing.T) {
237+
ff := extractAll(t, map[string]string{
238+
"app/svc/Service.java": `package app.svc;
239+
240+
import app.data.Repo.Inner;
241+
242+
public class Service {}
243+
`,
244+
"app/data/Repo.java": "package app.data;\npublic class Repo { public static class Inner {} }\n",
245+
})
246+
247+
var ok bool
248+
for _, f := range factsByKind(ff, facts.KindDependency) {
249+
if f.Props["import"] == "app.data.Repo.Inner" {
250+
// Resolves via parent type app.data.Repo (or package app.data) → app/data.
251+
if f.Props["source"] == "internal" && hasRelation(f, facts.RelImports, "app/data") {
252+
ok = true
253+
}
254+
}
255+
}
256+
if !ok {
257+
t.Error("import of un-indexed type app.data.Repo.Inner should resolve internal to app/data")
258+
}
259+
}
260+
261+
// TestExtract_WildcardNotOverResolved guards that the parent-FQN fallback is NOT
262+
// applied to wildcard imports: an external wildcard stays external (it must not
263+
// walk to a grandparent), while an internal wildcard still resolves normally.
264+
func TestExtract_WildcardNotOverResolved(t *testing.T) {
265+
ff := extractAll(t, map[string]string{
266+
"app/svc/Service.java": `package app.svc;
267+
268+
import app.data.*;
269+
import com.external.lib.*;
270+
271+
public class Service {}
272+
`,
273+
"app/data/Repo.java": "package app.data;\npublic class Repo {}\n",
274+
})
275+
276+
var internalWildcardOK, externalWildcardExternal = false, true
277+
for _, f := range factsByKind(ff, facts.KindDependency) {
278+
switch f.Props["import"] {
279+
case "app.data":
280+
if f.Props["source"] == "internal" && hasRelation(f, facts.RelImports, "app/data") {
281+
internalWildcardOK = true
282+
}
283+
case "com.external.lib":
284+
if f.Props["source"] != "external" {
285+
externalWildcardExternal = false
286+
}
287+
}
288+
}
289+
if !internalWildcardOK {
290+
t.Error("internal wildcard import app.data.* should resolve to app/data")
291+
}
292+
if !externalWildcardExternal {
293+
t.Error("external wildcard import com.external.lib.* must stay external (no grandparent fallback)")
294+
}
295+
}
296+
204297
func TestExtract_InstantiatesAndCalls(t *testing.T) {
205298
ff := extractAll(t, map[string]string{
206299
"m/Widget.java": "package m;\npublic class Widget {}\n",

internal/extractors/kotlinextractor/kotlin.go

Lines changed: 67 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -311,38 +311,82 @@ func extractTypeName(s string) string {
311311

312312
// --- Source-root and import resolution (project-level) ---
313313

314-
// detectKotlinSourceRoot derives the source root directory from the first
315-
// Kotlin file's package declaration. For "app/src/main/java/com/foo/Bar.kt"
316-
// declaring `package com.foo`, it returns "app/src/main/java/".
314+
// detectKotlinSourceRoot derives the source-root directory shared by the
315+
// project's production Kotlin files, by stripping each file's package path from
316+
// its directory. For "app/src/main/java/com/foo/Bar.kt" declaring `package
317+
// com.foo`, the per-file root is "app/src/main/java/".
318+
//
319+
// It deliberately ignores test source sets (src/test, src/androidTest) and picks
320+
// the MOST COMMON production root rather than the first file seen. File order is
321+
// not guaranteed: with the old "first file wins" logic, a project whose first
322+
// walked file was a test ("app/src/androidTest/java/…") resolved every internal
323+
// import under that test root, so the targets never matched the real (main)
324+
// module dirs and coupling collapsed to zero.
317325
func detectKotlinSourceRoot(repoPath string, files []string) string {
326+
counts := make(map[string]int) // production source root -> file count
327+
fallback := "" // any root seen, used only if all files are tests
328+
haveFallback := false
329+
318330
for _, relFile := range files {
319331
if !isKotlinFile(relFile) {
320332
continue
321333
}
322-
absFile := filepath.Join(repoPath, relFile)
323-
f, err := os.Open(absFile)
324-
if err != nil {
334+
root, ok := kotlinFileSourceRoot(repoPath, relFile)
335+
if !ok {
325336
continue
326337
}
327-
scanner := bufio.NewScanner(f)
328-
for scanner.Scan() {
329-
line := scanner.Text()
330-
if m := packageRe.FindStringSubmatch(line); m != nil {
331-
pkg := m[1]
332-
pkgPath := strings.ReplaceAll(pkg, ".", "/")
333-
dir := filepath.ToSlash(filepath.Dir(relFile))
334-
if strings.HasSuffix(dir, pkgPath) {
335-
root := strings.TrimSuffix(dir, pkgPath)
336-
f.Close()
337-
return root
338-
}
339-
f.Close()
340-
return ""
341-
}
338+
if !haveFallback {
339+
fallback, haveFallback = root, true
342340
}
343-
f.Close()
341+
if isKotlinTestSource(relFile) {
342+
continue
343+
}
344+
counts[root]++
344345
}
345-
return ""
346+
347+
best, bestN, found := "", 0, false
348+
for root, n := range counts {
349+
if !found || n > bestN || (n == bestN && root < best) {
350+
best, bestN, found = root, n, true
351+
}
352+
}
353+
if found {
354+
return best
355+
}
356+
return fallback
357+
}
358+
359+
// kotlinFileSourceRoot returns a single file's source root: its directory with
360+
// its package path stripped. ok is false when the file has no package decl.
361+
func kotlinFileSourceRoot(repoPath, relFile string) (string, bool) {
362+
absFile := filepath.Join(repoPath, relFile)
363+
f, err := os.Open(absFile)
364+
if err != nil {
365+
return "", false
366+
}
367+
defer f.Close()
368+
scanner := bufio.NewScanner(f)
369+
for scanner.Scan() {
370+
m := packageRe.FindStringSubmatch(scanner.Text())
371+
if m == nil {
372+
continue
373+
}
374+
pkgPath := strings.ReplaceAll(m[1], ".", "/")
375+
dir := filepath.ToSlash(filepath.Dir(relFile))
376+
if strings.HasSuffix(dir, pkgPath) {
377+
return strings.TrimSuffix(dir, pkgPath), true
378+
}
379+
return "", true // package found but dir doesn't mirror it — root is ""
380+
}
381+
return "", false
382+
}
383+
384+
// isKotlinTestSource reports whether a file lives in a Gradle test source set
385+
// (src/test or src/androidTest), which must not drive source-root detection.
386+
func isKotlinTestSource(relFile string) bool {
387+
p := filepath.ToSlash(relFile)
388+
return strings.Contains(p, "/src/test/") || strings.HasPrefix(p, "src/test/") ||
389+
strings.Contains(p, "/src/androidTest/") || strings.HasPrefix(p, "src/androidTest/")
346390
}
347391

348392
// detectKotlinBasePackage reads the Android namespace from build.gradle.kts so

0 commit comments

Comments
 (0)