From 8021c4459b4b02f96846a6a678783204881cba5e Mon Sep 17 00:00:00 2001 From: Robbie McMichael Date: Mon, 29 Sep 2025 14:58:22 +0800 Subject: [PATCH 1/3] cmd/gofmt: change -d to exit 1 if diffs exist When using the -d flag, set the exit code to 1 if there is a diff. Fixes #46289 --- src/cmd/gofmt/gofmt.go | 12 ++++++++++-- src/cmd/gofmt/gofmt_test.go | 23 +++++++++++++++++++++-- src/cmd/gofmt/testdata/exitcode0 | 4 ++++ src/cmd/gofmt/testdata/exitcode1 | 4 ++++ 4 files changed, 39 insertions(+), 4 deletions(-) create mode 100644 src/cmd/gofmt/testdata/exitcode0 create mode 100644 src/cmd/gofmt/testdata/exitcode1 diff --git a/src/cmd/gofmt/gofmt.go b/src/cmd/gofmt/gofmt.go index bbb8b4fd15c2f7..ad6ad636524479 100644 --- a/src/cmd/gofmt/gofmt.go +++ b/src/cmd/gofmt/gofmt.go @@ -41,6 +41,9 @@ var ( // debugging cpuprofile = flag.String("cpuprofile", "", "write cpu profile to this file") + + // errors + errFormattingDiffers = fmt.Errorf("formatting differs from gofmt's") ) // Keep these in sync with go/format/format.go. @@ -218,8 +221,12 @@ func (r *reporter) Report(err error) { panic("Report with nil error") } st := r.getState() - scanner.PrintError(st.err, err) - st.exitCode = 2 + if err == errFormattingDiffers { + st.exitCode = 1 + } else { + scanner.PrintError(st.err, err) + st.exitCode = 2 + } } func (r *reporter) ExitCode() int { @@ -281,6 +288,7 @@ func processFile(filename string, info fs.FileInfo, in io.Reader, r *reporter) e newName := filepath.ToSlash(filename) oldName := newName + ".orig" r.Write(diff.Diff(oldName, src, newName, res)) + return errFormattingDiffers } } diff --git a/src/cmd/gofmt/gofmt_test.go b/src/cmd/gofmt/gofmt_test.go index 6b80673af148f5..eed05b55c2a6ed 100644 --- a/src/cmd/gofmt/gofmt_test.go +++ b/src/cmd/gofmt/gofmt_test.go @@ -10,6 +10,7 @@ import ( "internal/diff" "os" "path/filepath" + "strconv" "strings" "testing" "text/scanner" @@ -55,8 +56,10 @@ func gofmtFlags(filename string, maxLines int) string { func runTest(t *testing.T, in, out string) { // process flags + *doDiff = false *simplifyAST = false *rewriteRule = "" + exitCode := 0 info, err := os.Lstat(in) if err != nil { t.Error(err) @@ -72,6 +75,8 @@ func runTest(t *testing.T, in, out string) { switch name { case "": // no flags + case "-d": + *doDiff = true case "-r": *rewriteRule = value case "-s": @@ -79,6 +84,12 @@ func runTest(t *testing.T, in, out string) { case "-stdin": // fake flag - pretend input is from stdin info = nil + case "-exitcode": + // fake flag - set an expected exit code + exitCode, err = strconv.Atoi(value) + if err != nil { + t.Errorf("couldn't convert string to int: %s", value) + } default: t.Errorf("unrecognized flag name: %s", name) } @@ -96,8 +107,13 @@ func runTest(t *testing.T, in, out string) { if errBuf.Len() > 0 { t.Logf("%q", errBuf.Bytes()) } - if s.GetExitCode() != 0 { - t.Fail() + if s.GetExitCode() != exitCode { + t.Errorf("expected exit code %d, got %d", exitCode, s.GetExitCode()) + } + + // don't try to compare output for diffs + if *doDiff { + return } expected, err := os.ReadFile(out) @@ -140,6 +156,9 @@ func TestRewrite(t *testing.T) { t.Fatal(err) } + // add exit code tests + match = append(match, "testdata/exitcode0", "testdata/exitcode1") + // add larger examples match = append(match, "gofmt.go", "gofmt_test.go") diff --git a/src/cmd/gofmt/testdata/exitcode0 b/src/cmd/gofmt/testdata/exitcode0 new file mode 100644 index 00000000000000..2ba3e3a3de26b6 --- /dev/null +++ b/src/cmd/gofmt/testdata/exitcode0 @@ -0,0 +1,4 @@ +//gofmt -d -exitcode=0 + +// Input has correct formatting +package main diff --git a/src/cmd/gofmt/testdata/exitcode1 b/src/cmd/gofmt/testdata/exitcode1 new file mode 100644 index 00000000000000..251873b8a69917 --- /dev/null +++ b/src/cmd/gofmt/testdata/exitcode1 @@ -0,0 +1,4 @@ +//gofmt -d -exitcode=1 + +// Input has incorrect formatting + package main From 722b65dc38b7b1d9b3e0de9f1a083e542c4affe7 Mon Sep 17 00:00:00 2001 From: Robbie McMichael Date: Mon, 6 Oct 2025 17:46:03 +0800 Subject: [PATCH 2/3] Add a separate test function for the -d flag --- src/cmd/gofmt/gofmt_test.go | 78 ++++++++++++++++++++++---------- src/cmd/gofmt/testdata/exitcode0 | 2 - src/cmd/gofmt/testdata/exitcode1 | 2 - 3 files changed, 54 insertions(+), 28 deletions(-) diff --git a/src/cmd/gofmt/gofmt_test.go b/src/cmd/gofmt/gofmt_test.go index eed05b55c2a6ed..9eb9fa607321e1 100644 --- a/src/cmd/gofmt/gofmt_test.go +++ b/src/cmd/gofmt/gofmt_test.go @@ -10,7 +10,6 @@ import ( "internal/diff" "os" "path/filepath" - "strconv" "strings" "testing" "text/scanner" @@ -54,12 +53,19 @@ func gofmtFlags(filename string, maxLines int) string { return "" } -func runTest(t *testing.T, in, out string) { - // process flags - *doDiff = false - *simplifyAST = false +// Reset global variables for all flags to their default value. +func resetFlags() { + *list = false + *write = false *rewriteRule = "" - exitCode := 0 + *simplifyAST = false + *doDiff = false + *allErrors = false + *cpuprofile = "" +} + +func runTest(t *testing.T, in, out string) { + resetFlags() info, err := os.Lstat(in) if err != nil { t.Error(err) @@ -75,8 +81,6 @@ func runTest(t *testing.T, in, out string) { switch name { case "": // no flags - case "-d": - *doDiff = true case "-r": *rewriteRule = value case "-s": @@ -84,12 +88,6 @@ func runTest(t *testing.T, in, out string) { case "-stdin": // fake flag - pretend input is from stdin info = nil - case "-exitcode": - // fake flag - set an expected exit code - exitCode, err = strconv.Atoi(value) - if err != nil { - t.Errorf("couldn't convert string to int: %s", value) - } default: t.Errorf("unrecognized flag name: %s", name) } @@ -107,13 +105,8 @@ func runTest(t *testing.T, in, out string) { if errBuf.Len() > 0 { t.Logf("%q", errBuf.Bytes()) } - if s.GetExitCode() != exitCode { - t.Errorf("expected exit code %d, got %d", exitCode, s.GetExitCode()) - } - - // don't try to compare output for diffs - if *doDiff { - return + if s.GetExitCode() != 0 { + t.Fail() } expected, err := os.ReadFile(out) @@ -156,9 +149,6 @@ func TestRewrite(t *testing.T) { t.Fatal(err) } - // add exit code tests - match = append(match, "testdata/exitcode0", "testdata/exitcode1") - // add larger examples match = append(match, "gofmt.go", "gofmt_test.go") @@ -178,6 +168,46 @@ func TestRewrite(t *testing.T) { } } +// TestDiff runs gofmt with the -d flag on the input files and checks that the +// expected exit code is set. +func TestDiff(t *testing.T) { + tests := []struct { + in string + exitCode int + }{ + {in: "testdata/exitcode0", exitCode: 0}, + {in: "testdata/exitcode1", exitCode: 1}, + } + + for _, tt := range tests { + resetFlags() + *doDiff = true + + initParserMode() + initRewrite() + + info, err := os.Lstat(tt.in) + if err != nil { + t.Error(err) + return + } + + const maxWeight = 2 << 20 + var buf, errBuf bytes.Buffer + s := newSequencer(maxWeight, &buf, &errBuf) + s.Add(fileWeight(tt.in, info), func(r *reporter) error { + return processFile(tt.in, info, nil, r) + }) + if errBuf.Len() > 0 { + t.Logf("%q", errBuf.Bytes()) + } + + if s.GetExitCode() != tt.exitCode { + t.Errorf("%s: expected exit code %d, got %d", tt.in, tt.exitCode, s.GetExitCode()) + } + } +} + // Test case for issue 3961. func TestCRLF(t *testing.T) { const input = "testdata/crlf.input" // must contain CR/LF's diff --git a/src/cmd/gofmt/testdata/exitcode0 b/src/cmd/gofmt/testdata/exitcode0 index 2ba3e3a3de26b6..e46983b2349139 100644 --- a/src/cmd/gofmt/testdata/exitcode0 +++ b/src/cmd/gofmt/testdata/exitcode0 @@ -1,4 +1,2 @@ -//gofmt -d -exitcode=0 - // Input has correct formatting package main diff --git a/src/cmd/gofmt/testdata/exitcode1 b/src/cmd/gofmt/testdata/exitcode1 index 251873b8a69917..09cccdb9ab5c54 100644 --- a/src/cmd/gofmt/testdata/exitcode1 +++ b/src/cmd/gofmt/testdata/exitcode1 @@ -1,4 +1,2 @@ -//gofmt -d -exitcode=1 - // Input has incorrect formatting package main From db2207fba9a8f7a2f50138ec1f086ac6a74e1b10 Mon Sep 17 00:00:00 2001 From: Robbie McMichael Date: Mon, 6 Oct 2025 20:42:45 +0800 Subject: [PATCH 3/3] Rename files for exit code tests --- src/cmd/gofmt/gofmt_test.go | 4 ++-- src/cmd/gofmt/testdata/exitcode.golden | 1 + src/cmd/gofmt/testdata/exitcode.input | 1 + src/cmd/gofmt/testdata/exitcode0 | 2 -- src/cmd/gofmt/testdata/exitcode1 | 2 -- 5 files changed, 4 insertions(+), 6 deletions(-) create mode 100644 src/cmd/gofmt/testdata/exitcode.golden create mode 100644 src/cmd/gofmt/testdata/exitcode.input delete mode 100644 src/cmd/gofmt/testdata/exitcode0 delete mode 100644 src/cmd/gofmt/testdata/exitcode1 diff --git a/src/cmd/gofmt/gofmt_test.go b/src/cmd/gofmt/gofmt_test.go index 9eb9fa607321e1..2aba0f03ff09e9 100644 --- a/src/cmd/gofmt/gofmt_test.go +++ b/src/cmd/gofmt/gofmt_test.go @@ -175,8 +175,8 @@ func TestDiff(t *testing.T) { in string exitCode int }{ - {in: "testdata/exitcode0", exitCode: 0}, - {in: "testdata/exitcode1", exitCode: 1}, + {in: "testdata/exitcode.input", exitCode: 1}, + {in: "testdata/exitcode.golden", exitCode: 0}, } for _, tt := range tests { diff --git a/src/cmd/gofmt/testdata/exitcode.golden b/src/cmd/gofmt/testdata/exitcode.golden new file mode 100644 index 00000000000000..06ab7d0f9a35a7 --- /dev/null +++ b/src/cmd/gofmt/testdata/exitcode.golden @@ -0,0 +1 @@ +package main diff --git a/src/cmd/gofmt/testdata/exitcode.input b/src/cmd/gofmt/testdata/exitcode.input new file mode 100644 index 00000000000000..4f2f092ce508de --- /dev/null +++ b/src/cmd/gofmt/testdata/exitcode.input @@ -0,0 +1 @@ + package main diff --git a/src/cmd/gofmt/testdata/exitcode0 b/src/cmd/gofmt/testdata/exitcode0 deleted file mode 100644 index e46983b2349139..00000000000000 --- a/src/cmd/gofmt/testdata/exitcode0 +++ /dev/null @@ -1,2 +0,0 @@ -// Input has correct formatting -package main diff --git a/src/cmd/gofmt/testdata/exitcode1 b/src/cmd/gofmt/testdata/exitcode1 deleted file mode 100644 index 09cccdb9ab5c54..00000000000000 --- a/src/cmd/gofmt/testdata/exitcode1 +++ /dev/null @@ -1,2 +0,0 @@ -// Input has incorrect formatting - package main