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.

authortmarek@google.com
changesete0547797db67
branchdefault
phasepublic
hiddenno
parent revision#21a44efb3abf Refactor _check_protected_attribute_access by extracting independent part in utils functions. Improve is_super_call docstring.
child revision#b91be9d00e50 split test to isolate py 2.7 specific syntax and get tests back to green with py 2.5
files modified by this revision
ChangeLog
checkers/utils.py
checkers/variables.py
test/input/func_w0623.py
test/messages/func_w0623.txt
# HG changeset patch
# User tmarek@google.com
# Date 1339073715 -7200
# Thu Jun 07 14:55:15 2012 +0200
# Node ID e0547797db676bebdd1a5850f63da19735152cc3
# Parent 21a44efb3abf371ec2611dac307b1f599b72359e
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.

diff --git a/ChangeLog b/ChangeLog
@@ -1,9 +1,12 @@
1  ChangeLog for PyLint
2  ====================
3 
4  	--
5 +    * #93591: Correctly emit warnings about clobbered variable names when an
6 +      except handler contains a tuple of names instead of a single name.
7 +      (patch by tmarek@google.com)
8 
9      * #7394: W0212 (access to protected member) not emited on assigments
10      (patch by lothiraldan@gmail.com)
11 
12      * #18772; no prototype consistency check for mangled methods (patch by
diff --git a/checkers/utils.py b/checkers/utils.py
@@ -25,13 +25,28 @@
13  from logilab.common.compat import builtins
14  BUILTINS_NAME = builtins.__name__
15 
16  COMP_NODE_TYPES = astng.ListComp, astng.SetComp, astng.DictComp, astng.GenExpr
17 
18 +
19  def is_inside_except(node):
20 -    """Returns true if node is directly inside an exception handler"""
21 -    return isinstance(node.parent, astng.ExceptHandler)
22 +    """Returns true if node is inside the name of an except handler."""
23 +    current = node
24 +    while current and not isinstance(current.parent, astng.ExceptHandler):
25 +        current = current.parent
26 +    
27 +    return current and current is current.parent.name
28 +
29 +
30 +def get_all_elements(node):
31 +    """Recursively returns all atoms in nested lists and tuples."""
32 +    if isinstance(node, (astng.Tuple, astng.List)):
33 +        for child in node.elts:
34 +            for e in get_all_elements(child):
35 +                yield e
36 +    else:
37 +        yield node
38 
39 
40  def clobber_in_except(node):
41      """Checks if an assignment node in an except handler clobbers an existing
42      variable.
@@ -39,11 +54,11 @@
43      Returns (True, args for W0623) if assignment clobbers an existing variable,
44      (False, None) otherwise.
45      """
46      if isinstance(node, astng.AssAttr):
47          return (True, (node.attrname, 'object %r' % (node.expr.name,)))
48 -    elif node is not None:
49 +    elif isinstance(node, astng.AssName):
50          name = node.name
51          if is_builtin(name):
52              return (True, (name, 'builtins'))
53          else:
54              scope, stmts = node.lookup(name)
diff --git a/checkers/variables.py b/checkers/variables.py
@@ -24,11 +24,12 @@
55 
56  from pylint.interfaces import IASTNGChecker
57  from pylint.checkers import BaseChecker
58  from pylint.checkers.utils import (PYMETHODS, is_ancestor_name, is_builtin,
59       is_defined_before, is_error, is_func_default, is_func_decorator,
60 -     assign_parent, check_messages, is_inside_except, clobber_in_except)
61 +     assign_parent, check_messages, is_inside_except, clobber_in_except,
62 +     get_all_elements)
63 
64 
65  def in_for_else_branch(parent, stmt):
66    """Returns True if stmt in inside the else branch for a parent For stmt."""
67    return (isinstance(parent, astng.For) and
@@ -364,18 +365,19 @@
68              if isinstance(ass, (astng.For, astng.Comprehension, astng.GenExpr)) \
69                     and not ass.statement() is node.statement():
70                  self.add_message('W0631', args=name, node=node)
71 
72      def visit_excepthandler(self, node):
73 -        clobbering, args = clobber_in_except(node.name)
74 -        if clobbering:
75 -            self.add_message('W0623', args=args, node=node)
76 +        for name in get_all_elements(node.name):
77 +            clobbering, args = clobber_in_except(name)
78 +            if clobbering:
79 +                self.add_message('W0623', args=args, node=name)
80 
81      def visit_assname(self, node):
82          if isinstance(node.ass_type(), astng.AugAssign):
83              self.visit_name(node)
84 -
85 +          
86      def visit_delname(self, node):
87          self.visit_name(node)
88 
89      def visit_name(self, node):
90          """check that a name is defined if the current scope and doesn't
diff --git a/test/input/func_w0623.py b/test/input/func_w0623.py
@@ -61,5 +61,15 @@
91      print exc4
92  except IOError, exc5: # this is fine
93      print exc5
94  except MyOtherError, exc5: # this is fine
95      print exc5
96 +
97 +def new_style():
98 +    """Some exceptions can be unpacked."""
99 +    try:
100 +        pass
101 +    except IOError as (errno, message): # this is fine
102 +        print errno, message
103 +    except IOError as (new_style, tuple): # W0623 twice
104 +        print new_style, tuple
105 +        
diff --git a/test/messages/func_w0623.txt b/test/messages/func_w0623.txt
@@ -7,5 +7,7 @@
106  W: 22:some_function: Redefining name 'MyError' from outer scope (line 7) in exception handler
107  W: 22:some_function: Unused variable 'MyError'
108  W: 45: Redefining name 'RuntimeError' from object 'exceptions' in exception handler
109  W: 47: Redefining name 'OSError' from builtins in exception handler
110  W: 49: Redefining name 'MyOtherError' from outer scope (line 36) in exception handler
111 +W: 73:new_style: Redefining name 'new_style' from outer scope (line 67) in exception handler
112 +W: 73:new_style: Redefining name 'tuple' from builtins in exception handler