summaryrefslogtreecommitdiff
path: root/pylint/checkers/refactoring/refactoring_checker.py
diff options
context:
space:
mode:
authorTim Martin <tim@asymptotic.co.uk>2022-06-11 07:47:55 +0100
committerGitHub <noreply@github.com>2022-06-11 08:47:55 +0200
commitdeec94c0e187f010f9d03c9cc8d23b3fd18ba39f (patch)
tree8ce3d5ef5bba1088026fe5c4bdd8b19edba7bb9b /pylint/checkers/refactoring/refactoring_checker.py
parent543ce6de6f022b2f5dcabeef36817610178ffda8 (diff)
downloadpylint-git-deec94c0e187f010f9d03c9cc8d23b3fd18ba39f.tar.gz
Suppress ``unnecessary-list-index-lookup`` for whole loop on write (#6845)
The ``unnecessary-list-index-lookup`` checker was assuming that if the subscript was written to then it would only invalidate access on later lines, but this is not necessarily the case if an inner loop is nested inside the one being checked. Fixes #6818 Co-authored-by: Jacob Walls <jacobtylerwalls@gmail.com>
Diffstat (limited to 'pylint/checkers/refactoring/refactoring_checker.py')
-rw-r--r--pylint/checkers/refactoring/refactoring_checker.py110
1 files changed, 92 insertions, 18 deletions
diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py
index 66773e2b3..265b59d99 100644
--- a/pylint/checkers/refactoring/refactoring_checker.py
+++ b/pylint/checkers/refactoring/refactoring_checker.py
@@ -1921,14 +1921,30 @@ class RefactoringChecker(checkers.BaseTokenChecker):
return
iterating_object_name = node.iter.func.expr.as_string()
+ # Store potential violations. These will only be reported if we don't
+ # discover any writes to the collection during the loop.
+ messages = []
+
# Verify that the body of the for loop uses a subscript
# with the object that was iterated. This uses some heuristics
# in order to make sure that the same object is used in the
# for body.
children = (
- node.body if isinstance(node, nodes.For) else node.parent.get_children()
+ node.body
+ if isinstance(node, nodes.For)
+ else list(node.parent.get_children())
+ )
+
+ # Check if there are any for / while loops within the loop in question;
+ # If so, we will be more conservative about reporting errors as we
+ # can't yet do proper control flow analysis to be sure when
+ # reassignment will affect us
+ nested_loops = itertools.chain.from_iterable(
+ child.nodes_of_class((nodes.For, nodes.While)) for child in children
)
+ has_nested_loops = next(nested_loops, None) is not None
+
for child in children:
for subscript in child.nodes_of_class(nodes.Subscript):
if not isinstance(subscript.value, (nodes.Name, nodes.Attribute)):
@@ -1968,11 +1984,19 @@ class RefactoringChecker(checkers.BaseTokenChecker):
# defined and compare that to the for loop's line number
continue
- self.add_message(
- "unnecessary-dict-index-lookup",
- node=subscript,
- args=(node.target.elts[1].as_string()),
- )
+ if has_nested_loops:
+ messages.append(
+ {
+ "node": subscript,
+ "variable": node.target.elts[1].as_string(),
+ }
+ )
+ else:
+ self.add_message(
+ "unnecessary-dict-index-lookup",
+ node=subscript,
+ args=(node.target.elts[1].as_string(),),
+ )
# Case where .items is assigned to single var (i.e., for item in d.items())
elif isinstance(value, nodes.Subscript):
@@ -1999,11 +2023,31 @@ class RefactoringChecker(checkers.BaseTokenChecker):
inferred = utils.safe_infer(value.slice)
if not isinstance(inferred, nodes.Const) or inferred.value != 0:
continue
- self.add_message(
- "unnecessary-dict-index-lookup",
- node=subscript,
- args=("1".join(value.as_string().rsplit("0", maxsplit=1)),),
- )
+
+ if has_nested_loops:
+ messages.append(
+ {
+ "node": subscript,
+ "variable": "1".join(
+ value.as_string().rsplit("0", maxsplit=1)
+ ),
+ }
+ )
+ else:
+ self.add_message(
+ "unnecessary-dict-index-lookup",
+ node=subscript,
+ args=(
+ "1".join(value.as_string().rsplit("0", maxsplit=1)),
+ ),
+ )
+
+ for message in messages:
+ self.add_message(
+ "unnecessary-dict-index-lookup",
+ node=message["node"],
+ args=(message["variable"],),
+ )
def _check_unnecessary_list_index_lookup(
self, node: nodes.For | nodes.Comprehension
@@ -2029,9 +2073,25 @@ class RefactoringChecker(checkers.BaseTokenChecker):
iterating_object_name = node.iter.args[0].name
value_variable = node.target.elts[1]
+ # Store potential violations. These will only be reported if we don't
+ # discover any writes to the collection during the loop.
+ bad_nodes = []
+
children = (
- node.body if isinstance(node, nodes.For) else node.parent.get_children()
+ node.body
+ if isinstance(node, nodes.For)
+ else list(node.parent.get_children())
+ )
+
+ # Check if there are any for / while loops within the loop in question;
+ # If so, we will be more conservative about reporting errors as we
+ # can't yet do proper control flow analysis to be sure when
+ # reassignment will affect us
+ nested_loops = itertools.chain.from_iterable(
+ child.nodes_of_class((nodes.For, nodes.While)) for child in children
)
+ has_nested_loops = next(nested_loops, None) is not None
+
for child in children:
for subscript in child.nodes_of_class(nodes.Subscript):
if isinstance(node, nodes.For) and _is_part_of_assignment_target(
@@ -2070,9 +2130,23 @@ class RefactoringChecker(checkers.BaseTokenChecker):
# reassigned on a later line, so it can't be used.
continue
- self.add_message(
- "unnecessary-list-index-lookup",
- node=subscript,
- args=(node.target.elts[1].name,),
- confidence=HIGH,
- )
+ if has_nested_loops:
+ # Have found a likely issue, but since there are nested
+ # loops we don't want to report this unless we get to the
+ # end of the loop without updating the collection
+ bad_nodes.append(subscript)
+ else:
+ self.add_message(
+ "unnecessary-list-index-lookup",
+ node=subscript,
+ args=(node.target.elts[1].name,),
+ confidence=HIGH,
+ )
+
+ for subscript in bad_nodes:
+ self.add_message(
+ "unnecessary-list-index-lookup",
+ node=subscript,
+ args=(node.target.elts[1].name,),
+ confidence=HIGH,
+ )