Merge 8f322f3 into e53e36c

WDL/Expr.py

@@ -78,14 +78,8 @@ def typecheck(self, expected: T.Base) -> TVBase:
         :raise WDL.Error.StaticTypeMismatch:
         :return: `self`
         """
-        expected2 = expected
-        if not self._check_quant:
-            if isinstance(expected, T.Array):
-                expected2 = expected.copy(nonempty=False, optional=True)
-            else:
-                expected2 = expected.copy(optional=True)
-        if not self.type.coerces(expected2):
-            raise Error.StaticTypeMismatch(self, expected2, self.type)
+        if not self.type.coerces(expected, self._check_quant):
+            raise Error.StaticTypeMismatch(self, expected, self.type)
         return self
 
     @abstractmethod

WDL/Lint.py

@@ -205,8 +205,9 @@ def call(self, obj: WDL.Tree.Call) -> Any:
 
 
 @a_linter
-class OptionalCoercion(Linter):
+class QuantityCoercion(Linter):
     # Expression of optional type where a non-optional value is expected
+    # or an array of possibly-empty type where a nonempty array is expected
     def expr(self, obj: WDL.Expr.Base) -> Any:
         if isinstance(obj, WDL.Expr.Apply):
             if obj.function_name in ["_add", "_sub", "_mul", "_div", "_land", "_lor"]:
@@ -231,48 +232,22 @@ def expr(self, obj: WDL.Expr.Base) -> Any:
                 F = WDL.Expr._stdlib[obj.function_name]
                 if isinstance(F, WDL.StdLib._StaticFunction):
                     for i in range(min(len(F.argument_types), len(obj.arguments))):
-                        if obj.arguments[i].type.optional and not F.argument_types[i].optional:
+                        if not obj.arguments[i].type.coerces(F.argument_types[i], True):
                             msg = "{} argument of {}() = :{}:".format(
                                 str(F.argument_types[i]), F.name, str(obj.arguments[i].type)
                             )
                             self.add(getattr(obj, "parent"), msg, obj.arguments[i])
 
     def decl(self, obj: WDL.Decl) -> Any:
-        if not obj.type.optional and obj.expr and obj.expr.type.optional:
-            self.add(obj, "{} {} = :{}:".format(str(obj.type), obj.name, str(obj.expr.type)))
-
-    def call(self, obj: WDL.Tree.Call) -> Any:
-        for name, inp_expr in obj.inputs.items():
-            decl = _find_input_decl(obj, name)
-            if not decl.type.optional and inp_expr.type.optional:
-                msg = "input {} {} = :{}:".format(str(decl.type), decl.name, str(inp_expr.type))
-                self.add(obj, msg, inp_expr)
-
-
-@a_linter
-class NonemptyArrayCoercion(Linter):
-    # Possibly empty array where a nonempty array is expected
-    def decl(self, obj: WDL.Decl) -> Any:
-        if (
-            isinstance(obj.type, WDL.Type.Array)
-            and obj.type.nonempty
-            and obj.expr
-            and isinstance(obj.expr.type, WDL.Type.Array)
-            and not obj.expr.type.nonempty
-        ):
+        if obj.expr and not obj.expr.type.coerces(obj.type, True):
             # heuristic exception for: Array[File]+ outp = glob(...)
             if not (isinstance(obj.expr, WDL.Expr.Apply) and obj.expr.function_name == "glob"):
                 self.add(obj, "{} {} = :{}:".format(str(obj.type), obj.name, str(obj.expr.type)))
 
     def call(self, obj: WDL.Tree.Call) -> Any:
         for name, inp_expr in obj.inputs.items():
             decl = _find_input_decl(obj, name)
-            if (
-                isinstance(decl.type, WDL.Type.Array)
-                and decl.type.nonempty
-                and isinstance(inp_expr.type, WDL.Type.Array)
-                and not inp_expr.type.nonempty
-            ):
+            if not inp_expr.type.coerces(decl.type, True):
                 msg = "input {} {} = :{}:".format(str(decl.type), decl.name, str(inp_expr.type))
                 self.add(obj, msg, inp_expr)
 

WDL/Type.py

@@ -55,14 +55,21 @@ class Base(ABC):
 
     _optional: bool  # immutable!!!
 
-    def coerces(self, rhs: TVBase) -> bool:
+    def coerces(self, rhs: TVBase, check_quant: bool = True) -> bool:
         """
         True if this is the same type as, or can be coerced to, ``rhs``.
+
+        :param check_quant: when ``False``, relaxes validation of the optional (?) and nonempty (+) type quantifiers
         """
-        if isinstance(rhs, Array) and self.coerces(rhs.item_type):
+        if isinstance(rhs, Array) and self.coerces(rhs.item_type, check_quant):
             # coerce T to Array[T]
             return True
-        return (type(self).__name__ == type(rhs).__name__) and (not self.optional or rhs.optional)
+        return (type(self).__name__ == type(rhs).__name__) and self._check_optional(
+            rhs, check_quant
+        )
+
+    def _check_optional(self, rhs: TVBase, check_quant: bool) -> bool:
+        return not (check_quant and (self.optional and not rhs.optional))
 
     @property
     def optional(self) -> bool:
@@ -95,55 +102,55 @@ class Boolean(Base):
     def __init__(self, optional: bool = False) -> None:
         self._optional = optional
 
-    def coerces(self, rhs: Base) -> bool:
+    def coerces(self, rhs: Base, check_quant: bool = True) -> bool:
         ""
         if isinstance(rhs, String):
             return True
-        return super().coerces(rhs)
+        return super().coerces(rhs, check_quant)
 
 
 class Float(Base):
     def __init__(self, optional: bool = False) -> None:
         self._optional = optional
 
-    def coerces(self, rhs: Base) -> bool:
+    def coerces(self, rhs: Base, check_quant: bool = True) -> bool:
         ""
         if isinstance(rhs, String):
             return True
-        return super().coerces(rhs)
+        return super().coerces(rhs, check_quant)
 
 
 class Int(Base):
     def __init__(self, optional: bool = False) -> None:
         self._optional = optional
 
-    def coerces(self, rhs: Base) -> bool:
+    def coerces(self, rhs: Base, check_quant: bool = True) -> bool:
         ""
         if isinstance(rhs, (Float, String)):
             return True
-        return super().coerces(rhs)
+        return super().coerces(rhs, check_quant)
 
 
 class File(Base):
     def __init__(self, optional: bool = False) -> None:
         self._optional = optional
 
-    def coerces(self, rhs: Base) -> bool:
+    def coerces(self, rhs: Base, check_quant: bool = True) -> bool:
         ""
         if isinstance(rhs, String):
             return True
-        return super().coerces(rhs)
+        return super().coerces(rhs, check_quant)
 
 
 class String(Base):
     def __init__(self, optional: bool = False) -> None:
         self._optional = optional
 
-    def coerces(self, rhs: Base) -> bool:
+    def coerces(self, rhs: Base, check_quant: bool = True) -> bool:
         ""
         if isinstance(rhs, File):
-            return True
-        return super().coerces(rhs)
+            return self._check_optional(rhs, check_quant)
+        return super().coerces(rhs, check_quant)
 
 
 class Array(Base):
@@ -189,12 +196,20 @@ def nonempty(self) -> bool:
         """
         return self._nonempty
 
-    def coerces(self, rhs: Base) -> bool:
+    def coerces(self, rhs: Base, check_quant: bool = True) -> bool:
         ""
         if isinstance(rhs, Array):
-            if self.item_type is None or rhs.item_type is None:
-                return True
-            return self.item_type.coerces(rhs.item_type) and (not rhs.nonempty or self.nonempty)
+            if self.item_type is None:
+                return self._check_optional(rhs, check_quant) and (
+                    not check_quant or not rhs.nonempty
+                )
+            if rhs.item_type is None:
+                return self._check_optional(rhs, check_quant)
+            return (
+                self.item_type.coerces(rhs.item_type, check_quant)
+                and (not check_quant or not rhs.nonempty or self.nonempty)
+                and self._check_optional(rhs, check_quant)
+            )
         if isinstance(rhs, String):
             return self.item_type is None or self.item_type.coerces(String())
         return False
@@ -237,16 +252,18 @@ def __str__(self) -> str:
             + ("?" if self.optional else "")
         )
 
-    def coerces(self, rhs: Base) -> bool:
+    def coerces(self, rhs: Base, check_quant: bool = True) -> bool:
         ""
         if isinstance(rhs, Map):
             if self.item_type is None or rhs.item_type is None:
                 return True
             # pyre-fixme
-            return self.item_type[0].coerces(rhs.item_type[0]) and self.item_type[1].coerces(
-                rhs.item_type[1]
+            return (
+                self.item_type[0].coerces(rhs.item_type[0], check_quant)
+                and self.item_type[1].coerces(rhs.item_type[1], check_quant)
+                and self._check_optional(rhs, check_quant)
             )
-        return super().coerces(rhs)
+        return False
 
 
 class Pair(Base):
@@ -275,3 +292,13 @@ def __str__(self) -> str:
             + "]"
             + ("?" if self.optional else "")
         )
+
+    def coerces(self, rhs: Base, check_quant: bool = True) -> bool:
+        ""
+        if isinstance(rhs, Pair):
+            return (
+                self.left_type.coerces(rhs.left_type, check_quant)
+                and self.right_type.coerces(rhs.right_type, check_quant)
+                and self._check_optional(rhs, check_quant)
+            )
+        return False

test_corpi/contrived/check_quant.wdl

@@ -3,5 +3,7 @@ import "contrived.wdl"
 workflow bs {
     Int? x
     Int y = x
+    Array[Int] a = [x]
+    Array[String]+ a2 = [x]
     call contrived.contrived
 }

test_corpi/contrived/contrived.wdl

@@ -14,7 +14,7 @@ workflow contrived {
     call popular { input:
         popular = popular,
         i = contrived,
-        y = contrived
+        y = select_first([contrived,23])
     }
     call popular as contrived { input:
         popular = 123

tests/test_2calls.py

@@ -100,6 +100,33 @@ def test_optional(self):
         doc = WDL.parse_document(txt)
         doc.typecheck(check_quant=False)
 
+        txt = tsk + r"""
+        workflow contrived {
+            Int? x = 0
+            String? s = "foo"
+            Pair[Int,String] p = (x,s)
+        }
+        """
+        doc = WDL.parse_document(txt)
+        with self.assertRaises(WDL.Error.StaticTypeMismatch):
+            doc.typecheck()
+        doc = WDL.parse_document(txt)
+        doc.typecheck(check_quant=False)
+
+        txt = tsk + r"""
+        workflow contrived {
+            Int? x = 0
+            Array[Int] y = [x]
+        }
+        """
+        doc = WDL.parse_document(txt)
+        with self.assertRaises(WDL.Error.StaticTypeMismatch):
+            doc.typecheck()
+        doc = WDL.parse_document(txt)
+        doc.typecheck(check_quant=False)
+
+        # TODO: test quant checking in Map & other composite types
+
     def test_nonempty(self):
         txt = r"""
         task p {

tests/test_3corpi.py

@@ -80,15 +80,15 @@ class gatk4_germline_snps_indels(unittest.TestCase):
 
 @test_corpus(
     ["test_corpi/gatk-workflows/gatk4-somatic-snvs-indels/**"],
-    expected_lint={'OptionalCoercion': 50, 'UnusedDeclaration': 29, 'ForwardReference': 6, 'NonemptyArrayCoercion': 4, 'StringCoercion': 20},
+    expected_lint={'QuantityCoercion': 54, 'UnusedDeclaration': 29, 'ForwardReference': 6, 'StringCoercion': 20},
     check_quant=False,
 )
 class gatk4_somatic_snvs_indels(unittest.TestCase):
     pass
 
 @test_corpus(
     ["test_corpi/gatk-workflows/gatk4-cnn-variant-filter/**"],
-    expected_lint={'UnusedDeclaration': 21, 'OptionalCoercion': 23, 'StringCoercion': 3, 'UnusedCall': 1},
+    expected_lint={'UnusedDeclaration': 21, 'QuantityCoercion': 23, 'StringCoercion': 3, 'UnusedCall': 1},
     check_quant=False,
 )
 class gatk4_cnn_variant_filter(unittest.TestCase):
@@ -117,7 +117,7 @@ class GTEx(unittest.TestCase):
     # need URI import
     blacklist=['CRAM_md5sum_checker_wrapper', 'checker-workflow-wrapping-alignment-workflow',
                 'topmed_freeze3_calling', 'topmed_freeze3_calling_checker', 'u_of_michigan_aligner_checker'],
-    expected_lint={'StringCoercion': 26, 'UnusedDeclaration': 74, 'OptionalCoercion': 1},
+    expected_lint={'StringCoercion': 26, 'UnusedDeclaration': 74, 'QuantityCoercion': 1},
     check_quant=False,
 )
 class TOPMed(unittest.TestCase):
@@ -132,15 +132,17 @@ class ViralNGS(unittest.TestCase):
     pass
 
 @test_corpus(
-     ["test_corpi/ENCODE-DCC/chip-seq-pipeline2/**"],
-     expected_lint={'StringCoercion': 224, 'NameCollision': 16, 'ArrayCoercion': 64, 'OptionalCoercion': 64},
+    ["test_corpi/ENCODE-DCC/chip-seq-pipeline2/**"],
+    expected_lint={'StringCoercion': 224, 'NameCollision': 16, 'ArrayCoercion': 64, 'QuantityCoercion': 64},
+    check_quant=False,
 )
 class ENCODE_ChIPseq(unittest.TestCase):
     pass
 
 @test_corpus(
-     ["test_corpi/ENCODE-DCC/atac-seq-pipeline/**"],
-     expected_lint={'StringCoercion': 182, 'ArrayCoercion': 41, 'OptionalCoercion': 26, 'UnusedCall': 13}
+    ["test_corpi/ENCODE-DCC/atac-seq-pipeline/**"],
+    expected_lint={'StringCoercion': 182, 'ArrayCoercion': 41, 'QuantityCoercion': 26, 'UnusedCall': 13},
+    check_quant=False,
 )
 class ENCODE_ATACseq(unittest.TestCase):
     pass
@@ -171,23 +173,23 @@ class ENCODE_WGBS(unittest.TestCase):
         # double quantifier
         "conditionals_base"
     ],
-    expected_lint={'UnusedDeclaration': 22, 'UnusedCall': 15, 'NameCollision': 2, 'OptionalCoercion': 1},
+    expected_lint={'UnusedDeclaration': 22, 'UnusedCall': 15, 'NameCollision': 2, 'QuantityCoercion': 1},
     check_quant=False,
 )
 class dxWDL(unittest.TestCase):
     pass
 
 @test_corpus(
     ["test_corpi/contrived/**"],
-    expected_lint={'UnusedImport': 2, 'NameCollision': 13, 'ArrayCoercion': 2, 'NonemptyArrayCoercion': 1, 'StringCoercion': 2, 'OptionalCoercion': 3, 'UnnecessaryQuantifier': 2, 'UnusedDeclaration': 2},
+    expected_lint={'UnusedImport': 2, 'NameCollision': 13, 'ArrayCoercion': 2, 'StringCoercion': 2, 'QuantityCoercion': 3, 'UnnecessaryQuantifier': 2, 'UnusedDeclaration': 2},
     blacklist=["check_quant"],
 )
 class Contrived(unittest.TestCase):
     pass
 
 @test_corpus(
     ["test_corpi/contrived/**"],
-    expected_lint={'UnusedImport': 4, 'NameCollision': 27, 'ArrayCoercion': 4, 'NonemptyArrayCoercion': 2, 'StringCoercion': 4, 'OptionalCoercion': 7, 'UnnecessaryQuantifier': 4, 'UnusedDeclaration': 5, 'IncompleteCall': 1},
+    expected_lint={'UnusedImport': 4, 'NameCollision': 27, 'ArrayCoercion': 4, 'StringCoercion': 4, 'QuantityCoercion': 8, 'UnnecessaryQuantifier': 4, 'UnusedDeclaration': 7, 'IncompleteCall': 1},
     check_quant=False,
 )
 class Contrived2(unittest.TestCase):