Skip to content

Commit 2cb0cbb

Browse files
plokhotnyukplokhotnyuk
committed
Fix #93 by reporting only first missing required field
1 parent 6ac344e commit 2cb0cbb

File tree

6 files changed

+47
-77
lines changed

6 files changed

+47
-77
lines changed

‎benchmark/src/test/scala/com/github/plokhotnyuk/jsoniter_scala/macros/MissingReqFieldBenchmarkSpec.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,21 @@ class MissingReqFieldBenchmarkSpec extends BenchmarkSpecBase {
1111
"""Missing required creator property 's' (index 0)
1212
| at [Source: (byte[])"{}"; line: 1, column: 2]""".stripMargin
1313
benchmark.readJsoniterScala() shouldBe
14-
"""missing required field(s) "s", "i", offset: 0x00000001, buf:
14+
"""missing required field "s", offset: 0x00000001, buf:
1515
| +-------------------------------------------------+
1616
| | 0 1 2 3 4 5 6 7 8 9 a b c d e f |
1717
|+----------+-------------------------------------------------+------------------+
1818
|| 00000000 | 7b 7d | {} |
1919
|+----------+-------------------------------------------------+------------------+""".stripMargin
2020
benchmark.readJsoniterStackless() shouldBe
21-
"""missing required field(s) "s", "i", offset: 0x00000001, buf:
21+
"""missing required field "s", offset: 0x00000001, buf:
2222
| +-------------------------------------------------+
2323
| | 0 1 2 3 4 5 6 7 8 9 a b c d e f |
2424
|+----------+-------------------------------------------------+------------------+
2525
|| 00000000 | 7b 7d | {} |
2626
|+----------+-------------------------------------------------+------------------+""".stripMargin
2727
benchmark.readJsoniterStacklessNoDump() shouldBe
28-
"""missing required field(s) "s", "i", offset: 0x00000001"""
28+
"""missing required field "s", offset: 0x00000001"""
2929
benchmark.readPlayJson() shouldBe
3030
"JsResultException(errors:List((/s,List(JsonValidationError(List(error.path.missing),WrappedArray()))), (/i,List(JsonValidationError(List(error.path.missing),WrappedArray())))))"
3131
}

‎core/src/main/scala/com/github/plokhotnyuk/jsoniter_scala/core/JsonReader.scala

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -63,30 +63,8 @@ final class JsonReader private[jsoniter_scala](
6363
decodeError(i, head - 1, null)
6464
}
6565

66-
def requiredFieldError(reqFields: Array[String], reqBits: Array[Int]): Nothing = {
67-
val len = reqBits.length
68-
var i = 0
69-
var j = 0
70-
while (j < len) {
71-
var bitset = reqBits(j)
72-
while (bitset != 0) {
73-
val lowestOneBit = bitset & -bitset
74-
if (lowestOneBit != 0) {
75-
i = appendString(if (i == 0) "missing required field(s) \"" else "\", \"", i)
76-
i = appendString(reqFields((j << 5) + java.lang.Integer.numberOfTrailingZeros(bitset)), i)
77-
}
78-
bitset ^= lowestOneBit
79-
}
80-
j += 1
81-
}
82-
if (i == 0) throw new IllegalArgumentException("missing required field(s) cannot be reported for arguments: " +
83-
s"reqFields = ${reqFields.mkString("Array(", ", ", ")")}, reqBits = ${reqBits.mkString("Array(", ", ", ")")}")
84-
i = appendChar('"', i)
85-
decodeError(i, head - 1, null)
86-
}
87-
8866
def unexpectedKeyError(len: Int): Nothing = {
89-
var i = prependString("unexpected field: \"", len)
67+
var i = prependString("unexpected field \"", len)
9068
i = appendChar('"', i)
9169
decodeError(i, head - 1, null)
9270
}
@@ -99,14 +77,14 @@ final class JsonReader private[jsoniter_scala](
9977
}
10078

10179
def enumValueError(value: String): Nothing = {
102-
var i = appendString("illegal enum value: \"", 0)
80+
var i = appendString("illegal enum value \"", 0)
10381
i = appendString(value, i)
10482
i = appendChar('"', i)
10583
decodeError(i, head - 1, null)
10684
}
10785

10886
def enumValueError(len: Int): Nothing = {
109-
var i = prependString("illegal enum value: \"", len)
87+
var i = prependString("illegal enum value \"", len)
11088
i = appendChar('"', i)
11189
decodeError(i, head - 1, null)
11290
}

‎core/src/test/scala/com/github/plokhotnyuk/jsoniter_scala/core/JsonReaderSpec.scala

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1966,29 +1966,19 @@ class JsonReaderSpec extends WordSpec with Matchers with PropertyChecks {
19661966
}
19671967
}
19681968
"JsonReader.requiredFieldError" should {
1969-
val jsonReader = reader("{}".getBytes)
1970-
jsonReader.nextToken()
1971-
"throw parsing exception with list of missing required fields that specified by bits" in {
1972-
def check(bits: Int, error: String): Unit =
1973-
assert(intercept[JsonParseException](jsonReader.requiredFieldError(Array("name", "device"), Array(bits)))
1974-
.getMessage.contains(error))
1975-
1976-
check(3, "missing required field(s) \"name\", \"device\", offset: 0x00000000")
1977-
check(2, "missing required field(s) \"device\", offset: 0x00000000")
1978-
check(1, "missing required field(s) \"name\", offset: 0x00000000")
1979-
}
1980-
"throw illegal argument exception in case of missing required fields cannot be selected" in {
1981-
assert(intercept[IllegalArgumentException](jsonReader.requiredFieldError(Array("name", "device"), Array(0)))
1982-
.getMessage.contains("missing required field(s) cannot be reported for arguments: " +
1983-
"reqFields = Array(name, device), reqBits = Array(0)"))
1969+
"throw parsing exception with missing required field" in {
1970+
val jsonReader = reader("{}".getBytes)
1971+
jsonReader.nextToken()
1972+
assert(intercept[JsonParseException](jsonReader.requiredFieldError("name"))
1973+
.getMessage.contains("missing required field \"name\", offset: 0x00000000"))
19841974
}
19851975
}
19861976
"JsonReader.unexpectedKeyError" should {
19871977
"throw parsing exception with name of unexpected key" in {
19881978
val jsonReader = reader("\"xxx\"".getBytes)
19891979
val len = jsonReader.readStringAsCharBuf()
19901980
assert(intercept[JsonParseException](jsonReader.unexpectedKeyError(len))
1991-
.getMessage.contains("unexpected field: \"xxx\", offset: 0x00000004"))
1981+
.getMessage.contains("unexpected field \"xxx\", offset: 0x00000004"))
19921982
}
19931983
}
19941984
"JsonReader.discriminatorValueError" should {
@@ -2004,7 +1994,7 @@ class JsonReaderSpec extends WordSpec with Matchers with PropertyChecks {
20041994
val value = jsonReader.readString(null)
20051995
"throw parsing exception with unexpected enum value" in {
20061996
assert(intercept[JsonParseException](jsonReader.enumValueError(value))
2007-
.getMessage.contains("illegal enum value: \"xxx\", offset: 0x00000004"))
1997+
.getMessage.contains("illegal enum value \"xxx\", offset: 0x00000004"))
20081998
}
20091999
}
20102000
"JsonReader.commaError" should {

‎core/src/test/scala/com/github/plokhotnyuk/jsoniter_scala/core/UserAPI.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ object UserAPI {
4949
} while (in.isNextToken(','))
5050
if (!in.isCurrentToken('}')) in.objectEndOrCommaError()
5151
}
52-
if (req0 == 0) new Device(id = _id, model = _model)
53-
else in.requiredFieldError(r1, Array(req0))
52+
if (req0 != 0) in.requiredFieldError(r1(Integer.numberOfTrailingZeros(req0)))
53+
new Device(id = _id, model = _model)
5454
} else in.readNullOrTokenError(default, '{')
5555

5656
private def d1(in: JsonReader, default: Seq[Device]): Seq[Device] =
@@ -89,8 +89,8 @@ object UserAPI {
8989
} while (in.isNextToken(','))
9090
if (in.isCurrentToken('}').`unary_!`) in.objectEndOrCommaError()
9191
}
92-
if (req0 == 0) new User(name = _name, devices = _devices)
93-
else in.requiredFieldError(r0, Array(req0))
92+
if (req0 != 0) in.requiredFieldError(r0(Integer.numberOfTrailingZeros(req0)))
93+
new User(name = _name, devices = _devices)
9494
} else in.readNullOrTokenError(default, '{')
9595

9696
private def e2(x: Device, out: JsonWriter): Unit = {

‎macros/src/main/scala/com/github/plokhotnyuk/jsoniter_scala/macros/JsonCodecMaker.scala

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -643,17 +643,18 @@ object JsonCodecMaker {
643643
val reqVars =
644644
if (lastReqVarBits == 0) Nil
645645
else reqVarNames.init.map(n => q"var $n = -1") :+ q"var ${reqVarNames.last} = $lastReqVarBits"
646-
val checkReqVars = reqVarNames.map(n => q"$n == 0").reduce((e1, e2) => q"$e1 && $e2")
647-
val construct = q"new $tpe(..${members.map(m => q"${m.name} = ${TermName("_" + m.name)}")})"
648-
val checkReqVarsAndConstruct =
649-
if (lastReqVarBits == 0) construct
646+
val checkReqVars: Seq[Tree] =
647+
if (lastReqVarBits == 0) Nil
650648
else {
651-
val reqFieldNames = withReqFieldsFor(tpe) {
649+
val reqFields = withReqFieldsFor(tpe) {
652650
required.map(r => getMappedName(annotations, r))
653651
}
654-
q"""if ($checkReqVars) $construct
655-
else in.requiredFieldError($reqFieldNames, Array(..$reqVarNames))"""
652+
reqVarNames.zipWithIndex.map { case (n, i) =>
653+
if (i == 0) q"if ($n != 0) in.requiredFieldError($reqFields(Integer.numberOfTrailingZeros($n)))"
654+
else q"if ($n != 0) in.requiredFieldError($reqFields(Integer.numberOfTrailingZeros($n) + ${i << 5}))"
655+
}
656656
}
657+
val construct = q"new $tpe(..${members.map(m => q"${m.name} = ${TermName("_" + m.name)}")})"
657658
val defaults = getDefaults(tpe)
658659
val readVars = members.map { m =>
659660
val tpe = methodType(m)
@@ -689,7 +690,8 @@ object JsonCodecMaker {
689690
} while (in.isNextToken(','))
690691
if (!in.isCurrentToken('}')) in.objectEndOrCommaError()
691692
}
692-
..$checkReqVarsAndConstruct
693+
..$checkReqVars
694+
$construct
693695
} else in.readNullOrTokenError(default, '{')"""
694696
} else if (isSealedAdtBase(tpe)) withDecoderFor(methodKey, default) {
695697
def hashCode(subTpe: Type): Int = {

‎macros/src/test/scala/com/github/plokhotnyuk/jsoniter_scala/macros/JsonCodecMakerSpec.scala

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,7 @@ class JsonCodecMakerSpec extends WordSpec with Matchers {
370370
}.getMessage.contains("expected '\"', offset: 0x00000006"))
371371
assert(intercept[JsonParseException] {
372372
verifyDeser(codecOfEnums, Enums(LocationType.GPS), """{"lt":"Galileo"}""".getBytes)
373-
}.getMessage.contains("illegal enum value: \"Galileo\", offset: 0x0000000e"))
373+
}.getMessage.contains("illegal enum value \"Galileo\", offset: 0x0000000e"))
374374
}
375375
"serialize and deserialize top-level enumerations" in {
376376
verifySerDeser(make[LocationType.LocationType](CodecMakerConfig()), LocationType.GPS,
@@ -392,7 +392,7 @@ class JsonCodecMakerSpec extends WordSpec with Matchers {
392392
assert(intercept[JsonParseException] {
393393
verifyDeser(codecOfJavaEnums, JavaEnums(Level.HIGH, Levels.InnerLevel.LOW),
394394
"""{"l":"LO","il":"HIGH"}""".getBytes)
395-
}.getMessage.contains("illegal enum value: \"LO\", offset: 0x00000008"))
395+
}.getMessage.contains("illegal enum value \"LO\", offset: 0x00000008"))
396396
}
397397
"serialize and deserialize top-level Java enumerations" in {
398398
verifySerDeser(make[Level](CodecMakerConfig()), Level.HIGH, "\"HIGH\"".getBytes)
@@ -693,12 +693,12 @@ class JsonCodecMakerSpec extends WordSpec with Matchers {
693693
verifyDeser(codecOfCamelAndSnakeCases,
694694
CamelSnakeKebabCases(1, 2, 3, 4, 5, 6),
695695
"""{"camel_case":1,"snake_case":2,"kebab_case":3,"camel_1":4,"snake_1":5,"kebab_1":6}""".getBytes)
696-
}.getMessage.contains("missing required field(s) \"camelCase\", \"snakeCase\", \"kebabCase\", \"camel1\", \"snake1\", \"kebab1\", offset: 0x00000051"))
696+
}.getMessage.contains("missing required field \"camelCase\", offset: 0x00000051"))
697697
assert(intercept[JsonParseException] {
698698
verifyDeser(codecOfCamelAndSnakeCases,
699699
CamelSnakeKebabCases(1, 2, 3, 4, 5, 6),
700700
"""{"camel-case":1,"snake-case":2,"kebab-case":3,"camel-1":4,"snake-1":5,"kebab-1":6}""".getBytes)
701-
}.getMessage.contains("missing required field(s) \"camelCase\", \"snakeCase\", \"kebabCase\", \"camel1\", \"snake1\", \"kebab1\", offset: 0x00000051"))
701+
}.getMessage.contains("missing required field \"camelCase\", offset: 0x00000051"))
702702
}
703703
"serialize and deserialize with keys enforced to snake_case and throw parse exception when they are missing" in {
704704
val codecOfCamelAndSnakeCases = make[CamelSnakeKebabCases](CodecMakerConfig(JsonCodecMaker.enforce_snake_case))
@@ -709,12 +709,12 @@ class JsonCodecMakerSpec extends WordSpec with Matchers {
709709
verifyDeser(codecOfCamelAndSnakeCases,
710710
CamelSnakeKebabCases(1, 2, 3, 4, 5, 6),
711711
"""{"camelCase":1,"snakeCase":2,"kebabCase":3,"camel1":4,"snake1":5,"kebab1":6}""".getBytes)
712-
}.getMessage.contains("missing required field(s) \"camel_case\", \"snake_case\", \"kebab_case\", \"camel_1\", \"snake_1\", \"kebab_1\", offset: 0x0000004b"))
712+
}.getMessage.contains("missing required field \"camel_case\", offset: 0x0000004b"))
713713
assert(intercept[JsonParseException] {
714714
verifyDeser(codecOfCamelAndSnakeCases,
715715
CamelSnakeKebabCases(1, 2, 3, 4, 5, 6),
716716
"""{"camel-case":1,"snake-case":2,"kebab-case":3,"camel-1":4,"snake-1":5,"kebab-1":6}""".getBytes)
717-
}.getMessage.contains("missing required field(s) \"camel_case\", \"snake_case\", \"kebab_case\", \"camel_1\", \"snake_1\", \"kebab_1\", offset: 0x00000051"))
717+
}.getMessage.contains("missing required field \"camel_case\", offset: 0x00000051"))
718718
}
719719
"serialize and deserialize with keys enforced to kebab-case and throw parse exception when they are missing" in {
720720
val codecOfCamelAndSnakeCases = make[CamelSnakeKebabCases](CodecMakerConfig(JsonCodecMaker.`enforce-kebab-case`))
@@ -725,18 +725,18 @@ class JsonCodecMakerSpec extends WordSpec with Matchers {
725725
verifyDeser(codecOfCamelAndSnakeCases,
726726
CamelSnakeKebabCases(1, 2, 3, 4, 5, 6),
727727
"""{"camelCase":1,"snakeCase":2,"kebabCase":3,"camel1":4,"snake1":5,"kebab1":6}""".getBytes)
728-
}.getMessage.contains("missing required field(s) \"camel-case\", \"snake-case\", \"kebab-case\", \"camel-1\", \"snake-1\", \"kebab-1\", offset: 0x0000004b"))
728+
}.getMessage.contains("missing required field \"camel-case\", offset: 0x0000004b"))
729729
assert(intercept[JsonParseException] {
730730
verifyDeser(codecOfCamelAndSnakeCases,
731731
CamelSnakeKebabCases(1, 2, 3, 4, 5, 6),
732732
"""{"camel_case":1,"snake_case":2,"kebab_case":3,"camel_1":4,"snake_1":5,"kebab_1":6}""".getBytes)
733-
}.getMessage.contains("missing required field(s) \"camel-case\", \"snake-case\", \"kebab-case\", \"camel-1\", \"snake-1\", \"kebab-1\", offset: 0x00000051"))
733+
}.getMessage.contains("missing required field \"camel-case\", offset: 0x00000051"))
734734
}
735735
"serialize and deserialize with keys overridden by annotation and throw parse exception when they are missing" in {
736736
verifySerDeser(codecOfNameOverridden, NameOverridden(oldName = "VVV"), """{"new_name":"VVV"}""".getBytes)
737737
assert(intercept[JsonParseException] {
738738
verifyDeser(codecOfNameOverridden, NameOverridden(oldName = "VVV"), """{"oldName":"VVV"}""".getBytes)
739-
}.getMessage.contains("missing required field(s) \"new_name\", offset: 0x00000010"))
739+
}.getMessage.contains("missing required field \"new_name\", offset: 0x00000010"))
740740
}
741741
"don't generate codec for case classes with field that have duplicated @named annotation" in {
742742
assert(intercept[TestFailedException](assertCompiles {
@@ -834,7 +834,7 @@ class JsonCodecMakerSpec extends WordSpec with Matchers {
834834
"throw parse exception for unknown case class fields if skipping of them wasn't allowed in materialize call" in {
835835
assert(intercept[JsonParseException] {
836836
verifyDeser(make[Unknown](CodecMakerConfig(skipUnexpectedFields = false)), Unknown(), """{"x":1,"y":[1,2],"z":{"a",3}}""".getBytes)
837-
}.getMessage.contains("unexpected field: \"x\", offset: 0x00000004"))
837+
}.getMessage.contains("unexpected field \"x\", offset: 0x00000004"))
838838
}
839839
"throw parse exception in case of missing required case class fields detected during deserialization" in {
840840
assert(intercept[JsonParseException] {
@@ -851,18 +851,18 @@ class JsonCodecMakerSpec extends WordSpec with Matchers {
851851
90, 91, 92, 93, 94, 95, 96, 97, 98, 99)
852852
verifyDeser(make[Required](CodecMakerConfig()), obj,
853853
"""{
854-
|"r00":0,"r01":1,"r02":2,"r03":3,"r04":4,"r05":5,"r06":6,"r07":7,"r08":8,
855-
|"r10":10,"r11":11,"r12":12,"r13":13,"r14":14,"r15":15,"r16":16,"r17":17,"r18":18,
856-
|"r20":20,"r21":21,"r22":22,"r23":23,"r24":24,"r25":25,"r26":26,"r27":27,"r28":28,
857-
|"r30":30,"r31":31,"r32":32,"r33":33,"r34":34,"r35":35,"r36":36,"r37":37,"r38":38,
858-
|"r40":40,"r41":41,"r42":42,"r43":43,"r44":44,"r45":45,"r46":46,"r47":47,"r48":48,
859-
|"r50":50,"r51":51,"r52":52,"r53":53,"r54":54,"r55":55,"r56":56,"r57":57,"r58":58,
860-
|"r60":60,"r61":61,"r62":62,"r63":63,"r64":64,"r65":65,"r66":66,"r67":67,"r68":68,
861-
|"r70":70,"r71":71,"r72":72,"r73":73,"r74":74,"r75":75,"r76":76,"r77":77,"r78":78,
862-
|"r80":80,"r81":81,"r82":82,"r83":83,"r84":84,"r85":85,"r86":86,"r87":87,"r88":88,
854+
|"r00":0,"r01":1,"r02":2,"r03":3,"r04":4,"r05":5,"r06":6,"r07":7,"r08":8,"r09":9,
855+
|"r10":10,"r11":11,"r12":12,"r13":13,"r14":14,"r15":15,"r16":16,"r17":17,"r18":18,"r19":19,
856+
|"r20":20,"r21":21,"r22":22,"r23":23,"r24":24,"r25":25,"r26":26,"r27":27,"r28":28,"r29":29,
857+
|"r30":30,"r31":31,"r32":32,"r33":33,"r34":34,"r35":35,"r36":36,"r37":37,"r38":38,"r39":39,
858+
|"r40":40,"r41":41,"r42":42,"r43":43,"r44":44,"r45":45,"r46":46,"r47":47,"r48":48,"r49":49,
859+
|"r50":50,"r51":51,"r52":52,"r53":53,"r54":54,"r55":55,"r56":56,"r57":57,"r58":58,"r59":59,
860+
|"r60":60,"r61":61,"r62":62,"r63":63,"r64":64,"r65":65,"r66":66,"r67":67,"r68":68,"r69":69,
861+
|"r70":70,"r71":71,"r72":72,"r73":73,"r74":74,"r75":75,"r76":76,"r77":77,"r78":78,"r79":79,
862+
|"r80":80,"r81":81,"r82":82,"r83":83,"r84":84,"r85":85,"r86":86,"r87":87,"r88":88,"r89":89,
863863
|"r90":90,"r91":91,"r92":92,"r93":93,"r94":94,"r95":95,"r96":96,"r97":97,"r98":98
864864
|}""".stripMargin.getBytes)
865-
}.getMessage.contains("""missing required field(s) "r09", "r19", "r29", "r39", "r49", "r59", "r69", "r79", "r89", "r99", offset: 0x0000032c"""))
865+
}.getMessage.contains("""missing required field "r99", offset: 0x0000037c"""))
866866
}
867867
"serialize and deserialize ADTs using ASCII discriminator field & value" in {
868868
verifySerDeser(codecOfADTList,

0 commit comments

Comments
 (0)