Importation classes with imported name and alias

In order to solve many corner cases related to imports,
more information is needed about each import.

This change creates two new classes:
- SubmoduleImportation
- ImportationFrom

And adds an optional parameter full_name to the super class
Importation.

Functionally, this change only improves existing error messages
to report the full imported name where previously an error
would include only the import alias.

Author: jayvdb

pyflakes/checker.py

@@ -136,16 +136,83 @@ class Importation(Definition):
     @type fullName: C{str}
     """
 
-    def __init__(self, name, source):
-        self.fullName = name
+    def __init__(self, name, source, full_name=None):
+        self.fullName = full_name or name
         self.redefined = []
-        name = name.split('.')[0]
         super(Importation, self).__init__(name, source)
 
+    def redefines(self, other):
+        return isinstance(other, Definition) and self.name == other.name
+
+    def _has_alias(self):
+        """Return whether importation needs an as clause."""
+        return not self.fullName.split('.')[-1] == self.name
+
+    @property
+    def source_statement(self):
+        """Generate a source statement equivalent to the import."""
+        if self._has_alias():
+            return 'import %s as %s' % (self.fullName, self.name)
+        else:
+            return 'import %s' % self.fullName
+
+    def __str__(self):
+        """Return import full name with alias."""
+        if self._has_alias():
+            return self.fullName + ' as ' + self.name
+        else:
+            return self.fullName
+
+
+class SubmoduleImportation(Importation):
+
+    def __init__(self, name, source):
+        # A dot should only appear in the name when it is a submodule import
+        # without an 'as' clause, which is a special type of import where the
+        # root module is implicitly imported, and the submodules are also
+        # accessible because Python does not restrict which attributes of the
+        # root module may be used.
+        assert '.' in name and (not source or isinstance(source, ast.Import))
+        package_name = name.split('.')[0]
+        super(SubmoduleImportation, self).__init__(package_name, source)
+        self.fullName = name
+
     def redefines(self, other):
         if isinstance(other, Importation):
             return self.fullName == other.fullName
-        return isinstance(other, Definition) and self.name == other.name
+        return super(SubmoduleImportation, self).redefines(other)
+
+    def __str__(self):
+        return self.fullName
+
+    @property
+    def source_statement(self):
+        return 'import ' + self.fullName
+
+
+class ImportationFrom(Importation):
+
+    def __init__(self, name, source, module, real_name=None):
+        self.module = module
+        self.real_name = real_name or name
+        full_name = module + '.' + self.real_name
+        super(ImportationFrom, self).__init__(name, source, full_name)
+
+    def __str__(self):
+        """Return import full name with alias."""
+        if self.real_name != self.name:
+            return self.fullName + ' as ' + self.name
+        else:
+            return self.fullName
+
+    @property
+    def source_statement(self):
+        if self.real_name != self.name:
+            return 'from %s import %s as %s' % (self.module,
+                                                self.real_name,
+                                                self.name)
+        else:
+            return 'from %s import %s' % (self.module, self.name)
 
 
 class StarImportation(Importation):
@@ -158,16 +225,23 @@ def __init__(self, name, source):
         self.name = name + '.*'
         self.fullName = name
 
+    @property
+    def source_statement(self):
+        return 'from ' + self.fullName + ' import *'
 
-class FutureImportation(Importation):
+    def __str__(self):
+        return self.name
+
+
+class FutureImportation(ImportationFrom):
     """
     A binding created by a from `__future__` import statement.
 
     `__future__` imports are implicitly used.
     """
 
     def __init__(self, name, source, scope):
-        super(FutureImportation, self).__init__(name, source)
+        super(FutureImportation, self).__init__(name, source, '__future__')
         self.used = (scope, source)
 
 
@@ -430,7 +504,7 @@ def checkDeadScopes(self):
                     used = value.used or value.name in all_names
                     if not used:
                         messg = messages.UnusedImport
-                        self.report(messg, value.source, value.name)
+                        self.report(messg, value.source, str(value))
                     for node in value.redefined:
                         if isinstance(self.getParent(node), ast.For):
                             messg = messages.ImportShadowedByLoopVar
@@ -1039,8 +1113,11 @@ def TUPLE(self, node):
 
     def IMPORT(self, node):
         for alias in node.names:
-            name = alias.asname or alias.name
-            importation = Importation(name, node)
+            if '.' in alias.name and not alias.asname:
+                importation = SubmoduleImportation(alias.name, node)
+            else:
+                name = alias.asname or alias.name
+                importation = Importation(name, node, alias.name)
             self.addBinding(node, importation)
 
     def IMPORTFROM(self, node):
@@ -1069,7 +1146,8 @@ def IMPORTFROM(self, node):
                 self.report(messages.ImportStarUsed, node, node.module)
                 importation = StarImportation(node.module, node)
             else:
-                importation = Importation(name, node)
+                importation = ImportationFrom(name, node,
+                                              node.module, alias.name)
             self.addBinding(node, importation)
 
     def TRY(self, node):

pyflakes/test/test_imports.py

@@ -2,9 +2,75 @@
 from sys import version_info
 
 from pyflakes import messages as m
+from pyflakes.checker import (
+    FutureImportation,
+    Importation,
+    ImportationFrom,
+    StarImportation,
+    SubmoduleImportation,
+)
 from pyflakes.test.harness import TestCase, skip, skipIf
 
 
+class TestImportationObject(TestCase):
+
+    def test_import_basic(self):
+        binding = Importation('a', None, 'a')
+        assert binding.source_statement == 'import a'
+        assert str(binding) == 'a'
+
+    def test_import_as(self):
+        binding = Importation('c', None, 'a')
+        assert binding.source_statement == 'import a as c'
+        assert str(binding) == 'a as c'
+
+    def test_import_submodule(self):
+        binding = SubmoduleImportation('a.b', None)
+        assert binding.source_statement == 'import a.b'
+        assert str(binding) == 'a.b'
+
+    def test_import_submodule_as(self):
+        # A submodule import with an as clause is not a SubmoduleImportation
+        binding = Importation('c', None, 'a.b')
+        assert binding.source_statement == 'import a.b as c'
+        assert str(binding) == 'a.b as c'
+
+    def test_import_submodule_as_source_name(self):
+        binding = Importation('a', None, 'a.b')
+        assert binding.source_statement == 'import a.b as a'
+        assert str(binding) == 'a.b as a'
+
+    def test_importfrom_member(self):
+        binding = ImportationFrom('b', None, 'a', 'b')
+        assert binding.source_statement == 'from a import b'
+        assert str(binding) == 'a.b'
+
+    def test_importfrom_submodule_member(self):
+        binding = ImportationFrom('c', None, 'a.b', 'c')
+        assert binding.source_statement == 'from a.b import c'
+        assert str(binding) == 'a.b.c'
+
+    def test_importfrom_member_as(self):
+        binding = ImportationFrom('c', None, 'a', 'b')
+        assert binding.source_statement == 'from a import b as c'
+        assert str(binding) == 'a.b as c'
+
+    def test_importfrom_submodule_member_as(self):
+        binding = ImportationFrom('d', None, 'a.b', 'c')
+        assert binding.source_statement == 'from a.b import c as d'
+        assert str(binding) == 'a.b.c as d'
+
+    def test_importfrom_star(self):
+        binding = StarImportation('a.b', None)
+        assert binding.source_statement == 'from a.b import *'
+        assert str(binding) == 'a.b.*'
+
+    def test_importfrom_future(self):
+        binding = FutureImportation('print_function', None, None)
+        assert binding.source_statement == 'from __future__ import print_function'
+        assert str(binding) == '__future__.print_function'
+
+
 class Test(TestCase):
 
     def test_unusedImport(self):
@@ -17,6 +83,12 @@ def test_aliasedImport(self):
         self.flakes('from moo import fu as FU, bar as FU',
                     m.RedefinedWhileUnused, m.UnusedImport)
 
+    def test_aliasedImportShadowModule(self):
+        """Imported aliases can shadow the source of the import."""
+        self.flakes('from moo import fu as moo; moo')
+        self.flakes('import fu as fu; fu')
+        self.flakes('import fu.bar as fu; fu')
+
     def test_usedImport(self):
         self.flakes('import fu; print(fu)')
         self.flakes('from baz import fu; print(fu)')
@@ -685,6 +757,29 @@ def test_differentSubmoduleImport(self):
         fu.bar, fu.baz
         ''')
 
+    def test_used_package_with_submodule_import(self):
+        """
+        Usage of package marks submodule imports as used.
+        """
+        self.flakes('''
+        import fu
+        import fu.bar
+        fu.x
+        ''')
+
+    def test_unused_package_with_submodule_import(self):
+        """
+        When a package and its submodule are imported, only report once.
+        """
+        checker = self.flakes('''
+        import fu
+        import fu.bar
+        ''', m.UnusedImport)
+        error = checker.messages[0]
+        assert error.message == '%r imported but unused'
+        assert error.message_args == ('fu.bar', )
+        assert error.lineno == 5 if self.withDoctest else 3
+
     def test_assignRHSFirst(self):
         self.flakes('import fu; fu = fu')
         self.flakes('import fu; fu, bar = fu')