From f54f780d2f4ed47da4bed87c37f7219df1b9879b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Mon, 29 Jan 2018 11:14:31 +0000
Subject: [PATCH] cmd/vet: unexported interface{} fields on %s are ok
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

For example, the following program is valid:

	type T struct {
		f interface{}
	}

	func main() {
		fmt.Printf("%s", T{"foo"}) // prints {foo}
	}

Since the field is of type interface{}, we might have any value in it.
For example, if we had T{3}, fmt would complain. However, not knowing
what the type under the interface is, we must be conservative.

However, as shown in #17798, we should issue an error if the field's
type is statically known to implement the error or fmt.Stringer
interfaces. In those cases, the user likely wanted the %s format to call
those methods. Keep the vet error in those cases.

While at it, add more field type test cases, such as custom error types,
and interfaces that extend the error interface.

Fixes #23563.

Change-Id: I063885955555917c59da000391b603f0d6dce432
Reviewed-on: https://go-review.googlesource.com/90516
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
---
 src/cmd/vet/testdata/print.go | 34 +++++++++++++++++++++++++++++++---
 src/cmd/vet/types.go          | 14 +++++++++++++-
 2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/src/cmd/vet/testdata/print.go b/src/cmd/vet/testdata/print.go
index 9a0a68dd98..6508c8e615 100644
--- a/src/cmd/vet/testdata/print.go
+++ b/src/cmd/vet/testdata/print.go
@@ -484,6 +484,10 @@ type RecursiveStruct2 struct {
 
 var recursiveStruct1V = &RecursiveStruct1{}
 
+type unexportedInterface struct {
+	f interface{}
+}
+
 // Issue 17798: unexported ptrStringer cannot be formatted.
 type unexportedStringer struct {
 	t ptrStringer
@@ -508,7 +512,23 @@ type errorer struct{}
 
 func (e errorer) Error() string { return "errorer" }
 
+type unexportedCustomError struct {
+	e errorer
+}
+
+type errorInterface interface {
+	error
+	ExtraMethod()
+}
+
+type unexportedErrorInterface struct {
+	e errorInterface
+}
+
 func UnexportedStringerOrError() {
+	fmt.Printf("%s", unexportedInterface{"foo"}) // ok; prints {foo}
+	fmt.Printf("%s", unexportedInterface{3})     // ok; we can't see the problem
+
 	us := unexportedStringer{}
 	fmt.Printf("%s", us)  // ERROR "Printf format %s has arg us of wrong type testdata.unexportedStringer"
 	fmt.Printf("%s", &us) // ERROR "Printf format %s has arg &us of wrong type [*]testdata.unexportedStringer"
@@ -534,10 +554,18 @@ func UnexportedStringerOrError() {
 	fmt.Printf("%s", uef)  // ERROR "Printf format %s has arg uef of wrong type testdata.unexportedErrorOtherFields"
 	fmt.Printf("%s", &uef) // ERROR "Printf format %s has arg &uef of wrong type [*]testdata.unexportedErrorOtherFields"
 
+	uce := unexportedCustomError{
+		e: errorer{},
+	}
+	fmt.Printf("%s", uce) // ERROR "Printf format %s has arg uce of wrong type testdata.unexportedCustomError"
+
+	uei := unexportedErrorInterface{}
+	fmt.Printf("%s", uei)       // ERROR "Printf format %s has arg uei of wrong type testdata.unexportedErrorInterface"
 	fmt.Println("foo\n", "bar") // not an error
-	fmt.Println("foo\n")        // ERROR "Println arg list ends with redundant newline"
-	fmt.Println("foo\\n")       // not an error
-	fmt.Println(`foo\n`)        // not an error
+
+	fmt.Println("foo\n")  // ERROR "Println arg list ends with redundant newline"
+	fmt.Println("foo\\n") // not an error
+	fmt.Println(`foo\n`)  // not an error
 
 	intSlice := []int{3, 4}
 	fmt.Printf("%s", intSlice) // ERROR "Printf format %s has arg intSlice of wrong type \[\]int"
diff --git a/src/cmd/vet/types.go b/src/cmd/vet/types.go
index cf785eed74..1f30b4b42b 100644
--- a/src/cmd/vet/types.go
+++ b/src/cmd/vet/types.go
@@ -269,7 +269,19 @@ func (f *File) matchArgTypeInternal(t printfArgType, typ types.Type, arg ast.Exp
 }
 
 func isConvertibleToString(typ types.Type) bool {
-	return types.AssertableTo(errorType, typ) || stringerType != nil && types.AssertableTo(stringerType, typ)
+	if bt, ok := typ.(*types.Basic); ok && bt.Kind() == types.UntypedNil {
+		// We explicitly don't want untyped nil, which is
+		// convertible to both of the interfaces below, as it
+		// would just panic anyway.
+		return false
+	}
+	if types.ConvertibleTo(typ, errorType) {
+		return true // via .Error()
+	}
+	if stringerType != nil && types.ConvertibleTo(typ, stringerType) {
+		return true // via .String()
+	}
+	return false
 }
 
 // hasBasicType reports whether x's type is a types.Basic with the given kind.