Closes #93591: Correctly emit W0623 on multiple assignment of unpackable exceptions

eg for code like

try:
  ...
except AnyException as (here, there):
  ...

Instead of warning about redefining tuple, recurse into the tuple and check all names.

5 files changed, 40 insertions, 8 deletions

diff --git a/ChangeLog b/ChangeLog
index 0117312fc..a69207ada 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -2,6 +2,9 @@ ChangeLog for PyLint
 ====================
 
 	--
+    * #93591: Correctly emit warnings about clobbered variable names when an
+      except handler contains a tuple of names instead of a single name.
+      (patch by tmarek@google.com)
 
     * #7394: W0212 (access to protected member) not emited on assigments
     (patch by lothiraldan@gmail.com)
diff --git a/checkers/utils.py b/checkers/utils.py
index 2102bbb78..f4fc4a88e 100644
--- a/checkers/utils.py
+++ b/checkers/utils.py
@@ -27,9 +27,24 @@ BUILTINS_NAME = builtins.__name__
 
 COMP_NODE_TYPES = astng.ListComp, astng.SetComp, astng.DictComp, astng.GenExpr
 
+
 def is_inside_except(node):
-    """Returns true if node is directly inside an exception handler"""
-    return isinstance(node.parent, astng.ExceptHandler)
+    """Returns true if node is inside the name of an except handler."""
+    current = node
+    while current and not isinstance(current.parent, astng.ExceptHandler):
+        current = current.parent
+    
+    return current and current is current.parent.name
+
+
+def get_all_elements(node):
+    """Recursively returns all atoms in nested lists and tuples."""
+    if isinstance(node, (astng.Tuple, astng.List)):
+        for child in node.elts:
+            for e in get_all_elements(child):
+                yield e
+    else:
+        yield node
 
 
 def clobber_in_except(node):
@@ -41,7 +56,7 @@ def clobber_in_except(node):
     """
     if isinstance(node, astng.AssAttr):
         return (True, (node.attrname, 'object %r' % (node.expr.name,)))
-    elif node is not None:
+    elif isinstance(node, astng.AssName):
         name = node.name
         if is_builtin(name):
             return (True, (name, 'builtins'))
diff --git a/checkers/variables.py b/checkers/variables.py
index 484a9526f..f0abd1ba6 100644
--- a/checkers/variables.py
+++ b/checkers/variables.py
@@ -26,7 +26,8 @@ from pylint.interfaces import IASTNGChecker
 from pylint.checkers import BaseChecker
 from pylint.checkers.utils import (PYMETHODS, is_ancestor_name, is_builtin,
      is_defined_before, is_error, is_func_default, is_func_decorator,
-     assign_parent, check_messages, is_inside_except, clobber_in_except)
+     assign_parent, check_messages, is_inside_except, clobber_in_except,
+     get_all_elements)
 
 
 def in_for_else_branch(parent, stmt):
@@ -366,14 +367,15 @@ builtins. Remember that you should avoid to define new builtins when possible.'
                 self.add_message('W0631', args=name, node=node)
 
     def visit_excepthandler(self, node):
-        clobbering, args = clobber_in_except(node.name)
-        if clobbering:
-            self.add_message('W0623', args=args, node=node)
+        for name in get_all_elements(node.name):
+            clobbering, args = clobber_in_except(name)
+            if clobbering:
+                self.add_message('W0623', args=args, node=name)
 
     def visit_assname(self, node):
         if isinstance(node.ass_type(), astng.AugAssign):
             self.visit_name(node)
-
+          
     def visit_delname(self, node):
         self.visit_name(node)
 
diff --git a/test/input/func_w0623.py b/test/input/func_w0623.py
index 35a95a3c3..c56da7f38 100644
--- a/test/input/func_w0623.py
+++ b/test/input/func_w0623.py
@@ -63,3 +63,13 @@ except IOError, exc5: # this is fine
     print exc5
 except MyOtherError, exc5: # this is fine
     print exc5
+
+def new_style():
+    """Some exceptions can be unpacked."""
+    try:
+        pass
+    except IOError as (errno, message): # this is fine
+        print errno, message
+    except IOError as (new_style, tuple): # W0623 twice
+        print new_style, tuple
+        
diff --git a/test/messages/func_w0623.txt b/test/messages/func_w0623.txt
index f17fedb4b..5b243dd3e 100644
--- a/test/messages/func_w0623.txt
+++ b/test/messages/func_w0623.txt
@@ -9,3 +9,5 @@ W: 22:some_function: Unused variable 'MyError'
 W: 45: Redefining name 'RuntimeError' from object 'exceptions' in exception handler
 W: 47: Redefining name 'OSError' from builtins in exception handler
 W: 49: Redefining name 'MyOtherError' from outer scope (line 36) in exception handler
+W: 73:new_style: Redefining name 'new_style' from outer scope (line 67) in exception handler
+W: 73:new_style: Redefining name 'tuple' from builtins in exception handler