Skip to content

Commit

Permalink
go/analysis/passes/printf: warn about verbs that don’t match a type
Browse files Browse the repository at this point in the history
parameter’s type set

Pending the acceptance of golang/go#49038, this CL updates the printf
analyzer to warn if any elements of a type parameter's type set do not
match the printf verb being considered. Since this may be a source of
confusion, it also refactors slightly to allow providing a reason why a
match failed.

Updates golang/go#48704
Updates golang/go#49038

Change-Id: I92d4d58dd0e9218ae9d522bf2a2999f4c6f9fd84
Reviewed-on: https://go-review.googlesource.com/c/tools/+/351832
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Tim King <taking@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
  • Loading branch information
findleyr committed Nov 6, 2021
1 parent b8b8e7f commit ddcef59
Show file tree
Hide file tree
Showing 5 changed files with 265 additions and 62 deletions.
16 changes: 12 additions & 4 deletions go/analysis/passes/printf/printf.go
Original file line number Diff line number Diff line change
Expand Up @@ -879,8 +879,12 @@ func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, state *formatState) (o
return
}
arg := call.Args[argNum]
if !matchArgType(pass, argInt, arg) {
pass.ReportRangef(call, "%s format %s uses non-int %s as argument of *", state.name, state.format, analysisutil.Format(pass.Fset, arg))
if reason, ok := matchArgType(pass, argInt, arg); !ok {
details := ""
if reason != "" {
details = " (" + reason + ")"
}
pass.ReportRangef(call, "%s format %s uses non-int %s%s as argument of *", state.name, state.format, analysisutil.Format(pass.Fset, arg), details)
return false
}
}
Expand All @@ -897,12 +901,16 @@ func okPrintfArg(pass *analysis.Pass, call *ast.CallExpr, state *formatState) (o
pass.ReportRangef(call, "%s format %s arg %s is a func value, not called", state.name, state.format, analysisutil.Format(pass.Fset, arg))
return false
}
if !matchArgType(pass, v.typ, arg) {
if reason, ok := matchArgType(pass, v.typ, arg); !ok {
typeString := ""
if typ := pass.TypesInfo.Types[arg].Type; typ != nil {
typeString = typ.String()
}
pass.ReportRangef(call, "%s format %s has arg %s of wrong type %s", state.name, state.format, analysisutil.Format(pass.Fset, arg), typeString)
details := ""
if reason != "" {
details = " (" + reason + ")"
}
pass.ReportRangef(call, "%s format %s has arg %s of wrong type %s%s", state.name, state.format, analysisutil.Format(pass.Fset, arg), typeString, details)
return false
}
if v.typ&argString != 0 && v.verb != 'T' && !bytes.Contains(state.flags, []byte{'#'}) {
Expand Down
8 changes: 7 additions & 1 deletion go/analysis/passes/printf/printf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,16 @@ import (

"golang.org/x/tools/go/analysis/analysistest"
"golang.org/x/tools/go/analysis/passes/printf"
"golang.org/x/tools/internal/typeparams"
)

func Test(t *testing.T) {
testdata := analysistest.TestData()
printf.Analyzer.Flags.Set("funcs", "Warn,Warnf")
analysistest.Run(t, testdata, printf.Analyzer, "a", "b", "nofmt")

tests := []string{"a", "b", "nofmt"}
if typeparams.Enabled {
tests = append(tests, "typeparams")
}
analysistest.Run(t, testdata, printf.Analyzer, tests...)
}
2 changes: 2 additions & 0 deletions go/analysis/passes/printf/testdata/src/a/a.go
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,7 @@ type unexportedInterface struct {
type unexportedStringer struct {
t ptrStringer
}

type unexportedStringerOtherFields struct {
s string
t ptrStringer
Expand All @@ -708,6 +709,7 @@ type unexportedStringerOtherFields struct {
type unexportedError struct {
e error
}

type unexportedErrorOtherFields struct {
s string
e error
Expand Down
111 changes: 111 additions & 0 deletions go/analysis/passes/printf/testdata/src/typeparams/diagnostics.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// Copyright 2021 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

//go:build go1.18
// +build go1.18

package typeparams

import "fmt"

func TestBasicTypeParams[T interface{ ~int }, E error, F fmt.Formatter, S fmt.Stringer, A any](t T, e E, f F, s S, a A) {
fmt.Printf("%d", t)
fmt.Printf("%s", t) // want "wrong type.*contains ~int"
fmt.Printf("%v", t)
fmt.Printf("%d", e)
fmt.Printf("%s", e)
fmt.Errorf("%w", e)
fmt.Printf("%a", f)
fmt.Printf("%d", f)
fmt.Printf("%T", f.Format)
fmt.Printf("%p", f.Format)
fmt.Printf("%s", s)
fmt.Errorf("%w", s) // want "wrong type"
fmt.Printf("%d", a) // want "wrong type"
fmt.Printf("%s", a) // want "wrong type"
fmt.Printf("%v", a)
fmt.Printf("%T", a)
}

func TestNestedTypeParams[T interface{ ~int }, S interface{ ~string }]() {
var x struct {
f int
t T
}
fmt.Printf("%d", x)
fmt.Printf("%s", x) // want "wrong type"
var y struct {
f string
t S
}
fmt.Printf("%d", y) // want "wrong type"
fmt.Printf("%s", y)
var m1 map[T]T
fmt.Printf("%d", m1)
fmt.Printf("%s", m1) // want "wrong type"
var m2 map[S]S
fmt.Printf("%d", m2) // want "wrong type"
fmt.Printf("%s", m2)
}

type R struct {
F []R
}

func TestRecursiveTypeDefinition() {
var r []R
fmt.Printf("%d", r) // No error: avoids infinite recursion.
}

func TestRecursiveTypeParams[T1 ~[]T2, T2 ~[]T1 | string, T3 ~struct{ F T3 }](t1 T1, t2 T2, t3 T3) {
// No error is reported on the following lines to avoid infinite recursion.
fmt.Printf("%s", t1)
fmt.Printf("%s", t2)
fmt.Printf("%s", t3)
}

func TestRecusivePointers[T1 ~*T2, T2 ~*T1](t1 T1, t2 T2) {
// No error: we can't determine if pointer rules apply.
fmt.Printf("%s", t1)
fmt.Printf("%s", t2)
}

func TestEmptyTypeSet[T interface{ int | string; float64 }](t T) {
fmt.Printf("%s", t) // No error: empty type set.
}

func TestPointerRules[T ~*[]int|*[2]int](t T) {
var slicePtr *[]int
var arrayPtr *[2]int
fmt.Printf("%d", slicePtr)
fmt.Printf("%d", arrayPtr)
fmt.Printf("%d", t)
}

func TestInterfacePromotion[E interface {
~int
Error() string
}, S interface {
float64
String() string
}](e E, s S) {
fmt.Printf("%d", e)
fmt.Printf("%s", e)
fmt.Errorf("%w", e)
fmt.Printf("%d", s) // want "wrong type.*contains float64"
fmt.Printf("%s", s)
fmt.Errorf("%w", s) // want "wrong type"
}

type myInt int

func TestTermReduction[T1 interface{ ~int | string }, T2 interface {
~int | string
myInt
}](t1 T1, t2 T2) {
fmt.Printf("%d", t1) // want "wrong type.*contains string"
fmt.Printf("%s", t1) // want "wrong type.*contains ~int"
fmt.Printf("%d", t2)
fmt.Printf("%s", t2) // want "wrong type.*contains typeparams.myInt"
}

0 comments on commit ddcef59

Please sign in to comment.