#17115,17116: Have modules initialize the __package__ and __loader__

attributes to None.

The long-term goal is for people to be able to rely on these
attributes existing and checking for None to see if they have been
set. Since import itself sets these attributes when a loader does not
the only instances when the attributes are None are from someone
overloading __import__() and not using a loader or someone creating a
module from scratch.

This patch also unifies module initialization. Before you could have
different attributes with default values depending on how the module
object was created. Now the only way to not get the same default set
of attributes is to circumvent initialization by calling
ModuleType.__new__() directly.

7 files changed, 34 insertions, 20 deletions

diff --git a/Lib/ctypes/test/__init__.py b/Lib/ctypes/test/__init__.py
index cc5fe02d1b..7c72210496 100644
--- a/Lib/ctypes/test/__init__.py
+++ b/Lib/ctypes/test/__init__.py
@@ -37,7 +37,7 @@ def requires(resource, msg=None):
 
 def find_package_modules(package, mask):
     import fnmatch
-    if (hasattr(package, "__loader__") and
+    if (package.__loader__ is not None and
             hasattr(package.__loader__, '_files')):
         path = package.__name__.replace(".", os.path.sep)
         mask = os.path.join(path, mask)
diff --git a/Lib/doctest.py b/Lib/doctest.py
index 16a732d037..1b8a9d4fe8 100644
--- a/Lib/doctest.py
+++ b/Lib/doctest.py
@@ -215,7 +215,7 @@ def _load_testfile(filename, package, module_relative, encoding):
     if module_relative:
         package = _normalize_module(package, 3)
         filename = _module_relative_path(package, filename)
-        if hasattr(package, '__loader__'):
+        if getattr(package, '__loader__', None) is not None:
             if hasattr(package.__loader__, 'get_data'):
                 file_contents = package.__loader__.get_data(filename)
                 file_contents = file_contents.decode(encoding)
diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py
index 1a046c5957..155403e429 100644
--- a/Lib/importlib/_bootstrap.py
+++ b/Lib/importlib/_bootstrap.py
@@ -1726,7 +1726,7 @@ def _setup(sys_module, _imp_module):
     module_type = type(sys)
     for name, module in sys.modules.items():
         if isinstance(module, module_type):
-            if not hasattr(module, '__loader__'):
+            if getattr(module, '__loader__', None) is None:
                 if name in sys.builtin_module_names:
                     module.__loader__ = BuiltinImporter
                 elif _imp.is_frozen(name):
diff --git a/Lib/inspect.py b/Lib/inspect.py
index e60a235fe4..22b9e84543 100644
--- a/Lib/inspect.py
+++ b/Lib/inspect.py
@@ -476,7 +476,7 @@ def getsourcefile(object):
     if os.path.exists(filename):
         return filename
     # only return a non-existent filename if the module has a PEP 302 loader
-    if hasattr(getmodule(object, filename), '__loader__'):
+    if getattr(getmodule(object, filename), '__loader__', None) is not None:
         return filename
     # or it is in the linecache
     if filename in linecache.cache:
diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py
index d38d1f40d0..8cb31553dd 100644
--- a/Lib/test/test_descr.py
+++ b/Lib/test/test_descr.py
@@ -2250,7 +2250,9 @@ order (MRO) for bases """
         minstance = M("m")
         minstance.b = 2
         minstance.a = 1
-        names = [x for x in dir(minstance) if x not in ["__name__", "__doc__"]]
+        default_attributes = ['__name__', '__doc__', '__package__',
+                              '__loader__']
+        names = [x for x in dir(minstance) if x not in default_attributes]
         self.assertEqual(names, ['a', 'b'])
 
         class M2(M):
diff --git a/Lib/test/test_importlib/test_api.py b/Lib/test/test_importlib/test_api.py
index ac88b2bf9c..297d6cbef6 100644
--- a/Lib/test/test_importlib/test_api.py
+++ b/Lib/test/test_importlib/test_api.py
@@ -197,14 +197,12 @@ class StartupTests(unittest.TestCase):
         # Issue #17098: all modules should have __loader__ defined.
         for name, module in sys.modules.items():
             if isinstance(module, types.ModuleType):
-                if name in sys.builtin_module_names:
-                    self.assertIn(module.__loader__,
-                            (importlib.machinery.BuiltinImporter,
-                             importlib._bootstrap.BuiltinImporter))
-                elif imp.is_frozen(name):
-                    self.assertIn(module.__loader__,
-                            (importlib.machinery.FrozenImporter,
-                             importlib._bootstrap.FrozenImporter))
+                self.assertTrue(hasattr(module, '__loader__'),
+                                '{!r} lacks a __loader__ attribute'.format(name))
+                if importlib.machinery.BuiltinImporter.find_module(name):
+                    self.assertIsNot(module.__loader__, None)
+                elif importlib.machinery.FrozenImporter.find_module(name):
+                    self.assertIsNot(module.__loader__, None)
 
 
 if __name__ == '__main__':
diff --git a/Lib/test/test_module.py b/Lib/test/test_module.py
index e5a2525d18..b34b30f91a 100644
--- a/Lib/test/test_module.py
+++ b/Lib/test/test_module.py
@@ -33,7 +33,10 @@ class ModuleTests(unittest.TestCase):
         foo = ModuleType("foo")
         self.assertEqual(foo.__name__, "foo")
         self.assertEqual(foo.__doc__, None)
-        self.assertEqual(foo.__dict__, {"__name__": "foo", "__doc__": None})
+        self.assertIs(foo.__loader__, None)
+        self.assertIs(foo.__package__, None)
+        self.assertEqual(foo.__dict__, {"__name__": "foo", "__doc__": None,
+                                        "__loader__": None, "__package__": None})
 
     def test_ascii_docstring(self):
         # ASCII docstring
@@ -41,7 +44,8 @@ class ModuleTests(unittest.TestCase):
         self.assertEqual(foo.__name__, "foo")
         self.assertEqual(foo.__doc__, "foodoc")
         self.assertEqual(foo.__dict__,
-                         {"__name__": "foo", "__doc__": "foodoc"})
+                         {"__name__": "foo", "__doc__": "foodoc",
+                          "__loader__": None, "__package__": None})
 
     def test_unicode_docstring(self):
         # Unicode docstring
@@ -49,7 +53,8 @@ class ModuleTests(unittest.TestCase):
         self.assertEqual(foo.__name__, "foo")
         self.assertEqual(foo.__doc__, "foodoc\u1234")
         self.assertEqual(foo.__dict__,
-                         {"__name__": "foo", "__doc__": "foodoc\u1234"})
+                         {"__name__": "foo", "__doc__": "foodoc\u1234",
+                          "__loader__": None, "__package__": None})
 
     def test_reinit(self):
         # Reinitialization should not replace the __dict__
@@ -61,7 +66,8 @@ class ModuleTests(unittest.TestCase):
         self.assertEqual(foo.__doc__, "foodoc")
         self.assertEqual(foo.bar, 42)
         self.assertEqual(foo.__dict__,
-              {"__name__": "foo", "__doc__": "foodoc", "bar": 42})
+              {"__name__": "foo", "__doc__": "foodoc", "bar": 42,
+               "__loader__": None, "__package__": None})
         self.assertTrue(foo.__dict__ is d)
 
     @unittest.expectedFailure
@@ -110,13 +116,19 @@ a = A(destroyed)"""
         m.__file__ = '/tmp/foo.py'
         self.assertEqual(repr(m), "<module '?' from '/tmp/foo.py'>")
 
+    def test_module_repr_with_loader_as_None(self):
+        m = ModuleType('foo')
+        assert m.__loader__ is None
+        self.assertEqual(repr(m), "<module 'foo'>")
+
     def test_module_repr_with_bare_loader_but_no_name(self):
         m = ModuleType('foo')
         del m.__name__
         # Yes, a class not an instance.
         m.__loader__ = BareLoader
+        loader_repr = repr(BareLoader)
         self.assertEqual(
-            repr(m), "<module '?' (<class 'test.test_module.BareLoader'>)>")
+            repr(m), "<module '?' ({})>".format(loader_repr))
 
     def test_module_repr_with_full_loader_but_no_name(self):
         # m.__loader__.module_repr() will fail because the module has no
@@ -126,15 +138,17 @@ a = A(destroyed)"""
         del m.__name__
         # Yes, a class not an instance.
         m.__loader__ = FullLoader
+        loader_repr = repr(FullLoader)
         self.assertEqual(
-            repr(m), "<module '?' (<class 'test.test_module.FullLoader'>)>")
+            repr(m), "<module '?' ({})>".format(loader_repr))
 
     def test_module_repr_with_bare_loader(self):
         m = ModuleType('foo')
         # Yes, a class not an instance.
         m.__loader__ = BareLoader
+        module_repr = repr(BareLoader)
         self.assertEqual(
-            repr(m), "<module 'foo' (<class 'test.test_module.BareLoader'>)>")
+            repr(m), "<module 'foo' ({})>".format(module_repr))
 
     def test_module_repr_with_full_loader(self):
         m = ModuleType('foo')