Skip to content

Commit b7136a9

Browse files
dsnetdsnet
committed
Add error to Decoder.PeekKind and add Decoder.More
Add an error return variable to PeekKind. This avoids accidentally ignoring an error condition during peeking. Add a Decoder.More method to compensate for loss of ergonomics. The More method can be used to quickly iterate through all elements or members of a JSON array or object. This mirrors the existing jsonv1.Decoder.More method. Switch existing usages of PeekKind to use More. Performance: name old time/op new time/op delta Testdata/CanadaGeometry/Unmarshal/Concrete 1.20ms ± 1% 1.20ms ± 1% ~ (p=0.310 n=5+5) Testdata/CanadaGeometry/Unmarshal/Interface 1.64ms ± 0% 1.66ms ± 1% +1.18% (p=0.008 n=5+5) Testdata/CitmCatalog/Unmarshal/Concrete 2.04ms ± 1% 2.13ms ± 0% +4.37% (p=0.008 n=5+5) Testdata/CitmCatalog/Unmarshal/Interface 3.88ms ± 1% 4.02ms ± 1% +3.63% (p=0.008 n=5+5) Testdata/GolangSource/Unmarshal/Concrete 5.51ms ± 0% 5.70ms ± 0% +3.54% (p=0.008 n=5+5) Testdata/GolangSource/Unmarshal/Interface 9.09ms ± 1% 9.60ms ± 1% +5.59% (p=0.008 n=5+5) Testdata/SyntheaFhir/Unmarshal/Concrete 3.60ms ± 1% 3.56ms ± 0% -1.34% (p=0.008 n=5+5) Testdata/SyntheaFhir/Unmarshal/Interface 5.33ms ± 1% 5.40ms ± 1% +1.26% (p=0.016 n=5+5) Testdata/TwitterStatus/Unmarshal/Concrete 1.16ms ± 1% 1.13ms ± 0% -2.94% (p=0.008 n=5+5) Testdata/TwitterStatus/Unmarshal/Interface 2.02ms ± 1% 2.02ms ± 1% ~ (p=0.421 n=5+5)
1 parent 517a081 commit b7136a9

16 files changed

+141
-103
lines changed

‎arshal_any.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,11 @@ func marshalValueAny(enc *jsontext.Encoder, val any, mo *jsonopts.Struct) error
5454
// for any possible nested value.
5555
// Duplicate names must be rejected since this does not implement merging.
5656
func unmarshalValueAny(dec *jsontext.Decoder, uo *jsonopts.Struct) (any, error) {
57-
switch k := dec.PeekKind(); k {
57+
k, err := dec.PeekKind()
58+
if err != nil {
59+
return nil, err
60+
}
61+
switch k {
5862
case '{':
5963
return unmarshalObjectAny(dec, uo)
6064
case '[':
@@ -180,7 +184,7 @@ func unmarshalObjectAny(dec *jsontext.Decoder, uo *jsonopts.Struct) (map[string]
180184
export.Decoder(dec).Tokens.Last.DisableNamespace()
181185
}
182186
var errUnmarshal error
183-
for dec.PeekKind() != '}' {
187+
for dec.More() {
184188
tok, err := dec.ReadToken()
185189
if err != nil {
186190
return obj, err
@@ -264,7 +268,7 @@ func unmarshalArrayAny(dec *jsontext.Decoder, uo *jsonopts.Struct) ([]any, error
264268
}
265269
arr := []any{}
266270
var errUnmarshal error
267-
for dec.PeekKind() != ']' {
271+
for dec.More() {
268272
val, err := unmarshalValueAny(dec, uo)
269273
arr = append(arr, val)
270274
if err != nil {

‎arshal_default.go

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ func makeBytesArshaler(t reflect.Type, fncs *arshaler) *arshaler {
363363
return newInvalidFormatError(dec, t, uo)
364364
}
365365
} else if uo.Flags.Get(jsonflags.FormatBytesWithLegacySemantics) &&
366-
(va.Kind() == reflect.Array || dec.PeekKind() == '[') {
366+
(va.Kind() == reflect.Array || xd.PeekKind() == '[') {
367367
return unmarshalArray(dec, va, uo)
368368
}
369369
var flags jsonwire.ValueFlags
@@ -926,7 +926,7 @@ func makeMapArshaler(t reflect.Type) *arshaler {
926926
}
927927

928928
var errUnmarshal error
929-
for dec.PeekKind() != '}' {
929+
for dec.More() {
930930
// Unmarshal the map entry key.
931931
k.SetZero()
932932
err := unmarshalKey(dec, k, uo)
@@ -1214,7 +1214,7 @@ func makeStructArshaler(t reflect.Type) *arshaler {
12141214
var seenIdxs uintSet
12151215
xd.Tokens.Last.DisableNamespace()
12161216
var errUnmarshal error
1217-
for dec.PeekKind() != '}' {
1217+
for dec.More() {
12181218
// Process the object member name.
12191219
var flags jsonwire.ValueFlags
12201220
val, err := xd.ReadValue(&flags)
@@ -1482,7 +1482,7 @@ func makeSliceArshaler(t reflect.Type) *arshaler {
14821482
}
14831483
var i int
14841484
var errUnmarshal error
1485-
for dec.PeekKind() != ']' {
1485+
for dec.More() {
14861486
if i == cap {
14871487
va.Value.Grow(1)
14881488
cap = va.Cap()
@@ -1578,7 +1578,7 @@ func makeArrayArshaler(t reflect.Type) *arshaler {
15781578
}
15791579
var i int
15801580
var errUnmarshal error
1581-
for dec.PeekKind() != ']' {
1581+
for dec.More() {
15821582
if i >= n {
15831583
if err := dec.SkipValue(); err != nil {
15841584
return err
@@ -1648,7 +1648,10 @@ func makePointerArshaler(t reflect.Type) *arshaler {
16481648
}
16491649
fncs.unmarshal = func(dec *jsontext.Decoder, va addressableValue, uo *jsonopts.Struct) error {
16501650
// NOTE: Struct.Format is forwarded to underlying unmarshal.
1651-
if dec.PeekKind() == 'n' {
1651+
switch k, err := dec.PeekKind(); {
1652+
case err != nil:
1653+
return err
1654+
case k == 'n':
16521655
if _, err := dec.ReadToken(); err != nil {
16531656
return err
16541657
}
@@ -1744,6 +1747,10 @@ func makeInterfaceArshaler(t reflect.Type) *arshaler {
17441747
if uo.Format != "" && uo.FormatDepth == xd.Tokens.Depth() {
17451748
return newInvalidFormatError(dec, t, uo)
17461749
}
1750+
k, err := dec.PeekKind()
1751+
if err != nil {
1752+
return err
1753+
}
17471754
if uo.Flags.Get(jsonflags.MergeWithLegacySemantics) && !va.IsNil() {
17481755
// Legacy merge behavior is difficult to explain.
17491756
// In general, it only merges for non-nil pointer kinds.
@@ -1752,7 +1759,7 @@ func makeInterfaceArshaler(t reflect.Type) *arshaler {
17521759
// (rather than setting the interface value itself to nil).
17531760
e := va.Elem()
17541761
if e.Kind() == reflect.Pointer && !e.IsNil() {
1755-
if dec.PeekKind() == 'n' && e.Elem().Kind() == reflect.Pointer {
1762+
if k == 'n' && e.Elem().Kind() == reflect.Pointer {
17561763
if _, err := dec.ReadToken(); err != nil {
17571764
return err
17581765
}
@@ -1763,7 +1770,7 @@ func makeInterfaceArshaler(t reflect.Type) *arshaler {
17631770
va.SetZero()
17641771
}
17651772
}
1766-
if dec.PeekKind() == 'n' {
1773+
if k == 'n' {
17671774
if _, err := dec.ReadToken(); err != nil {
17681775
return err
17691776
}
@@ -1789,7 +1796,10 @@ func makeInterfaceArshaler(t reflect.Type) *arshaler {
17891796
return err
17901797
}
17911798

1792-
k := dec.PeekKind()
1799+
k, err := dec.PeekKind()
1800+
if err != nil {
1801+
return err
1802+
}
17931803
if !isAnyType(t) {
17941804
return newUnmarshalErrorBeforeWithSkipping(dec, uo, t, errNilInterface)
17951805
}
@@ -1827,7 +1837,7 @@ func makeInterfaceArshaler(t reflect.Type) *arshaler {
18271837
if uo.Unmarshalers != nil {
18281838
unmarshal, _ = uo.Unmarshalers.(*Unmarshalers).lookup(unmarshal, v.Type())
18291839
}
1830-
err := unmarshal(dec, v, uo)
1840+
err = unmarshal(dec, v, uo)
18311841
va.Set(v.Value)
18321842
return err
18331843
}

‎arshal_inlined.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func marshalInlinedFallbackAll(enc *jsontext.Encoder, va addressableValue, mo *j
6969
if tok.Kind() != '{' {
7070
return newMarshalErrorBefore(enc, v.Type(), errRawInlinedNotObject)
7171
}
72-
for dec.PeekKind() != '}' {
72+
for dec.More() {
7373
// Parse the JSON object name.
7474
var flags jsonwire.ValueFlags
7575
val, err := xd.ReadValue(&flags)

‎arshal_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7138,7 +7138,7 @@ func TestUnmarshal(t *testing.T) {
71387138
name: jsontest.Name("Structs/Invalid/NestedErrUnexpectedEOF"),
71397139
inBuf: `{"Pointer":`,
71407140
inVal: addr(structAll{}),
7141-
want: addr(structAll{Pointer: new(structAll)}),
7141+
want: addr(structAll{}),
71427142
wantErr: &jsontext.SyntacticError{ByteOffset: len64(`{"Pointer":`), JSONPointer: "/Pointer", Err: io.ErrUnexpectedEOF},
71437143
}, {
71447144
name: jsontest.Name("Structs/Invalid/Conflicting"),

‎bench_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ func runSlowStreamingDecode(t testing.TB, typeName string, data []byte) {
563563
dec := jsontext.NewDecoder(iotest.OneByteReader(bytes.NewReader(data)))
564564
switch typeName {
565565
case "Token":
566-
for dec.PeekKind() > 0 {
566+
for dec.More() {
567567
if _, err := dec.ReadToken(); err != nil {
568568
t.Fatalf("Decoder.ReadToken error: %v", err)
569569
}

‎example_orderedobject_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,13 @@ func (obj *OrderedObject[V]) MarshalJSONTo(enc *jsontext.Encoder, opts json.Opti
5050

5151
// UnmarshalJSONFrom decodes a JSON object from dec into obj.
5252
func (obj *OrderedObject[V]) UnmarshalJSONFrom(dec *jsontext.Decoder, opts json.Options) error {
53-
if k := dec.PeekKind(); k != '{' {
54-
return fmt.Errorf("expected object start, but encountered %v", k)
55-
}
56-
if _, err := dec.ReadToken(); err != nil {
53+
switch tok, err := dec.ReadToken(); {
54+
case err != nil:
5755
return err
56+
case tok.Kind() != '{':
57+
return fmt.Errorf("expected object start, but encountered %v", tok.Kind())
5858
}
59-
for dec.PeekKind() != '}' {
59+
for dec.More() {
6060
*obj = append(*obj, ObjectMember[V]{})
6161
member := &(*obj)[len(*obj)-1]
6262
if err := json.UnmarshalDecode(dec, &member.Name, opts); err != nil {

‎example_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -607,7 +607,10 @@ func ExampleWithUnmarshalers_rawNumber() {
607607
json.UnmarshalFromFunc(func(dec *jsontext.Decoder, val *any, opts json.Options) error {
608608
// If the next value to be decoded is a JSON number,
609609
// then provide a concrete Go type to unmarshal into.
610-
if dec.PeekKind() == '0' {
610+
switch k, err := dec.PeekKind(); {
611+
case err != nil:
612+
return err
613+
case k == '0':
611614
*val = jsontext.Value(nil)
612615
}
613616
// Return SkipFunc to fallback on default unmarshal behavior.

‎inline_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ func TestInline(t *testing.T) {
4545
},
4646
"./jsontext": {
4747
"encoderState.NeedFlush": true,
48+
"Decoder.More": true,
4849
"Decoder.ReadToken": true, // thin wrapper over decoderState.ReadToken
4950
"Decoder.ReadValue": true, // thin wrapper over decoderState.ReadValue
5051
"Encoder.WriteToken": true, // thin wrapper over encoderState.WriteToken

‎jsontext/coder_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ func testCoderInterleaved(t *testing.T, where jsontest.CasePos, modeName string,
458458
tickTock := modeName == "TokenFirst"
459459
for {
460460
if modeName == "TokenDelims" {
461-
switch dec.PeekKind() {
461+
switch dec.s.PeekKind() {
462462
case '{', '}', '[', ']':
463463
tickTock = true // as token
464464
default:
@@ -482,7 +482,7 @@ func testCoderInterleaved(t *testing.T, where jsontest.CasePos, modeName string,
482482
// It is a syntactic error to call ReadValue
483483
// at the end of an object or array.
484484
// Retry as a ReadToken call.
485-
expectError := dec.PeekKind() == '}' || dec.PeekKind() == ']'
485+
expectError := dec.s.PeekKind() == '}' || dec.s.PeekKind() == ']'
486486
if expectError {
487487
if !errors.As(err, new(*SyntacticError)) {
488488
t.Fatalf("%s: Decoder.ReadToken error is %T, want %T", where, err, new(SyntacticError))
@@ -707,7 +707,7 @@ func TestCoderMaxDepth(t *testing.T) {
707707
checkReadToken(t, '{', nil)
708708
checkReadToken(t, '"', nil)
709709
}
710-
checkReadToken(t, 0, wantErr)
710+
checkReadToken(t, invalidKind, wantErr)
711711
})
712712
})
713713

‎jsontext/decode.go

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -285,10 +285,18 @@ func (d *decodeBuffer) PreviousTokenOrValue() []byte {
285285
}
286286

287287
// PeekKind retrieves the next token kind, but does not advance the read offset.
288-
// It returns 0 if there are no more tokens.
289-
func (d *Decoder) PeekKind() Kind {
290-
return d.s.PeekKind()
288+
func (d *Decoder) PeekKind() (k Kind, err error) {
289+
k = d.s.PeekKind()
290+
if err = d.s.peekErr; err != nil {
291+
d.s.peekPos, d.s.peekErr = 0, nil // clear the cached error
292+
}
293+
return k, err
291294
}
295+
296+
// PeekKind is like Decoder.PeekKind, but stores an out-of-band error,
297+
// which can retrieved by a call to PeekKind, ReadToken, or ReadValue.
298+
// An error-less return allows for ergonomic iteration with Decoder.More.
299+
// It returns 0 only for [io.EOF].
292300
func (d *decoderState) PeekKind() Kind {
293301
// Check whether we have a cached peek result.
294302
if d.peekPos > 0 {
@@ -307,6 +315,9 @@ func (d *decoderState) PeekKind() Kind {
307315
err = io.EOF // EOF possibly if no Tokens present after top-level value
308316
}
309317
d.peekPos, d.peekErr = -1, wrapSyntacticError(d, err, pos, 0)
318+
if err == io.EOF {
319+
return 0
320+
}
310321
return invalidKind
311322
}
312323
}
@@ -339,6 +350,13 @@ func (d *decoderState) PeekKind() Kind {
339350
return next
340351
}
341352

353+
// More reports whether there is another element in the
354+
// current array or object being parsed.
355+
func (d *Decoder) More() bool {
356+
k := d.s.PeekKind()
357+
return k > 0 && k != ']' && k != '}'
358+
}
359+
342360
// checkDelimBeforeIOError checks whether the delim is even valid
343361
// before returning an IO error, which occurs after the delim.
344362
func (d *decoderState) checkDelimBeforeIOError(delim byte, err error) error {

0 commit comments

Comments
 (0)