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: errors on unknown Printf verb that type implements when using log.Printf #31000

Closed
theckman opened this issue Mar 22, 2019 · 10 comments

Comments

Projects
None yet
4 participants
@theckman
Copy link
Contributor

commented Mar 22, 2019

Based on quick testing, this issue looks like it may be fixed in tip and needs a backport to 1.12.x.

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

$ go version
go version go1.12.1 darwin/amd64

Does this issue reproduce with the latest release?

1.11.6: no
1.12.1: yes
tip: no

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/theckman/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/theckman/go/"
GOPROXY=""
GORACE=""
GOROOT="/Users/theckman/.gimme/versions/go"
GOTMPDIR=""
GOTOOLDIR="/Users/theckman/.gimme/versions/go/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/f5/zjcsdkrx2bxdm32zzqfg5yp80000gn/T/go-build648711792=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

$ go vet file.go
# command-line-arguments
./file.go:14:2: Printf format %S has unknown verb S
Contents of file.go:
package main

import (
	"log"

	"github.com/gofrs/uuid"
)

func main() {
	u2, err := uuid.NewV4()
	if err != nil {
		log.Fatalf("failed to generate UUID: %v", err)
	}
	log.Printf("capitalized UUID: %S\n", u2) /* ERROR HERE !! */
}

What did you expect to see?

No error.

@theckman theckman changed the title cmd/go: vet complains about unknown Printf verb that type implements cmd/go: vet errors on unknown Printf verb that type implements Mar 22, 2019

@theckman theckman changed the title cmd/go: vet errors on unknown Printf verb that type implements cmd/vet: vet errors on unknown Printf verb that type implements Mar 22, 2019

@theckman theckman changed the title cmd/vet: vet errors on unknown Printf verb that type implements cmd/vet: errors on unknown Printf verb that type implements Mar 22, 2019

@theckman

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

Here's the implementation of %S support:

@mvdan

This comment has been minimized.

Copy link
Member

commented Mar 22, 2019

Hmm, I can reproduce the vet warning on go1.11.5, go1.12.1, and go version devel +277609f844 Thu Mar 21 08:41:51 2019 +0000 linux/amd64. They are all using an empty module with just require github.com/gofrs/uuid v3.2.0+incompatible and the file you provided.

I haven't formed an opinion on whether this is a bug or not; I just can't reproduce this being a regression or being fixed in tip.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Mar 22, 2019

Seems the same as #30399: cmd/vet shouldn't complain about the use of an unrecognized formatting character with a type that implements fmt.Formatter.

@mvdan

This comment has been minimized.

Copy link
Member

commented Mar 22, 2019

Hmm, I don't think it's an exact duplicate. That was fixed in x/tools master weeks ago, and cmd/vendor contains a recent enough copy of it. I just manually checked the code, and my fix for go/analysis/passes/printf is in there.

Edit: to clarify, the reason I don't think it's an exact duplicate is because I still get the warning with cmd/vet on master, which contains my fix for #30399.

@dylan-bourque

This comment has been minimized.

Copy link

commented Mar 22, 2019

I was the author of the change in gofrs/uuid and I cannot reproduce the go vet warning with v1.12.1. See my comment here -> gofrs/uuid#77 (comment).

I definitely agree that cmd/vet shouldn't be complaining about non-standard formatting characters, though.

@dylan-bourque

This comment has been minimized.

Copy link

commented Mar 22, 2019

Did some more testing and this does seem to be the same issue as #30399. If I change my code to use log.Printf instead of fmt.Printf then go vet does indeed complain about the unknown formatting verb.

@dylan-bourque

This comment has been minimized.

Copy link

commented Mar 22, 2019

This generates the error:

package main

import (
    "log"
    "github.com/gofrs/uuid"
)

func main() {
    u := uuid.Must(uuid.NewV4())
    log.Printf("capitalized UUID: %S", u)
}

This does not:

package main

import (
    "fmt"
    "github.com/gofrs/uuid"
)

func main() {
    u := uuid.Must(uuid.NewV4())
    fmt.Printf("capitalized UUID: %S", u)
}

@theckman theckman changed the title cmd/vet: errors on unknown Printf verb that type implements cmd/vet: errors on unknown Printf verb that type implements when using log.Printf Mar 22, 2019

@mvdan

This comment has been minimized.

Copy link
Member

commented Mar 22, 2019

Ok, I finally get what's going on. This is using github.com/gofrs/uuid@master, not github.com/gofrs/uuid@latest. The %S change was done after the latest tagged release.

This explains why I was getting the warning at every Go version.

Anyway, yes, this is indeed a duplicate of #30399, and a backport is already ready for 1.12.2.

@mvdan mvdan closed this Mar 22, 2019

@theckman

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

@mvdan I'm not using modules, so I'm using neither.

@mvdan

This comment has been minimized.

Copy link
Member

commented Mar 22, 2019

Well, the original report didn't mention any version, so my assumption was "the latest tagged version". It would have been easier if the post was clear :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.