From 9e6f7b9dbb046bd4dca3061a6baec9e3fa49ba28 Mon Sep 17 00:00:00 2001 From: Dani Alcala <112832187+clavedeluna@users.noreply.github.com> Date: Sun, 13 Nov 2022 09:39:18 -0300 Subject: [PATCH] false positive `unnecessary-list-index-lookup` for enumerate (#7685) * do not report unnecessary list index lookup if start arg is passed * account for calling start with 0 or negative num Co-authored-by: Pierre Sassoulas --- doc/whatsnew/fragments/7682.false_positive | 3 + .../refactoring/refactoring_checker.py | 60 ++++++++++++++++++- .../unnecessary_list_index_lookup.py | 49 +++++++++++++++ .../unnecessary_list_index_lookup.txt | 4 ++ 4 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 doc/whatsnew/fragments/7682.false_positive diff --git a/doc/whatsnew/fragments/7682.false_positive b/doc/whatsnew/fragments/7682.false_positive new file mode 100644 index 0000000000..3f94f447fd --- /dev/null +++ b/doc/whatsnew/fragments/7682.false_positive @@ -0,0 +1,3 @@ +``unnecessary-list-index-lookup`` will not be wrongly emitted if ``enumerate`` is called with ``start``. + +Closes #7682 diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py index 665a170c58..2d4d632c4d 100644 --- a/pylint/checkers/refactoring/refactoring_checker.py +++ b/pylint/checkers/refactoring/refactoring_checker.py @@ -21,7 +21,7 @@ from pylint import checkers from pylint.checkers import utils from pylint.checkers.utils import node_frame_class -from pylint.interfaces import HIGH, INFERENCE +from pylint.interfaces import HIGH, INFERENCE, Confidence if TYPE_CHECKING: from pylint.lint import PyLinter @@ -2110,6 +2110,12 @@ def _check_unnecessary_list_index_lookup( # destructured, so we can't necessarily use it. return + has_start_arg, confidence = self._enumerate_with_start(node) + if has_start_arg: + # enumerate is being called with start arg/kwarg so resulting index lookup + # is not redundant, hence we should not report an error. + return + iterating_object_name = node.iter.args[0].name value_variable = node.target.elts[1] @@ -2191,7 +2197,7 @@ def _check_unnecessary_list_index_lookup( "unnecessary-list-index-lookup", node=subscript, args=(node.target.elts[1].name,), - confidence=HIGH, + confidence=confidence, ) for subscript in bad_nodes: @@ -2199,5 +2205,53 @@ def _check_unnecessary_list_index_lookup( "unnecessary-list-index-lookup", node=subscript, args=(node.target.elts[1].name,), - confidence=HIGH, + confidence=confidence, ) + + def _enumerate_with_start( + self, node: nodes.For | nodes.Comprehension + ) -> tuple[bool, Confidence]: + """Check presence of `start` kwarg or second argument to enumerate. + + For example: + + `enumerate([1,2,3], start=1)` + `enumerate([1,2,3], 1)` + + If `start` is assigned to `0`, the default value, this is equivalent to + not calling `enumerate` with start. + """ + confidence = HIGH + + if len(node.iter.args) > 1: + # We assume the second argument to `enumerate` is the `start` int arg. + # It's a reasonable assumption for now as it's the only possible argument: + # https://docs.python.org/3/library/functions.html#enumerate + start_arg = node.iter.args[1] + start_val, confidence = self._get_start_value(start_arg) + if start_val is None: + return False, confidence + return not start_val == 0, confidence + + for keyword in node.iter.keywords: + if keyword.arg == "start": + start_val, confidence = self._get_start_value(keyword.value) + if start_val is None: + return False, confidence + return not start_val == 0, confidence + + return False, confidence + + def _get_start_value(self, node: nodes.NodeNG) -> tuple[int | None, Confidence]: + confidence = HIGH + + if isinstance(node, (nodes.Name, nodes.Call)): + inferred = utils.safe_infer(node) + start_val = inferred.value if inferred else None + confidence = INFERENCE + elif isinstance(node, nodes.UnaryOp): + start_val = node.operand.value + else: + start_val = node.value + + return start_val, confidence diff --git a/tests/functional/u/unnecessary/unnecessary_list_index_lookup.py b/tests/functional/u/unnecessary/unnecessary_list_index_lookup.py index 62690ca5a3..e5cb135148 100644 --- a/tests/functional/u/unnecessary/unnecessary_list_index_lookup.py +++ b/tests/functional/u/unnecessary/unnecessary_list_index_lookup.py @@ -81,3 +81,52 @@ def process_list_again(data): if i == 3: # more complex condition actually parts.insert(i, "X") print(part, parts[i]) + +# regression tests for https://github.com/PyCQA/pylint/issues/7682 +series = [1, 2, 3, 4, 5] +output_list = [ + (item, series[index]) + for index, item in enumerate(series, start=1) + if index < len(series) +] + +output_list = [ + (item, series[index]) + for index, item in enumerate(series, 1) + if index < len(series) +] + +for idx, val in enumerate(series, start=2): + print(series[idx]) + +for idx, val in enumerate(series, 2): + print(series[idx]) + +for idx, val in enumerate(series, start=-2): + print(series[idx]) + +for idx, val in enumerate(series, -2): + print(series[idx]) + +for idx, val in enumerate(series, start=0): + print(series[idx]) # [unnecessary-list-index-lookup] + +for idx, val in enumerate(series, 0): + print(series[idx]) # [unnecessary-list-index-lookup] + +START = 0 +for idx, val in enumerate(series, start=START): + print(series[idx]) # [unnecessary-list-index-lookup] + +for idx, val in enumerate(series, START): + print(series[idx]) # [unnecessary-list-index-lookup] + +START = [1, 2, 3] +for i, k in enumerate(series, len(START)): + print(series[idx]) + +def return_start(start): + return start + +for i, k in enumerate(series, return_start(20)): + print(series[idx]) diff --git a/tests/functional/u/unnecessary/unnecessary_list_index_lookup.txt b/tests/functional/u/unnecessary/unnecessary_list_index_lookup.txt index 9a894e139e..a38788cc89 100644 --- a/tests/functional/u/unnecessary/unnecessary_list_index_lookup.txt +++ b/tests/functional/u/unnecessary/unnecessary_list_index_lookup.txt @@ -2,3 +2,7 @@ unnecessary-list-index-lookup:8:10:8:22::Unnecessary list index lookup, use 'val unnecessary-list-index-lookup:43:52:43:64::Unnecessary list index lookup, use 'val' instead:HIGH unnecessary-list-index-lookup:46:10:46:22::Unnecessary list index lookup, use 'val' instead:HIGH unnecessary-list-index-lookup:74:10:74:27::Unnecessary list index lookup, use 'val' instead:HIGH +unnecessary-list-index-lookup:112:10:112:21::Unnecessary list index lookup, use 'val' instead:HIGH +unnecessary-list-index-lookup:115:10:115:21::Unnecessary list index lookup, use 'val' instead:HIGH +unnecessary-list-index-lookup:119:10:119:21::Unnecessary list index lookup, use 'val' instead:INFERENCE +unnecessary-list-index-lookup:122:10:122:21::Unnecessary list index lookup, use 'val' instead:INFERENCE