From 49456fc51ed368f65f80ca117507977f1d2ab5bc Mon Sep 17 00:00:00 2001 From: Mike Lin Date: Thu, 5 Dec 2019 21:57:02 -1000 Subject: [PATCH 1/2] forbid calls with the same name as the containing workflow --- WDL/Lint.py | 6 +----- WDL/Tree.py | 5 +++++ test_corpi/contrived/contrived.wdl | 6 +++--- tests/test_2calls.py | 13 +++++++++++-- tests/test_3corpi.py | 4 ++-- 5 files changed, 22 insertions(+), 12 deletions(-) diff --git a/WDL/Lint.py b/WDL/Lint.py index 66b214e8..6c0dd7ce 100644 --- a/WDL/Lint.py +++ b/WDL/Lint.py @@ -478,7 +478,6 @@ def call(self, obj: Tree.Call) -> Any: class NameCollision(Linter): # Name collisions between # - call and import - # - call and its containing workflow # - call and struct type/alias # - decl and import # - decl and workflow @@ -503,13 +502,10 @@ def call(self, obj: Tree.Call) -> Any: if imp.namespace == obj.name: msg = "call name '{}' collides with imported document namespace".format(obj.name) self.add(obj, msg) - if doc.workflow and doc.workflow.name == obj.name: - msg = "call name '{}' collides with workflow name".format(obj.name) - self.add(obj, msg) for stb in doc.struct_typedefs: assert isinstance(stb, Env.Binding) and isinstance(stb.value, Tree.StructTypeDef) if stb.name == obj.name: - msg = "call name '{}' colides with {}struct type".format( + msg = "call name '{}' collides with {}struct type".format( obj.name, "imported " if stb.value.imported else "" ) self.add(obj, msg) diff --git a/WDL/Tree.py b/WDL/Tree.py index 52cb0543..b9a90f49 100644 --- a/WDL/Tree.py +++ b/WDL/Tree.py @@ -483,6 +483,11 @@ def resolve(self, doc: "Document") -> None: self.callee = task if self.callee is None: raise Error.NoSuchTask(self, ".".join(self.callee_id)) + assert doc.workflow + if self.name == doc.workflow.name: + raise Error.MultipleDefinitions( + self, "Call's name may not equal the containing workflow's" + ) assert isinstance(self.callee, (Task, Workflow)) def add_to_type_env( diff --git a/test_corpi/contrived/contrived.wdl b/test_corpi/contrived/contrived.wdl index 49ab5692..abfcef6c 100644 --- a/test_corpi/contrived/contrived.wdl +++ b/test_corpi/contrived/contrived.wdl @@ -17,7 +17,7 @@ workflow contrived { Float required } Int? fallaciously_optional = 123 - call popular as contrived { input: + call popular as c1 { input: popular = popular, i = fortytwo, y = [select_first([fortytwo,23])] @@ -28,8 +28,8 @@ workflow contrived { Pair[Pair[String,String],Pair[Int,Int]] p2 = ((c2.left_contents, c2.right_contents), (4,2)) output { - Int read_int = read_json(contrived.json) + p2.right.left + p2.right.right - Array[Boolean] read_array = read_json(contrived.json) + Int read_int = read_json(c1.json) + p2.right.left + p2.right.right + Array[Boolean] read_array = read_json(c1.json) String left_contents = p2.left.left String right_contents = p2.left.right } diff --git a/tests/test_2calls.py b/tests/test_2calls.py index 6bfd119d..d44702b6 100644 --- a/tests/test_2calls.py +++ b/tests/test_2calls.py @@ -319,6 +319,15 @@ def test_collision(self): with self.assertRaises(WDL.Error.MultipleDefinitions): doc.typecheck() + txt = task_no_outputs + r""" + workflow contrived { + call p as contrived + } + """ + doc = WDL.parse_document(txt) + with self.assertRaises(WDL.Error.MultipleDefinitions): + doc.typecheck() + def test_if_defined(self): # test how we typecheck a construct like # if defined(x) then EXPR_WITH_x else SOME_DEFAULT @@ -497,8 +506,8 @@ def test_io_propagation(self): doc.workflow.available_inputs.resolve("popular") doc.workflow.available_inputs.resolve("fortytwo") doc.workflow.available_inputs.resolve("required") - self.assertTrue(doc.workflow.available_inputs.has_namespace("contrived")) - doc.workflow.available_inputs.resolve("contrived.opt") + self.assertTrue(doc.workflow.available_inputs.has_namespace("c1")) + doc.workflow.available_inputs.resolve("c1.opt") self.assertTrue(doc.workflow.available_inputs.has_namespace("c2")) doc.workflow.available_inputs.enter_namespace("c2").resolve("opt") doc.workflow.available_inputs.enter_namespace("c2").resolve("i") diff --git a/tests/test_3corpi.py b/tests/test_3corpi.py index 00e6c83a..3fdac8bc 100644 --- a/tests/test_3corpi.py +++ b/tests/test_3corpi.py @@ -247,7 +247,7 @@ class dxWDL(unittest.TestCase): @wdl_corpus( ["test_corpi/contrived/**"], - expected_lint={'UnusedImport': 4, 'NameCollision': 30, 'StringCoercion': 4, 'FileCoercion': 2, 'NonemptyCoercion': 1, 'UnnecessaryQuantifier': 2, 'UnusedDeclaration': 2, "IncompleteCall": 2, 'SelectArray': 1}, + expected_lint={'UnusedImport': 4, 'NameCollision': 27, 'StringCoercion': 4, 'FileCoercion': 2, 'NonemptyCoercion': 1, 'UnnecessaryQuantifier': 2, 'UnusedDeclaration': 2, "IncompleteCall": 2, 'SelectArray': 1}, blacklist=["check_quant", "incomplete_call"], ) class Contrived(unittest.TestCase): @@ -255,7 +255,7 @@ class Contrived(unittest.TestCase): @wdl_corpus( ["test_corpi/contrived/**"], - expected_lint={'UnusedImport': 6, 'NameCollision': 49, 'StringCoercion': 9, 'FileCoercion': 4, 'OptionalCoercion': 3, 'NonemptyCoercion': 2, 'UnnecessaryQuantifier': 4, 'UnusedDeclaration': 9, 'IncompleteCall': 3, 'ArrayCoercion': 2, 'SelectArray': 4}, + expected_lint={'UnusedImport': 6, 'NameCollision': 43, 'StringCoercion': 9, 'FileCoercion': 4, 'OptionalCoercion': 3, 'NonemptyCoercion': 2, 'UnnecessaryQuantifier': 4, 'UnusedDeclaration': 9, 'IncompleteCall': 3, 'ArrayCoercion': 2, 'SelectArray': 4}, check_quant=False, blacklist=["incomplete_call"], ) From 4369f07ce34c0a8d5242f7c8a4946491ce93817e Mon Sep 17 00:00:00 2001 From: Mike Lin Date: Thu, 5 Dec 2019 23:04:19 -1000 Subject: [PATCH 2/2] WDL.values_from_json: remove extra subworkflow name component from input keys --- WDL/__init__.py | 5 +++++ tests/test_6workflowrun.py | 14 ++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/WDL/__init__.py b/WDL/__init__.py index ab9f5800..440a6430 100644 --- a/WDL/__init__.py +++ b/WDL/__init__.py @@ -178,6 +178,11 @@ def values_from_json( key2 = key if namespace and key.startswith(namespace): key2 = key[len(namespace) :] + if key2 not in available: + # attempt to simplify .. + key2parts = key2.split(".") + if len(key2parts) == 3 and key2parts[0] and key2parts[1] and key2parts[2]: + key2 = ".".join([key2parts[0], key2parts[2]]) try: ty = available[key2].type except KeyError: diff --git a/tests/test_6workflowrun.py b/tests/test_6workflowrun.py index 8d630e51..27373653 100644 --- a/tests/test_6workflowrun.py +++ b/tests/test_6workflowrun.py @@ -580,6 +580,20 @@ def test_subworkflow(self): self.assertEqual(outputs["sums"], [1, 5, 14]) self.assertEqual(outputs["sum"], 20) + subwf_input = R""" + version 1.0 + import "sum_sq.wdl" as lib + + workflow subwf_input { + call lib.sum_sq as summer + output { + Int ans = summer.ans + } + } + """ + self._test_workflow(subwf_input, {"summer.n": 3}) + self._test_workflow(subwf_input, {"summer.sum_sq.n": 3}) + def test_host_file_access(self): exn = self._test_workflow(""" version 1.0