Skip to content

Commit 8cf4887

Browse files
[fix] Crash in 'nested-min-max' when using builtins.min instead of min directly. (#10629)
Closes #10626 Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>
1 parent 9d4f550 commit 8cf4887

File tree

4 files changed

+44
-14
lines changed

4 files changed

+44
-14
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix a crash in :ref:`nested-min-max` when using ``builtins.min`` or ``builtins.max``
2+
instead of ``min`` or ``max`` directly.
3+
4+
Closes #10626

pylint/checkers/nested_min_max.py

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -46,24 +46,28 @@ class NestedMinMaxChecker(BaseChecker):
4646
}
4747

4848
@classmethod
49-
def is_min_max_call(cls, node: nodes.NodeNG) -> bool:
50-
if not isinstance(node, nodes.Call):
51-
return False
52-
49+
def maybe_get_inferred_min_max_call(
50+
cls, node: nodes.Call
51+
) -> nodes.FunctionDef | None:
5352
inferred = safe_infer(node.func)
54-
return (
53+
if (
5554
isinstance(inferred, nodes.FunctionDef)
5655
and inferred.qname() in cls.FUNC_NAMES
57-
)
56+
):
57+
return inferred
58+
return None
5859

5960
@classmethod
60-
def get_redundant_calls(cls, node: nodes.Call) -> list[nodes.Call]:
61+
def get_redundant_calls(
62+
cls, node: nodes.Call, inferred_call: nodes.FunctionDef
63+
) -> list[nodes.Call]:
6164
return [
6265
arg
6366
for arg in node.args
6467
if (
65-
cls.is_min_max_call(arg)
66-
and arg.func.name == node.func.name
68+
isinstance(arg, nodes.Call)
69+
and (inferred := cls.maybe_get_inferred_min_max_call(arg))
70+
and inferred.qname == inferred_call.qname
6771
# Nesting is useful for finding the maximum in a matrix.
6872
# Allow: max(max([[1, 2, 3], [4, 5, 6]]))
6973
# Meaning, redundant call only if parent max call has more than 1 arg.
@@ -73,10 +77,11 @@ def get_redundant_calls(cls, node: nodes.Call) -> list[nodes.Call]:
7377

7478
@only_required_for_messages("nested-min-max")
7579
def visit_call(self, node: nodes.Call) -> None:
76-
if not self.is_min_max_call(node):
80+
inferred = self.maybe_get_inferred_min_max_call(node)
81+
if inferred is None:
7782
return
7883

79-
redundant_calls = self.get_redundant_calls(node)
84+
redundant_calls = self.get_redundant_calls(node, inferred)
8085
if not redundant_calls:
8186
return
8287

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

99-
redundant_calls = self.get_redundant_calls(fixed_node)
104+
redundant_calls = self.get_redundant_calls(fixed_node, inferred)
100105

101106
for idx, arg in enumerate(fixed_node.args):
102107
if not isinstance(arg, nodes.Const):
@@ -121,11 +126,15 @@ def visit_call(self, node: nodes.Call) -> None:
121126
splat_node,
122127
*fixed_node.args[idx + 1 : idx],
123128
]
124-
129+
func_name = (
130+
node.func.attrname
131+
if isinstance(node.func, nodes.Attribute)
132+
else node.func.name
133+
)
125134
self.add_message(
126135
"nested-min-max",
127136
node=node,
128-
args=(node.func.name, fixed_node.as_string()),
137+
args=(func_name, fixed_node.as_string()),
129138
confidence=INFERENCE,
130139
)
131140

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
""" Test case for issue #10626: nested builtins.min / builtins.max calls"""
2+
3+
import builtins
4+
5+
builtins.min(1, min(2, 3)) # [nested-min-max]
6+
min(1, builtins.min(2, 3)) # [nested-min-max]
7+
builtins.min(1, builtins.min(2, 3)) # [nested-min-max]
8+
9+
builtins.max(1, max(2, 3)) # [nested-min-max]
10+
max(1, builtins.max(2, 3)) # [nested-min-max]
11+
builtins.max(1, builtins.max(2, 3)) # [nested-min-max]
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
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
2+
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
3+
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
4+
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
5+
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
6+
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

0 commit comments

Comments
 (0)