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
17 changes: 14 additions & 3 deletions blast_radius.go
Original file line number Diff line number Diff line change
Expand Up @@ -1468,7 +1468,17 @@ func pluralFiles(n int, noun string) string {
}

func renderImportersReport(w io.Writer, report scanner.ImportersReport) {
if scanner.CountHubImporters(report.Importers) >= scanner.HubThreshold {
if report.Package != "" && report.PackageSiblings > 0 && len(report.Importers) > 0 {
fmt.Fprintf(w, "📍 File: %s\n", report.File)
fmt.Fprintf(w, " Package %s is imported by %s (Go imports name packages, not files)\n", report.Package, pluralFiles(len(report.Importers), "file"))
for i, imp := range report.Importers {
if i >= 5 {
fmt.Fprintf(w, " ... and %d more\n", len(report.Importers)-5)
break
}
fmt.Fprintf(w, " • %s\n", imp)
}
} else if scanner.CountHubImporters(report.Importers) >= scanner.HubThreshold {
fmt.Fprintf(w, "⚠️ HUB FILE: %s\n", report.File)
fmt.Fprintf(w, " Imported by %d files%s - changes have wide impact!\n", len(report.Importers), importersGranularity(report))
fmt.Fprintln(w)
Expand All @@ -1494,7 +1504,8 @@ func renderImportersReport(w io.Writer, report scanner.ImportersReport) {
}
// Every non-test file of a hub Go package is a hub, so list the
// package once with its file count rather than each file.
fmt.Fprintf(w, " Imports %d hub(s): %s\n", len(report.HubImports), strings.Join(scanner.CollapseGoPackageFiles(report.HubImports), ", "))
hubs := scanner.CollapseGoPackageFiles(report.HubImports)
fmt.Fprintf(w, " Imports %d hub(s): %s\n", len(hubs), strings.Join(hubs, ", "))
}

renderCoverage(w, report.CoverageStatus, report.CoverageNotes)
Expand Down Expand Up @@ -1554,7 +1565,7 @@ func buildImportersReportFromGraph(root, file string, fg *scanner.FileGraph) (sc
Importers: importers,
Imports: imports,
ImporterCount: len(importers),
IsHub: fg.IsHub(file),
IsHub: fg.IsFileHub(file),
NotIndexed: !fg.Indexed(file),
CoverageStatus: string(coverage.EffectiveStatus()),
CoverageNotes: append([]string(nil), coverage.Notes...),
Expand Down
15 changes: 10 additions & 5 deletions cmd/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -452,13 +452,17 @@ func hookSessionStart(root string) error {
info := getHubInfo(root)
if info != nil && len(info.Hubs) > 0 {
fmt.Println("⚠️ High-impact files (hubs):")
for i, hub := range info.Hubs {
entries := scanner.CollapseGoPackageHubs(info.Hubs, info.Importers)
for i, entry := range entries {
if i >= maxHubs {
fmt.Printf(" ... and %d more\n", len(info.Hubs)-maxHubs)
fmt.Printf(" ... and %d more\n", len(entries)-maxHubs)
break
}
importers := len(info.Importers[hub])
fmt.Printf(" ⚠️ HUB FILE: %s (imported by %s)\n", hub, filesText(importers))
if entry.IsPackage() {
fmt.Printf(" ⚠️ HUB PACKAGE: %s (%d files, imported by %s)\n", entry.Path, len(entry.Files), filesText(entry.Importers))
continue
}
fmt.Printf(" ⚠️ HUB FILE: %s (imported by %s)\n", entry.Path, filesText(entry.Importers))
}
} else if shouldSkipHubAnalysis(fileCount, fileCountKnown) {
fmt.Printf("ℹ️ Hub analysis skipped for large repo (%d files)\n", fileCount)
Expand Down Expand Up @@ -2340,7 +2344,8 @@ func checkFileImportersWithPhase(root, filePath, phase string) error {
}
}
if len(hubImports) > 0 {
fmt.Printf(" Imports %d hub(s): %s\n", len(hubImports), strings.Join(scanner.CollapseGoPackageFiles(hubImports), ", "))
hubs := scanner.CollapseGoPackageFiles(hubImports)
fmt.Printf(" Imports %d hub(s): %s\n", len(hubs), strings.Join(hubs, ", "))
fmt.Println()
}

Expand Down
12 changes: 10 additions & 2 deletions find/find.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ type Hit struct {
Matched []string `json:"matched"`
Importers int `json:"importers"`
Hub bool `json:"hub"`
// Package is set when Importers counts the importers of the file's
// multi-file Go package rather than of the file itself.
Package string `json:"package,omitempty"`
}

type Report struct {
Expand Down Expand Up @@ -69,7 +72,10 @@ func Run(ctx context.Context, root string, filters scanner.Filters, query string
for i := range report.Hits {
importers := fg.Importers[report.Hits[i].Path]
report.Hits[i].Importers = len(importers)
report.Hits[i].Hub = scanner.CountHubImporters(importers) >= scanner.HubThreshold
report.Hits[i].Hub = fg.IsFileHub(report.Hits[i].Path)
if pkg, shared := fg.SharedGoPackage(report.Hits[i].Path); shared {
report.Hits[i].Package = pkg
}
}
}
report.Coverage = scanner.CoverageFromSources(sources)
Expand Down Expand Up @@ -233,7 +239,9 @@ func Render(w io.Writer, r Report) {
fmt.Fprintf(w, " matched: %s\n", strings.Join(h.Matched, ", "))
}
hub := ""
if h.Hub {
if h.Package != "" {
hub = " (of package " + h.Package + ")"
} else if h.Hub {
hub = " (hub)"
}
fmt.Fprintf(w, " importers: %d%s\n", h.Importers, hub)
Expand Down
29 changes: 17 additions & 12 deletions handoff/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,24 +364,29 @@ func summarizeHubs(importersByFile map[string][]string, maxHubs int) []HubSummar
return []HubSummary{}
}

hubs := make([]HubSummary, 0, len(importersByFile))
paths := make([]string, 0, len(importersByFile))
for path, importers := range importersByFile {
if len(importers) < 3 {
continue
if len(importers) >= 3 {
paths = append(paths, path)
}
hubs = append(hubs, HubSummary{
Path: path,
Importers: len(importers),
})
}

sort.Slice(hubs, func(i, j int) bool {
if hubs[i].Importers != hubs[j].Importers {
return hubs[i].Importers > hubs[j].Importers
sort.Slice(paths, func(i, j int) bool {
if len(importersByFile[paths[i]]) != len(importersByFile[paths[j]]) {
return len(importersByFile[paths[i]]) > len(importersByFile[paths[j]])
}
return hubs[i].Path < hubs[j].Path
return paths[i] < paths[j]
})

entries := scanner.CollapseGoPackageHubs(paths, importersByFile)
hubs := make([]HubSummary, 0, len(entries))
for _, entry := range entries {
hub := HubSummary{Path: entry.Path, Importers: entry.Importers}
if entry.IsPackage() {
hub.Files = len(entry.Files)
}
hubs = append(hubs, hub)
}

if maxHubs > 0 && len(hubs) > maxHubs {
hubs = hubs[:maxHubs]
}
Expand Down
21 changes: 21 additions & 0 deletions handoff/build_more_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package handoff
import (
"context"
"reflect"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -137,6 +138,26 @@ func TestSummarizeHubsFiltersSortsAndCaps(t *testing.T) {
}
}

// Every file of a Go package carries the package's importers (#285), so the
// prefix lists the package once instead of one identical row per file.
func TestSummarizeHubsListsGoPackageOnce(t *testing.T) {
pkg := []string{"cmd/a.go", "cmd/b.go", "main.go"}
importers := map[string][]string{
"scanner/filegraph.go": pkg,
"scanner/types.go": pkg,
"scanner/walker.go": pkg,
"web/app.ts": pkg,
}
got := summarizeHubs(importers, 0)
want := []HubSummary{{Path: "scanner/", Importers: 3, Files: 3}, {Path: "web/app.ts", Importers: 3}}
if !reflect.DeepEqual(got, want) {
t.Fatalf("summarizeHubs = %v, want %v", got, want)
}
if out := RenderPrefixMarkdown(PrefixSnapshot{Hubs: got}); !strings.Contains(out, "- `scanner/` (package, 3 files, 3 importers)\n") {
t.Fatalf("prefix markdown missing the package row:\n%s", out)
}
}

func TestGitCurrentBranchContextErrors(t *testing.T) {
if _, err := gitCurrentBranchContext(context.Background(), t.TempDir()); err == nil {
t.Fatal("expected error outside a git repository")
Expand Down
4 changes: 2 additions & 2 deletions handoff/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func RenderMarkdown(a *Artifact) string {
b.WriteString(fmt.Sprintf("- ... and %d more\n", len(a.Prefix.Hubs)-15))
break
}
b.WriteString(fmt.Sprintf("- `%s` (%d importers)\n", hub.Path, hub.Importers))
b.WriteString("- " + hub.String() + "\n")
}
}

Expand Down Expand Up @@ -113,7 +113,7 @@ func RenderPrefixMarkdown(p PrefixSnapshot) string {
}
b.WriteString(fmt.Sprintf("- Hub files: %d\n", len(p.Hubs)))
for _, hub := range p.Hubs {
b.WriteString(fmt.Sprintf("- `%s` (%d importers)\n", hub.Path, hub.Importers))
b.WriteString("- " + hub.String() + "\n")
}
return b.String()
}
Expand Down
11 changes: 11 additions & 0 deletions handoff/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package handoff

import (
"context"
"fmt"
"time"

"codemap/watch"
Expand All @@ -17,6 +18,16 @@ const (
type HubSummary struct {
Path string `json:"path"`
Importers int `json:"importers"`
// Files is set when Path is a multi-file Go package directory listed once.
Files int `json:"files,omitempty"`
}

// String is the markdown list form of the hub.
func (h HubSummary) String() string {
if h.Files > 0 {
return fmt.Sprintf("`%s` (package, %d files, %d importers)", h.Path, h.Files, h.Importers)
}
return fmt.Sprintf("`%s` (%d importers)", h.Path, h.Importers)
}

// FileStub is a lightweight file descriptor for lazy detail loading.
Expand Down
54 changes: 53 additions & 1 deletion importers_go_package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"context"
"os"
"path/filepath"
"strings"
"testing"
Expand Down Expand Up @@ -40,7 +41,7 @@ func TestImportersCLIReportsGoPackageGranularity(t *testing.T) {
if report.Package != "example.com/demo/multi" || report.PackageSiblings != 1 {
t.Fatalf("report package = %q siblings = %d, want example.com/demo/multi with 1 sibling", report.Package, report.PackageSiblings)
}
want := " Imported by 1 file(s) (package example.com/demo/multi: Go imports name packages, not files)\n • app/main.go\n"
want := " Package example.com/demo/multi is imported by 1 file (Go imports name packages, not files)\n • app/main.go\n"
if !strings.Contains(out, want) {
t.Fatalf("--importers multi/a.go output:\n%q\nwant it to contain:\n%q", out, want)
}
Expand All @@ -62,6 +63,57 @@ func TestImportersCLIReportsGoPackageGranularity(t *testing.T) {
}
}

// Three importers of a two-file package make the package a hub, not the file:
// --importers names the package as the thing imported and raises no HUB FILE
// warning, while a one-file package at the same count still does (#285).
func TestImportersCLIGoPackageHubIsNotAFileHub(t *testing.T) {
root := t.TempDir()
files := map[string]string{
"go.mod": "module example.com/hub\n\ngo 1.21\n",
"multi/a.go": "package multi\n\nfunc A() {}\n",
"multi/b.go": "package multi\n\nfunc B() {}\n",
"single/single.go": "package single\n\nfunc S() {}\n",
}
for _, user := range []string{"u1", "u2", "u3"} {
files[user+"/"+user+".go"] = "package " + user + "\n\nimport (\n\t\"example.com/hub/multi\"\n\t\"example.com/hub/single\"\n)\n\nfunc F() { multi.A(); single.S() }\n"
}
for rel, body := range files {
path := filepath.Join(root, filepath.FromSlash(rel))
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(path, []byte(body), 0o644); err != nil {
t.Fatal(err)
}
}
fg, err := scanner.BuildFileGraph(context.Background(), root, scanner.Filters{})
if err != nil {
t.Fatalf("BuildFileGraph: %v", err)
}
render := func(file string) (scanner.ImportersReport, string) {
t.Helper()
report, err := buildImportersReportFromGraph(root, file, fg)
if err != nil {
t.Fatalf("buildImportersReportFromGraph(%s): %v", file, err)
}
var buf strings.Builder
renderImportersReportCLI(&buf, report)
return report, buf.String()
}

report, out := render("multi/a.go")
if report.IsHub || strings.Contains(out, "HUB") {
t.Fatalf("multi/a.go flagged as a hub file (is_hub=%v):\n%s", report.IsHub, out)
}
if !strings.Contains(out, "Package example.com/hub/multi is imported by 3 files") {
t.Fatalf("--importers multi/a.go must name the package as imported:\n%s", out)
}
report, out = render("single/single.go")
if !report.IsHub || !strings.Contains(out, "HUB FILE: single/single.go") {
t.Fatalf("one-file package lost its hub warning (is_hub=%v):\n%s", report.IsHub, out)
}
}

// End to end through the binary, as the issue reproduced it.
func TestImportersCommandListsMultiFilePackageImporter(t *testing.T) {
root, err := filepath.Abs("testdata/go-multi-file-package")
Expand Down
32 changes: 24 additions & 8 deletions mcp/analysis_output.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,14 @@ import (
const maxStructuredItems = 500

type StructureOutput struct {
Kind string `json:"kind"`
Project scanner.Project `json:"project"`
Hubs []string `json:"hubs"`
Truncated bool `json:"truncated"`
OmittedCount int `json:"omitted_count"`
Kind string `json:"kind"`
Project scanner.Project `json:"project"`
Hubs []string `json:"hubs"`
// HubEntries is Hubs as the text output lists them: a multi-file Go
// package is one entry whose files share one importer set.
HubEntries []scanner.HubEntry `json:"hub_entries"`
Truncated bool `json:"truncated"`
OmittedCount int `json:"omitted_count"`
}

type DiffOutput struct {
Expand All @@ -35,6 +38,12 @@ type ImportersOutput struct {
HubImports []string `json:"hub_imports"`
ImporterCount int `json:"importer_count"`
IsHub bool `json:"is_hub"`
// Granularity is "package" when importer_count counts importers of the
// Go package (Package) that File belongs to, else "file". PackageHub is
// true when that package is a hub though File itself is not claimed one.
Granularity string `json:"granularity"`
Package string `json:"package,omitempty"`
PackageHub bool `json:"package_hub,omitempty"`
// NotIndexed mirrors scanner.ImportersReport.NotIndexed: true when the
// queried file was never part of the scanned set, so importer_count 0 is
// a coverage gap, not a confirmed zero. NotIndexedReason names the one
Expand Down Expand Up @@ -67,10 +76,11 @@ func boundedProject(project scanner.Project) (scanner.Project, int) {
return bounded, omittedCount
}

func newStructureOutput(project scanner.Project, hubs []string) StructureOutput {
func newStructureOutput(project scanner.Project, hubs []string, entries []scanner.HubEntry) StructureOutput {
bounded, omittedCount := boundedProject(project)
boundedHubs, omittedCount := addBounded(hubs, omittedCount)
return StructureOutput{Kind: "structure", Project: bounded, Hubs: boundedHubs, Truncated: omittedCount > 0, OmittedCount: omittedCount}
boundedEntries, omittedCount := addMirrorBounded(entries, len(hubs), omittedCount)
return StructureOutput{Kind: "structure", Project: bounded, Hubs: boundedHubs, HubEntries: boundedEntries, Truncated: omittedCount > 0, OmittedCount: omittedCount}
}

func newDiffOutput(kind, ref string, project scanner.Project) DiffOutput {
Expand Down Expand Up @@ -98,10 +108,16 @@ func newImportersOutput(root, file string, graph *scanner.FileGraph) ImportersOu
kind = "empty"
}
coverage := graph.FileCoverage(file)
granularity, pkg, packageHub := "file", "", false
if goPkg, pkgFiles, ok := graph.GoPackage(file); ok {
granularity, pkg = "package", goPkg
packageHub = len(pkgFiles) > 1 && graph.IsHub(file)
}
return ImportersOutput{
Kind: kind, Root: root, Mode: "importers", File: file,
Importers: boundedImporters, Imports: boundedImports, HubImports: boundedHubs,
ImporterCount: len(importers), IsHub: graph.IsHub(file),
ImporterCount: len(importers), IsHub: graph.IsFileHub(file),
Comment thread
JordanCoin marked this conversation as resolved.
Resolved
Granularity: granularity, Package: pkg, PackageHub: packageHub,
CoverageStatus: string(coverage.EffectiveStatus()), CoverageNotes: nonNilCopy(coverage.Notes),
Truncated: omittedCount > 0, OmittedCount: omittedCount,
}
Expand Down
Loading
Loading