Skip to content

Commit

Permalink
Merge 41e5813 into dfd2245
Browse files Browse the repository at this point in the history
  • Loading branch information
mlin committed Sep 13, 2020
2 parents dfd2245 + 41e5813 commit 56e8a5e
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 11 deletions.
9 changes: 7 additions & 2 deletions WDL/Expr.py
Original file line number Diff line number Diff line change
Expand Up @@ -609,10 +609,15 @@ def _eval(
) -> Value.Base:
""
assert isinstance(self.type, Type.Map)
keystrs = set()
eitems = []
for k, v in self.items:
eitems.append((k.eval(env, stdlib), v.eval(env, stdlib)))
# TODO: complain of duplicate keys
ek = k.eval(env, stdlib)
sk = str(ek)
if sk in keystrs:
raise Error.EvalError(self, "duplicate keys in Map literal")
eitems.append((ek, v.eval(env, stdlib)))
keystrs.add(sk)
return Value.Map(self.type.item_type, eitems)

@property
Expand Down
22 changes: 22 additions & 0 deletions WDL/Tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,8 @@ def typecheck(
Type.String()
)
)
for b in self.available_inputs:
errors.try1(lambda: _check_serializable_map_keys(b.value.type, b.name, b.value))
# Typecheck runtime expressions
for _, runtime_expr in self.runtime.items():
errors.try1(
Expand All @@ -405,6 +407,7 @@ def typecheck(
errors.try1(
lambda: decl.typecheck(type_env, stdlib=stdlib, check_quant=check_quant)
)
errors.try1(lambda: _check_serializable_map_keys(decl.type, decl.name, decl))

# check for cyclic dependencies among decls
_detect_cycles(
Expand Down Expand Up @@ -1026,6 +1029,8 @@ def typecheck(self, doc: "Document", check_quant: bool) -> None:
)
if errors.try1(lambda: _typecheck_workflow_body(doc, check_quant)) is False:
self.complete_calls = False
for b in self.available_inputs:
errors.try1(lambda: _check_serializable_map_keys(b.value.type, b.name, b.value))
# 4. convert deprecated output_idents, if any, to output declarations
if self._output_idents:
self._rewrite_output_idents()
Expand Down Expand Up @@ -1062,6 +1067,9 @@ def typecheck(self, doc: "Document", check_quant: bool) -> None:
)
)
output_type_env = output_type_env2
errors.try1(
lambda: _check_serializable_map_keys(output.type, output.name, output)
)
# 6. check for cyclic dependencies
_detect_cycles(_workflow_dependency_matrix(self))

Expand Down Expand Up @@ -1804,3 +1812,17 @@ def _add_struct_instance_to_type_env(
else:
ans = ans.bind(namespace + "." + member_name, member_type, ctx)
return ans


def _check_serializable_map_keys(t: Type.Base, name: str, node: SourceNode) -> None:
# For any Map[K,V] in an input or output declaration, K must be coercible to & from String, so
# that it can be de/serialized as JSON.
if isinstance(t, Type.Map):
kt = t.item_type[0]
if not kt.coerces(Type.String()) or not Type.String().coerces(kt):
raise Error.ValidationError(
node,
f"{str(t)} may not be used in input/output {name} because the keys cannot be written to/from JSON",
)
for p in t.parameters:
_check_serializable_map_keys(p, name, node)
10 changes: 7 additions & 3 deletions WDL/Value.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,8 @@ def coerce(self, desired_type: Optional[Type.Base] = None) -> Base:
return Array(
desired_type, [v.coerce(desired_type.item_type) for v in self.value], self.expr
)
if isinstance(desired_type, Type.String):
return String(json.dumps(self.json))
return super().coerce(desired_type)


Expand All @@ -270,8 +272,10 @@ def __str__(self) -> str:
def json(self) -> Any:
""
ans = {}
if not self.type.item_type[0].coerces(Type.String()):
msg = f"cannot write {str(self.type)} to JSON"
raise (Error.EvalError(self.expr, msg) if self.expr else Error.RuntimeError(msg))
for k, v in self.value:
assert k.type.coerces(Type.String())
kstr = k.coerce(Type.String()).value
if kstr not in ans:
ans[kstr] = v.json
Expand Down Expand Up @@ -460,13 +464,13 @@ def from_json(type: Type.Base, value: Any) -> Base:
)
if (
isinstance(type, Type.Map)
and type.item_type[0] == Type.String()
and Type.String().coerces(type.item_type[0])
and isinstance(value, dict)
):
items = []
for k, v in value.items():
assert isinstance(k, str)
items.append((from_json(type.item_type[0], k), from_json(type.item_type[1], v)))
items.append((String(k).coerce(type.item_type[0]), from_json(type.item_type[1], v)))
return Map(type.item_type, items)
if (
isinstance(type, Type.StructInstance)
Expand Down
4 changes: 2 additions & 2 deletions WDL/_grammar.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@
ESCAPED_STRING1: "'" STRING_INNER1* "'"
string_literal: ESCAPED_STRING | ESCAPED_STRING1
?map_key: literal | string
?map_key: expr_core
map_kv: map_key ":" expr
// expression core (everything but infix)
Expand Down Expand Up @@ -413,7 +413,7 @@
| expr_core "." CNAME -> get_name
| "object" "{" [object_kv ("," object_kv)* ","?] "}" -> obj
?map_key: literal | string
?map_key: expr_core
map_kv: map_key ":" expr
object_kv: CNAME ":" expr
Expand Down
15 changes: 11 additions & 4 deletions tests/test_0eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,10 @@ def _test_tuples(self, *tuples):
else:
ex = WDL.parse_expr(expr, version=version).infer_type(type_env)
v = ex.eval(env).expect(expected_type)
self.assertEqual(str(v), expected, str(expr))
if ex.literal:
self.assertEqual(str(ex.literal), expected)
if expected:
self.assertEqual(str(v), expected, str(expr))
if ex.literal:
self.assertEqual(str(ex.literal), expected)

def test_logic(self):
self._test_tuples(
Expand Down Expand Up @@ -334,10 +335,16 @@ def test_map(self):
("{'foo': 1, 'bar': 2, 'baz': 3.0}['bar']", "2.0", WDL.Type.Float()),
("{0: 1, 2: 3}[false]", "", WDL.Error.StaticTypeMismatch),
("{0: 1, 2: 3}['foo']", "", WDL.Error.EvalError),
("{0: 1, 0: 3}", "", WDL.Error.EvalError),
("{'foo': 1, 'bar': 2}[3]", "", WDL.Error.OutOfBounds), # int coerces to string...
("{3: 1, false: 2}", "", WDL.Error.IndeterminateType),
("{'foo': true, 'bar': 0,}", '{"foo": true, "bar": 0}', WDL.Type.Map((WDL.Type.String(), WDL.Type.String())))
("{'foo': true, 'bar': 0,}", '{"foo": true, "bar": 0}', WDL.Type.Map((WDL.Type.String(), WDL.Type.String()))),
("{[1,2]: true, []: false}", '{"[1, 2]": true, "[]": false}', WDL.Type.Map((WDL.Type.Array(WDL.Type.Int()), WDL.Type.Boolean()))),
("{[1]: true, [1]: false}", "", WDL.Error.EvalError),
("{(false, false): 0, (false, true): 1}", "", WDL.Type.Map((WDL.Type.Pair(WDL.Type.Boolean(), WDL.Type.Boolean()), WDL.Type.Int()))),
)
with self.assertRaisesRegex(WDL.Error.EvalError, "to JSON"):
WDL.parse_expr("{(false, false): 0, (false, true): 1}").infer_type(WDL.Env.Bindings()).eval(WDL.Env.Bindings()).json

def test_errors(self):
self._test_tuples(
Expand Down
20 changes: 20 additions & 0 deletions tests/test_1doc.py
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,26 @@ def test_unify(self):
""")
doc.typecheck()

def test_map_io(self):
with self.assertRaisesRegex(WDL.Error.ValidationError, "keys cannot"):
WDL.parse_document("""
workflow w {
input {
Map[Pair[Int,Int],String] m
}
}
""").typecheck()

with self.assertRaisesRegex(WDL.Error.ValidationError, "keys cannot"):
WDL.parse_document("""
task t {
command {}
output {
Map[Pair[Int,Int],String] m = read_json("bogus")
}
}
""").typecheck()

class TestDoc(unittest.TestCase):
def test_count_foo(self):
doc = r"""#foo
Expand Down
2 changes: 2 additions & 0 deletions tests/test_5stdlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -937,13 +937,15 @@ def test_keys(self):
Array[Int] k2 = keys(m2)
Array[Int] k3 = keys(m3)
Array[Boolean] k4 = keys({})
Array[Pair[Int,Boolean]] k5 = keys({(1,false): "foo", (3,true): "bar"})
}
}
""")
self.assertEqual(outputs["k1"], ["a", "c"])
self.assertEqual(outputs["k2"], [1,-1])
self.assertEqual(outputs["k3"], [])
self.assertEqual(outputs["k4"], [])
self.assertEqual(outputs["k5"], [{"left": 1, "right": False}, {"left": 3, "right": True}])

with self.assertRaises(WDL.Error.StaticTypeMismatch):
self._test_task(R"""
Expand Down

0 comments on commit 56e8a5e

Please sign in to comment.