Skip to content

Commit

Permalink
Merge pull request #2116 from usethesource/json-io-tests-for-null-han…
Browse files Browse the repository at this point in the history 
…dlers

Improved testing of JSON IO, especially in de code that handles null objects"
  • Loading branch information
@jurgenvinju
jurgenvinju authored Jan 7, 2025
2 parents fc8b581 + 7dbb8be commit 79b6b00
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 31 deletions.
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@
<dependency>
<groupId>io.usethesource</groupId>
<artifactId>vallang</artifactId>
<version>1.0.0-RC12</version>
<version>1.0.0-RC15</version>
</dependency>
<dependency>
<groupId>org.ow2.asm</groupId>
Expand Down
4 changes: 3 additions & 1 deletion src/org/rascalmpl/library/lang/json/IO.rsc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,16 @@ public map[type[value] forType, value nullValue] defaultJSONNULLValues = (
#Maybe[value] : nothing(),
#node : "null"(),
#int : -1,
#num : -1.0,
#real : -1.0,
#rat : -1r1,
#value : "null"(),
#str : "",
#list[value] : [],
#set[value] : {},
#map[value,value] : (),
#loc : |unknown:///|
#loc : |unknown:///|,
#bool : false
);

@javaClass{org.rascalmpl.library.lang.json.IO}
Expand Down
102 changes: 73 additions & 29 deletions src/org/rascalmpl/library/lang/json/internal/JsonValueReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.stream.Collectors;
import java.util.Set;
import org.rascalmpl.debug.IRascalMonitor;
import org.rascalmpl.exceptions.RuntimeExceptionFactory;
Expand Down Expand Up @@ -132,9 +134,10 @@ public IValue visitReal(Type type) throws IOException {
}

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

@Override
public IValue visitTuple(Type type) throws IOException {
if (isNull()) {
return null;
}
@Override
public IValue visitTuple(Type type) throws IOException {
if (isNull()) {
return null;
}

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

if (type.hasFieldNames()) {
for (int i = 0; i < type.getArity(); i++) {
l.add(read(in, type.getFieldType(i)));
l.add(type.getFieldType(i).accept(this));
}
}
else {
for (int i = 0; i < type.getArity(); i++) {
l.add(read(in, type.getFieldType(i)));
l.add(type.getFieldType(i).accept(this));
}
}

in.endArray();

// filter all the null values
l.forEach(e -> {
if (e == null) {
throw parseErrorHere("Tuples can not have null elements.");
}
});

assert type.getArity() == l.size();

return vf.tuple(l.toArray(new IValue[l.size()]));
}

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

@Override
public IValue visitSourceLocation(Type type) throws IOException {
if (isNull()) {
return inferNullValue(nulls, type);
}

switch (in.peek()) {
case STRING:
return sourceLocationString();
Expand Down Expand Up @@ -300,7 +317,7 @@ public IValue visitValue(Type type) throws IOException {
case BEGIN_OBJECT:
return visitNode(TF.nodeType());
case BOOLEAN:
return visitBool(TF.nodeType());
return visitBool(TF.boolType());
case NAME:
// this would be weird though
return vf.string(nextName());
Expand Down Expand Up @@ -341,8 +358,8 @@ public IValue visitRational(Type type) throws IOException {
switch (in.peek()) {
case BEGIN_ARRAY:
in.beginArray();
IInteger numA = (IInteger) read(in, TF.integerType());
IInteger denomA = (IInteger) read(in, TF.integerType());
IInteger numA = (IInteger) TF.integerType().accept(this);
IInteger denomA = (IInteger) TF.integerType().accept(this);
in.endArray();
return vf.rational(numA, denomA);
case STRING:
Expand All @@ -367,17 +384,23 @@ public IValue visitMap(Type type) throws IOException {
}

while (in.hasNext()) {
w.put(vf.string(nextName()), read(in, type.getValueType()));
IString label = vf.string(nextName());
IValue value = type.getValueType().accept(this);
if (value != null) {
w.put(label, value);
}
}
in.endObject();
return w.done();
case BEGIN_ARRAY:
in.beginArray();
while (in.hasNext()) {
in.beginArray();
IValue key = read(in, type.getKeyType());
IValue value = read(in, type.getValueType());
w.put(key,value);
IValue key = type.getKeyType().accept(this);
IValue value = type.getValueType().accept(this);
if (key != null && value != null) {
w.put(key,value);
}
in.endArray();
}
in.endArray();
Expand Down Expand Up @@ -534,7 +557,7 @@ private IValue visitObjectAsAbstractData(Type type) throws IOException {
if (explicitConstructorNames || explicitDataTypes) {
String consName = null;
String typeName = null; // this one is optional, and the order with cons is not defined.
String consLabel = in.nextName();
String consLabel = nextName();

// first we read either a cons name or a type name
if (explicitConstructorNames && "_constructor".equals(consLabel)) {
Expand All @@ -547,14 +570,14 @@ else if (explicitDataTypes && "_type".equals(consLabel)) {
// optionally read the second field
if (explicitDataTypes && typeName == null) {
// we've read a constructor name, but we still need a type name
consLabel = in.nextName();
consLabel = nextName();
if (explicitDataTypes && "_type".equals(consLabel)) {
typeName = in.nextString();
}
}
else if (explicitDataTypes && consName == null) {
// we've read type name, but we still need a constructor name
consLabel = in.nextName();
consLabel = nextName();
if (explicitDataTypes && "_constructor".equals(consLabel)) {
consName = in.nextString();
}
Expand Down Expand Up @@ -599,9 +622,9 @@ else if (alternatives.size() == 0) {
}

while (in.hasNext()) {
String label = in.nextName();
String label = nextName();
if (cons.hasField(label)) {
IValue val = read(in, cons.getFieldType(label));
IValue val = cons.getFieldType(label).accept(this);
if (val != null) {
args[cons.getFieldIndex(label)] = val;
}
Expand All @@ -611,7 +634,7 @@ else if (alternatives.size() == 0) {
}
else if (cons.hasKeywordField(label, store)) {
if (!isNull()) { // lookahead for null to give default parameters the preference.
IValue val = read(in, store.getKeywordParameterType(cons, label));
IValue val = store.getKeywordParameterType(cons, label).accept(this);
// null can still happen if the nulls map doesn't have a default
if (val != null) {
// if the value is null we'd use the default value of the defined field in the constructor
Expand Down Expand Up @@ -688,7 +711,7 @@ public IValue visitAbstractData(Type type) throws IOException {

@Override
public IValue visitConstructor(Type type) throws IOException {
return read(in, type.getAbstractDataType());
return type.getAbstractDataType().accept(this);
}

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

if (kwName.equals("_name")) {
name = ((IString) read(in, TF.stringType())).getValue();
name = ((IString) TF.stringType().accept(this)).getValue();
continue;
}

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

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

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

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

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

@Override
public IValue visitNumber(Type type) throws IOException {
if (isNull()) {
return inferNullValue(nulls, type);
}

if (in.peek() == JsonToken.BEGIN_ARRAY) {
return visitRational(type);
}
Expand Down Expand Up @@ -802,7 +830,13 @@ public IValue visitList(Type type) throws IOException {
in.beginArray();
while (in.hasNext()) {
// here we pass label from the higher context
w.append(read(in, type.getElementType()));
IValue elem = isNull()
? inferNullValue(nulls, type.getElementType())
: type.getElementType().accept(this);

if (elem != null) {
w.append(elem);
}
}

in.endArray();
Expand All @@ -818,7 +852,13 @@ public IValue visitSet(Type type) throws IOException {
in.beginArray();
while (in.hasNext()) {
// here we pass label from the higher context
w.insert(read(in, type.getElementType()));
IValue elem = isNull()
? inferNullValue(nulls, type.getElementType())
: type.getElementType().accept(this);

if (elem != null) {
w.insert(elem);
}
}

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

try {
return expected.accept(dispatch);
var result = expected.accept(dispatch);
if (result == null) {
throw new JsonParseException("null occurred outside an optionality context and without a registered representation.");
}
return result;
}
catch (EOFException | JsonParseException | NumberFormatException | MalformedJsonException | IllegalStateException | NullPointerException e) {
throw dispatch.parseErrorHere(e.getMessage());
Expand Down
23 changes: 23 additions & 0 deletions src/org/rascalmpl/library/lang/rascal/tests/library/lang/json/JSONIOTests.rsc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,29 @@ test bool dealWithNull() {
// the builtin Maybe interpreter with non-null
assert parseJSON(#map[str,Maybe[str]], "{\"bla\": \"foo\"}") == ("bla":just("foo"));

// test different specific nulls for different expected types:
for (t <- defaultJSONNULLValues<0>) {
assert parseJSON(t, "null") == (defaultJSONNULLValues[t]?"default-not-found");
}

assert parseJSON(#list[int], "[1,null,2]", nulls=()) == [1, 2];
assert parseJSON(#list[int], "[1,null,2]") == [1, defaultJSONNULLValues[#int], 2];
assert parseJSON(#set[int], "[1,null,2]", nulls=()) == {1, 2};
assert parseJSON(#set[int], "[1,null,2]") == {1, defaultJSONNULLValues[#int], 2};

try {
assert parseJSON(#tuple[int,int], "[null,null]", nulls=()) == [];
}
catch ParseError(_):
assert true;

// test undefined top-level null
try {
parseJSON(#int, "null", nulls=());
assert false;
}
catch ParseError(_): assert true;

// keyword parameters and null
assert cons(bla="foo") := parseJSON(#Cons, "{\"bla\": \"foo\"}");
assert cons() := parseJSON(#Cons, "{\"bla\": null}");
Expand Down

0 comments on commit 79b6b00

Please sign in to comment.