From 18a5e263c75920b0a138abd16fdf61bd20ec4feb Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Mon, 11 Mar 2024 15:02:53 -0700 Subject: [PATCH] Update Ruff configuration Use Ruff for formatting as well as linting, and drop Black for code. Update the version of Black used for documentation and apply the formatting changes it wants to make. --- .pre-commit-config.yaml | 8 ++------ docs/user-guide/arq.rst | 6 +++--- docs/user-guide/fastapi-errors.rst | 3 +-- pyproject.toml | 27 ++++++++++++++++++++++++--- src/safir/asyncio.py | 2 +- src/safir/dependencies/gafaelfawr.py | 4 ++-- src/safir/pydantic.py | 2 +- src/safir/testing/kubernetes.py | 3 ++- tests/dependencies/gafaelfawr_test.py | 2 +- tests/logging_test.py | 4 ++-- tests/middleware/ivoa_test.py | 2 +- 11 files changed, 40 insertions(+), 23 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 97d36a04..de34be07 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -12,15 +12,11 @@ repos: hooks: - id: ruff args: [--fix, --exit-non-zero-on-fix] - - - repo: https://github.com/ambv/black - rev: 24.2.0 - hooks: - - id: black + - id: ruff-format - repo: https://github.com/asottile/blacken-docs rev: 1.16.0 hooks: - id: blacken-docs - additional_dependencies: [black==23.1.0] + additional_dependencies: [black==24.2.0] args: [-l, '76', -t, py311] diff --git a/docs/user-guide/arq.rst b/docs/user-guide/arq.rst index dd5b7764..9b432983 100644 --- a/docs/user-guide/arq.rst +++ b/docs/user-guide/arq.rst @@ -71,9 +71,9 @@ If your app uses a configuration system like ``pydantic.BaseSettings``, this exa redis_settings = RedisSettings( host=url_parts.hostname or "localhost", port=url_parts.port or 6379, - database=int(url_parts.path.lstrip("/")) - if url_parts.path - else 0, + database=( + int(url_parts.path.lstrip("/")) if url_parts.path else 0 + ), ) return redis_settings diff --git a/docs/user-guide/fastapi-errors.rst b/docs/user-guide/fastapi-errors.rst index 285aa3fe..178544ad 100644 --- a/docs/user-guide/fastapi-errors.rst +++ b/docs/user-guide/fastapi-errors.rst @@ -147,8 +147,7 @@ For example, for a single route: ..., responses={404: {"description": "Not found", "model": ErrorModel}}, ) - async def route(foo: str) -> None: - ... + async def route(foo: str) -> None: ... If all routes provided by a router have the same error handling behavior for a given response code, it saves some effort to instead do this when including the router, normally in ``main.py``: diff --git a/pyproject.toml b/pyproject.toml index 1e8c942a..c9c4f1ce 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -211,10 +211,13 @@ ignore = [ "FIX002", # point of a TODO comment is that we're not ready to fix it "G004", # forbidding logging f-strings is appealing, but not our style "RET505", # disagree that omitting else always makes code more readable + "PLR0911", # often many returns is clearer and simpler style "PLR0913", # factory pattern uses constructors with many arguments "PLR2004", # too aggressive about magic values + "PLW0603", # yes global is discouraged but if needed, it's needed "S105", # good idea but too many false positives on non-passwords "S106", # good idea but too many false positives on non-passwords + "S107", # good idea but too many false positives on non-passwords "S603", # not going to manually mark every subprocess call as reviewed "S607", # using PATH is not a security vulnerability "SIM102", # sometimes the formatting of nested if statements is clearer @@ -225,14 +228,31 @@ ignore = [ "TD003", # we don't require issues be created for TODOs "TID252", # if we're going to use relative imports, use them always "TRY003", # good general advice but lint is way too aggressive + "TRY301", # sometimes raising exceptions inside try is the best flow - # Safir-specific rules. - "N818", # Exception is correct in some cases, others are part of API - "PLW0603", # necessary trick for safir.logging + # The following settings should be disabled when using ruff format + # per https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules + "W191", + "E111", + "E114", + "E117", + "D206", + "D300", + "Q000", + "Q001", + "Q002", + "Q003", + "COM812", + "COM819", + "ISC001", + "ISC002", ] select = ["ALL"] [tool.ruff.lint.per-file-ignores] +"src/safir/**" = [ + "N818", # Exception is correct in some cases, others are part of API +] "src/safir/testing/**" = [ "S101", # test support functions are allowed to use assert ] @@ -243,6 +263,7 @@ select = ["ALL"] "PLR0915", # tests are allowed to be long, sometimes that's convenient "PT012", # way too aggressive about limiting pytest.raises blocks "S101", # tests should use assert + "S106", # tests are allowed to hard-code dummy passwords "SLF001", # tests are allowed to access private members ] diff --git a/src/safir/asyncio.py b/src/safir/asyncio.py index f817846d..53d33448 100644 --- a/src/safir/asyncio.py +++ b/src/safir/asyncio.py @@ -200,7 +200,7 @@ def qsize(self) -> int: def run_with_asyncio( - f: Callable[P, Coroutine[None, None, T]] + f: Callable[P, Coroutine[None, None, T]], ) -> Callable[P, T]: """Run the decorated function with `asyncio.run`. diff --git a/src/safir/dependencies/gafaelfawr.py b/src/safir/dependencies/gafaelfawr.py index ef4b26de..50beff89 100644 --- a/src/safir/dependencies/gafaelfawr.py +++ b/src/safir/dependencies/gafaelfawr.py @@ -15,7 +15,7 @@ async def auth_dependency( - x_auth_request_user: Annotated[str, Header(include_in_schema=False)] + x_auth_request_user: Annotated[str, Header(include_in_schema=False)], ) -> str: """Retrieve authentication information from HTTP headers. @@ -27,7 +27,7 @@ async def auth_dependency( async def auth_delegated_token_dependency( - x_auth_request_token: Annotated[str, Header(include_in_schema=False)] + x_auth_request_token: Annotated[str, Header(include_in_schema=False)], ) -> str: """Retrieve Gafaelfawr delegated token from HTTP headers. diff --git a/src/safir/pydantic.py b/src/safir/pydantic.py index 8bed00d7..813de1e1 100644 --- a/src/safir/pydantic.py +++ b/src/safir/pydantic.py @@ -158,7 +158,7 @@ class Model(BaseModel): def _copy_type( - parent: Callable[P, T] + parent: Callable[P, T], ) -> Callable[[Callable[..., T]], Callable[P, T]]: """Copy the type of a parent method. diff --git a/src/safir/testing/kubernetes.py b/src/safir/testing/kubernetes.py index 7d0c9c16..b10a0eec 100644 --- a/src/safir/testing/kubernetes.py +++ b/src/safir/testing/kubernetes.py @@ -169,7 +169,8 @@ class _KubernetesModel(Protocol): """ def to_dict( - self, serialize: bool = False # noqa: FBT001, FBT002 + self, + serialize: bool = False, # noqa: FBT001, FBT002 ) -> dict[str, Any]: ... diff --git a/tests/dependencies/gafaelfawr_test.py b/tests/dependencies/gafaelfawr_test.py index 5b4e5768..663c502d 100644 --- a/tests/dependencies/gafaelfawr_test.py +++ b/tests/dependencies/gafaelfawr_test.py @@ -26,7 +26,7 @@ async def test_auth_dependency() -> None: @app.get("/") async def handler( - user: Annotated[str, Depends(auth_dependency)] + user: Annotated[str, Depends(auth_dependency)], ) -> dict[str, str]: return {"user": user} diff --git a/tests/logging_test.py b/tests/logging_test.py index 03fd0e83..35790076 100644 --- a/tests/logging_test.py +++ b/tests/logging_test.py @@ -208,7 +208,7 @@ def test_dev_exception_logging(caplog: LogCaptureFixture) -> None: logger = structlog.get_logger("myapp") try: - raise ValueError("this is some exception") # noqa: TRY301 + raise ValueError("this is some exception") except Exception: logger.exception("exception happened", foo="bar") @@ -226,7 +226,7 @@ def test_production_exception_logging(caplog: LogCaptureFixture) -> None: logger = structlog.get_logger("myapp") try: - raise ValueError("this is some exception") # noqa: TRY301 + raise ValueError("this is some exception") except Exception: logger.exception("exception happened", foo="bar") diff --git a/tests/middleware/ivoa_test.py b/tests/middleware/ivoa_test.py index 5866b286..77b625fd 100644 --- a/tests/middleware/ivoa_test.py +++ b/tests/middleware/ivoa_test.py @@ -32,7 +32,7 @@ async def simple_handler() -> dict[str, str]: @app.get("/list") async def list_handler( - param: Annotated[list[str], Query()] + param: Annotated[list[str], Query()], ) -> dict[str, list[str]]: return {"param": param}