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. @@ -134,6 +135,19 @@ def setup_parser(subparser): default=True, 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", @@ -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 prefix_relative(path): 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: