Skip to content

Commit dbe2270

Browse files
committed
attributes: format invalid argument for '%p' to string in style <type=value>, not %!verb(type=value)
#6574 (comment)
1 parent 1114f39 commit dbe2270

File tree

2 files changed

+60
-10
lines changed

2 files changed

+60
-10
lines changed

attributes/attributes.go

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ func (a *Attributes) String() string {
122122
return sb.String()
123123
}
124124

125+
const nilAngleString = "<nil>"
126+
125127
func str(x any) (s string) {
126128
defer func() {
127129
if r := recover(); r != nil {
@@ -131,7 +133,7 @@ func str(x any) (s string) {
131133
//
132134
// Adapted from the code in fmt/print.go.
133135
if v := reflect.ValueOf(x); v.Kind() == reflect.Pointer && v.IsNil() {
134-
s = "<nil>"
136+
s = nilAngleString
135137
return
136138
}
137139

@@ -140,13 +142,58 @@ func str(x any) (s string) {
140142
}
141143
}()
142144
if x == nil { // NOTE: typed nils will not be caught by this check
143-
return "<nil>"
145+
return nilAngleString
144146
} else if v, ok := x.(fmt.Stringer); ok {
145147
return v.String()
146148
} else if v, ok := x.(string); ok {
147149
return v
148150
}
149-
return fmt.Sprintf("<%p>", x)
151+
value := reflect.ValueOf(x)
152+
switch value.Kind() {
153+
case reflect.Chan, reflect.Func, reflect.Map, reflect.Pointer, reflect.Slice, reflect.UnsafePointer:
154+
return fmt.Sprintf("<%p>", x)
155+
default:
156+
// This will call badVerb to print as "<%p>", but without leading "%!(" and tailing ")"
157+
return badVerb(x, value)
158+
}
159+
}
160+
161+
// badVerb is like fmt.Sprintf("%p", arg), but with
162+
// leading "%!verb(" replaced by "<" and tailing ")" replaced by ">".
163+
// If an invalid argument is given for a '%p', such as providing
164+
// an int to %p, the generated string will contain a
165+
// description of the problem, as in these examples:
166+
//
167+
// # our style
168+
//
169+
// Wrong type or unknown verb: <type=value>
170+
// Printf("%p", 1): <int=1>
171+
//
172+
// # fmt style as `fmt.Sprintf("%p", arg)`
173+
//
174+
// Wrong type or unknown verb: %!verb(type=value)
175+
// Printf("%p", 1): %!d(int=1)
176+
//
177+
// Adapted from the code in fmt/print.go.
178+
func badVerb(arg any, value reflect.Value) string {
179+
var buf strings.Builder
180+
switch {
181+
case arg != nil:
182+
buf.WriteByte('<')
183+
buf.WriteString(reflect.TypeOf(arg).String())
184+
buf.WriteByte('=')
185+
_, _ = fmt.Fprintf(&buf, "%v", arg)
186+
buf.WriteByte('>')
187+
case value.IsValid():
188+
buf.WriteByte('<')
189+
buf.WriteString(value.Type().String())
190+
buf.WriteByte('=')
191+
_, _ = fmt.Fprintf(&buf, "%v", 0)
192+
buf.WriteByte('>')
193+
default:
194+
buf.WriteString(nilAngleString)
195+
}
196+
return buf.String()
150197
}
151198

152199
// MarshalJSON helps implement the json.Marshaler interface, thereby rendering

attributes/attributes_test.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,21 +75,24 @@ func ExampleAttributes_String() {
7575
a5 := attributes.New(key{}, 1)
7676
a6 := attributes.New(key{}, stringerVal{s: "two"})
7777
a7 := attributes.New(key{}, stringVal{s: "two"})
78+
a8 := attributes.New(1, true)
7879
fmt.Println("a1:", a1.String())
7980
fmt.Println("a2:", a2.String())
8081
fmt.Println("a3:", a3.String())
8182
fmt.Println("a4:", a4.String())
8283
fmt.Println("a5:", a5.String())
8384
fmt.Println("a6:", a6.String())
8485
fmt.Println("a7:", a7.String())
86+
fmt.Println("a8:", a8.String())
8587
// Output:
86-
// a1: {"<%!p(attributes_test.key={})>": "<nil>" }
87-
// a2: {"<%!p(attributes_test.key={})>": "<nil>" }
88-
// a3: {"<%!p(attributes_test.key={})>": "<0x0>" }
89-
// a4: {"<%!p(attributes_test.key={})>": "<nil>" }
90-
// a5: {"<%!p(attributes_test.key={})>": "<%!p(int=1)>" }
91-
// a6: {"<%!p(attributes_test.key={})>": "two" }
92-
// a7: {"<%!p(attributes_test.key={})>": "<%!p(attributes_test.stringVal={two})>" }
88+
// a1: {"<attributes_test.key={}>": "<nil>" }
89+
// a2: {"<attributes_test.key={}>": "<nil>" }
90+
// a3: {"<attributes_test.key={}>": "<0x0>" }
91+
// a4: {"<attributes_test.key={}>": "<nil>" }
92+
// a5: {"<attributes_test.key={}>": "<int=1>" }
93+
// a6: {"<attributes_test.key={}>": "two" }
94+
// a7: {"<attributes_test.key={}>": "<attributes_test.stringVal={two}>" }
95+
// a8: {"<int=1>": "<bool=true>" }
9396
}
9497

9598
// Test that two attributes with the same content are Equal.

0 commit comments

Comments
 (0)