cmd/vet: **T is not Stringer if *T has a String method

vet recorded what types had String methods defined on them, but it did
not record whether the receivers were pointer types. That information is
important, as the following program is valid:

	type T string

	func (t *T) String() string {
		return fmt.Sprint(&t) // prints address
	}

Teach vet that, if *T is Stringer, **T is not.

Fixes #23550.

Change-Id: I1062e60e6d82e789af9cca396546db6bfc3541e8
Reviewed-on: https://go-review.googlesource.com/90417
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

diff --git a/src/cmd/vet/main.go b/src/cmd/vet/main.go
index 807e80095904d4e088fbc700f2caf86075b44597..7265aa6f5793f7ca6b8f0407414db5fd57547464 100644 (file)
--- a/src/cmd/vet/main.go
+++ b/src/cmd/vet/main.go
@@ -195,9 +195,11 @@ type File struct {
        // Parsed package "foo" when checking package "foo_test"
        basePkg *Package
 
-       // The objects that are receivers of a "String() string" method.
+       // The keys are the objects that are receivers of a "String()
+       // string" method. The value reports whether the method has a
+       // pointer receiver.
        // This is used by the recursiveStringer method in print.go.
-       stringers map[*ast.Object]bool
+       stringerPtrs map[*ast.Object]bool
 
        // Registered checkers to run.
        checkers map[ast.Node][]func(*File, ast.Node)

diff --git a/src/cmd/vet/print.go b/src/cmd/vet/print.go
index 1c015913d5f4fa5ce580e2712b59c77dc5037798..0cff951f6f173b4f120d56ff2ddafb517f108933 100644 (file)
--- a/src/cmd/vet/print.go
+++ b/src/cmd/vet/print.go
@@ -187,12 +187,14 @@ func checkFmtPrintfCall(f *File, node ast.Node) {
 
        if d, ok := node.(*ast.FuncDecl); ok && isStringer(f, d) {
                // Remember we saw this.
-               if f.stringers == nil {
-                       f.stringers = make(map[*ast.Object]bool)
+               if f.stringerPtrs == nil {
+                       f.stringerPtrs = make(map[*ast.Object]bool)
                }
                if l := d.Recv.List; len(l) == 1 {
                        if n := l[0].Names; len(n) == 1 {
-                               f.stringers[n[0].Obj] = true
+                               typ := f.pkg.types[l[0].Type]
+                               _, ptrRecv := typ.Type.(*types.Pointer)
+                               f.stringerPtrs[n[0].Obj] = ptrRecv
                        }
                }
                return
@@ -628,9 +630,10 @@ func (f *File) okPrintfArg(call *ast.CallExpr, state *formatState) (ok bool) {
 // recursiveStringer reports whether the provided argument is r or &r for the
 // fmt.Stringer receiver identifier r.
 func (f *File) recursiveStringer(e ast.Expr) bool {
-       if len(f.stringers) == 0 {
+       if len(f.stringerPtrs) == 0 {
                return false
        }
+       ptr := false
        var obj *ast.Object
        switch e := e.(type) {
        case *ast.Ident:
@@ -638,6 +641,7 @@ func (f *File) recursiveStringer(e ast.Expr) bool {
        case *ast.UnaryExpr:
                if id, ok := e.X.(*ast.Ident); ok && e.Op == token.AND {
                        obj = id.Obj
+                       ptr = true
                }
        }
 
@@ -652,7 +656,16 @@ func (f *File) recursiveStringer(e ast.Expr) bool {
        // We compare the underlying Object, which checks that the identifier
        // is the one we declared as the receiver for the String method in
        // which this printf appears.
-       return f.stringers[obj]
+       ptrRecv, exist := f.stringerPtrs[obj]
+       if !exist {
+               return false
+       }
+       // We also need to check that using &t when we declared String
+       // on (t *T) is ok; in such a case, the address is printed.
+       if ptr && ptrRecv {
+               return false
+       }
+       return true
 }
 
 // isFunctionValue reports whether the expression is a function as opposed to a function call.

diff --git a/src/cmd/vet/testdata/print.go b/src/cmd/vet/testdata/print.go
index 6725bafadfb4a49804339c38f8445cc5f2a19192..b36abfc127da6ceeeb4b46d7e49dc114d3914380 100644 (file)
--- a/src/cmd/vet/testdata/print.go
+++ b/src/cmd/vet/testdata/print.go
@@ -440,6 +440,7 @@ type recursivePtrStringer int
 
 func (p *recursivePtrStringer) String() string {
        _ = fmt.Sprintf("%v", *p)
+       _ = fmt.Sprint(&p)     // ok; prints address
        return fmt.Sprintln(p) // ERROR "Sprintln arg p causes recursive call to String method"
 }