cmd/fix: change -diff to exit 1 if diffs exist

Currently "go fix -diff" and "go vet -fix -diff" always exit with status
0 even when they print diffs, which is inconsistent with "gofmt -d"
(#46289) and "go mod tidy -diff" (#27005) that exit non-zero when diffs
are present.

The root cause is that the default VetHandleStdout (copyToStdout) simply
copies the tool stdout through without checking whether any content was
produced. This change installs a new copyAndDetectDiff handler in -diff
mode that copies the tool stdout through and calls base.SetExitStatus(1)
when content is present, matching the pattern used by "go mod tidy -diff".

Fixes #77583

Change-Id: I588fbaae8b3690da2f821240baa4a3b14b78f280
Reviewed-on: https://go-review.googlesource.com/c/go/+/749700
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>

6 files changed, 79 insertions, 17 deletions

diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go
index e394ef836a..8533fd2af1 100644
--- a/src/cmd/go/alldocs.go
+++ b/src/cmd/go/alldocs.go
@@ -511,7 +511,8 @@
 // It supports these flags:
 //
 //	  -diff
-//		instead of applying each fix, print the patch as a unified diff
+//		instead of applying each fix, print the patch as a unified diff;
+//		exit with a non-zero status if the diff is not empty
 //
 // The -fixtool=prog flag selects a different analysis tool with
 // alternative or additional fixers; see the documentation for go vet's
@@ -2056,7 +2057,8 @@
 //	  -fix
 //		instead of printing each diagnostic, apply its first fix (if any)
 //	  -diff
-//		instead of applying each fix, print the patch as a unified diff
+//		instead of applying each fix, print the patch as a unified diff;
+//		exit with a non-zero status if the diff is not empty
 //
 // The -vettool=prog flag selects a different analysis tool with
 // alternative or additional checks. For example, the 'shadow' analyzer
diff --git a/src/cmd/go/internal/vet/vet.go b/src/cmd/go/internal/vet/vet.go
index 569f959225..61e3f59426 100644
--- a/src/cmd/go/internal/vet/vet.go
+++ b/src/cmd/go/internal/vet/vet.go
@@ -44,7 +44,8 @@ It supports these flags:
   -fix
 	instead of printing each diagnostic, apply its first fix (if any)
   -diff
-	instead of applying each fix, print the patch as a unified diff
+	instead of applying each fix, print the patch as a unified diff;
+	exit with a non-zero status if the diff is not empty
 
 The -vettool=prog flag selects a different analysis tool with
 alternative or additional checks. For example, the 'shadow' analyzer
@@ -81,7 +82,8 @@ and applies suggested fixes.
 It supports these flags:
 
   -diff
-	instead of applying each fix, print the patch as a unified diff
+	instead of applying each fix, print the patch as a unified diff;
+	exit with a non-zero status if the diff is not empty
 
 The -fixtool=prog flag selects a different analysis tool with
 alternative or additional fixers; see the documentation for go vet's
@@ -165,8 +167,8 @@ func run(ctx context.Context, cmd *base.Command, args []string) {
 	// command args                 tool args
 	// go vet               =>      cmd/vet -json           Parse stdout, print diagnostics to stderr.
 	// go vet -json         =>      cmd/vet -json           Pass stdout through.
-	// go vet -fix [-diff]  =>      cmd/vet -fix [-diff]    Pass stdout through.
-	// go fix [-diff]       =>      cmd/fix -fix [-diff]    Pass stdout through.
+	// go vet -fix [-diff]  =>      cmd/vet -fix [-diff]    Pass stdout through (and exit 1 if diffs).
+	// go fix [-diff]       =>      cmd/fix -fix [-diff]    Pass stdout through (and exit 1 if diffs).
 	// go fix -json         =>      cmd/fix -json           Pass stdout through.
 	//
 	// Notes:
@@ -189,6 +191,10 @@ func run(ctx context.Context, cmd *base.Command, args []string) {
 			toolFlags = append(toolFlags, "-fix")
 			if diffFlag {
 				toolFlags = append(toolFlags, "-diff")
+				// In -diff mode, the tool prints unified diffs to stdout.
+				// Copy stdout through and exit non-zero if diffs were printed,
+				// consistent with gofmt -d and go mod tidy -diff.
+				work.VetHandleStdout = copyAndDetectDiff
 			} else {
 				applyFixes = true
 			}
@@ -343,6 +349,24 @@ func readZip(zipfile string, out map[string][]byte) error {
 	return nil
 }
 
+// copyAndDetectDiff copies the tool's stdout to the go command's stdout
+// and sets exit status 1 if any output was produced (meaning diffs exist).
+// This is used in -diff mode to implement the convention that "go fix -diff"
+// exits non-zero when the diff is not empty, consistent with gofmt -d
+// and go mod tidy -diff.
+func copyAndDetectDiff(r io.Reader) error {
+	stdouterrMu.Lock()
+	defer stdouterrMu.Unlock()
+	n, err := io.Copy(os.Stdout, r)
+	if err != nil {
+		return fmt.Errorf("copying diff output: %w", err)
+	}
+	if n > 0 {
+		base.SetExitStatus(1)
+	}
+	return nil
+}
+
 // printJSONDiagnostics parses JSON (from the tool's stdout) and
 // prints it (to stderr) in "file:line: message" form.
 // It also ensures that we exit nonzero if there were diagnostics.
@@ -386,11 +410,11 @@ func printJSONDiagnostics(r io.Reader) error {
 	return nil
 }
 
-var stderrMu sync.Mutex // serializes concurrent writes to stdout
+var stdouterrMu sync.Mutex // serializes concurrent writes to stdout and stderr
 
 func printJSONDiagnostic(analyzer string, diag jsonDiagnostic) {
-	stderrMu.Lock()
-	defer stderrMu.Unlock()
+	stdouterrMu.Lock()
+	defer stdouterrMu.Unlock()
 
 	type posn struct {
 		file      string
diff --git a/src/cmd/go/testdata/script/fix_diff_exitcode.txt b/src/cmd/go/testdata/script/fix_diff_exitcode.txt
new file mode 100644
index 0000000000..2622c0265b
--- /dev/null
+++ b/src/cmd/go/testdata/script/fix_diff_exitcode.txt
@@ -0,0 +1,36 @@
+# Test that go fix -diff exits with non-zero status when diffs exist,
+# and exits with zero status when no diffs are needed.
+# This is consistent with gofmt -d (#46289) and go mod tidy -diff (#27005).
+
+# When the source needs fixes, go fix -diff should print the diff
+# and exit with a non-zero code.
+! go fix -diff example.com/needsfix
+stdout 'net.JoinHostPort'
+
+# When the source is already clean, go fix -diff should print nothing
+# and exit with a zero code.
+go fix -diff example.com/clean
+! stdout .
+
+-- go.mod --
+module example.com
+go 1.26
+
+-- needsfix/x.go --
+package needsfix
+
+import (
+	"fmt"
+	"net"
+)
+
+var s string
+var _, _ = net.Dial("tcp", fmt.Sprintf("%s:%d", s, 80))
+
+-- clean/x.go --
+package clean
+
+import "net"
+
+var s string
+var _, _ = net.Dial("tcp", net.JoinHostPort(s, "80"))
diff --git a/src/cmd/go/testdata/script/fix_suite.txt b/src/cmd/go/testdata/script/fix_suite.txt
index 455629dc17..28ef96e4d1 100644
--- a/src/cmd/go/testdata/script/fix_suite.txt
+++ b/src/cmd/go/testdata/script/fix_suite.txt
@@ -5,9 +5,9 @@
 # Each assertion matches the expected diff.
 #
 # Tip: to see the actual stdout,
-# temporarily prefix the go command with "! ".
+# temporarily remove the "! " prefix from the go command.
 
-go fix -diff example.com/x
+! go fix -diff example.com/x
 
 # buildtag
 stdout '-// \+build go1.26'
diff --git a/src/cmd/go/testdata/script/fix_vendor.txt b/src/cmd/go/testdata/script/fix_vendor.txt
index f03f326922..b1ee1975db 100644
--- a/src/cmd/go/testdata/script/fix_vendor.txt
+++ b/src/cmd/go/testdata/script/fix_vendor.txt
@@ -7,8 +7,8 @@ go mod vendor
 
 # Show fixes on two packages, one in the main module
 # and one in a vendored dependency.
-# Only the main one (a) is shown.
-go fix -diff example.com/a example.com/b
+# Only the main one (a) is shown. Diff => nonzero exit.
+! go fix -diff example.com/a example.com/b
 stdout 'a[/\\]a.go'
 stdout '\-var _ interface\{\}'
 stdout '\+var _ any'
diff --git a/src/cmd/go/testdata/script/vet_basic.txt b/src/cmd/go/testdata/script/vet_basic.txt
index a0dd3ae2d8..faff586fcb 100644
--- a/src/cmd/go/testdata/script/vet_basic.txt
+++ b/src/cmd/go/testdata/script/vet_basic.txt
@@ -28,8 +28,8 @@ stdout '"message": "address format .* does not work with IPv6",'
 stdout '"suggested_fixes":'
 stdout '"message": "Replace fmt.Sprintf with net.JoinHostPort",'
 
-# vet -fix -diff displays a diff.
-go vet -fix -diff example.com/x
+# vet -fix -diff displays a diff. Diff => nonzero exit.
+! go vet -fix -diff example.com/x
 stdout '\-var _ = fmt.Sprintf\(s\)'
 stdout '\+var _ = fmt.Sprintf\("%s", s\)'
 stdout '\-var _, _ = net.Dial\("tcp", fmt.Sprintf\("%s:%d", s, 80\)\)'
@@ -53,8 +53,8 @@ grep 'net.JoinHostPort' x.go
 ! stderr .
 cp x.go.bak x.go
 
-# Show diff of fixes from the fix suite.
-go fix -diff example.com/x
+# Show diff of fixes from the fix suite. Diff => nonzero exit.
+! go fix -diff example.com/x
 ! stdout '\-var _ = fmt.Sprintf\(s\)'
 stdout '\-var _, _ = net.Dial\("tcp", fmt.Sprintf\("%s:%d", s, 80\)\)'
 stdout '\+var _, _ = net.Dial\("tcp", net.JoinHostPort\(s, "80"\)\)'