bugfix: a leading field with a 'none' value was producing an incorrect encoding (#50)

The field was omitted, but not the comma following it, resulting in an
encoding such as '{, otherFields: ...}'

json_serialization/reader.nim

@@ -418,7 +418,7 @@ func expectedFieldsBitmask*(TT: type): auto {.compileTime.} =
         enumAllSerializedFields
 
   const requiredWords =
-    (totalExpectedFields(T) + bitsPerWord - 1) div bitsPerWord
+    (totalSerializedFields(T) + bitsPerWord - 1) div bitsPerWord
 
   var res: array[requiredWords, uint]
 

json_serialization/std/options.nim

@@ -4,11 +4,13 @@ export options
 template writeObjectField*(w: var JsonWriter,
                            record: auto,
                            fieldName: static string,
-                           field: Option) =
+                           field: Option): bool =
   mixin writeObjectField
 
   if field.isSome:
     writeObjectField(w, record, fieldName, field.get)
+  else:
+    false
 
 proc writeValue*(writer: var JsonWriter, value: Option) =
   mixin writeValue

json_serialization/stew/results.nim

@@ -7,11 +7,13 @@ export
 template writeObjectField*[T](w: var JsonWriter,
                               record: auto,
                               fieldName: static string,
-                              field: Result[T, void]) =
+                              field: Result[T, void]): bool =
   mixin writeObjectField
 
   if field.isOk:
     writeObjectField(w, record, fieldName, field.get)
+  else:
+    false
 
 proc writeValue*[T](writer: var JsonWriter, value: Result[T, void]) =
   mixin writeValue

json_serialization/writer.nim

@@ -148,7 +148,7 @@ template isStringLike(v: auto): bool = false
 template writeObjectField*[FieldType, RecordType](w: var JsonWriter,
                                                   record: RecordType,
                                                   fieldName: static string,
-                                                  field: FieldType) =
+                                                  field: FieldType): bool =
   mixin writeFieldIMPL, writeValue
 
   type
@@ -159,6 +159,7 @@ template writeObjectField*[FieldType, RecordType](w: var JsonWriter,
     w.writeValue(field)
   else:
     w.writeFieldIMPL(FieldTag[R, fieldName], field, record)
+  true
 
 proc writeValue*(w: var JsonWriter, value: auto) =
   mixin enumInstanceSerializedFields, writeValue
@@ -238,7 +239,7 @@ proc writeValue*(w: var JsonWriter, value: auto) =
     w.beginRecord RecordType
     value.enumInstanceSerializedFields(fieldName, field):
       mixin writeObjectField
-      writeObjectField(w, value, fieldName, field)
+      if writeObjectField(w, value, fieldName, field):
         w.state = AfterField
     w.endRecord()
 

tests/test_serialization.nim

@@ -74,13 +74,30 @@ type
     dup: bool
 
   HoldsResultOpt* = object
-    r*: ref Simple
     o*: Opt[Simple]
+    r*: ref Simple
 
   WithCustomFieldRule* = object
     str*: string
     intVal*: int
 
+  OtherOptionTest* = object
+    a*: Option[Meter]
+    b*: Option[Meter]
+
+  NestedOptionTest* = object
+    c*: Option[OtherOptionTest]
+    d*: Option[OtherOptionTest]
+
+  SeqOptionTest* = object
+    a*: seq[Option[Meter]]
+    b*: Meter
+
+  OtherOptionTest2* = object
+    a*: Option[Meter]
+    b*: Option[Meter]
+    c*: Option[Meter]
+
 var
   customVisit: TokenRegistry
 
@@ -350,20 +367,70 @@ suite "toJson tests":
     let
       h1 = HoldsOption(o: some Simple(x: 1, y: "2", distance: Meter(3)))
       h2 = HoldsOption(r: newSimple(1, "2", Meter(3)))
+      h3 = Json.decode("""{"r":{"distance":3,"x":1,"y":"2"}}""",
+                       HoldsOption, requireAllFields = true)
 
     Json.roundtripTest h1, """{"r":null,"o":{"distance":3,"x":1,"y":"2"}}"""
     Json.roundtripTest h2, """{"r":{"distance":3,"x":1,"y":"2"}}"""
 
-    let
-      h3 = Json.decode("""{"r":{"distance":3,"x":1,"y":"2"}}""",
-                       HoldsOption, requireAllFields = true)
-
     check h3 == h2
 
     expect SerializationError:
      let h4 = Json.decode("""{"o":{"distance":3,"x":1,"y":"2"}}""",
                           HoldsOption, requireAllFields = true)
 
+  test "Nested option types":
+    let
+      h3 = OtherOptionTest()
+      h4 = OtherOptionTest(a: some Meter(1))
+      h5 = OtherOptionTest(b: some Meter(2))
+      h6 = OtherOptionTest(a: some Meter(3), b: some Meter(4))
+
+    Json.roundtripTest h3, """{}"""
+    Json.roundtripTest h4, """{"a":1}"""
+    Json.roundtripTest h5, """{"b":2}"""
+    Json.roundtripTest h6, """{"a":3,"b":4}"""
+
+    let
+      arr = @[some h3, some h4, some h5, some h6, none(OtherOptionTest)]
+      results = @[
+        """{"c":{},"d":{}}""",
+        """{"c":{},"d":{"a":1}}""",
+        """{"c":{},"d":{"b":2}}""",
+        """{"c":{},"d":{"a":3,"b":4}}""",
+        """{"c":{}}""",
+        """{"c":{"a":1},"d":{}}""",
+        """{"c":{"a":1},"d":{"a":1}}""",
+        """{"c":{"a":1},"d":{"b":2}}""",
+        """{"c":{"a":1},"d":{"a":3,"b":4}}""",
+        """{"c":{"a":1}}""",
+        """{"c":{"b":2},"d":{}}""",
+        """{"c":{"b":2},"d":{"a":1}}""",
+        """{"c":{"b":2},"d":{"b":2}}""",
+        """{"c":{"b":2},"d":{"a":3,"b":4}}""",
+        """{"c":{"b":2}}""",
+        """{"c":{"a":3,"b":4},"d":{}}""",
+        """{"c":{"a":3,"b":4},"d":{"a":1}}""",
+        """{"c":{"a":3,"b":4},"d":{"b":2}}""",
+        """{"c":{"a":3,"b":4},"d":{"a":3,"b":4}}""",
+        """{"c":{"a":3,"b":4}}""",
+        """{"d":{}}""",
+        """{"d":{"a":1}}""",
+        """{"d":{"b":2}}""",
+        """{"d":{"a":3,"b":4}}""",
+        """{}"""
+      ]
+
+
+    var r = 0
+    for a in arr:
+      for b in arr:
+        Json.roundtripTest NestedOptionTest(c: a, d: b), results[r]
+        r.inc
+
+    Json.roundtripTest SeqOptionTest(a: @[some 5.Meter, none Meter], b: Meter(5)), """{"a":[5,null],"b":5}"""
+    Json.roundtripTest OtherOptionTest2(a: some 5.Meter, b: none Meter, c: some 10.Meter), """{"a":5,"c":10}"""
+
   test "Result Opt types":
     check:
       false == static(isFieldExpected Opt[Simple])
@@ -374,7 +441,7 @@ suite "toJson tests":
       h1 = HoldsResultOpt(o: Opt[Simple].ok Simple(x: 1, y: "2", distance: Meter(3)))
       h2 = HoldsResultOpt(r: newSimple(1, "2", Meter(3)))
 
-    Json.roundtripTest h1, """{"r":null,"o":{"distance":3,"x":1,"y":"2"}}"""
+    Json.roundtripTest h1, """{"o":{"distance":3,"x":1,"y":"2"},"r":null}"""
     Json.roundtripTest h2, """{"r":{"distance":3,"x":1,"y":"2"}}"""
 
     let