claudetool: ignore some gopls (/vet) checks
Fixes #95
Co-Authored-By: sketch <hello@sketch.dev>
Change-ID: s934f225a42c1d576k
diff --git a/claudetool/codereview/differential.go b/claudetool/codereview/differential.go
index e3a819a..44c0ecb 100644
--- a/claudetool/codereview/differential.go
+++ b/claudetool/codereview/differential.go
@@ -165,7 +165,9 @@
}
func (r *CodeReviewer) checkTests(ctx context.Context, pkgList []string) (string, error) {
- goTestArgs := []string{"test", "-json", "-v"}
+ // 'gopls check' covers everything that 'go vet' covers.
+ // Disabling vet here speeds things up, and allows more precise filtering and reporting.
+ goTestArgs := []string{"test", "-json", "-v", "-vet=off"}
goTestArgs = append(goTestArgs, pkgList...)
afterTestCmd := exec.CommandContext(ctx, "go", goTestArgs...)
@@ -207,6 +209,15 @@
Message string // Description of the issue
}
+// goplsIgnore contains substring patterns for gopls (and vet) diagnostic messages that should be suppressed.
+var goplsIgnore = []string{
+ // these are often just wrong, see https://github.com/golang/go/issues/57059#issuecomment-2884771470
+ "ends with redundant newline",
+
+ // as of May 2025, Claude doesn't understand strings/bytes.SplitSeq well enough to use it
+ "SplitSeq",
+}
+
// checkGopls runs gopls check on the provided files in both the current and initial state,
// compares the results, and reports any new issues introduced in the current state.
func (r *CodeReviewer) checkGopls(ctx context.Context, changedFiles []string) (string, error) {
@@ -246,7 +257,6 @@
slog.WarnContext(ctx, "CodeReviewer.checkGopls: gopls check failed to run properly", "err", err, "output", string(afterGoplsOut))
return "", nil // Skip rather than failing the entire code review
}
- // Otherwise, proceed with parsing - it's likely just the non-zero exit code due to found issues
}
// Parse the output
@@ -309,7 +319,8 @@
return r.formatGoplsRegressions(goplsRegressions), nil
}
-// parseGoplsOutput parses the text output from gopls check
+// parseGoplsOutput parses the text output from gopls check.
+// It drops any that match the patterns in goplsIgnore.
// Each line has the format: '/path/to/file.go:448:22-26: unused parameter: path'
func parseGoplsOutput(root string, output []byte) []GoplsIssue {
var issues []GoplsIssue
@@ -360,6 +371,11 @@
continue // Not enough position information
}
+ // Skip diagnostics that match any of our ignored patterns
+ if shouldIgnoreDiagnostic(message) {
+ continue
+ }
+
issues = append(issues, GoplsIssue{
Position: position,
Message: message,
@@ -1066,3 +1082,13 @@
fmt.Fprintf(buf, "\nThese files have historically changed with the files you have modified. Consider whether they require updates as well.\n")
return buf.String()
}
+
+// shouldIgnoreDiagnostic reports whether a diagnostic message matches any of the patterns in goplsIgnore.
+func shouldIgnoreDiagnostic(message string) bool {
+ for _, pattern := range goplsIgnore {
+ if strings.Contains(message, pattern) {
+ return true
+ }
+ }
+ return false
+}
diff --git a/claudetool/codereview/testdata/gopls_ignored_redundant_newline.txtar b/claudetool/codereview/testdata/gopls_ignored_redundant_newline.txtar
new file mode 100644
index 0000000..910e20c
--- /dev/null
+++ b/claudetool/codereview/testdata/gopls_ignored_redundant_newline.txtar
@@ -0,0 +1,24 @@
+Test that gopls/vet diagnostic checks for redundant newlines are completely filtered out with no errors reported
+
+See https://github.com/boldsoftware/sketch/issues/95 and https://github.com/golang/go/issues/57059
+
+-- p.go --
+package p
+
+-- .commit --
+Initial commit
+
+-- p.go --
+package p
+
+import "fmt"
+
+func F() {
+ fmt.Println("abc\n")
+}
+
+-- .commit --
+Add file with redundant newline error
+
+-- .run_test --
+OK
diff --git a/claudetool/codereview/testdata/vet_error_test.txtar b/claudetool/codereview/testdata/vet_error_test.txtar
index 6f2c7c7..eaaf0c2 100644
--- a/claudetool/codereview/testdata/vet_error_test.txtar
+++ b/claudetool/codereview/testdata/vet_error_test.txtar
@@ -1,4 +1,4 @@
-Check that vet errors are detected in a test run
+Verify that gopls provides vet error coverage
-- p.go --
package p
@@ -23,11 +23,6 @@
-- .run_test --
# Errors
-Test regressions detected between initial commit (INITIAL_COMMIT_HASH) and HEAD:
-
-1: sketch.dev: Previously had no tests, now has build/vet errors
-
-
Gopls check issues detected:
1. /PATH/TO/REPO/p.go:8:2-38: fmt.Printf format %s has arg 10 of wrong type int
diff --git a/claudetool/codereview/update_tests.sh b/claudetool/codereview/update_tests.sh
new file mode 100755
index 0000000..324f28a
--- /dev/null
+++ b/claudetool/codereview/update_tests.sh
@@ -0,0 +1,10 @@
+#!/bin/bash
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+CURRENT_DIR=$(pwd)
+
+cd "$SCRIPT_DIR"
+
+go test -update .
+
+cd "$CURRENT_DIR"