fix(json): tighten the surrogate docs and error coverage

Follow-up nits from review.

Two doc claims were imprecise. "Each escape must denote a scalar value"
is not true of either half of an accepted pair, so it now says an escape
must denote a scalar on its own *or* be one half of a correctly ordered
pair. And the promise that a failed pairing always raises `InvalidChar`
at the backslash overstated it: running out of input still raises
`InvalidEof`, and a malformed second escape is reported as the hex-digit
error it is. Both are qualified now. The comparison to other parsers is
narrowed too — serde_json rejects when parsing into `String`/`Value`
while its byte mode admits WTF-8, and it is Go's `encoding/json` that
substitutes U+FFFD, its v2 parser being stricter.

The test preamble claimed the old parser accepted every rejection below
it, which was false: the ones that also run out of input or misspell the
second escape were already errors. Corrected, and the escaped-leading +
raw-trailing case moved last, since that is the one that aborted the
process outright before this change and would otherwise mask the blocks
after it.

Added the exact assertions that were missing: the second mismatch arm, a
second escape that is well formed but is not a trailing surrogate, and a
trailing backslash at EOF — all pinning the one documented position. Also
covers the two arms whose `shift` was corrected, where an unknown escape
naming a non-BMP character and a non-BMP character among hex digits are
now reported whole rather than as a broken half one column further on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: bobzhang

json/README.mbt.md

@@ -54,10 +54,11 @@ test "parse and validate jsons" {
 #### What may appear inside a string
 
 Every string in a parsed document is well-formed Unicode, so each `\uXXXX`
-escape has to denote a Unicode scalar value. An escaped leading surrogate
-must be followed immediately by an escaped trailing surrogate, and the pair
-decodes to the single character it stands for; an escape that cannot pair up
-is a parse error.
+escape must denote a Unicode scalar value on its own or be one half of a
+correctly ordered surrogate pair. An escaped leading surrogate must be
+followed immediately by an escaped trailing surrogate, and the pair decodes
+to the single character it stands for; an escape that cannot pair up is a
+parse error, reported at the backslash that opens it.
 
 ```mbt check
 ///|
@@ -76,7 +77,8 @@ so the alternatives would be to hand one back that is not, or to substitute
 U+FFFD and lose the difference between two distinct keys. Note that
 `JSON.stringify` in JavaScript does emit lone surrogates this way, so a
 document JavaScript and Python accept can be rejected here — as it is by
-Rust's serde_json; Go substitutes U+FFFD instead.
+Rust's serde_json when parsing into `String` or `Value`; Go's
+`encoding/json` substitutes U+FFFD instead.
 
 ### Object Navigation
 

json/lex_string_test.mbt

@@ -99,14 +99,17 @@ test "lex_hex_digits accepts all hex digit ranges" {
 // `ParseError`.
 //
 // The cases are split by shape so that one failing assertion cannot mask the
-// rest; the old parser accepted all of the rejections below, and aborted
-// outright on the escaped-leading + raw-trailing one.
+// rest. The old parser accepted the *unpaired-surrogate* spellings below —
+// it already rejected the ones that also run out of input or misspell the
+// second escape — and aborted the process outright on the escaped-leading +
+// raw-trailing one, which is why that case is kept last.
 
 ///|
 test "unpaired leading-surrogate escape is rejected" {
-  // Nothing after it at all, and a closing quote after it.
-  assert_false(@json.valid("\"\\uD800"))
+  // A closing quote after it, and nothing after it at all. (The second was
+  // already an EOF error before the surrogate rule.)
   assert_false(@json.valid("\"\\uD800\""))
+  assert_false(@json.valid("\"\\uD800"))
   // A character that is not the start of an escape.
   assert_false(@json.valid("\"\\uD800x\""))
   // An escape that is not `\u`.
@@ -128,12 +131,6 @@ test "bare trailing-surrogate escape is rejected" {
   assert_false(@json.valid("\"\\uDE00\\uD83D\""))
 }
 
-///|
-test "escaped and raw surrogate halves do not pair up" {
-  let lone_low = String::from_array([(0xDC00).unsafe_to_char()])
-  assert_false(@json.valid("\"\\uD800" + lone_low + "\""))
-}
-
 ///|
 test "unpaired surrogate escapes are rejected in object keys too" {
   assert_false(@json.valid("{\"\\uD800\": 1}"))
@@ -168,11 +165,50 @@ test "the surrogate error names the escape that could not pair up" {
       #|InvalidChar({ line: 1, column: 1 }, '\\')
     ),
   )
-  // Running out of input is still an EOF error rather than a character one.
+  // The same position whichever way the pairing fails: a follower that is
+  // not a backslash, a backslash not followed by `u`, and a second escape
+  // that is well formed but is not a trailing surrogate.
+  debug_inspect(
+    expect_parse_error("\"\\uD800\\n\"", "expected InvalidChar"),
+    content=(
+      #|InvalidChar({ line: 1, column: 1 }, '\\')
+    ),
+  )
+  debug_inspect(
+    expect_parse_error("\"\\uD800\\u0041\"", "expected InvalidChar"),
+    content=(
+      #|InvalidChar({ line: 1, column: 1 }, '\\')
+    ),
+  )
+  // Running out of input is still an EOF error rather than a character one,
+  // including on a trailing backslash where the second escape should start.
   debug_inspect(
     expect_parse_error("\"\\uD800", "expected InvalidEof"),
     content="InvalidEof",
   )
+  debug_inspect(
+    expect_parse_error("\"\\uD800\\", "expected InvalidEof"),
+    content="InvalidEof",
+  )
+}
+
+///|
+test "an escape naming a non-BMP character reports it whole" {
+  // These two arms read a character and then step back by its width. Using
+  // a fixed step of one code unit landed inside a non-BMP character and
+  // reported a broken half one column further on.
+  debug_inspect(
+    expect_parse_error("\"\\\u{1F600}\"", "expected InvalidChar"),
+    content=(
+      #|InvalidChar({ line: 1, column: 2 }, '😀')
+    ),
+  )
+  debug_inspect(
+    expect_parse_error("\"\\u1\u{1F600}23\"", "expected InvalidChar"),
+    content=(
+      #|InvalidChar({ line: 1, column: 4 }, '😀')
+    ),
+  )
 }
 
 ///|
@@ -194,3 +230,12 @@ test "valid surrogate pairs decode to one scalar value" {
   // character, which is a scalar value like any other.
   assert_true(@json.parse("\"\\u0041\\uFFFD\"") == Json::string("A\u{FFFD}"))
 }
+
+///|
+test "escaped and raw surrogate halves do not pair up" {
+  // Kept last: before the surrogate rule this input aborted the process
+  // rather than raising, so a regression here would take the whole test
+  // binary with it.
+  let lone_low = String::from_array([(0xDC00).unsafe_to_char()])
+  assert_false(@json.valid("\"\\uD800" + lone_low + "\""))
+}

json/parse.mbt

@@ -32,11 +32,17 @@ pub fn valid(input : StringView) -> Bool {
 /// ## What strings may contain
 ///
 /// Every string in the result is well-formed Unicode: each `\uXXXX` escape
-/// has to denote a Unicode scalar value. An escaped leading surrogate
+/// must denote a Unicode scalar value on its own, or be one half of a
+/// correctly ordered surrogate pair. An escaped leading surrogate
 /// (`\uD800`–`\uDBFF`) must therefore be followed immediately by an escaped
 /// trailing surrogate (`\uDC00`–`\uDFFF`), and the pair is decoded as the
-/// one character it stands for; an escape that cannot pair up raises
-/// `InvalidChar`, positioned at the backslash that opens it.
+/// one character it stands for.
+///
+/// An escape that cannot pair up raises `InvalidChar` positioned at the
+/// backslash that opens it — that one position, whatever the scan actually
+/// stopped on. Input that simply runs out still raises `InvalidEof`, and a
+/// second escape that is itself malformed is reported as the hex-digit
+/// error it is, at the offending digit.
 ///
 /// This is a limit on what a string may *contain*, which RFC 8259 §9 leaves
 /// to the implementation — not a claim about which documents are
@@ -50,7 +56,9 @@ pub fn valid(input : StringView) -> Bool {
 ///
 /// The cost is real: `JSON.stringify` in JavaScript emits lone surrogates as
 /// `\uXXXX`, so some JSON that JavaScript and Python accept is rejected
-/// here. Rust's serde_json rejects it too; Go substitutes U+FFFD.
+/// here. Rust's serde_json rejects it too when parsing into `String` or
+/// `Value`, though its byte-oriented mode admits WTF-8; Go's `encoding/json`
+/// substitutes U+FFFD, while its experimental v2 parser is stricter.
 #label_migration(max_nesting_depth, fill=false)
 pub fn parse(
   input : StringView,