diff --git a/WDL/StdLib.py b/WDL/StdLib.py index 2811d60b..4ece08d4 100644 --- a/WDL/StdLib.py +++ b/WDL/StdLib.py @@ -5,6 +5,7 @@ import tempfile from typing import List, Tuple, Callable, BinaryIO, Optional from abc import ABC, abstractmethod +from contextlib import suppress import regex from . import Type, Value, Expr, Env, Error from ._util import byte_size_units, chmod_R_plus @@ -444,8 +445,8 @@ def infer_type(self, expr: "Expr.Apply") -> Type.Base: rhs, lhs.type.item_type[0], rhs.type, "Map key" ) from None return lhs.type.item_type[1] - if isinstance(lhs.type, Type.Any): - # e.g. read_json(): assume lhs is Array[Any] or Map[String,Any] + if isinstance(lhs.type, Type.Any) and not lhs.type.optional: + # e.g. read_json(): assume lhs is Array[Any] or Struct return Type.Any() raise Error.NotAnArray(lhs) @@ -465,6 +466,15 @@ def _call_eager(self, expr: "Expr.Apply", arguments: List[Value.Base]) -> Value. if ans is None: raise Error.OutOfBounds(expr.arguments[1], "Map key not found") return ans + elif isinstance(lhs, Value.Struct): + # allow member access from read_json() (issue #320) + key = None + if rhs.type.coerces(Type.String()): + with suppress(Error.RuntimeError): + key = rhs.coerce(Type.String()).value + if key is None or key not in lhs.value: + raise Error.OutOfBounds(expr.arguments[1], "struct member not found") + return lhs.value[key] else: lhs = lhs.coerce(Type.Array(Type.Any())) rhs = rhs.coerce(Type.Int()) diff --git a/WDL/Type.py b/WDL/Type.py index 0b77ad9e..1d339c6a 100644 --- a/WDL/Type.py +++ b/WDL/Type.py @@ -522,9 +522,9 @@ def unify(types: List[Base], check_quant: bool = True, force_string: bool = Fals if not types: return Any() - # begin with first type; or if --no-quant-check, the first array type (as we can try to promote - # other T to Array[T]) - t = next((t for t in types if not isinstance(t, Any)), types[0]) + # begin with first non-String type (as almost everything is coercible to string); or if + # --no-quant-check, the first array type (as we can try to promote other T to Array[T]) + t = next((t for t in types if not isinstance(t, (String, Any))), types[0]) if not check_quant: t = next((a for a in types if isinstance(a, Array) and not isinstance(a.item_type, Any)), t) t = t.copy() # pyre-ignore @@ -554,13 +554,16 @@ def unify(types: List[Base], check_quant: bool = True, force_string: bool = Fals t = Float() if isinstance(t, String) and isinstance(t2, File): t = File() + if isinstance(t, String) and isinstance(t2, Directory): + t = Directory() # String if ( isinstance(t2, String) - and not isinstance(t2, File) - and not isinstance(t, File) + and not isinstance(t2, (File, Directory)) + and not isinstance(t, (File, Directory)) and (not check_quant or not isinstance(t, Array)) + and (not isinstance(t, (Pair, Map))) ): t = String() if not t2.coerces(String(optional=True), check_quant=check_quant): diff --git a/WDL/Value.py b/WDL/Value.py index 48dad92f..f392dda4 100644 --- a/WDL/Value.py +++ b/WDL/Value.py @@ -13,6 +13,7 @@ import hashlib from abc import ABC from typing import Any, List, Optional, Tuple, Dict, Iterable, Union, Callable +from contextlib import suppress from . import Error, Type, Env @@ -185,9 +186,8 @@ def coerce(self, desired_type: Optional[Type.Base] = None) -> Base: if isinstance(desired_type, Type.Float): return Float(float(self.value), self.expr) except ValueError as exn: - if self.expr: - raise Error.EvalError(self.expr, "coercing String to number: " + str(exn)) from exn - raise + msg = f"coercing String to {desired_type}: {exn}" + raise Error.EvalError(self.expr, msg) if self.expr else Error.RuntimeError(msg) return super().coerce(desired_type) @@ -325,13 +325,16 @@ def coerce(self, desired_type: Optional[Type.Base] = None) -> Base: ) if isinstance(desired_type, Type.StructInstance): if self.type.item_type[0] == Type.String(): - # Runtime typecheck for initializing struct from read_{object[s],map,json} + # Runtime typecheck for initializing struct from read_{object,objects,map} # This couldn't have been checked statically because the map keys weren't known. litty = Type.Map( self.type.item_type, self.type.optional, set(kv[0].value for kv in self.value) ) if not litty.coerces(desired_type): - msg = "runtime struct initializer doesn't have the required fields with the expected types" + msg = ( + "unusable runtime struct initializer" + " (member type mismatch, lacking required member, or extra member)" + ) raise Error.EvalError( self.expr, msg, @@ -340,8 +343,17 @@ def coerce(self, desired_type: Optional[Type.Base] = None) -> Base: ans = {} for k, v in self.value: k = k.coerce(Type.String()).value - assert k in desired_type.members - ans[k] = v + try: + ans[k] = v.coerce(desired_type.members[k]) + except Error.RuntimeError as exc: + msg = ( + "runtime type mismatch initializing struct member " + f"{str(desired_type.members[k])} {k}" + ) + raise Error.EvalError( + self.expr, + msg, + ) if self.expr else Error.RuntimeError(msg) return Struct(desired_type, ans, self.expr) return super().coerce(desired_type) @@ -395,7 +407,7 @@ class Null(Base): ``type`` and ``value`` are both None.""" def __init__(self, expr: "Optional[Expr.Base]" = None) -> None: - super().__init__(Type.Any(optional=True), None, expr) + super().__init__(Type.Any(null=True), None, expr) def coerce(self, desired_type: Optional[Type.Base] = None) -> Base: """""" @@ -430,26 +442,86 @@ def __init__( expr: "Optional[Expr.Base]" = None, ) -> None: value = dict(value) - if isinstance(type, Type.StructInstance): - assert type.members + if isinstance(type, (Type.Object, Type.StructInstance)): + type_members = type.members + assert type_members # coerce values to member types for k in value: - assert k in type.members - value[k] = value[k].coerce(type.members[k]) - # if initializer (map or object literal) omits optional members, - # fill them in with null - for k in type.members: + try: + value[k] = value[k].coerce(type_members[k]) + except Error.RuntimeError: + msg = ( + f"runtime type mismatch initializing struct member" + f" {str(type_members[k])} {k}" + ) + raise Error.EvalError( + expr, + msg, + ) if expr else Error.RuntimeError(msg) + # if initializer omits optional members, fill them in with null + for k in type_members: if k not in value: - assert type.members[k].optional + assert type_members[k].optional value[k] = Null() self.value = value super().__init__(type, value, expr) def coerce(self, desired_type: Optional[Type.Base] = None) -> Base: """""" - if isinstance(self.type, Type.Object) and isinstance(desired_type, Type.StructInstance): + if isinstance(desired_type, (Type.Object, Type.StructInstance)): + if not self.type.coerces(desired_type): + msg = ( + "unusable runtime struct initializer" + " (member type mismatch, lacking required member, or extra member)" + ) + raise Error.EvalError( + self.expr, + msg, + ) if self.expr else Error.RuntimeError(msg) return Struct(desired_type, self.value, self.expr) - return self + if isinstance(desired_type, Type.Map): + return self._coerce_to_map(desired_type) + if isinstance(desired_type, Type.Any): + return self + msg = f"cannot coerce struct to {desired_type}" + raise Error.EvalError( + self.expr, + msg, + ) if self.expr else Error.RuntimeError(msg) + + def _coerce_to_map(self, desired_type: Type.Map) -> Map: + # runtime coercion e.g. Map[String,String] foo = read_json("foo.txt") + assert isinstance(self.type, Type.Object) + + def fail(msg): + raise Error.EvalError( + self.expr, + msg, + ) if self.expr else Error.RuntimeError(msg) + + key_type = desired_type.item_type[0] + if not Type.String().coerces(key_type): + fail(f"cannot coerce member names to {key_type} map keys") + value_type = desired_type.item_type[1] + entries = [] + for k, v in self.value.items(): + if not (isinstance(v, Null) and value_type.optional): + map_key = None + try: + map_key = String(k).coerce(key_type) + except Error.RuntimeError: + fail(f"cannot coerce member name {k} to {key_type} map key") + map_value = None + if self.type.members[k].coerces(value_type): + with suppress(Error.RuntimeError): + map_value = v.coerce(value_type) + if map_value is None: + fail( + "cannot coerce struct member" + f" {self.type.members[k]} {k} to {value_type} map value" + ) + entries.append((map_key, map_value)) + return Map(desired_type.item_type, entries) def __str__(self) -> Any: return "{" + ", ".join(f"{k}: {str(v)}" for k, v in self.value.items()) + "}" @@ -552,9 +624,13 @@ def _infer_from_json(j: Any) -> Base: item_type = Type.unify([item.type for item in items]) return Array(item_type, [item.coerce(item_type) for item in items]) if isinstance(j, dict): - items = [(String(str(k)), _infer_from_json(j[k])) for k in j] - value_type = Type.unify([v.type for _, v in items]) - return Map((Type.String(), value_type), [(k, v.coerce(value_type)) for k, v in items]) + members = {} + member_types = {} + for k in j: + assert isinstance(k, str) + members[k] = _infer_from_json(j[k]) + member_types[k] = members[k].type + return Struct(Type.Object(member_types), members) raise Error.InputError(f"couldn't construct value from: {json.dumps(j)}") diff --git a/tests/test_0eval.py b/tests/test_0eval.py index 49613d52..5c4d8966 100644 --- a/tests/test_0eval.py +++ b/tests/test_0eval.py @@ -484,7 +484,6 @@ def test_json(self): (WDL.Type.Array(WDL.Type.String(optional=True)), ["apple", "orange", None]), (WDL.Type.Map((WDL.Type.String(), WDL.Type.Int())), {"cats": 42, "dogs": 99}), (pty, {"name": "Alyssa", "age": 42, "pets": None}), - (pty, {"name": "Alyssa", "age": 42}), (pty, {"name": "Alyssa", "age": 42, "pets": {"cats": 42, "dogs": 99}}), (WDL.Type.Array(WDL.Type.Pair(WDL.Type.String(), WDL.Type.Int())), [{"left": "a", "right": 0},{"left": "b", "right": 1}]), diff --git a/tests/test_4taskrun.py b/tests/test_4taskrun.py index 9bc17683..0f8afdf3 100644 --- a/tests/test_4taskrun.py +++ b/tests/test_4taskrun.py @@ -578,6 +578,23 @@ def test_coercion(self): """) self.assertEqual(outputs["car"], {"model": "Mazda", "year": 2017, "mileage": None}) self.assertEqual(outputs["car2"], {"model": "Toyota", "year": None, "mileage": None}) + # bad struct init from map + self._test_task(R""" + version 1.0 + struct Car { + String model + Float mileage + } + task t { + command {} + output { + Car car = { + "model": "Mazda", + "mileage": "bogus" + } + } + } + """, expected_exception=WDL.Error.EvalError) def test_errors(self): self._test_task(R""" @@ -794,10 +811,11 @@ def test_runtime_memory_limit(self): String memory } command <<< - cat /sys/fs/cgroup/memory/memory.limit_in_bytes + cat /sys/fs/cgroup/memory/memory.limit_in_bytes \ + || cat /sys/fs/cgroup/memory.max >>> output { - Int memory_limit_in_bytes = read_int(stdout()) + String memory_limit_in_bytes = read_string(stdout()) } runtime { cpu: 1 @@ -807,10 +825,11 @@ def test_runtime_memory_limit(self): """ cfg = WDL.runtime.config.Loader(logging.getLogger(self.id()), []) outputs = self._test_task(txt, {"memory": "256MB"}, cfg=cfg) - self.assertGreater(outputs["memory_limit_in_bytes"], 300*1024*1024) + if outputs["memory_limit_in_bytes"] != "max": + self.assertGreater(int(outputs["memory_limit_in_bytes"]), 300*1024*1024) cfg.override({"task_runtime": {"memory_limit_multiplier": 0.9}}) outputs = self._test_task(txt, {"memory": "256MB"}, cfg=cfg) - self.assertLess(outputs["memory_limit_in_bytes"], 300*1024*1024) + self.assertLess(int(outputs["memory_limit_in_bytes"]), 300*1024*1024) def test_runtime_returnCodes(self): txt = R""" diff --git a/tests/test_5stdlib.py b/tests/test_5stdlib.py index e96bb9a8..235832fe 100644 --- a/tests/test_5stdlib.py +++ b/tests/test_5stdlib.py @@ -1,3 +1,4 @@ +from math import exp import unittest import logging import tempfile @@ -550,7 +551,7 @@ def test_read_json(self): Array[String] my_array = read_json(stdout()) } } - """, expected_exception=WDL.Error.InputError) + """, expected_exception=WDL.Error.EvalError) outputs = self._test_task(R""" version 1.0 @@ -577,6 +578,18 @@ def test_read_json(self): } """, expected_exception=WDL.Error.InputError) + self._test_task(R""" + version 1.0 + task test { + command <<< + echo '{"foo":"bar"}' + >>> + output { + String baz = read_json(stdout())["baz"] + } + } + """, expected_exception=WDL.Error.OutOfBounds) + def test_read_map_ints(self): outputs = self._test_task(R""" version 1.0 @@ -675,6 +688,205 @@ def test_struct_from_read(self): self.assertEqual(outputs["alice"], alice) self.assertEqual(outputs["samplesheet2"], samplesheet2) + def test_issue524(self): + # additional cases for struct initialization from read_json(), motivated by issue #524 + + # explicit null value should be acceptable initializer for optional struct field + outp = self._test_task(R""" + version 1.0 + + struct MyStruct { + Int x + String? y + } + + task mytask { + input { + } + + command <<< + cat > data.json <>> + + output { + MyStruct data = read_json("data.json") + } + } + """) + self.assertEqual(outp["data"], {"x": 123, "y": None}) + # elaboration with a heterogeneous unification: + outp = self._test_task(R""" + version 1.0 + + struct MyStruct { + Float x + String? y + Array[Int?] z + } + + task mytask { + input { + } + + command <<< + cat > data.json <>> + + output { + MyStruct data = read_json("data.json") + } + } + """) + self.assertEqual(outp["data"], {"x": 3.14159, "y": None, "z": [4,2,None]}) + # unusable null + self._test_task(R""" + version 1.0 + + struct MyStruct { + Float x + String? y + Array[Int?] z + } + + task mytask { + input { + } + + command <<< + cat > data.json <>> + + output { + MyStruct data = read_json("data.json") + } + } + """, expected_exception=WDL.Error.EvalError) + # top-level null + outp = self._test_task(R""" + version 1.0 + + struct MyStruct { + Float x + String? y + Array[Int?] z + } + + task mytask { + input { + } + + command <<< + echo null > data.json + >>> + + output { + MyStruct? data = read_json("data.json") + } + } + """) + self.assertEqual(outp, {"data": None}) + # coercion failure -- required member missing + self._test_task(R""" + version 1.0 + + struct MyStruct { + Float x + String? y + Array[Int?] z + } + + task mytask { + input { + } + + command <<< + cat > data.json <>> + + output { + MyStruct data = read_json("data.json") + } + } + """, expected_exception=WDL.Error.EvalError) + # bad coercion to Map (key type) + self._test_task(R""" + version 1.0 + + struct MyStruct { + Float x + String? y + Array[Int?] z + } + + task mytask { + input { + } + + command <<< + cat > data.json <>> + + output { + Map[Float,String] data = read_json("data.json") + } + } + """, expected_exception=WDL.Error.EvalError) + # bad coercion to Map (value type) + self._test_task(R""" + version 1.0 + + struct MyStruct { + Float x + String? y + Array[Int?] z + } + + task mytask { + input { + } + + command <<< + cat > data.json <>> + + output { + Map[String,Float] data = read_json("data.json") + } + } + """, expected_exception=WDL.Error.EvalError) + def test_bad_object(self): self._test_task(R""" version 1.0