Skip to content

Commit ac4e8cd

Browse files
dsnetdsnet
authored
Fix minor regressions in legacy error strings (#130)
The Go 1 compatability document does not guarantee that error strings remain unchanged across Go releases. However, there is a practical benefit to reducing the churn even if this is not a guaranteed property. Adjust the formatting of v1 error strings to better match how they were historically rendered. This does not ensure 100% equivalency with legacy error strings, but does a better job at mostly preserving the exact string. In the jsontext and jsonwire packages, we make changes to assist in the construction legacy error values. We also make syntactic errors returned by the Decoder more consistent regardless of whether they were produced by a ReadValue or ReadToken call. All changes made to jsontext and jsonwire are an improvement, so we are not taking on technical debt in those packages to support legacy error strings. All legacy transformations are kept in the v1 code.
1 parent d8c9bc4 commit ac4e8cd

19 files changed

+220
-154
lines changed

‎arshal_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2531,7 +2531,7 @@ func TestMarshal(t *testing.T) {
25312531
name: jsontest.Name("Structs/InlinedFallback/TextValue/InvalidObjectEnd"),
25322532
in: structInlineTextValue{X: jsontext.Value(` { "name" : false , } `)},
25332533
want: `{"name":false`,
2534-
wantErr: EM(newInvalidCharacterError(",", "before next token", len64(` { "name" : false `), "")).withPos(`{"name":false,`, "").withType(0, T[jsontext.Value]()),
2534+
wantErr: EM(newInvalidCharacterError(",", "at start of value", len64(` { "name" : false `), "")).withPos(`{"name":false,`, "").withType(0, T[jsontext.Value]()),
25352535
}, {
25362536
name: jsontest.Name("Structs/InlinedFallback/TextValue/InvalidDualObject"),
25372537
in: structInlineTextValue{X: jsontext.Value(`{}{}`)},
@@ -6626,7 +6626,7 @@ func TestUnmarshal(t *testing.T) {
66266626
inBuf: `{"A":1,"fizz":nil,"B":2}`,
66276627
inVal: new(structInlineTextValue),
66286628
want: addr(structInlineTextValue{A: 1, X: jsontext.Value(`{"fizz":`)}),
6629-
wantErr: newInvalidCharacterError("i", "within literal null (expecting 'u')", len64(`{"A":1,"fizz":n`), "/fizz"),
6629+
wantErr: newInvalidCharacterError("i", "in literal null (expecting 'u')", len64(`{"A":1,"fizz":n`), "/fizz"),
66306630
}, {
66316631
name: jsontest.Name("Structs/InlinedFallback/TextValue/CaseSensitive"),
66326632
inBuf: `{"A":1,"fizz":"buzz","B":2,"a":3}`,
@@ -6736,13 +6736,13 @@ func TestUnmarshal(t *testing.T) {
67366736
inBuf: `{"A":1,"fizz":nil,"B":2}`,
67376737
inVal: new(structInlineMapStringAny),
67386738
want: addr(structInlineMapStringAny{A: 1, X: jsonObject{"fizz": nil}}),
6739-
wantErr: newInvalidCharacterError("i", "within literal null (expecting 'u')", len64(`{"A":1,"fizz":n`), "/fizz"),
6739+
wantErr: newInvalidCharacterError("i", "in literal null (expecting 'u')", len64(`{"A":1,"fizz":n`), "/fizz"),
67406740
}, {
67416741
name: jsontest.Name("Structs/InlinedFallback/MapStringAny/MergeInvalidValue/Existing"),
67426742
inBuf: `{"A":1,"fizz":nil,"B":2}`,
67436743
inVal: addr(structInlineMapStringAny{A: 1, X: jsonObject{"fizz": true}}),
67446744
want: addr(structInlineMapStringAny{A: 1, X: jsonObject{"fizz": true}}),
6745-
wantErr: newInvalidCharacterError("i", "within literal null (expecting 'u')", len64(`{"A":1,"fizz":n`), "/fizz"),
6745+
wantErr: newInvalidCharacterError("i", "in literal null (expecting 'u')", len64(`{"A":1,"fizz":n`), "/fizz"),
67466746
}, {
67476747
name: jsontest.Name("Structs/InlinedFallback/MapStringAny/CaseSensitive"),
67486748
inBuf: `{"A":1,"fizz":"buzz","B":2,"a":3}`,
@@ -6959,13 +6959,13 @@ func TestUnmarshal(t *testing.T) {
69596959
inBuf: `{"A":1,"fizz":nil,"B":2}`,
69606960
inVal: new(structInlineMapNamedStringAny),
69616961
want: addr(structInlineMapNamedStringAny{A: 1, X: map[namedString]any{"fizz": nil}}),
6962-
wantErr: newInvalidCharacterError("i", "within literal null (expecting 'u')", len64(`{"A":1,"fizz":n`), "/fizz"),
6962+
wantErr: newInvalidCharacterError("i", "in literal null (expecting 'u')", len64(`{"A":1,"fizz":n`), "/fizz"),
69636963
}, {
69646964
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/MergeInvalidValue/Existing"),
69656965
inBuf: `{"A":1,"fizz":nil,"B":2}`,
69666966
inVal: addr(structInlineMapNamedStringAny{A: 1, X: map[namedString]any{"fizz": true}}),
69676967
want: addr(structInlineMapNamedStringAny{A: 1, X: map[namedString]any{"fizz": true}}),
6968-
wantErr: newInvalidCharacterError("i", "within literal null (expecting 'u')", len64(`{"A":1,"fizz":n`), "/fizz"),
6968+
wantErr: newInvalidCharacterError("i", "in literal null (expecting 'u')", len64(`{"A":1,"fizz":n`), "/fizz"),
69696969
}, {
69706970
name: jsontest.Name("Structs/InlinedFallback/MapNamedStringAny/CaseSensitive"),
69716971
inBuf: `{"A":1,"fizz":"buzz","B":2,"a":3}`,

‎internal/jsonwire/decode.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func ConsumeTrue(b []byte) int {
7474
func ConsumeLiteral(b []byte, lit string) (n int, err error) {
7575
for i := 0; i < len(b) && i < len(lit); i++ {
7676
if b[i] != lit[i] {
77-
return i, NewInvalidCharacterError(b[i:], "within literal "+lit+" (expecting "+strconv.QuoteRune(rune(lit[i]))+")")
77+
return i, NewInvalidCharacterError(b[i:], "in literal "+lit+" (expecting "+strconv.QuoteRune(rune(lit[i]))+")")
7878
}
7979
}
8080
if len(b) < len(lit) {
@@ -240,7 +240,7 @@ func ConsumeStringResumable(flags *ValueFlags, b []byte, resumeOffset int, valid
240240
// Handle invalid control characters.
241241
case r < ' ':
242242
flags.Join(stringNonVerbatim | stringNonCanonical)
243-
return n, NewInvalidCharacterError(b[n:], "within string (expecting non-control character)")
243+
return n, NewInvalidCharacterError(b[n:], "in string (expecting non-control character)")
244244
default:
245245
panic("BUG: unhandled character " + QuoteRune(b[n:]))
246246
}
@@ -374,7 +374,7 @@ func AppendUnquote[Bytes ~[]byte | ~string](dst []byte, src Bytes) (v []byte, er
374374
// Handle invalid control characters.
375375
case r < ' ':
376376
dst = append(dst, src[i:n]...)
377-
return dst, NewInvalidCharacterError(src[n:], "within string (expecting non-control character)")
377+
return dst, NewInvalidCharacterError(src[n:], "in string (expecting non-control character)")
378378
default:
379379
panic("BUG: unhandled character " + QuoteRune(src[n:]))
380380
}
@@ -513,7 +513,7 @@ beforeInteger:
513513
}
514514
state = withinIntegerDigits
515515
default:
516-
return n, state, NewInvalidCharacterError(b[n:], "within number (expecting digit)")
516+
return n, state, NewInvalidCharacterError(b[n:], "in number (expecting digit)")
517517
}
518518

519519
// Consume optional fractional component.
@@ -527,7 +527,7 @@ beforeFractional:
527527
case '0' <= b[n] && b[n] <= '9':
528528
n++
529529
default:
530-
return n, state, NewInvalidCharacterError(b[n:], "within number (expecting digit)")
530+
return n, state, NewInvalidCharacterError(b[n:], "in number (expecting digit)")
531531
}
532532
for uint(len(b)) > uint(n) && ('0' <= b[n] && b[n] <= '9') {
533533
n++
@@ -549,7 +549,7 @@ beforeExponent:
549549
case '0' <= b[n] && b[n] <= '9':
550550
n++
551551
default:
552-
return n, state, NewInvalidCharacterError(b[n:], "within number (expecting digit)")
552+
return n, state, NewInvalidCharacterError(b[n:], "in number (expecting digit)")
553553
}
554554
for uint(len(b)) > uint(n) && ('0' <= b[n] && b[n] <= '9') {
555555
n++

‎internal/jsonwire/decode_test.go

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ func TestConsumeLiteral(t *testing.T) {
4949
{"null", "nul", 3, io.ErrUnexpectedEOF},
5050
{"null", "null", 4, nil},
5151
{"null", "nullx", 4, nil},
52-
{"null", "x", 0, NewInvalidCharacterError("x", "within literal null (expecting 'n')")},
53-
{"null", "nuxx", 2, NewInvalidCharacterError("x", "within literal null (expecting 'l')")},
52+
{"null", "x", 0, NewInvalidCharacterError("x", "in literal null (expecting 'n')")},
53+
{"null", "nuxx", 2, NewInvalidCharacterError("x", "in literal null (expecting 'l')")},
5454

5555
{"false", "", 0, io.ErrUnexpectedEOF},
5656
{"false", "f", 1, io.ErrUnexpectedEOF},
@@ -59,17 +59,17 @@ func TestConsumeLiteral(t *testing.T) {
5959
{"false", "fals", 4, io.ErrUnexpectedEOF},
6060
{"false", "false", 5, nil},
6161
{"false", "falsex", 5, nil},
62-
{"false", "x", 0, NewInvalidCharacterError("x", "within literal false (expecting 'f')")},
63-
{"false", "falsx", 4, NewInvalidCharacterError("x", "within literal false (expecting 'e')")},
62+
{"false", "x", 0, NewInvalidCharacterError("x", "in literal false (expecting 'f')")},
63+
{"false", "falsx", 4, NewInvalidCharacterError("x", "in literal false (expecting 'e')")},
6464

6565
{"true", "", 0, io.ErrUnexpectedEOF},
6666
{"true", "t", 1, io.ErrUnexpectedEOF},
6767
{"true", "tr", 2, io.ErrUnexpectedEOF},
6868
{"true", "tru", 3, io.ErrUnexpectedEOF},
6969
{"true", "true", 4, nil},
7070
{"true", "truex", 4, nil},
71-
{"true", "x", 0, NewInvalidCharacterError("x", "within literal true (expecting 't')")},
72-
{"true", "trux", 3, NewInvalidCharacterError("x", "within literal true (expecting 'e')")},
71+
{"true", "x", 0, NewInvalidCharacterError("x", "in literal true (expecting 't')")},
72+
{"true", "trux", 3, NewInvalidCharacterError("x", "in literal true (expecting 'e')")},
7373
}
7474

7575
for _, tt := range tests {
@@ -120,9 +120,9 @@ func TestConsumeString(t *testing.T) {
120120
{` ""x`, false, 0, 0, 0, "", NewInvalidCharacterError(" ", "at start of string (expecting '\"')"), errPrev, errPrev},
121121
{`"hello`, false, 6, 6, 0, "hello", io.ErrUnexpectedEOF, errPrev, errPrev},
122122
{`"hello"`, true, 7, 7, 0, "hello", nil, nil, nil},
123-
{"\"\x00\"", false, 1, 1, stringNonVerbatim | stringNonCanonical, "", NewInvalidCharacterError("\x00", "within string (expecting non-control character)"), errPrev, errPrev},
123+
{"\"\x00\"", false, 1, 1, stringNonVerbatim | stringNonCanonical, "", NewInvalidCharacterError("\x00", "in string (expecting non-control character)"), errPrev, errPrev},
124124
{`"\u0000"`, false, 8, 8, stringNonVerbatim, "\x00", nil, nil, nil},
125-
{"\"\x1f\"", false, 1, 1, stringNonVerbatim | stringNonCanonical, "", NewInvalidCharacterError("\x1f", "within string (expecting non-control character)"), errPrev, errPrev},
125+
{"\"\x1f\"", false, 1, 1, stringNonVerbatim | stringNonCanonical, "", NewInvalidCharacterError("\x1f", "in string (expecting non-control character)"), errPrev, errPrev},
126126
{`"\u001f"`, false, 8, 8, stringNonVerbatim, "\x1f", nil, nil, nil},
127127
{`"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"`, true, 54, 54, 0, "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz", nil, nil, nil},
128128
{"\" !#$%'()*+,-./0123456789:;=?@[]^_`{|}~\x7f\"", true, 41, 41, 0, " !#$%'()*+,-./0123456789:;=?@[]^_`{|}~\x7f", nil, nil, nil},
@@ -227,13 +227,13 @@ func TestConsumeNumber(t *testing.T) {
227227
wantErr error
228228
}{
229229
{"", false, 0, io.ErrUnexpectedEOF},
230-
{`"NaN"`, false, 0, NewInvalidCharacterError("\"", "within number (expecting digit)")},
231-
{`"Infinity"`, false, 0, NewInvalidCharacterError("\"", "within number (expecting digit)")},
232-
{`"-Infinity"`, false, 0, NewInvalidCharacterError("\"", "within number (expecting digit)")},
233-
{".0", false, 0, NewInvalidCharacterError(".", "within number (expecting digit)")},
230+
{`"NaN"`, false, 0, NewInvalidCharacterError("\"", "in number (expecting digit)")},
231+
{`"Infinity"`, false, 0, NewInvalidCharacterError("\"", "in number (expecting digit)")},
232+
{`"-Infinity"`, false, 0, NewInvalidCharacterError("\"", "in number (expecting digit)")},
233+
{".0", false, 0, NewInvalidCharacterError(".", "in number (expecting digit)")},
234234
{"0", true, 1, nil},
235235
{"-0", false, 2, nil},
236-
{"+0", false, 0, NewInvalidCharacterError("+", "within number (expecting digit)")},
236+
{"+0", false, 0, NewInvalidCharacterError("+", "in number (expecting digit)")},
237237
{"1", true, 1, nil},
238238
{"-1", false, 2, nil},
239239
{"00", true, 1, nil},
@@ -248,8 +248,8 @@ func TestConsumeNumber(t *testing.T) {
248248
{"-9876543210", false, 11, nil},
249249
{"9876543210x", true, 10, nil},
250250
{"-9876543210x", false, 11, nil},
251-
{" 9876543210", true, 0, NewInvalidCharacterError(" ", "within number (expecting digit)")},
252-
{"- 9876543210", false, 1, NewInvalidCharacterError(" ", "within number (expecting digit)")},
251+
{" 9876543210", true, 0, NewInvalidCharacterError(" ", "in number (expecting digit)")},
252+
{"- 9876543210", false, 1, NewInvalidCharacterError(" ", "in number (expecting digit)")},
253253
{strings.Repeat("9876543210", 1000), true, 10000, nil},
254254
{"-" + strings.Repeat("9876543210", 1000), false, 1 + 10000, nil},
255255
{"0.", false, 1, io.ErrUnexpectedEOF},
@@ -266,16 +266,16 @@ func TestConsumeNumber(t *testing.T) {
266266
{"-0E0", false, 4, nil},
267267
{"0.0123456789", false, 12, nil},
268268
{"-0.0123456789", false, 13, nil},
269-
{"1.f", false, 2, NewInvalidCharacterError("f", "within number (expecting digit)")},
270-
{"-1.f", false, 3, NewInvalidCharacterError("f", "within number (expecting digit)")},
271-
{"1.e", false, 2, NewInvalidCharacterError("e", "within number (expecting digit)")},
272-
{"-1.e", false, 3, NewInvalidCharacterError("e", "within number (expecting digit)")},
269+
{"1.f", false, 2, NewInvalidCharacterError("f", "in number (expecting digit)")},
270+
{"-1.f", false, 3, NewInvalidCharacterError("f", "in number (expecting digit)")},
271+
{"1.e", false, 2, NewInvalidCharacterError("e", "in number (expecting digit)")},
272+
{"-1.e", false, 3, NewInvalidCharacterError("e", "in number (expecting digit)")},
273273
{"1e0", false, 3, nil},
274274
{"-1e0", false, 4, nil},
275275
{"1E0", false, 3, nil},
276276
{"-1E0", false, 4, nil},
277-
{"1Ex", false, 2, NewInvalidCharacterError("x", "within number (expecting digit)")},
278-
{"-1Ex", false, 3, NewInvalidCharacterError("x", "within number (expecting digit)")},
277+
{"1Ex", false, 2, NewInvalidCharacterError("x", "in number (expecting digit)")},
278+
{"-1Ex", false, 3, NewInvalidCharacterError("x", "in number (expecting digit)")},
279279
{"1e-0", false, 4, nil},
280280
{"-1e-0", false, 5, nil},
281281
{"1e+0", false, 4, nil},

‎internal/jsonwire/wire.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,9 @@ func NewInvalidEscapeSequenceError[Bytes ~[]byte | ~string](what Bytes) error {
157157
return r == '`' || r == utf8.RuneError || unicode.IsSpace(r) || !unicode.IsPrint(r)
158158
}) >= 0
159159
if needEscape {
160-
return errors.New("invalid " + label + " " + strconv.Quote(string(what)) + " within string")
160+
return errors.New("invalid " + label + " " + strconv.Quote(string(what)) + " in string")
161161
} else {
162-
return errors.New("invalid " + label + " `" + string(what) + "` within string")
162+
return errors.New("invalid " + label + " `" + string(what) + "` in string")
163163
}
164164
}
165165

‎jsontext/decode.go

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -364,9 +364,22 @@ func (d *decoderState) CountNextDelimWhitespace() int {
364364

365365
// checkDelim checks whether delim is valid for the given next kind.
366366
func (d *decoderState) checkDelim(delim byte, next Kind) error {
367+
where := "at start of value"
368+
switch d.Tokens.needDelim(next) {
369+
case delim:
370+
return nil
371+
case ':':
372+
where = "after object name (expecting ':')"
373+
case ',':
374+
if d.Tokens.Last.isObject() {
375+
where = "after object value (expecting ',' or '}')"
376+
} else {
377+
where = "after array element (expecting ',' or ']')"
378+
}
379+
}
367380
pos := d.prevEnd // restore position to right after leading whitespace
368381
pos += jsonwire.ConsumeWhitespace(d.buf[pos:])
369-
err := d.Tokens.checkDelim(delim, next)
382+
err := jsonwire.NewInvalidCharacterError(d.buf[pos:], where)
370383
return wrapSyntacticError(d, err, pos, 0)
371384
}
372385

@@ -618,7 +631,7 @@ func (d *decoderState) ReadToken() (Token, error) {
618631
return ArrayEnd, nil
619632

620633
default:
621-
err = jsonwire.NewInvalidCharacterError(d.buf[pos:], "at start of token")
634+
err = jsonwire.NewInvalidCharacterError(d.buf[pos:], "at start of value")
622635
return Token{}, wrapSyntacticError(d, err, pos, +1)
623636
}
624637
}
@@ -833,6 +846,9 @@ func (d *decoderState) consumeValue(flags *jsonwire.ValueFlags, pos, depth int)
833846
case '[':
834847
return d.consumeArray(flags, pos, depth)
835848
default:
849+
if (d.Tokens.Last.isObject() && next == ']') || (d.Tokens.Last.isArray() && next == '}') {
850+
return pos, errMismatchDelim
851+
}
836852
return pos, jsonwire.NewInvalidCharacterError(d.buf[pos:], "at start of value")
837853
}
838854
if err == io.ErrUnexpectedEOF {
@@ -1068,7 +1084,7 @@ func (d *decoderState) consumeArray(flags *jsonwire.ValueFlags, pos, depth int)
10681084
pos++
10691085
return pos, nil
10701086
default:
1071-
return pos, jsonwire.NewInvalidCharacterError(d.buf[pos:], "after array value (expecting ',' or ']')")
1087+
return pos, jsonwire.NewInvalidCharacterError(d.buf[pos:], "after array element (expecting ',' or ']')")
10721088
}
10731089
}
10741090
}

0 commit comments

Comments
 (0)