diff options
author | Osher De Paz <odepaz@redhat.com> | 2022-11-21 10:12:22 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-11-21 09:12:22 +0100 |
commit | bb5781cd94eace021edf82954d151810770d241b (patch) | |
tree | 78bcbfc8196bbf5f6d58bd5ca9a6c22b89f587d8 | |
parent | f2e8ba3690431957c74ccb5e2dd6e1c4512fa0bc (diff) | |
download | pylint-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.py | 1 | ||||
-rw-r--r-- | doc/data/messages/n/nested-min-max/good.py | 1 | ||||
-rw-r--r-- | doc/whatsnew/fragments/7546.new_check | 4 | ||||
-rw-r--r-- | pylint/checkers/nested_min_max.py | 95 | ||||
-rw-r--r-- | tests/functional/n/nested_min_max.py | 21 | ||||
-rw-r--r-- | tests/functional/n/nested_min_max.txt | 8 |
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 |