| Josh Bleecher Snyder | f4047bb | 2025-05-05 23:02:56 +0000 | [diff] [blame] | 1 | package codereview |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 2 | |
| 3 | import ( |
| 4 | "bytes" |
| 5 | "context" |
| 6 | "fmt" |
| 7 | "log/slog" |
| 8 | "os" |
| 9 | "os/exec" |
| 10 | "path/filepath" |
| Josh Bleecher Snyder | c72ceb2 | 2025-05-05 23:30:15 +0000 | [diff] [blame] | 11 | "slices" |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 12 | "strings" |
| Josh Bleecher Snyder | 6534c7a | 2025-07-01 01:48:52 +0000 | [diff] [blame] | 13 | "sync" |
| Josh Bleecher Snyder | f4047bb | 2025-05-05 23:02:56 +0000 | [diff] [blame] | 14 | |
| 15 | "sketch.dev/claudetool" |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 16 | ) |
| 17 | |
| 18 | // A CodeReviewer manages quality checks. |
| 19 | type CodeReviewer struct { |
| 20 | repoRoot string |
| Philip Zeyliger | 49edc92 | 2025-05-14 09:45:45 -0700 | [diff] [blame] | 21 | sketchBaseRef string |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 22 | initialStatus []fileStatus // git status of files at initial commit, absolute paths |
| 23 | reviewed []string // history of all commits which have been reviewed |
| 24 | initialWorktree string // git worktree at initial commit, absolute path |
| Josh Bleecher Snyder | 26b6f9b | 2025-07-01 01:41:11 +0000 | [diff] [blame] | 25 | // "Related files" caching |
| Josh Bleecher Snyder | 6534c7a | 2025-07-01 01:48:52 +0000 | [diff] [blame] | 26 | processedChangedFileSets map[string]bool // hash of sorted changedFiles -> processed |
| 27 | reportedRelatedFiles map[string]bool // file path -> reported |
| 28 | // Pre-warming of Go build/test cache |
| 29 | warmMutex sync.Mutex // protects warmedPackages map |
| 30 | warmedPackages map[string]bool // packages that have been cache warmed |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 31 | } |
| 32 | |
| Josh Bleecher Snyder | 9daa518 | 2025-05-16 18:34:00 +0000 | [diff] [blame] | 33 | func NewCodeReviewer(ctx context.Context, repoRoot, sketchBaseRef string) (*CodeReviewer, error) { |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 34 | r := &CodeReviewer{ |
| Josh Bleecher Snyder | 26b6f9b | 2025-07-01 01:41:11 +0000 | [diff] [blame] | 35 | repoRoot: repoRoot, |
| 36 | sketchBaseRef: sketchBaseRef, |
| 37 | processedChangedFileSets: make(map[string]bool), |
| 38 | reportedRelatedFiles: make(map[string]bool), |
| Josh Bleecher Snyder | 6534c7a | 2025-07-01 01:48:52 +0000 | [diff] [blame] | 39 | warmedPackages: make(map[string]bool), |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 40 | } |
| 41 | if r.repoRoot == "" { |
| 42 | return nil, fmt.Errorf("NewCodeReviewer: repoRoot must be non-empty") |
| 43 | } |
| Philip Zeyliger | 49edc92 | 2025-05-14 09:45:45 -0700 | [diff] [blame] | 44 | if r.sketchBaseRef == "" { |
| 45 | return nil, fmt.Errorf("NewCodeReviewer: sketchBaseRef must be non-empty") |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 46 | } |
| 47 | // Confirm that root is in fact the git repo root. |
| Josh Bleecher Snyder | f4047bb | 2025-05-05 23:02:56 +0000 | [diff] [blame] | 48 | root, err := claudetool.FindRepoRoot(r.repoRoot) |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 49 | if err != nil { |
| 50 | return nil, err |
| 51 | } |
| 52 | if root != r.repoRoot { |
| 53 | return nil, fmt.Errorf("NewCodeReviewer: repoRoot=%q but git repo root is %q", r.repoRoot, root) |
| 54 | } |
| 55 | |
| 56 | // Get an initial list of dirty and untracked files. |
| 57 | // We'll filter them out later when deciding whether the worktree is clean. |
| 58 | status, err := r.repoStatus(ctx) |
| 59 | if err != nil { |
| 60 | return nil, err |
| 61 | } |
| 62 | r.initialStatus = status |
| 63 | return r, nil |
| 64 | } |
| 65 | |
| Josh Bleecher Snyder | c72ceb2 | 2025-05-05 23:30:15 +0000 | [diff] [blame] | 66 | // autoformat formats all files changed in HEAD. |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 67 | // It returns a list of all files that were formatted. |
| 68 | // It is best-effort only. |
| Josh Bleecher Snyder | c72ceb2 | 2025-05-05 23:30:15 +0000 | [diff] [blame] | 69 | func (r *CodeReviewer) autoformat(ctx context.Context) []string { |
| 70 | // Refuse to format if initial commit is not an ancestor of HEAD |
| Philip Zeyliger | 49edc92 | 2025-05-14 09:45:45 -0700 | [diff] [blame] | 71 | err := r.requireHEADDescendantOfSketchBaseRef(ctx) |
| Josh Bleecher Snyder | c72ceb2 | 2025-05-05 23:30:15 +0000 | [diff] [blame] | 72 | if err != nil { |
| 73 | slog.WarnContext(ctx, "CodeReviewer.Autoformat refusing to format", "err", err) |
| 74 | return nil |
| 75 | } |
| 76 | |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 77 | head, err := r.CurrentCommit(ctx) |
| 78 | if err != nil { |
| 79 | slog.WarnContext(ctx, "CodeReviewer.Autoformat unable to get current commit", "err", err) |
| 80 | return nil |
| 81 | } |
| 82 | parent, err := r.ResolveCommit(ctx, "HEAD^1") |
| 83 | if err != nil { |
| 84 | slog.WarnContext(ctx, "CodeReviewer.Autoformat unable to get parent commit", "err", err) |
| 85 | return nil |
| 86 | } |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 87 | // Retrieve a list of all files changed |
| 88 | // TODO: instead of one git diff --name-only and then N --name-status, do one --name-status. |
| Philip Zeyliger | 49edc92 | 2025-05-14 09:45:45 -0700 | [diff] [blame] | 89 | changedFiles, err := r.changedFiles(ctx, r.sketchBaseRef, head) |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 90 | if err != nil { |
| 91 | slog.WarnContext(ctx, "CodeReviewer.Autoformat unable to get changed files", "err", err) |
| 92 | return nil |
| 93 | } |
| 94 | |
| 95 | // General strategy: For all changed files, |
| 96 | // run the strictest formatter that passes on the original version. |
| 97 | // TODO: add non-Go formatters? |
| 98 | // TODO: at a minimum, for common file types, ensure trailing newlines and maybe trim trailing whitespace per line? |
| 99 | var fmtFiles []string |
| 100 | for _, file := range changedFiles { |
| 101 | if !strings.HasSuffix(file, ".go") { |
| 102 | continue |
| 103 | } |
| 104 | fileStatus, err := r.gitFileStatus(ctx, file) |
| 105 | if err != nil { |
| 106 | slog.WarnContext(ctx, "CodeReviewer.Autoformat unable to get file status", "file", file, "err", err) |
| 107 | continue |
| 108 | } |
| 109 | if fileStatus == "D" { // deleted, nothing to format |
| 110 | continue |
| 111 | } |
| 112 | code, err := r.getFileContentAtCommit(ctx, file, head) |
| 113 | if err != nil { |
| 114 | slog.WarnContext(ctx, "CodeReviewer.Autoformat unable to get file content at head", "file", file, "err", err) |
| 115 | continue |
| 116 | } |
| Josh Bleecher Snyder | f4047bb | 2025-05-05 23:02:56 +0000 | [diff] [blame] | 117 | if claudetool.IsAutogeneratedGoFile(code) { // leave autogenerated files alone |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 118 | continue |
| 119 | } |
| 120 | onDisk, err := os.ReadFile(file) |
| 121 | if err != nil { |
| 122 | slog.WarnContext(ctx, "CodeReviewer.Autoformat unable to read file", "file", file, "err", err) |
| 123 | continue |
| 124 | } |
| 125 | if !bytes.Equal(code, onDisk) { // file has been modified since HEAD |
| 126 | slog.WarnContext(ctx, "CodeReviewer.Autoformat file modified since HEAD", "file", file, "err", err) |
| 127 | continue |
| 128 | } |
| 129 | var formatterToUse string |
| 130 | if fileStatus == "A" { |
| 131 | formatterToUse = "gofumpt" // newly added, so we can format how we please: use gofumpt |
| 132 | } else { |
| 133 | prev, err := r.getFileContentAtCommit(ctx, file, parent) |
| 134 | if err != nil { |
| 135 | slog.WarnContext(ctx, "CodeReviewer.Autoformat unable to get file content at parent", "file", file, "err", err) |
| 136 | continue |
| 137 | } |
| 138 | formatterToUse = r.pickFormatter(ctx, prev) // pick the strictest formatter that passes on the original version |
| 139 | } |
| 140 | |
| 141 | // Apply the chosen formatter to the current file |
| 142 | newCode := r.runFormatter(ctx, formatterToUse, code) |
| 143 | if newCode == nil { // no changes made |
| 144 | continue |
| 145 | } |
| 146 | // write to disk |
| 147 | if err := os.WriteFile(file, newCode, 0o600); err != nil { |
| 148 | slog.WarnContext(ctx, "CodeReviewer.Autoformat unable to write formatted file", "file", file, "err", err) |
| 149 | continue |
| 150 | } |
| 151 | fmtFiles = append(fmtFiles, file) |
| 152 | } |
| 153 | return fmtFiles |
| 154 | } |
| 155 | |
| 156 | // RequireNormalGitState checks that the git repo state is pretty normal. |
| 157 | func (r *CodeReviewer) RequireNormalGitState(_ context.Context) error { |
| 158 | rebaseDirs := []string{"rebase-merge", "rebase-apply"} |
| 159 | for _, dir := range rebaseDirs { |
| 160 | _, err := os.Stat(filepath.Join(r.repoRoot, dir)) |
| 161 | if err == nil { |
| 162 | return fmt.Errorf("git repo is not clean: rebase in progress") |
| 163 | } |
| 164 | } |
| 165 | filesReason := map[string]string{ |
| 166 | "MERGE_HEAD": "merge is in progress", |
| 167 | "CHERRY_PICK_HEAD": "cherry-pick is in progress", |
| 168 | "REVERT_HEAD": "revert is in progress", |
| 169 | "BISECT_LOG": "bisect is in progress", |
| 170 | } |
| 171 | for file, reason := range filesReason { |
| 172 | _, err := os.Stat(filepath.Join(r.repoRoot, file)) |
| 173 | if err == nil { |
| 174 | return fmt.Errorf("git repo is not clean: %s", reason) |
| 175 | } |
| 176 | } |
| 177 | return nil |
| 178 | } |
| 179 | |
| 180 | func (r *CodeReviewer) RequireNoUncommittedChanges(ctx context.Context) error { |
| 181 | // Check that there are no uncommitted changes, whether staged or not. |
| 182 | // (Changes in r.initialStatus are OK, no other changes are.) |
| 183 | statuses, err := r.repoStatus(ctx) |
| 184 | if err != nil { |
| 185 | return fmt.Errorf("unable to get repo status: %w", err) |
| 186 | } |
| 187 | uncommitted := new(strings.Builder) |
| 188 | for _, status := range statuses { |
| Josh Bleecher Snyder | cf19190 | 2025-06-04 18:18:40 +0000 | [diff] [blame] | 189 | if !statusesContainFile(r.initialStatus, status.Path) { |
| 190 | fmt.Fprintf(uncommitted, "%s %s\n", status.RawStatus, status.Path) |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 191 | } |
| 192 | } |
| 193 | if uncommitted.Len() > 0 { |
| Josh Bleecher Snyder | 83b2d35 | 2025-05-23 11:39:50 -0700 | [diff] [blame] | 194 | return fmt.Errorf("uncommitted changes in repo, please commit relevant changes and revert/delete others:\n%s", uncommitted.String()) |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 195 | } |
| 196 | return nil |
| 197 | } |
| 198 | |
| Josh Bleecher Snyder | cf19190 | 2025-06-04 18:18:40 +0000 | [diff] [blame] | 199 | func statusesContainFile(statuses []fileStatus, file string) bool { |
| 200 | for _, s := range statuses { |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 201 | if s.Path == file { |
| 202 | return true |
| 203 | } |
| 204 | } |
| 205 | return false |
| 206 | } |
| 207 | |
| Josh Bleecher Snyder | cf19190 | 2025-06-04 18:18:40 +0000 | [diff] [blame] | 208 | // parseGitStatusLine parses a single line from git status --porcelain output. |
| 209 | // Returns the file path and status, or empty strings if the line should be ignored. |
| 210 | func parseGitStatusLine(line string) (path, status string) { |
| 211 | if len(line) <= 3 { |
| 212 | return "", "" // empty line or invalid format |
| 213 | } |
| 214 | return strings.TrimSpace(line[3:]), line[:2] |
| 215 | } |
| 216 | |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 217 | type fileStatus struct { |
| 218 | Path string |
| 219 | RawStatus string // always 2 characters |
| 220 | } |
| 221 | |
| 222 | func (r *CodeReviewer) repoStatus(ctx context.Context) ([]fileStatus, error) { |
| 223 | // Run git status --porcelain, split into lines |
| 224 | cmd := exec.CommandContext(ctx, "git", "status", "--porcelain") |
| 225 | cmd.Dir = r.repoRoot |
| 226 | out, err := cmd.CombinedOutput() |
| 227 | if err != nil { |
| 228 | return nil, fmt.Errorf("failed to run git status: %w\n%s", err, out) |
| 229 | } |
| 230 | var statuses []fileStatus |
| 231 | for line := range strings.Lines(string(out)) { |
| Josh Bleecher Snyder | cf19190 | 2025-06-04 18:18:40 +0000 | [diff] [blame] | 232 | path, status := parseGitStatusLine(line) |
| 233 | if path == "" { |
| 234 | continue // empty or invalid line |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 235 | } |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 236 | absPath := r.absPath(path) |
| 237 | statuses = append(statuses, fileStatus{Path: absPath, RawStatus: status}) |
| 238 | } |
| 239 | return statuses, nil |
| 240 | } |
| 241 | |
| 242 | // CurrentCommit retrieves the current git commit hash |
| 243 | func (r *CodeReviewer) CurrentCommit(ctx context.Context) (string, error) { |
| 244 | return r.ResolveCommit(ctx, "HEAD") |
| 245 | } |
| 246 | |
| 247 | func (r *CodeReviewer) ResolveCommit(ctx context.Context, ref string) (string, error) { |
| 248 | cmd := exec.CommandContext(ctx, "git", "rev-parse", ref) |
| 249 | cmd.Dir = r.repoRoot |
| 250 | out, err := cmd.CombinedOutput() |
| 251 | if err != nil { |
| 252 | return "", fmt.Errorf("failed to get current commit hash: %w\n%s", err, out) |
| 253 | } |
| 254 | return strings.TrimSpace(string(out)), nil |
| 255 | } |
| 256 | |
| 257 | func (r *CodeReviewer) absPath(relPath string) string { |
| Josh Bleecher Snyder | 6534c7a | 2025-07-01 01:48:52 +0000 | [diff] [blame] | 258 | if filepath.IsAbs(relPath) { |
| 259 | return relPath |
| 260 | } |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 261 | return filepath.Clean(filepath.Join(r.repoRoot, relPath)) |
| 262 | } |
| 263 | |
| 264 | // gitFileStatus returns the status of a file (A for added, M for modified, D for deleted, etc.) |
| 265 | func (r *CodeReviewer) gitFileStatus(ctx context.Context, file string) (string, error) { |
| Philip Zeyliger | 49edc92 | 2025-05-14 09:45:45 -0700 | [diff] [blame] | 266 | cmd := exec.CommandContext(ctx, "git", "diff", "--name-status", r.sketchBaseRef, "HEAD", "--", file) |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 267 | cmd.Dir = r.repoRoot |
| 268 | out, err := cmd.CombinedOutput() |
| 269 | if err != nil { |
| 270 | return "", fmt.Errorf("failed to get file status: %w\n%s", err, out) |
| 271 | } |
| 272 | status := strings.TrimSpace(string(out)) |
| 273 | if status == "" { |
| 274 | return "", fmt.Errorf("no status found for file: %s", file) |
| 275 | } |
| 276 | return string(status[0]), nil |
| 277 | } |
| 278 | |
| 279 | // getFileContentAtCommit retrieves file content at a specific commit |
| 280 | func (r *CodeReviewer) getFileContentAtCommit(ctx context.Context, file, commit string) ([]byte, error) { |
| 281 | relFile, err := filepath.Rel(r.repoRoot, file) |
| 282 | if err != nil { |
| 283 | slog.WarnContext(ctx, "CodeReviewer.getFileContentAtCommit: failed to get relative path", "repo_root", r.repoRoot, "file", file, "err", err) |
| 284 | file = relFile |
| 285 | } |
| 286 | cmd := exec.CommandContext(ctx, "git", "show", fmt.Sprintf("%s:%s", commit, relFile)) |
| 287 | cmd.Dir = r.repoRoot |
| 288 | out, err := cmd.CombinedOutput() |
| 289 | if err != nil { |
| 290 | return nil, fmt.Errorf("failed to get file content at commit %s: %w\n%s", commit, err, out) |
| 291 | } |
| 292 | return out, nil |
| 293 | } |
| 294 | |
| 295 | // runFormatter runs the specified formatter on a file and returns the results. |
| 296 | // A nil result indicates that the file is unchanged, or that an error occurred. |
| 297 | func (r *CodeReviewer) runFormatter(ctx context.Context, formatter string, content []byte) []byte { |
| 298 | if formatter == "" { |
| 299 | return nil // no formatter |
| 300 | } |
| 301 | // Run the formatter and capture the output |
| 302 | cmd := exec.CommandContext(ctx, formatter) |
| 303 | cmd.Dir = r.repoRoot |
| 304 | cmd.Stdin = bytes.NewReader(content) |
| 305 | out, err := cmd.CombinedOutput() |
| 306 | if err != nil { |
| 307 | // probably a parse error, err on the side of safety |
| 308 | return nil |
| 309 | } |
| 310 | if bytes.Equal(content, out) { |
| 311 | return nil // no changes |
| 312 | } |
| 313 | return out |
| 314 | } |
| 315 | |
| 316 | // formatterWouldChange reports whether a formatter would make changes to the content. |
| 317 | // If the contents are invalid, it returns false. |
| 318 | // It works by piping the content to the formatter with the -l flag. |
| 319 | func (r *CodeReviewer) formatterWouldChange(ctx context.Context, formatter string, content []byte) bool { |
| 320 | cmd := exec.CommandContext(ctx, formatter, "-l") |
| 321 | cmd.Dir = r.repoRoot |
| 322 | cmd.Stdin = bytes.NewReader(content) |
| 323 | out, err := cmd.CombinedOutput() |
| 324 | if err != nil { |
| 325 | // probably a parse error, err on the side of safety |
| 326 | return false |
| 327 | } |
| 328 | |
| 329 | // If the output is empty, the file passes the formatter |
| 330 | // If the output contains "<standard input>", the file would be changed |
| 331 | return len(bytes.TrimSpace(out)) > 0 |
| 332 | } |
| 333 | |
| 334 | // pickFormatter picks a formatter to use for code. |
| 335 | // If something goes wrong, it recommends no formatter (empty string). |
| 336 | func (r *CodeReviewer) pickFormatter(ctx context.Context, code []byte) string { |
| 337 | // Test each formatter from strictest to least strict. |
| 338 | // Keep the first one that doesn't make changes. |
| 339 | formatters := []string{"gofumpt", "goimports", "gofmt"} |
| 340 | for _, formatter := range formatters { |
| 341 | if r.formatterWouldChange(ctx, formatter, code) { |
| 342 | continue |
| 343 | } |
| 344 | return formatter |
| 345 | } |
| 346 | return "" // no safe formatter found |
| 347 | } |
| 348 | |
| 349 | // changedFiles retrieves a list of all files changed between two commits |
| 350 | func (r *CodeReviewer) changedFiles(ctx context.Context, fromCommit, toCommit string) ([]string, error) { |
| 351 | cmd := exec.CommandContext(ctx, "git", "diff", "--name-only", fromCommit, toCommit) |
| 352 | cmd.Dir = r.repoRoot |
| 353 | out, err := cmd.CombinedOutput() |
| 354 | if err != nil { |
| 355 | return nil, fmt.Errorf("failed to get changed files: %w\n%s", err, out) |
| 356 | } |
| 357 | var files []string |
| 358 | for line := range strings.Lines(string(out)) { |
| 359 | line = strings.TrimSpace(line) |
| 360 | if len(line) == 0 { |
| 361 | continue |
| 362 | } |
| 363 | path := r.absPath(line) |
| Josh Bleecher Snyder | cf19190 | 2025-06-04 18:18:40 +0000 | [diff] [blame] | 364 | if statusesContainFile(r.initialStatus, path) { |
| Earl Lee | 2e463fb | 2025-04-17 11:22:22 -0700 | [diff] [blame] | 365 | continue |
| 366 | } |
| 367 | files = append(files, path) |
| 368 | } |
| 369 | return files, nil |
| 370 | } |
| Josh Bleecher Snyder | c72ceb2 | 2025-05-05 23:30:15 +0000 | [diff] [blame] | 371 | |
| Josh Bleecher Snyder | cf19190 | 2025-06-04 18:18:40 +0000 | [diff] [blame] | 372 | // runGenerate runs go generate on all packages and returns a list of files changed. |
| 373 | // Errors returned will be reported to the LLM. |
| 374 | func (r *CodeReviewer) runGenerate(ctx context.Context, packages []string) ([]string, error) { |
| 375 | if len(packages) == 0 { |
| 376 | return nil, nil |
| 377 | } |
| 378 | |
| 379 | args := []string{"generate"} |
| 380 | for _, pkg := range packages { |
| 381 | // Sigh. Working around test packages is a PITA. |
| 382 | if strings.HasSuffix(pkg, ".test") || strings.HasSuffix(pkg, "_test") { |
| 383 | continue |
| 384 | } |
| 385 | args = append(args, pkg) |
| 386 | } |
| 387 | gen := exec.CommandContext(ctx, "go", args...) |
| 388 | gen.Dir = r.repoRoot |
| 389 | out, err := gen.CombinedOutput() |
| 390 | if err != nil { |
| 391 | return nil, fmt.Errorf("$ go %s\n%s", strings.Join(args, " "), out) |
| 392 | } |
| 393 | |
| 394 | status := exec.CommandContext(ctx, "git", "status", "--porcelain") |
| 395 | status.Dir = r.repoRoot |
| 396 | statusOut, err := status.CombinedOutput() |
| 397 | if err != nil { |
| 398 | return nil, fmt.Errorf("unable to get git status: %w", err) |
| 399 | } |
| 400 | |
| 401 | var changed []string |
| 402 | for line := range strings.Lines(string(statusOut)) { |
| 403 | path, _ := parseGitStatusLine(line) |
| 404 | if path == "" { |
| 405 | continue |
| 406 | } |
| Josh Bleecher Snyder | 95632c8 | 2025-06-05 01:23:05 +0000 | [diff] [blame] | 407 | absPath := filepath.Join(r.repoRoot, path) |
| 408 | if statusesContainFile(r.initialStatus, absPath) { |
| 409 | continue |
| 410 | } |
| 411 | changed = append(changed, absPath) |
| Josh Bleecher Snyder | cf19190 | 2025-06-04 18:18:40 +0000 | [diff] [blame] | 412 | } |
| 413 | |
| 414 | return changed, nil |
| 415 | } |
| 416 | |
| 417 | // isGoRepository checks if the repository contains a go.mod file |
| 418 | // TODO: check in subdirs? |
| 419 | func (r *CodeReviewer) isGoRepository() bool { |
| 420 | _, err := os.Stat(filepath.Join(r.repoRoot, "go.mod")) |
| 421 | return err == nil |
| 422 | } |
| 423 | |
| Josh Bleecher Snyder | 1ed1cc4 | 2025-05-07 20:21:40 +0000 | [diff] [blame] | 424 | // ModTidy runs go mod tidy if go module files have changed. |
| 425 | // Returns a list of files changed by go mod tidy (empty if none). |
| 426 | func (r *CodeReviewer) ModTidy(ctx context.Context) ([]string, error) { |
| Philip Zeyliger | 49edc92 | 2025-05-14 09:45:45 -0700 | [diff] [blame] | 427 | err := r.requireHEADDescendantOfSketchBaseRef(ctx) |
| Josh Bleecher Snyder | c72ceb2 | 2025-05-05 23:30:15 +0000 | [diff] [blame] | 428 | if err != nil { |
| Josh Bleecher Snyder | 1ed1cc4 | 2025-05-07 20:21:40 +0000 | [diff] [blame] | 429 | return nil, fmt.Errorf("cannot run ModTidy: %w", err) |
| Josh Bleecher Snyder | c72ceb2 | 2025-05-05 23:30:15 +0000 | [diff] [blame] | 430 | } |
| 431 | |
| Josh Bleecher Snyder | 1ed1cc4 | 2025-05-07 20:21:40 +0000 | [diff] [blame] | 432 | // Check if any go.mod, go.sum, etc. files have changed |
| 433 | currentCommit, err := r.CurrentCommit(ctx) |
| 434 | if err != nil { |
| 435 | return nil, fmt.Errorf("failed to get current commit: %w", err) |
| 436 | } |
| 437 | |
| Philip Zeyliger | 49edc92 | 2025-05-14 09:45:45 -0700 | [diff] [blame] | 438 | changedFiles, err := r.changedFiles(ctx, r.sketchBaseRef, currentCommit) |
| Josh Bleecher Snyder | 1ed1cc4 | 2025-05-07 20:21:40 +0000 | [diff] [blame] | 439 | if err != nil { |
| 440 | return nil, fmt.Errorf("failed to get changed files: %w", err) |
| 441 | } |
| 442 | |
| 443 | // Check if any of the changed files are go module files |
| 444 | goModsChanged := false |
| 445 | for _, file := range changedFiles { |
| 446 | if isGoModFile(file) { |
| 447 | goModsChanged = true |
| 448 | break |
| 449 | } |
| 450 | } |
| 451 | |
| 452 | if !goModsChanged { |
| 453 | // No go module files changed, so don't run go mod tidy |
| 454 | return nil, nil |
| 455 | } |
| 456 | |
| 457 | // Run go mod tidy |
| Josh Bleecher Snyder | c72ceb2 | 2025-05-05 23:30:15 +0000 | [diff] [blame] | 458 | cmd := exec.CommandContext(ctx, "go", "mod", "tidy") |
| 459 | cmd.Dir = r.repoRoot |
| 460 | out, err := cmd.CombinedOutput() |
| 461 | if err != nil { |
| Josh Bleecher Snyder | 1ed1cc4 | 2025-05-07 20:21:40 +0000 | [diff] [blame] | 462 | return nil, fmt.Errorf("go mod tidy failed: %w\n%s", err, out) |
| Josh Bleecher Snyder | c72ceb2 | 2025-05-05 23:30:15 +0000 | [diff] [blame] | 463 | } |
| 464 | |
| Josh Bleecher Snyder | 1ed1cc4 | 2025-05-07 20:21:40 +0000 | [diff] [blame] | 465 | // Check which files were changed by go mod tidy |
| 466 | statusCmd := exec.CommandContext(ctx, "git", "status", "--porcelain") |
| 467 | statusCmd.Dir = r.repoRoot |
| 468 | statusOut, err := statusCmd.CombinedOutput() |
| 469 | if err != nil { |
| 470 | return nil, fmt.Errorf("unable to get git status: %w", err) |
| 471 | } |
| 472 | |
| 473 | var changedByTidy []string |
| 474 | |
| 475 | for line := range strings.Lines(string(statusOut)) { |
| Josh Bleecher Snyder | cf19190 | 2025-06-04 18:18:40 +0000 | [diff] [blame] | 476 | file, _ := parseGitStatusLine(line) |
| 477 | if file == "" { |
| 478 | continue // empty or invalid line |
| Josh Bleecher Snyder | 1ed1cc4 | 2025-05-07 20:21:40 +0000 | [diff] [blame] | 479 | } |
| Josh Bleecher Snyder | 1ed1cc4 | 2025-05-07 20:21:40 +0000 | [diff] [blame] | 480 | if !isGoModFile(file) { |
| 481 | continue |
| 482 | } |
| 483 | path := filepath.Join(r.repoRoot, file) |
| 484 | changedByTidy = append(changedByTidy, path) |
| 485 | } |
| 486 | |
| 487 | return changedByTidy, nil |
| Josh Bleecher Snyder | c72ceb2 | 2025-05-05 23:30:15 +0000 | [diff] [blame] | 488 | } |
| 489 | |
| 490 | // RunMechanicalChecks runs all mechanical checks and returns a message describing any changes made. |
| 491 | func (r *CodeReviewer) RunMechanicalChecks(ctx context.Context) string { |
| 492 | var actions []string |
| 493 | |
| 494 | changed := r.autoformat(ctx) |
| 495 | if len(changed) > 0 { |
| 496 | actions = append(actions, "autoformatters") |
| 497 | } |
| 498 | |
| Josh Bleecher Snyder | 1ed1cc4 | 2025-05-07 20:21:40 +0000 | [diff] [blame] | 499 | // Run go mod tidy (only if go module files have changed) |
| 500 | tidyChanges, err := r.ModTidy(ctx) |
| Josh Bleecher Snyder | c72ceb2 | 2025-05-05 23:30:15 +0000 | [diff] [blame] | 501 | if err != nil { |
| 502 | slog.WarnContext(ctx, "CodeReviewer.RunMechanicalChecks: ModTidy failed", "err", err) |
| Josh Bleecher Snyder | 1ed1cc4 | 2025-05-07 20:21:40 +0000 | [diff] [blame] | 503 | } |
| 504 | if len(tidyChanges) > 0 { |
| 505 | changed = append(changed, tidyChanges...) |
| 506 | actions = append(actions, "`go mod tidy`") |
| Josh Bleecher Snyder | c72ceb2 | 2025-05-05 23:30:15 +0000 | [diff] [blame] | 507 | } |
| 508 | |
| 509 | if len(changed) == 0 { |
| 510 | return "" |
| 511 | } |
| 512 | |
| 513 | slices.Sort(changed) |
| 514 | |
| 515 | msg := fmt.Sprintf(`I ran %s, which updated these files: |
| 516 | |
| 517 | %s |
| 518 | |
| 519 | Please amend your latest git commit with these changes and then continue with what you were doing.`, |
| 520 | strings.Join(actions, " and "), |
| 521 | strings.Join(changed, "\n"), |
| 522 | ) |
| 523 | |
| 524 | return msg |
| 525 | } |
| Josh Bleecher Snyder | 1ed1cc4 | 2025-05-07 20:21:40 +0000 | [diff] [blame] | 526 | |
| 527 | // isGoModFile returns true if the file is a Go module file (go.mod, go.sum, etc.) |
| 528 | func isGoModFile(path string) bool { |
| 529 | basename := filepath.Base(path) |
| 530 | return strings.HasPrefix(basename, "go.") |
| 531 | } |