Issue #23014: Make importlib.abc.Loader.create_module() required when

importlib.abc.Loader.exec_module() is also defined.

Before this change, create_module() was optional **and** could return
None to trigger default semantics. This change now reduces the
options for choosing default semantics to one and in the most
backporting-friendly way (define create_module() to return None).

7 files changed, 36 insertions, 6 deletions

diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py
index e694521..0ed7cc6 100644
--- a/Lib/importlib/_bootstrap.py
+++ b/Lib/importlib/_bootstrap.py
@@ -1055,6 +1055,10 @@ def module_from_spec(spec):
         # If create_module() returns `None` then it means default
         # module creation should be used.
         module = spec.loader.create_module(spec)
+    elif hasattr(spec.loader, 'exec_module'):
+        _warnings.warn('starting in Python 3.6, loaders defining exec_module() '
+                       'must also define create_module()',
+                       DeprecationWarning, stacklevel=2)
     if module is None:
         module = _new_module(spec.name)
     _init_module_attrs(spec, module)
@@ -1298,6 +1302,10 @@ class FrozenImporter:
         """
         return cls if _imp.is_frozen(fullname) else None
 
+    @classmethod
+    def create_module(cls, spec):
+        """Use default semantics for module creation."""
+
     @staticmethod
     def exec_module(module):
         name = module.__spec__.name
@@ -1411,6 +1419,9 @@ class _LoaderBasics:
         tail_name = fullname.rpartition('.')[2]
         return filename_base == '__init__' and tail_name != '__init__'
 
+    def create_module(self, spec):
+        """Use default semantics for module creation."""
+
     def exec_module(self, module):
         """Execute the module."""
         code = self.get_code(module.__name__)
@@ -1771,6 +1782,9 @@ class _NamespaceLoader:
     def get_code(self, fullname):
         return compile('', '<string>', 'exec', dont_inherit=True)
 
+    def create_module(self, spec):
+        """Use default semantics for module creation."""
+
     def exec_module(self, module):
         pass
 
diff --git a/Lib/importlib/abc.py b/Lib/importlib/abc.py
index 6b6a602..2878488 100644
--- a/Lib/importlib/abc.py
+++ b/Lib/importlib/abc.py
@@ -122,9 +122,6 @@ class Loader(metaclass=abc.ABCMeta):
         This method should raise ImportError if anything prevents it
         from creating a new module.  It may return None to indicate
         that the spec should create the new module.
-
-        create_module() is optional.
-
         """
         # By default, defer to default semantics for the new module.
         return None
diff --git a/Lib/test/test_importlib/import_/test___loader__.py b/Lib/test/test_importlib/import_/test___loader__.py
index 9998cd6..4b18093 100644
--- a/Lib/test/test_importlib/import_/test___loader__.py
+++ b/Lib/test/test_importlib/import_/test___loader__.py
@@ -11,6 +11,9 @@ class SpecLoaderMock:
     def find_spec(self, fullname, path=None, target=None):
         return machinery.ModuleSpec(fullname, self)
 
+    def create_module(self, spec):
+        return None
+
     def exec_module(self, module):
         pass
 
diff --git a/Lib/test/test_importlib/import_/test_api.py b/Lib/test/test_importlib/import_/test_api.py
index 2c61b01..7069d9e 100644
--- a/Lib/test/test_importlib/import_/test_api.py
+++ b/Lib/test/test_importlib/import_/test_api.py
@@ -17,6 +17,10 @@ class BadSpecFinderLoader:
             return spec
 
     @staticmethod
+    def create_module(spec):
+        return None
+
+    @staticmethod
     def exec_module(module):
         if module.__name__ == SUBMOD_NAME:
             raise ImportError('I cannot be loaded!')
diff --git a/Lib/test/test_importlib/test_spec.py b/Lib/test/test_importlib/test_spec.py
index 418b4c0..8b333e8 100644
--- a/Lib/test/test_importlib/test_spec.py
+++ b/Lib/test/test_importlib/test_spec.py
@@ -34,6 +34,9 @@ class TestLoader:
     def _is_package(self, name):
         return self.package
 
+    def create_module(self, spec):
+        return None
+
 
 class NewLoader(TestLoader):
 
diff --git a/Lib/test/test_importlib/test_util.py b/Lib/test/test_importlib/test_util.py
index cd9344c..2493af7 100644
--- a/Lib/test/test_importlib/test_util.py
+++ b/Lib/test/test_importlib/test_util.py
@@ -41,10 +41,16 @@ class DecodeSourceBytesTests:
 class ModuleFromSpecTests:
 
     def test_no_create_module(self):
-        class Loader(self.abc.Loader):
-            pass
+        class Loader:
+            def exec_module(self, module):
+                pass
         spec = self.machinery.ModuleSpec('test', Loader())
-        module = self.util.module_from_spec(spec)
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter('always')
+            module = self.util.module_from_spec(spec)
+        self.assertEqual(1, len(w))
+        self.assertTrue(issubclass(w[0].category, DeprecationWarning))
+        self.assertIn('create_module', str(w[0].message))
         self.assertIsInstance(module, types.ModuleType)
         self.assertEqual(module.__name__, spec.name)
 
diff --git a/Lib/test/test_pkgutil.py b/Lib/test/test_pkgutil.py
index e0c8635de..57ebf1f 100644
--- a/Lib/test/test_pkgutil.py
+++ b/Lib/test/test_pkgutil.py
@@ -104,6 +104,9 @@ class PkgutilTests(unittest.TestCase):
 class PkgutilPEP302Tests(unittest.TestCase):
 
     class MyTestLoader(object):
+        def create_module(self, spec):
+            return None
+
         def exec_module(self, mod):
             # Count how many times the module is reloaded
             mod.__dict__['loads'] = mod.__dict__.get('loads', 0) + 1