fmt: only use Stringer or Error for strings

This is a slight change to fmt's semantics, but means that if you use
%d to print an integer with a Stringable value, it will print as an integer.
This came up because Time.Month() couldn't cleanly print as an integer
rather than a name. Using %d on Stringables is silly anyway, so there
should be no effect outside the fmt tests.
As a mild bonus, certain recursive failures of String methods
will also be avoided this way.

R=golang-dev, adg
CC=golang-dev
https://golang.org/cl/5453053

3 files changed, 42 insertions, 27 deletions

diff --git a/src/pkg/fmt/doc.go b/src/pkg/fmt/doc.go
index 6713f0a16e..747865c6f9 100644
--- a/src/pkg/fmt/doc.go
+++ b/src/pkg/fmt/doc.go
@@ -89,18 +89,22 @@
 	If an operand implements interface Formatter, that interface
 	can be used for fine control of formatting.
 
-	Next, if an operand implements the error interface, the Error method
+	If the format (which is implicitly %v for Println etc.) is valid
+	for a string (%s %q %v %x %X), the following two rules also apply:
+
+	1. If an operand implements the error interface, the Error method
 	will be used to convert the object to a string, which will then
 	be formatted as required by the verb (if any).
 
-	Finally, if an operand implements method String() string that method
+	2. If an operand implements method String() string, that method
 	will be used to convert the object to a string, which will then
 	be formatted as required by the verb (if any).
+
 	To avoid recursion in cases such as
-		type X int
-		func (x X) String() string { return Sprintf("%d", x) }
+		type X string
+		func (x X) String() string { return Sprintf("<%s>", x) }
 	cast the value before recurring:
-		func (x X) String() string { return Sprintf("%d", int(x)) }
+		func (x X) String() string { return Sprintf("<%s>", string(x)) }
 
 	Format errors:
 
diff --git a/src/pkg/fmt/fmt_test.go b/src/pkg/fmt/fmt_test.go
index f937a454ed..d42a8fe1f2 100644
--- a/src/pkg/fmt/fmt_test.go
+++ b/src/pkg/fmt/fmt_test.go
@@ -12,6 +12,7 @@ import (
 	"runtime" // for the malloc count test only
 	"strings"
 	"testing"
+	"time"
 )
 
 type (
@@ -352,7 +353,7 @@ var fmttests = []struct {
 	{"%s", I(23), `<23>`},
 	{"%q", I(23), `"<23>"`},
 	{"%x", I(23), `3c32333e`},
-	{"%d", I(23), `%!d(string=<23>)`},
+	{"%d", I(23), `23`}, // Stringer applies only to string formats.
 
 	// go syntax
 	{"%#v", A{1, 2, "a", []int{1, 2}}, `fmt_test.A{i:1, j:0x2, s:"a", x:[]int{1, 2}}`},
@@ -430,6 +431,10 @@ var fmttests = []struct {
 	{"%p", make([]int, 1), "0xPTR"},
 	{"%p", 27, "%!p(int=27)"}, // not a pointer at all
 
+	// %d on Stringer should give integer if possible
+	{"%s", time.Time{}.Month(), "January"},
+	{"%d", time.Time{}.Month(), "1"},
+
 	// erroneous things
 	{"%s %", "hello", "hello %!(NOVERB)"},
 	{"%s %.2", "hello", "hello %!(NOVERB)"},
@@ -772,9 +777,9 @@ var panictests = []struct {
 	out string
 }{
 	// String
-	{"%d", (*Panic)(nil), "<nil>"}, // nil pointer special case
-	{"%d", Panic{io.ErrUnexpectedEOF}, "%d(PANIC=unexpected EOF)"},
-	{"%d", Panic{3}, "%d(PANIC=3)"},
+	{"%s", (*Panic)(nil), "<nil>"}, // nil pointer special case
+	{"%s", Panic{io.ErrUnexpectedEOF}, "%s(PANIC=unexpected EOF)"},
+	{"%s", Panic{3}, "%s(PANIC=3)"},
 	// GoString
 	{"%#v", (*Panic)(nil), "<nil>"}, // nil pointer special case
 	{"%#v", Panic{io.ErrUnexpectedEOF}, "%v(PANIC=unexpected EOF)"},
diff --git a/src/pkg/fmt/print.go b/src/pkg/fmt/print.go
index e5ca117240..8b15a82e77 100644
--- a/src/pkg/fmt/print.go
+++ b/src/pkg/fmt/print.go
@@ -631,24 +631,30 @@ func (p *pp) handleMethods(verb rune, plus, goSyntax bool, depth int) (wasString
 			return
 		}
 	} else {
-		// Is it an error or Stringer?
-		// The duplication in the bodies is necessary:
-		// setting wasString and handled and deferring catchPanic
-		// must happen before calling the method.
-		switch v := p.field.(type) {
-		case error:
-			wasString = false
-			handled = true
-			defer p.catchPanic(p.field, verb)
-			p.printField(v.Error(), verb, plus, false, depth)
-			return
-
-		case Stringer:
-			wasString = false
-			handled = true
-			defer p.catchPanic(p.field, verb)
-			p.printField(v.String(), verb, plus, false, depth)
-			return
+		// If a string is acceptable according to the format, see if
+		// the value satisfies one of the string-valued interfaces.
+		// Println etc. set verb to %v, which is "stringable".
+		switch verb {
+		case 'v', 's', 'x', 'X', 'q':
+			// Is it an error or Stringer?
+			// The duplication in the bodies is necessary:
+			// setting wasString and handled, and deferring catchPanic,
+			// must happen before calling the method.
+			switch v := p.field.(type) {
+			case error:
+				wasString = false
+				handled = true
+				defer p.catchPanic(p.field, verb)
+				p.printField(v.Error(), verb, plus, false, depth)
+				return
+
+			case Stringer:
+				wasString = false
+				handled = true
+				defer p.catchPanic(p.field, verb)
+				p.printField(v.String(), verb, plus, false, depth)
+				return
+			}
 		}
 	}
 	handled = false