diff --git a/pydocstringformatter/_testutils/__init__.py b/pydocstringformatter/_testutils/__init__.py index e11db4c7..c0a95bd6 100644 --- a/pydocstringformatter/_testutils/__init__.py +++ b/pydocstringformatter/_testutils/__init__.py @@ -10,6 +10,7 @@ from pydocstringformatter import run_docstring_formatter from pydocstringformatter._formatting import Formatter from pydocstringformatter._testutils.example_formatters import ( + AddBFormatter, MakeAFormatter, MakeBFormatter, ) @@ -83,4 +84,4 @@ def __exit__( return None -__all__ = ["FormatterAsserter", "MakeAFormatter", "MakeBFormatter"] +__all__ = ["FormatterAsserter", "MakeAFormatter", "MakeBFormatter", "AddBFormatter"] diff --git a/pydocstringformatter/_testutils/example_formatters.py b/pydocstringformatter/_testutils/example_formatters.py index 47b214c5..f2a587c9 100644 --- a/pydocstringformatter/_testutils/example_formatters.py +++ b/pydocstringformatter/_testutils/example_formatters.py @@ -27,3 +27,16 @@ def treat_token(self, tokeninfo: tokenize.TokenInfo) -> tokenize.TokenInfo: token_dict = tokeninfo._asdict() token_dict["string"] = token_dict["string"].replace("A", "B") return type(tokeninfo)(**token_dict) + + +class AddBFormatter(Formatter): + """A formatter that adds Bs.""" + + name = "add-b-formatter" + style = ["default"] + + def treat_token(self, tokeninfo: tokenize.TokenInfo) -> tokenize.TokenInfo: + """Add a B to the end of the string.""" + token_dict = tokeninfo._asdict() + token_dict["string"] += "B" + return type(tokeninfo)(**token_dict) diff --git a/pydocstringformatter/_utils/__init__.py b/pydocstringformatter/_utils/__init__.py index 163ce4d3..99972f85 100644 --- a/pydocstringformatter/_utils/__init__.py +++ b/pydocstringformatter/_utils/__init__.py @@ -2,10 +2,12 @@ ParsingError, PydocstringFormatterError, TomlParsingError, + UnstableResultError, ) from pydocstringformatter._utils.file_diference import compare_formatters, generate_diff from pydocstringformatter._utils.find_docstrings import is_docstring from pydocstringformatter._utils.find_python_file import find_python_files +from pydocstringformatter._utils.issue_template import create_gh_issue_template from pydocstringformatter._utils.output import print_to_console, sys_exit __all__ = [ @@ -16,6 +18,8 @@ "ParsingError", "PydocstringFormatterError", "TomlParsingError", + "UnstableResultError", + "create_gh_issue_template", "print_to_console", "sys_exit", ] diff --git a/pydocstringformatter/_utils/exceptions.py b/pydocstringformatter/_utils/exceptions.py index 06b15664..3719b797 100644 --- a/pydocstringformatter/_utils/exceptions.py +++ b/pydocstringformatter/_utils/exceptions.py @@ -12,3 +12,7 @@ class UnrecognizedOption(PydocstringFormatterError): class TomlParsingError(PydocstringFormatterError): """Raised when there are errors with the parsing of the toml file.""" + + +class UnstableResultError(PydocstringFormatterError): + """Raised when the result of the formatting is unstable.""" diff --git a/pydocstringformatter/_utils/issue_template.py b/pydocstringformatter/_utils/issue_template.py new file mode 100644 index 00000000..0be8192b --- /dev/null +++ b/pydocstringformatter/_utils/issue_template.py @@ -0,0 +1,78 @@ +from __future__ import annotations + +import tokenize + +from pydocstringformatter._formatting import Formatter +from pydocstringformatter._utils.file_diference import compare_formatters + + +def create_gh_issue_template( + token: tokenize.TokenInfo, formatters: dict[str, Formatter], filename: str +) -> str: + """Make a template for a GitHub issue. + + Args: + token: The token that caused the issue. + formatters: The formatters that caused the issue. + filename: The filename of the file that caused the issue. + """ + formatter_names = list(formatters) + msg = "" + if len(formatter_names) > 2: + msg = f""" +Conflicting formatters: {", ".join(formatters)} +""" + diff = f"Diff too intricate to compute for {len(formatter_names)} formatters." + else: + if len(formatter_names) == 2: + msg = f""" +Conflicting formatters: {" and ".join(formatter_names)} +These formatters conflict with each other for: + +```python +{token.string} +``` +""" + formatter_1 = formatters[formatter_names[0]] + formatter_2 = formatters[formatter_names[1]] + else: + msg = f""" +Formatter: {formatter_names[0]} +This formatter is not able to make stable changes for: + +```python +{token.string} +``` +""" + formatter_1 = formatters[formatter_names[0]] + formatter_2 = formatter_1 + + diff = compare_formatters( + token, + formatter_1, + formatter_2, + title_extra=str(filename), + ) + + out = f""" +Unfortunately an error occurred while formatting a docstring. +Please help us fix this bug by opening an issue at: +https://github.com/DanielNoord/pydocstringformatter/issues/new + +{"-" * 80} + +You can use the following template when you open the issue: + +# Description: + +{msg} + +# Diff: + +```diff +{diff} +``` + +""" + + return out diff --git a/pydocstringformatter/run.py b/pydocstringformatter/run.py index dffaf120..2556efab 100644 --- a/pydocstringformatter/run.py +++ b/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,9 +113,22 @@ 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 @@ -122,20 +136,68 @@ def format_file_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) diff --git a/tests/data/format/linewrap_summary/function_docstrings.py.out b/tests/data/format/linewrap_summary/function_docstrings.py.out index 816d3842..9160c296 100644 --- a/tests/data/format/linewrap_summary/function_docstrings.py.out +++ b/tests/data/format/linewrap_summary/function_docstrings.py.out @@ -1,5 +1,6 @@ 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. @@ -7,7 +8,8 @@ def func(): 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. """ diff --git a/tests/test_conflicting_formatters.py b/tests/test_conflicting_formatters.py new file mode 100644 index 00000000..d38e99d0 --- /dev/null +++ b/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)