summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOsher De Paz <odepaz@redhat.com>2022-11-21 10:12:22 +0200
committerGitHub <noreply@github.com>2022-11-21 09:12:22 +0100
commitbb5781cd94eace021edf82954d151810770d241b (patch)
tree78bcbfc8196bbf5f6d58bd5ca9a6c22b89f587d8
parentf2e8ba3690431957c74ccb5e2dd6e1c4512fa0bc (diff)
downloadpylint-git-bb5781cd94eace021edf82954d151810770d241b.tar.gz
Add extension checker for nested min/max (#7550)
This adds a new checker (not active by default) which identifies usages similar to ``min(<arg1>, min(<arg2>, <arg3>))`` and suggests using a simplified form of ``min(<arg1>, <arg2>, <arg3>)``. Same goes for ``max`` usage. The logic is as follows: it detects calls to either ``min`` or ``max`` functions, and whenever one of their arguments is that same function, it emits the message. Co-authored-by: Daniƫl van Noord <13665637+DanielNoord@users.noreply.github.com>
-rw-r--r--doc/data/messages/n/nested-min-max/bad.py1
-rw-r--r--doc/data/messages/n/nested-min-max/good.py1
-rw-r--r--doc/whatsnew/fragments/7546.new_check4
-rw-r--r--pylint/checkers/nested_min_max.py95
-rw-r--r--tests/functional/n/nested_min_max.py21
-rw-r--r--tests/functional/n/nested_min_max.txt8
6 files changed, 130 insertions, 0 deletions
diff --git a/doc/data/messages/n/nested-min-max/bad.py b/doc/data/messages/n/nested-min-max/bad.py
new file mode 100644
index 000000000..b3e13db3a
--- /dev/null
+++ b/doc/data/messages/n/nested-min-max/bad.py
@@ -0,0 +1 @@
+print(min(1, min(2, 3))) # [nested-min-max]
diff --git a/doc/data/messages/n/nested-min-max/good.py b/doc/data/messages/n/nested-min-max/good.py
new file mode 100644
index 000000000..2d348b224
--- /dev/null
+++ b/doc/data/messages/n/nested-min-max/good.py
@@ -0,0 +1 @@
+print(min(1, 2, 3))
diff --git a/doc/whatsnew/fragments/7546.new_check b/doc/whatsnew/fragments/7546.new_check
new file mode 100644
index 000000000..5b1f5a327
--- /dev/null
+++ b/doc/whatsnew/fragments/7546.new_check
@@ -0,0 +1,4 @@
+Added ``nested-min-max`` which flags ``min(1, min(2, 3))`` to simplify to
+``min(1, 2, 3)``.
+
+Closes #7546
diff --git a/pylint/checkers/nested_min_max.py b/pylint/checkers/nested_min_max.py
new file mode 100644
index 000000000..39feb5f42
--- /dev/null
+++ b/pylint/checkers/nested_min_max.py
@@ -0,0 +1,95 @@
+# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+# For details: https://github.com/PyCQA/pylint/blob/main/LICENSE
+# Copyright (c) https://github.com/PyCQA/pylint/blob/main/CONTRIBUTORS.txt
+
+"""Check for use of nested min/max functions."""
+
+from __future__ import annotations
+
+import copy
+from typing import TYPE_CHECKING
+
+from astroid import nodes
+
+from pylint.checkers import BaseChecker
+from pylint.checkers.utils import only_required_for_messages, safe_infer
+from pylint.interfaces import INFERENCE
+
+if TYPE_CHECKING:
+ from pylint.lint import PyLinter
+
+
+class NestedMinMaxChecker(BaseChecker):
+ """Multiple nested min/max calls on the same line will raise multiple messages.
+
+ This behaviour is intended as it would slow down the checker to check
+ for nested call with minimal benefits.
+ """
+
+ FUNC_NAMES = ("builtins.min", "builtins.max")
+
+ name = "nested_min_max"
+ msgs = {
+ "W3301": (
+ "Do not use nested call of '%s'; it's possible to do '%s' instead",
+ "nested-min-max",
+ "Nested calls ``min(1, min(2, 3))`` can be rewritten as ``min(1, 2, 3)``.",
+ )
+ }
+
+ @classmethod
+ def is_min_max_call(cls, node: nodes.NodeNG) -> bool:
+ if not isinstance(node, nodes.Call):
+ return False
+
+ inferred = safe_infer(node.func)
+ return (
+ isinstance(inferred, nodes.FunctionDef)
+ and inferred.qname() in cls.FUNC_NAMES
+ )
+
+ @classmethod
+ def get_redundant_calls(cls, node: nodes.Call) -> list[nodes.Call]:
+ return [
+ arg
+ for arg in node.args
+ if cls.is_min_max_call(arg) and arg.func.name == node.func.name
+ ]
+
+ @only_required_for_messages("nested-min-max")
+ def visit_call(self, node: nodes.Call) -> None:
+ if not self.is_min_max_call(node):
+ return
+
+ redundant_calls = self.get_redundant_calls(node)
+ if not redundant_calls:
+ return
+
+ fixed_node = copy.copy(node)
+ while len(redundant_calls) > 0:
+ for i, arg in enumerate(fixed_node.args):
+ # Exclude any calls with generator expressions as there is no
+ # clear better suggestion for them.
+ if isinstance(arg, nodes.Call) and any(
+ isinstance(a, nodes.GeneratorExp) for a in arg.args
+ ):
+ return
+
+ if arg in redundant_calls:
+ fixed_node.args = (
+ fixed_node.args[:i] + arg.args + fixed_node.args[i + 1 :]
+ )
+ break
+
+ redundant_calls = self.get_redundant_calls(fixed_node)
+
+ self.add_message(
+ "nested-min-max",
+ node=node,
+ args=(node.func.name, fixed_node.as_string()),
+ confidence=INFERENCE,
+ )
+
+
+def register(linter: PyLinter) -> None:
+ linter.register_checker(NestedMinMaxChecker(linter))
diff --git a/tests/functional/n/nested_min_max.py b/tests/functional/n/nested_min_max.py
new file mode 100644
index 000000000..cef63dc2b
--- /dev/null
+++ b/tests/functional/n/nested_min_max.py
@@ -0,0 +1,21 @@
+"""Test detection of redundant nested calls to min/max functions"""
+
+# pylint: disable=redefined-builtin,unnecessary-lambda-assignment
+
+min(1, min(2, 3)) # [nested-min-max]
+max(1, max(2, 3)) # [nested-min-max]
+min(min(1, 2), 3) # [nested-min-max]
+min(min(min(1, 2), 3), 4) # [nested-min-max, nested-min-max]
+min(1, max(2, 3))
+min(1, 2, 3)
+min(min(1, 2), min(3, 4)) # [nested-min-max]
+min(len([]), min(len([1]), len([1, 2]))) # [nested-min-max]
+
+orig_min = min
+min = lambda *args: args[0]
+min(1, min(2, 3))
+orig_min(1, orig_min(2, 3)) # [nested-min-max]
+
+# This is too complicated (for now) as there is no clear better way to write it
+max(max(i for i in range(10)), 0)
+max(max(max(i for i in range(10)), 0), 1)
diff --git a/tests/functional/n/nested_min_max.txt b/tests/functional/n/nested_min_max.txt
new file mode 100644
index 000000000..9bcb663e3
--- /dev/null
+++ b/tests/functional/n/nested_min_max.txt
@@ -0,0 +1,8 @@
+nested-min-max:5:0:5:17::Do not use nested call of 'min'; it's possible to do 'min(1, 2, 3)' instead:INFERENCE
+nested-min-max:6:0:6:17::Do not use nested call of 'max'; it's possible to do 'max(1, 2, 3)' instead:INFERENCE
+nested-min-max:7:0:7:17::Do not use nested call of 'min'; it's possible to do 'min(1, 2, 3)' instead:INFERENCE
+nested-min-max:8:4:8:21::Do not use nested call of 'min'; it's possible to do 'min(1, 2, 3)' instead:INFERENCE
+nested-min-max:8:0:8:25::Do not use nested call of 'min'; it's possible to do 'min(1, 2, 3, 4)' instead:INFERENCE
+nested-min-max:11:0:11:25::Do not use nested call of 'min'; it's possible to do 'min(1, 2, 3, 4)' instead:INFERENCE
+nested-min-max:12:0:12:40::Do not use nested call of 'min'; it's possible to do 'min(len([]), len([1]), len([1, 2]))' instead:INFERENCE
+nested-min-max:17:0:17:27::Do not use nested call of 'orig_min'; it's possible to do 'orig_min(1, 2, 3)' instead:INFERENCE