claudetool/codereview: add go generate support
Automatically run 'go generate' as part of codereview,
since sketch sometimes forgets to do it.
Co-Authored-By: sketch <hello@sketch.dev>
Change-ID: safe9348e7c24beffk
diff --git a/claudetool/codereview/codereview.go b/claudetool/codereview/codereview.go
index 11f897e..085479e 100644
--- a/claudetool/codereview/codereview.go
+++ b/claudetool/codereview/codereview.go
@@ -176,8 +176,8 @@
}
uncommitted := new(strings.Builder)
for _, status := range statuses {
- if !r.initialStatusesContainFile(status.Path) {
- fmt.Fprintf(uncommitted, "%s %s\n", status.Path, status.RawStatus)
+ if !statusesContainFile(r.initialStatus, status.Path) {
+ fmt.Fprintf(uncommitted, "%s %s\n", status.RawStatus, status.Path)
}
}
if uncommitted.Len() > 0 {
@@ -186,8 +186,8 @@
return nil
}
-func (r *CodeReviewer) initialStatusesContainFile(file string) bool {
- for _, s := range r.initialStatus {
+func statusesContainFile(statuses []fileStatus, file string) bool {
+ for _, s := range statuses {
if s.Path == file {
return true
}
@@ -195,6 +195,15 @@
return false
}
+// parseGitStatusLine parses a single line from git status --porcelain output.
+// Returns the file path and status, or empty strings if the line should be ignored.
+func parseGitStatusLine(line string) (path, status string) {
+ if len(line) <= 3 {
+ return "", "" // empty line or invalid format
+ }
+ return strings.TrimSpace(line[3:]), line[:2]
+}
+
type fileStatus struct {
Path string
RawStatus string // always 2 characters
@@ -210,14 +219,10 @@
}
var statuses []fileStatus
for line := range strings.Lines(string(out)) {
- if len(line) == 0 {
- continue
+ path, status := parseGitStatusLine(line)
+ if path == "" {
+ continue // empty or invalid line
}
- if len(line) < 3 {
- return nil, fmt.Errorf("invalid status line: %s", line)
- }
- path := line[3:]
- status := line[:2]
absPath := r.absPath(path)
statuses = append(statuses, fileStatus{Path: absPath, RawStatus: status})
}
@@ -343,7 +348,7 @@
continue
}
path := r.absPath(line)
- if r.initialStatusesContainFile(path) {
+ if statusesContainFile(r.initialStatus, path) {
continue
}
files = append(files, path)
@@ -351,6 +356,54 @@
return files, nil
}
+// runGenerate runs go generate on all packages and returns a list of files changed.
+// Errors returned will be reported to the LLM.
+func (r *CodeReviewer) runGenerate(ctx context.Context, packages []string) ([]string, error) {
+ if len(packages) == 0 {
+ return nil, nil
+ }
+
+ args := []string{"generate"}
+ for _, pkg := range packages {
+ // Sigh. Working around test packages is a PITA.
+ if strings.HasSuffix(pkg, ".test") || strings.HasSuffix(pkg, "_test") {
+ continue
+ }
+ args = append(args, pkg)
+ }
+ gen := exec.CommandContext(ctx, "go", args...)
+ gen.Dir = r.repoRoot
+ out, err := gen.CombinedOutput()
+ if err != nil {
+ return nil, fmt.Errorf("$ go %s\n%s", strings.Join(args, " "), out)
+ }
+
+ status := exec.CommandContext(ctx, "git", "status", "--porcelain")
+ status.Dir = r.repoRoot
+ statusOut, err := status.CombinedOutput()
+ if err != nil {
+ return nil, fmt.Errorf("unable to get git status: %w", err)
+ }
+
+ var changed []string
+ for line := range strings.Lines(string(statusOut)) {
+ path, _ := parseGitStatusLine(line)
+ if path == "" {
+ continue
+ }
+ changed = append(changed, filepath.Join(r.repoRoot, path))
+ }
+
+ return changed, nil
+}
+
+// isGoRepository checks if the repository contains a go.mod file
+// TODO: check in subdirs?
+func (r *CodeReviewer) isGoRepository() bool {
+ _, err := os.Stat(filepath.Join(r.repoRoot, "go.mod"))
+ return err == nil
+}
+
// ModTidy runs go mod tidy if go module files have changed.
// Returns a list of files changed by go mod tidy (empty if none).
func (r *CodeReviewer) ModTidy(ctx context.Context) ([]string, error) {
@@ -403,11 +456,10 @@
var changedByTidy []string
for line := range strings.Lines(string(statusOut)) {
- if len(line) <= 3 {
- // empty line, defensiveness to avoid panics
- continue
+ file, _ := parseGitStatusLine(line)
+ if file == "" {
+ continue // empty or invalid line
}
- file := line[3:]
if !isGoModFile(file) {
continue
}
diff --git a/claudetool/codereview/differential.go b/claudetool/codereview/differential.go
index 334e67d..8688843 100644
--- a/claudetool/codereview/differential.go
+++ b/claudetool/codereview/differential.go
@@ -86,8 +86,25 @@
var errorMessages []string // problems we want the model to address
var infoMessages []string // info the model should consider
+ // Run 'go generate' early, so that it can potentially fix tests that would otherwise fail.
+ generateChanges, err := r.runGenerate(ctx, allPkgList)
+ if err != nil {
+ errorMessages = append(errorMessages, err.Error())
+ }
+ if len(generateChanges) > 0 {
+ buf := new(strings.Builder)
+ buf.WriteString("The following files were changed by running `go generate`:\n\n")
+ for _, f := range generateChanges {
+ buf.WriteString(f)
+ buf.WriteString("\n")
+ }
+ buf.WriteString("\nPlease amend your latest git commit with these changes.\n")
+ infoMessages = append(infoMessages, buf.String())
+ }
+
// Find potentially related files that should also be considered
// TODO: add some caching here, since this depends only on the initial commit and the changed files, not the details of the changes
+ // TODO: arrange for this to run even in non-Go repos!
relatedFiles, err := r.findRelatedFiles(ctx, changedFiles)
if err != nil {
slog.DebugContext(ctx, "CodeReviewer.Run: failed to find related files", "err", err)
diff --git a/claudetool/codereview/testdata/generate_error.txtar b/claudetool/codereview/testdata/generate_error.txtar
new file mode 100644
index 0000000..e5c195f
--- /dev/null
+++ b/claudetool/codereview/testdata/generate_error.txtar
@@ -0,0 +1,30 @@
+Test go generate error handling in mechanical checks
+
+-- go.mod --
+module sketch.dev
+
+go 1.23
+
+-- .commit --
+Initial commit
+
+-- main.go --
+package main
+
+//go:generate false
+
+func main() {
+ println("Hello, world!")
+}
+
+-- .commit --
+Commit with failing go generate directive
+
+-- .run_test --
+# Errors
+
+$ go generate sketch.dev
+main.go:3: running "false": exit status 1
+
+
+Please fix before proceeding.
diff --git a/claudetool/codereview/testdata/generate_no_changes.txtar b/claudetool/codereview/testdata/generate_no_changes.txtar
new file mode 100644
index 0000000..e38d7e9
--- /dev/null
+++ b/claudetool/codereview/testdata/generate_no_changes.txtar
@@ -0,0 +1,24 @@
+Test go generate with no changes in mechanical checks
+
+-- go.mod --
+module sketch.dev
+
+go 1.23
+
+-- .commit --
+Initial commit
+
+-- main.go --
+package main
+
+//go:generate echo "No-op generation"
+
+func main() {
+ println("Hello, world!")
+}
+
+-- .commit --
+Commit with no-op go generate directive
+
+-- .run_test --
+OK
diff --git a/claudetool/codereview/testdata/generate_success.txtar b/claudetool/codereview/testdata/generate_success.txtar
new file mode 100644
index 0000000..b6b6183
--- /dev/null
+++ b/claudetool/codereview/testdata/generate_success.txtar
@@ -0,0 +1,49 @@
+Test go generate as part of mechanical checks
+
+-- go.mod --
+module sketch.dev
+
+go 1.23
+
+-- .commit --
+Initial commit
+
+-- gen.go --
+//go:build ignore
+
+package main
+
+import (
+ "os"
+)
+
+func main() {
+ f, _ := os.Create("generated.go")
+ defer f.Close()
+ f.WriteString("// Code generated by go generate; DO NOT EDIT.\n")
+ f.WriteString("package main\n")
+ f.WriteString("const GeneratedValue = 42\n")
+}
+
+-- main.go --
+package main
+
+//go:generate go run gen.go
+
+func main() {
+ println("Hello, world!")
+}
+
+-- .commit --
+Initial commit with go generate directive
+
+-- .run_test --
+# Info
+
+The following files were changed by running `go generate`:
+
+/PATH/TO/REPO/generated.go
+
+Please amend your latest git commit with these changes.
+
+
diff --git a/claudetool/codereview/testdata/related_files_cooccurrence.txtar b/claudetool/codereview/testdata/related_files_cooccurrence.txtar
index b15f325..d42ee70 100644
--- a/claudetool/codereview/testdata/related_files_cooccurrence.txtar
+++ b/claudetool/codereview/testdata/related_files_cooccurrence.txtar
@@ -81,12 +81,15 @@
x := 42 // new gopls issue to view mixed info/error lines
}
+-- .commit --
+Update file a.go only (again)
+
-- .run_test --
# Info
Potentially related files:
-- p.go (33%)
+- p.go (30%)
These files have historically changed with the files you have modified. Consider whether they require updates as well.