code review suggestions

Author: jspaezp

pydocstringformatter/run.py

@@ -136,17 +136,15 @@ def format_file_tokens(
             new_tokeninfo = tokeninfo
 
             if _utils.is_docstring(new_tokeninfo, tokens[index - 1]):
-                new_tokeninfo, changers, stable = self.apply_formatters_twice(
-                    new_tokeninfo
-                )
+                new_tokeninfo, changers = self.apply_formatters(new_tokeninfo)
                 is_changed = is_changed or bool(changers)
 
                 # Run formatters again to check if the result is stable
-                new_tokeninfo, changers, stable = self.apply_formatters_twice(
+                _, changers = self._apply_formatters_once(
                     new_tokeninfo,
                 )
 
-                if not stable:
+                if changers:
                     conflicting_formatters = {
                         k: v
                         for k, v in self.enabled_formatters.items()
@@ -162,9 +160,9 @@ def format_file_tokens(
 
         return formatted_tokens, is_changed
 
-    def apply_formatters_twice(
+    def apply_formatters(
         self, token: tokenize.TokenInfo
-    ) -> tuple[tokenize.TokenInfo, list[str], bool]:
+    ) -> tuple[tokenize.TokenInfo, list[str]]:
         """Apply the formatters twice to a token.
 
         Also tracks which formatters changed the token.
@@ -175,17 +173,13 @@ def apply_formatters_twice(
             [2] a list of formatters that changed the token.
             [3] a boolean indicating if the token was in the second pass.
         """
-        stable = False
-        token, changers = self.apply_formatters(token)
+        token, changers = self._apply_formatters_once(token)
         if changers:
-            token, changers2 = self.apply_formatters(token)
-            stable = not bool(changers2)
+            token, changers2 = self._apply_formatters_once(token)
             changers = list(set(changers + changers2))
-        else:
-            stable = True
-        return token, changers, stable
+        return token, changers
 
-    def apply_formatters(
+    def _apply_formatters_once(
         self, token: tokenize.TokenInfo
     ) -> tuple[tokenize.TokenInfo, list[str]]:
         """Applies formatters to a token and keeps track of what changes it.

tests/test_conflicting_formatters.py

@@ -1,23 +1,22 @@
 from __future__ import annotations
 
-import tempfile
 from collections.abc import Generator
+from pathlib import Path
 
 import pytest
 from _pytest.fixtures import SubRequest
 
+from pydocstringformatter import _formatting
 from pydocstringformatter import _testutils as test_utils
 from pydocstringformatter._utils import UnstableResultError
 from pydocstringformatter.run import _Run
 
 
 @pytest.fixture(scope="function")
-def patched_run(request: SubRequest) -> Generator[Type[_Run], None, None]:
+def patched_run(request: SubRequest) -> Generator[type[_Run], None, None]:
     """Patches the _Run class to use the MakeAFormatter and MakeBFormatter."""
     # This monkeypatches the formatters to make them conflict.
     # and the only formatters applied
-    # pylint: disable=wrong-import-position, import-outside-toplevel
-    from pydocstringformatter import _formatting  # noqa: E402
 
     old_formatters = _formatting.FORMATTERS
     _formatting.FORMATTERS = request.param
@@ -26,9 +25,8 @@ def cleanup() -> None:
         """Returns the formatters to their original state."""
         _formatting.FORMATTERS = old_formatters
 
-    request.addfinalizer(cleanup)
-
     yield _Run
+    cleanup()
 
 
 @pytest.mark.parametrize(
@@ -54,20 +52,20 @@ def cleanup() -> None:
     indirect=["patched_run"],
 )
 def test_conflicting_formatters(
-    patched_run: Type[_Run],  # pylint: disable=redefined-outer-name
-    expect_errors: List[str],
+    patched_run: type[_Run],  # pylint: disable=redefined-outer-name
+    expect_errors: list[str],
+    tmp_path: Path,
 ) -> None:
     """Tests that conflicting formatters raise an error."""
-    with tempfile.TemporaryDirectory() as tmp:
-        tmp_file = tmp + "/test.py"
-        with open(tmp_file, "w", encoding="utf-8") as f:
-            content = [
-                '"""AAA AA AAA"""',
-            ]
-            f.writelines(content)
+    tmp_file = tmp_path / "test.py"
+    with open(tmp_file, "w", encoding="utf-8") as f:
+        content = [
+            '"""AAA AA AAA"""',
+        ]
+        f.writelines(content)
 
-        with pytest.raises(UnstableResultError) as err:
-            patched_run([tmp_file])
+    with pytest.raises(UnstableResultError) as err:
+        patched_run([str(tmp_file)])
 
-        for expect_err in expect_errors:
-            assert expect_err in str(err.value), str(err.value)
+    for expect_err in expect_errors:
+        assert expect_err in str(err.value), str(err.value)