mypy 0.720 & strict types (#1171)

* increase test coverage, fix a bug

Author: mr-c

Makefile

@@ -175,9 +175,6 @@ mypy2: ${PYSOURCES}
 	rm -Rf typeshed/2and3/ruamel/yaml
 	ln -s $(shell python -c 'from __future__ import print_function; import ruamel.yaml; import os.path; print(os.path.dirname(ruamel.yaml.__file__))') \
 		typeshed/2and3/ruamel/yaml
-	rm -Rf typeshed/2and3/schema_salad
-	ln -s $(shell python -c 'from __future__ import print_function; import schema_salad; import os.path; print(os.path.dirname(schema_salad.__file__))') \
-		typeshed/2and3/schema_salad
 	MYPYPATH=$$MYPYPATH:typeshed/2.7:typeshed/2and3 mypy --py2 --disallow-untyped-calls \
 		 --warn-redundant-casts \
 		 cwltool
@@ -186,9 +183,6 @@ mypy3: ${PYSOURCES}
 	rm -Rf typeshed/2and3/ruamel/yaml
 	ln -s $(shell python3 -c 'from __future__ import print_function; import ruamel.yaml; import os.path; print(os.path.dirname(ruamel.yaml.__file__))') \
 		typeshed/2and3/ruamel/yaml
-	rm -Rf typeshed/2and3/schema_salad
-	ln -s $(shell python3 -c 'from __future__ import print_function; import schema_salad; import os.path; print(os.path.dirname(schema_salad.__file__))') \
-		typeshed/2and3/schema_salad
 	MYPYPATH=$$MYPYPATH:typeshed/3:typeshed/2and3 mypy --disallow-untyped-calls \
 		 --warn-redundant-casts \
 		 cwltool

cwltool/argparser.py

@@ -340,10 +340,14 @@ def __init__(self, option_strings, dest, nargs=None, **kwargs):
             raise ValueError("nargs not allowed")
         super(FSAction, self).__init__(option_strings, dest, **kwargs)
 
-    def __call__(self, parser, namespace, values, option_string=None):
-        # type: (argparse.ArgumentParser, argparse.Namespace, Union[AnyStr, Sequence[Any], None], AnyStr) -> None
+    def __call__(self,
+                 parser,             # type: argparse.ArgumentParser
+                 namespace,          # type: argparse.Namespace
+                 values,             # type: Union[AnyStr, Sequence[Any], None]
+                 option_string=None  # type: Optional[Text]
+                ):  # type: (...) -> None
         setattr(namespace,
-                self.dest,  # type: ignore
+                self.dest,
                 {"class": self.objclass,
                  "location": file_uri(str(os.path.abspath(cast(AnyStr, values))))})
 
@@ -358,16 +362,16 @@ def __init__(self, option_strings, dest, nargs=None, **kwargs):
             raise ValueError("nargs not allowed")
         super(FSAppendAction, self).__init__(option_strings, dest, **kwargs)
 
-    def __call__(self, parser, namespace, values, option_string=None):
-        # type: (argparse.ArgumentParser, argparse.Namespace, Union[AnyStr, Sequence[Any], None], AnyStr) -> None
-        g = getattr(namespace,
-                    self.dest  # type: ignore
-                    )
+    def __call__(self,
+                 parser,             # type: argparse.ArgumentParser
+                 namespace,          # type: argparse.Namespace
+                 values,             # type: Union[AnyStr, Sequence[Any], None]
+                 option_string=None  # type: Optional[Text]
+                ):  # type: (...) -> None
+        g = getattr(namespace, self.dest)
         if not g:
             g = []
-            setattr(namespace,
-                    self.dest,  # type: ignore
-                    g)
+            setattr(namespace, self.dest, g)
         g.append(
             {"class": self.objclass,
              "location": file_uri(str(os.path.abspath(cast(AnyStr, values))))})

cwltool/builder.py

@@ -39,14 +39,14 @@
     from .provenance import ProvenanceProfile  # pylint: disable=unused-import
 
 
-def content_limit_respected_read_bytes(f):  # type: (IO) -> bytes
+def content_limit_respected_read_bytes(f):  # type: (IO[bytes]) -> bytes
     contents = f.read(CONTENT_LIMIT + 1)
     if len(contents) > CONTENT_LIMIT:
         raise WorkflowException("loadContents handling encountered buffer that is exceeds maximum lenght of %d bytes" % CONTENT_LIMIT)
     return contents
 
 
-def content_limit_respected_read(f):  # type: (IO) -> Text
+def content_limit_respected_read(f):  # type: (IO[bytes]) -> Text
     return content_limit_respected_read_bytes(f).decode("utf-8")
 
 
@@ -92,7 +92,7 @@ def formatSubclassOf(fmt, cls, ontology, visited):
 
     return False
 
-def check_format(actual_file,    # type: Union[Dict[Text, Any], List, Text]
+def check_format(actual_file,    # type: Union[Dict[Text, Any], List[Dict[Text, Any]], Text]
                  input_formats,  # type: Union[List[Text], Text]
                  ontology        # type: Optional[Graph]
                 ):  # type: (...) -> None
@@ -113,10 +113,10 @@ def check_format(actual_file,    # type: Union[Dict[Text, Any], List, Text]
                 json_dumps(afile, indent=4)))
 
 class HasReqsHints(object):
-    def __init__(self):
+    def __init__(self):  # type: () -> None
         """Initialize this reqs decorator."""
-        self.requirements = []  # List[Dict[Text, Any]]
-        self.hints = []         # List[Dict[Text, Any]]
+        self.requirements = []  # type: List[Dict[Text, Any]]
+        self.hints = []         # type: List[Dict[Text, Any]]
 
     def get_requirement(self,
                         feature  # type: Text
@@ -131,7 +131,7 @@ def get_requirement(self,
 
 class Builder(HasReqsHints):
     def __init__(self,
-                 job,                  # type: Dict[Text, Union[Dict[Text, Any], List, Text, None]]
+                 job,                  # type: Dict[Text, expression.JSON]
                  files,                # type: List[Dict[Text, Text]]
                  bindings,             # type: List[Dict[Text, Any]]
                  schemaDefs,           # type: Dict[Text, Dict[Text, Any]]
@@ -208,7 +208,7 @@ def bind_input(self,
             lead_pos = []
 
         bindings = []  # type: List[MutableMapping[Text, Text]]
-        binding = None  # type: Optional[MutableMapping[Text,Any]]
+        binding = {}  # type: Union[MutableMapping[Text, Text], CommentedMap]
         value_from_expression = False
         if "inputBinding" in schema and isinstance(schema["inputBinding"], MutableMapping):
             binding = CommentedMap(schema["inputBinding"].items())
@@ -254,8 +254,7 @@ def bind_input(self,
                 raise validate.ValidationException(u"'%s' is not a valid union %s" % (datum, schema["type"]))
         elif isinstance(schema["type"], MutableMapping):
             st = copy.deepcopy(schema["type"])
-            if binding is not None\
-                    and "inputBinding" not in st\
+            if binding and "inputBinding" not in st\
                     and "type" in st\
                     and st["type"] == "array"\
                     and "itemSeparator" not in binding:
@@ -281,7 +280,7 @@ def bind_input(self,
             if schema["type"] == "array":
                 for n, item in enumerate(datum):
                     b2 = None
-                    if binding is not None:
+                    if binding:
                         b2 = copy.deepcopy(binding)
                         b2["datum"] = item
                     itemschema = {
@@ -293,9 +292,9 @@ def bind_input(self,
                             itemschema[k] = schema[k]
                     bindings.extend(
                         self.bind_input(itemschema, item, lead_pos=n, tail_pos=tail_pos, discover_secondaryFiles=discover_secondaryFiles))
-                binding = None
+                binding = {}
 
-            def _capture_files(f):
+            def _capture_files(f):  # type: (Dict[Text, Text]) -> Dict[Text, Text]
                 self.files.append(f)
                 return f
 
@@ -365,14 +364,14 @@ def _capture_files(f):
                 visit_class(datum, ("File", "Directory"), _capture_files)
 
         # Position to front of the sort key
-        if binding is not None:
+        if binding:
             for bi in bindings:
                 bi["position"] = binding["position"] + bi["position"]
             bindings.append(binding)
 
         return bindings
 
-    def tostr(self, value):  # type: (Any) -> Text
+    def tostr(self, value):  # type: (Union[MutableMapping[Text, Text], Any]) -> Text
         if isinstance(value, MutableMapping) and value.get("class") in ("File", "Directory"):
             if "path" not in value:
                 raise WorkflowException(u"%s object missing \"path\": %s" % (value["class"], value))

cwltool/checker.py

@@ -95,7 +95,7 @@ def can_assign_src_to_sink(src, sink, strict=False):  # type: (Any, Any, bool) -
             if can_assign_src_to_sink(src, this_sink):
                 return True
         return False
-    return src == sink
+    return bool(src == sink)
 
 
 def _compare_records(src, sink, strict=False):
@@ -126,8 +126,7 @@ def _rec_fields(rec):  # type: (MutableMapping[Text, Any]) -> MutableMapping[Tex
             return False
     return True
 
-def missing_subset(fullset, subset):
-    # type: (List, List) -> List
+def missing_subset(fullset, subset): # type: (List[Any], List[Any]) -> List[Any]
     missing = []
     for i in subset:
         if i not in fullset:

cwltool/command_line_tool.py

@@ -12,8 +12,8 @@
 import tempfile
 import threading
 from functools import cmp_to_key, partial
-from typing import (Any, Callable, Dict, Generator, List, Mapping, MutableMapping,
-                    MutableSequence, Optional, Set, Union, cast)
+from typing import (Any, Callable, Dict, Generator, IO, List, Mapping,
+                    MutableMapping, MutableSequence, Optional, Set, Union, cast)
 
 from typing_extensions import Text, Type, TYPE_CHECKING  # pylint: disable=unused-import
 # move to a regular typing import when Python 3.3-3.6 is no longer supported
@@ -83,8 +83,8 @@ def __init__(self,
                      builder,          # type: Builder
                      script,           # type: Dict[Text, Text]
                      output_callback,  # type: Callable[[Any, Any], Any]
-                     requirements,     # type: Dict[Text, Text]
-                     hints,            # type: Dict[Text, Text]
+                     requirements,     # type: List[Dict[Text, Text]]
+                     hints,            # type: List[Dict[Text, Text]]
                      outdir=None,      # type: Optional[Text]
                      tmpdir=None,      # type: Optional[Text]
                     ):  # type: (...) -> None
@@ -101,7 +101,7 @@ def __init__(self,
 
         def run(self,
                 runtimeContext,   # type: RuntimeContext
-                tmpdir_lock=None  # type: threading.Lock
+                tmpdir_lock=None  # type: Optional[threading.Lock]
                ):  # type: (...) -> None
             try:
                 normalizeFilesDirs(self.builder.job)
@@ -194,8 +194,10 @@ def __init__(self, job, output_callback, cachebuilder, jobcache):
         self.outdir = jobcache
         self.prov_obj = None  # type: Optional[ProvenanceProfile]
 
-    def run(self, runtimeContext):
-        # type: (RuntimeContext) -> None
+    def run(self,
+            runtimeContext,   # type: RuntimeContext
+            tmpdir_lock=None  # type: Optional[threading.Lock]
+            ):  # type: (...) -> None
         self.output_callback(self.job.collect_output_ports(
             self.job.tool["outputs"],
             self.cachebuilder,
@@ -232,7 +234,7 @@ def check_adjust(builder, file_o):
                 file_o["basename"]))
     return file_o
 
-def check_valid_locations(fs_access, ob):
+def check_valid_locations(fs_access, ob):  # type: (StdFsAccess, Dict[Text, Any]) -> None
     if ob["location"].startswith("_:"):
         pass
     if ob["class"] == "File" and not fs_access.isfile(ob["location"]):
@@ -285,7 +287,7 @@ def make_path_mapper(self, reffiles, stagedir, runtimeContext, separateDirs):
         return PathMapper(reffiles, runtimeContext.basedir, stagedir, separateDirs)
 
     def updatePathmap(self, outdir, pathmap, fn):
-        # type: (Text, PathMapper, Dict) -> None
+        # type: (Text, PathMapper, Dict[Text, Any]) -> None
         if "location" in fn and fn["location"] in pathmap:
             pathmap.update(fn["location"], pathmap.mapper(fn["location"]).resolved,
                            os.path.join(outdir, fn["basename"]),
@@ -298,7 +300,7 @@ def updatePathmap(self, outdir, pathmap, fn):
     def job(self,
             job_order,         # type: Mapping[Text, Text]
             output_callbacks,  # type: Callable[[Any, Any], Any]
-            runtimeContext     # RuntimeContext
+            runtimeContext     # type: RuntimeContext
            ):
         # type: (...) -> Generator[Union[JobBase, CallbackJob], None, None]
 
@@ -332,9 +334,9 @@ def job(self,
             if dockerimg is not None:
                 cmdline = ["docker", "run", dockerimg] + cmdline
                 # not really run using docker, just for hashing purposes
-            keydict = {u"cmdline": cmdline}
+            keydict = {u"cmdline": cmdline}  # type: Dict[Text, Union[Dict[Text, Any], List[Any]]]
 
-            for shortcut in ["stdout", "stderr"]:  # later, add "stdin"
+            for shortcut in ["stdin", "stdout", "stderr"]:
                 if shortcut in self.tool:
                     keydict[shortcut] = self.tool[shortcut]
 
@@ -409,8 +411,12 @@ def job(self,
                 runtimeContext = runtimeContext.copy()
                 runtimeContext.outdir = jobcache
 
-                def update_status_output_callback(output_callbacks, jobcachelock,
-                                               outputs, processStatus):
+                def update_status_output_callback(
+                        output_callbacks,  # type: Callable[[List[Dict[Text, Any]], Text], None]
+                        jobcachelock,      # type: IO[Any] 
+                        outputs,           # type: List[Dict[Text, Any]]
+                        processStatus      # type: Text
+                        ):  # type: (...) -> None
                     # save status to the lockfile then release the lock
                     jobcachelock.seek(0)
                     jobcachelock.truncate()
@@ -556,13 +562,15 @@ def update_status_output_callback(output_callbacks, jobcachelock,
         muts = set()  # type: Set[Text]
 
         if builder.mutation_manager is not None:
-            def register_mut(f):
+            def register_mut(f):  # type: (Dict[Text, Any]) -> None
+                mm = cast(MutationManager, builder.mutation_manager)
                 muts.add(f["location"])
-                builder.mutation_manager.register_mutation(j.name, f)
+                mm.register_mutation(j.name, f)
 
-            def register_reader(f):
+            def register_reader(f):  # type: (Dict[Text, Any]) -> None
+                mm = cast(MutationManager, builder.mutation_manager)
                 if f["location"] not in muts:
-                    builder.mutation_manager.register_reader(j.name, f)
+                    mm.register_reader(j.name, f)
                     readers[f["location"]] = copy.deepcopy(f)
 
             for li in j.generatefiles["listing"]:
@@ -628,7 +636,7 @@ def collect_output_ports(self,
                              rcode,                  # type: int
                              compute_checksum=True,  # type: bool
                              jobname="",             # type: Text
-                             readers=None            # type: Dict[Text, Any]
+                             readers=None            # type: Optional[Dict[Text, Any]]
                             ):  # type: (...) -> OutputPorts
         ret = {}  # type: OutputPorts
         debug = _logger.isEnabledFor(logging.DEBUG)
@@ -647,11 +655,12 @@ def collect_output_ports(self,
                                   json_dumps(ret, indent=4))
             else:
                 for i, port in enumerate(ports):
-                    def makeWorkflowException(msg):
-                        return WorkflowException(
-                            u"Error collecting output for parameter '%s':\n%s"
-                            % (shortname(port["id"]), msg))
-                    with SourceLine(ports, i, makeWorkflowException, debug):
+                    class ParameterOutputWorkflowException(WorkflowException):
+                        def __init__(self, msg, **kwargs):  # type: (Text, **Any) -> None
+                            super(ParameterOutputWorkflowException, self).__init__(
+                                u"Error collecting output for parameter '%s':\n%s"
+                                % (shortname(port["id"]), msg), kwargs)
+                    with SourceLine(ports, i, ParameterOutputWorkflowException, debug):
                         fragment = shortname(port["id"])
                         ret[fragment] = self.collect_output(port, builder, outdir, fs_access,
                                                             compute_checksum=compute_checksum)

cwltool/context.py

@@ -45,8 +45,8 @@ def __init__(self, kwargs=None):  # type: (Optional[Dict[str, Any]]) -> None
         """Initialize the LoadingContext from the kwargs."""
         self.debug = False                 # type: bool
         self.metadata = {}                 # type: Dict[Text, Any]
-        self.requirements = None
-        self.hints = None
+        self.requirements = None           # type: Optional[List[Dict[Text, Any]]]
+        self.hints = None                  # type: Optional[List[Dict[Text, Any]]]
         self.overrides_list = []           # type: List[Dict[Text, Any]]
         self.loader = None                 # type: Optional[Loader]
         self.avsc_names = None             # type: Optional[schema.Names]

cwltool/cwlrdf.py

@@ -1,6 +1,6 @@
 from __future__ import absolute_import
 
-from typing import IO, Any, Dict
+from typing import cast, IO, Any, Dict, MutableMapping
 
 from rdflib import Graph
 from schema_salad.jsonld_context import makerdf
@@ -15,26 +15,26 @@
 def gather(tool, ctx):  # type: (Process, ContextType) -> Graph
     g = Graph()
 
-    def visitor(t):
+    def visitor(t):  # type: (MutableMapping[Text, Any]) -> None
         makerdf(t["id"], t, ctx, graph=g)
 
     tool.visit(visitor)
     return g
 
 
-def printrdf(wflow, ctx, style):  # type: (Process, ContextType, Text) -> Text
+def printrdf(wflow, ctx, style):  # type: (Process, ContextType, str) -> Text
     """Serialize the CWL document into a string, ready for printing."""
     rdf = gather(wflow, ctx).serialize(format=style, encoding='utf-8')
     if not rdf:
         return u""
-    return rdf.decode('utf-8')
+    return cast(Text, rdf.decode('utf-8'))
 
 
 def lastpart(uri):  # type: (Any) -> Text
-    uri = Text(uri)
-    if "/" in uri:
-        return uri[uri.rindex("/") + 1:]
-    return uri
+    uri2 = Text(uri)
+    if "/" in uri2:
+        return uri2[uri2.rindex("/") + 1:]
+    return uri2
 
 
 def dot_with_parameters(g, stdout):  # type: (Graph, IO[Any]) -> None