Skip to content

Commit 79b6b00

Browse files
jurgenvinjujurgenvinju
authored
Merge pull request #2116 from usethesource/json-io-tests-for-null-handlers
Improved testing of JSON IO, especially in de code that handles null objects"
2 parents fc8b581 + 7dbb8be commit 79b6b00

4 files changed

Lines changed: 100 additions & 31 deletions

File tree

‎pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@
434434
<dependency>
435435
<groupId>io.usethesource</groupId>
436436
<artifactId>vallang</artifactId>
437-
<version>1.0.0-RC12</version>
437+
<version>1.0.0-RC15</version>
438438
</dependency>
439439
<dependency>
440440
<groupId>org.ow2.asm</groupId>

‎src/org/rascalmpl/library/lang/json/IO.rsc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,16 @@ public map[type[value] forType, value nullValue] defaultJSONNULLValues = (
112112
#Maybe[value] : nothing(),
113113
#node : "null"(),
114114
#int : -1,
115+
#num : -1.0,
115116
#real : -1.0,
116117
#rat : -1r1,
117118
#value : "null"(),
118119
#str : "",
119120
#list[value] : [],
120121
#set[value] : {},
121122
#map[value,value] : (),
122-
#loc : |unknown:///|
123+
#loc : |unknown:///|,
124+
#bool : false
123125
);
124126

125127
@javaClass{org.rascalmpl.library.lang.json.IO}

‎src/org/rascalmpl/library/lang/json/internal/JsonValueReader.java‎

Lines changed: 73 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import java.util.HashMap;
2727
import java.util.List;
2828
import java.util.Map;
29+
import java.util.Map.Entry;
30+
import java.util.stream.Collectors;
2931
import java.util.Set;
3032
import org.rascalmpl.debug.IRascalMonitor;
3133
import org.rascalmpl.exceptions.RuntimeExceptionFactory;
@@ -132,9 +134,10 @@ public IValue visitReal(Type type) throws IOException {
132134
}
133135

134136
private IValue inferNullValue(Map<Type, IValue> nulls, Type expected) {
135-
return nulls.keySet().stream()
136-
.sorted((x,y) -> x.compareTo(y)) // smaller types are matched first
137-
.filter(superType -> expected.isSubtypeOf(superType)) // remove any type that does not fit
137+
return nulls.entrySet().stream()
138+
.map(Entry::getKey)
139+
.sorted(Type::compareTo)
140+
.filter(superType -> expected.isSubtypeOf(superType))
138141
.findFirst() // give the most specific match
139142
.map(t -> nulls.get(t)) // lookup the corresponding null value
140143
.filter(r -> r.getType().isSubtypeOf(expected)) // the value in the table still has to fit the currently expected type
@@ -157,27 +160,37 @@ public IValue visitString(Type type) throws IOException {
157160
return vf.string(nextString());
158161
}
159162

160-
@Override
161-
public IValue visitTuple(Type type) throws IOException {
162-
if (isNull()) {
163-
return null;
164-
}
163+
@Override
164+
public IValue visitTuple(Type type) throws IOException {
165+
if (isNull()) {
166+
return null;
167+
}
165168

166169
List<IValue> l = new ArrayList<>();
167170
in.beginArray();
168171

169172
if (type.hasFieldNames()) {
170173
for (int i = 0; i < type.getArity(); i++) {
171-
l.add(read(in, type.getFieldType(i)));
174+
l.add(type.getFieldType(i).accept(this));
172175
}
173176
}
174177
else {
175178
for (int i = 0; i < type.getArity(); i++) {
176-
l.add(read(in, type.getFieldType(i)));
179+
l.add(type.getFieldType(i).accept(this));
177180
}
178181
}
179182

180183
in.endArray();
184+
185+
// filter all the null values
186+
l.forEach(e -> {
187+
if (e == null) {
188+
throw parseErrorHere("Tuples can not have null elements.");
189+
}
190+
});
191+
192+
assert type.getArity() == l.size();
193+
181194
return vf.tuple(l.toArray(new IValue[l.size()]));
182195
}
183196

@@ -193,6 +206,10 @@ public IValue visitFunction(Type type) throws IOException {
193206

194207
@Override
195208
public IValue visitSourceLocation(Type type) throws IOException {
209+
if (isNull()) {
210+
return inferNullValue(nulls, type);
211+
}
212+
196213
switch (in.peek()) {
197214
case STRING:
198215
return sourceLocationString();
@@ -300,7 +317,7 @@ public IValue visitValue(Type type) throws IOException {
300317
case BEGIN_OBJECT:
301318
return visitNode(TF.nodeType());
302319
case BOOLEAN:
303-
return visitBool(TF.nodeType());
320+
return visitBool(TF.boolType());
304321
case NAME:
305322
// this would be weird though
306323
return vf.string(nextName());
@@ -341,8 +358,8 @@ public IValue visitRational(Type type) throws IOException {
341358
switch (in.peek()) {
342359
case BEGIN_ARRAY:
343360
in.beginArray();
344-
IInteger numA = (IInteger) read(in, TF.integerType());
345-
IInteger denomA = (IInteger) read(in, TF.integerType());
361+
IInteger numA = (IInteger) TF.integerType().accept(this);
362+
IInteger denomA = (IInteger) TF.integerType().accept(this);
346363
in.endArray();
347364
return vf.rational(numA, denomA);
348365
case STRING:
@@ -367,17 +384,23 @@ public IValue visitMap(Type type) throws IOException {
367384
}
368385

369386
while (in.hasNext()) {
370-
w.put(vf.string(nextName()), read(in, type.getValueType()));
387+
IString label = vf.string(nextName());
388+
IValue value = type.getValueType().accept(this);
389+
if (value != null) {
390+
w.put(label, value);
391+
}
371392
}
372393
in.endObject();
373394
return w.done();
374395
case BEGIN_ARRAY:
375396
in.beginArray();
376397
while (in.hasNext()) {
377398
in.beginArray();
378-
IValue key = read(in, type.getKeyType());
379-
IValue value = read(in, type.getValueType());
380-
w.put(key,value);
399+
IValue key = type.getKeyType().accept(this);
400+
IValue value = type.getValueType().accept(this);
401+
if (key != null && value != null) {
402+
w.put(key,value);
403+
}
381404
in.endArray();
382405
}
383406
in.endArray();
@@ -534,7 +557,7 @@ private IValue visitObjectAsAbstractData(Type type) throws IOException {
534557
if (explicitConstructorNames || explicitDataTypes) {
535558
String consName = null;
536559
String typeName = null; // this one is optional, and the order with cons is not defined.
537-
String consLabel = in.nextName();
560+
String consLabel = nextName();
538561

539562
// first we read either a cons name or a type name
540563
if (explicitConstructorNames && "_constructor".equals(consLabel)) {
@@ -547,14 +570,14 @@ else if (explicitDataTypes && "_type".equals(consLabel)) {
547570
// optionally read the second field
548571
if (explicitDataTypes && typeName == null) {
549572
// we've read a constructor name, but we still need a type name
550-
consLabel = in.nextName();
573+
consLabel = nextName();
551574
if (explicitDataTypes && "_type".equals(consLabel)) {
552575
typeName = in.nextString();
553576
}
554577
}
555578
else if (explicitDataTypes && consName == null) {
556579
// we've read type name, but we still need a constructor name
557-
consLabel = in.nextName();
580+
consLabel = nextName();
558581
if (explicitDataTypes && "_constructor".equals(consLabel)) {
559582
consName = in.nextString();
560583
}
@@ -599,9 +622,9 @@ else if (alternatives.size() == 0) {
599622
}
600623

601624
while (in.hasNext()) {
602-
String label = in.nextName();
625+
String label = nextName();
603626
if (cons.hasField(label)) {
604-
IValue val = read(in, cons.getFieldType(label));
627+
IValue val = cons.getFieldType(label).accept(this);
605628
if (val != null) {
606629
args[cons.getFieldIndex(label)] = val;
607630
}
@@ -611,7 +634,7 @@ else if (alternatives.size() == 0) {
611634
}
612635
else if (cons.hasKeywordField(label, store)) {
613636
if (!isNull()) { // lookahead for null to give default parameters the preference.
614-
IValue val = read(in, store.getKeywordParameterType(cons, label));
637+
IValue val = store.getKeywordParameterType(cons, label).accept(this);
615638
// null can still happen if the nulls map doesn't have a default
616639
if (val != null) {
617640
// if the value is null we'd use the default value of the defined field in the constructor
@@ -688,7 +711,7 @@ public IValue visitAbstractData(Type type) throws IOException {
688711

689712
@Override
690713
public IValue visitConstructor(Type type) throws IOException {
691-
return read(in, type.getAbstractDataType());
714+
return type.getAbstractDataType().accept(this);
692715
}
693716

694717
@Override
@@ -711,14 +734,14 @@ public IValue visitNode(Type type) throws IOException {
711734
String kwName = nextName();
712735

713736
if (kwName.equals("_name")) {
714-
name = ((IString) read(in, TF.stringType())).getValue();
737+
name = ((IString) TF.stringType().accept(this)).getValue();
715738
continue;
716739
}
717740

718741
boolean positioned = kwName.startsWith("arg");
719742

720743
if (!isNull()) { // lookahead for null to give default parameters the preference.
721-
IValue val = read(in, TF.valueType());
744+
IValue val = TF.valueType().accept(this);
722745

723746
if (val != null) {
724747
// if the value is null we'd use the default value of the defined field in the constructor
@@ -744,6 +767,7 @@ public IValue visitNode(Type type) throws IOException {
744767

745768
IValue[] argArray = args.entrySet().stream()
746769
.sorted((e, f) -> e.getKey().compareTo(f.getKey()))
770+
.filter(e -> e.getValue() != null)
747771
.map(e -> e.getValue())
748772
.toArray(IValue[]::new);
749773

@@ -752,6 +776,10 @@ public IValue visitNode(Type type) throws IOException {
752776

753777
@Override
754778
public IValue visitNumber(Type type) throws IOException {
779+
if (isNull()) {
780+
return inferNullValue(nulls, type);
781+
}
782+
755783
if (in.peek() == JsonToken.BEGIN_ARRAY) {
756784
return visitRational(type);
757785
}
@@ -802,7 +830,13 @@ public IValue visitList(Type type) throws IOException {
802830
in.beginArray();
803831
while (in.hasNext()) {
804832
// here we pass label from the higher context
805-
w.append(read(in, type.getElementType()));
833+
IValue elem = isNull()
834+
? inferNullValue(nulls, type.getElementType())
835+
: type.getElementType().accept(this);
836+
837+
if (elem != null) {
838+
w.append(elem);
839+
}
806840
}
807841

808842
in.endArray();
@@ -818,7 +852,13 @@ public IValue visitSet(Type type) throws IOException {
818852
in.beginArray();
819853
while (in.hasNext()) {
820854
// here we pass label from the higher context
821-
w.insert(read(in, type.getElementType()));
855+
IValue elem = isNull()
856+
? inferNullValue(nulls, type.getElementType())
857+
: type.getElementType().accept(this);
858+
859+
if (elem != null) {
860+
w.insert(elem);
861+
}
822862
}
823863

824864
in.endArray();
@@ -972,7 +1012,11 @@ public IValue read(JsonReader in, Type expected) throws IOException {
9721012
var dispatch = new ExpectedTypeDispatcher(in);
9731013

9741014
try {
975-
return expected.accept(dispatch);
1015+
var result = expected.accept(dispatch);
1016+
if (result == null) {
1017+
throw new JsonParseException("null occurred outside an optionality context and without a registered representation.");
1018+
}
1019+
return result;
9761020
}
9771021
catch (EOFException | JsonParseException | NumberFormatException | MalformedJsonException | IllegalStateException | NullPointerException e) {
9781022
throw dispatch.parseErrorHere(e.getMessage());

‎src/org/rascalmpl/library/lang/rascal/tests/library/lang/json/JSONIOTests.rsc‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,29 @@ test bool dealWithNull() {
208208
// the builtin Maybe interpreter with non-null
209209
assert parseJSON(#map[str,Maybe[str]], "{\"bla\": \"foo\"}") == ("bla":just("foo"));
210210

211+
// test different specific nulls for different expected types:
212+
for (t <- defaultJSONNULLValues<0>) {
213+
assert parseJSON(t, "null") == (defaultJSONNULLValues[t]?"default-not-found");
214+
}
215+
216+
assert parseJSON(#list[int], "[1,null,2]", nulls=()) == [1, 2];
217+
assert parseJSON(#list[int], "[1,null,2]") == [1, defaultJSONNULLValues[#int], 2];
218+
assert parseJSON(#set[int], "[1,null,2]", nulls=()) == {1, 2};
219+
assert parseJSON(#set[int], "[1,null,2]") == {1, defaultJSONNULLValues[#int], 2};
220+
221+
try {
222+
assert parseJSON(#tuple[int,int], "[null,null]", nulls=()) == [];
223+
}
224+
catch ParseError(_):
225+
assert true;
226+
227+
// test undefined top-level null
228+
try {
229+
parseJSON(#int, "null", nulls=());
230+
assert false;
231+
}
232+
catch ParseError(_): assert true;
233+
211234
// keyword parameters and null
212235
assert cons(bla="foo") := parseJSON(#Cons, "{\"bla\": \"foo\"}");
213236
assert cons() := parseJSON(#Cons, "{\"bla\": null}");

0 commit comments

Comments
 (0)