Merge a970de2 into aa21f1a

WDL/Error.py

@@ -72,7 +72,7 @@ class StaticTypeMismatch(Base):
     def __init__(self, node : SourceNode, expected : T.Base, actual : T.Base, message : Optional[str] = None) -> None:
         msg = "Expected {} instead of {}".format(str(expected), str(actual))
         if message is not None:
-            msg = msg + "; " + message
+            msg = msg + " " + message
         super().__init__(node, msg)
 
 class IncompatibleOperand(Base):

WDL/Expr.py

@@ -195,24 +195,25 @@ def _infer_type(self, type_env : Env.Types) -> T.Base:
         for item in self.items:
             item.infer_type(type_env)
         # Start by assuming the type of the first item is the item type
-        item_type = self.items[0].type
+        item_type : T.Base = self.items[0].type
         # Allow a mixture of Int and Float to construct Array[Float]
         if isinstance(item_type, T.Int):
             for item in self.items:
                 if isinstance(item.type, T.Float):
                     item_type = T.Float()
         # If any item is String, assume item type is String
+        # If any item has optional type, assume item type is optional
         for item in self.items:
             if isinstance(item.type, T.String):
-                item_type = T.String()
+                item_type = T.String(optional=item_type.optional)
+            if item.type.optional:
+                item_type = item_type.copy(optional=True)
         # Check all items are coercible to item_type
         for item in self.items:
             try:
                 item.typecheck(item_type)
             except Error.StaticTypeMismatch:
                 raise Error.StaticTypeMismatch(self, item_type, item.type, "inconsistent types within array") from None
-            if item.type.optional:
-                item_type.optional = True
         return T.Array(item_type, False, True)
 
     def typecheck(self, expected : Optional[T.Base]) -> Base:
@@ -251,8 +252,10 @@ def _infer_type(self, type_env : Env.Types) -> T.Base:
         self_type = self.consequent.infer_type(type_env).type
         assert isinstance(self_type, T.Base)
         self.alternative.infer_type(type_env)
+        if self.alternative.type.optional:
+            self_type = self_type.copy(optional=True)
         if isinstance(self_type, T.Int) and isinstance(self.alternative.type, T.Float):
-            self_type = T.Float()
+            self_type = T.Float(optional=self_type.optional)
         try:
             self.alternative.typecheck(self_type)
         except Error.StaticTypeMismatch:

WDL/StdLib.py

@@ -5,7 +5,6 @@
 import WDL.Expr as E
 import WDL.Env as Env
 import WDL.Error as Error
-import copy
 
 # Special function for array access arr[index], returning the element type
 #                      or map access map[key], returning the value type
@@ -112,7 +111,7 @@ def infer_type(self, expr : E.Apply) -> T.Base:
             try:
                 expr.arguments[i].typecheck(self.argument_types[i])
             except Error.StaticTypeMismatch:
-                raise Error.StaticTypeMismatch(expr.arguments[i], self.argument_types[i], expr.arguments[i].type, "{} argument #{}".format(self.name, i+1)) from None
+                raise Error.StaticTypeMismatch(expr.arguments[i], self.argument_types[i], expr.arguments[i].type, "for {} argument #{}".format(self.name, i+1)) from None
         return self.return_type
 
     def __call__(self, expr : E.Apply, env : E.Env) -> V.Base:
@@ -128,7 +127,12 @@ def __call__(self, expr : E.Apply, env : E.Env) -> V.Base:
     ("_rem", [T.Int(), T.Int()], T.Int(), lambda l,r: V.Int(l.value % r.value)), # pyre-fixme
     ("stdout", [], T.File(), lambda: exec('raise NotImplementedError()')),
     ("basename", [T.String(), T.String(optional=True)], T.String(), lambda file: exec('raise NotImplementedError()')),
-    ("size", [T.File(), T.String(optional=True)], T.Float(), lambda file: exec('raise NotImplementedError()')),
+    # TODO: size() argument is optional to admit a pattern seen in the test corpi:
+    #         if (defined(f)) then size(f) else 100
+    #       unclear how this should apply generaly to functions other than size().
+    #       alternatively, during typechecking, we could infer that the f can't
+    #       be null in the consequent branch specifically.
+    ("size", [T.File(optional=True), T.String(optional=True)], T.Float(), lambda file: exec('raise NotImplementedError()')),
     ("ceil", [T.Float()], T.Int(), lambda x: exec('raise NotImplementedError()')),
     ("round", [T.Float()], T.Int(), lambda x: exec('raise NotImplementedError()')),
     ("glob", [T.String()], T.Array(T.File()), lambda pattern: exec('raise NotImplementedError()')),
@@ -203,7 +207,7 @@ def infer_type(self, expr : E.Apply) -> T.Base:
         if t2 is None:
             # neither operand is a string; defer to _ArithmeticOperator
             return super().infer_type(expr)
-        if not t2.coerces(T.String()):
+        if not t2.coerces(T.String(optional=True)):
             raise Error.IncompatibleOperand(expr, "Cannot add/concatenate {} and {}".format(str(expr.arguments[0].type), str(expr.arguments[1].type)))
         return T.String()
 
@@ -285,10 +289,9 @@ def infer_type(self, expr : E.Apply) -> T.Base:
             raise Error.StaticTypeMismatch(expr.arguments[0], T.Array(None), expr.arguments[0].type)
         if expr.arguments[0].type.item_type is None:
             raise Error.EmptyArray(expr.arguments[0]) # TODO: error for 'indeterminate type'
-        ty = copy.copy(expr.arguments[0].type.item_type)
+        ty = expr.arguments[0].type.item_type
         assert isinstance(ty, T.Base)
-        ty.optional = False
-        return ty
+        return ty.copy(optional=False)
 
     def __call__(self, expr : E.Apply, env : Env.Values) -> V.Base:
         raise NotImplementedError()
@@ -302,10 +305,9 @@ def infer_type(self, expr : E.Apply) -> T.Base:
             raise Error.StaticTypeMismatch(expr.arguments[0], T.Array(None), expr.arguments[0].type)
         if expr.arguments[0].type.item_type is None:
             raise Error.EmptyArray(expr.arguments[0]) # TODO: error for 'indeterminate type'
-        ty = copy.copy(expr.arguments[0].type.item_type)
+        ty = expr.arguments[0].type.item_type
         assert isinstance(ty, T.Base)
-        ty.optional = False
-        return T.Array(ty)
+        return T.Array(ty.copy(optional=False))
 
     def __call__(self, expr : E.Apply, env : Env.Values) -> V.Base:
         raise NotImplementedError()

WDL/Tree.py

@@ -11,7 +11,7 @@
 import WDL.Env as Env
 import WDL.Error as Err
 from WDL.Error import SourcePosition, SourceNode
-import copy, os, errno
+import os, errno
 import WDL._parser
 
 class Decl(SourceNode):
@@ -87,9 +87,16 @@ def required_inputs(self) -> List[Decl]:
 # type-check a declaration within a type environment, and return the type
 # environment with the new binding
 def _typecheck_decl(decl : Decl, type_env : Env.Types) -> Env.Types:
+    # subtlety: in a declaration like: String? x = "who"
+    # we record x in the type environment as String instead of String?
+    # since it can't actually be null at runtime
+    nonnull = False
     if decl.expr is not None:
         decl.expr.infer_type(type_env).typecheck(decl.type)
-    ans : Env.Types = Env.bind(decl.name, decl.type, type_env)
+        if decl.expr.type.optional is False:
+            nonnull = True
+    ty = decl.type.copy(optional=False) if nonnull else decl.type
+    ans : Env.Types = Env.bind(decl.name,ty, type_env)
     return ans
 
 # forward-declaration of Document and Workflow types
@@ -158,7 +165,10 @@ def typecheck(self, type_env : Env.Types, doc : TVDocument) -> Env.Types:
                         decl = ele
             if decl is None:
                 raise Err.NoSuchInput(expr, name)
-            expr.infer_type(type_env).typecheck(decl.type)
+            try:
+                expr.infer_type(type_env).typecheck(decl.type)
+            except Err.StaticTypeMismatch as exn:
+                raise Err.StaticTypeMismatch(expr, decl.type, expr.type, "for input " + decl.name) from None
             if name in required_inputs:
                 required_inputs.remove(name)
 
@@ -188,8 +198,7 @@ def _optionalize_types(type_env : Env.Types) -> Env.Types:
     ans = []
     for node in type_env:
         if isinstance(node, Env.Binding):
-            ty = copy.copy(node.rhs)
-            ty.optional = True
+            ty = node.rhs.copy(optional=True)
             ans.append(Env.Binding(node.name, ty))
         elif isinstance(node, Env.Namespace):
             ans.append(Env.Namespace(node.namespace, _optionalize_types(node.bindings)))

WDL/Type.py

@@ -24,6 +24,7 @@
 """
 from abc import ABC, abstractmethod
 from typing import Optional, TypeVar, Tuple
+import copy
 
 TVBase = TypeVar("TVBase", bound="Base")
 class Base(ABC):
@@ -35,55 +36,72 @@ class Base(ABC):
         assert isinstance(WDL.Type.Array(WDL.Type.Int()), WDL.Type.Base)
     """
 
-    optional : bool
-    """True in declarations with the optional quantifier, ``Type?``"""
+    _optional : bool # immutable!!!
 
     def coerces(self, rhs : TVBase) -> bool:
-        """True if ``rhs`` is the same type, or can be coerced to, ``self``. Optional/nonempty quantifiers are disregarded for this purpose."""
+        """
+        True if ``rhs`` is the same type, or can be coerced to, ``self``.
+
+        ``T`` coerces to ``Array[T]`` (an array of length 1).
+
+        ``T`` coerces to ``T?`` but the reverse is not true in general.
+        """
         if isinstance(rhs, Array) and rhs.item_type == self: # coerce T to Array[T]
             return True
-        return (self == rhs)
+        return (type(self).__name__ == type(rhs).__name__) and (not self.optional or rhs.optional)
+
+    @property
+    def optional(self) -> bool:
+        """True in declarations with the optional quantifier, ``T?``"""
+        return self._optional
+
+    def copy(self, optional : Optional[bool] = None) -> TVBase:
+        """Return a copy of the type with a different setting of the ``optional`` quantifier."""
+        ans : Base = copy.copy(self)
+        if optional is not None:
+            ans._optional = optional
+        return ans
 
     def __str__(self) -> str:
-        return type(self).__name__
+        return type(self).__name__ + ('?' if self.optional else '')
     def __eq__(self, rhs) -> bool:
         return isinstance(rhs,Base) and str(self) == str(rhs)
 
 class Boolean(Base):
     def __init__(self, optional : bool = False) -> None:
-        self.optional = optional
+        self._optional = optional
     def coerces(self, rhs : Base) -> bool:
         if isinstance(rhs, String):
             return True
         return super().coerces(rhs)
 
 class Float(Base):
     def __init__(self, optional : bool = False) -> None:
-        self.optional = optional
+        self._optional = optional
     def coerces(self, rhs : Base) -> bool:
         if isinstance(rhs, String):
             return True
         return super().coerces(rhs)
 
 class Int(Base):
     def __init__(self, optional : bool = False) -> None:
-        self.optional = optional
+        self._optional = optional
     def coerces(self, rhs : Base) -> bool:
         if isinstance(rhs, Float) or isinstance(rhs, String):
             return True
         return super().coerces(rhs)
 
 class File(Base):
     def __init__(self, optional : bool = False) -> None:
-        self.optional = optional
+        self._optional = optional
     def coerces(self, rhs : Base) -> bool:
         if isinstance(rhs, String):
             return True
         return super().coerces(rhs)
 
 class String(Base):
     def __init__(self, optional : bool = False) -> None:
-        self.optional = optional
+        self._optional = optional
     def coerces(self, rhs : Base) -> bool:
         if isinstance(rhs, File):
             return True
@@ -99,16 +117,16 @@ class Array(Base):
     statically coercible to any array type (but may fail at runtime)
     """
     item_type : Optional[Base]
-    nonempty : bool
+    _nonempty : bool
     """True in declarations with the nonempty quantifier, ``Array[type]+``"""
 
     def __init__(self, item_type : Optional[Base], optional : bool = False, nonempty : bool = False) -> None:
         self.item_type = item_type
         assert isinstance(nonempty, bool)
-        self.optional = optional
-        self.nonempty = nonempty
+        self._optional = optional
+        self._nonempty = nonempty
     def __str__(self) -> str:
-        ans = "Array[" + (str(self.item_type) if self.item_type is not None else "") + "]"
+        ans = "Array[" + (str(self.item_type) if self.item_type is not None else "") + "]" + ('?' if self.optional else '')
         return ans
     def coerces(self, rhs : Base) -> bool:
         if isinstance(rhs, Array):
@@ -119,6 +137,14 @@ def coerces(self, rhs : Base) -> bool:
         if isinstance(rhs, String):
             return self.item_type is None or self.item_type.coerces(String())
         return super().coerces(rhs)
+    def copy(self, optional : Optional[bool] = None, nonempty : Optional[bool] = None) -> Base:
+        ans : Array = super().copy(optional)
+        if nonempty is not None:
+            ans._nonempty = nonempty
+        return ans
+    @property
+    def nonempty(self) -> bool:
+        return self._nonempty
 
 class Map(Base):
     """
@@ -132,10 +158,10 @@ class Map(Base):
     item_type : Optional[Tuple[Base,Base]]
 
     def __init__(self, item_type : Optional[Tuple[Base,Base]], optional : bool = False) -> None:
-        self.optional = optional
+        self._optional = optional
         self.item_type = item_type
     def __str__(self) -> str:
-        return "Map[" + (str(self.item_type[0]) + "," + str(self.item_type[1]) if self.item_type is not None else "") + "]" # pyre-fixme
+        return "Map[" + (str(self.item_type[0]) + "," + str(self.item_type[1]) if self.item_type is not None else "") + "]" + ('?' if self.optional else '') # pyre-fixme
     def coerces(self, rhs : Base) -> bool:
         if isinstance(rhs, Map):
             if self.item_type is None or rhs.item_type is None:
@@ -152,8 +178,8 @@ class Pair(Base):
     right_type : Base
 
     def __init__(self, left_type : Base, right_type : Base, optional : bool = False) -> None:
-        self.optional = optional
+        self._optional = optional
         self.left_type = left_type
         self.right_type = right_type
     def __str__(self) -> str:
-        return "Pair[" + (str(self.left_type) + "," + str(self.right_type)) + "]"
+        return "Pair[" + (str(self.left_type) + "," + str(self.right_type)) + "]" + ('?' if self.optional else '')

tests/test_2calls.py

@@ -74,3 +74,23 @@ def test_duplicate_input(self):
         """
         with self.assertRaises(WDL.Error.MultipleDefinitions):
             doc = WDL.parse_document(txt)
+
+    def test_optional(self):
+        txt = tsk + r"""
+        workflow contrived {
+            Int? x
+            call sum { input: x = x }
+        }
+        """
+        doc = WDL.parse_document(txt)
+        with self.assertRaises(WDL.Error.StaticTypeMismatch):
+            doc.typecheck()
+
+        txt = tsk + r"""
+        workflow contrived {
+            Int? x = 0
+            call sum { input: x = x }
+        }
+        """
+        doc = WDL.parse_document(txt)
+        doc.typecheck()