style: add support for `isort` and `--fix`

3 files changed, 68 insertions, 11 deletions

diff --git a/.github/workflows/unit_tests.yaml b/.github/workflows/unit_tests.yaml
index 90ff168848..434e6d04b4 100644
--- a/.github/workflows/unit_tests.yaml
+++ b/.github/workflows/unit_tests.yaml
@@ -39,7 +39,7 @@ jobs:
         python-version: 3.9
     - name: Install Python packages
       run: |
-        pip install --upgrade pip six setuptools flake8 flake8-import-order mypy>=0.800 black types-six types-python-dateutil
+        pip install --upgrade pip six setuptools flake8 flake8-import-order isort mypy>=0.800 black types-six types-python-dateutil
     - name: Setup git configuration
       run: |
         # Need this for the git tests to succeed.
@@ -370,7 +370,7 @@ jobs:
       run: |
           pip install --upgrade pip six setuptools
           pip install --upgrade codecov coverage
-          pip install --upgrade flake8 flake8-import-order pep8-naming mypy
+          pip install --upgrade flake8 flake8-import-order isort pep8-naming mypy
           pip install --upgrade python-dateutil
     - name: Setup Homebrew packages
       run: |
diff --git a/.isort.cfg b/.isort.cfg
new file mode 100644
index 0000000000..e787561d14
--- /dev/null
+++ b/.isort.cfg
@@ -0,0 +1,11 @@
+[settings]
+profile=black
+sections=FUTURE,STDLIB,THIRDPARTY,ARCHSPEC,LLNL,FIRSTPARTY,LOCALFOLDER
+src_paths=lib
+force_sort_within_sections=True
+line_length=79
+known_first_party=spack
+known_archspec=archspec
+known_llnl=llnl
+honor_noqa=True
+use_parentheses=True
diff --git a/lib/spack/spack/cmd/style.py b/lib/spack/spack/cmd/style.py
index 43c8b52d3b..1f15db7b8d 100644
--- a/lib/spack/spack/cmd/style.py
+++ b/lib/spack/spack/cmd/style.py
@@ -32,7 +32,8 @@ 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)
+    for group in zip_longest(*args, fillvalue=fillvalue):
+        yield filter(None, group)
 
 
 #: List of directories to exclude from checks.
@@ -135,6 +136,19 @@ def setup_parser(subparser):
         help="exclude untracked files from checks",
     )
     subparser.add_argument(
+        "-f",
+        "--fix",
+        action="store_true",
+        default=False,
+        help="If the tool supports automatically editing file contents, do that.",
+    )
+    subparser.add_argument(
+        "--no-isort",
+        dest="isort",
+        action="store_false",
+        help="Do not run isort, default is run isort if available",
+    )
+    subparser.add_argument(
         "--no-flake8",
         dest="flake8",
         action="store_false",
@@ -213,7 +227,6 @@ def run_flake8(file_list, args):
     # 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
@@ -242,7 +255,7 @@ def run_mypy(file_list, args):
         return 1
 
     print_tool_header("mypy")
-    mpy_args = ["--package", "spack", "--package", "llnl"]
+    mpy_args = ["--package", "spack", "--package", "llnl", "--show-error-codes"]
     # not yet, need other updates to enable this
     # if any([is_package(f) for f in file_list]):
     #     mpy_args.extend(["--package", "packages"])
@@ -259,6 +272,35 @@ def run_mypy(file_list, args):
     return returncode
 
 
+def run_isort(file_list, args):
+    isort_cmd = which("isort")
+    if isort_cmd is None:
+        tty.error("style: isort is not available in path, skipping")
+        return 1
+
+    print_tool_header("isort")
+    # TODO: isort apparently decides to avoid writing colored output at all unless the
+    # output is a tty, even when --color is provided.
+    check_fix_args = () if args.fix else ("--check", "--diff", "--color")
+
+    pat = re.compile("ERROR: (.*) Imports are incorrectly sorted and/or formatted")
+    replacement = "ERROR: {0} Imports are incorrectly sorted and/or formatted"
+    returncode = 0
+    for chunk in grouper(file_list, 100):
+        # TODO: py2 does not support multiple * unpacking expressions in a single call.
+        packed_args = check_fix_args + chunk
+        output = isort_cmd(*packed_args, fail_on_error=False, output=str, error=str)
+        returncode |= isort_cmd.returncode
+
+        rewrite_and_print_output(output, args, pat, replacement)
+
+    if returncode == 0:
+        tty.msg("isort style checks were clean")
+    else:
+        tty.error("isort checks found errors")
+    return returncode
+
+
 def run_black(file_list, args):
     black_cmd = which("black")
     if black_cmd is None:
@@ -266,6 +308,7 @@ def run_black(file_list, args):
         return 1
 
     print_tool_header("black")
+    check_fix_args = () if args.fix else ("--check", "--diff", "--color")
 
     pat = re.compile("would reformat +(.*)")
     replacement = "would reformat {0}"
@@ -274,11 +317,9 @@ def run_black(file_list, args):
     # 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
-        )
+        # TODO: py2 does not support multiple * unpacking expressions in a single call.
+        packed_args = check_fix_args + chunk
+        output = black_cmd(*packed_args, fail_on_error=False, output=str, error=str)
         returncode |= black_cmd.returncode
 
         rewrite_and_print_output(output, args, pat, replacement)
@@ -306,8 +347,13 @@ def style(parser, args):
         if not file_list:
             file_list = changed_files(args.base, args.untracked, args.all)
         print_style_header(file_list, args)
+        if args.isort:
+            # We run isort before flake8, assuming that #23947 with flake8-import-order
+            # is also in, to allow flake8 to double-check the result of executing isort,
+            # if --fix was provided.
+            returncode = run_isort(file_list, args)
         if args.flake8:
-            returncode = run_flake8(file_list, args)
+            returncode |= run_flake8(file_list, args)
         if args.mypy:
             returncode |= run_mypy(file_list, args)
         if args.black: