Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

false positive unnecessary-list-index-lookup for enumerate #7685

Merged
merged 7 commits into from Nov 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -2146,6 +2146,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 @@ -2227,13 +2233,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)
Pierre-Sassoulas marked this conversation as resolved.
Show resolved Hide resolved
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