Fix a false positive with catching-non-exception and tuples of

 exceptions.

4 files changed, 69 insertions, 27 deletions

diff --git a/ChangeLog b/ChangeLog
index cc0ac42..1e0c489 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -16,6 +16,9 @@ ChangeLog for Pylint
 
     * Use a mro traversal for finding abstract methods. Closes issue #415.
 
+    * Fix a false positive with catching-non-exception and tuples of
+      exceptions.
+
 
 2014-11-23 -- 1.4.0
 
diff --git a/checkers/exceptions.py b/checkers/exceptions.py
index 21c863d..c9b6eb1 100644
--- a/checkers/exceptions.py
+++ b/checkers/exceptions.py
@@ -199,6 +199,46 @@ class ExceptionsChecker(BaseChecker):
             value_found = False
         return value_found
 
+    def _check_catching_non_exception(self, handler, exc, part):
+        if isinstance(exc, astroid.Tuple):
+            # Check if it is a tuple of exceptions.
+            inferred = [safe_infer(elt) for elt in exc.elts]
+            if any(node is astroid.YES for node in inferred):
+                # Don't emit if we don't know every component.
+                return
+            if all(node and inherit_from_std_ex(node)
+                   for node in inferred):
+                return
+
+        if not isinstance(exc, astroid.Class):
+            # Don't emit the warning if the infered stmt
+            # is None, but the exception handler is something else,
+            # maybe it was redefined.
+            if (isinstance(exc, astroid.Const) and
+                    exc.value is None):
+                if ((isinstance(handler.type, astroid.Const) and
+                     handler.type.value is None) or
+                        handler.type.parent_of(exc)):
+                    # If the exception handler catches None or
+                    # the exception component, which is None, is
+                    # defined by the entire exception handler, then
+                    # emit a warning.
+                    self.add_message('catching-non-exception',
+                                     node=handler.type,
+                                     args=(part.as_string(), ))
+            else:
+                self.add_message('catching-non-exception',
+                                 node=handler.type,
+                                 args=(part.as_string(), ))
+            return
+        if (not inherit_from_std_ex(exc) and
+                exc.root().name != BUILTINS_NAME):
+            if has_known_bases(exc):
+                self.add_message('catching-non-exception',
+                                 node=handler.type,
+                                 args=(exc.name, ))
+
+
     @check_messages('bare-except', 'broad-except', 'pointless-except',
                     'binary-op-exception', 'bad-except-order',
                     'catching-non-exception')
@@ -233,26 +273,10 @@ class ExceptionsChecker(BaseChecker):
                         continue
                     if isinstance(exc, astroid.Instance) and inherit_from_std_ex(exc):
                         exc = exc._proxied
+
+                    self._check_catching_non_exception(handler, exc, part)
+
                     if not isinstance(exc, astroid.Class):
-                        # Don't emit the warning if the infered stmt
-                        # is None, but the exception handler is something else,
-                        # maybe it was redefined.
-                        if (isinstance(exc, astroid.Const) and
-                                exc.value is None):
-                            if ((isinstance(handler.type, astroid.Const) and
-                                 handler.type.value is None) or
-                                    handler.type.parent_of(exc)):
-                                # If the exception handler catches None or
-                                # the exception component, which is None, is
-                                # defined by the entire exception handler, then
-                                # emit a warning.
-                                self.add_message('catching-non-exception',
-                                                 node=handler.type,
-                                                 args=(part.as_string(), ))
-                        else:
-                            self.add_message('catching-non-exception',
-                                             node=handler.type,
-                                             args=(part.as_string(), ))
                         continue
 
                     exc_ancestors = [anc for anc in exc.ancestors()
@@ -269,13 +293,6 @@ class ExceptionsChecker(BaseChecker):
                         self.add_message('broad-except',
                                          args=exc.name, node=handler.type)
 
-                    if (not inherit_from_std_ex(exc) and
-                            exc.root().name != BUILTINS_NAME):
-                        if has_known_bases(exc):
-                            self.add_message('catching-non-exception',
-                                             node=handler.type,
-                                             args=(exc.name, ))
-
                 exceptions_classes += [exc for _, exc in excs]
 
 
diff --git a/test/functional/invalid_exceptions_caught.py b/test/functional/invalid_exceptions_caught.py
index 0b54ed7..c58d2e9 100644
--- a/test/functional/invalid_exceptions_caught.py
+++ b/test/functional/invalid_exceptions_caught.py
@@ -1,5 +1,5 @@
 """Test for catching non-exceptions."""
-# pylint: disable=too-many-ancestors, print-statement, no-absolute-import
+# pylint: disable=too-many-ancestors, print-statement, no-absolute-import, import-error
 import socket
 
 class MyException(object):
@@ -69,3 +69,24 @@ try:
 # +1:[catching-non-exception,catching-non-exception,catching-non-exception]
 except (list([4, 5, 6]), None, ZeroDivisionError, 4):
     print "caught"
+
+EXCEPTION_TUPLE = (ZeroDivisionError, OSError)
+NON_EXCEPTION_TUPLE = (ZeroDivisionError, OSError, 4)
+
+try:
+    1 + 42
+except EXCEPTION_TUPLE:
+    print "caught"
+
+try:
+    1 + 42
+except NON_EXCEPTION_TUPLE: # [catching-non-exception]
+    print "caught"
+
+from missing_import import UnknownError
+UNKNOWN_COMPONENTS = (ZeroDivisionError, UnknownError)
+
+try:
+    1 + 42
+except UNKNOWN_COMPONENTS:
+    print "caught"
diff --git a/test/functional/invalid_exceptions_caught.txt b/test/functional/invalid_exceptions_caught.txt
index 85c721d..d2e6f3c 100644
--- a/test/functional/invalid_exceptions_caught.txt
+++ b/test/functional/invalid_exceptions_caught.txt
@@ -7,3 +7,4 @@ catching-non-exception:57::"Catching an exception which doesn't inherit from Bas
 catching-non-exception:70::"Catching an exception which doesn't inherit from BaseException: 4"
 catching-non-exception:70::"Catching an exception which doesn't inherit from BaseException: None"
 catching-non-exception:70::"Catching an exception which doesn't inherit from BaseException: list([4, 5, 6])"
+catching-non-exception:83::"Catching an exception which doesn't inherit from BaseException: NON_EXCEPTION_TUPLE"