Fix pylint-dev#5608: Emit redefined-outer-name for redefinitions of…

… loop variables in body

ChangeLog

@@ -16,6 +16,10 @@ Release date: TBA
 
   Closes #578
 
+* Emit ``redefined-outer-name`` when a loop variable is redefined in the loop
+  body. Previously, only redefinitions taking place in nested loops were flagged.
+
+  Closes #5608
 
 * Fix false negative for ``no-member`` when attempting to assign an instance
   attribute to itself without any prior assignment.

doc/whatsnew/2.14.rst

@@ -33,3 +33,8 @@ Other Changes
   attribute to itself without any prior assignment.
 
   Closes #1555
+
+* Emit ``redefined-outer-name`` when a loop variable is redefined in the loop
+  body. Previously, only redefinitions taking place in nested loops were flagged.
+
+  Closes #5608

pylint/checkers/variables.py

@@ -467,9 +467,10 @@ def _has_locals_call_after_node(stmt, scope):
         "`'from X import *'` style import.",
     ),
     "W0621": (
-        "Redefining name %r from outer scope (line %s)",
+        "Redefining name %r from outer scope or loop (line %s)",
         "redefined-outer-name",
-        "Used when a variable's name hides a name defined in the outer scope.",
+        "Used when a variable's name hides a name defined in an outer scope, "
+        "for loop, or except handler.",
     ),
     "W0622": (
         "Redefining built-in %r",
@@ -982,7 +983,7 @@ class VariablesChecker(BaseChecker):
     Checks for
     * unused variables / imports
     * undefined variables
-    * redefinition of variable from builtins or from an outer scope
+    * redefinition of variable from builtins or from an outer scope, for loop, or except handler
     * use of variable before assignment
     * __all__ consistency
     * self/cls assignment
@@ -1358,6 +1359,19 @@ def visit_global(self, node: nodes.Global) -> None:
             self.add_message("global-statement", node=node)
 
     def visit_assignname(self, node: nodes.AssignName) -> None:
+        if self.linter.is_message_enabled("redefined-outer-name") and isinstance(
+            node.parent, nodes.Assign
+        ):
+            for outer_for, outer_variables in self._loop_variables:
+                if node.name in outer_variables and not in_for_else_branch(
+                    outer_for, node
+                ):
+                    self.add_message(
+                        "redefined-outer-name",
+                        args=(node.name, outer_for.fromlineno),
+                        node=node,
+                    )
+                    break
         if isinstance(node.assign_type(), nodes.AugAssign):
             self.visit_name(node)
 

tests/functional/b/bad_indentation.py

@@ -13,6 +13,7 @@ def titii():
 
 def tataa(kdict):
     for key in ['1', '2', '3']:
+        # pylint: disable=redefined-outer-name
         key = key.lower()
 
         if key in kdict:

tests/functional/c/cellvar_escaping_loop.py

@@ -76,7 +76,7 @@ def good_case10():
     lst = []
     for i in range(10):  # pylint: disable=unused-variable
         def func():
-            i = 100
+            i = 100  # pylint: disable=redefined-outer-name
             def func2(arg=i):
                 return arg
 

tests/functional/f/function_redefined.txt

@@ -1,7 +1,7 @@
 function-redefined:18:4:18:15:AAAA.method2:method already defined line 15:UNDEFINED
 function-redefined:21:0:21:10:AAAA:class already defined line 8:UNDEFINED
 function-redefined:35:0:35:9:func2:function already defined line 32:UNDEFINED
-redefined-outer-name:37:4:37:16:func2:Redefining name '__revision__' from outer scope (line 7):UNDEFINED
+redefined-outer-name:37:4:37:16:func2:Redefining name '__revision__' from outer scope or loop (line 7):UNDEFINED
 function-redefined:54:4:54:23:exclusive_func2:function already defined line 48:UNDEFINED
 function-redefined:89:0:89:8:ceil:function already defined line 88:UNDEFINED
 function-redefined:93:0:93:8:math:function already defined line 92:UNDEFINED

tests/functional/n/no/no_dummy_redefined.txt

@@ -1,2 +1,2 @@
 invalid-name:7:0:7:5::"Constant name ""value"" doesn't conform to UPPER_CASE naming style":HIGH
-redefined-outer-name:12:4:12:9:clobbering:Redefining name 'value' from outer scope (line 7):UNDEFINED
+redefined-outer-name:12:4:12:9:clobbering:Redefining name 'value' from outer scope or loop (line 7):UNDEFINED

tests/functional/r/redefine_loop.txt

@@ -1 +1 @@
-redefined-outer-name:4:4:6:20::Redefining name 'item' from outer scope (line 2):UNDEFINED
+redefined-outer-name:4:4:6:20::Redefining name 'item' from outer scope or loop (line 2):UNDEFINED

tests/functional/r/redefined/redefined_except_handler.txt

@@ -1,4 +1,4 @@
-redefined-outer-name:11:4:12:12::Redefining name 'err' from outer scope (line 8):UNDEFINED
-redefined-outer-name:57:8:58:16::Redefining name 'err' from outer scope (line 51):UNDEFINED
+redefined-outer-name:11:4:12:12::Redefining name 'err' from outer scope or loop (line 8):UNDEFINED
+redefined-outer-name:57:8:58:16::Redefining name 'err' from outer scope or loop (line 51):UNDEFINED
 used-before-assignment:69:14:69:29:func:Using variable 'CustomException' before assignment:HIGH
-redefined-outer-name:71:4:72:12:func:Redefining name 'CustomException' from outer scope (line 62):UNDEFINED
+redefined-outer-name:71:4:72:12:func:Redefining name 'CustomException' from outer scope or loop (line 62):UNDEFINED

tests/functional/r/redefined/redefined_loop_name.py

@@ -0,0 +1,15 @@
+"""Tests for redefinitions of loop variables inside the loop body.
+
+See: https://github.com/PyCQA/pylint/issues/5608
+"""
+# pylint: disable=invalid-name
+
+lines = ["1\t", "2\t"]
+for line in lines:
+    line = line.strip()  # [redefined-outer-name]
+
+lines = [(1, "1\t"), (2, "2\t")]
+for i, line in lines:
+    line = line.strip()  # [redefined-outer-name]
+
+line = "no warning"

tests/functional/r/redefined/redefined_loop_name.txt

@@ -0,0 +1,2 @@
+redefined-outer-name:9:4:9:8::Redefining name 'line' from outer scope or loop (line 8):UNDEFINED
+redefined-outer-name:13:4:13:8::Redefining name 'line' from outer scope or loop (line 12):UNDEFINED

tests/functional/r/reimported.txt

@@ -5,5 +5,5 @@ reimported:10:0:10:27::Reimport 'deque' (imported line 9):UNDEFINED
 reimported:24:0:24:33::Reimport 'ElementTree' (imported line 23):UNDEFINED
 reimported:27:0:27:21::Reimport 'email.encoders' (imported line 26):UNDEFINED
 reimported:29:0:29:10::Reimport 'sys' (imported line 21):UNDEFINED
-redefined-outer-name:40:4:40:14:reimport:Redefining name 'sys' from outer scope (line 16):UNDEFINED
+redefined-outer-name:40:4:40:14:reimport:Redefining name 'sys' from outer scope or loop (line 16):UNDEFINED
 reimported:40:4:40:14:reimport:Reimport 'sys' (imported line 21):UNDEFINED

tests/functional/r/reused_outer_loop_variable.txt

@@ -1,4 +1,4 @@
-redefined-outer-name:7:4:8:16::Redefining name 'i' from outer scope (line 6):UNDEFINED
-redefined-outer-name:12:4:13:25::Redefining name 'i' from outer scope (line 11):UNDEFINED
-redefined-outer-name:17:4:18:25::Redefining name 'i' from outer scope (line 16):UNDEFINED
-redefined-outer-name:22:4:23:25::Redefining name 'a' from outer scope (line 21):UNDEFINED
+redefined-outer-name:7:4:8:16::Redefining name 'i' from outer scope or loop (line 6):UNDEFINED
+redefined-outer-name:12:4:13:25::Redefining name 'i' from outer scope or loop (line 11):UNDEFINED
+redefined-outer-name:17:4:18:25::Redefining name 'i' from outer scope or loop (line 16):UNDEFINED
+redefined-outer-name:22:4:23:25::Redefining name 'a' from outer scope or loop (line 21):UNDEFINED

tests/functional/r/reused_outer_loop_variable_py3.txt

@@ -1 +1 @@
-redefined-outer-name:4:4:5:16::Redefining name 'j' from outer scope (line 3):UNDEFINED
+redefined-outer-name:4:4:5:16::Redefining name 'j' from outer scope or loop (line 3):UNDEFINED

tests/functional/u/undefined/undefined_loop_variable.py

@@ -21,7 +21,7 @@ def do_else(some_random_list):
 VAR2 = B # nor this one
 
 for var1, var2 in TEST_LC:
-    var1 = var2 + 4
+    var1 = var2 + 4  # pylint: disable=redefined-outer-name
 VAR3 = var1 # [undefined-loop-variable]
 
 for note in __revision__:

tests/functional/u/unnecessary/unnecessary_dict_index_lookup.py

@@ -12,7 +12,7 @@
 
 for k, v in b_dict.items():
     print(k)
-    k = "another key"
+    k = "another key"  # pylint: disable=redefined-outer-name
     print(b_dict[k])  # This is fine, key reassigned
 
 
@@ -68,7 +68,7 @@ class Foo:
 for item in d.items():
     print(item[0])
     print(d[item[0]])  # [unnecessary-dict-index-lookup]
-    item = (2, "b")
+    item = (2, "b")  # pylint: disable=redefined-outer-name
     print(d[item[0]])  # This is fine, no warning thrown as key has been reassigned
 
 

tests/functional/u/used/used_before_assignment_issue1081.txt

@@ -1,7 +1,7 @@
 used-before-assignment:7:7:7:8:used_before_assignment_1:Using variable 'x' before assignment:HIGH
-redefined-outer-name:8:12:8:13:used_before_assignment_1:Redefining name 'x' from outer scope (line 3):UNDEFINED
+redefined-outer-name:8:12:8:13:used_before_assignment_1:Redefining name 'x' from outer scope or loop (line 3):UNDEFINED
 used-before-assignment:13:7:13:8:used_before_assignment_2:Using variable 'x' before assignment:HIGH
-redefined-outer-name:15:4:15:5:used_before_assignment_2:Redefining name 'x' from outer scope (line 3):UNDEFINED
+redefined-outer-name:15:4:15:5:used_before_assignment_2:Redefining name 'x' from outer scope or loop (line 3):UNDEFINED
 used-before-assignment:19:7:19:8:used_before_assignment_3:Using variable 'x' before assignment:HIGH
-redefined-outer-name:21:12:21:13:used_before_assignment_3:Redefining name 'x' from outer scope (line 3):UNDEFINED
-redefined-outer-name:30:4:30:5:not_used_before_assignment_2:Redefining name 'x' from outer scope (line 3):UNDEFINED
+redefined-outer-name:21:12:21:13:used_before_assignment_3:Redefining name 'x' from outer scope or loop (line 3):UNDEFINED
+redefined-outer-name:30:4:30:5:not_used_before_assignment_2:Redefining name 'x' from outer scope or loop (line 3):UNDEFINED