Skip to content

Commit

Permalink
Fix suggestion for min-max with expressions (#9131) (#9141)
Browse files Browse the repository at this point in the history
Closes #8524

(cherry picked from commit ce8d1a9)

Co-authored-by: theirix <theirix@gmail.com>
  • Loading branch information
github-actions[bot] and theirix authored Oct 11, 2023
1 parent f2cded4 commit a9d9dc3
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 5 deletions.
3 changes: 3 additions & 0 deletions doc/whatsnew/fragments/8524.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fixes suggestion for ``nested-min-max`` for expressions with additive operators, list and dict comprehensions.

Closes #8524
41 changes: 36 additions & 5 deletions pylint/checkers/nested_min_max.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

from pylint.checkers import BaseChecker
from pylint.checkers.utils import only_required_for_messages, safe_infer
from pylint.constants import PY39_PLUS
from pylint.interfaces import INFERENCE

if TYPE_CHECKING:
Expand Down Expand Up @@ -93,13 +94,10 @@ def visit_call(self, node: nodes.Call) -> None:

for idx, arg in enumerate(fixed_node.args):
if not isinstance(arg, nodes.Const):
inferred = safe_infer(arg)
if isinstance(
inferred, (nodes.List, nodes.Tuple, nodes.Set, *DICT_TYPES)
):
if self._is_splattable_expression(arg):
splat_node = nodes.Starred(
ctx=Context.Load,
lineno=inferred.lineno,
lineno=arg.lineno,
col_offset=0,
parent=nodes.NodeNG(
lineno=None,
Expand All @@ -125,6 +123,39 @@ def visit_call(self, node: nodes.Call) -> None:
confidence=INFERENCE,
)

def _is_splattable_expression(self, arg: nodes.NodeNG) -> bool:
"""Returns true if expression under min/max could be converted to splat
expression.
"""
# Support sequence addition (operator __add__)
if isinstance(arg, nodes.BinOp) and arg.op == "+":
return self._is_splattable_expression(
arg.left
) and self._is_splattable_expression(arg.right)
# Support dict merge (operator __or__ in Python 3.9)
if isinstance(arg, nodes.BinOp) and arg.op == "|" and PY39_PLUS:
return self._is_splattable_expression(
arg.left
) and self._is_splattable_expression(arg.right)

inferred = safe_infer(arg)
if inferred and inferred.pytype() in {"builtins.list", "builtins.tuple"}:
return True
if isinstance(
inferred or arg,
(
nodes.List,
nodes.Tuple,
nodes.Set,
nodes.ListComp,
nodes.DictComp,
*DICT_TYPES,
),
):
return True

return False


def register(linter: PyLinter) -> None:
linter.register_checker(NestedMinMaxChecker(linter))
12 changes: 12 additions & 0 deletions tests/functional/n/nested_min_max.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,15 @@

lst2 = [3, 7, 10]
max(3, max(nums), max(lst2)) # [nested-min-max]

max(3, max([5] + [6, 7])) # [nested-min-max]
max(3, *[5] + [6, 7])

max(3, max([5] + [i for i in range(4) if i])) # [nested-min-max]
max(3, *[5] + [i for i in range(4) if i])

max(3, max([5] + list(range(4)))) # [nested-min-max]
max(3, *[5] + list(range(4)))

max(3, max(list(range(4)))) # [nested-min-max]
max(3, *list(range(4)))
4 changes: 4 additions & 0 deletions tests/functional/n/nested_min_max.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,7 @@ nested-min-max:33:0:33:17::Do not use nested call of 'max'; it's possible to do
nested-min-max:37:0:37:17::Do not use nested call of 'max'; it's possible to do 'max(3, *nums)' instead:INFERENCE
nested-min-max:40:0:40:26::Do not use nested call of 'max'; it's possible to do 'max(3, *nums.values())' instead:INFERENCE
nested-min-max:44:0:44:28::Do not use nested call of 'max'; it's possible to do 'max(3, *nums, *lst2)' instead:INFERENCE
nested-min-max:46:0:46:25::Do not use nested call of 'max'; it's possible to do 'max(3, *[5] + [6, 7])' instead:INFERENCE
nested-min-max:49:0:49:45::Do not use nested call of 'max'; it's possible to do 'max(3, *[5] + [i for i in range(4) if i])' instead:INFERENCE
nested-min-max:52:0:52:33::Do not use nested call of 'max'; it's possible to do 'max(3, *[5] + list(range(4)))' instead:INFERENCE
nested-min-max:55:0:55:27::Do not use nested call of 'max'; it's possible to do 'max(3, *list(range(4)))' instead:INFERENCE
6 changes: 6 additions & 0 deletions tests/functional/n/nested_min_max_py39.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
"""Test detection of redundant nested calls to min/max functions"""

# pylint: disable=redefined-builtin,unnecessary-lambda-assignment

max(3, max({1: 2} | {i: i for i in range(4) if i})) # [nested-min-max]
max(3, *{1: 2} | {i: i for i in range(4) if i})
2 changes: 2 additions & 0 deletions tests/functional/n/nested_min_max_py39.rc
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[testoptions]
min_pyver=3.9
1 change: 1 addition & 0 deletions tests/functional/n/nested_min_max_py39.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
nested-min-max:5:0:5:51::"Do not use nested call of 'max'; it's possible to do 'max(3, *{1: 2} | {i: i for i in range(4) if i})' instead":INFERENCE

0 comments on commit a9d9dc3

Please sign in to comment.