Merged logilab/pylint into default

15 files changed, 251 insertions, 24 deletions

diff --git a/ChangeLog b/ChangeLog
index 390b374..e27c966 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -2,6 +2,10 @@ ChangeLog for Pylint
 ====================
 
 --
+
+    * Emit 'undefined-variable' for undefined names when using the
+      Python 3 `metaclass=` argument.
+
     * Checkers respect priority now. Close issue #229.
     
     * Fix a false positive regarding W0511. Closes issue #149.
@@ -39,6 +43,16 @@ ChangeLog for Pylint
     * Fix an 'unused-variable' false positive, where the variable is
       assigned through an import. Closes issue #196.
 
+    * Definition order is considered for classes, function arguments
+      and annotations. Closes issue #257.
+
+    * Don't emit 'unused-variable' when assigning to a nonlocal.
+      Closes issue #275.
+   
+    * Do not let ImportError propagate from the import checker, leading to crash
+      in some namespace package related cases. Closes issue #203.
+
+
 2014-04-30  --  1.2.1
     * Restore the ability to specify the init-hook option via the
       configuration file, which was accidentally broken in 1.2.0.
diff --git a/checkers/__init__.py b/checkers/__init__.py
index af7965b..693a5ff 100644
--- a/checkers/__init__.py
+++ b/checkers/__init__.py
@@ -42,7 +42,6 @@ import sys
 import tokenize
 import warnings
 
-from astroid.utils import ASTWalker
 from logilab.common.configuration import OptionsProviderMixIn
 
 from pylint.reporters import diff_string
@@ -69,7 +68,7 @@ def table_lines_from_stats(stats, old_stats, columns):
     return lines
 
 
-class BaseChecker(OptionsProviderMixIn, ASTWalker):
+class BaseChecker(OptionsProviderMixIn):
     """base class for checkers"""
     # checker name (you may reuse an existing one)
     name = None
@@ -87,7 +86,6 @@ class BaseChecker(OptionsProviderMixIn, ASTWalker):
 
         linter is an object implementing ILinter
         """
-        ASTWalker.__init__(self, self)
         self.name = self.name.lower()
         OptionsProviderMixIn.__init__(self)
         self.linter = linter
diff --git a/checkers/imports.py b/checkers/imports.py
index 8b73c6f..7194134 100644
--- a/checkers/imports.py
+++ b/checkers/imports.py
@@ -18,11 +18,11 @@
 import sys
 
 from logilab.common.graph import get_cycles, DotBackend
-from logilab.common.modutils import get_module_part, is_standard_module
 from logilab.common.ureports import VerbatimText, Paragraph
 
 import astroid
 from astroid import are_exclusive
+from astroid.modutils import get_module_part, is_standard_module
 
 from pylint.interfaces import IAstroidChecker
 from pylint.utils import EmptyReport
@@ -299,7 +299,10 @@ given file (report RP0402 must not be disabled)'}
 
     def _add_imported_module(self, node, importedmodname):
         """notify an imported module, used to analyze dependencies"""
-        importedmodname = get_module_part(importedmodname)
+        try:
+            importedmodname = get_module_part(importedmodname)
+        except ImportError:
+            pass
         context_name = node.root().name
         if context_name == importedmodname:
             # module importing itself !
diff --git a/checkers/strings.py b/checkers/strings.py
index 4fe16dd..ad63580 100644
--- a/checkers/strings.py
+++ b/checkers/strings.py
@@ -327,10 +327,10 @@ class StringMethodsChecker(BaseChecker):
             self.add_message('bad-format-string', node=node)
             return
 
-        manual_fields = {field[0] for field in fields
-                         if isinstance(field[0], int)}
-        named_fields = {field[0] for field in fields
-                        if isinstance(field[0], str)}
+        manual_fields = set(field[0] for field in fields
+                            if isinstance(field[0], int))
+        named_fields = set(field[0] for field in fields
+                           if isinstance(field[0], str))
         if manual_fields and num_args:
             self.add_message('format-combined-specification',
                              node=node)
diff --git a/checkers/variables.py b/checkers/variables.py
index 94a8d6e..8f8ee87 100644
--- a/checkers/variables.py
+++ b/checkers/variables.py
@@ -69,6 +69,71 @@ def _get_unpacking_extra_info(node, infered):
         more = ' defined at line %s of %s' % (infered.lineno, infered_module)
     return more
 
+def _detect_global_scope(node, frame, defframe):
+    """ Detect that the given frames shares a global
+    scope.
+
+    Two frames shares a global scope when neither
+    of them are hidden under a function scope, as well
+    as any of parent scope of them, until the root scope.
+    In this case, depending from something defined later on
+    will not work, because it is still undefined.
+
+    Example:
+        class A:
+            # B has the same global scope as `C`, leading to a NameError.
+            class B(C): ...
+        class C: ...
+
+    """
+    def_scope = scope = None
+    if frame and frame.parent:
+        scope = frame.parent.scope()
+    if defframe and defframe.parent:
+        def_scope = defframe.parent.scope()
+    if isinstance(frame, astroid.Function):
+        # If the parent of the current node is a
+        # function, then it can be under its scope
+        # (defined in, which doesn't concern us) or
+        # the `->` part of annotations. The same goes
+        # for annotations of function arguments, they'll have
+        # their parent the Arguments node.
+        if not isinstance(node.parent,
+                          (astroid.Function, astroid.Arguments)):
+            return False
+    elif any(not isinstance(f, (astroid.Class, astroid.Module))
+            for f in (frame, defframe)):
+        # Not interested in other frames, since they are already
+        # not in a global scope.
+        return False
+
+    break_scopes = []
+    for s in (scope, def_scope):
+        # Look for parent scopes. If there is anything different
+        # than a module or a class scope, then they frames don't
+        # share a global scope.
+        parent_scope = s
+        while parent_scope:
+            if not isinstance(parent_scope, (astroid.Class, astroid.Module)):
+                break_scopes.append(parent_scope)
+                break
+            if parent_scope.parent:
+                parent_scope = parent_scope.parent.scope()
+            else:
+                break
+    if break_scopes and len(set(break_scopes)) != 1:
+        # Store different scopes than expected.
+        # If the stored scopes are, in fact, the very same, then it means
+        # that the two frames (frame and defframe) shares the same scope,
+        # and we could apply our lineno analysis over them.
+        # For instance, this works when they are inside a function, the node
+        # that uses a definition and the definition itself.
+        return False
+    # At this point, we are certain that frame and defframe shares a scope
+    # and the definition of the first depends on the second.
+    return frame.lineno < defframe.lineno
+
+
 MSGS = {
     'E0601': ('Using variable %r before assignment',
               'used-before-assignment',
@@ -357,8 +422,11 @@ builtins. Remember that you should avoid to define new builtins when possible.'
         called_overridden = False
         argnames = node.argnames()
         global_names = set()
+        nonlocal_names = set()
         for global_stmt in node.nodes_of_class(astroid.Global):
             global_names.update(set(global_stmt.names))
+        for nonlocal_stmt in node.nodes_of_class(astroid.Nonlocal):
+            nonlocal_names.update(set(nonlocal_stmt.names))
 
         for name, stmts in not_consumed.iteritems():
             # ignore some special names specified by user configuration
@@ -405,6 +473,9 @@ builtins. Remember that you should avoid to define new builtins when possible.'
                     continue
                 self.add_message('unused-argument', args=name, node=stmt)
             else:
+                if stmt.parent and isinstance(stmt.parent, astroid.Assign):
+                    if name in nonlocal_names:
+                        continue
                 self.add_message('unused-variable', args=name, node=stmt)
 
     @check_messages('global-variable-undefined', 'global-variable-not-assigned', 'global-statement',
@@ -593,7 +664,7 @@ builtins. Remember that you should avoid to define new builtins when possible.'
                 defframe = defstmt.frame()
                 maybee0601 = True
                 if not frame is defframe:
-                    maybee0601 = False
+                    maybee0601 = _detect_global_scope(node, frame, defframe)
                 elif defframe.parent is None:
                     # we are at the module level, check the name is not
                     # defined in builtins
@@ -770,16 +841,52 @@ class VariablesChecker3k(VariablesChecker):
         """ Update consumption analysis variable
         for metaclasses.
         """
+        module_locals = self._to_consume[0][0]
+        module_imports = self._to_consume[0][1]
+        consumed = {}
+
         for klass in node.nodes_of_class(astroid.Class):
-            if klass._metaclass:
-                metaclass = klass.metaclass()
-                module_locals = self._to_consume[0][0]
+            found = metaclass = name = None
+            if not klass._metaclass:
+                # Skip if this class doesn't use
+                # explictly a metaclass, but inherits it from ancestors
+                continue
+
+            metaclass = klass.metaclass()
+
+            # Look the name in the already found locals.
+            # If it's not found there, look in the module locals
+            # and in the imported modules.
+            if isinstance(klass._metaclass, astroid.Name):
+                name = klass._metaclass.name
+            elif metaclass:
+                # if it uses a `metaclass=module.Class`
+                name = metaclass.root().name
+
+            if name:
+                found = consumed.setdefault(name,
+                    module_locals.get(name, module_imports.get(name))
+                )
 
+            if found is None and not metaclass:
+                name = None
                 if isinstance(klass._metaclass, astroid.Name):
-                    module_locals.pop(klass._metaclass.name, None)
-                if metaclass:
-                    # if it uses a `metaclass=module.Class`
-                    module_locals.pop(metaclass.root().name, None)
+                    name = klass._metaclass.name
+                elif isinstance(klass._metaclass, astroid.Getattr):
+                    name = klass._metaclass.as_string()
+
+                if name is not None:
+                    if not (name in astroid.Module.scope_attrs or
+                            is_builtin(name) or
+                            name in self.config.additional_builtins or
+                            name in node.locals):
+                        self.add_message('undefined-variable',
+                                         node=klass,
+                                         args=(name, ))
+        # Pop the consumed items, in order to
+        # avoid having unused-import false positives
+        for name in consumed:
+            module_locals.pop(name, None)
         super(VariablesChecker3k, self).leave_module(node)
 
 if sys.version_info >= (3, 0):
diff --git a/debian/control b/debian/control
index 442737d..da0c1d1 100644
--- a/debian/control
+++ b/debian/control
@@ -19,7 +19,7 @@ Architecture: all
 Depends: ${python:Depends},
          ${misc:Depends},
 	 python-logilab-common (>= 0.53.0),
-	 python-astroid (>= 1.1)
+	 python-astroid (>= 1.2)
 Suggests: python-tk
 XB-Python-Version: ${python:Versions}
 Description: python code static checker and UML diagram generator
diff --git a/lint.py b/lint.py
index 26e099d..0b7b44f 100644
--- a/lint.py
+++ b/lint.py
@@ -38,7 +38,6 @@ from warnings import warn
 
 from logilab.common.configuration import UnsupportedAction, OptionsManagerMixIn
 from logilab.common.optik_ext import check_csv
-from logilab.common.modutils import load_module_from_name, get_module_part
 from logilab.common.interface import implements
 from logilab.common.textutils import splitstrip, unquote
 from logilab.common.ureports import Table, Text, Section
@@ -46,6 +45,7 @@ from logilab.common.__pkginfo__ import version as common_version
 
 from astroid import MANAGER, nodes, AstroidBuildingException
 from astroid.__pkginfo__ import version as astroid_version
+from astroid.modutils import load_module_from_name, get_module_part
 
 from pylint.utils import (
     MSG_TYPES, OPTION_RGX,
diff --git a/test/input/func_noerror_unused_variable_py30.py b/test/input/func_noerror_unused_variable_py30.py
new file mode 100644
index 0000000..ffcc978
--- /dev/null
+++ b/test/input/func_noerror_unused_variable_py30.py
@@ -0,0 +1,14 @@
+""" Test nonlocal uses and unused-variable. """
+
+__revision__ = 1
+
+def test_nonlocal():
+    """ Test that assigning to a nonlocal does not trigger
+    an 'unused-variable' warnings.
+    """
+    attr = True
+    def set_value(val):
+        """ Set the value in a nonlocal. """
+        nonlocal attr
+        attr = val
+    return set_value
diff --git a/test/input/func_undefined_metaclass_var_py30.py b/test/input/func_undefined_metaclass_var_py30.py
new file mode 100644
index 0000000..307a431
--- /dev/null
+++ b/test/input/func_undefined_metaclass_var_py30.py
@@ -0,0 +1,27 @@
+"""test access to undefined variables in Python 3 metaclass syntax """
+# pylint: disable=no-init, invalid-name, too-few-public-methods
+__revision__ = '$Id:'
+
+import abc
+from abc import ABCMeta
+
+class Bad(metaclass=ABCMet):
+    """ Notice the typo """
+
+class SecondBad(metaclass=ab.ABCMeta):
+    """ Notice the `ab` module. """
+
+class Good(metaclass=int):
+    """ int is not a proper metaclass, but it is defined. """
+
+class SecondGood(metaclass=Good):
+    """ empty """
+
+class ThirdGood(metaclass=ABCMeta):
+    """ empty """
+
+class FourthGood(ThirdGood):
+    """ This should not trigger anything. """
+
+data = abc
+testdata = ABCMeta
diff --git a/test/input/func_undefined_var.py b/test/input/func_undefined_var.py
index 407f3f6..fb5fc30 100644
--- a/test/input/func_undefined_var.py
+++ b/test/input/func_undefined_var.py
@@ -1,5 +1,5 @@
 """test access to undefined variables"""
-
+# pylint: disable=too-few-public-methods, no-init, no-self-use
 __revision__ = '$Id:'
 
 DEFINED = 1
@@ -83,3 +83,37 @@ def func1():
 def func2():
     """A function with a decorator that contains a genexpr."""
     pass
+
+# Test shared scope.
+
+def test_arguments(arg=TestClass):
+    """ TestClass isn't defined yet. """
+    return arg
+
+class TestClass(Ancestor):
+    """ contains another class, which uses an undefined ancestor. """
+
+    class MissingAncestor(Ancestor1):
+        """ no op """
+
+    def test1(self):
+        """ It should trigger here, because the two classes
+        have the same scope.
+        """
+        class UsingBeforeDefinition(Empty):
+            """ uses Empty before definition """
+        class Empty(object):
+            """ no op """
+        return UsingBeforeDefinition
+
+    def test(self):
+        """ Ancestor isn't defined yet, but we don't care. """
+        class MissingAncestor1(Ancestor):
+            """ no op """
+        return MissingAncestor1
+
+class Ancestor(object):
+    """ No op """
+
+class Ancestor1(object):
+    """ No op """
diff --git a/test/input/func_used_before_assignment_py30.py b/test/input/func_used_before_assignment_py30.py
index b5d0bf3..ae979a1 100644
--- a/test/input/func_used_before_assignment_py30.py
+++ b/test/input/func_used_before_assignment_py30.py
@@ -1,5 +1,5 @@
 """Check for nonlocal and used-before-assignment"""

-# pylint: disable=missing-docstring, unused-variable

+# pylint: disable=missing-docstring, unused-variable, no-init, too-few-public-methods

 

 __revision__ = 0

 

@@ -26,3 +26,23 @@ def test_fail2():
         nonlocal count

         cnt = cnt + 1

     wrap()

+

+def test_fail3(arg: test_fail4):

+    """ Depends on `test_fail4`, in argument annotation. """

+    return arg

+

+def test_fail4(*args: test_fail5, **kwargs: undefined):

+    """ Depends on `test_fail5` and `undefined` in

+    variable and named arguments annotations.

+    """

+    return args, kwargs

+

+def test_fail5()->undefined1:

+    """ Depends on `undefined1` in function return annotation. """

+

+def undefined():

+    """ no op """

+

+def undefined1():

+    """ no op """

+

diff --git a/test/messages/func_undefined_metaclass_var_py30.txt b/test/messages/func_undefined_metaclass_var_py30.txt
new file mode 100644
index 0000000..a82ac6a
--- /dev/null
+++ b/test/messages/func_undefined_metaclass_var_py30.txt
@@ -0,0 +1,2 @@
+E:  8:Bad: Undefined variable 'ABCMet'

+E: 11:SecondBad: Undefined variable 'ab.ABCMeta'
\ No newline at end of file
diff --git a/test/messages/func_undefined_var.txt b/test/messages/func_undefined_var.txt
index 25fb2c3..5505156 100644
--- a/test/messages/func_undefined_var.txt
+++ b/test/messages/func_undefined_var.txt
@@ -7,4 +7,8 @@ E: 27:bad_default: Undefined variable 'augvar'
 E: 28:bad_default: Undefined variable 'vardel'
 E: 56: Using variable 'PLOUF' before assignment
 E: 65:if_branch_test: Using variable 'xxx' before assignment
+E: 89:test_arguments: Using variable 'TestClass' before assignment
+E: 93:TestClass: Using variable 'Ancestor' before assignment
+E: 96:TestClass.MissingAncestor: Using variable 'Ancestor1' before assignment
+E:103:TestClass.test1.UsingBeforeDefinition: Using variable 'Empty' before assignment
 W: 27:bad_default: Unused variable 'augvar'
diff --git a/test/messages/func_used_before_assignment_py30.txt b/test/messages/func_used_before_assignment_py30.txt
index 5b6080f..8bb131d 100644
--- a/test/messages/func_used_before_assignment_py30.txt
+++ b/test/messages/func_used_before_assignment_py30.txt
@@ -1,2 +1,6 @@
 E: 18:test_fail.wrap: Using variable 'cnt' before assignment

-E: 27:test_fail2.wrap: Using variable 'cnt' before assignment
\ No newline at end of file
+E: 27:test_fail2.wrap: Using variable 'cnt' before assignment

+E: 30:test_fail3: Using variable 'test_fail4' before assignment

+E: 34:test_fail4: Using variable 'test_fail5' before assignment

+E: 34:test_fail4: Using variable 'undefined' before assignment

+E: 40:test_fail5: Using variable 'undefined1' before assignment
\ No newline at end of file
diff --git a/utils.py b/utils.py
index c2f1704..6fbb480 100644
--- a/utils.py
+++ b/utils.py
@@ -25,13 +25,13 @@ from warnings import warn
 from os.path import dirname, basename, splitext, exists, isdir, join, normpath
 
 from logilab.common.interface import implements
-from logilab.common.modutils import modpath_from_file, get_module_files, \
-                                    file_from_modpath, load_module_from_file
 from logilab.common.textutils import normalize_text
 from logilab.common.configuration import rest_format_section
 from logilab.common.ureports import Section
 
 from astroid import nodes, Module
+from astroid.modutils import modpath_from_file, get_module_files, \
+    file_from_modpath, load_module_from_file
 
 from pylint.interfaces import IRawChecker, ITokenChecker