diff --git a/README.md b/README.md index 2bf0ca2..1b65bda 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,9 @@ diffguard /path/to/repo # Specify a base branch diffguard --base main /path/to/repo +# Restrict diff mode to a subtree +diffguard --base develop --include-paths miner/ /path/to/repo + # Refactoring mode: analyze entire files/dirs (no diff required) diffguard --paths internal/foo/bar.go /path/to/repo diffguard --paths internal/foo/,internal/bar/ /path/to/repo @@ -61,9 +64,9 @@ diffguard \ ### Modes -**Diff mode (default):** Analyzes only the regions changed between `HEAD` and the base branch. Use this as a CI gate for PRs. +**Diff mode (default):** Analyzes only the regions changed between `HEAD` and the base branch. Use this as a CI gate for PRs. Add `--include-paths` to limit diff mode to specific files or directories after the git diff is computed. -**Refactoring mode (`--paths`):** Analyzes the full content of the specified files or directories, ignoring git diff entirely. Use this when iterating on an existing file's quality without a base to compare against. +**Refactoring mode (`--paths`):** Analyzes the full content of the specified files or directories, ignoring git diff entirely. Use this when iterating on an existing file's quality without a base to compare against. `--paths` is mutually exclusive with `--base` and `--include-paths`. ## What It Measures @@ -158,6 +161,7 @@ diffguard [flags] Flags: --base string Base branch to diff against (default: auto-detect) + --include-paths string Comma-separated files/dirs to restrict diff mode to after git diff --paths string Comma-separated files/dirs to analyze in full (refactoring mode); skips git diff --complexity-threshold int Maximum cognitive complexity per function (default 10) --function-size-threshold int Maximum lines per function (default 50) diff --git a/cmd/diffguard/main.go b/cmd/diffguard/main.go index 95bffdb..2ac97bb 100644 --- a/cmd/diffguard/main.go +++ b/cmd/diffguard/main.go @@ -34,6 +34,7 @@ func main() { flag.StringVar(&cfg.FailOn, "fail-on", "warn", "Exit non-zero if thresholds breached: none, warn, all") flag.StringVar(&cfg.BaseBranch, "base", "", "Base branch to diff against (default: auto-detect)") flag.StringVar(&cfg.Paths, "paths", "", "Comma-separated files/dirs to analyze in full (refactoring mode); skips git diff") + flag.StringVar(&cfg.IncludePaths, "include-paths", "", "Comma-separated files/dirs to restrict diff mode to after git diff") flag.Parse() if flag.NArg() < 1 { @@ -74,9 +75,14 @@ type Config struct { FailOn string BaseBranch string Paths string + IncludePaths string } func run(repoPath string, cfg Config) error { + if err := validateConfig(cfg); err != nil { + return err + } + d, err := loadFiles(repoPath, cfg) if err != nil { return err @@ -102,11 +108,7 @@ func run(repoPath string, cfg Config) error { } func announceRun(d *diff.Result, cfg Config) { - if cfg.Paths != "" { - fmt.Fprintf(os.Stderr, "Analyzing %d Go files (refactoring mode)...\n", len(d.Files)) - } else { - fmt.Fprintf(os.Stderr, "Analyzing %d changed Go files against %s...\n", len(d.Files), cfg.BaseBranch) - } + fmt.Fprintln(os.Stderr, announceMessage(len(d.Files), cfg)) } func runAnalyses(repoPath string, d *diff.Result, cfg Config) ([]report.Section, error) { @@ -182,20 +184,78 @@ func checkExitCode(r report.Report, failOn string) error { func loadFiles(repoPath string, cfg Config) (*diff.Result, error) { if cfg.Paths != "" { - paths := strings.Split(cfg.Paths, ",") - for i := range paths { - paths[i] = strings.TrimSpace(paths[i]) - } - d, err := diff.CollectPaths(repoPath, paths) - if err != nil { - return nil, fmt.Errorf("collecting paths: %w", err) + return loadRefactoringFiles(repoPath, cfg.Paths) + } + return loadDiffFiles(repoPath, cfg) +} + +func validateConfig(cfg Config) error { + if cfg.Paths != "" && cfg.BaseBranch != "" { + return fmt.Errorf("--paths and --base are mutually exclusive; use --include-paths to scope diff mode") + } + if cfg.Paths != "" && cfg.IncludePaths != "" { + return fmt.Errorf("--paths and --include-paths are mutually exclusive") + } + return nil +} + +func parsePathList(raw, flagName string) ([]string, error) { + parts := strings.Split(raw, ",") + paths := make([]string, 0, len(parts)) + for _, part := range parts { + part = strings.TrimSpace(part) + if part != "" { + paths = append(paths, part) } - return d, nil } + if len(paths) == 0 { + return nil, fmt.Errorf("--%s requires at least one path", flagName) + } + return paths, nil +} + +func announceMessage(fileCount int, cfg Config) string { + if cfg.Paths != "" { + return fmt.Sprintf("Analyzing %d Go files (refactoring mode)...", fileCount) + } + if cfg.IncludePaths != "" { + return fmt.Sprintf("Analyzing %d changed Go files against %s (filtered to %s)...", fileCount, cfg.BaseBranch, cfg.IncludePaths) + } + return fmt.Sprintf("Analyzing %d changed Go files against %s...", fileCount, cfg.BaseBranch) +} + +func loadRefactoringFiles(repoPath, rawPaths string) (*diff.Result, error) { + paths, err := parsePathList(rawPaths, "paths") + if err != nil { + return nil, err + } + d, err := diff.CollectPaths(repoPath, paths) + if err != nil { + return nil, fmt.Errorf("collecting paths: %w", err) + } + return d, nil +} + +func loadDiffFiles(repoPath string, cfg Config) (*diff.Result, error) { d, err := diff.Parse(repoPath, cfg.BaseBranch) if err != nil { return nil, fmt.Errorf("parsing diff: %w", err) } + return filterDiffFiles(repoPath, d, cfg.IncludePaths) +} + +func filterDiffFiles(repoPath string, d *diff.Result, rawPaths string) (*diff.Result, error) { + if rawPaths == "" { + return d, nil + } + paths, err := parsePathList(rawPaths, "include-paths") + if err != nil { + return nil, err + } + d, err = diff.FilterPaths(repoPath, d, paths) + if err != nil { + return nil, fmt.Errorf("filtering paths: %w", err) + } return d, nil } diff --git a/cmd/diffguard/main_test.go b/cmd/diffguard/main_test.go new file mode 100644 index 0000000..88b2ca8 --- /dev/null +++ b/cmd/diffguard/main_test.go @@ -0,0 +1,165 @@ +package main + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/0xPolygon/diffguard/internal/diff" +) + +func TestRun_RejectsPathsWithBase(t *testing.T) { + err := run(t.TempDir(), Config{Paths: "miner", BaseBranch: "develop"}) + if err == nil { + t.Fatal("expected error for --paths with --base") + } + if !strings.Contains(err.Error(), "--paths and --base are mutually exclusive") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestRun_RejectsPathsWithIncludePaths(t *testing.T) { + err := run(t.TempDir(), Config{Paths: "miner", IncludePaths: "miner"}) + if err == nil { + t.Fatal("expected error for --paths with --include-paths") + } + if !strings.Contains(err.Error(), "--paths and --include-paths are mutually exclusive") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestAnnounceMessage(t *testing.T) { + tests := []struct { + name string + cfg Config + want string + }{ + { + name: "refactoring mode", + cfg: Config{Paths: "miner"}, + want: "Analyzing 2 Go files (refactoring mode)...", + }, + { + name: "filtered diff mode", + cfg: Config{BaseBranch: "develop", IncludePaths: "miner"}, + want: "Analyzing 2 changed Go files against develop (filtered to miner)...", + }, + { + name: "plain diff mode", + cfg: Config{BaseBranch: "develop"}, + want: "Analyzing 2 changed Go files against develop...", + }, + } + + for _, tt := range tests { + if got := announceMessage(2, tt.cfg); got != tt.want { + t.Fatalf("%s: announceMessage() = %q, want %q", tt.name, got, tt.want) + } + } +} + +func TestLoadFiles_IncludePathsFiltersDiff(t *testing.T) { + repo := initGitRepo(t) + writeFile(t, filepath.Join(repo, "miner", "a.go"), "package miner\n\nfunc A() int { return 1 }\n") + writeFile(t, filepath.Join(repo, "other", "b.go"), "package other\n\nfunc B() int { return 1 }\n") + git(t, repo, "add", ".") + git(t, repo, "commit", "-m", "initial") + + git(t, repo, "checkout", "-b", "feature") + writeFile(t, filepath.Join(repo, "miner", "a.go"), "package miner\n\nfunc A() int { return 2 }\n") + writeFile(t, filepath.Join(repo, "other", "b.go"), "package other\n\nfunc B() int { return 2 }\n") + git(t, repo, "add", ".") + git(t, repo, "commit", "-m", "change") + + d, err := loadFiles(repo, Config{BaseBranch: "develop", IncludePaths: "miner"}) + if err != nil { + t.Fatalf("loadFiles error: %v", err) + } + + if len(d.Files) != 1 { + t.Fatalf("expected 1 filtered file, got %d", len(d.Files)) + } + if d.Files[0].Path != "miner/a.go" { + t.Fatalf("filtered path = %q, want miner/a.go", d.Files[0].Path) + } +} + +func TestFilterDiffFiles_EmptyIncludePaths(t *testing.T) { + d := &diff.Result{ + BaseBranch: "develop", + Files: []diff.FileChange{ + {Path: "miner/a.go"}, + }, + } + + filtered, err := filterDiffFiles(t.TempDir(), d, "") + if err != nil { + t.Fatalf("filterDiffFiles error: %v", err) + } + if filtered != d { + t.Fatal("expected empty include-paths to return original diff result") + } +} + +func TestFilterDiffFiles_InvalidIncludePaths(t *testing.T) { + d := &diff.Result{ + BaseBranch: "develop", + Files: []diff.FileChange{ + {Path: "miner/a.go"}, + }, + } + + _, err := filterDiffFiles(t.TempDir(), d, " , ") + if err == nil { + t.Fatal("expected invalid include-paths to return an error") + } + if !strings.Contains(err.Error(), "--include-paths requires at least one path") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestParsePathList_RejectsEmptyInput(t *testing.T) { + _, err := parsePathList(" , ", "paths") + if err == nil { + t.Fatal("expected empty path list to return an error") + } + if !strings.Contains(err.Error(), "--paths requires at least one path") { + t.Fatalf("unexpected error: %v", err) + } +} + +func initGitRepo(t *testing.T) string { + t.Helper() + + repo := t.TempDir() + git(t, repo, "init") + git(t, repo, "config", "user.email", "test@example.com") + git(t, repo, "config", "user.name", "Test User") + git(t, repo, "config", "commit.gpgsign", "false") + git(t, repo, "checkout", "-b", "develop") + return repo +} + +func git(t *testing.T, repo string, args ...string) { + t.Helper() + + cmd := exec.Command("git", args...) + cmd.Dir = repo + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %s failed: %v\n%s", strings.Join(args, " "), err, out) + } +} + +func writeFile(t *testing.T, path, content string) { + t.Helper() + + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + t.Fatalf("MkdirAll(%q): %v", path, err) + } + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatalf("WriteFile(%q): %v", path, err) + } +} diff --git a/internal/diff/diff.go b/internal/diff/diff.go index 74fcc8c..af0fc4d 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -126,6 +126,99 @@ func CollectPaths(repoPath string, paths []string) (*Result, error) { return &Result{Files: files}, nil } +// FilterPaths restricts an existing Result to files matching the given file or +// directory paths. Directory inputs match recursively. +func FilterPaths(repoPath string, r *Result, paths []string) (*Result, error) { + scopes, err := compilePathScopes(repoPath, paths) + if err != nil { + return nil, err + } + + filtered := &Result{BaseBranch: r.BaseBranch} + for _, f := range r.Files { + if matchesAnyScope(f.Path, scopes) { + filtered.Files = append(filtered.Files, f) + } + } + return filtered, nil +} + +type pathScope struct { + path string + isDir bool +} + +func compilePathScopes(repoPath string, paths []string) ([]pathScope, error) { + scopes := make([]pathScope, 0, len(paths)) + for _, raw := range paths { + scope, err := compilePathScope(repoPath, raw) + if err != nil { + return nil, err + } + scopes = append(scopes, scope) + } + return scopes, nil +} + +func compilePathScope(repoPath, raw string) (pathScope, error) { + if raw == "" { + return pathScope{}, fmt.Errorf("empty path filter") + } + + isDir := strings.HasSuffix(raw, "/") || strings.HasSuffix(raw, string(filepath.Separator)) + normalized := raw + if filepath.IsAbs(raw) { + rel, err := filepath.Rel(repoPath, raw) + if err != nil { + return pathScope{}, err + } + normalized = rel + } + normalized = filepath.Clean(normalized) + if !isRepoRelative(normalized) { + return pathScope{}, fmt.Errorf("path %q is outside repo", raw) + } + + if info, err := os.Stat(filepath.Join(repoPath, normalized)); err == nil && info.IsDir() { + isDir = true + } + + return pathScope{path: normalized, isDir: isDir}, nil +} + +func isRepoRelative(path string) bool { + if path == "." { + return true + } + if path == ".." { + return false + } + return !strings.HasPrefix(path, ".."+string(filepath.Separator)) +} + +func matchesAnyScope(path string, scopes []pathScope) bool { + for _, scope := range scopes { + if matchesScope(path, scope) { + return true + } + } + return false +} + +func matchesScope(path string, scope pathScope) bool { + if scope.path == "." { + return true + } + if scope.isDir { + return matchesDirScope(path, scope.path) + } + return path == scope.path +} + +func matchesDirScope(path, dir string) bool { + return path == dir || strings.HasPrefix(path, dir+string(filepath.Separator)) +} + func collectPath(repoPath, p string, files *[]FileChange, seen map[string]bool) error { absPath := p if !filepath.IsAbs(p) { diff --git a/internal/diff/diff_extra_test.go b/internal/diff/diff_extra_test.go index 62dec06..aff59cc 100644 --- a/internal/diff/diff_extra_test.go +++ b/internal/diff/diff_extra_test.go @@ -206,6 +206,132 @@ func TestHandleHunkLine_PureDeletion(t *testing.T) { } } +func TestFilterPaths_DirectoryAndFile(t *testing.T) { + dir := t.TempDir() + os.MkdirAll(filepath.Join(dir, "miner", "sub"), 0755) + os.MkdirAll(filepath.Join(dir, "other"), 0755) + os.WriteFile(filepath.Join(dir, "miner", "a.go"), []byte("package miner\n"), 0644) + os.WriteFile(filepath.Join(dir, "miner", "sub", "b.go"), []byte("package miner\n"), 0644) + os.WriteFile(filepath.Join(dir, "other", "c.go"), []byte("package other\n"), 0644) + + r := &Result{ + BaseBranch: "develop", + Files: []FileChange{ + {Path: "miner/a.go"}, + {Path: "miner/sub/b.go"}, + {Path: "other/c.go"}, + }, + } + + filtered, err := FilterPaths(dir, r, []string{"miner", "other/c.go"}) + if err != nil { + t.Fatalf("FilterPaths error: %v", err) + } + + if filtered.BaseBranch != "develop" { + t.Fatalf("BaseBranch = %q, want develop", filtered.BaseBranch) + } + if len(filtered.Files) != 3 { + t.Fatalf("expected 3 matched files, got %d", len(filtered.Files)) + } +} + +func TestFilterPaths_AbsolutePathAndRoot(t *testing.T) { + dir := t.TempDir() + os.MkdirAll(filepath.Join(dir, "miner"), 0755) + os.WriteFile(filepath.Join(dir, "miner", "a.go"), []byte("package miner\n"), 0644) + + r := &Result{ + Files: []FileChange{ + {Path: "miner/a.go"}, + }, + } + + filtered, err := FilterPaths(dir, r, []string{filepath.Join(dir, "miner", "a.go")}) + if err != nil { + t.Fatalf("FilterPaths(abs file) error: %v", err) + } + if len(filtered.Files) != 1 || filtered.Files[0].Path != "miner/a.go" { + t.Fatalf("unexpected absolute-path filter result: %+v", filtered.Files) + } + + filtered, err = FilterPaths(dir, r, []string{"."}) + if err != nil { + t.Fatalf("FilterPaths(.) error: %v", err) + } + if len(filtered.Files) != 1 { + t.Fatalf("expected root filter to keep all files, got %d", len(filtered.Files)) + } +} + +func TestFilterPaths_RejectsOutsideRepo(t *testing.T) { + dir := t.TempDir() + + _, err := FilterPaths(dir, &Result{}, []string{"../outside"}) + if err == nil { + t.Fatal("expected error for path outside repo") + } +} + +func TestCompilePathScope_RejectsEmptyPath(t *testing.T) { + _, err := compilePathScope(t.TempDir(), "") + if err == nil { + t.Fatal("expected error for empty path") + } +} + +func TestCompilePathScope_DetectsDirectoryAndNormalizesAbsolutePath(t *testing.T) { + dir := t.TempDir() + if err := os.MkdirAll(filepath.Join(dir, "miner", "sub"), 0755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + scope, err := compilePathScope(dir, filepath.Join(dir, "miner")) + if err != nil { + t.Fatalf("compilePathScope(dir) error: %v", err) + } + if scope.path != "miner" { + t.Fatalf("scope.path = %q, want miner", scope.path) + } + if !scope.isDir { + t.Fatal("expected miner scope to be treated as directory") + } + + scope, err = compilePathScope(dir, filepath.Join(dir, "miner", "sub")) + if err != nil { + t.Fatalf("compilePathScope(abs dir) error: %v", err) + } + if scope.path != filepath.Join("miner", "sub") { + t.Fatalf("scope.path = %q, want %q", scope.path, filepath.Join("miner", "sub")) + } + if !scope.isDir { + t.Fatal("expected nested scope to be treated as directory") + } +} + +func TestMatchesAnyScope(t *testing.T) { + scopes := []pathScope{ + {path: "miner", isDir: true}, + {path: "other/c.go"}, + } + + tests := []struct { + path string + want bool + }{ + {path: "miner", want: true}, + {path: "miner/a.go", want: true}, + {path: "other/c.go", want: true}, + {path: "other/d.go", want: false}, + } + + for _, tt := range tests { + if got := matchesAnyScope(tt.path, scopes); got != tt.want { + t.Fatalf("matchesAnyScope(%q) = %v, want %v", tt.path, got, tt.want) + } + } +} + func TestFileChange_IsNew(t *testing.T) { newFile := FileChange{ Path: "new.go",