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

diff --git a/src/pkg/fmt/doc.go b/src/pkg/fmt/doc.go
index 6713f0a16ed27c8e0495263d1797230068ff36fa..747865c6f963d170fa1af2be5c9af835d8a1dd8a 100644 (file)
--- 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 f937a454ed377b5c8c4f355a67526365ded64563..d42a8fe1f2912846cec9068a2a0f583278fd12b4 100644 (file)
--- 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 e5ca1172405c1799efc7474144dc7f559e45477e..8b15a82e773b69b3d9e97351bac9990ba712569d 100644 (file)
--- 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