Golang fmt: Shouldn't %q (and %s) treat nil as a valid value?

What version of Go are you using (go version)?

$ go version
go version go1.14.4 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="$HOME/.cache/go-build"
GOENV="$HOME/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="$HOME/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/google-golang"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/google-golang/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build196177420=/tmp/go-build -gno-record-gcc-switches"

What did you do?

https://play.golang.org/p/YVE8n6N0xUT

func printError(err error) {
    fmt.Printf("%v, %s, %q\n", err, err, err)
}

func main() {
    printError(nil)
    printError(io.EOF)
}

What did you expect to see?

<nil>, <nil>, <nil>
EOF, EOF, "EOF"

What did you see instead?

<nil>, %!s(<nil>), %!q(<nil>)
EOF, EOF, "EOF"

What's the rationale?

It's often useful to print error values enclosed with quotes in test failures:

if (err != nil) != tc.wantErr {
  t.Fatalf("Foo(...) returned %q, wantErr: %t", err, tc.wantErr) // problematic when err == nil
}

but %q (as well as %s) treats a nil value as a format error and prints %!q(<nil>) instead of plain <nil>. Writing \"%v\" instead of %q avoids the ugly error output, but it doesn't handle escaping well, and it's not desirable to have even "<nil>" enclosed with quotes. This leaves the plain %v as the only easy option (unless you are willing to have another if).

Wouldn't it be nice to have %q (and %s by extension) print just <nil>, as with %v, if the argument is nil? %q and %s already handle an error argument as a string (through the Error method) if the argument is not nil, but having an error output for a nil argument limits its usefulness.

Comment From: ianlancetaylor

CC @robpike

Comment From: robpike

I'm not convinced either way. It seems odd to me that a correct output from %q would not include quotes, but "<nil>" is also clearly wrong.

It may be wisest to do nothing for most types and do something special only for the error type, where people are used to seeing nil in test output and such.

But it's a fiddly thing to do.

Comment From: musiphil

< and > in <nil> (kind of) serve as quotes in the nil case. As you said, using the "..." syntax (even "") would be clearly wrong, but <nil> signals that it's not even a valid string but just the plain nil.

As for the work, wouldn't it suffice to add 'q' and 's' in printArg?

Comment From: dmitshur

I've also run into this issue in the past, and my initial suspicion was that was an issue in fmt package that needed to be fixed. I've spent more time investigating and considering what can be done, but in the end, I couldn't convince myself that there were any favorable changes that should be done in fmt itself. (Of course, it's possible I missed something that can be done.)

I instead modified the code to use %v in cases where the error might be nil, and reserved my use of %s (or %q) to cases where I've checked that it's not.

Comment From: ianlancetaylor

@robpike The catch is that since error is an interface type, a nil error gets converted to the empty interface as nil when calling fmt.Printf and friends. By the time that the fmt package sees the value, it's just a nil value of type interface{}, and the fact that it was once an error has been lost.

Comment From: robpike

I'm not opposed to this change, and I'm also not enthusiastically in favor. My main concern at this point is the effect it will have on tests, as it could change golden output. It might be more disruptive than it's worth, but we can try it in the cycle if you like. As stated above, it's very easy to do. The challenge lies elsewhere.

Comment From: robpike

@ianlancetaylor The package knows it's an error as it needs to check for the Error method, so it could be done there. But it's simpler and easier to explain if the change, should it happen, is done at the top of printArg.

Comment From: ianlancetaylor

@robpike I may be missing something but I think that in this case there is no way that the fmt functions can tell that the value was originally an error. https://play.golang.org/p/8xDEPCAqoFa

Comment From: robpike

@ianlancetaylor I see what you mean. Let's make the conversation more confusing: https://play.golang.org/p/pVnlOIX1ArU

In any case, as I said most recently, it's probably best to do this for all types in printArg if we do it at all. I'd like some discussion about that, though, because it might be disruptive.

Comment From: gonnafaraway

@robpike hi it's still actual.