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: go vet incorrectly warns of "no formatting directive in Errorf call" for non core methods with same name #12294

Closed
piersy opened this issue Aug 24, 2015 · 7 comments

Comments

@piersy
Copy link

piersy commented Aug 24, 2015

We have a log object it provides a method Errorf

Go vet prints a warning - "no formatting directive in Errorf call"

I would expect go vet to be able to determine that this Errorf call is not a call to the standard library Errorf function.

go version go1.5 linux/amd64

@ianlancetaylor ianlancetaylor changed the title go vet incorrectly warns of "no formatting directive in Errorf call" for non core methods with same name cmd/vet: go vet incorrectly warns of "no formatting directive in Errorf call" for non core methods with same name Aug 24, 2015
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Aug 24, 2015
@bradfitz
Copy link
Contributor

What is the signature of your Errorf method?

govet doesn't care about the standard library; it enforces the rule that if your function is named one of the popular -f ending functions, then it should probably be acting like them:

var printfList = map[string]int{
    "errorf":  0,
    "fatalf":  0,
    "fprintf": 1,
    "logf":    0,
    "panicf":  0,
    "printf":  0,
    "sprintf": 0,
}

This is probably working as intended, but please share more details.

@Setomidor
Copy link

I have exactly the same problem, and my Fatalf looks as follows:

func Fatalf(tag string, fullID string, format string, a ...interface{}) {

Is it the extra arguments that matter? The signature of the original method is:

func Fatalf(format string, v ...interface{})

@m3ng9i
Copy link

m3ng9i commented Sep 15, 2015

Similar problem. Here is the code:

// testvet.go
package main

import "fmt"

type S struct {
}

func (this *S) Printf(i int) {
    fmt.Println(i)
}

func main() {
    s := &S{}
    s.Printf(1)
}

After run go vet testvet.go, I got an error: testvet.go:16: constant 1 not a string in call to Printf.

But parameter of S.Printf() is exactly int, not string.

My go version: go1.5.1 windows/386

@ianlancetaylor
Copy link
Contributor

@m3ng9i This is working as expected. Your choices are 1) rename your Printf function; 2) don't run vet.

@spenczar
Copy link
Contributor

I'd like to revive this topic, motivated by gRPC's Errorf function which triggers vet.

gRPC has a public function with this signature:

func Errorf(c codes.Code, format string, a ...interface{}) error

Using this function causes vet to shriek, as noted in grpc/grpc-go#90. Given that grpc is maintained by core Go authors who are unwilling to rename their function, I think it's fair to say some more effort in go vet's side here would be worthwhile to expand its set of covered programs.

It seems to me that vet could be a little more intelligent here. It should be easily possible to identify the function's call signature and figure out whether its first parameter is a string. Vet could merely warn that it's unable to handle non-standard Printf calls, or perhaps it could try to suss out which parameter is a format string, and appropriately vet the arguments passed in.

This comment is a proposal to gauge interest; I'm happy to be the one to do the work of writing the CL but I want to make sure that this change to vet's behavior is desirable.

@gopherbot
Copy link

CL https://golang.org/cl/20163 mentions this issue.

@valyala
Copy link
Contributor

valyala commented Mar 10, 2016

The CL https://golang.org/cl/20163 breaks go vet -printfuncs=FooBarf if FooBarf is defined outside the current package. It looks like signature() returns nil in this case, which results in no formatting directive in FooBarf call warnings for each FooBarf(format, <args>) call.
I'd suggest falling back to the previous behavior if signature() returns nil.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants