From 29680f1af240c2a135f4cb81f94e2191b3007e37 Mon Sep 17 00:00:00 2001 From: theirix Date: Sun, 8 Oct 2023 18:56:14 +0300 Subject: [PATCH 1/5] Fix suggestion for min-max with expressions --- pylint/checkers/nested_min_max.py | 41 +++++++++++++++++++++++---- tests/functional/n/nested_min_max.py | 15 ++++++++++ tests/functional/n/nested_min_max.txt | 5 ++++ 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/pylint/checkers/nested_min_max.py b/pylint/checkers/nested_min_max.py index 219382ff52..67bd938cb4 100644 --- a/pylint/checkers/nested_min_max.py +++ b/pylint/checkers/nested_min_max.py @@ -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: @@ -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, @@ -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 minmax 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)) diff --git a/tests/functional/n/nested_min_max.py b/tests/functional/n/nested_min_max.py index 151e035dd1..f1fb03fb0a 100644 --- a/tests/functional/n/nested_min_max.py +++ b/tests/functional/n/nested_min_max.py @@ -42,3 +42,18 @@ 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))) + +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}) diff --git a/tests/functional/n/nested_min_max.txt b/tests/functional/n/nested_min_max.txt index c03f1b500c..68ebd310aa 100644 --- a/tests/functional/n/nested_min_max.txt +++ b/tests/functional/n/nested_min_max.txt @@ -12,3 +12,8 @@ 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 +nested-min-max:58:0:58: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 From a1f354dfd12f9bf6c171247e9549b6c0b8e1f464 Mon Sep 17 00:00:00 2001 From: theirix Date: Sun, 8 Oct 2023 23:38:29 +0300 Subject: [PATCH 2/5] Extract py39-specific test case --- tests/functional/n/nested_min_max.py | 3 --- tests/functional/n/nested_min_max.txt | 1 - tests/functional/n/nested_min_max_py39.py | 6 ++++++ tests/functional/n/nested_min_max_py39.txt | 1 + 4 files changed, 7 insertions(+), 4 deletions(-) create mode 100644 tests/functional/n/nested_min_max_py39.py create mode 100644 tests/functional/n/nested_min_max_py39.txt diff --git a/tests/functional/n/nested_min_max.py b/tests/functional/n/nested_min_max.py index f1fb03fb0a..7bb11264e9 100644 --- a/tests/functional/n/nested_min_max.py +++ b/tests/functional/n/nested_min_max.py @@ -54,6 +54,3 @@ max(3, max(list(range(4)))) # [nested-min-max] max(3, *list(range(4))) - -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}) diff --git a/tests/functional/n/nested_min_max.txt b/tests/functional/n/nested_min_max.txt index 68ebd310aa..87b31daf65 100644 --- a/tests/functional/n/nested_min_max.txt +++ b/tests/functional/n/nested_min_max.txt @@ -16,4 +16,3 @@ nested-min-max:46:0:46:25::Do not use nested call of 'max'; it's possible to do 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 -nested-min-max:58:0:58: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 diff --git a/tests/functional/n/nested_min_max_py39.py b/tests/functional/n/nested_min_max_py39.py new file mode 100644 index 0000000000..e60146ca1b --- /dev/null +++ b/tests/functional/n/nested_min_max_py39.py @@ -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}) diff --git a/tests/functional/n/nested_min_max_py39.txt b/tests/functional/n/nested_min_max_py39.txt new file mode 100644 index 0000000000..49541ccc2c --- /dev/null +++ b/tests/functional/n/nested_min_max_py39.txt @@ -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 From 7f9e0494d3de67957dd9d20700561154dc4740ec Mon Sep 17 00:00:00 2001 From: theirix Date: Mon, 9 Oct 2023 00:02:07 +0300 Subject: [PATCH 3/5] Add documentation fragment --- doc/whatsnew/fragments/8524.bugfix | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 doc/whatsnew/fragments/8524.bugfix diff --git a/doc/whatsnew/fragments/8524.bugfix b/doc/whatsnew/fragments/8524.bugfix new file mode 100644 index 0000000000..76bae6e88c --- /dev/null +++ b/doc/whatsnew/fragments/8524.bugfix @@ -0,0 +1,3 @@ +Fixes suggestion for ``nested-min-max`` for expressions with additive operators, list and dict comprehensions. + +Closes #8524 From 1dcd06b78bbe374a3a8e6a7007265a95afa31b06 Mon Sep 17 00:00:00 2001 From: theirix Date: Mon, 9 Oct 2023 00:03:59 +0300 Subject: [PATCH 4/5] Fix comment typo --- pylint/checkers/nested_min_max.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/checkers/nested_min_max.py b/pylint/checkers/nested_min_max.py index 67bd938cb4..a935d62f53 100644 --- a/pylint/checkers/nested_min_max.py +++ b/pylint/checkers/nested_min_max.py @@ -124,7 +124,7 @@ def visit_call(self, node: nodes.Call) -> None: ) def _is_splattable_expression(self, arg: nodes.NodeNG) -> bool: - """Returns true if expression under minmax could be converted to splat + """Returns true if expression under min/max could be converted to splat expression. """ # Support sequence addition (operator __add__) From ee8234ab0e51fa8226a93e48b0ef2a9ab7397ed7 Mon Sep 17 00:00:00 2001 From: theirix Date: Mon, 9 Oct 2023 00:14:47 +0300 Subject: [PATCH 5/5] Require minimal Python version for test case --- tests/functional/n/nested_min_max_py39.rc | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 tests/functional/n/nested_min_max_py39.rc diff --git a/tests/functional/n/nested_min_max_py39.rc b/tests/functional/n/nested_min_max_py39.rc new file mode 100644 index 0000000000..16b75eea75 --- /dev/null +++ b/tests/functional/n/nested_min_max_py39.rc @@ -0,0 +1,2 @@ +[testoptions] +min_pyver=3.9