Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/whatsnew/fragments/10626.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Fix a crash in :ref:`nested-min-max` when using ``builtins.min`` or ``builtins.max``
instead of ``min`` or ``max`` directly.

Closes #10626
37 changes: 23 additions & 14 deletions pylint/checkers/nested_min_max.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,24 +46,28 @@ class NestedMinMaxChecker(BaseChecker):
}

@classmethod
def is_min_max_call(cls, node: nodes.NodeNG) -> bool:
if not isinstance(node, nodes.Call):
return False

def maybe_get_inferred_min_max_call(
cls, node: nodes.Call
) -> nodes.FunctionDef | None:
inferred = safe_infer(node.func)
return (
if (
isinstance(inferred, nodes.FunctionDef)
and inferred.qname() in cls.FUNC_NAMES
)
):
return inferred
return None

@classmethod
def get_redundant_calls(cls, node: nodes.Call) -> list[nodes.Call]:
def get_redundant_calls(
cls, node: nodes.Call, inferred_call: nodes.FunctionDef
) -> list[nodes.Call]:
return [
arg
for arg in node.args
if (
cls.is_min_max_call(arg)
and arg.func.name == node.func.name
isinstance(arg, nodes.Call)
and (inferred := cls.maybe_get_inferred_min_max_call(arg))
and inferred.qname == inferred_call.qname
# Nesting is useful for finding the maximum in a matrix.
# Allow: max(max([[1, 2, 3], [4, 5, 6]]))
# Meaning, redundant call only if parent max call has more than 1 arg.
Expand All @@ -73,10 +77,11 @@ def get_redundant_calls(cls, node: nodes.Call) -> list[nodes.Call]:

@only_required_for_messages("nested-min-max")
def visit_call(self, node: nodes.Call) -> None:
if not self.is_min_max_call(node):
inferred = self.maybe_get_inferred_min_max_call(node)
if inferred is None:
return

redundant_calls = self.get_redundant_calls(node)
redundant_calls = self.get_redundant_calls(node, inferred)
if not redundant_calls:
return

Expand All @@ -96,7 +101,7 @@ def visit_call(self, node: nodes.Call) -> None:
)
break

redundant_calls = self.get_redundant_calls(fixed_node)
redundant_calls = self.get_redundant_calls(fixed_node, inferred)

for idx, arg in enumerate(fixed_node.args):
if not isinstance(arg, nodes.Const):
Expand All @@ -121,11 +126,15 @@ def visit_call(self, node: nodes.Call) -> None:
splat_node,
*fixed_node.args[idx + 1 : idx],
]

func_name = (
node.func.attrname
if isinstance(node.func, nodes.Attribute)
else node.func.name
)
self.add_message(
"nested-min-max",
node=node,
args=(node.func.name, fixed_node.as_string()),
args=(func_name, fixed_node.as_string()),
confidence=INFERENCE,
)

Expand Down
11 changes: 11 additions & 0 deletions tests/functional/r/regression_02/regression_10626.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
""" Test case for issue #10626: nested builtins.min / builtins.max calls"""

import builtins

builtins.min(1, min(2, 3)) # [nested-min-max]
min(1, builtins.min(2, 3)) # [nested-min-max]
builtins.min(1, builtins.min(2, 3)) # [nested-min-max]

builtins.max(1, max(2, 3)) # [nested-min-max]
max(1, builtins.max(2, 3)) # [nested-min-max]
builtins.max(1, builtins.max(2, 3)) # [nested-min-max]
6 changes: 6 additions & 0 deletions tests/functional/r/regression_02/regression_10626.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
nested-min-max:5:0:5:26::Do not use nested call of 'min'; it's possible to do 'builtins.min(1, 2, 3)' instead:INFERENCE
nested-min-max:6:0:6:26::Do not use nested call of 'min'; it's possible to do 'min(1, 2, 3)' instead:INFERENCE
nested-min-max:7:0:7:35::Do not use nested call of 'min'; it's possible to do 'builtins.min(1, 2, 3)' instead:INFERENCE
nested-min-max:9:0:9:26::Do not use nested call of 'max'; it's possible to do 'builtins.max(1, 2, 3)' instead:INFERENCE
nested-min-max:10:0:10:26::Do not use nested call of 'max'; it's possible to do 'max(1, 2, 3)' instead:INFERENCE
nested-min-max:11:0:11:35::Do not use nested call of 'max'; it's possible to do 'builtins.max(1, 2, 3)' instead:INFERENCE
Loading