Skip to content

Commit

Permalink
false positive unnecessary-list-index-lookup for enumerate (#7685)
Browse files Browse the repository at this point in the history
* 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 <pierre.sassoulas@gmail.com>
  • Loading branch information
clavedeluna and Pierre-Sassoulas committed Nov 17, 2022
1 parent 7b0bc19 commit 19c205d
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 3 deletions.
3 changes: 3 additions & 0 deletions 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
60 changes: 57 additions & 3 deletions pylint/checkers/refactoring/refactoring_checker.py
Expand Up @@ -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
Expand Down Expand Up @@ -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]

Expand Down Expand Up @@ -2191,13 +2197,61 @@ 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:
self.add_message(
"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
49 changes: 49 additions & 0 deletions tests/functional/u/unnecessary/unnecessary_list_index_lookup.py
Expand Up @@ -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])
Expand Up @@ -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

0 comments on commit 19c205d

Please sign in to comment.