usdigitalresponse / cpf-reporter

CPF Reporter application maintained by the USDR Grants program.
Apache License 2.0
0 stars 4 forks source link

[Feature]: Expand ruff checkers #495

Open boxydog opened 1 week ago

boxydog commented 1 week ago

Why is this issue important?

ruff does a good job of checking and auto-fixing python to be more correct and more consistent.

This project is just using the default 4 checkers, but there are dozens. One I like is UP, like pyupgrade, that updates all the code to be most idiomatic for our chosen python version (e.g., python 3.12). As an example, I noticed a bunch of Optional[str] type hints, which is now str | None.

Another which is aggressive but helpful is "Q" just to make sure all quotes are the same (default: double quotes). This is the kind of tiny stuff that it's good to just have a machine do.

In combination with pre-commit, ruff can auto-rewrite our code safely and with no effort as we push stuff up.

Current State

Default checkers.

Expected State

More checkers.

Implementation Plan

Relevant Code Snippets

# Below is an example I have from another project

[tool.ruff.lint]
# see https://docs.astral.sh/ruff/configuration/#using-pyprojecttoml
select = [
  # default Ruff checkers as of ruff 0.3.4: E4, E7, E9, F
  "E4",
  "E7",
  "E9",
  # "F" contains autoflake, see https://github.com/astral-sh/ruff/issues/1647
  "F",  # pyflakes

  # TODO: "A",   # flake8-builtins
  # TODO: "ARG", # flake8-unused-arguments
  "B",   # flake8-bugbear
  "BLE", # flake8-blind-except
  # TODO: Do I want "COM", # flake8-commas
  "C4",  # flake8-comprehensions
  # TODO: "DTZ", # flake8-datetimez
  # TODO: "EM",  # flake8-errmsg
  "EXE", # flake8-executable
  # TODO: "FURB", # refurb
  # TODO: "FBT", # flake8-boolean-trap
  # TODO: "G",   # flake8-logging-format
  "I",   # isort
  "ICN", # flake8-import-conventions
  "INP", # flake8-no-pep420
  # TODO: "INT", # flake8-gettext
  "ISC", # flake8-implicit-str-concat
  # TODO: "LOG", # flake8-logging
  "PERF", # perflint
  "PIE", # flake8-pie
  "PL",  # pylint
  "PYI", # flake8-pyi
  # TODO: "RET", # flake8-return
  "RSE", # flake8-raise
  "RUF",
  # TODO: "SIM", # flake8-simplify
  "SLF", # flake8-self
  "SLOT", # flake8-slots
  "TID", # flake8-tidy-imports
  "UP",  # pyupgrade
  "Q",   # flake8-quotes
  "TCH", # flake8-type-checking
  "T10", # flake8-debugger
  "T20", # flake8-print
  # TODO: "S",   # flake8-bandit
  "YTT", # flake8-2020
  # TODO: add more flake8 rules
]

ignore = [
  "ISC001",  # ruff formatter thinks we should ignore this one
  "PLR2004",
  "INP001",
  "T201",
  "RUF012",
  "SLF001",
  "PLR0913",
  "PLR0912",
  "PLW2901",
  "B904",
  "PLR0915",
  "RUF015",
  # I don't like RUF005, it looks less readable to me, and the performance improvement
  # is likely unimportant.
  "RUF005",
]
boxydog commented 1 week ago

For example, just adding "I", "UP", and "SIM" on my branch produced this:

$ ruff check --fix
src/functions/create_archive.py:88:5: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements
src/functions/generate_treasury_report.py:457:9: SIM118 Use `key in dict` instead of `key in dict.keys()`
src/functions/subrecipient_treasury_report_gen.py:135:5: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements
src/functions/validate_workbook.py:34:5: SIM117 Use a single `with` statement with multiple contexts instead of nested `with` statements
tests/conftest.py:32:12: SIM115 Use context handler for opening files
tests/conftest.py:231:12: SIM115 Use context handler for opening files
Found 318 errors (312 fixed, 6 remaining).
No fixes available (2 hidden fixes can be enabled with the `--unsafe-fixes` option).

$ git diff | wc -l
    2121

A lot of it is just re-ordering imports, and using a newer style of type hinting.

This is stuff we'd like a computer to keep straight for us.