Report each usage of star imports

Also detect unused star imports.

4 files changed, 82 insertions, 13 deletions

diff --git a/pyflakes/checker.py b/pyflakes/checker.py
index 58b3826..e693f20 100644
--- a/pyflakes/checker.py
+++ b/pyflakes/checker.py
@@ -147,6 +147,17 @@ class Importation(Definition):
         return isinstance(other, Definition) and self.name == other.name
 
 
+class StarImportation(Importation):
+    """A binding created by an 'from x import *' statement."""
+
+    def __init__(self, name, source):
+        super(StarImportation, self).__init__('*', source)
+        # Each star importation needs a unique name, and
+        # may not be the module name otherwise it will be deemed imported
+        self.name = name + '.*'
+        self.fullName = name
+
+
 class Argument(Binding):
     """
     Represents binding a name as an argument.
@@ -358,17 +369,29 @@ class Checker(object):
             if isinstance(scope, ClassScope):
                 continue
 
-            if isinstance(scope.get('__all__'), ExportBinding):
-                all_names = set(scope['__all__'].names)
+            all_binding = scope.get('__all__')
+            if all_binding and not isinstance(all_binding, ExportBinding):
+                all_binding = None
+
+            if all_binding:
+                all_names = set(all_binding.names)
+                undefined = all_names.difference(scope)
+            else:
+                all_names = undefined = []
+
+            if undefined:
                 if not scope.importStarred and \
                    os.path.basename(self.filename) != '__init__.py':
                     # Look for possible mistakes in the export list
-                    undefined = all_names.difference(scope)
                     for name in undefined:
                         self.report(messages.UndefinedExport,
                                     scope['__all__'].source, name)
-            else:
-                all_names = []
+
+                # mark all import '*' as used by the undefined in __all__
+                if scope.importStarred:
+                    for binding in scope.values():
+                        if isinstance(binding, StarImportation):
+                            binding.used = all_binding
 
             # Look for imported names that aren't used.
             for value in scope.values():
@@ -504,8 +527,24 @@ class Checker(object):
                 in_generators = isinstance(scope, GeneratorScope)
 
         # look in the built-ins
-        if importStarred or name in self.builtIns:
+        if name in self.builtIns:
             return
+
+        if importStarred:
+            from_list = []
+
+            for scope in self.scopeStack[-1::-1]:
+                for binding in scope.values():
+                    if isinstance(binding, StarImportation):
+                        # mark '*' imports as used for each scope
+                        binding.used = (self.scope, node)
+                        from_list.append(binding.fullName)
+
+            # report * usage, with a list of possible sources
+            from_list = ', '.join(sorted(from_list))
+            self.report(messages.ImportStarUsage, node, name, from_list)
+            return
+
         if name == '__path__' and os.path.basename(self.filename) == '__init__.py':
             # the special name __path__ is valid only in packages
             return
@@ -976,17 +1015,19 @@ class Checker(object):
             self.futuresAllowed = False
 
         for alias in node.names:
+            name = alias.asname or alias.name
             if alias.name == '*':
                 # Only Python 2, local import * is a SyntaxWarning
                 if not PY2 and not isinstance(self.scope, ModuleScope):
                     self.report(messages.ImportStarNotPermitted,
                                 node, node.module)
                     continue
+
                 self.scope.importStarred = True
                 self.report(messages.ImportStarUsed, node, node.module)
-                continue
-            name = alias.asname or alias.name
-            importation = Importation(name, node)
+                importation = StarImportation(node.module, node)
+            else:
+                importation = Importation(name, node)
             if node.module == '__future__':
                 importation.used = (self.scope, node)
             self.addBinding(node, importation)
diff --git a/pyflakes/messages.py b/pyflakes/messages.py
index 9380d02..e9aef81 100644
--- a/pyflakes/messages.py
+++ b/pyflakes/messages.py
@@ -65,6 +65,14 @@ class ImportStarUsed(Message):
         self.message_args = (modname,)
 
 
+class ImportStarUsage(Message):
+    message = "%s may be undefined, or defined from star imports: %s"
+
+    def __init__(self, filename, loc, name, from_list):
+        Message.__init__(self, filename, loc)
+        self.message_args = (name, from_list)
+
+
 class UndefinedName(Message):
     message = 'undefined name %r'
 
diff --git a/pyflakes/test/test_imports.py b/pyflakes/test/test_imports.py
index 54351e9..d6f9205 100644
--- a/pyflakes/test/test_imports.py
+++ b/pyflakes/test/test_imports.py
@@ -607,13 +607,13 @@ class Test(TestCase):
 
     def test_importStar(self):
         """Use of import * at module level is reported."""
-        self.flakes('from fu import *', m.ImportStarUsed)
+        self.flakes('from fu import *', m.ImportStarUsed, m.UnusedImport)
         self.flakes('''
         try:
             from fu import *
         except:
             pass
-        ''', m.ImportStarUsed)
+        ''', m.ImportStarUsed, m.UnusedImport)
 
     @skipIf(version_info < (3,),
             'import * below module level is a warning on Python 2')
@@ -628,6 +628,17 @@ class Test(TestCase):
             from fu import *
         ''', m.ImportStarNotPermitted)
 
+    @skipIf(version_info > (3,),
+            'import * below module level is an error on Python 3')
+    def test_importStarNested(self):
+        """All star imports are marked as used by an undefined variable."""
+        self.flakes('''
+        from fu import *
+        def f():
+            from bar import *
+            x
+        ''', m.ImportStarUsed, m.ImportStarUsed, m.ImportStarUsage)
+
     def test_packageImport(self):
         """
         If a dotted name is imported and used, no warning is reported.
@@ -868,6 +879,14 @@ class TestSpecialAll(TestCase):
         __all__ = ["foo"]
         ''', m.ImportStarUsed)
 
+    def test_importStarNotExported(self):
+        """Report unused import when not needed to satisfy __all__."""
+        self.flakes('''
+        from foolib import *
+        a = 1
+        __all__ = ['a']
+        ''', m.ImportStarUsed, m.UnusedImport)
+
     def test_usedInGenExp(self):
         """
         Using a global in a generator expression results in no warnings.
diff --git a/pyflakes/test/test_undefined_names.py b/pyflakes/test/test_undefined_names.py
index 81dab13..5653b91 100644
--- a/pyflakes/test/test_undefined_names.py
+++ b/pyflakes/test/test_undefined_names.py
@@ -71,7 +71,8 @@ class Test(TestCase):
 
     def test_globalImportStar(self):
         """Can't find undefined names with import *."""
-        self.flakes('from fu import *; bar', m.ImportStarUsed)
+        self.flakes('from fu import *; bar',
+                    m.ImportStarUsed, m.ImportStarUsage)
 
     @skipIf(version_info >= (3,), 'obsolete syntax')
     def test_localImportStar(self):
@@ -83,7 +84,7 @@ class Test(TestCase):
         def a():
             from fu import *
         bar
-        ''', m.ImportStarUsed, m.UndefinedName)
+        ''', m.ImportStarUsed, m.UndefinedName, m.UnusedImport)
 
     @skipIf(version_info >= (3,), 'obsolete syntax')
     def test_unpackedParameter(self):