Lint: refactor UnusedImport

Disentagle MarkCalled and MarkImportsUsed walkers, simplifying both.

WDL/CLI.py

@@ -185,7 +185,7 @@ def descend(dobj=None, first_descent=first_descent):
         if obj.workflow:
             descend(obj.workflow)
         # tasks
-        for task in sorted(obj.tasks, key=lambda task: (not task.used, task.name)):
+        for task in sorted(obj.tasks, key=lambda task: (not task.called, task.name)):
             descend(task)
         # imports
         for imp in sorted(obj.imports, key=lambda t: t.namespace):
@@ -195,7 +195,7 @@ def descend(dobj=None, first_descent=first_descent):
     elif isinstance(obj, Workflow):
         print(
             "{}workflow {}{}".format(
-                s, obj.name, " (not called)" if show_called and not obj.used else ""
+                s, obj.name, " (not called)" if show_called and not obj.called else ""
             ),
             file=file,
         )
@@ -205,7 +205,7 @@ def descend(dobj=None, first_descent=first_descent):
     elif isinstance(obj, Task):
         print(
             "{}task {}{}".format(
-                s, obj.name, " (not called)" if show_called and not obj.used else ""
+                s, obj.name, " (not called)" if show_called and not obj.called else ""
             ),
             file=file,
         )

WDL/Lint.py

@@ -79,8 +79,8 @@ def lint(doc, descend_imports: bool = True):
 
     # Add additional markups to the AST for use by the linters
     Walker.SetParents()(doc)
-    Walker.MarkUsed()(doc)
-    Walker.SetReferrers()(doc)
+    Walker.MarkCalled()(doc)
+    Walker.Multi([Walker.MarkImportsUsed(), Walker.SetReferrers()])(doc)
 
     # instantiate linters
     linter_instances = [cons(descend_imports=descend_imports) for cons in _all_linters]
@@ -616,24 +616,15 @@ def document(self, obj: Tree.Document) -> Any:
 @a_linter
 class UnusedImport(Linter):
     # Nothing used from an imported document
-    # TODO: suppress if document is imported just to use its struct type declarations
+    # TODO: clarify confusion when none of an imported document D's structs are used because all
+    #       the same struct definitions were imported from a different document E (probably because
+    #       E itself imported D)
     def document(self, obj: Tree.Document) -> Any:
         for imp in obj.imports:
-            assert imp.doc is not None
-            any_used = False
-            for stb in obj.struct_typedefs:
-                st: Tree.StructTypeDef = stb.value
-                if st.imported and st.imported[0] is imp and getattr(st, "used", False):
-                    any_used = True
-            for task in imp.doc.tasks:
-                if getattr(task, "used", False):
-                    any_used = True
-            if imp.doc.workflow and getattr(imp.doc.workflow, "used", False):
-                any_used = True
-            if not any_used and (imp.doc.tasks or imp.doc.workflow):
+            if imp.namespace not in getattr(obj, "imports_used"):
                 self.add(
                     obj,
-                    "no calls to tasks/workflow, nor use of structs defined, in the imported document "
+                    "no use of workflow, tasks, or structs defined in the imported document "
                     + imp.namespace,
                 )
 

WDL/Tree.py

@@ -1679,7 +1679,7 @@ def _import_structs(doc: Document):
             except KeyError:
                 pass
             if not existing:
-                st2 = StructTypeDef(imp.pos, name, st.members, imported=(imp, st))
+                st2 = StructTypeDef(imp.pos, name, st.members, imported=(imp.doc, st))
                 doc.struct_typedefs = doc.struct_typedefs.bind(name, st2)
 
 

WDL/Walker.py

@@ -245,36 +245,26 @@ def expr(self, obj: Expr.Base) -> None:
             self._placeholder_depth -= 1
 
 
-class MarkUsed(Base):
+class MarkCalled(Base):
     """
-    Mark each Task and Workflow with ``used : bool`` according to whether there exists a Call to it
+    Mark each Task and Workflow with ``called : bool`` according to whether there exists a Call to it
     in the top-level workflow (or a called sub-workflow). The top-level workflow is considered
     called.
 
-    Also mark each StructTypeDef used if there exists a Decl instance of it in any workflow,
-    sub-workflow, or task (regardless of whether those are called). If the StructTypeDef is imported
-    from another document, propagate the flag there as well.
-
     Requires SetParents to have been applied previously.
     """
 
     marking: bool = False  # True while recursing from the top-level workflow
 
-    def document(self, obj: Tree.Document) -> None:
-        for stb in obj.struct_typedefs:
-            st: Tree.StructTypeDef = stb.value
-            st.used = False
-            # if struct has members that are imported structs, mark those used
-            for ty in st.members.values():
-                self._mark_structs(obj, ty, imported_only=True)
-
-        super().document(obj)
+    def task(self, obj: Tree.Task) -> None:
+        obj.called = getattr(obj, "called", False)
+        super().task(obj)
 
     def workflow(self, obj: Tree.Workflow) -> None:
-        obj.used = False
+        obj.called = getattr(obj, "called", False)
         if obj.parent.parent is None:  # pyre-ignore
             assert not self.marking
-            obj.used = True
+            obj.called = True
             self.marking = True
             super().workflow(obj)
             self.marking = False
@@ -285,29 +275,58 @@ def call(self, obj: Tree.Call) -> None:
         assert self.marking
         if isinstance(obj.callee, Tree.Workflow):
             self(obj.callee)
-        obj.callee.used = True
+        obj.callee.called = True
 
-    def task(self, obj: Tree.Task) -> None:
-        obj.used = False
-        super().task(obj)
+
+class MarkImportsUsed(Base):
+    """
+    Mark each DocImport object with ``used=True`` if the any of the workflow, tasks, or struct
+    typedefs in the imported document are used in the current document
+
+    Requires SetParents to have been applied previously.
+    """
+
+    def __init__(self):
+        super().__init__(auto_descend=True)
+
+    def document(self, obj: Tree.Document) -> None:
+        obj.imports_used = set()
+        for stb in obj.struct_typedefs:
+            st: Tree.StructTypeDef = stb.value
+            # if struct has members that are imported structs, mark those used
+            for ty in st.members.values():
+                self._mark_structs(obj, ty)
+
+    def call(self, obj: Tree.Call) -> None:
+        doc = obj
+        while not isinstance(doc, Tree.Document):
+            doc = getattr(doc, "parent")
+        assert isinstance(doc, Tree.Document)
+        callee_doc = obj.callee
+        while not isinstance(callee_doc, Tree.Document):
+            callee_doc = getattr(callee_doc, "parent")
+        assert isinstance(callee_doc, Tree.Document)
+        if doc is not callee_doc:
+            imp = next(imp for imp in doc.imports if imp.doc is callee_doc)
+            if imp:
+                getattr(doc, "imports_used").add(imp.namespace)
 
     def decl(self, obj: Tree.Decl) -> None:
         doc = obj
         while not isinstance(doc, Tree.Document):
             doc = getattr(doc, "parent")
         self._mark_structs(doc, obj.type)
 
-    def _mark_structs(self, doc: Tree.Document, ty: Type.Base, imported_only: bool = False) -> None:
+    def _mark_structs(self, doc: Tree.Document, ty: Type.Base) -> None:
         if isinstance(ty, Type.StructInstance):
             st: Tree.StructTypeDef = doc.struct_typedefs[ty.type_name]
-            if not imported_only:
-                st.used = True
-            while st.imported:
-                st.used = True
-                st = st.imported[1]
-                st.used = True
+            if st.imported:
+                assert isinstance(st.imported[0], Tree.Document)
+                imp = next((imp for imp in doc.imports if imp.doc is st.imported[0]), None)
+                if imp:
+                    getattr(doc, "imports_used").add(imp.namespace)
         for ch in ty.parameters:
-            self._mark_structs(doc, ch, imported_only=imported_only)
+            self._mark_structs(doc, ch)
 
 
 class SetReferrers(Base):

tests/test_3corpi.py

@@ -132,7 +132,7 @@ class HCAskylab_workflow(unittest.TestCase):
 
 @wdl_corpus(
     ["test_corpi/gatk-workflows/five-dollar-genome-analysis-pipeline/**"],
-    expected_lint={'UnusedDeclaration': 5, 'NameCollision': 2, 'UnusedImport': 2},
+    expected_lint={'UnusedDeclaration': 5, 'NameCollision': 2, 'UnusedImport': 8},
 )
 class GATK_five_dollar(unittest.TestCase):
     pass