golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.85k stars 17.65k forks source link

cmd/vet: Printf format %.T has unrecognized flag #30363

Open bryancallahan opened 5 years ago

bryancallahan commented 5 years ago

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

$ go version
go version go1.11.2 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/demouser/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/demouser/workspace/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.11.2/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.11.2/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
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=/var/folders/rf/xcy7qhgd7nldd0txw__11gpw0000gn/T/go-build280768088=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

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

package main
import "fmt"
func main() {
    fmt.Printf("arg1=%s, arg2=%.T, arg3=%.T, arg4=%.T, arg=%s", "arg1", 2.123, true, "arg4", "arg5")
}

What did you expect to see?

arg1=arg1, arg2=, arg3=, arg4=, arg=arg5
Program exited.

What did you see instead?

prog.go:6: Printf format %.T has unrecognized flag .
Go vet exited.

arg1=arg1, arg2=, arg3=, arg4=, arg=arg5
Program exited.
antong commented 5 years ago

EDIT: Oh, or did you mean that the dot flag is and should be expected (as for strings), and you really wanted the input truncated to 0 in length and you wanted to produce the empty output and wanted vet to shut up? Perhaps clearer example: https://play.golang.org/p/k7yNgXsiVGX


Old-comment:

So, the %T formatting verb doesn't accept the dot flag . . You should write "%T" and not "%.T". The vet command warns about this incorrect usage. I think the result is exactly as expected!

Another thing though, the documentation for flags says:

Flags are ignored by verbs that do not expect them. For example there is no alternate decimal format, so %#d and %d behave identically.

This seems to be a bit in conflict with the implementation. To me that reads as though %T and %.T should give identical output, because %T doesn't expect the dot . and so should ignore it. Now %.T produces empty output.

antong commented 5 years ago

So clearly fmt accepts e.g., %.3T and actually does something with it (truncates the type string). Vet complains and only expects the - flag (src). I couldn't find any clear mention of whether and how precision should work for the T verb in the documentation.

josharian commented 5 years ago

@martisch for fmt

@mvdan for the vet check

@robpike for the correct behavior of %.T and possible docs needed

robpike commented 5 years ago

This is new behavior. %.3T didn't work in 1.11 (https://play.golang.org/p/WzF7OfLxUiw) but it does work in 1.12. (It treats the type name as a string.) . I do not know why it changed, or whether it was a deliberate change. I'd like to understand that before making any decisions.

That said, "%.s" means print the string with zero length, so "%.T" in 1.12 produces no output. Not sure what you're trying to achieve here.

josharian commented 5 years ago

Tentatively marking as release-blocker for 1.12 just in case we want to revert this behavior change before we're locked into it.

antong commented 5 years ago

Behavior of vet or fmt.Printf? It seems like Printf has accepted a precision for %T for a long time:

~ $ docker run -it golang:1.2
...
root@9216d56256e1:/go# cd
root@9216d56256e1:~# cat > foo.go
package main

import "fmt"

func main() {
        fmt.Printf("%.4T\n", int64(33))
}
root@9216d56256e1:~# go run foo.go
int6
root@9216d56256e1:~# go version
go version go1.2.2 linux/amd64

Vet also complained in 1.10:

~ $ docker run -it golang:1.10
root@291a4e845228:/go# cd
root@291a4e845228:~# cat > foo.go
package main

import "fmt"

func main() {
        fmt.Printf("%.4T\n", int64(33))
}
root@291a4e845228:~# go vet foo.go
# command-line-arguments
./foo.go:6: Printf format %.4T has unrecognized flag .
root@291a4e845228:~# go run foo.go
int6
root@291a4e845228:~# go version
go version go1.10.8 linux/amd64
robpike commented 5 years ago

It worked all the way back in go1.4, the oldest version I have installed. So perhaps it broke in 1.11 only, and its behavior has been restored. No need to block the release over this.

bryancallahan commented 5 years ago

That said, "%.s" means print the string with zero length, so "%.T" in 1.12 produces no output. Not sure what you're trying to achieve here.

For my use case, I am trying to truncate the type string to zero (in order words, trying to produce no output) as you mentioned.

As a concrete example, I let admins configure a weather alert (https://play.golang.org/p/B1oWVEWxTvy) like this:

adminConfiguredMessage = "Ski resort just hit with over %.0[1]f\" of fresh snow today. %[3]s%.[2]T" ...or... adminConfiguredMessage = "Ski resort just hit with over %.0[1]f\" of fresh snow and it's %.0[2]f degrees today.%.[3]T"

Then I can format either string with: fmt.Printf(fmt.Sprintf(adminConfiguredMessage, 4.221, 33.12, "http://my.weatheralerts.com")). So they result in:

Ski resort just hit with over 4" of fresh snow today. http://my.weatheralerts.com ...or... Ski resort just hit with over 4" of fresh snow and it's 33 degrees today.

If they don't want to use a string argument they can suppress it with %.s. For float strings, however, the hope was to convert to the type string %T and then truncate the string %.T. From what I read of the docs, it seemed like this should work properly. It does but I just ran into that "Go vet" issue so I'm not sure if this is a safe approach to satisfy my use case or not.

bcmills commented 5 years ago

(CC @alandonovan @matloob @ianthehat for cmd/vet.)

alandonovan commented 5 years ago

@bryancallahan It is unfortunate that printf provides no clear way to consume and discard argument of any type without printing anything. That said, printing a zero-width type string is clearly a hack, and it would likely get called out during a code review. If you need to dynamically vary the format string and can't do so in logic, the text/template package is the right tool for the job: its template strings are self-documenting.

All of which is to say: this may be a bug in vet, but I don't think it's very important.

[Update: I was mistaken. Thanks to antong for pointing out that a format string using indexed notation is not subject to the unused parameter check.]

antong commented 5 years ago

Aah, so you were trying to suppress the printf error output like !(EXTRA float64=33.12) ? Actually, if you do use the [n] indexing notation (e.g., "%2.3[2]f") anywhere in the format string, then the extra error output will be suppressed and I think vet will not complain either. So you could perhaps use indexed arguments everywhere and no need to resort to the %T type verb.

Example: https://play.golang.org/p/GqlrZ42nNBF

But yes, this sounds like a job for a template package. I don't know how safe text/template is for user controlled templates.

alandonovan commented 5 years ago

if you do use the [n] indexing notation...then the extra error output will be suppressed.

Much better!

bryancallahan commented 5 years ago

Very awesome, @antong! Thanks so much for the help and clarification on this. This is exactly what I was going for. @alandonovan in the future I may look into the text/template package. (I started out w/ this approach because I wanted to keep things simple until I had a sufficient requirement to "grow into" the text/template package.) Again, I very much appreciate the clarification and consideration on this you two! 👍