Do a double (or triple) pass on each file

Co-authored-by: J. Sebastian Paez <[email protected]>

Author: DanielNoord

pydocstringformatter/run.py

@@ -9,6 +9,7 @@
 
 from pydocstringformatter import __version__, _formatting, _utils
 from pydocstringformatter._configuration.arguments_manager import ArgumentsManager
+from pydocstringformatter._utils.exceptions import UnstableResultError
 
 
 class _Run:
@@ -64,7 +65,7 @@ def format_file(self, filename: Path) -> bool:
             # the same later on.
             newlines = file.newlines
 
-        formatted_tokens, is_changed = self.format_file_tokens(tokens)
+        formatted_tokens, is_changed = self.format_file_tokens(tokens, filename)
 
         if is_changed:
             try:
@@ -112,30 +113,91 @@ def get_enabled_formatters(self) -> dict[str, _formatting.Formatter]:
         return enabled
 
     def format_file_tokens(
-        self, tokens: list[tokenize.TokenInfo]
+        self, tokens: list[tokenize.TokenInfo], filename: Path
     ) -> tuple[list[tokenize.TokenInfo], bool]:
-        """Format a list of tokens."""
+        """Format a list of tokens.
+
+        tokens: List of tokens to format.
+        filename: Name of the file the tokens are from.
+
+        Returns:
+            A tuple containing [1] the formatted tokens in a list
+            and [2] a boolean indicating if the tokens were changed.
+
+        Raises:
+            UnstableResultError::
+                If the formatters are not able to get to a stable result.
+                It reports what formatters are still modifying the tokens.
+        """
         formatted_tokens: list[tokenize.TokenInfo] = []
         is_changed = False
 
         for index, tokeninfo in enumerate(tokens):
             new_tokeninfo = tokeninfo
 
             if _utils.is_docstring(new_tokeninfo, tokens[index - 1]):
-                for _, formatter in self.enabled_formatters.items():
-                    new_tokeninfo = formatter.treat_token(new_tokeninfo)
-            formatted_tokens.append(new_tokeninfo)
+                new_tokeninfo, changers = self.apply_formatters(new_tokeninfo)
+                is_changed = is_changed or bool(changers)
+
+                # Run formatters again (3rd time) to check if the result is stable
+                _, changers = self._apply_formatters_once(
+                    new_tokeninfo,
+                )
+
+                if changers:
+                    conflicting_formatters = {
+                        k: v
+                        for k, v in self.enabled_formatters.items()
+                        if k in changers
+                    }
+                    template = _utils.create_gh_issue_template(
+                        new_tokeninfo, conflicting_formatters, str(filename)
+                    )
 
-            if tokeninfo != new_tokeninfo:
-                is_changed = True
+                    raise UnstableResultError(template)
+
+            formatted_tokens.append(new_tokeninfo)
 
         return formatted_tokens, is_changed
 
+    def apply_formatters(
+        self, token: tokenize.TokenInfo
+    ) -> tuple[tokenize.TokenInfo, set[str]]:
+        """Apply the formatters twice to a token.
+
+        Also tracks which formatters changed the token.
+
+        Returns:
+            A tuple containing:
+            [1] the formatted token and
+            [2] a set of formatters that changed the token.
+        """
+        token, changers = self._apply_formatters_once(token)
+        if changers:
+            token, changers2 = self._apply_formatters_once(token)
+            changers.update(changers2)
+        return token, changers
+
+    def _apply_formatters_once(
+        self, token: tokenize.TokenInfo
+    ) -> tuple[tokenize.TokenInfo, set[str]]:
+        """Applies formatters to a token and keeps track of what changes it.
+
+        token: Token to apply formatters to
+
+        Returns:
+            A tuple containing [1] the formatted token and [2] a set
+            of formatters that changed the token.
+        """
+        changers: set[str] = set()
+        for formatter_name, formatter in self.enabled_formatters.items():
+            if (new_token := formatter.treat_token(token)) != token:
+                changers.add(formatter_name)
+                token = new_token
+
+        return token, changers
+
     def format_files(self, filepaths: list[Path]) -> bool:
         """Format a list of files."""
-        is_changed = False
-
-        for file in filepaths:
-            is_changed = self.format_file(file) or is_changed
-
-        return is_changed
+        is_changed = [self.format_file(file) for file in filepaths]
+        return any(is_changed)

tests/data/format/linewrap_summary/function_docstrings.py.out

@@ -1,13 +1,15 @@
 def func():
-    """A very long summary line that needs to be wrapped, A very long summary line that
+    """A very long summary line that needs to be wrapped, A very long summary line that.
+
     needs to be wrapped.
 
     A description that is not too long.
     """
 
 
 def func():
-    """A very long multi-line summary line that needs to be wrapped, A very long multi-
+    """A very long multi-line summary line that needs to be wrapped, A very long multi-.
+
     line summary line that needs to be wrapped.
 
     A very long summary line that needs to be wrapped.
@@ -28,13 +30,15 @@ def func():
 # Since the ending quotes will be appended on the same line this
 # exceeds the max length.
 def func():
-    """A multi-line summary that can be on one line, Something that is just too
+    """A multi-line summary that can be on one line, Something that is just too.
+
     longgg.
     """
 
 
 def func():
-    """A multi-line summary that can be on one line, Something that is just too
+    """A multi-line summary that can be on one line, Something that is just too.
+
     long.
     """
 
@@ -46,6 +50,7 @@ def func():
 # Regression for bug found in pylint
 # We should re-add the quotes to line length if they will never be on the first line.
 class LinesChunk:
-    """The LinesChunk object computes and stores the hash of some consecutive stripped
+    """The LinesChunk object computes and stores the hash of some consecutive stripped.
+
     lines of a lineset.
     """

tests/test_conflicting_formatters.py

@@ -0,0 +1,64 @@
+from __future__ import annotations
+
+from collections.abc import Generator
+from contextlib import contextmanager
+from pathlib import Path
+
+import pytest
+
+from pydocstringformatter import _formatting
+from pydocstringformatter import _testutils as test_utils
+from pydocstringformatter._formatting import Formatter
+from pydocstringformatter._utils import UnstableResultError
+from pydocstringformatter.run import _Run
+
+
+@contextmanager
+def patched_run(formatters: list[Formatter]) -> Generator[type[_Run], None, None]:
+    """Patches formatters so Run only uses the provided formatters."""
+    old_formatters = _formatting.FORMATTERS
+    _formatting.FORMATTERS = formatters
+    yield _Run
+    _formatting.FORMATTERS = old_formatters
+
+
+@pytest.mark.parametrize(
+    "formatters,expected_errors",
+    [
+        (
+            [test_utils.MakeAFormatter(), test_utils.MakeBFormatter()],
+            ["Conflicting formatters"],
+        ),
+        (
+            [test_utils.MakeBFormatter(), test_utils.AddBFormatter()],
+            ["not able to make stable changes"],
+        ),
+        (
+            [
+                test_utils.MakeBFormatter(),
+                test_utils.AddBFormatter(),
+                test_utils.MakeAFormatter(),
+            ],
+            ["Conflicting formatters:", "Diff too intricate to compute"],
+        ),
+    ],
+)
+def test_conflicting_formatters(
+    formatters: list[Formatter],
+    expected_errors: list[str],
+    tmp_path: Path,
+) -> None:
+    """Tests that conflicting formatters raise an error."""
+    tmp_file = tmp_path / "test.py"
+    with open(tmp_file, "w", encoding="utf-8") as f:
+        content = [
+            '"""AAA AA AAA"""',
+        ]
+        f.writelines(content)
+
+    with patched_run(formatters) as run:
+        with pytest.raises(UnstableResultError) as err:
+            run([str(tmp_file)])
+
+    for expect_err in expected_errors:
+        assert expect_err in str(err.value), str(err.value)