leadpony / jsonp-test-suite

Test Suite for implementations of Jakarta JSON Processing API (JSON-P)
Apache License 2.0
3 stars 2 forks source link

JsonExclusiveBadTerminationTestCase and EOF behaviour, expect exception? #10

Open ssilverman opened 3 years ago

ssilverman commented 3 years ago

https://github.com/leadpony/jsonp-test-suite/blob/5efa9962d5a0452a00e8a2b2546b541991be5190/src/main/java/org/leadpony/jsonp/testsuite/tests/AbstractJsonParserTest.java#L173-L175

Similar to https://github.com/leadpony/jsonp-test-suite/issues/9, since hasNext() is used to check for a next event, and since there's no next event because of an unexpected EOF, shouldn't a JsonParsingException be thrown instead? I'm wondering why this behaviour should be any different than the exceptions expected from the other test cases in these lists (JsonExclusiveBadTerminationTestCase and BadTerminationTestCase).

Open question: When should hasNext() return false and when should it throw an exception due to malformed JSON, i.e. EOF? Personally, I believe that an unexpected EOF violates the JSON grammar and should cause a parsing exception, and the only time hasNext() should return false is at the end of input.

leadpony commented 3 years ago

@ssilverman Sorry for my slow response. I checked the test cases and found inconsistencies in hasNext() method in the Reference Implementation, as shown below.

No. JSON to parse Result of hasNext()
1 (empty) hasNext(); // returns true :confused:
2 { next();
hasNext(); // throws JsonParsingException
3 {"a" next();
next();
hasNext(); // throws JsonParsingException
4 {"a": next();
next();
hasNext(); // returns true :confused:
5 {"a":1 next();
next();
next();
hasNext(); // throws JsonParsingException
6 {"a":1, next();
next();
next();
hasNext(); // returns true :confused:
7 [ next();
hasNext(); // throws JsonParsingException
8 [1 next();
next();
hasNext(); // throws JsonParsingException
9 [1, next();
next();
hasNext(); // returns true :confused:

I believe that all of hasNext() calls in No.1, 4, 6, and 9 should throw JsonParsingException. If you can agree with me, I will post a new issue in Eclipse Jakarta JSON Processing.

ssilverman commented 3 years ago

I agree with you.