mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fmt: make the %#v verb a special flag 

The %#v verb is special: it says all values below need to print as %#v.
However, for some situations the # flag has other meanings and this
causes some issues, particularly in how Formatters work. Since %#v
dominates all formatting, translate it into actual state of the formatter
and decouple it from the # flag itself within the calculations (although
it must be restored when methods are doing the work.)
The result is cleaner code and correct handling of # for Formatters.
TODO: Apply the same thinking to the + flag in a followup CL.

Also, the wasString return value in handleMethods is always false,
so eliminate it.

Update #8835

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/146650043
This commit is contained in:
Rob Pike 2014-10-02 14:16:58 -07:00
parent 28ddfb090c
commit 7c8e057ad3
3 changed files with 196 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

95
src/fmt/fmt_test.go
View File

@ -1144,10 +1144,10 @@ var panictests = []struct {
}
func TestPanics(t *testing.T) {
for _, tt := range panictests {
for i, tt := range panictests {
s := Sprintf(tt.fmt, tt.in)
if s != tt.out {
t.Errorf("%q: got %q expected %q", tt.fmt, s, tt.out)
t.Errorf("%d: %q: got %q expected %q", i, tt.fmt, s, tt.out)
}
}
}
@ -1207,3 +1207,94 @@ func TestNilDoesNotBecomeTyped(t *testing.T) {
t.Errorf("expected:\n\t%q\ngot:\n\t%q", expect, got)
}
}
// Formatters did not get delivered flags correctly in all cases. Issue 8835.
type fp struct{}
func (fp) Format(f State, c rune) {
s := "%"
for i := 0; i < 128; i++ {
if f.Flag(i) {
s += string(i)
}
}
if w, ok := f.Width(); ok {
s += Sprintf("%d", w)
}
if p, ok := f.Precision(); ok {
s += Sprintf(".%d", p)
}
s += string(c)
io.WriteString(f, "["+s+"]")
}
var formatterFlagTests = []struct {
in string
val interface{}
out string
}{
// scalar values with the (unused by fmt) 'a' verb.
{"%a", fp{}, "[%a]"},
{"%-a", fp{}, "[%-a]"},
{"%+a", fp{}, "[%+a]"},
{"%#a", fp{}, "[%#a]"},
{"% a", fp{}, "[% a]"},
{"%0a", fp{}, "[%0a]"},
{"%1.2a", fp{}, "[%1.2a]"},
{"%-1.2a", fp{}, "[%-1.2a]"},
{"%+1.2a", fp{}, "[%+1.2a]"},
{"%-+1.2a", fp{}, "[%+-1.2a]"},
{"%-+1.2abc", fp{}, "[%+-1.2a]bc"},
{"%-1.2abc", fp{}, "[%-1.2a]bc"},
// composite values with the 'a' verb
{"%a", [1]fp{}, "[[%a]]"},
{"%-a", [1]fp{}, "[[%-a]]"},
{"%+a", [1]fp{}, "[[%+a]]"},
{"%#a", [1]fp{}, "[[%#a]]"},
{"% a", [1]fp{}, "[[% a]]"},
{"%0a", [1]fp{}, "[[%0a]]"},
{"%1.2a", [1]fp{}, "[[%1.2a]]"},
{"%-1.2a", [1]fp{}, "[[%-1.2a]]"},
{"%+1.2a", [1]fp{}, "[[%+1.2a]]"},
{"%-+1.2a", [1]fp{}, "[[%+-1.2a]]"},
{"%-+1.2abc", [1]fp{}, "[[%+-1.2a]]bc"},
{"%-1.2abc", [1]fp{}, "[[%-1.2a]]bc"},
// simple values with the 'v' verb
{"%v", fp{}, "[%v]"},
{"%-v", fp{}, "[%-v]"},
{"%+v", fp{}, "[%+v]"},
{"%#v", fp{}, "[%#v]"},
{"% v", fp{}, "[% v]"},
{"%0v", fp{}, "[%0v]"},
{"%1.2v", fp{}, "[%1.2v]"},
{"%-1.2v", fp{}, "[%-1.2v]"},
{"%+1.2v", fp{}, "[%+1.2v]"},
{"%-+1.2v", fp{}, "[%+-1.2v]"},
{"%-+1.2vbc", fp{}, "[%+-1.2v]bc"},
{"%-1.2vbc", fp{}, "[%-1.2v]bc"},
// composite values with the 'v' verb. Some are still broken.
{"%v", [1]fp{}, "[[%v]]"},
{"%-v", [1]fp{}, "[[%-v]]"},
//{"%+v", [1]fp{}, "[[%+v]]"},
{"%#v", [1]fp{}, "[1]fmt_test.fp{[%#v]}"},
{"% v", [1]fp{}, "[[% v]]"},
{"%0v", [1]fp{}, "[[%0v]]"},
{"%1.2v", [1]fp{}, "[[%1.2v]]"},
{"%-1.2v", [1]fp{}, "[[%-1.2v]]"},
//{"%+1.2v", [1]fp{}, "[[%+1.2v]]"},
//{"%-+1.2v", [1]fp{}, "[[%+-1.2v]]"},
//{"%-+1.2vbc", [1]fp{}, "[[%+-1.2v]]bc"},
{"%-1.2vbc", [1]fp{}, "[[%-1.2v]]bc"},
}
func TestFormatterFlags(t *testing.T) {
for _, tt := range formatterFlagTests {
s := Sprintf(tt.in, tt.val)
if s != tt.out {
t.Errorf("Sprintf(%q, %T) = %q, want %q", tt.in, tt.val, s, tt.out)
}
}
}

12
src/fmt/format.go
View File

@ -49,9 +49,14 @@ type fmt struct {
plus bool
sharp bool
space bool
unicode bool
uniQuote bool // Use 'x'= prefix for %U if printable.
zero bool
// For the format %#v, we set this flag and
// clear the plus flag, since it is in effect
// a different, flagless format set at the top level.
// TODO: plusV could use this too.
sharpV bool
unicode bool
uniQuote bool // Use 'x'= prefix for %U if printable.
zero bool
}
func (f *fmt) clearflags() {
@ -63,6 +68,7 @@ func (f *fmt) clearflags() {
f.plus = false
f.sharp = false
f.space = false
f.sharpV = false
f.unicode = false
f.uniQuote = false
f.zero = false

173
src/fmt/print.go
View File

@ -317,11 +317,11 @@ func (p *pp) badVerb(verb rune) {
case p.arg != nil:
p.buf.WriteString(reflect.TypeOf(p.arg).String())
p.add('=')
p.printArg(p.arg, 'v', false, false, 0)
p.printArg(p.arg, 'v', false, 0)
case p.value.IsValid():
p.buf.WriteString(p.value.Type().String())
p.add('=')
p.printValue(p.value, 'v', false, false, 0)
p.printValue(p.value, 'v', false, 0)
default:
p.buf.Write(nilAngleBytes)
}
@ -406,7 +406,7 @@ func (p *pp) fmtUnicode(v int64) {
p.fmt.sharp = sharp
}
func (p *pp) fmtUint64(v uint64, verb rune, goSyntax bool) {
func (p *pp) fmtUint64(v uint64, verb rune) {
switch verb {
case 'b':
p.fmt.integer(int64(v), 2, unsigned, ldigits)
@ -415,7 +415,7 @@ func (p *pp) fmtUint64(v uint64, verb rune, goSyntax bool) {
case 'd':
p.fmt.integer(int64(v), 10, unsigned, ldigits)
case 'v':
if goSyntax {
if p.fmt.sharpV {
p.fmt0x64(v, true)
} else {
p.fmt.integer(int64(v), 10, unsigned, ldigits)
@ -499,10 +499,10 @@ func (p *pp) fmtComplex128(v complex128, verb rune) {
}
}
func (p *pp) fmtString(v string, verb rune, goSyntax bool) {
func (p *pp) fmtString(v string, verb rune) {
switch verb {
case 'v':
if goSyntax {
if p.fmt.sharpV {
p.fmt.fmt_q(v)
} else {
p.fmt.fmt_s(v)
@ -520,9 +520,9 @@ func (p *pp) fmtString(v string, verb rune, goSyntax bool) {
}
}
func (p *pp) fmtBytes(v []byte, verb rune, goSyntax bool, typ reflect.Type, depth int) {
func (p *pp) fmtBytes(v []byte, verb rune, typ reflect.Type, depth int) {
if verb == 'v' || verb == 'd' {
if goSyntax {
if p.fmt.sharpV {
if v == nil {
if typ == nil {
p.buf.WriteString("[]byte(nil)")
@ -543,15 +543,15 @@ func (p *pp) fmtBytes(v []byte, verb rune, goSyntax bool, typ reflect.Type, dept
}
for i, c := range v {
if i > 0 {
if goSyntax {
if p.fmt.sharpV {
p.buf.Write(commaSpaceBytes)
} else {
p.buf.WriteByte(' ')
}
}
p.printArg(c, 'v', p.fmt.plus, goSyntax, depth+1)
p.printArg(c, 'v', p.fmt.plus, depth+1)
}
if goSyntax {
if p.fmt.sharpV {
p.buf.WriteByte('}')
} else {
p.buf.WriteByte(']')
@ -572,7 +572,7 @@ func (p *pp) fmtBytes(v []byte, verb rune, goSyntax bool, typ reflect.Type, dept
}
}
func (p *pp) fmtPointer(value reflect.Value, verb rune, goSyntax bool) {
func (p *pp) fmtPointer(value reflect.Value, verb rune) {
use0x64 := true
switch verb {
case 'p', 'v':
@ -594,7 +594,7 @@ func (p *pp) fmtPointer(value reflect.Value, verb rune, goSyntax bool) {
return
}
if goSyntax {
if p.fmt.sharpV {
p.add('(')
p.buf.WriteString(value.Type().String())
p.add(')')
@ -611,7 +611,7 @@ func (p *pp) fmtPointer(value reflect.Value, verb rune, goSyntax bool) {
if use0x64 {
p.fmt0x64(uint64(u), !p.fmt.sharp)
} else {
p.fmtUint64(uint64(u), verb, false)
p.fmtUint64(uint64(u), verb)
}
}
}
@ -636,24 +636,44 @@ func (p *pp) catchPanic(arg interface{}, verb rune) {
// Nested panics; the recursion in printArg cannot succeed.
panic(err)
}
p.fmt.clearflags() // We are done, and for this output we want default behavior.
p.buf.Write(percentBangBytes)
p.add(verb)
p.buf.Write(panicBytes)
p.panicking = true
p.printArg(err, 'v', false, false, 0)
p.printArg(err, 'v', false, 0)
p.panicking = false
p.buf.WriteByte(')')
}
}
func (p *pp) handleMethods(verb rune, plus, goSyntax bool, depth int) (wasString, handled bool) {
// clearSpecialFlags pushes %#v back into the regular flags and returns their old state.
func (p *pp) clearSpecialFlags() bool {
ret := p.fmt.sharpV
if ret {
p.fmt.sharp = true
p.fmt.sharpV = false
}
return ret
}
// restoreSpecialFlags, whose argument should be a call to clearSpecialFlags,
// restores the setting of the sharpV flag.
func (p *pp) restoreSpecialFlags(sharpV bool) {
if sharpV {
p.fmt.sharp = false
p.fmt.sharpV = true
}
}
func (p *pp) handleMethods(verb rune, plus bool, depth int) (handled bool) {
if p.erroring {
return
}
// Is it a Formatter?
if formatter, ok := p.arg.(Formatter); ok {
handled = true
wasString = false
defer p.restoreSpecialFlags(p.clearSpecialFlags())
defer p.catchPanic(p.arg, verb)
formatter.Format(p, verb)
return
@ -664,14 +684,13 @@ func (p *pp) handleMethods(verb rune, plus, goSyntax bool, depth int) (wasString
}
// If we're doing Go syntax and the argument knows how to supply it, take care of it now.
if goSyntax {
p.fmt.sharp = false
if p.fmt.sharpV {
if stringer, ok := p.arg.(GoStringer); ok {
wasString = false
handled = true
defer p.restoreSpecialFlags(p.clearSpecialFlags())
defer p.catchPanic(p.arg, verb)
// Print the result of GoString unadorned.
p.fmtString(stringer.GoString(), 's', false)
p.fmtString(stringer.GoString(), 's')
return
}
} else {
@ -682,30 +701,27 @@ func (p *pp) handleMethods(verb rune, plus, goSyntax bool, depth int) (wasString
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,
// setting handled and deferring catchPanic
// must happen before calling the method.
switch v := p.arg.(type) {
case error:
wasString = false
handled = true
defer p.catchPanic(p.arg, verb)
p.printArg(v.Error(), verb, plus, false, depth)
p.printArg(v.Error(), verb, plus, depth)
return
case Stringer:
wasString = false
handled = true
defer p.catchPanic(p.arg, verb)
p.printArg(v.String(), verb, plus, false, depth)
p.printArg(v.String(), verb, plus, depth)
return
}
}
}
handled = false
return
return false
}
func (p *pp) printArg(arg interface{}, verb rune, plus, goSyntax bool, depth int) (wasString bool) {
func (p *pp) printArg(arg interface{}, verb rune, plus bool, depth int) (wasString bool) {
p.arg = arg
p.value = reflect.Value{}
@ -722,10 +738,10 @@ func (p *pp) printArg(arg interface{}, verb rune, plus, goSyntax bool, depth int
// %T (the value's type) and %p (its address) are special; we always do them first.
switch verb {
case 'T':
p.printArg(reflect.TypeOf(arg).String(), 's', false, false, 0)
p.printArg(reflect.TypeOf(arg).String(), 's', false, 0)
return false
case 'p':
p.fmtPointer(reflect.ValueOf(arg), verb, goSyntax)
p.fmtPointer(reflect.ValueOf(arg), verb)
return false
}
@ -734,13 +750,9 @@ func (p *pp) printArg(arg interface{}, verb rune, plus, goSyntax bool, depth int
// We could call handleMethods here and avoid this work, but
// handleMethods is expensive enough to be worth delaying.
oldPlus := p.fmt.plus
oldSharp := p.fmt.sharp
if plus {
p.fmt.plus = false
}
if goSyntax {
p.fmt.sharp = false
}
// Some types can be done without reflection.
switch f := arg.(type) {
@ -765,40 +777,39 @@ func (p *pp) printArg(arg interface{}, verb rune, plus, goSyntax bool, depth int
case int64:
p.fmtInt64(f, verb)
case uint:
p.fmtUint64(uint64(f), verb, goSyntax)
p.fmtUint64(uint64(f), verb)
case uint8:
p.fmtUint64(uint64(f), verb, goSyntax)
p.fmtUint64(uint64(f), verb)
case uint16:
p.fmtUint64(uint64(f), verb, goSyntax)
p.fmtUint64(uint64(f), verb)
case uint32:
p.fmtUint64(uint64(f), verb, goSyntax)
p.fmtUint64(uint64(f), verb)
case uint64:
p.fmtUint64(f, verb, goSyntax)
p.fmtUint64(f, verb)
case uintptr:
p.fmtUint64(uint64(f), verb, goSyntax)
p.fmtUint64(uint64(f), verb)
case string:
p.fmtString(f, verb, goSyntax)
p.fmtString(f, verb)
wasString = verb == 's' || verb == 'v'
case []byte:
p.fmtBytes(f, verb, goSyntax, nil, depth)
p.fmtBytes(f, verb, nil, depth)
wasString = verb == 's'
default:
// Restore flags in case handleMethods finds a Formatter.
p.fmt.plus = oldPlus
p.fmt.sharp = oldSharp
// If the type is not simple, it might have methods.
if isString, handled := p.handleMethods(verb, plus, goSyntax, depth); handled {
return isString
if handled := p.handleMethods(verb, plus, depth); handled {
return false
}
// Need to use reflection
return p.printReflectValue(reflect.ValueOf(arg), verb, plus, goSyntax, depth)
return p.printReflectValue(reflect.ValueOf(arg), verb, plus, depth)
}
p.arg = nil
return
}
// printValue is like printArg but starts with a reflect value, not an interface{} value.
func (p *pp) printValue(value reflect.Value, verb rune, plus, goSyntax bool, depth int) (wasString bool) {
func (p *pp) printValue(value reflect.Value, verb rune, plus bool, depth int) (wasString bool) {
if !value.IsValid() {
if verb == 'T' || verb == 'v' {
p.buf.Write(nilAngleBytes)
@ -812,10 +823,10 @@ func (p *pp) printValue(value reflect.Value, verb rune, plus, goSyntax bool, dep
// %T (the value's type) and %p (its address) are special; we always do them first.
switch verb {
case 'T':
p.printArg(value.Type().String(), 's', false, false, 0)
p.printArg(value.Type().String(), 's', false, 0)
return false
case 'p':
p.fmtPointer(value, verb, goSyntax)
p.fmtPointer(value, verb)
return false
}
@ -825,18 +836,18 @@ func (p *pp) printValue(value reflect.Value, verb rune, plus, goSyntax bool, dep
if value.CanInterface() {
p.arg = value.Interface()
}
if isString, handled := p.handleMethods(verb, plus, goSyntax, depth); handled {
return isString
if handled := p.handleMethods(verb, plus, depth); handled {
return false
}
return p.printReflectValue(value, verb, plus, goSyntax, depth)
return p.printReflectValue(value, verb, plus, depth)
}
var byteType = reflect.TypeOf(byte(0))
// printReflectValue is the fallback for both printArg and printValue.
// It uses reflect to print the value.
func (p *pp) printReflectValue(value reflect.Value, verb rune, plus, goSyntax bool, depth int) (wasString bool) {
func (p *pp) printReflectValue(value reflect.Value, verb rune, plus bool, depth int) (wasString bool) {
oldValue := p.value
p.value = value
BigSwitch:
@ -846,7 +857,7 @@ BigSwitch:
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
p.fmtInt64(f.Int(), verb)
case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
p.fmtUint64(f.Uint(), verb, goSyntax)
p.fmtUint64(f.Uint(), verb)
case reflect.Float32, reflect.Float64:
if f.Type().Size() == 4 {
p.fmtFloat32(float32(f.Float()), verb)
@ -860,9 +871,9 @@ BigSwitch:
p.fmtComplex128(f.Complex(), verb)
}
case reflect.String:
p.fmtString(f.String(), verb, goSyntax)
p.fmtString(f.String(), verb)
case reflect.Map:
if goSyntax {
if p.fmt.sharpV {
p.buf.WriteString(f.Type().String())
if f.IsNil() {
p.buf.WriteString("(nil)")
@ -875,23 +886,23 @@ BigSwitch:
keys := f.MapKeys()
for i, key := range keys {
if i > 0 {
if goSyntax {
if p.fmt.sharpV {
p.buf.Write(commaSpaceBytes)
} else {
p.buf.WriteByte(' ')
}
}
p.printValue(key, verb, plus, goSyntax, depth+1)
p.printValue(key, verb, plus, depth+1)
p.buf.WriteByte(':')
p.printValue(f.MapIndex(key), verb, plus, goSyntax, depth+1)
p.printValue(f.MapIndex(key), verb, plus, depth+1)
}
if goSyntax {
if p.fmt.sharpV {
p.buf.WriteByte('}')
} else {
p.buf.WriteByte(']')
}
case reflect.Struct:
if goSyntax {
if p.fmt.sharpV {
p.buf.WriteString(value.Type().String())
}
p.add('{')
@ -899,32 +910,32 @@ BigSwitch:
t := v.Type()
for i := 0; i < v.NumField(); i++ {
if i > 0 {
if goSyntax {
if p.fmt.sharpV {
p.buf.Write(commaSpaceBytes)
} else {
p.buf.WriteByte(' ')
}
}
if plus || goSyntax {
if plus || p.fmt.sharpV {
if f := t.Field(i); f.Name != "" {
p.buf.WriteString(f.Name)
p.buf.WriteByte(':')
}
}
p.printValue(getField(v, i), verb, plus, goSyntax, depth+1)
p.printValue(getField(v, i), verb, plus, depth+1)
}
p.buf.WriteByte('}')
case reflect.Interface:
value := f.Elem()
if !value.IsValid() {
if goSyntax {
if p.fmt.sharpV {
p.buf.WriteString(f.Type().String())
p.buf.Write(nilParenBytes)
} else {
p.buf.Write(nilAngleBytes)
}
} else {
wasString = p.printValue(value, verb, plus, goSyntax, depth+1)
wasString = p.printValue(value, verb, plus, depth+1)
}
case reflect.Array, reflect.Slice:
// Byte slices are special:
@ -947,11 +958,11 @@ BigSwitch:
bytes[i] = byte(f.Index(i).Uint())
}
}
p.fmtBytes(bytes, verb, goSyntax, typ, depth)
p.fmtBytes(bytes, verb, typ, depth)
wasString = verb == 's'
break
}
if goSyntax {
if p.fmt.sharpV {
p.buf.WriteString(value.Type().String())
if f.Kind() == reflect.Slice && f.IsNil() {
p.buf.WriteString("(nil)")
@ -963,15 +974,15 @@ BigSwitch:
}
for i := 0; i < f.Len(); i++ {
if i > 0 {
if goSyntax {
if p.fmt.sharpV {
p.buf.Write(commaSpaceBytes)
} else {
p.buf.WriteByte(' ')
}
}
p.printValue(f.Index(i), verb, plus, goSyntax, depth+1)
p.printValue(f.Index(i), verb, plus, depth+1)
}
if goSyntax {
if p.fmt.sharpV {
p.buf.WriteByte('}')
} else {
p.buf.WriteByte(']')
@ -984,17 +995,17 @@ BigSwitch:
switch a := f.Elem(); a.Kind() {
case reflect.Array, reflect.Slice:
p.buf.WriteByte('&')
p.printValue(a, verb, plus, goSyntax, depth+1)
p.printValue(a, verb, plus, depth+1)
break BigSwitch
case reflect.Struct:
p.buf.WriteByte('&')
p.printValue(a, verb, plus, goSyntax, depth+1)
p.printValue(a, verb, plus, depth+1)
break BigSwitch
}
}
fallthrough
case reflect.Chan, reflect.Func, reflect.UnsafePointer:
p.fmtPointer(value, verb, goSyntax)
p.fmtPointer(value, verb)
default:
p.unknownType(f)
}
@ -1160,9 +1171,13 @@ func (p *pp) doPrintf(format string, a []interface{}) {
arg := a[argNum]
argNum++
goSyntax := c == 'v' && p.fmt.sharp
if c == 'v' && p.fmt.sharp {
// Go syntax. Set the flag in the fmt and clear the sharp flag.
p.fmt.sharp = false
p.fmt.sharpV = true
}
plus := c == 'v' && p.fmt.plus
p.printArg(arg, c, plus, goSyntax, 0)
p.printArg(arg, c, plus, 0)
}
// Check for extra arguments unless the call accessed the arguments
@ -1176,7 +1191,7 @@ func (p *pp) doPrintf(format string, a []interface{}) {
p.buf.WriteString(reflect.TypeOf(arg).String())
p.buf.WriteByte('=')
}
p.printArg(arg, 'v', false, false, 0)
p.printArg(arg, 'v', false, 0)
if argNum+1 < len(a) {
p.buf.Write(commaSpaceBytes)
}
@ -1197,7 +1212,7 @@ func (p *pp) doPrint(a []interface{}, addspace, addnewline bool) {
p.buf.WriteByte(' ')
}
}
prevString = p.printArg(arg, 'v', false, false, 0)
prevString = p.printArg(arg, 'v', false, 0)
}
if addnewline {
p.buf.WriteByte('\n')