From cd9959d7961630233395ad0a1167a51c3f5b06ef Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Mon, 25 Aug 2014 12:31:49 -0700 Subject: [PATCH] go.tools/cmd/vet: validate calls to t.Log and t.Logf Be careful not to complain about math.Log and cmplx.Log. Seems worthwhile since t.Log and t.Logf are often written but rarely executed. Nothing new turned up in the standard library. Fixes golang/go#8504. LGTM=josharian, dsymonds R=golang-codereviews, josharian, dsymonds CC=golang-codereviews https://golang.org/cl/130490043 --- cmd/vet/doc.go | 1 + cmd/vet/print.go | 14 ++++++++++++++ cmd/vet/testdata/print.go | 8 ++++++++ 3 files changed, 23 insertions(+) diff --git a/cmd/vet/doc.go b/cmd/vet/doc.go index 2b7e1432ee..b610b40174 100644 --- a/cmd/vet/doc.go +++ b/cmd/vet/doc.go @@ -49,6 +49,7 @@ with these names, disregarding case: Sprint Sprintf Sprintln Error Errorf Fatal Fatalf + Log Logf Panic Panicf Panicln If the function name ends with an 'f', the function is assumed to take a format descriptor string in the manner of fmt.Printf. If not, vet diff --git a/cmd/vet/print.go b/cmd/vet/print.go index 86cb9c87b0..86958c71ca 100644 --- a/cmd/vet/print.go +++ b/cmd/vet/print.go @@ -35,6 +35,7 @@ var printfList = map[string]int{ "errorf": 0, "fatalf": 0, "fprintf": 1, + "logf": 0, "panicf": 0, "printf": 0, "sprintf": 0, @@ -47,6 +48,7 @@ var printList = map[string]int{ "error": 0, "fatal": 0, "fprint": 1, "fprintln": 1, + "log": 0, "panic": 0, "panicln": 0, "print": 0, "println": 0, "sprint": 0, "sprintln": 0, @@ -494,6 +496,18 @@ func (f *File) checkPrint(call *ast.CallExpr, name string, firstArg int) { isLn := strings.HasSuffix(name, "ln") isF := strings.HasPrefix(name, "F") args := call.Args + if name == "Log" { + // Special case: Don't complain about math.Log or cmplx.Log. + // Not strictly necessary because the only complaint likely is for Log("%d") + // but it feels wrong to check that math.Log is a good print function. + if sel, ok := args[0].(*ast.SelectorExpr); ok { + if x, ok := sel.X.(*ast.Ident); ok { + if x.Name == "math" || x.Name == "cmplx" { + return + } + } + } + } // check for Println(os.Stderr, ...) if firstArg == 0 && !isF && len(args) > 0 { if sel, ok := args[0].(*ast.SelectorExpr); ok { diff --git a/cmd/vet/testdata/print.go b/cmd/vet/testdata/print.go index 4f3d249d7c..3875ac5069 100644 --- a/cmd/vet/testdata/print.go +++ b/cmd/vet/testdata/print.go @@ -8,6 +8,7 @@ package testdata import ( "fmt" + "math" "os" "unsafe" // just for test case printing unsafe.Pointer ) @@ -197,6 +198,13 @@ func PrintfTests() { Printf("%p %x", recursiveStruct1V, recursiveStruct1V.next) Printf("%p %x", recursiveSliceV, recursiveSliceV) Printf("%p %x", recursiveMapV, recursiveMapV) + // Special handling for Log. + math.Log(3) // OK + Log(3) // OK + Log("%d", 3) // ERROR "possible formatting directive in Log call" + Logf("%d", 3) + Logf("%d", "hi") // ERROR "arg .hi. for printf verb %d of wrong type: untyped string" + } // Printf is used by the test so we must declare it.