From a67d9f812f78b39ddf747225592e25c5263e7bb7 Mon Sep 17 00:00:00 2001 From: "Michael A. Perlin" Date: Thu, 8 Aug 2024 11:32:06 -0400 Subject: [PATCH] Add option to use `ruff` for formatting and linting (#1016) [Ruff](https://docs.astral.sh/ruff/) ([GitHub](https://github.com/astral-sh/ruff)) is a Python linter and formatter that plays the role of `black`, `isort`, `flake8`, and `pylint`, and does it all much faster (for whatever that is worth). This PR adds the **option** to use `ruff` in place of these tools to `checks-superstaq`. Specifically, this PR functionally adds: - `ruff_format_.py` (which aspires to replace one day replace `format_.py`) - `ruff_lint_.py` (which aspires to one day replace `flake8_.py` and `pylint_.py`, at which point it can be renamed to (say) `lint_.py`). - A `--ruff` flag to `checks_superstaq.all_` to use `ruff` instead of the previous formatters+linters. Note that `ruff_format_.py` has a `--fix` option instead of `--apply` for parity with the `--fix` option recognized by `ruff check` (which is called by `ruff_lint_.py`). That's because `ruff format` [refuses to change a program's AST](https://github.com/astral-sh/ruff/issues/8926#issuecomment-1834048218), so it's not allowed to change import order. Import order therefore has to be handled by `ruff check` with `./checks/ruff_lint_.py --fix`. (Incidentally, maybe `ruff_lint_.py` should be renamed to `ruff_check_` because it calls `ruff check`? But then if+when this repo switches to using ruff it will feel silly to call `checks/check_.py`. Or maybe if+when `ruff` takes over we can have `ruff_format.py` and `ruff_check.py`?) `ruff` comes with many reasonable defaults, so I did not need to add much to `pyproject.toml`. Having said that, `checks-superstaq` has extensive configuration of `flake8` and `pylint`, and I am sure that not all of these linting rules are enforced by ruff. I leave it up to you all @Infleqtion to make the call on whether it's okay to merge in a "partial" switch to ruff (at the cost of having duplicate formatting/linting tools in the repo) vs. whether you'd like to do one big switch (configs and all). At the moment, all checks of `ruff_lint_.py` pass, and `ruff_format_.py` asks for a few minor formatting changes. The requested formatting changes are not included in this PR (though if I do make the changes, `checks/format_.py` is happy with all except those made to one `.pynb` file). --- checks-superstaq/checks_superstaq/__init__.py | 4 ++ checks-superstaq/checks_superstaq/all_.py | 23 ++++++-- .../checks_superstaq/checks-pyproject.toml | 4 ++ .../checks_superstaq/ruff_format_.py | 58 +++++++++++++++++++ .../checks_superstaq/ruff_lint_.py | 53 +++++++++++++++++ checks/ruff_format_.py | 9 +++ checks/ruff_lint_.py | 9 +++ 7 files changed, 156 insertions(+), 4 deletions(-) create mode 100644 checks-superstaq/checks_superstaq/ruff_format_.py create mode 100644 checks-superstaq/checks_superstaq/ruff_lint_.py create mode 100755 checks/ruff_format_.py create mode 100755 checks/ruff_lint_.py diff --git a/checks-superstaq/checks_superstaq/__init__.py b/checks-superstaq/checks_superstaq/__init__.py index 8d8a47b87..6832a7521 100644 --- a/checks-superstaq/checks_superstaq/__init__.py +++ b/checks-superstaq/checks_superstaq/__init__.py @@ -12,6 +12,8 @@ pylint_, pytest_, requirements, + ruff_format_, + ruff_lint_, ) __all__ = [ @@ -27,4 +29,6 @@ "pylint_", "pytest_", "requirements", + "ruff_format_", + "ruff_lint_", ] diff --git a/checks-superstaq/checks_superstaq/all_.py b/checks-superstaq/checks_superstaq/all_.py index 58cf45977..39844e91f 100644 --- a/checks-superstaq/checks_superstaq/all_.py +++ b/checks-superstaq/checks_superstaq/all_.py @@ -14,6 +14,8 @@ mypy_, pylint_, requirements, + ruff_format_, + ruff_lint_, ) @@ -50,6 +52,11 @@ def run(*args: str, sphinx_paths: list[str] | None = None) -> int: action="store_true", help="'Hard force' ~ continue past (i.e. do not exit after) all failing checks.", ) + parser.add_argument( + "--ruff", + action="store_true", + help="Use ruff for formatting and linting (replaces black, isort, flake8, and pylint).", + ) parsed_args, _ = parser.parse_known_intermixed_args(args) if parsed_args.revisions is not None: @@ -59,16 +66,24 @@ def run(*args: str, sphinx_paths: list[str] | None = None) -> int: default_mode = not parsed_args.files and parsed_args.revisions is None checks_failed = 0 - args_to_pass = [arg for arg in args if arg not in ("-f", "--force-formats", "-F", "--force")] + args_to_pass = [ + arg for arg in args if arg not in ("-f", "--force-formats", "-F", "--force", "--ruff") + ] # run formatting checks # silence most checks to avoid printing duplicate info about incremental files # silencing does not affect warnings and errors exit_on_failure = not (parsed_args.force_formats or parsed_args.force_all) checks_failed |= configs.run(*args_to_pass, exit_on_failure=exit_on_failure, silent=True) - checks_failed |= format_.run(*args_to_pass, exit_on_failure=exit_on_failure, silent=True) - checks_failed |= flake8_.run(*args_to_pass, exit_on_failure=exit_on_failure, silent=True) - checks_failed |= pylint_.run(*args_to_pass, exit_on_failure=exit_on_failure, silent=True) + if parsed_args.ruff: + checks_failed |= ruff_format_.run( + *args_to_pass, exit_on_failure=exit_on_failure, silent=True + ) + checks_failed |= ruff_lint_.run(*args_to_pass, exit_on_failure=exit_on_failure, silent=True) + else: + checks_failed |= format_.run(*args_to_pass, exit_on_failure=exit_on_failure, silent=True) + checks_failed |= flake8_.run(*args_to_pass, exit_on_failure=exit_on_failure, silent=True) + checks_failed |= pylint_.run(*args_to_pass, exit_on_failure=exit_on_failure, silent=True) # run typing and coverage checks exit_on_failure = not parsed_args.force_all diff --git a/checks-superstaq/checks_superstaq/checks-pyproject.toml b/checks-superstaq/checks_superstaq/checks-pyproject.toml index c917f0f11..0c02ef6bb 100644 --- a/checks-superstaq/checks_superstaq/checks-pyproject.toml +++ b/checks-superstaq/checks_superstaq/checks-pyproject.toml @@ -8,6 +8,10 @@ filterwarnings = [ "ignore::PendingDeprecationWarning:ruamel.yaml.main", ] +[tool.ruff] +line-length = 100 +lint.extend-select = ["I"] + [tool.black] color = true line_length = 100 diff --git a/checks-superstaq/checks_superstaq/ruff_format_.py b/checks-superstaq/checks_superstaq/ruff_format_.py new file mode 100644 index 000000000..1e9ac0110 --- /dev/null +++ b/checks-superstaq/checks_superstaq/ruff_format_.py @@ -0,0 +1,58 @@ +#!/usr/bin/env python3 +from __future__ import annotations + +import subprocess +import sys +import textwrap +from collections.abc import Iterable + +from checks_superstaq import check_utils + + +@check_utils.enable_exit_on_failure +def run( + *args: str, + include: str | Iterable[str] = ("*.py", "*.ipynb"), + exclude: str | Iterable[str] = (), + silent: bool = False, +) -> int: + """Runs 'ruff format' on the repository (formatting check). + + Args: + *args: Command line arguments. + include: Glob(s) indicating which tracked files to consider (e.g. "*.py"). + exclude: Glob(s) indicating which tracked files to skip (e.g. "*integration_test.py"). + silent: If True, restrict printing to warning and error messages. + + Returns: + Terminal exit code. 0 indicates success, while any other integer indicates a test failure. + """ + + parser = check_utils.get_check_parser() + parser.description = textwrap.dedent( + """ + Runs 'ruff format' on the repository (formatting check). + """ + ) + + parser.add_argument("--fix", action="store_true", help="Apply changes to files.") + + parsed_args, args_to_pass = parser.parse_known_intermixed_args(args) + if "ruff_format" in parsed_args.skip: + return 0 + + if not parsed_args.fix: + args_to_pass.append("--diff") + + files = check_utils.extract_files(parsed_args, include, exclude, silent) + + if files: + return subprocess.call( + ["python", "-m", "ruff", "format", *files, *args_to_pass], cwd=check_utils.root_dir + ) + + return 0 + + +if __name__ == "__main__": + exit(run(*sys.argv[1:])) diff --git a/checks-superstaq/checks_superstaq/ruff_lint_.py b/checks-superstaq/checks_superstaq/ruff_lint_.py new file mode 100644 index 000000000..bab875161 --- /dev/null +++ b/checks-superstaq/checks_superstaq/ruff_lint_.py @@ -0,0 +1,53 @@ +#!/usr/bin/env python3 +from __future__ import annotations + +import subprocess +import sys +import textwrap +from collections.abc import Iterable + +from checks_superstaq import check_utils + + +@check_utils.enable_exit_on_failure +def run( + *args: str, + include: str | Iterable[str] = "*.py", + exclude: str | Iterable[str] = (), + silent: bool = False, +) -> int: + """Runs 'ruff check' on the repository (formatting check). + + Args: + *args: Command line arguments. + include: Glob(s) indicating which tracked files to consider (e.g. "*.py"). + exclude: Glob(s) indicating which tracked files to skip (e.g. "*integration_test.py"). + silent: If True, restrict printing to warning and error messages. + + Returns: + Terminal exit code. 0 indicates success, while any other integer indicates a test failure. + """ + + parser = check_utils.get_check_parser() + parser.description = textwrap.dedent( + """ + Runs 'ruff check' on the repository (formatting check). + """ + ) + + parsed_args, args_to_pass = parser.parse_known_intermixed_args(args) + if "ruff_check" in parsed_args.skip: + return 0 + + files = check_utils.extract_files(parsed_args, include, exclude, silent) + + if files: + return subprocess.call( + ["python", "-m", "ruff", "check", *files, *args_to_pass], cwd=check_utils.root_dir + ) + + return 0 + + +if __name__ == "__main__": + exit(run(*sys.argv[1:])) diff --git a/checks/ruff_format_.py b/checks/ruff_format_.py new file mode 100755 index 000000000..d19c5cfc4 --- /dev/null +++ b/checks/ruff_format_.py @@ -0,0 +1,9 @@ +#!/usr/bin/env python3 +from __future__ import annotations + +import sys + +import checks_superstaq as checks + +if __name__ == "__main__": + exit(checks.ruff_format_.run(*sys.argv[1:])) diff --git a/checks/ruff_lint_.py b/checks/ruff_lint_.py new file mode 100755 index 000000000..3f528faf6 --- /dev/null +++ b/checks/ruff_lint_.py @@ -0,0 +1,9 @@ +#!/usr/bin/env python3 +from __future__ import annotations + +import sys + +import checks_superstaq as checks + +if __name__ == "__main__": + exit(checks.ruff_lint_.run(*sys.argv[1:]))