Don't emit 'invalid-name' when assigning a name in an ImportError handler. Closes issue #302.

6 files changed, 89 insertions, 23 deletions

diff --git a/ChangeLog b/ChangeLog
index b39c9d6..e92d198 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -45,6 +45,9 @@ ChangeLog for Pylint
       for all the members imported with 'from import' form.
       Closes issue #304.
 
+    * Don't emit 'invalid-name' when assigning a name in an
+      ImportError handler. Closes issue #302.
+
 2014-07-26  --  1.3.0
 
     * Allow hanging continued indentation for implicitly concatenated
diff --git a/checkers/base.py b/checkers/base.py
index c53d3a1..6e5804f 100644
--- a/checkers/base.py
+++ b/checkers/base.py
@@ -35,6 +35,7 @@ from pylint.checkers.utils import (
     safe_infer,
     get_argument_from_call,
     NoSuchArgumentError,
+    is_import_error,
     )
 
 
@@ -71,6 +72,26 @@ DEFAULT_ARGUMENT_SYMBOLS = dict(
 
 del re
 
+def _redefines_import(node):
+    """ Detect that the given node (AssName) is inside an
+    exception handler and redefines an import from the tryexcept body.
+    Returns True if the node redefines an import, False otherwise.
+    """
+    current = node
+    while current and not isinstance(current.parent, astroid.ExceptHandler):
+        current = current.parent
+    if not current or not is_import_error(current.parent):
+        return False
+    try_block = current.parent.parent
+    for import_node in try_block.nodes_of_class((astroid.From, astroid.Import)):
+        for name, alias in import_node.names:
+            if alias:
+                if alias == node.name:
+                    return True
+            elif name == node.name:
+                return True
+    return False
+
 def in_loop(node):
     """return True if the node is inside a kind of for loop"""
     parent = node.parent
@@ -978,13 +999,17 @@ class NameChecker(_BasicChecker):
                 if isinstance(safe_infer(ass_type.value), astroid.Class):
                     self._check_name('class', node.name, node)
                 else:
-                    self._check_name('const', node.name, node)
+                    if not _redefines_import(node):
+                        # Don't emit if the name redefines an import
+                        # in an ImportError except handler.
+                        self._check_name('const', node.name, node)
             elif isinstance(ass_type, astroid.ExceptHandler):
                 self._check_name('variable', node.name, node)
         elif isinstance(frame, astroid.Function):
             # global introduced variable aren't in the function locals
             if node.name in frame and node.name not in frame.argnames():
-                self._check_name('variable', node.name, node)
+                if not _redefines_import(node):
+                    self._check_name('variable', node.name, node)
         elif isinstance(frame, astroid.Class):
             if not list(frame.local_attr_ancestors(node.name)):
                 self._check_name('class_attribute', node.name, node)
diff --git a/checkers/imports.py b/checkers/imports.py
index d9bdfa6..a861ce6 100644
--- a/checkers/imports.py
+++ b/checkers/imports.py
@@ -27,7 +27,7 @@ from astroid.modutils import get_module_part, is_standard_module
 from pylint.interfaces import IAstroidChecker
 from pylint.utils import EmptyReport
 from pylint.checkers import BaseChecker
-from pylint.checkers.utils import check_messages, inherit_from_std_ex
+from pylint.checkers.utils import check_messages, is_import_error
 
 def _except_import_error(node):
     """
@@ -36,26 +36,7 @@ def _except_import_error(node):
     """
     if not isinstance(node, astroid.TryExcept):
         return
-    for handler in node.handlers:
-        names = None
-        if isinstance(handler.type, astroid.Tuple):
-            names = [name for name in handler.type.elts
-                     if isinstance(name, astroid.Name)]
-        elif isinstance(handler.type, astroid.Name):
-            names = [handler.type]
-        else:
-            # Don't try to infer that.
-            return
-        for name in names:
-            try:
-                for infered in name.infer():
-                    if (isinstance(infered, astroid.Class) and
-                            inherit_from_std_ex(infered) and
-                            infered.name == 'ImportError'):
-                        return True
-            except astroid.InferenceError:
-                continue
-    return False
+    return any(map(is_import_error, node.handlers))
 
 def get_first_import(node, context, name, base, level):
     """return the node where [base.]<name> is imported or None if not found
diff --git a/checkers/utils.py b/checkers/utils.py
index 6fdfde2..000aee7 100644
--- a/checkers/utils.py
+++ b/checkers/utils.py
@@ -435,3 +435,30 @@ def inherit_from_std_ex(node):
         return True
     return any(inherit_from_std_ex(parent)
                for parent in node.ancestors(recurs=False))
+
+def is_import_error(handler):
+    """
+    Check if the given exception handler catches
+    ImportError.
+
+    :param handler: A node, representing an ExceptHandler node.
+    :returns: True if the handler catches ImportError, False otherwise.
+    """
+    names = None
+    if isinstance(handler.type, astroid.Tuple):
+        names = [name for name in handler.type.elts
+                 if isinstance(name, astroid.Name)]
+    elif isinstance(handler.type, astroid.Name):
+        names = [handler.type]
+    else:
+        # Don't try to infer that.
+        return
+    for name in names:
+        try:
+            for infered in name.infer():
+                if (isinstance(infered, astroid.Class) and
+                        inherit_from_std_ex(infered) and
+                        infered.name == 'ImportError'):
+                    return True
+        except astroid.InferenceError:
+            continue
diff --git a/test/functional/invalid_name.py b/test/functional/invalid_name.py
new file mode 100644
index 0000000..54b73f9
--- /dev/null
+++ b/test/functional/invalid_name.py
@@ -0,0 +1,28 @@
+""" Tests for invalid-name checker. """

+# pylint: disable=unused-import

+

+AAA = 24

+try:

+    import collections

+except ImportError:

+    collections = None

+

+aaa = 42 # [invalid-name]

+try:

+    import time

+except ValueError:

+    time = None # [invalid-name]

+

+try:

+    from sys import argv, executable as python

+except ImportError:

+    argv = 42

+    python = 24

+

+def test():

+    """ Shouldn't emit an invalid-name here. """

+    try:

+        import re

+    except ImportError:

+        re = None

+    return re

diff --git a/test/functional/invalid_name.txt b/test/functional/invalid_name.txt
new file mode 100644
index 0000000..c722861
--- /dev/null
+++ b/test/functional/invalid_name.txt
@@ -0,0 +1,2 @@
+invalid-name:10::Invalid constant name "aaa"

+invalid-name:14::Invalid constant name "time"