From 0945361426a47ef0a590c4689529869b7a2ba6c1 Mon Sep 17 00:00:00 2001 From: Francesco Pezzella Date: Thu, 5 Nov 2026 18:07:47 +0100 Subject: [PATCH] Add run_ruff_format to ruff_utils and document both helpers; the type parameter CLI's ruff pass no longer runs UP049, whose fix can leave a same-named TypeVar behind (astral-sh/ruff#21004). --- docs/developer/feature-test-map/core.md | 2 + docs/developer/modules/recipes.md | 8 ++- .../features/type-parameter-modernization.md | 9 ++-- src/renaissance/utils/ruff_utils.py | 16 +++++- test/utils/test_ruff_utils.py | 49 +++++++++++++++++-- 5 files changed, 68 insertions(+), 16 deletions(-) diff --git a/docs/developer/feature-test-map/core.md b/docs/developer/feature-test-map/core.md index 5fb2f1d7..f8349253 100644 --- a/docs/developer/feature-test-map/core.md +++ b/docs/developer/feature-test-map/core.md @@ -56,6 +56,7 @@ https://github.com/TNO/Renaissance.Py/blob/main/LICENSE - `test/recipes/test_python_refactoring.py` - `test/utils/test_cst_utils.py` - `test/utils/test_import_resolution.py` + - `test/utils/test_ruff_utils.py` - `test/rejuvenation/test_migration_type_recipes.py` - **Code file(s):** `src/renaissance/recipes/type_parameter_check.py`, `src/renaissance/recipes/type_parameter_rules.py`, `src/renaissance/recipes/type_parameter_declarations.py`, `src/renaissance/recipes/type_parameter_localization.py`, @@ -63,4 +64,5 @@ https://github.com/TNO/Renaissance.Py/blob/main/LICENSE `src/renaissance/recipes/step_runner.py`, `src/renaissance/recipes/python_refactoring.py`, `src/renaissance/utils/import_resolution.py`, `src/renaissance/utils/cst_utils.py`, + `src/renaissance/utils/ruff_utils.py`, `src/rejuvenation/migration_type_recipes.py` diff --git a/docs/developer/modules/recipes.md b/docs/developer/modules/recipes.md index 54c9e99f..28f06ec4 100644 --- a/docs/developer/modules/recipes.md +++ b/docs/developer/modules/recipes.md @@ -34,11 +34,9 @@ safely cover. This page covers `TypeParameterCheck`, the recipe behind names each project file imports from the others. - `src/renaissance/utils/cst_utils.py`: a module's top-level statements, the names they read or bind and the comment trailing a statement's line. - - - +- `src/renaissance/utils/ruff_utils.py`: `run_ruff_import_cleanup` and `run_ruff_format`, which the CLI runs on the + files the recipe changed. The first removes unused imports and sorts the imports, the second formats the files. + ## Public entry points - `TypeParameterCheck.check()` (and `run()`): runs the three phases through `run_steps`, committing to disk between them. diff --git a/docs/user/features/type-parameter-modernization.md b/docs/user/features/type-parameter-modernization.md index 2ffa60c0..387ed870 100644 --- a/docs/user/features/type-parameter-modernization.md +++ b/docs/user/features/type-parameter-modernization.md @@ -404,6 +404,7 @@ run the tool again. - `test/recipes/test_python_refactoring.py` - `test/utils/test_cst_utils.py` - `test/utils/test_import_resolution.py` +- `test/utils/test_ruff_utils.py` - `test/rejuvenation/test_migration_type_recipes.py` ## Implemented by code modules @@ -411,7 +412,7 @@ run the tool again. - [Refactoring recipes](../../developer/modules/recipes.md) - `src/renaissance/recipes/type_parameter_check.py`, `type_parameter_rules.py`, `type_parameter_declarations.py`, `type_parameter_localization.py`, `type_parameter_uses.py`, `step_runner.py` and `python_refactoring.py`. -- `src/renaissance/utils/import_resolution.py` and `cst_utils.py` +- `src/renaissance/utils/import_resolution.py`, `cst_utils.py` and `ruff_utils.py` - `src/rejuvenation/migration_type_recipes.py` (the CLI) ## API entry points @@ -432,12 +433,10 @@ python src/rejuvenation/migration_type_recipes.py --py MAJOR.MINOR \ - `--second-pass`: process again only the files that kept a declaration because another file imported it when the run started, and merge the result into the same report. -After the recipe, the CLI runs `ruff check --fix --select F401,I001,UP049` on the changed files. This removes **every** +After the recipe, the CLI runs `ruff check --fix --select F401,I001` on the changed files. This removes **every** unused import in them, including ones that were unused before (such as `from typing import TypeVar` after the declaration is gone). It also merges and sorts **every** import block in them, including imports that were unsorted -before, using the `ruff` settings of the project. It removes the leading underscores of **every** PEP 695 type -parameter in them, so `def f[_T]` becomes `def f[T]`, unless the new name would shadow another one. Docstrings, -comments and the report keep the old name. The project's `select` and `ignore` lists do not apply to these three +before, using the `ruff` settings of the project. The project's `select` and `ignore` lists do not apply to these two rules. Then it runs `ruff format` on the same files, which reformats **every** line in them with the project's `ruff format` settings, not only the changed ones. `ruff` must be installed in the same environment. Files are changed directly, so run it on a git checkout. diff --git a/src/renaissance/utils/ruff_utils.py b/src/renaissance/utils/ruff_utils.py index b1914e7d..81ed43b0 100644 --- a/src/renaissance/utils/ruff_utils.py +++ b/src/renaissance/utils/ruff_utils.py @@ -4,7 +4,7 @@ # of this repository, also available at: # https://github.com/TNO/Renaissance.Py/blob/main/LICENSE -"""Ruff fixes for the files a recipe changed.""" +"""Ruff fixes and formatting for the files a recipe changed.""" import subprocess import sys @@ -25,3 +25,17 @@ def run_ruff_import_cleanup(paths: Sequence[Path]) -> None: capture_output=True, check=False, ) + + +def run_ruff_format(paths: Sequence[Path]) -> None: + """Format each whole file in place. + + Runs `ruff format` with the ruff settings that apply to each path. Does nothing when paths is empty. + """ + if not paths: + return + subprocess.run( # noqa: S603 (fixed arguments plus the caller's paths, no shell) + [sys.executable, "-m", "ruff", "format", *(str(path) for path in paths)], + capture_output=True, + check=False, + ) diff --git a/test/utils/test_ruff_utils.py b/test/utils/test_ruff_utils.py index ee7f725a..416fc622 100644 --- a/test/utils/test_ruff_utils.py +++ b/test/utils/test_ruff_utils.py @@ -4,18 +4,25 @@ # of this repository, also available at: # https://github.com/TNO/Renaissance.Py/blob/main/LICENSE -"""Tests for the ruff import cleanup helper.""" +"""Tests for the ruff helpers that clean up and format the files a recipe changed.""" +from collections.abc import Callable, Sequence from pathlib import Path import pytest from hamcrest import assert_that, equal_to -from renaissance.utils.ruff_utils import run_ruff_import_cleanup +from renaissance.utils.ruff_utils import run_ruff_format, run_ruff_import_cleanup UNUSED_IMPORT = ("import os\nimport sys\n\nprint(sys.argv)\n", "import sys\n\nprint(sys.argv)\n") UNSORTED_IMPORTS = ("import sys\nimport os\n\nprint(os, sys)\n", "import os\nimport sys\n\nprint(os, sys)\n") NOTHING_TO_FIX = ("import os\n\nprint(os)\n", "import os\n\nprint(os)\n") +UNFORMATTED = ("x = { 'a':1 }\n", 'x = {"a": 1}\n') +LONG_SIGNATURE = ( + "def f[T](first: list[T], second: list[T]) -> T: ...\n", + "def f[T](\n first: list[T], second: list[T]\n) -> T: ...\n", +) +ALREADY_FORMATTED = ('x = {"a": 1}\n', 'x = {"a": 1}\n') @pytest.mark.parametrize( @@ -38,13 +45,45 @@ def test_cleans_up_the_imports_of_each_file(tmp_path: Path, cases: list[tuple[st assert_that([path.read_text(encoding="utf-8") for path in paths], equal_to([expected for _, expected in cases])) -def test_no_paths_changes_nothing(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: +@pytest.mark.parametrize( + "cases", + [ + pytest.param([UNFORMATTED], id="unformatted"), + pytest.param([LONG_SIGNATURE], id="long_signature"), + pytest.param([ALREADY_FORMATTED], id="already_formatted"), + pytest.param([UNFORMATTED, LONG_SIGNATURE], id="several_files"), + ], +) +def test_formats_each_file(tmp_path: Path, cases: list[tuple[str, str]]) -> None: + """Each file is rewritten in the ruff format style, with the line length of the ruff settings next to it.""" + (tmp_path / "ruff.toml").write_text("line-length = 40\n", encoding="utf-8") + paths = [tmp_path / f"module_{index}.py" for index in range(len(cases))] + for path, (source, _) in zip(paths, cases, strict=True): + path.write_text(source, encoding="utf-8") + + run_ruff_format(paths) + + assert_that([path.read_text(encoding="utf-8") for path in paths], equal_to([expected for _, expected in cases])) + + +@pytest.mark.parametrize( + ("ruff_helper", "source"), + [ + pytest.param(run_ruff_import_cleanup, UNUSED_IMPORT[0], id="import_cleanup"), + pytest.param(run_ruff_format, UNFORMATTED[0], id="format"), + ], +) +def test_no_paths_changes_nothing( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + ruff_helper: Callable[[Sequence[Path]], None], + source: str, +) -> None: """An empty list leaves the files in the working directory untouched.""" - source, _ = UNUSED_IMPORT path = tmp_path / "module.py" path.write_text(source, encoding="utf-8") monkeypatch.chdir(tmp_path) - run_ruff_import_cleanup([]) + ruff_helper([]) assert_that(path.read_text(encoding="utf-8"), equal_to(source))