[go] cmd/gofmt: return a proper error for empty Go files

[Daniel Martí (Gerrit)]

Daniel Martí submitted this change.

View Change

2 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: src/cmd/gofmt/gofmt.go
Insertions: 2, Deletions: 2.

@@ -348,12 +348,12 @@
 	src := make([]byte, size+1)
 	n, err := io.ReadFull(in, src)
 	switch err {
-	default:
-		return nil, err
 	case nil, io.EOF, io.ErrUnexpectedEOF:
 		// io.ReadFull returns io.EOF (for an empty file) or io.ErrUnexpectedEOF
 		// (for a non-empty file) if the file was changed unexpectedly. Continue
 		// with comparing file sizes in those cases.
+	default:
+		return nil, err
 	}
 	if n < size {
 		return nil, fmt.Errorf("error: size of %s changed during reading (from %d to %d bytes)", filename, size, n)
```

Approvals: Robert Griesemer: Looks good to me, approved Matt Layher: Trusted Daniel Martí: Trusted; Run TryBots Gopher Robot: TryBots succeeded

cmd/gofmt: return a proper error for empty Go files

I was testing edge cases in gofumpt, a fork of gofmt,
and noticed that gofmt will return a bare io error on empty files,
as demonstrated by the added test case without a fix:

        > ! exec $GOROOT/bin/gofmt empty.go nopackage.go
        [stderr]
        EOF
        nopackage.go:1:1: expected 'package', found not

The problem is the code that detects concurrent modifications.
It relies on ReadFull and correctly deals with io.ErrUnexpectedEOF,
but it did not pay attention to io.EOF, which can happen when size==0.

Change-Id: I6092391721edad4584fb5922d3e3a8fb3da86493
Reviewed-on: https://go-review.googlesource.com/c/go/+/393757
Run-TryBot: Daniel Martí <mv...@mvdan.cc>
Trust: Matt Layher <mdla...@gmail.com>
Reviewed-by: Robert Griesemer <g...@golang.org>
Trust: Daniel Martí <mv...@mvdan.cc>
TryBot-Result: Gopher Robot <go...@golang.org>
---
M src/cmd/go/testdata/script/fmt_load_errors.txt
M src/cmd/gofmt/gofmt.go
2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/src/cmd/go/testdata/script/fmt_load_errors.txt b/src/cmd/go/testdata/script/fmt_load_errors.txt
index 559dcc5..e3a9034 100644
--- a/src/cmd/go/testdata/script/fmt_load_errors.txt
+++ b/src/cmd/go/testdata/script/fmt_load_errors.txt
@@ -16,6 +16,10 @@
 exec $GOROOT/bin/gofmt gofmt-dir
 ! stdout 'package x'
 
+! exec $GOROOT/bin/gofmt empty.go nopackage.go
+stderr -count=1 'empty\.go:1:1: expected .package., found .EOF.'
+stderr -count=1 'nopackage\.go:1:1: expected .package., found not'
+
 -- exclude/empty/x.txt --
 -- exclude/ignore/_x.go --
 package x
@@ -29,3 +33,6 @@
 package x
 -- gofmt-dir/no-extension --
 package x
+-- empty.go --
+-- nopackage.go --
+not the proper start to a Go file
diff --git a/src/cmd/gofmt/gofmt.go b/src/cmd/gofmt/gofmt.go
index 8efc88d..5fa883f 100644
--- a/src/cmd/gofmt/gofmt.go
+++ b/src/cmd/gofmt/gofmt.go
@@ -347,7 +347,12 @@
 	// stop to avoid corrupting it.)
 	src := make([]byte, size+1)
 	n, err := io.ReadFull(in, src)
-	if err != nil && err != io.ErrUnexpectedEOF {
+	switch err {
+	case nil, io.EOF, io.ErrUnexpectedEOF:
+		// io.ReadFull returns io.EOF (for an empty file) or io.ErrUnexpectedEOF
+		// (for a non-empty file) if the file was changed unexpectedly. Continue
+		// with comparing file sizes in those cases.
+	default:
 		return nil, err
 	}
 	if n < size {

To view, visit change 393757. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I6092391721edad4584fb5922d3e3a8fb3da86493

Gerrit-Change-Number: 393757

Gerrit-PatchSet: 4

Gerrit-Owner: Daniel Martí <mv...@mvdan.cc>

Gerrit-Reviewer: Daniel Martí <mv...@mvdan.cc>

Gerrit-Reviewer: Gopher Robot <go...@golang.org>

Gerrit-Reviewer: Matt Layher <mdla...@gmail.com>

Gerrit-Reviewer: Robert Griesemer <g...@golang.org>

Gerrit-Reviewer: Robert Griesemer <g...@google.com>

Gerrit-MessageType: merged