add mypy to style checks; rename `spack flake8` to `spack style` (#20384)

I lost my mind a bit after getting the completion stuff working and
decided to get Mypy working for spack as well. This adds a 
`.mypy.ini` that checks all of the spack and llnl modules, though
not yet packages, and fixes all of the identified missing types and
type issues for the spack library.

In addition to these changes, this includes:

* rename `spack flake8` to `spack style`

Aliases flake8 to style, and just runs flake8 as before, but with
a warning.  The style command runs both `flake8` and `mypy`,
in sequence. Added --no-<tool> options to turn off one or the
other, they are on by default.  Fixed two issues caught by the tools.

* stub typing module for python2.x

We don't support typing in Spack for python 2.x. To allow 2.x to
support `import typing` and `from typing import ...` without a
try/except dance to support old versions, this adds a stub module
*just* for python 2.x.  Doing it this way means we can only reliably
use all type hints in python3.7+, and mypi.ini has been updated to
reflect that.

* add non-default black check to spack style

This is a first step to requiring black.  It doesn't enforce it by
default, but it will check it if requested.  Currently enforcing the
line length of 79 since that's what flake8 requires, but it's a bit odd
for a black formatted project to be quite that narrow.  All settings are
in the style command since spack has no pyproject.toml and I don't
want to add one until more discussion happens. Also re-format
`style.py` since it no longer passed the black style check
with the new length.

* use style check in github action

Update the style and docs action to use `spack style`, adding in mypy
and black to the action even if it isn't running black right now.

50 files changed, 661 insertions, 317 deletions

diff --git a/.flake8 b/.flake8
index 04ca460eeb..a4c8aa2fbc 100644
--- a/.flake8
+++ b/.flake8
@@ -27,9 +27,17 @@
 # - N813: camelcase imported as lowercase
 # - N814: camelcase imported as constant
 #
+# F4: pyflakes import checks, these are now checked by mypy more precisely
+# - F403: from module import *
+# - F405: undefined name or from *
+#
+# Black ignores, these are incompatible with black style and do not follow PEP-8
+# - E203: white space around slice operators can be required, ignore : warn
+# - W503: see above, already ignored for line-breaks
+#
 [flake8]
-ignore = E129,E221,E241,E272,E731,W503,W504,F999,N801,N813,N814
-max-line-length = 79
+ignore = E129,E221,E241,E272,E731,W503,W504,F999,N801,N813,N814,F403,F405
+max-line-length = 88
 
 # F4: Import
 # - F405: `name` may be undefined, or undefined from star imports: `module`
diff --git a/.github/workflows/macos_unit_tests.yaml b/.github/workflows/macos_unit_tests.yaml
index 29caaa2e08..e5918968d2 100644
--- a/.github/workflows/macos_unit_tests.yaml
+++ b/.github/workflows/macos_unit_tests.yaml
@@ -26,7 +26,7 @@ jobs:
       run: |
           pip install --upgrade pip six setuptools
           pip install --upgrade codecov coverage
-          pip install --upgrade flake8 pep8-naming
+          pip install --upgrade flake8 pep8-naming mypy
     - name: Setup Homebrew packages
       run: |
         brew install dash fish gcc gnupg2 kcov
diff --git a/.github/workflows/style_and_docs.yaml b/.github/workflows/style_and_docs.yaml
index 5abedab784..5a940da99e 100644
--- a/.github/workflows/style_and_docs.yaml
+++ b/.github/workflows/style_and_docs.yaml
@@ -22,10 +22,10 @@ jobs:
         pip install --upgrade pip
         pip install --upgrade vermin
     - name: Minimum Version (Spack's Core)
-      run: vermin --backport argparse -t=2.6- -t=3.5- -v lib/spack/spack/ lib/spack/llnl/ bin/
+      run: vermin --backport argparse --backport typing -t=2.6- -t=3.5- -v lib/spack/spack/ lib/spack/llnl/ bin/
     - name: Minimum Version (Repositories)
-      run: vermin --backport argparse -t=2.6- -t=3.5- -v var/spack/repos
-  flake8:
+      run: vermin --backport argparse --backport typing -t=2.6- -t=3.5- -v var/spack/repos
+  style:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v2
@@ -36,15 +36,15 @@ jobs:
         python-version: 3.9
     - name: Install Python packages
       run: |
-        pip install --upgrade pip six setuptools flake8
+        pip install --upgrade pip six setuptools flake8 mypy black
     - name: Setup git configuration
       run: |
         # Need this for the git tests to succeed.
         git --version
         . .github/workflows/setup_git.sh
-    - name: Run flake8 tests
+    - name: Run style tests
       run: |
-          share/spack/qa/run-flake8-tests
+          share/spack/qa/run-style-tests
   documentation:
     runs-on: ubuntu-latest
     steps:
diff --git a/.mypy.ini b/.mypy.ini
new file mode 100644
index 0000000000..4257b1bd3f
--- /dev/null
+++ b/.mypy.ini
@@ -0,0 +1,35 @@
+[mypy]
+python_version = 3.7
+files=lib/spack/llnl/**/*.py,lib/spack/spack/**/*.py
+mypy_path=bin,lib/spack,lib/spack/external,var/spack/repos/builtin
+# This and a generated import file allows supporting packages
+namespace_packages=True
+# To avoid re-factoring all the externals, ignore errors and missing imports
+# globally, then turn back on in spack and spack submodules
+ignore_errors=True
+ignore_missing_imports=True
+
+[mypy-spack.*]
+ignore_errors=False
+ignore_missing_imports=False
+
+[mypy-packages.*]
+ignore_errors=False
+ignore_missing_imports=False
+
+[mypy-llnl.*]
+ignore_errors=False
+ignore_missing_imports=False
+
+[mypy-spack.test.packages]
+ignore_errors=True
+
+# ignore errors in fake import path for packages
+[mypy-spack.pkg.*]
+ignore_errors=True
+ignore_missing_imports=True
+
+# jinja has syntax in it that requires python3 and causes a parse error
+# skip importing it
+[mypy-jinja2]
+follow_imports=skip
diff --git a/bin/spack b/bin/spack
index 7e2e7e06de..1f9e257b8d 100755
--- a/bin/spack
+++ b/bin/spack
@@ -42,6 +42,8 @@ sys.path.insert(0, spack_lib_path)
 # Add external libs
 spack_external_libs = os.path.join(spack_lib_path, "external")
 
+if sys.version_info[:2] <= (2, 7):
+    sys.path.insert(0, os.path.join(spack_external_libs, 'py2'))
 if sys.version_info[:2] == (2, 6):
     sys.path.insert(0, os.path.join(spack_external_libs, 'py26'))
 
diff --git a/lib/spack/docs/contribution_guide.rst b/lib/spack/docs/contribution_guide.rst
index 8df8ad65ba..f554a76d29 100644
--- a/lib/spack/docs/contribution_guide.rst
+++ b/lib/spack/docs/contribution_guide.rst
@@ -179,24 +179,26 @@ how to write tests!
    run the unit tests yourself, we suggest you use ``spack unit-test``.
 
 ^^^^^^^^^^^^
-Flake8 Tests
+Style Tests
 ^^^^^^^^^^^^
 
 Spack uses `Flake8 <http://flake8.pycqa.org/en/latest/>`_ to test for
-`PEP 8 <https://www.python.org/dev/peps/pep-0008/>`_ conformance. PEP 8 is
+`PEP 8 <https://www.python.org/dev/peps/pep-0008/>`_ conformance and
+`mypy <https://mypy.readthedocs.io/en/stable/>` for type checking. PEP 8 is
 a series of style guides for Python that provide suggestions for everything
 from variable naming to indentation. In order to limit the number of PRs that
 were mostly style changes, we decided to enforce PEP 8 conformance. Your PR
-needs to comply with PEP 8 in order to be accepted.
+needs to comply with PEP 8 in order to be accepted, and if it modifies the
+spack library it needs to successfully type-check with mypy as well.
 
-Testing for PEP 8 compliance is easy. Simply run the ``spack flake8``
+Testing for compliance with spack's style is easy. Simply run the ``spack style``
 command:
 
 .. code-block:: console
 
-   $ spack flake8
+   $ spack style
 
-``spack flake8`` has a couple advantages over running ``flake8`` by hand:
+``spack style`` has a couple advantages over running the tools by hand:
 
 #. It only tests files that you have modified since branching off of
    ``develop``.
@@ -207,7 +209,9 @@ command:
    checks. For example, URLs are often longer than 80 characters, so we
    exempt them from line length checks. We also exempt lines that start
    with "homepage", "url", "version", "variant", "depends_on", and
-   "extends" in ``package.py`` files.
+   "extends" in ``package.py`` files.  This is now also possible when directly
+   running flake8 if you can use the ``spack`` formatter plugin included with
+   spack.
 
 More approved flake8 exemptions can be found
 `here <https://github.com/spack/spack/blob/develop/.flake8>`_.
@@ -240,13 +244,13 @@ However, if you aren't compliant with PEP 8, flake8 will complain:
 
 Most of the error messages are straightforward, but if you don't understand what
 they mean, just ask questions about them when you submit your PR. The line numbers
-will change if you add or delete lines, so simply run ``spack flake8`` again
+will change if you add or delete lines, so simply run ``spack style`` again
 to update them.
 
 .. tip::
 
    Try fixing flake8 errors in reverse order. This eliminates the need for
-   multiple runs of ``spack flake8`` just to re-compute line numbers and
+   multiple runs of ``spack style`` just to re-compute line numbers and
    makes it much easier to fix errors directly off of the CI output.
 
 .. warning::
diff --git a/lib/spack/external/py2/typing.py b/lib/spack/external/py2/typing.py
new file mode 100644
index 0000000000..2b06797660
--- /dev/null
+++ b/lib/spack/external/py2/typing.py
@@ -0,0 +1,84 @@
+# Copyright 2013-2020 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+"""
+This is a fake set of symbols to allow spack to import typing in python
+versions where we do not support type checking (<3)
+"""
+Annotated = None
+Any = None
+Callable = None
+ForwardRef = None
+Generic = None
+Literal = None
+Optional = None
+Tuple = None
+TypeVar = None
+Union = None
+AbstractSet = None
+ByteString = None
+Container = None
+Hashable = None
+ItemsView = None
+Iterable = None
+Iterator = None
+KeysView = None
+Mapping = None
+MappingView = None
+MutableMapping = None
+MutableSequence = None
+MutableSet = None
+Sequence = None
+Sized = None
+ValuesView = None
+Awaitable = None
+AsyncIterator = None
+AsyncIterable = None
+Coroutine = None
+Collection = None
+AsyncGenerator = None
+AsyncContextManager = None
+Reversible = None
+SupportsAbs = None
+SupportsBytes = None
+SupportsComplex = None
+SupportsFloat = None
+SupportsInt = None
+SupportsRound = None
+ChainMap = None
+Dict = None
+List = None
+OrderedDict = None
+Set = None
+FrozenSet = None
+NamedTuple = None
+Generator = None
+AnyStr = None
+cast = None
+get_args = None
+get_origin = None
+get_type_hints = None
+no_type_check = None
+no_type_check_decorator = None
+NoReturn = None
+
+# these are the typing extension symbols
+ClassVar = None
+Final = None
+Protocol = None
+Type = None
+TypedDict = None
+ContextManager = None
+Counter = None
+Deque = None
+DefaultDict = None
+SupportsIndex = None
+final = None
+IntVar = None
+Literal = None
+NewType = None
+overload = None
+runtime_checkable = None
+Text = None
+TYPE_CHECKING = None
diff --git a/lib/spack/llnl/util/tty/log.py b/lib/spack/llnl/util/tty/log.py
index 658de5c596..4f6acd7a1e 100644
--- a/lib/spack/llnl/util/tty/log.py
+++ b/lib/spack/llnl/util/tty/log.py
@@ -20,12 +20,17 @@ from contextlib import contextmanager
 from six import string_types
 from six import StringIO
 
+from typing import Optional  # novm
+from types import ModuleType  # novm
+
 import llnl.util.tty as tty
 
+termios = None  # type: Optional[ModuleType]
 try:
-    import termios
+    import termios as term_mod
+    termios = term_mod
 except ImportError:
-    termios = None
+    pass
 
 
 # Use this to strip escape sequences
diff --git a/lib/spack/spack/architecture.py b/lib/spack/spack/architecture.py
index c0930fe2d0..6377477822 100644
--- a/lib/spack/spack/architecture.py
+++ b/lib/spack/spack/architecture.py
@@ -235,18 +235,19 @@ class Platform(object):
         Will return a instance of it once it is returned.
     """
 
-    priority        = None   # Subclass sets number. Controls detection order
+    # Subclass sets number. Controls detection order
+    priority        = None   # type: int
 
     #: binary formats used on this platform; used by relocation logic
     binary_formats  = ['elf']
 
-    front_end       = None
-    back_end        = None
-    default         = None   # The default back end target.
+    front_end       = None   # type: str
+    back_end        = None   # type: str
+    default         = None   # type: str # The default back end target.
 
-    front_os        = None
-    back_os         = None
-    default_os      = None
+    front_os        = None   # type: str
+    back_os         = None   # type: str
+    default_os      = None   # type: str
 
     reserved_targets = ['default_target', 'frontend', 'fe', 'backend', 'be']
     reserved_oss = ['default_os', 'frontend', 'fe', 'backend', 'be']
diff --git a/lib/spack/spack/build_systems/autotools.py b/lib/spack/spack/build_systems/autotools.py
index ee9fb6884e..2f9245638b 100644
--- a/lib/spack/spack/build_systems/autotools.py
+++ b/lib/spack/spack/build_systems/autotools.py
@@ -8,6 +8,7 @@ import os
 import os.path
 from subprocess import PIPE
 from subprocess import check_call
+from typing import List  # novm
 
 import llnl.util.tty as tty
 import llnl.util.filesystem as fs
@@ -61,7 +62,7 @@ class AutotoolsPackage(PackageBase):
 
     #: Targets for ``make`` during the :py:meth:`~.AutotoolsPackage.build`
     #: phase
-    build_targets = []
+    build_targets = []  # type: List[str]
     #: Targets for ``make`` during the :py:meth:`~.AutotoolsPackage.install`
     #: phase
     install_targets = ['install']
@@ -75,7 +76,7 @@ class AutotoolsPackage(PackageBase):
     #: Set to true to force the autoreconf step even if configure is present
     force_autoreconf = False
     #: Options to be passed to autoreconf when using the default implementation
-    autoreconf_extra_args = []
+    autoreconf_extra_args = []  # type: List[str]
 
     #: If False deletes all the .la files in the prefix folder
     #: after the installation. If True instead it installs them.
diff --git a/lib/spack/spack/build_systems/cmake.py b/lib/spack/spack/build_systems/cmake.py
index 1336069846..fd55040652 100644
--- a/lib/spack/spack/build_systems/cmake.py
+++ b/lib/spack/spack/build_systems/cmake.py
@@ -8,6 +8,7 @@ import inspect
 import os
 import platform
 import re
+from typing import List  # novm
 
 import spack.build_environment
 from llnl.util.filesystem import working_dir
@@ -74,7 +75,7 @@ class CMakePackage(PackageBase):
     #: system base class
     build_system_class = 'CMakePackage'
 
-    build_targets = []
+    build_targets = []  # type: List[str]
     install_targets = ['install']
 
     build_time_test_callbacks = ['check']
diff --git a/lib/spack/spack/build_systems/makefile.py b/lib/spack/spack/build_systems/makefile.py
index 6cd05c7ad9..c09971c45f 100644
--- a/lib/spack/spack/build_systems/makefile.py
+++ b/lib/spack/spack/build_systems/makefile.py
@@ -5,6 +5,7 @@
 
 
 import inspect
+from typing import List  # novm
 
 import llnl.util.tty as tty
 from llnl.util.filesystem import working_dir
@@ -48,7 +49,7 @@ class MakefilePackage(PackageBase):
 
     #: Targets for ``make`` during the :py:meth:`~.MakefilePackage.build`
     #: phase
-    build_targets = []
+    build_targets = []  # type: List[str]
     #: Targets for ``make`` during the :py:meth:`~.MakefilePackage.install`
     #: phase
     install_targets = ['install']
diff --git a/lib/spack/spack/build_systems/meson.py b/lib/spack/spack/build_systems/meson.py
index 825b4c98c3..ee56f50861 100644
--- a/lib/spack/spack/build_systems/meson.py
+++ b/lib/spack/spack/build_systems/meson.py
@@ -6,6 +6,7 @@
 
 import inspect
 import os
+from typing import List  # novm
 
 from llnl.util.filesystem import working_dir
 from spack.directives import depends_on, variant
@@ -46,7 +47,7 @@ class MesonPackage(PackageBase):
     #: system base class
     build_system_class = 'MesonPackage'
 
-    build_targets = []
+    build_targets = []  # type: List[str]
     install_targets = ['install']
 
     build_time_test_callbacks = ['check']
diff --git a/lib/spack/spack/cmd/flake8.py b/lib/spack/spack/cmd/flake8.py
index ce6dbfed20..5d2d97143e 100644
--- a/lib/spack/spack/cmd/flake8.py
+++ b/lib/spack/spack/cmd/flake8.py
@@ -5,200 +5,22 @@
 
 from __future__ import print_function
 
-try:
-    from itertools import zip_longest  # novm
-except ImportError:
-    from itertools import izip_longest  # novm
+import llnl.util.tty as tty
 
-    zip_longest = izip_longest
+import spack.cmd.style
 
-import re
-import os
-import sys
-import argparse
 
-from llnl.util.filesystem import working_dir
-
-import spack.paths
-from spack.util.executable import which
-
-
-description = "runs source code style checks on Spack. requires flake8"
-section = "developer"
-level = "long"
-
-
-def grouper(iterable, n, fillvalue=None):
-    "Collect data into fixed-length chunks or blocks"
-    # grouper('ABCDEFG', 3, 'x') --> ABC DEF Gxx"
-    args = [iter(iterable)] * n
-    return zip_longest(*args, fillvalue=fillvalue)
-
-
-def is_package(f):
-    """Whether flake8 should consider a file as a core file or a package.
-
-    We run flake8 with different exceptions for the core and for
-    packages, since we allow `from spack import *` and poking globals
-    into packages.
-    """
-    return f.startswith("var/spack/repos/") or "docs/tutorial/examples" in f
-
-
-#: List of directories to exclude from checks.
-exclude_directories = [spack.paths.external_path]
-
-#: max line length we're enforcing (note: this duplicates what's in .flake8)
-max_line_length = 79
-
-
-def changed_files(base=None, untracked=True, all_files=False):
-    """Get list of changed files in the Spack repository."""
-
-    git = which("git", required=True)
-
-    if base is None:
-        base = os.environ.get("TRAVIS_BRANCH", "develop")
-
-    range = "{0}...".format(base)
-
-    git_args = [
-        # Add changed files committed since branching off of develop
-        ["diff", "--name-only", "--diff-filter=ACMR", range],
-        # Add changed files that have been staged but not yet committed
-        ["diff", "--name-only", "--diff-filter=ACMR", "--cached"],
-        # Add changed files that are unstaged
-        ["diff", "--name-only", "--diff-filter=ACMR"],
-    ]
-
-    # Add new files that are untracked
-    if untracked:
-        git_args.append(["ls-files", "--exclude-standard", "--other"])
-
-    # add everything if the user asked for it
-    if all_files:
-        git_args.append(["ls-files", "--exclude-standard"])
-
-    excludes = [os.path.realpath(f) for f in exclude_directories]
-    changed = set()
-
-    for arg_list in git_args:
-        files = git(*arg_list, output=str).split("\n")
-
-        for f in files:
-            # Ignore non-Python files
-            if not (f.endswith(".py") or f == "bin/spack"):
-                continue
-
-            # Ignore files in the exclude locations
-            if any(os.path.realpath(f).startswith(e) for e in excludes):
-                continue
-
-            changed.add(f)
-
-    return sorted(changed)
+description = "alias for spack style (deprecated)"
+section = spack.cmd.style.section
+level = spack.cmd.style.level
 
 
 def setup_parser(subparser):
-    subparser.add_argument(
-        "-b",
-        "--base",
-        action="store",
-        default=None,
-        help="select base branch for collecting list of modified files",
-    )
-    subparser.add_argument(
-        "-a",
-        "--all",
-        action="store_true",
-        help="check all files, not just changed files",
-    )
-    subparser.add_argument(
-        "-o",
-        "--output",
-        action="store_true",
-        help="send filtered files to stdout as well as temp files",
-    )
-    subparser.add_argument(
-        "-r",
-        "--root-relative",
-        action="store_true",
-        default=False,
-        help="print root-relative paths (default: cwd-relative)",
-    )
-    subparser.add_argument(
-        "-U",
-        "--no-untracked",
-        dest="untracked",
-        action="store_false",
-        default=True,
-        help="exclude untracked files from checks",
-    )
-    subparser.add_argument(
-        "files", nargs=argparse.REMAINDER, help="specific files to check"
-    )
+    spack.cmd.style.setup_parser(subparser)
 
 
 def flake8(parser, args):
-    file_list = args.files
-    if file_list:
-
-        def prefix_relative(path):
-            return os.path.relpath(
-                os.path.abspath(os.path.realpath(path)), spack.paths.prefix
-            )
-
-        file_list = [prefix_relative(p) for p in file_list]
-
-    returncode = 0
-    with working_dir(spack.paths.prefix):
-        if not file_list:
-            file_list = changed_files(args.base, args.untracked, args.all)
-
-        print("=======================================================")
-        print("flake8: running flake8 code checks on spack.")
-        print()
-        print("Modified files:")
-        for filename in file_list:
-            print("  {0}".format(filename.strip()))
-        print("=======================================================")
-
-        output = ""
-        # run in chunks of 100 at a time to avoid line length limit
-        # filename parameter in config *does not work* for this reliably
-        for chunk in grouper(file_list, 100):
-            flake8_cmd = which("flake8", required=True)
-            chunk = filter(lambda e: e is not None, chunk)
-
-            output = flake8_cmd(
-                # use .flake8 implicitly to work around bug in flake8 upstream
-                # append-config is ignored if `--config` is explicitly listed
-                # see: https://gitlab.com/pycqa/flake8/-/issues/455
-                # "--config=.flake8",
-                *chunk,
-                fail_on_error=False,
-                output=str
-            )
-            returncode |= flake8_cmd.returncode
-
-            if args.root_relative:
-                # print results relative to repo root.
-                print(output)
-            else:
-                # print results relative to current working directory
-                def cwd_relative(path):
-                    return "{0}: [".format(
-                        os.path.relpath(
-                            os.path.join(spack.paths.prefix, path.group(1)),
-                            os.getcwd(),
-                        )
-                    )
-
-                for line in output.split("\n"):
-                    print(re.sub(r"^(.*): \[", cwd_relative, line))
-
-    if returncode != 0:
-        print("Flake8 found errors.")
-        sys.exit(1)
-    else:
-        print("Flake8 checks were clean.")
+    tty.warn(
+        "spack flake8 is deprecated", "please use `spack style` to run style checks"
+    )
+    return spack.cmd.style.style(parser, args)
diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/module.py
index f86f3e5f25..459fb3748c 100644
--- a/lib/spack/spack/cmd/module.py
+++ b/lib/spack/spack/cmd/module.py
@@ -4,6 +4,7 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 import argparse
+from typing import Dict, Callable  # novm
 
 import llnl.util.tty as tty
 
@@ -15,7 +16,7 @@ section = "user environment"
 level = "short"
 
 
-_subcommands = {}
+_subcommands = {}  # type: Dict[str, Callable]
 
 _deprecated_commands = ('refresh', 'find', 'rm', 'loads')
 
diff --git a/lib/spack/spack/cmd/style.py b/lib/spack/spack/cmd/style.py
new file mode 100644
index 0000000000..62a7b72d8e
--- /dev/null
+++ b/lib/spack/spack/cmd/style.py
@@ -0,0 +1,330 @@
+# Copyright 2013-2020 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+from __future__ import print_function
+
+import re
+import os
+import sys
+import argparse
+
+if sys.version_info < (3, 0):
+    from itertools import izip_longest  # novm
+
+    zip_longest = izip_longest
+else:
+    from itertools import zip_longest  # novm
+
+from llnl.util.filesystem import working_dir
+import llnl.util.tty as tty
+
+import spack.paths
+from spack.util.executable import which
+
+
+description = (
+    "runs source code style checks on Spack. Requires flake8, mypy, black for "
+    + "their respective checks"
+)
+section = "developer"
+level = "long"
+
+
+def grouper(iterable, n, fillvalue=None):
+    "Collect data into fixed-length chunks or blocks"
+    # grouper('ABCDEFG', 3, 'x') --> ABC DEF Gxx"
+    args = [iter(iterable)] * n
+    return zip_longest(*args, fillvalue=fillvalue)
+
+
+#: List of directories to exclude from checks.
+exclude_directories = [spack.paths.external_path]
+
+#: max line length we're enforcing (note: this duplicates what's in .flake8)
+max_line_length = 79
+
+
+def is_package(f):
+    """Whether flake8 should consider a file as a core file or a package.
+
+    We run flake8 with different exceptions for the core and for
+    packages, since we allow `from spack import *` and poking globals
+    into packages.
+    """
+    return f.startswith("var/spack/repos/") or "docs/tutorial/examples" in f
+
+
+def changed_files(base=None, untracked=True, all_files=False):
+    """Get list of changed files in the Spack repository."""
+
+    git = which("git", required=True)
+
+    if base is None:
+        base = os.environ.get("TRAVIS_BRANCH", "develop")
+
+    range = "{0}...".format(base)
+
+    git_args = [
+        # Add changed files committed since branching off of develop
+        ["diff", "--name-only", "--diff-filter=ACMR", range],
+        # Add changed files that have been staged but not yet committed
+        ["diff", "--name-only", "--diff-filter=ACMR", "--cached"],
+        # Add changed files that are unstaged
+        ["diff", "--name-only", "--diff-filter=ACMR"],
+    ]
+
+    # Add new files that are untracked
+    if untracked:
+        git_args.append(["ls-files", "--exclude-standard", "--other"])
+
+    # add everything if the user asked for it
+    if all_files:
+        git_args.append(["ls-files", "--exclude-standard"])
+
+    excludes = [os.path.realpath(f) for f in exclude_directories]
+    changed = set()
+
+    for arg_list in git_args:
+        files = git(*arg_list, output=str).split("\n")
+
+        for f in files:
+            # Ignore non-Python files
+            if not (f.endswith(".py") or f == "bin/spack"):
+                continue
+
+            # Ignore files in the exclude locations
+            if any(os.path.realpath(f).startswith(e) for e in excludes):
+                continue
+
+            changed.add(f)
+
+    return sorted(changed)
+
+
+def setup_parser(subparser):
+    subparser.add_argument(
+        "-b",
+        "--base",
+        action="store",
+        default=None,
+        help="select base branch for collecting list of modified files",
+    )
+    subparser.add_argument(
+        "-a",
+        "--all",
+        action="store_true",
+        help="check all files, not just changed files",
+    )
+    subparser.add_argument(
+        "-o",
+        "--output",
+        action="store_true",
+        help="send filtered files to stdout as well as temp files",
+    )
+    subparser.add_argument(
+        "-r",
+        "--root-relative",
+        action="store_true",
+        default=False,
+        help="print root-relative paths (default: cwd-relative)",
+    )
+    subparser.add_argument(
+        "-U",
+        "--no-untracked",
+        dest="untracked",
+        action="store_false",
+        default=True,
+        help="exclude untracked files from checks",
+    )
+    subparser.add_argument(
+        "--no-flake8",
+        dest="flake8",
+        action="store_false",
+        help="Do not run flake8, default is run flake8",
+    )
+    subparser.add_argument(
+        "--no-mypy",
+        dest="mypy",
+        action="store_false",
+        help="Do not run mypy, default is run mypy if available",
+    )
+    subparser.add_argument(
+        "--black",
+        dest="black",
+        action="store_true",
+        help="Run black checks, default is skip",
+    )
+    subparser.add_argument(
+        "files", nargs=argparse.REMAINDER, help="specific files to check"
+    )
+
+
+def rewrite_and_print_output(
+    output,
+    args,
+    re_obj=re.compile(r"^(.+):([0-9]+):"),
+    replacement=r"{0}:{1}:",
+):
+    """rewrite ouput with <file>:<line>: format to respect path args"""
+    if args.root_relative or re_obj is None:
+        # print results relative to repo root.
+        print(output)
+    else:
+        # print results relative to current working directory
+        def cwd_relative(path):
+            return replacement.format(
+                os.path.relpath(
+                    os.path.join(spack.paths.prefix, path.group(1)),
+                    os.getcwd(),
+                ),
+                *list(path.groups()[1:])
+            )
+
+        for line in output.split("\n"):
+            print(
+                re_obj.sub(
+                    cwd_relative,
+                    line,
+                )
+            )
+
+
+def print_style_header(file_list, args):
+    tty.msg("style: running code checks on spack.")
+    tools = []
+    if args.flake8:
+        tools.append("flake8")
+    if args.mypy:
+        tools.append("mypy")
+    if args.black:
+        tools.append("black")
+    tty.msg("style: tools selected: " + ", ".join(tools))
+    tty.msg("Modified files:", *[filename.strip() for filename in file_list])
+    sys.stdout.flush()
+
+
+def print_tool_header(tool):
+    sys.stdout.flush()
+    tty.msg("style: running %s checks on spack." % tool)
+    sys.stdout.flush()
+
+
+def run_flake8(file_list, args):
+    returncode = 0
+    print_tool_header("flake8")
+    flake8_cmd = which("flake8", required=True)
+
+    output = ""
+    # run in chunks of 100 at a time to avoid line length limit
+    # filename parameter in config *does not work* for this reliably
+    for chunk in grouper(file_list, 100):
+        chunk = filter(lambda e: e is not None, chunk)
+
+        output = flake8_cmd(
+            # use .flake8 implicitly to work around bug in flake8 upstream
+            # append-config is ignored if `--config` is explicitly listed
+            # see: https://gitlab.com/pycqa/flake8/-/issues/455
+            # "--config=.flake8",
+            *chunk,
+            fail_on_error=False,
+            output=str
+        )
+        returncode |= flake8_cmd.returncode
+
+        rewrite_and_print_output(output, args)
+
+    if returncode == 0:
+        tty.msg("Flake8 style checks were clean")
+    else:
+        tty.error("Flake8 style checks found errors")
+    return returncode
+
+
+def run_mypy(file_list, args):
+    mypy_cmd = which("mypy")
+    if mypy_cmd is None:
+        tty.error("style: mypy is not available in path, skipping")
+        return 1
+
+    print_tool_header("mypy")
+
+    returncode = 0
+    output = ""
+    # run in chunks of 100 at a time to avoid line length limit
+    # filename parameter in config *does not work* for this reliably
+    for chunk in grouper(file_list, 100):
+        chunk = filter(lambda e: e is not None, chunk)
+
+        output = mypy_cmd(*chunk, fail_on_error=False, output=str)
+        returncode |= mypy_cmd.returncode
+
+        rewrite_and_print_output(output, args)
+
+    if returncode == 0:
+        tty.msg("mypy checks were clean")
+    else:
+        tty.error("mypy checks found errors")
+    return returncode
+
+
+def run_black(file_list, args):
+    black_cmd = which("black")
+    if black_cmd is None:
+        tty.error("style: black is not available in path, skipping")
+        return 1
+
+    print_tool_header("black")
+
+    pat = re.compile("would reformat +(.*)")
+    replacement = "would reformat {0}"
+    returncode = 0
+    output = ""
+    # run in chunks of 100 at a time to avoid line length limit
+    # filename parameter in config *does not work* for this reliably
+    for chunk in grouper(file_list, 100):
+        chunk = filter(lambda e: e is not None, chunk)
+
+        output = black_cmd(
+            "--check", "--diff", *chunk, fail_on_error=False, output=str, error=str
+        )
+        returncode |= black_cmd.returncode
+
+        rewrite_and_print_output(output, args, pat, replacement)
+
+    if returncode == 0:
+        tty.msg("black style checks were clean")
+    else:
+        tty.error("black checks found errors")
+    return returncode
+
+
+def style(parser, args):
+    file_list = args.files
+    if file_list:
+
+        def prefix_relative(path):
+            return os.path.relpath(
+                os.path.abspath(os.path.realpath(path)), spack.paths.prefix
+            )
+
+        file_list = [prefix_relative(p) for p in file_list]
+
+    returncode = 0
+    with working_dir(spack.paths.prefix):
+        if not file_list:
+            file_list = changed_files(args.base, args.untracked, args.all)
+        print_style_header(file_list, args)
+        if args.flake8:
+            returncode = run_flake8(file_list, args)
+        if args.mypy:
+            returncode |= run_mypy(file_list, args)
+        if args.black:
+            returncode |= run_black(file_list, args)
+
+    if returncode != 0:
+        print("spack style found errors.")
+        sys.exit(1)
+    else:
+        print("spack style checks were clean.")
diff --git a/lib/spack/spack/compiler.py b/lib/spack/spack/compiler.py
index d498803633..cc5a6c7d5f 100644
--- a/lib/spack/spack/compiler.py
+++ b/lib/spack/spack/compiler.py
@@ -10,6 +10,7 @@ import re
 import itertools
 import shutil
 import tempfile
+from typing import Sequence, List  # novm
 
 import llnl.util.lang
 from llnl.util.filesystem import (
@@ -190,20 +191,20 @@ class Compiler(object):
        and how to identify the particular type of compiler."""
 
     # Subclasses use possible names of C compiler
-    cc_names = []
+    cc_names = []  # type: List[str]
 
     # Subclasses use possible names of C++ compiler
-    cxx_names = []
+    cxx_names = []  # type: List[str]
 
     # Subclasses use possible names of Fortran 77 compiler
-    f77_names = []
+    f77_names = []  # type: List[str]
 
     # Subclasses use possible names of Fortran 90 compiler
-    fc_names = []
+    fc_names = []  # type: List[str]
 
     # Optional prefix regexes for searching for this type of compiler.
     # Prefixes are sometimes used for toolchains
-    prefixes = []
+    prefixes = []  # type: List[str]
 
     # Optional suffix regexes for searching for this type of compiler.
     # Suffixes are used by some frameworks, e.g. macports uses an '-mp-X.Y'
@@ -214,7 +215,7 @@ class Compiler(object):
     version_argument = '-dumpversion'
 
     #: Return values to ignore when invoking the compiler to get its version
-    ignore_version_errors = ()
+    ignore_version_errors = ()  # type: Sequence[int]
 
     #: Regex used to extract version from compiler's output
     version_regex = '(.*)'
@@ -266,9 +267,9 @@ class Compiler(object):
         return ['-O', '-O0', '-O1', '-O2', '-O3']
 
     # Cray PrgEnv name that can be used to load this compiler
-    PrgEnv = None
+    PrgEnv = None  # type: str
     # Name of module used to switch versions of this compiler
-    PrgEnv_compiler = None
+    PrgEnv_compiler = None  # type: str
 
     def __init__(self, cspec, operating_system, target,
                  paths, modules=None, alias=None, environment=None,
diff --git a/lib/spack/spack/compilers/__init__.py b/lib/spack/spack/compilers/__init__.py
index 44a0f90371..2392340570 100644
--- a/lib/spack/spack/compilers/__init__.py
+++ b/lib/spack/spack/compilers/__init__.py
@@ -11,6 +11,7 @@ import itertools
 import multiprocessing.pool
 import os
 import six
+from typing import Dict  # novm
 
 import llnl.util.lang
 import llnl.util.filesystem as fs
@@ -21,6 +22,7 @@ import spack.paths
 import spack.error
 import spack.spec
 import spack.config
+import spack.compiler
 import spack.architecture
 import spack.util.imp as simp
 from spack.util.environment import get_path
@@ -36,7 +38,7 @@ _cache_config_file = []
 # TODO: Caches at module level make it difficult to mock configurations in
 # TODO: unit tests. It might be worth reworking their implementation.
 #: cache of compilers constructed from config data, keyed by config entry id.
-_compiler_cache = {}
+_compiler_cache = {}  # type: Dict[str, spack.compiler.Compiler]
 
 _compiler_to_pkg = {
     'clang': 'llvm+clang'
diff --git a/lib/spack/spack/compilers/nag.py b/lib/spack/spack/compilers/nag.py
index 503a31e404..7d9f0e8245 100644
--- a/lib/spack/spack/compilers/nag.py
+++ b/lib/spack/spack/compilers/nag.py
@@ -3,15 +3,17 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
+from typing import List  # novm
+
 import spack.compiler
 
 
 class Nag(spack.compiler.Compiler):
     # Subclasses use possible names of C compiler
-    cc_names = []
+    cc_names = []  # type: List[str]
 
     # Subclasses use possible names of C++ compiler
-    cxx_names = []
+    cxx_names = []  # type: List[str]
 
     # Subclasses use possible names of Fortran 77 compiler
     f77_names = ['nagfor']
diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 84d8e8ca3f..27309be75b 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -38,6 +38,7 @@ import multiprocessing
 from contextlib import contextmanager
 from six import iteritems
 from ordereddict_backport import OrderedDict
+from typing import List  # novm
 
 import ruamel.yaml as yaml
 from ruamel.yaml.error import MarkedYAMLError
@@ -735,7 +736,7 @@ def override(path_or_scope, value=None):
 
 #: configuration scopes added on the command line
 #: set by ``spack.main.main()``.
-command_line_scopes = []
+command_line_scopes = []  # type: List[str]
 
 
 def _add_platform_scope(cfg, scope_type, name, path):
diff --git a/lib/spack/spack/database.py b/lib/spack/spack/database.py
index db1e6a636b..ed1bb2f381 100644
--- a/lib/spack/spack/database.py
+++ b/lib/spack/spack/database.py
@@ -27,6 +27,8 @@ import six
 import socket
 import sys
 import time
+from typing import Dict  # novm
+
 try:
     import uuid
     _use_uuid = True
@@ -289,10 +291,10 @@ _query_docstring = """
 class Database(object):
 
     """Per-process lock objects for each install prefix."""
-    _prefix_locks = {}
+    _prefix_locks = {}  # type: Dict[str, lk.Lock]
 
     """Per-process failure (lock) objects for each install prefix."""
-    _prefix_failures = {}
+    _prefix_failures = {}  # type: Dict[str, lk.Lock]
 
     def __init__(self, root, db_dir=None, upstream_dbs=None,
                  is_upstream=False, enable_transaction_locking=True,
@@ -1464,6 +1466,8 @@ class Database(object):
 
         return results
 
+    if _query.__doc__ is None:
+        _query.__doc__ = ""
     _query.__doc__ += _query_docstring
 
     def query_local(self, *args, **kwargs):
@@ -1471,6 +1475,8 @@ class Database(object):
         with self.read_transaction():
             return sorted(self._query(*args, **kwargs))
 
+    if query_local.__doc__ is None:
+        query_local.__doc__ = ""
     query_local.__doc__ += _query_docstring
 
     def query(self, *args, **kwargs):
@@ -1489,6 +1495,8 @@ class Database(object):
 
         return sorted(results)
 
+    if query.__doc__ is None:
+        query.__doc__ = ""
     query.__doc__ += _query_docstring
 
     def query_one(self, query_spec, known=any, installed=True):
diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py
index 41276b5b48..65c355edce 100644
--- a/lib/spack/spack/directives.py
+++ b/lib/spack/spack/directives.py
@@ -33,6 +33,7 @@ import functools
 import os.path
 import re
 from six import string_types
+from typing import Set, List  # novm
 
 import llnl.util.lang
 import llnl.util.tty.color
@@ -103,8 +104,8 @@ class DirectiveMeta(type):
     """
 
     # Set of all known directives
-    _directive_names = set()
-    _directives_to_be_executed = []
+    _directive_names = set()  # type: Set[str]
+    _directives_to_be_executed = []  # type: List[str]
 
     def __new__(cls, name, bases, attr_dict):
         # Initialize the attribute containing the list of directives
diff --git a/lib/spack/spack/fetch_strategy.py b/lib/spack/spack/fetch_strategy.py
index f075b893ae..4452a5898f 100644
--- a/lib/spack/spack/fetch_strategy.py
+++ b/lib/spack/spack/fetch_strategy.py
@@ -29,6 +29,7 @@ import os.path
 import re
 import shutil
 import sys
+from typing import Optional, List  # novm
 
 import llnl.util.tty as tty
 import six
@@ -92,11 +93,12 @@ class FetchStrategy(object):
     #: The URL attribute must be specified either at the package class
     #: level, or as a keyword argument to ``version()``.  It is used to
     #: distinguish fetchers for different versions in the package DSL.
-    url_attr = None
+    url_attr = None  # type: Optional[str]
 
     #: Optional attributes can be used to distinguish fetchers when :
     #: classes have multiple ``url_attrs`` at the top-level.
-    optional_attrs = []  # optional attributes in version() args.
+    # optional attributes in version() args.
+    optional_attrs = []  # type: List[str]
 
     def __init__(self, **kwargs):
         # The stage is initialized late, so that fetch strategies can be
@@ -420,7 +422,7 @@ class URLFetchStrategy(FetchStrategy):
             warn_content_type_mismatch(self.archive_file or "the archive")
         return partial_file, save_file
 
-    @property
+    @property  # type: ignore # decorated properties unsupported in mypy
     @_needs_stage
     def archive_file(self):
         """Path to the source archive within this stage directory."""
diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py
index c19d9018b2..5199adde68 100644
--- a/lib/spack/spack/installer.py
+++ b/lib/spack/spack/installer.py
@@ -1542,7 +1542,7 @@ class PackageInstaller(object):
                     (stop_before_phase is None and last_phase is None)
 
             except spack.directory_layout.InstallDirectoryAlreadyExistsError \
-                    as err:
+                    as exc:
                 tty.debug('Install prefix for {0} exists, keeping {1} in '
                           'place.'.format(pkg.name, pkg.prefix))
                 self._update_installed(task)
@@ -1553,7 +1553,7 @@ class PackageInstaller(object):
                     raise
 
                 if task.explicit:
-                    exists_errors.append((pkg_id, str(err)))
+                    exists_errors.append((pkg_id, str(exc)))
 
             except KeyboardInterrupt as exc:
                 # The build has been terminated with a Ctrl-C so terminate
diff --git a/lib/spack/spack/mirror.py b/lib/spack/spack/mirror.py
index 045ca5ffec..a4c9fce2f0 100644
--- a/lib/spack/spack/mirror.py
+++ b/lib/spack/spack/mirror.py
@@ -23,9 +23,9 @@ import ruamel.yaml.error as yaml_error
 
 from ordereddict_backport import OrderedDict
 
-try:
+if sys.version_info >= (3, 5):
     from collections.abc import Mapping  # novm
-except ImportError:
+else:
     from collections import Mapping
 
 import llnl.util.tty as tty
diff --git a/lib/spack/spack/mixins.py b/lib/spack/spack/mixins.py
index 6da12e4dce..63abcb561e 100644
--- a/lib/spack/spack/mixins.py
+++ b/lib/spack/spack/mixins.py
@@ -8,6 +8,12 @@ package.
 """
 import collections
 import os
+import sys
+from typing import Callable, DefaultDict, Dict, List  # novm
+if sys.version_info >= (3, 5):
+    CallbackDict = DefaultDict[str, List[Callable]]
+else:
+    CallbackDict = None
 
 import llnl.util.filesystem
 
@@ -26,12 +32,12 @@ class PackageMixinsMeta(type):
     gets implicitly attached to the package class by calling the mixin.
     """
 
-    _methods_to_be_added = {}
-    _add_method_before = collections.defaultdict(list)
-    _add_method_after = collections.defaultdict(list)
+    _methods_to_be_added = {}  # type: Dict[str, Callable]
+    _add_method_before = collections.defaultdict(list)  # type: CallbackDict
+    _add_method_after = collections.defaultdict(list)  # type: CallbackDict
 
     @staticmethod
-    def register_method_before(fn, phase):
+    def register_method_before(fn, phase):  # type: (Callable, str) -> None
         """Registers a method to be run before a certain phase.
 
         Args:
@@ -42,7 +48,7 @@ class PackageMixinsMeta(type):
         PackageMixinsMeta._add_method_before[phase].append(fn)
 
     @staticmethod
-    def register_method_after(fn, phase):
+    def register_method_after(fn, phase):  # type: (Callable, str) -> None
         """Registers a method to be run after a certain phase.
 
         Args:
diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py
index cbc064520c..fb688e4f1f 100644
--- a/lib/spack/spack/modules/common.py
+++ b/lib/spack/spack/modules/common.py
@@ -34,6 +34,7 @@ import datetime
 import inspect
 import os.path
 import re
+from typing import Optional  # novm
 
 import llnl.util.filesystem
 from llnl.util.lang import dedupe
@@ -540,7 +541,7 @@ class BaseFileLayout(object):
     """
 
     #: This needs to be redefined
-    extension = None
+    extension = None  # type: Optional[str]
 
     def __init__(self, configuration):
         self.conf = configuration
diff --git a/lib/spack/spack/modules/lmod.py b/lib/spack/spack/modules/lmod.py
index 80f6933063..d9d1bca5d7 100644
--- a/lib/spack/spack/modules/lmod.py
+++ b/lib/spack/spack/modules/lmod.py
@@ -8,6 +8,7 @@ import os.path
 import llnl.util.lang as lang
 import itertools
 import collections
+from typing import Dict, Any  # novm
 
 import spack.config
 import spack.compilers
@@ -26,7 +27,7 @@ def configuration():
 
 
 #: Caches the configuration {spec_hash: configuration}
-configuration_registry = {}
+configuration_registry = {}  # type: Dict[str, Any]
 
 
 def make_configuration(spec):
diff --git a/lib/spack/spack/modules/tcl.py b/lib/spack/spack/modules/tcl.py
index 0efc6332fe..65cdf5d862 100644
--- a/lib/spack/spack/modules/tcl.py
+++ b/lib/spack/spack/modules/tcl.py
@@ -8,6 +8,7 @@ non-hierarchical modules.
 """
 import os.path
 import string
+from typing import Dict, Any  # novm
 
 import llnl.util.tty as tty
 
@@ -24,7 +25,7 @@ def configuration():
 
 
 #: Caches the configuration {spec_hash: configuration}
-configuration_registry = {}
+configuration_registry = {}  # type: Dict[str, Any]
 
 
 def make_configuration(spec):
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 531f4aadee..1046b2d5eb 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -26,6 +26,7 @@ import time
 import traceback
 import six
 import types
+from typing import Optional, List, Dict, Any, Callable  # novm
 
 import llnl.util.filesystem as fsys
 import llnl.util.tty as tty
@@ -253,8 +254,9 @@ class PackageMeta(
     """
     phase_fmt = '_InstallPhase_{0}'
 
-    _InstallPhase_run_before = {}
-    _InstallPhase_run_after = {}
+    # These are accessed only through getattr, by name
+    _InstallPhase_run_before = {}  # type: Dict[str, List[Callable]]
+    _InstallPhase_run_after = {}  # type: Dict[str, List[Callable]]
 
     def __new__(cls, name, bases, attr_dict):
         """
@@ -555,7 +557,7 @@ class PackageBase(six.with_metaclass(PackageMeta, PackageViewMixin, object)):
 
     #: A list or set of build time test functions to be called when tests
     #: are executed or 'None' if there are no such test functions.
-    build_time_test_callbacks = None
+    build_time_test_callbacks = None  # type: Optional[List[str]]
 
     #: By default, packages are not virtual
     #: Virtual packages override this attribute
@@ -567,7 +569,7 @@ class PackageBase(six.with_metaclass(PackageMeta, PackageViewMixin, object)):
 
     #: A list or set of install time test functions to be called when tests
     #: are executed or 'None' if there are no such test functions.
-    install_time_test_callbacks = None
+    install_time_test_callbacks = None  # type: Optional[List[str]]
 
     #: By default we build in parallel.  Subclasses can override this.
     parallel = True
@@ -589,19 +591,19 @@ class PackageBase(six.with_metaclass(PackageMeta, PackageViewMixin, object)):
     #: List of prefix-relative file paths (or a single path). If these do
     #: not exist after install, or if they exist but are not files,
     #: sanity checks fail.
-    sanity_check_is_file = []
+    sanity_check_is_file = []  # type: List[str]
 
     #: List of prefix-relative directory paths (or a single path). If
     #: these do not exist after install, or if they exist but are not
     #: directories, sanity checks will fail.
-    sanity_check_is_dir = []
+    sanity_check_is_dir = []  # type: List[str]
 
     #: List of glob expressions. Each expression must either be
     #: absolute or relative to the package source path.
     #: Matching artifacts found at the end of the build process will be
     #: copied in the same directory tree as _spack_build_logfile and
     #: _spack_build_envfile.
-    archive_files = []
+    archive_files = []  # type: List[str]
 
     #: Boolean. Set to ``True`` for packages that require a manual download.
     #: This is currently used by package sanity tests and generation of a
@@ -609,7 +611,7 @@ class PackageBase(six.with_metaclass(PackageMeta, PackageViewMixin, object)):
     manual_download = False
 
     #: Set of additional options used when fetching package versions.
-    fetch_options = {}
+    fetch_options = {}  # type: Dict[str, Any]
 
     #
     # Set default licensing information
@@ -627,12 +629,12 @@ class PackageBase(six.with_metaclass(PackageMeta, PackageViewMixin, object)):
     #: looking for a license. All file paths must be relative to the
     #: installation directory. More complex packages like Intel may require
     #: multiple licenses for individual components. Defaults to the empty list.
-    license_files = []
+    license_files = []  # type: List[str]
 
     #: List of strings. Environment variables that can be set to tell the
     #: software where to look for a license if it is not in the usual location.
     #: Defaults to the empty list.
-    license_vars = []
+    license_vars = []  # type: List[str]
 
     #: String. A URL pointing to license setup instructions for the software.
     #: Defaults to the empty string.
@@ -646,7 +648,7 @@ class PackageBase(six.with_metaclass(PackageMeta, PackageViewMixin, object)):
 
     #: List of strings which contains GitHub usernames of package maintainers.
     #: Do not include @ here in order not to unnecessarily ping the users.
-    maintainers = []
+    maintainers = []  # type: List[str]
 
     #: List of attributes to be excluded from a package's hash.
     metadata_attrs = ['homepage', 'url', 'urls', 'list_url', 'extendable',
@@ -2591,7 +2593,7 @@ class BundlePackage(PackageBase):
     """General purpose bundle, or no-code, package class."""
     #: There are no phases by default but the property is required to support
     #: post-install hooks (e.g., for module generation).
-    phases = []
+    phases = []  # type: List[str]
     #: This attribute is used in UI queries that require to know which
     #: build-system class we are using
     build_system_class = 'BundlePackage'
diff --git a/lib/spack/spack/repo.py b/lib/spack/spack/repo.py
index 4af7b382f0..12ce448504 100644
--- a/lib/spack/spack/repo.py
+++ b/lib/spack/spack/repo.py
@@ -13,18 +13,18 @@ import itertools
 import os
 import re
 import shutil
+import six
 import stat
 import sys
 import traceback
 import types
+from typing import Dict  # novm
 
-try:
+if sys.version_info >= (3, 5):
     from collections.abc import Mapping  # novm
-except ImportError:
+else:
     from collections import Mapping
 
-import six
-
 import ruamel.yaml as yaml
 
 import llnl.util.lang
@@ -131,7 +131,7 @@ class FastPackageChecker(Mapping):
     during instance initialization.
     """
     #: Global cache, reused by every instance
-    _paths_cache = {}
+    _paths_cache = {}  # type: Dict[str, Dict[str, os.stat_result]]
 
     def __init__(self, packages_path):
         # The path of the repository managed by this instance
@@ -149,7 +149,7 @@ class FastPackageChecker(Mapping):
         self._paths_cache[self.packages_path] = self._create_new_cache()
         self._packages_to_stats = self._paths_cache[self.packages_path]
 
-    def _create_new_cache(self):
+    def _create_new_cache(self):  # type: () -> Dict[str, os.stat_result]
         """Create a new cache for packages in a repo.
 
         The implementation here should try to minimize filesystem
@@ -159,7 +159,7 @@ class FastPackageChecker(Mapping):
         """
         # Create a dictionary that will store the mapping between a
         # package name and its stat info
-        cache = {}
+        cache = {}  # type: Dict[str, os.stat_result]
         for pkg_name in os.listdir(self.packages_path):
             # Skip non-directories in the package root.
             pkg_dir = os.path.join(self.packages_path, pkg_name)
@@ -341,7 +341,7 @@ class PatchIndexer(Indexer):
     def _create(self):
         return spack.patch.PatchCache()
 
-    def needs_update():
+    def needs_update(self):
         # TODO: patches can change under a package and we should handle
         # TODO: it, but we currently punt. This should be refactored to
         # TODO: check whether patches changed each time a package loads,
diff --git a/lib/spack/spack/schema/environment.py b/lib/spack/spack/schema/environment.py
index d251aeba96..4d0d3c780e 100644
--- a/lib/spack/spack/schema/environment.py
+++ b/lib/spack/spack/schema/environment.py
@@ -38,11 +38,12 @@ def parse(config_obj):
         config_obj: a configuration dictionary conforming to the
             schema definition for environment modifications
     """
+    import sys
     import spack.util.environment as ev
-    try:
-        from collections import Sequence  # novm
-    except ImportError:
+    if sys.version_info >= (3, 5):
         from collections.abc import Sequence  # novm
+    else:
+        from collections import Sequence  # novm
 
     env = ev.EnvironmentModifications()
     for command, variable in config_obj.items():
diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 555b77090f..2322260369 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -19,7 +19,7 @@ import archspec.cpu
 try:
     import clingo
 except ImportError:
-    clingo = None
+    clingo = None  # type: ignore
 
 import llnl.util.lang
 import llnl.util.tty as tty
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index f0ae6a1431..012b725adc 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -3269,7 +3269,7 @@ class Spec(object):
         """Return list of any virtual deps in this spec."""
         return [spec for spec in self.traverse() if spec.virtual]
 
-    @property
+    @property  # type: ignore[misc] # decorated prop not supported in mypy
     @lang.memoized
     def patches(self):
         """Return patch objects for any patch sha256 sums on this Spec.
diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index 719d408d84..275a70c227 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -16,6 +16,7 @@ import sys
 import tempfile
 from six import string_types
 from six import iteritems
+from typing import Dict  # novm
 
 import llnl.util.tty as tty
 from llnl.util.filesystem import mkdirp, can_access, install, install_tree
@@ -221,7 +222,7 @@ class Stage(object):
     """
 
     """Shared dict of all stage locks."""
-    stage_locks = {}
+    stage_locks = {}  # type: Dict[str, spack.util.lock.Lock]
 
     """Most staging is managed by Spack.  DIYStage is one exception."""
     managed_by_spack = True
diff --git a/lib/spack/spack/tengine.py b/lib/spack/spack/tengine.py
index 15268e682d..41f8d913ce 100644
--- a/lib/spack/spack/tengine.py
+++ b/lib/spack/spack/tengine.py
@@ -4,6 +4,7 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 import itertools
 import textwrap
+from typing import List  # novm
 
 import llnl.util.lang
 import six
@@ -18,7 +19,7 @@ class ContextMeta(type):
     """
     #: Keeps track of the context properties that have been added
     #: by the class that is being defined
-    _new_context_properties = []
+    _new_context_properties = []  # type: List[str]
 
     def __new__(cls, name, bases, attr_dict):
         # Merge all the context properties that are coming from base classes
diff --git a/lib/spack/spack/test/cmd/flake8.py b/lib/spack/spack/test/cmd/flake8.py
index d4bfc5618d..8e3619ca4b 100644
--- a/lib/spack/spack/test/cmd/flake8.py
+++ b/lib/spack/spack/test/cmd/flake8.py
@@ -11,12 +11,12 @@ import sys
 from llnl.util.filesystem import FileFilter
 
 import spack.paths
-from spack.cmd.flake8 import flake8, setup_parser, changed_files
+from spack.cmd.style import style, setup_parser, changed_files
 from spack.repo import Repo
 from spack.util.executable import which
 
 
-@pytest.fixture(scope='module')
+@pytest.fixture(scope="module")
 def parser():
     """Returns the parser for the ``flake8`` command"""
     parser = argparse.ArgumentParser()
@@ -24,7 +24,7 @@ def parser():
     return parser
 
 
-@pytest.fixture(scope='module')
+@pytest.fixture(scope="module")
 def flake8_package():
     """Flake8 only checks files that have been modified.
     This fixture makes a small change to the ``flake8``
@@ -32,7 +32,7 @@ def flake8_package():
     change on cleanup.
     """
     repo = Repo(spack.paths.mock_packages_path)
-    filename = repo.filename_for_package_name('flake8')
+    filename = repo.filename_for_package_name("flake8")
     package = FileFilter(filename)
 
     # Make the change
@@ -60,16 +60,18 @@ def test_changed_files(parser, flake8_package):
 # As of flake8 3.0.0, Python 2.6 and 3.3 are no longer supported
 # http://flake8.pycqa.org/en/latest/release-notes/3.0.0.html
 @pytest.mark.skipif(
-    sys.version_info[:2] <= (2, 6) or
-    (3, 0) <= sys.version_info[:2] <= (3, 3),
-    reason='flake8 no longer supports Python 2.6 or 3.3 and older')
-@pytest.mark.skipif(not which('flake8'), reason='flake8 is not installed.')
+    sys.version_info[:2] <= (2, 6) or (3, 0) <= sys.version_info[:2] <= (3, 3),
+    reason="flake8 no longer supports Python 2.6 or 3.3 and older",
+)
+@pytest.mark.skipif(not which("flake8"), reason="flake8 is not installed.")
 def test_flake8(parser, flake8_package):
     # Only test the flake8_package that we modified
     # Otherwise, the unit tests would fail every time
     # the flake8 tests fail
-    args = parser.parse_args([flake8_package])
-    flake8(parser, args)
+    args = parser.parse_args(["--no-mypy", flake8_package])
+    style(parser, args)
     # Get even more coverage
-    args = parser.parse_args(['--output', '--root-relative', flake8_package])
-    flake8(parser, args)
+    args = parser.parse_args(
+        ["--no-mypy", "--output", "--root-relative", flake8_package]
+    )
+    style(parser, args)
diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py
index 8615b14abe..7701b243b0 100644
--- a/lib/spack/spack/test/conftest.py
+++ b/lib/spack/spack/test/conftest.py
@@ -318,7 +318,7 @@ def _skip_if_missing_executables(request):
 # FIXME: The lines below should better be added to a fixture with
 # FIXME: session-scope. Anyhow doing it is not easy, as it seems
 # FIXME: there's some weird interaction with compilers during concretization.
-spack.architecture.real_platform = spack.architecture.platform
+spack.architecture.real_platform = spack.architecture.platform  # type: ignore
 
 
 def test_platform():
@@ -1238,7 +1238,7 @@ repo:
 class MockBundle(object):
     has_code = False
     name = 'mock-bundle'
-    versions = {}
+    versions = {}  # type: ignore
 
 
 @pytest.fixture
diff --git a/lib/spack/spack/test/package_sanity.py b/lib/spack/spack/test/package_sanity.py
index d50169a1a4..039297381f 100644
--- a/lib/spack/spack/test/package_sanity.py
+++ b/lib/spack/spack/test/package_sanity.py
@@ -16,7 +16,7 @@ import spack.util.executable as executable
 import spack.variant
 # A few functions from this module are used to
 # do sanity checks only on packagess modified by a PR
-import spack.cmd.flake8 as flake8
+import spack.cmd.style as style
 import spack.util.crypto as crypto
 import pickle
 
@@ -207,7 +207,7 @@ def test_prs_update_old_api():
     deprecated calls to any method.
     """
     changed_package_files = [
-        x for x in flake8.changed_files() if flake8.is_package(x)
+        x for x in style.changed_files() if style.is_package(x)
     ]
     failing = []
     for file in changed_package_files:
diff --git a/lib/spack/spack/test/sbang.py b/lib/spack/spack/test/sbang.py
index 9307d2bc4b..44d5b5b868 100644
--- a/lib/spack/spack/test/sbang.py
+++ b/lib/spack/spack/test/sbang.py
@@ -44,7 +44,7 @@ sbang_line = '#!/bin/sh %s/bin/sbang\n' % spack.store.store.unpadded_root
 last_line  = "last!\n"
 
 
-@pytest.fixture
+@pytest.fixture  # type: ignore[no-redef]
 def sbang_line():
     yield '#!/bin/sh %s/bin/sbang\n' % spack.store.layout.root
 
diff --git a/lib/spack/spack/util/crypto.py b/lib/spack/spack/util/crypto.py
index 7e264ff0c7..5a2329ad32 100644
--- a/lib/spack/spack/util/crypto.py
+++ b/lib/spack/spack/util/crypto.py
@@ -5,10 +5,10 @@
 
 import sys
 import hashlib
+from typing import Dict, Callable, Any  # novm
 
 import llnl.util.tty as tty
 
-
 #: Set of hash algorithms that Spack can use, mapped to digest size in bytes
 hashes = {
     'md5': 16,
@@ -30,7 +30,7 @@ _deprecated_hash_algorithms = ['md5']
 
 
 #: cache of hash functions generated
-_hash_functions = {}
+_hash_functions = {}  # type: Dict[str, Callable[[], Any]]
 
 
 class DeprecatedHash(object):
diff --git a/lib/spack/spack/util/gpg.py b/lib/spack/spack/util/gpg.py
index eaa199e417..9d53ed1815 100644
--- a/lib/spack/spack/util/gpg.py
+++ b/lib/spack/spack/util/gpg.py
@@ -115,7 +115,7 @@ if not cached_property:
         return result
 
 
-class GpgConstants(object):
+class _GpgConstants(object):
     @cached_property
     def target_version(self):
         return spack.version.Version('2')
@@ -189,7 +189,7 @@ class GpgConstants(object):
         #                "file-name-too-long" errors in gpg.
 
         try:
-            has_suitable_gpgconf = bool(GpgConstants.gpgconf_string)
+            has_suitable_gpgconf = bool(_GpgConstants.gpgconf_string)
         except SpackGPGError:
             has_suitable_gpgconf = False
 
@@ -244,7 +244,7 @@ class GpgConstants(object):
                 pass
 
 
-GpgConstants = GpgConstants()
+GpgConstants = _GpgConstants()
 
 
 def ensure_gpg(reevaluate=False):
@@ -382,7 +382,7 @@ def gpg(*args, **kwargs):
     return get_global_gpg_instance()(*args, **kwargs)
 
 
-gpg.name = 'gpg'
+gpg.name = 'gpg'  # type: ignore[attr-defined]
 
 
 @functools.wraps(Gpg.create)
diff --git a/lib/spack/spack/util/lock.py b/lib/spack/spack/util/lock.py
index b748aa056a..3703965238 100644
--- a/lib/spack/spack/util/lock.py
+++ b/lib/spack/spack/util/lock.py
@@ -15,7 +15,7 @@ import spack.error
 import spack.paths
 
 
-class Lock(llnl.util.lock.Lock):
+class Lock(llnl.util.lock.Lock):  # type: ignore[no-redef]
     """Lock that can be disabled.
 
     This overrides the ``_lock()`` and ``_unlock()`` methods from
diff --git a/lib/spack/spack/util/log_parse.py b/lib/spack/spack/util/log_parse.py
index 4b01c18c91..26cd295e2b 100644
--- a/lib/spack/spack/util/log_parse.py
+++ b/lib/spack/spack/util/log_parse.py
@@ -43,7 +43,7 @@ def parse_log_events(stream, context=6, jobs=None, profile=False):
 
 
 #: lazily constructed CTest log parser
-parse_log_events.ctest_parser = None
+parse_log_events.ctest_parser = None  # type: ignore[attr-defined]
 
 
 def _wrap(text, width):
@@ -107,7 +107,7 @@ def make_log_context(log_events, width=None):
         for i in range(start, event.end):
             # wrap to width
             lines = _wrap(event[i], wrap_width)
-            lines[1:] = [indent + l for l in lines[1:]]
+            lines[1:] = [indent + ln for ln in lines[1:]]
             wrapped_line = line_fmt % (i, '\n'.join(lines))
 
             if i in error_lines:
diff --git a/lib/spack/spack/util/spack_yaml.py b/lib/spack/spack/util/spack_yaml.py
index 40e66ee62c..3d2374e18f 100644
--- a/lib/spack/spack/util/spack_yaml.py
+++ b/lib/spack/spack/util/spack_yaml.py
@@ -14,6 +14,7 @@
 """
 import ctypes
 import collections
+from typing import List  # novm
 
 from ordereddict_backport import OrderedDict
 from six import string_types, StringIO
@@ -219,7 +220,7 @@ def file_line(mark):
 #: This is nasty but YAML doesn't give us many ways to pass arguments --
 #: yaml.dump() takes a class (not an instance) and instantiates the dumper
 #: itself, so we can't just pass an instance
-_annotations = []
+_annotations = []  # type: List[str]
 
 
 class LineAnnotationDumper(OrderedLineDumper):
diff --git a/lib/spack/spack/util/web.py b/lib/spack/spack/util/web.py
index 1aad85550a..47010fdbfd 100644
--- a/lib/spack/spack/util/web.py
+++ b/lib/spack/spack/util/web.py
@@ -20,10 +20,10 @@ import six
 from six.moves.urllib.error import URLError
 from six.moves.urllib.request import urlopen, Request
 
-try:
+if sys.version_info < (3, 0):
     # Python 2 had these in the HTMLParser package.
     from HTMLParser import HTMLParser, HTMLParseError  # novm
-except ImportError:
+else:
     # In Python 3, things moved to html.parser
     from html.parser import HTMLParser
 
diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py
index 8fa52f9738..aa36b9b4ad 100644
--- a/lib/spack/spack/variant.py
+++ b/lib/spack/spack/variant.py
@@ -12,6 +12,12 @@ import inspect
 import itertools
 import re
 from six import StringIO
+import sys
+
+if sys.version_info >= (3, 5):
+    from collections.abc import Sequence  # novm
+else:
+    from collections import Sequence
 
 import llnl.util.tty.color
 import llnl.util.lang as lang
@@ -20,11 +26,6 @@ from spack.util.string import comma_or
 import spack.directives
 import spack.error as error
 
-try:
-    from collections.abc import Sequence  # novm
-except ImportError:
-    from collections import Sequence
-
 special_variant_values = [None, 'none', '*']
 
 
diff --git a/share/spack/qa/run-flake8-tests b/share/spack/qa/run-style-tests
index d6077fceb8..b91834f9d8 100755
--- a/share/spack/qa/run-flake8-tests
+++ b/share/spack/qa/run-style-tests
@@ -15,10 +15,10 @@
 #     run-flake8-tests
 #
 . "$(dirname $0)/setup.sh"
-check_dependencies flake8
+check_dependencies flake8 mypy
 
 # verify that the code style is correct
-spack flake8
+spack style
 
 # verify that the license headers are present
 spack license verify
diff --git a/share/spack/qa/setup.sh b/share/spack/qa/setup.sh
index e614bae909..3940d497c2 100755
--- a/share/spack/qa/setup.sh
+++ b/share/spack/qa/setup.sh
@@ -66,6 +66,10 @@ check_dependencies() {
                     spack_package=py-flake8
                     pip_package=flake8
                     ;;
+                mypy)
+                    spack_package=py-mypy
+                    pip_package=mypy
+                    ;;
                 dot)
                     spack_package=graphviz
                     ;;
diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash
index cbe59bc90a..36880dfdef 100755
--- a/share/spack/spack-completion.bash
+++ b/share/spack/spack-completion.bash
@@ -333,7 +333,7 @@ _spack() {
     then
         SPACK_COMPREPLY="-h --help -H --all-help --color -C --config-scope -d --debug --timestamp --pdb -e --env -D --env-dir -E --no-env --use-env-repo -k --insecure -l --enable-locks -L --disable-locks -m --mock -p --profile --sorted-profile --lines -v --verbose --stacktrace -V --version --print-shell-vars"
     else
-        SPACK_COMPREPLY="activate add arch blame build-env buildcache cd checksum ci clean clone commands compiler compilers concretize config containerize create deactivate debug dependencies dependents deprecate dev-build develop docs edit env extensions external fetch find flake8 gc gpg graph help info install license list load location log-parse maintainers mark mirror module patch pkg providers pydoc python reindex remove rm repo resource restage setup solve spec stage test test-env tutorial undevelop uninstall unit-test unload url verify versions view"
+        SPACK_COMPREPLY="activate add arch blame build-env buildcache cd checksum ci clean clone commands compiler compilers concretize config containerize create deactivate debug dependencies dependents deprecate dev-build develop docs edit env extensions external fetch find flake8 gc gpg graph help info install license list load location log-parse maintainers mark mirror module patch pkg providers pydoc python reindex remove rm repo resource restage setup solve spec stage style test test-env tutorial undevelop uninstall unit-test unload url verify versions view"
     fi
 }
 
@@ -913,7 +913,7 @@ _spack_find() {
 _spack_flake8() {
     if $list_options
     then
-        SPACK_COMPREPLY="-h --help -b --base -a --all -o --output -r --root-relative -U --no-untracked"
+        SPACK_COMPREPLY="-h --help -b --base -a --all -o --output -r --root-relative -U --no-untracked --no-flake8 --no-mypy --black"
     else
         SPACK_COMPREPLY=""
     fi
@@ -1513,6 +1513,15 @@ _spack_stage() {
     fi
 }
 
+_spack_style() {
+    if $list_options
+    then
+        SPACK_COMPREPLY="-h --help -b --base -a --all -o --output -r --root-relative -U --no-untracked --no-flake8 --no-mypy --black"
+    else
+        SPACK_COMPREPLY=""
+    fi
+}
+
 _spack_test() {
     if $list_options
     then