From 2ed57a8cd86cec36b8370fb16d450e5a29a9375f Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Mon, 5 Dec 2011 16:45:51 -0800 Subject: [PATCH] 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 --- src/pkg/fmt/doc.go | 14 +++++++++----- src/pkg/fmt/fmt_test.go | 13 +++++++++---- src/pkg/fmt/print.go | 40 +++++++++++++++++++++++----------------- 3 files changed, 41 insertions(+), 26 deletions(-) diff --git a/src/pkg/fmt/doc.go b/src/pkg/fmt/doc.go index 6713f0a16ed..747865c6f96 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 f937a454ed3..d42a8fe1f29 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 pointer special case - {"%d", Panic{io.ErrUnexpectedEOF}, "%d(PANIC=unexpected EOF)"}, - {"%d", Panic{3}, "%d(PANIC=3)"}, + {"%s", (*Panic)(nil), ""}, // nil pointer special case + {"%s", Panic{io.ErrUnexpectedEOF}, "%s(PANIC=unexpected EOF)"}, + {"%s", Panic{3}, "%s(PANIC=3)"}, // GoString {"%#v", (*Panic)(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 e5ca1172405..8b15a82e773 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 + // 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 + case Stringer: + wasString = false + handled = true + defer p.catchPanic(p.field, verb) + p.printField(v.String(), verb, plus, false, depth) + return + } } } handled = false