From 0796dfadf2358dd0319528f1f696a58dfcdce425 Mon Sep 17 00:00:00 2001 From: theirix Date: Thu, 12 Oct 2023 17:06:31 +0300 Subject: [PATCH] Optionally disable consider-using-join for non-empty separator (#9129) * Skip consider-using-join for non-empty separators Only suggest joining with separators when a new option 'suggest-join-with-non-empty-separator' is specified. Co-authored-by: Pierre Sassoulas --- doc/user_guide/configuration/all-options.rst | 10 +++++++ doc/whatsnew/fragments/8701.feature | 3 +++ .../refactoring/refactoring_checker.py | 26 +++++++++++++++++-- .../consider_join_for_non_empty_separator.py | 24 +++++++++++++++++ .../consider_join_for_non_empty_separator.rc | 2 ++ 5 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 doc/whatsnew/fragments/8701.feature create mode 100644 tests/functional/c/consider/consider_join_for_non_empty_separator.py create mode 100644 tests/functional/c/consider/consider_join_for_non_empty_separator.rc diff --git a/doc/user_guide/configuration/all-options.rst b/doc/user_guide/configuration/all-options.rst index 178ee0b195..94d2c1775e 100644 --- a/doc/user_guide/configuration/all-options.rst +++ b/doc/user_guide/configuration/all-options.rst @@ -1196,6 +1196,14 @@ Standard Checkers **Default:** ``('sys.exit', 'argparse.parse_error')`` +--suggest-join-with-non-empty-separator +""""""""""""""""""""""""""""""""""""""" +*Let 'consider-using-join' be raised when the separator to join on would be non-empty (resulting in expected fixes of the type: ``"- " + " +- ".join(items)``)* + +**Default:** ``True`` + + .. raw:: html @@ -1211,6 +1219,8 @@ Standard Checkers never-returning-functions = ["sys.exit", "argparse.parse_error"] + suggest-join-with-non-empty-separator = true + .. raw:: html diff --git a/doc/whatsnew/fragments/8701.feature b/doc/whatsnew/fragments/8701.feature new file mode 100644 index 0000000000..4a9781accf --- /dev/null +++ b/doc/whatsnew/fragments/8701.feature @@ -0,0 +1,3 @@ +Skip ``consider-using-join`` check for non-empty separators if an ``suggest-join-with-non-empty-separator`` option is set to ``no``. + +Closes #8701 diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index d29326693a..3b074a7c02 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -506,6 +506,19 @@ class RefactoringChecker(checkers.BaseTokenChecker): "and no message will be printed.", }, ), + ( + "suggest-join-with-non-empty-separator", + { + "default": True, + "type": "yn", + "metavar": "", + "help": ( + "Let 'consider-using-join' be raised when the separator to " + "join on would be non-empty (resulting in expected fixes " + 'of the type: ``"- " + "\n- ".join(items)``)' + ), + }, + ), ) def __init__(self, linter: PyLinter) -> None: @@ -514,6 +527,7 @@ def __init__(self, linter: PyLinter) -> None: self._consider_using_with_stack = ConsiderUsingWithStack() self._init() self._never_returning_functions: set[str] = set() + self._suggest_join_with_non_empty_separator: bool = False def _init(self) -> None: self._nested_blocks: list[NodesWithNestedBlocks] = [] @@ -527,6 +541,9 @@ def open(self) -> None: self._never_returning_functions = set( self.linter.config.never_returning_functions ) + self._suggest_join_with_non_empty_separator = ( + self.linter.config.suggest_join_with_non_empty_separator + ) @cached_property def _dummy_rgx(self) -> Pattern[str]: @@ -1667,8 +1684,7 @@ def _dict_literal_suggestion(node: nodes.Call) -> str: suggestion = ", ".join(elements) return f"{{{suggestion}{', ... ' if len(suggestion) > 64 else ''}}}" - @staticmethod - def _name_to_concatenate(node: nodes.NodeNG) -> str | None: + def _name_to_concatenate(self, node: nodes.NodeNG) -> str | None: """Try to extract the name used in a concatenation loop.""" if isinstance(node, nodes.Name): return cast("str | None", node.name) @@ -1680,6 +1696,12 @@ def _name_to_concatenate(node: nodes.NodeNG) -> str | None: ] if len(values) != 1 or not isinstance(values[0].value, nodes.Name): return None + # If there are more values in joined string than formatted values, + # they are probably separators. + # Allow them only if the option `suggest-join-with-non-empty-separator` is set + with_separators = len(node.values) > len(values) + if with_separators and not self._suggest_join_with_non_empty_separator: + return None return cast("str | None", values[0].value.name) def _check_consider_using_join(self, aug_assign: nodes.AugAssign) -> None: diff --git a/tests/functional/c/consider/consider_join_for_non_empty_separator.py b/tests/functional/c/consider/consider_join_for_non_empty_separator.py new file mode 100644 index 0000000000..16a5f370c7 --- /dev/null +++ b/tests/functional/c/consider/consider_join_for_non_empty_separator.py @@ -0,0 +1,24 @@ +# pylint: disable=missing-docstring,invalid-name,undefined-variable,multiple-statements + +result = 'a' +for number in ['1', '2', '3']: + result += f'b{number}' +assert result == 'ab1b2b3' +assert result == 'b'.join(['a', '1', '2', '3']) + +result = 'a' +for number in ['1', '2', '3']: + result += f'{number}c' +assert result == 'a1c2c3c' +assert result == 'a' + 'c'.join(['1', '2', '3']) + 'c' + +result = 'a' +for number in ['1', '2', '3']: + result += f'b{number}c' +assert result == 'ab1cb2cb3c' +assert result == 'ab' + 'cb'.join(['1', '2', '3']) + 'c' + +result = '' +for number in ['1', '2', '3']: + result += f"{number}, " +result = result[:-2] diff --git a/tests/functional/c/consider/consider_join_for_non_empty_separator.rc b/tests/functional/c/consider/consider_join_for_non_empty_separator.rc new file mode 100644 index 0000000000..0d6adeb837 --- /dev/null +++ b/tests/functional/c/consider/consider_join_for_non_empty_separator.rc @@ -0,0 +1,2 @@ +[variables] +suggest-join-with-non-empty-separator=no