Skip to content

Commit

Permalink
Optionally disable consider-using-join for non-empty separator (#9129)
Browse files Browse the repository at this point in the history
* 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 <pierre.sassoulas@gmail.com>
  • Loading branch information
theirix and Pierre-Sassoulas committed Oct 12, 2023
1 parent 3afa971 commit 0796dfa
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 2 deletions.
10 changes: 10 additions & 0 deletions doc/user_guide/configuration/all-options.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -1211,6 +1219,8 @@ Standard Checkers
never-returning-functions = ["sys.exit", "argparse.parse_error"]
suggest-join-with-non-empty-separator = true
.. raw:: html
Expand Down
3 changes: 3 additions & 0 deletions doc/whatsnew/fragments/8701.feature
Original file line number Diff line number Diff line change
@@ -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
26 changes: 24 additions & 2 deletions pylint/checkers/refactoring/refactoring_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": "<y or n>",
"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:
Expand All @@ -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] = []
Expand All @@ -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]:
Expand Down Expand Up @@ -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)
Expand All @@ -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:
Expand Down
Original file line number Diff line number Diff line change
@@ -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]
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[variables]
suggest-join-with-non-empty-separator=no

0 comments on commit 0796dfa

Please sign in to comment.