cmd/vet: verify potentially-recursive Stringers are actually Stringers
The printf recursiveStringer check was checking for a function called String(),
but wasn't checking that it matched the actual function signature of Stringer.
Fixes golang/go#30441
Change-Id: I09d5fba035bb717036f7edf57efc63e2e3fe51d5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/164217
Reviewed-by: Alan Donovan <adonovan@google.com>
diff --git a/go/analysis/passes/printf/printf.go b/go/analysis/passes/printf/printf.go
index d4697ea..f59e95d 100644
--- a/go/analysis/passes/printf/printf.go
+++ b/go/analysis/passes/printf/printf.go
@@ -856,20 +856,28 @@
return false
}
- // Is it the receiver r, or &r?
- recv := stringMethod.Type().(*types.Signature).Recv()
- if recv == nil {
+ sig := stringMethod.Type().(*types.Signature)
+ if !isStringer(sig) {
return false
}
+
+ // Is it the receiver r, or &r?
if u, ok := e.(*ast.UnaryExpr); ok && u.Op == token.AND {
e = u.X // strip off & from &r
}
if id, ok := e.(*ast.Ident); ok {
- return pass.TypesInfo.Uses[id] == recv
+ return pass.TypesInfo.Uses[id] == sig.Recv()
}
return false
}
+// isStringer reports whether the method signature matches the String() definition in fmt.Stringer.
+func isStringer(sig *types.Signature) bool {
+ return sig.Params().Len() == 0 &&
+ sig.Results().Len() == 1 &&
+ sig.Results().At(0).Type() == types.Typ[types.String]
+}
+
// isFunctionValue reports whether the expression is a function as opposed to a function call.
// It is almost always a mistake to print a function value.
func isFunctionValue(pass *analysis.Pass, e ast.Expr) bool {
diff --git a/go/analysis/passes/printf/testdata/src/a/a.go b/go/analysis/passes/printf/testdata/src/a/a.go
index 4aeecd5..b783f10 100644
--- a/go/analysis/passes/printf/testdata/src/a/a.go
+++ b/go/analysis/passes/printf/testdata/src/a/a.go
@@ -516,6 +516,20 @@
return fmt.Sprintln(p) // want "Sprintln arg p causes recursive call to String method"
}
+// implements a String() method but with non-matching return types
+type nonStringerWrongReturn int
+
+func (s nonStringerWrongReturn) String() (string, error) {
+ return "", fmt.Errorf("%v", s)
+}
+
+// implements a String() method but with non-matching arguments
+type nonStringerWrongArgs int
+
+func (s nonStringerWrongArgs) String(i int) string {
+ return fmt.Sprintf("%d%v", i, s)
+}
+
type cons struct {
car int
cdr *cons
