Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
bryancallahan opened this issue Feb 23, 2019 · 13 comments
Open

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

bryancallahan opened this issue Feb 23, 2019 · 13 comments
Milestone

Comments

@bryancallahan
Copy link

@bryancallahan bryancallahan commented Feb 23, 2019

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
Copy link
Contributor

@antong antong commented Feb 23, 2019

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
Copy link
Contributor

@antong antong commented Feb 23, 2019

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
Copy link
Contributor

@josharian josharian commented Feb 23, 2019

@martisch for fmt

@mvdan for the vet check

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

@robpike
Copy link
Contributor

@robpike robpike commented Feb 23, 2019

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 josharian added this to the Go1.12 milestone Feb 23, 2019
@josharian
Copy link
Contributor

@josharian josharian commented Feb 23, 2019

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
Copy link
Contributor

@antong antong commented Feb 23, 2019

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
Copy link
Contributor

@robpike robpike commented Feb 23, 2019

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.

@josharian josharian modified the milestones: Go1.12, Go1.13 Feb 23, 2019
@bryancallahan
Copy link
Author

@bryancallahan bryancallahan commented Feb 25, 2019

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
Copy link
Member

@bcmills bcmills commented Feb 25, 2019

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

@alandonovan
Copy link
Contributor

@alandonovan alandonovan commented Feb 25, 2019

@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
Copy link
Contributor

@antong antong commented Feb 25, 2019

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
Copy link
Contributor

@alandonovan alandonovan commented Feb 25, 2019

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

Much better!

@bryancallahan
Copy link
Author

@bryancallahan bryancallahan commented Feb 26, 2019

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! 👍

@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
You can’t perform that action at this time.