Improve json conversion and stringify behavior

json/deprecated.mbt

@@ -11,3 +11,76 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
+
+///|
+/// Try to get this element as a Null
+#deprecated("Suggestion: `if json is Null { Some(()) } else { None }`")
+pub fn Json::as_null(self : Json) -> Unit? {
+  guard self is Null else { return None }
+  Some(())
+}
+
+///|
+/// Try to get this element as a Boolean
+#deprecated("Suggestion: `if json is True { Some(true) } else if json is False { Some(false) } else { None }`")
+pub fn Json::as_bool(self : Json) -> Bool? {
+  match self {
+    True => Some(true)
+    False => Some(false)
+    _ => None
+  }
+}
+
+///|
+/// Try to get this element as a Number
+#deprecated("Suggestion: `if json is Number(n) { Some(n) } else { None }`")
+pub fn Json::as_number(self : Json) -> Double? {
+  guard self is Number(n, ..) else { return None }
+  Some(n)
+}
+
+///|
+/// Try to get this element as a String
+#deprecated("Suggestion: `if json is String(s) { Some(s) } else { None }`")
+pub fn Json::as_string(self : Json) -> String? {
+  guard self is String(s) else { return None }
+  Some(s)
+}
+
+///|
+/// Try to get this element as an Array
+#deprecated("Suggestion: `if json is Array(array) { Some(array) } else { None }`")
+pub fn Json::as_array(self : Json) -> Array[Json]? {
+  guard self is Array(arr) else { return None }
+  Some(arr)
+}
+
+///|
+/// Try to get this element as a Json Array and get the element at the `index` as a Json Value
+#deprecated("Suggestion: `if json is Array(array) { array.get(index) } else { None }`")
+pub fn Json::item(self : Json, index : Int) -> Json? {
+  if self is Array(arr) {
+    arr.get(index)
+  } else {
+    None
+  }
+}
+
+///|
+/// Try to get this element as an Object
+#deprecated
+pub fn Json::as_object(self : Json) -> Map[String, Json]? {
+  guard self is Object(obj) else { return None }
+  Some(obj)
+}
+
+///|
+/// Try to get this element as a Json Object and get the element with the `key` as a Json Value
+#deprecated("Suggestion: `if json is Object(obj) { obj.get(key) } else { None }`")
+pub fn Json::value(self : Json, key : String) -> Json? {
+  if self is Object(obj) {
+    obj.get(key)
+  } else {
+    None
+  }
+}

json/from_json.mbt

@@ -24,6 +24,8 @@ pub(open) trait FromJson {
 }
 
 ///|
+/// Decode a Json value into a concrete type that implements `FromJson`.
+/// The `path` parameter is used to annotate errors with a JSON Pointer location.
 pub fn[T : FromJson] from_json(
   json : Json,
   path? : JsonPath = Root,
@@ -52,6 +54,9 @@ pub impl FromJson for Int with from_json(json, path) {
     n != @double.neg_infinity else {
     decode_error(path, "Int::from_json: expected number")
   }
+  if n != n.trunc() {
+    decode_error(path, "Int::from_json: expected integer")
+  }
   // Range check before conversion to avoid silent wrap/truncation
   let max_ok = 2147483647.0
   let min_ok = -2147483648.0
@@ -80,6 +85,9 @@ pub impl FromJson for UInt with from_json(json, path) {
     n != @double.neg_infinity else {
     decode_error(path, "UInt::from_json: expected number")
   }
+  if n != n.trunc() {
+    decode_error(path, "UInt::from_json: expected integer")
+  }
   // Range check before conversion to avoid silent wrap/truncation
   let max_ok = 4294967295.0
   if n < 0.0 || n > max_ok {

json/from_json_test.mbt

@@ -407,6 +407,17 @@ test "int roundtrip" {
   inspect(v, content="-123")
 }
 
+///|
+test "int from_json rejects fractional numbers" {
+  let res : Result[Int, _] = try? @json.from_json(Json::number(1.5))
+  inspect(
+    res,
+    content=(
+      #|Err(JsonDecodeError((, "Int::from_json: expected integer")))
+    ),
+  )
+}
+
 ///|
 test "uint roundtrip" {
   let u = 123U.to_json()
@@ -417,6 +428,17 @@ test "uint roundtrip" {
   inspect(v, content="4294967295")
 }
 
+///|
+test "uint from_json rejects fractional numbers" {
+  let res : Result[UInt, _] = try? @json.from_json(Json::number(3.14))
+  inspect(
+    res,
+    content=(
+      #|Err(JsonDecodeError((, "UInt::from_json: expected integer")))
+    ),
+  )
+}
+
 ///|
 test "uint" {
   // valid max

json/json.mbt

@@ -12,79 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-///|
-/// Try to get this element as a Null
-#deprecated("Suggestion: `if json is Null { Some(()) } else { None }`")
-pub fn Json::as_null(self : Json) -> Unit? {
-  guard self is Null else { return None }
-  Some(())
-}
-
-///|
-/// Try to get this element as a Boolean
-#deprecated("Suggestion: `if json is True { Some(true) } else if json is False { Some(false) } else { None }`")
-pub fn Json::as_bool(self : Json) -> Bool? {
-  match self {
-    True => Some(true)
-    False => Some(false)
-    _ => None
-  }
-}
-
-///|
-/// Try to get this element as a Number
-#deprecated("Suggestion: `if json is Number(n) { Some(n) } else { None }`")
-pub fn Json::as_number(self : Json) -> Double? {
-  guard self is Number(n, ..) else { return None }
-  Some(n)
-}
-
-///|
-/// Try to get this element as a String
-#deprecated("Suggestion: `if json is String(s) { Some(s) } else { None }`")
-pub fn Json::as_string(self : Json) -> String? {
-  guard self is String(s) else { return None }
-  Some(s)
-}
-
-///|
-/// Try to get this element as an Array
-#deprecated("Suggestion: `if json is Array(array) { Some(array) } else { None }`")
-pub fn Json::as_array(self : Json) -> Array[Json]? {
-  guard self is Array(arr) else { return None }
-  Some(arr)
-}
-
-///|
-/// Try to get this element as a Json Array and get the element at the `index` as a Json Value
-#deprecated("Suggestion: `if json is Array(array) { array.get(index) } else { None }`")
-pub fn Json::item(self : Json, index : Int) -> Json? {
-  if self is Array(arr) {
-    arr.get(index)
-  } else {
-    None
-  }
-}
-
-///|
-/// Try to get this element as an Object
-#deprecated
-pub fn Json::as_object(self : Json) -> Map[String, Json]? {
-  guard self is Object(obj) else { return None }
-  Some(obj)
-}
-
-///|
-/// Try to get this element as a Json Object and get the element with the `key` as a Json Value
-#deprecated("Suggestion: `if json is Object(obj) { obj.get(key) } else { None }`")
-pub fn Json::value(self : Json, key : String) -> Json? {
-  if self is Object(obj) {
-    obj.get(key)
-  } else {
-    None
-  }
-}
-
 ///|
 fn indent_str(level : Int, indent : Int) -> String {
   if indent == 0 {
@@ -284,7 +211,6 @@ pub fn Json::stringify(
           } else {
             depth += 1
             buf.write_char('{')
-            buf.write_string(indent_str(depth, indent))
             // After child value printed, we resume from this frame
             stack.push(WriteFrame::Object(members.iter(), first=true))
           }
@@ -344,7 +270,9 @@ pub fn Json::stringify(
                   continue None
                 }
               }
-              if !first {
+              if first {
+                buf.write_string(indent_str(depth, indent))
+              } else {
                 buf.write_char(',')
                 buf.write_string(indent_str(depth, indent))
               }
@@ -361,7 +289,9 @@ pub fn Json::stringify(
             None => {
               depth -= 1
               ignore(stack.pop())
-              buf.write_string(indent_str(depth, indent))
+              if !first {
+                buf.write_string(indent_str(depth, indent))
+              }
               buf.write_char('}')
               continue None
             }
@@ -373,8 +303,31 @@ pub fn Json::stringify(
 
 ///|
 fn escape(str : String, escape_slash~ : Bool) -> String {
+  let mut needs_escape = false
+  for c in str.iter() {
+    match c {
+      '"' | '\\' | '\n' | '\r' | '\b' | '\t' => {
+        needs_escape = true
+        break
+      }
+      '/' if escape_slash => {
+        needs_escape = true
+        break
+      }
+      _ => {
+        let code = c.to_int()
+        if code == 0x0C || code < ' ' {
+          needs_escape = true
+          break
+        }
+      }
+    }
+  }
+  if !needs_escape {
+    return str
+  }
   let buf = StringBuilder::new(size_hint=str.length())
-  for c in str {
+  for c in str.iter() {
     match c {
       '"' => buf.write_string("\\\"")
       '\\' => buf.write_string("\\\\")

json/json_path.mbt

@@ -13,18 +13,22 @@
 // limitations under the License.
 
 ///|
+/// A JSON Pointer path (RFC 6901) used to locate values in a JSON document.
+/// This is primarily used in decode errors to indicate where a failure occurred.
 enum JsonPath {
   Root
   Key(JsonPath, mut key~ : String)
   Index(JsonPath, mut index~ : Int)
 } derive(Eq)
 
 ///|
+/// Append an array index segment to the current JSON path.
 pub fn JsonPath::add_index(self : JsonPath, index : Int) -> JsonPath {
   Index(self, index~)
 }
 
 ///|
+/// Append an object key segment to the current JSON path.
 pub fn JsonPath::add_key(self : JsonPath, key : String) -> JsonPath {
   Key(self, key~)
 }

json/json_test.mbt

@@ -162,6 +162,9 @@ test "stringify with replacer" {
       #|{"a":1,"c":3}
     ),
   )
+  // keep none with indentation should still be compact
+  let replacer = @json.Replacer::keep([])
+  inspect(json.stringify(indent=2, replacer~), content="{}")
 }
 
 ///|