Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Ruff linter on the project #1102

Closed
benhoyt opened this issue Jan 8, 2024 · 0 comments · Fixed by #1120
Closed

Use Ruff linter on the project #1102

benhoyt opened this issue Jan 8, 2024 · 0 comments · Fixed by #1120
Assignees

Comments

@benhoyt
Copy link
Collaborator

benhoyt commented Jan 8, 2024

We'd like to switch from flake8 et al to https://docs.astral.sh/ruff/linter/. This issue tracks that work. The pyproject.toml work should be merged first.

@benhoyt benhoyt added the 24.04 label Jan 8, 2024
@tonyandrewmeyer tonyandrewmeyer self-assigned this Jan 23, 2024
benhoyt pushed a commit that referenced this issue Feb 4, 2024
This PR replaces flake8, flake8-docstrings, flake8-builtins, isort, autopep8, pep8-naming, pyproject-flake8, and two test_infra unit tests with ruff for linting. Formatting remains unchanged (to be addressed in #1103).

Notes:

1. Ruff does not yet implement all of the pycodestyle rules, which we checked/fixed with autopep8, see: astral-sh/ruff#2402
2. Ruff implements all Flake8 rules (the "F" rules).
3. flake8-docstrings is not listed in the Ruff re-implementation list by pydocstyle is there, which flake8-docstrings is based on
4. Ruff reimplements flake8-builtins (but picks up much more than we were previously, which is odd).
5. Ruff's isort is "profile='black'".
6. With flake8 we had "R" rules enabled, but I can't figure out what those were, or what provided them.

Where the linter picks up issues that the old tools did not, handle in one of these ways:
* Ignore with a `noqa:` directive if it's a false positive or should otherwise be permanently ignored in that specific case
* Ignore for a file or group of files (the docs and tests have several of these) where it's something we want to pick up in the core code but not everywhere
* Ignore with a note to review later when it's likely that there would be too much additional noise in this PR
* Make the recommended change, when it's small and seems reasonable

#1104 will continue on from this with a few more changes that are minimal and reasonable, and enabling additional rule sets (since they are bundled with ruff, and since ruff is so fast, they are basically free) that I agree with/like the most.

A few outdated `noqa:`  directives have been removed (ruff detects these as well).

Fixes #1102.
benhoyt pushed a commit that referenced this issue Feb 15, 2024
…#1114)

This PR enables a few additional `ruff` checks - since they are bundled with `ruff` and `ruff` is extremely fast, they are basically free. I cherry-picked the set that I agree with/like the most - I'm happy to argue for the inclusion of any specific ones where you disagree, or consider [other rules](https://docs.astral.sh/ruff/rules/) if you have preferences.

* pyupgrade (UP) - I love this tool - I find it's a super easy way to adopt new features as you move up your minimum-supported Python version (in using it for a year or so I haven't ever had it suggest something that I didn't like)
* flake8-2020 (YTT)
* bandit (S) - a good tool for avoiding accidentally missing things, in my opinion (marking them with `noqa` shows that thought has gone into it, which has value in itself)
* flake8-bugbear (B)
* flake8-simplify (SIM)
* Ruff specific (RUF) - seems like we should since we're using the tool
* perflint (PERF) - seems to make good suggestions in general

Notes:

Where the linter picks up new issues, these are handled in one of these ways:
* Ignore with a `noqa:` directive if it's a false positive or should otherwise be permanently ignored in that specific case
* Ignore for a file or group of files (the docs and tests have several of these) where it's something we want to pick up in the core code but not everywhere
* Ignore with a note to review later when it's likely that there would be too much additional noise in this PR
* Make the recommended change, when it's small and seems reasonable

Continues from #1102.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants