Merge 87d2003 into 155e23d

WDL/Error.py

@@ -1,5 +1,5 @@
 # pyre-strict
-from typing import List, Optional, NamedTuple, Union, Iterable
+from typing import List, Optional, NamedTuple, Union, Iterable, TypeVar
 from functools import total_ordering
 import WDL.Type as T
 
@@ -23,6 +23,8 @@ def __init__(self, document: str, import_uri: str, message: Optional[str] = None
 )
 """Source file, line, and column, attached to each AST node"""
 
+TVSourceNode = TypeVar("TVSourceNode", bound="SourceNode")
+
 
 @total_ordering
 class SourceNode:
@@ -54,6 +56,15 @@ def __lt__(self, rhs) -> bool:
     def __eq__(self, rhs) -> bool:
         return self.pos == rhs.pos
 
+    @property
+    def children(self: TVSourceNode) -> Iterable[TVSourceNode]:
+        """
+        :type: Iterable[SourceNode]
+
+        Yield all child nodes
+        """
+        return []
+
 
 class Base(Exception):
     node: Optional[SourceNode]

WDL/Expr.py

@@ -9,7 +9,7 @@
 a suitable Env.
 """
 from abc import ABC, abstractmethod
-from typing import List, Optional, Dict, TypeVar, Tuple, Union, Any
+from typing import List, Optional, Dict, TypeVar, Tuple, Union, Any, Iterable
 import WDL.Type as T
 import WDL.Value as V
 import WDL.Env as Env
@@ -180,6 +180,10 @@ def __init__(self, pos: SourcePosition, options: Dict[str, str], expr: Base) ->
         self.options = options
         self.expr = expr
 
+    @property
+    def children(self) -> Iterable[SourceNode]:
+        yield self.expr
+
     def _infer_type(self, type_env: Env.Types) -> T.Base:
         self.expr.infer_type(type_env, self._check_quant)
         if isinstance(self.expr.type, T.Array):
@@ -248,6 +252,12 @@ def __init__(self, pos: SourcePosition, parts: List[Union[str, Placeholder]]) ->
         super().__init__(pos)
         self.parts = parts
 
+    @property
+    def children(self) -> Iterable[SourceNode]:
+        for p in self.parts:
+            if isinstance(p, Base):
+                yield p
+
     def _infer_type(self, type_env: Env.Types) -> T.Base:
         for part in self.parts:
             if isinstance(part, Placeholder):
@@ -289,6 +299,11 @@ def __init__(self, pos: SourcePosition, items: List[Base]) -> None:
         super(Array, self).__init__(pos)
         self.items = items
 
+    @property
+    def children(self) -> Iterable[SourceNode]:
+        for it in self.items:
+            yield it
+
     def _infer_type(self, type_env: Env.Types) -> T.Base:
         if not self.items:
             return T.Array(None)
@@ -376,6 +391,12 @@ def __init__(
         self.consequent = consequent
         self.alternative = alternative
 
+    @property
+    def children(self) -> Iterable[SourceNode]:
+        yield self.condition
+        yield self.consequent
+        yield self.alternative
+
     def _infer_type(self, type_env: Env.Types) -> T.Base:
         # check for Boolean condition
         if self.condition.infer_type(type_env, self._check_quant).type != T.Boolean():
@@ -479,6 +500,11 @@ def __init__(self, pos: SourcePosition, function: str, arguments: List[Base]) ->
             raise Error.NoSuchFunction(self, function) from None
         self.arguments = arguments
 
+    @property
+    def children(self) -> Iterable[SourceNode]:
+        for arg in self.arguments:
+            yield arg
+
     def _infer_type(self, type_env: Env.Types) -> T.Base:
         for arg in self.arguments:
             arg.infer_type(type_env, self._check_quant)
@@ -520,6 +546,10 @@ def __init__(self, pos: SourcePosition, parts: List[str]) -> None:
         self.name = parts[-1]
         self.namespace = parts[:-1]
 
+    @property
+    def children(self) -> Iterable[SourceNode]:
+        return []
+
     def _infer_type(self, type_env: Env.Types) -> T.Base:
         if self.namespace and (self.name in ["left", "right"]):
             # Special case for pair access, IDENT.left or IDENT.right
@@ -584,6 +614,11 @@ def __init__(self, pos: SourcePosition, left: Base, right: Base) -> None:
         self.left = left
         self.right = right
 
+    @property
+    def children(self) -> Iterable[SourceNode]:
+        yield self.left
+        yield self.right
+
     def _infer_type(self, type_env: Env.Types) -> T.Base:
         self.left.infer_type(type_env, self._check_quant)
         self.right.infer_type(type_env, self._check_quant)
@@ -612,6 +647,12 @@ def __init__(self, pos: SourcePosition, items: List[Tuple[Base, Base]]) -> None:
         super().__init__(pos)
         self.items = items
 
+    @property
+    def children(self) -> Iterable[SourceNode]:
+        for k, v in self.items:
+            yield k
+            yield v
+
     def _infer_type(self, type_env: Env.Types) -> T.Base:
         kty = None
         vty = None

WDL/Lint.py

@@ -11,8 +11,14 @@ class Linter(WDL.Walker.Base):
         ``lint : List[Tuple[SourceNode,str,str]]``
     providing lint warnings with a node (possibly more-specific than the
     node it's attached to), short codename, and message.
+
+    Linters initialize the base Walker with ``auto_descend=True`` by default,
+    but this can be overridden if control of recursive descent is needed.
     """
 
+    def __init__(self, auto_descend: bool = True):
+        super().__init__(auto_descend=auto_descend)
+
     def add(self, obj: WDL.SourceNode, message: str, subnode: Optional[WDL.SourceNode] = None):
         """
         Used by subclasses to attach lint to a node.
@@ -47,48 +53,28 @@ def lint(doc):
     WDL.Walker.MarkCalled()(doc)
     WDL.Walker.SetReferrers()(doc)
 
-    for linter in _all_linters:
-        linter()(doc)
+    # instantiate linters
+    linter_instances = [cons() for cons in _all_linters]
+
+    # run auto-descend linters "concurrently"
+    WDL.Walker.Multi([linter for linter in linter_instances if linter.auto_descend])(doc)
+    # run each non-auto-descend linter
+    for linter in linter_instances:
+        if not linter.auto_descend:
+            linter(doc)
 
     return doc
 
 
 class _Collector(WDL.Walker.Base):
     def __init__(self):
-        super().__init__()
+        super().__init__(auto_descend=True)
         self.lint = []
 
-    def _collect(self, obj):
+    def __call__(self, obj):
         if hasattr(obj, "lint"):
             self.lint.extend(getattr(obj, "lint"))
-
-    def document(self, obj) -> Any:
-        self._collect(obj)
-        super().document(obj)
-
-    def workflow(self, obj) -> Any:
-        self._collect(obj)
-        super().workflow(obj)
-
-    def call(self, obj) -> Any:
-        self._collect(obj)
-        super().call(obj)
-
-    def scatter(self, obj) -> Any:
-        self._collect(obj)
-        super().scatter(obj)
-
-    def conditional(self, obj) -> Any:
-        self._collect(obj)
-        super().conditional(obj)
-
-    def decl(self, obj) -> Any:
-        self._collect(obj)
-        super().decl(obj)
-
-    def task(self, obj) -> Any:
-        self._collect(obj)
-        super().task(obj)
+        super().__call__(obj)
 
 
 def collect(doc):
@@ -121,7 +107,6 @@ def decl(self, obj: WDL.Decl) -> Any:
         if isinstance(obj.type, WDL.Type.String) and obj.expr:
             if not isinstance(obj.expr.type, (WDL.Type.String, WDL.Type.File)):
                 self.add(obj, "{} {} = :{}:".format(str(obj.type), obj.name, str(obj.expr.type)))
-        super().decl(obj)
 
     def expr(self, obj: WDL.Expr.Base) -> Any:
         pt = getattr(obj, "parent")
@@ -169,7 +154,6 @@ def expr(self, obj: WDL.Expr.Base) -> Any:
                     str(obj.type), ", ".join(":{}:".format(ty) for ty in item_types)
                 )
                 self.add(pt, msg, obj)
-        super().expr(obj)
 
     def call(self, obj: WDL.Tree.Call) -> Any:
         for name, inp_expr in obj.inputs.items():
@@ -202,7 +186,6 @@ def decl(self, obj: WDL.Decl) -> Any:
         if obj.expr and _is_array_coercion(obj.type, obj.expr.type):
             msg = "{} {} = :{}:".format(str(obj.type), obj.name, str(obj.expr.type))
             self.add(obj, msg)
-        super().decl(obj)
 
     def expr(self, obj: WDL.Expr.Base) -> Any:
         pt = getattr(obj, "parent")
@@ -215,7 +198,6 @@ def expr(self, obj: WDL.Expr.Base) -> Any:
                     if _is_array_coercion(F_i, arg_i.type):
                         msg = "{} argument of {}() = :{}:".format(str(F_i), F.name, str(arg_i.type))
                         self.add(pt, msg, arg_i)
-        super().expr(obj)
 
     def call(self, obj: WDL.Tree.Call) -> Any:
         for name, inp_expr in obj.inputs.items():
@@ -257,12 +239,10 @@ def expr(self, obj: WDL.Expr.Base) -> Any:
                                 str(F.argument_types[i]), F.name, str(obj.arguments[i].type)
                             )
                             self.add(getattr(obj, "parent"), msg, obj.arguments[i])
-        super().expr(obj)
 
     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)))
-        super().decl(obj)
 
     def call(self, obj: WDL.Tree.Call) -> Any:
         for name, inp_expr in obj.inputs.items():
@@ -286,7 +266,6 @@ def decl(self, obj: WDL.Decl) -> Any:
             # 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)))
-        super().decl(obj)
 
     def call(self, obj: WDL.Tree.Call) -> Any:
         for name, inp_expr in obj.inputs.items():
@@ -315,7 +294,6 @@ def call(self, obj: WDL.Call) -> Any:
                 obj.callee.name, ", ".join(required_inputs)
             )
             self.add(obj, msg)
-        super().call(obj)
 
 
 @a_linter
@@ -349,7 +327,6 @@ def call(self, obj: WDL.Call) -> Any:
             self.add(obj, msg)
         except KeyError:
             pass
-        super().call(obj)
 
     def decl(self, obj: WDL.Decl) -> Any:
         doc = obj
@@ -379,7 +356,6 @@ def workflow(self, obj: WDL.Workflow) -> Any:
                     obj.name
                 )
                 self.add(obj, msg)
-        super().workflow(obj)
 
     def task(self, obj: WDL.Task) -> Any:
         doc = obj
@@ -389,7 +365,6 @@ def task(self, obj: WDL.Task) -> Any:
             if namespace == obj.name:
                 msg = "task name '{}' collides with imported document namespace".format(obj.name)
                 self.add(obj, msg)
-        super().task(obj)
 
 
 @a_linter
@@ -406,7 +381,6 @@ def document(self, obj: WDL.Document) -> Any:
                 any_called = True
             if not any_called:
                 self.add(obj, "nothing used from the import " + namespace)
-        super().document(obj)
 
 
 @a_linter
@@ -428,7 +402,6 @@ def expr(self, obj: WDL.Expr.Base) -> Any:
             else:
                 assert False
             self.add(getattr(obj, "parent"), msg, obj)
-        super().expr(obj)
 
 
 @a_linter
@@ -472,19 +445,17 @@ def decl(self, obj: WDL.Tree.Decl) -> Any:
 @a_linter
 class UnusedCall(Linter):
     # the outputs of a Call are neither used nor propagated
-    _workflow_with_outputs: bool = False
-
-    def workflow(self, obj: WDL.Tree.Workflow) -> Any:
-        self._workflow_with_outputs = getattr(obj, "called", False) and obj.outputs is not None
-        super().workflow(obj)
-        self._workflow_with_outputs = False
 
     def call(self, obj: WDL.Tree.Call) -> Any:
-        if self._workflow_with_outputs and not getattr(obj, "referrers", []):
-            self.add(
-                obj,
-                "nothing references the outputs of call "
-                + obj.name
-                + " nor are are they output from the workflow",
-            )
-        super().call(obj)
+        if not getattr(obj, "referrers", []):
+            workflow = obj
+            while not isinstance(workflow, WDL.Tree.Workflow):
+                workflow = getattr(workflow, "parent")
+            if workflow.outputs is not None:
+                self.add(
+                    obj,
+                    "nothing references the outputs of the call "
+                    + obj.name
+                    + " nor are are they output from the workflow "
+                    + workflow.name,
+                )