go.tools/cmd/vet: check for recursive stringers

Fixes golang/go#6129.

R=r
CC=golang-dev
https://golang.org/cl/12936046

cmd/vet/main.go

@@ -87,6 +87,10 @@ type File struct {
 	content []byte
 	file    *ast.File
 	b       bytes.Buffer // for use by methods
+
+	// The last "String() string" method receiver we saw while walking.
+	// This is used by the recursiveStringer method in print.go.
+	lastStringerReceiver *ast.Object
 }
 
 func main() {
@@ -429,6 +433,29 @@ func (f *File) walkFuncDecl(d *ast.FuncDecl) {
 	if d.Recv != nil {
 		f.walkMethod(d.Name, d.Type)
 	}
+	f.prepStringerReceiver(d)
+}
+
+// prepStringerReceiver checks whether the given declaration is a fmt.Stringer
+// implementation, and if so sets the File's lastStringerReceiver field to the
+// declaration's receiver object.
+func (f *File) prepStringerReceiver(d *ast.FuncDecl) {
+	if !f.isStringer(d) {
+		return
+	}
+	if l := d.Recv.List; len(l) == 1 {
+		if n := l[0].Names; len(n) == 1 {
+			f.lastStringerReceiver = n[0].Obj
+		}
+	}
+}
+
+// isStringer returns true if the provided declaration is a "String() string"
+// method; an implementation of fmt.Stringer.
+func (f *File) isStringer(d *ast.FuncDecl) bool {
+	return d.Recv != nil && d.Name.Name == "String" &&
+		len(d.Type.Params.List) == 0 && len(d.Type.Results.List) == 1 &&
+		f.pkg.types[d.Type.Results.List[0].Type] == types.Typ[types.String]
 }
 
 // walkGenDecl walks a general declaration.

cmd/vet/print.go

@@ -385,9 +385,38 @@ func (f *File) okPrintfArg(call *ast.CallExpr, state *formatState) (ok bool) {
 		f.Badf(call.Pos(), "arg %s for printf verb %%%c of wrong type: %s", f.gofmt(arg), state.verb, typeString)
 		return false
 	}
+	if v.typ&argString != 0 && f.recursiveStringer(arg) {
+		f.Badf(call.Pos(), "arg %s for printf causes recursive call to String method", f.gofmt(arg))
+		return false
+	}
 	return true
 }
 
+// recursiveStringer reports whether the provided argument is r, *r, or &r
+// for the fmt.Stringer receiver identifier r.
+func (f *File) recursiveStringer(arg ast.Expr) bool {
+	if f.lastStringerReceiver == nil {
+		return false
+	}
+	var obj *ast.Object
+	switch e := arg.(type) {
+	case *ast.Ident:
+		obj = e.Obj
+	case *ast.StarExpr:
+		if id, ok := e.X.(*ast.Ident); ok {
+			obj = id.Obj
+		}
+	case *ast.UnaryExpr:
+		if id, ok := e.X.(*ast.Ident); ok && e.Op == token.AND {
+			obj = id.Obj
+		}
+	}
+	// 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 obj == f.lastStringerReceiver
+}
+
 // argCanBeChecked reports whether the specified argument is statically present;
 // it may be beyond the list of arguments or in a terminal slice... argument, which
 // means we can't see it.
@@ -464,4 +493,9 @@ func (f *File) checkPrint(call *ast.CallExpr, name string, firstArg int) {
 			}
 		}
 	}
+	for _, arg := range args {
+		if f.recursiveStringer(arg) {
+			f.Badf(call.Pos(), "arg %s for print causes recursive call to String method", f.gofmt(arg))
+		}
+	}
 }

cmd/vet/testdata/print.go

@@ -269,3 +269,19 @@ type percentSStruct struct {
 }
 
 var percentSV percentSStruct
+
+type recursiveStringer int
+
+func (s recursiveStringer) String() string {
+	fmt.Sprintf("%v", &s) // ERROR "arg &s for printf causes recursive call to String method"
+	fmt.Sprintf("%d", s)
+	fmt.Sprintf("%v", s)   // ERROR "arg s for printf causes recursive call to String method"
+	return fmt.Sprintln(s) // ERROR "arg s for print causes recursive call to String method"
+}
+
+type recursivePtrStringer int
+
+func (p *recursivePtrStringer) String() string {
+	fmt.Sprintf("%v", *p)  // ERROR "arg \*p for printf causes recursive call to String method"
+	return fmt.Sprintln(p) // ERROR "arg p for print causes recursive call to String method"
+}