Issue #17093,17566,17567: Methods from classes in importlib.abc now raise/return

the default exception/value when called instead of raising/returning
NotimplementedError/NotImplemented (except where appropriate).
This should allow for the ABCs to act as the bottom/end of the MRO with expected
default results.

As part of this work, also make importlib.abc.Loader.module_repr()
optional instead of an abstractmethod.

Doc/library/importlib.rst

@@ -153,6 +153,10 @@ ABC hierarchy::
       module.  Originally specified in :pep:`302`, this method was meant
       for use in :data:`sys.meta_path` and in the path-based import subsystem.
 
+      .. versionchanged:: 3.4
+         Returns ``None`` when called instead of raising
+         :exc:`NotImplementedError`.
+
 
 .. class:: MetaPathFinder
 
@@ -169,12 +173,19 @@ ABC hierarchy::
       will be the value of :attr:`__path__` from the parent
       package. If a loader cannot be found, ``None`` is returned.
 
+      .. versionchanged:: 3.4
+         Returns ``None`` when called instead of raising
+         :exc:`NotImplementedError`.
+
    .. method:: invalidate_caches()
 
       An optional method which, when called, should invalidate any internal
       cache used by the finder. Used by :func:`importlib.invalidate_caches`
       when invalidating the caches of all finders on :data:`sys.meta_path`.
 
+      .. versionchanged:: 3.4
+         Returns ``None`` when called instead of ``NotImplemented``.
+
 
 .. class:: PathEntryFinder
 
@@ -182,7 +193,7 @@ ABC hierarchy::
    it bears some similarities to :class:`MetaPathFinder`, ``PathEntryFinder``
    is meant for use only within the path-based import subsystem provided
    by :class:`PathFinder`. This ABC is a subclass of :class:`Finder` for
-   compatibility.
+   compatibility reasons only.
 
    .. versionadded:: 3.3
 
@@ -194,9 +205,12 @@ ABC hierarchy::
       package. The loader may be ``None`` while specifying ``portion`` to
       signify the contribution of the file system locations to a namespace
       package. An empty list can be used for ``portion`` to signify the loader
-      is not part of a package. If ``loader`` is ``None`` and ``portion`` is
+      is not part of a namespace package. If ``loader`` is ``None`` and
-      the empty list then no loader or location for a namespace package were
+      ``portion`` is the empty list then no loader or location for a namespace
-      found (i.e. failure to find anything for the module).
+      package were found (i.e. failure to find anything for the module).
+
+      .. versionchanged:: 3.4
+         Returns ``(None, [])`` instead of raising :exc:`NotImplementedError`.
 
    .. method:: find_module(fullname):
 
@@ -249,21 +263,29 @@ ABC hierarchy::
         - :attr:`__package__`
             The parent package for the module/package. If the module is
             top-level then it has a value of the empty string. The
-            :func:`importlib.util.set_package` decorator can handle the details
+            :func:`importlib.util.module_for_loader` decorator can handle the
-            for :attr:`__package__`.
+            details for :attr:`__package__`.
 
         - :attr:`__loader__`
-            The loader used to load the module.
+            The loader used to load the module. The
-            (This is not set by the built-in import machinery,
+            :func:`importlib.util.module_for_loader` decorator can handle the
-            but it should be set whenever a :term:`loader` is used.)
+            details for :attr:`__package__`.
+
+        .. versionchanged:: 3.4
+           Raise :exc:`ImportError` when called instead of
+           :exc:`NotImplementedError`.
 
     .. method:: module_repr(module)
 
-        An abstract method which when implemented calculates and returns the
+        An optional method which when implemented calculates and returns the
-        given module's repr, as a string.
+        given module's repr, as a string. The module type's default repr() will
+        use the result of this method as appropriate.
 
         .. versionadded: 3.3
 
+        .. versionchanged:: 3.4
+        Made optional instead of an abstractmethod.
+
 
 .. class:: ResourceLoader
 
@@ -281,6 +303,9 @@ ABC hierarchy::
         be found. The *path* is expected to be constructed using a module's
         :attr:`__file__` attribute or an item from a package's :attr:`__path__`.
 
+        .. versionchanged:: 3.4
+           Raises :exc:`IOError` instead of :exc:`NotImplementedError`.
+
 
 .. class:: InspectLoader
 
@@ -297,6 +322,9 @@ ABC hierarchy::
         .. index::
            single: universal newlines; importlib.abc.InspectLoader.get_source method
 
+        .. versionchanged:: 3.4
+           Raises :exc:`ImportError` instead of :exc:`NotImplementedError`.
+
     .. method:: get_source(fullname)
 
         An abstract method to return the source of a module. It is returned as
@@ -305,12 +333,18 @@ ABC hierarchy::
         if no source is available (e.g. a built-in module). Raises
         :exc:`ImportError` if the loader cannot find the module specified.
 
+        .. versionchanged:: 3.4
+           Raises :exc:`ImportError` instead of :exc:`NotImplementedError`.
+
     .. method:: is_package(fullname)
 
         An abstract method to return a true value if the module is a package, a
         false value otherwise. :exc:`ImportError` is raised if the
         :term:`loader` cannot find the module.
 
+        .. versionchanged:: 3.4
+           Raises :exc:`ImportError` instead of :exc:`NotImplementedError`.
+
 
 .. class:: ExecutionLoader
 
@@ -328,6 +362,9 @@ ABC hierarchy::
         the source file, regardless of whether a bytecode was used to load the
         module.
 
+        .. versionchanged:: 3.4
+           Raises :exc:`ImportError` instead of :exc:`NotImplementedError`.
+
 
 .. class:: FileLoader(fullname, path)
 
@@ -392,10 +429,13 @@ ABC hierarchy::
         - ``'size'`` (optional): the size in bytes of the source code.
 
         Any other keys in the dictionary are ignored, to allow for future
-        extensions.
+        extensions. If the path cannot be handled, :exc:`IOError` is raised.
 
         .. versionadded:: 3.3
 
+        .. versionchanged:: 3.4
+           Raise :exc:`IOError` instead of :exc:`NotImplementedError`.
+
     .. method:: path_mtime(path)
 
         Optional abstract method which returns the modification time for the
@@ -404,7 +444,10 @@ ABC hierarchy::
         .. deprecated:: 3.3
            This method is deprecated in favour of :meth:`path_stats`.  You don't
            have to implement it, but it is still available for compatibility
-           purposes.
+           purposes. Raise :exc:`IOError` if the path cannot be handled.
+
+          .. versionchanged:: 3.4
+             Raise :exc:`IOError` instead of :exc:`NotImplementedError`.
 
     .. method:: set_data(path, data)
 
@@ -415,6 +458,9 @@ ABC hierarchy::
         When writing to the path fails because the path is read-only
         (:attr:`errno.EACCES`), do not propagate the exception.
 
+        .. versionchanged:: 3.4
+           No longer raises :exc:`NotImplementedError` when called.
+
     .. method:: source_to_code(data, path)
 
         Create a code object from Python source.

Lib/importlib/_bootstrap.py

@@ -893,8 +893,10 @@ class SourceLoader(_LoaderBasics):
     def path_mtime(self, path):
         """Optional method that returns the modification time (an int) for the
         specified path, where path is a str.
+
+        Raises IOError when the path cannot be handled.
         """
-        raise NotImplementedError
+        raise IOError
 
     def path_stats(self, path):
         """Optional method returning a metadata dict for the specified path
@@ -905,6 +907,7 @@ class SourceLoader(_LoaderBasics):
         - 'size' (optional) is the size in bytes of the source code.
 
         Implementing this method allows the loader to read bytecode files.
+        Raises IOError when the path cannot be handled.
         """
         return {'mtime': self.path_mtime(path)}
 
@@ -922,9 +925,7 @@ class SourceLoader(_LoaderBasics):
         """Optional method which writes data (bytes) to a file path (a str).
 
         Implementing this method allows for the writing of bytecode files.
-
         """
-        raise NotImplementedError
 
 
     def get_source(self, fullname):
@@ -973,7 +974,7 @@ class SourceLoader(_LoaderBasics):
         else:
             try:
                 st = self.path_stats(source_path)
-            except NotImplementedError:
+            except IOError:
                 pass
             else:
                 source_mtime = int(st['mtime'])

Lib/importlib/abc.py

@@ -37,9 +37,8 @@ class Finder(metaclass=abc.ABCMeta):
     def find_module(self, fullname, path=None):
         """An abstract method that should find a module.
         The fullname is a str and the optional path is a str or None.
-        Returns a Loader object.
+        Returns a Loader object or None.
         """
-        raise NotImplementedError
 
 
 class MetaPathFinder(Finder):
@@ -49,16 +48,14 @@ class MetaPathFinder(Finder):
     @abc.abstractmethod
     def find_module(self, fullname, path):
         """Abstract method which, when implemented, should find a module.
-        The fullname is a str and the path is a str or None.
+        The fullname is a str and the path is a list of strings or None.
-        Returns a Loader object.
+        Returns a Loader object or None.
         """
-        raise NotImplementedError
 
     def invalidate_caches(self):
         """An optional method for clearing the finder's cache, if any.
         This method is used by importlib.invalidate_caches().
         """
-        return NotImplemented
 
 _register(MetaPathFinder, machinery.BuiltinImporter, machinery.FrozenImporter,
           machinery.PathFinder, machinery.WindowsRegistryFinder)
@@ -70,13 +67,14 @@ class PathEntryFinder(Finder):
 
     @abc.abstractmethod
     def find_loader(self, fullname):
-        """Abstract method which, when implemented, returns a module loader.
+        """Abstract method which, when implemented, returns a module loader or
+        a possible part of a namespace.
         The fullname is a str.  Returns a 2-tuple of (Loader, portion) where
         portion is a sequence of file system locations contributing to part of
         a namespace package.  The sequence may be empty and the loader may be
         None.
         """
-        raise NotImplementedError
+        return None, []
 
     find_module = _bootstrap._find_module_shim
 
@@ -84,25 +82,34 @@ class PathEntryFinder(Finder):
         """An optional method for clearing the finder's cache, if any.
         This method is used by PathFinder.invalidate_caches().
         """
-        return NotImplemented
 
 _register(PathEntryFinder, machinery.FileFinder)
 
 
 class Loader(metaclass=abc.ABCMeta):
 
-    """Abstract base class for import loaders."""
+    """Abstract base class for import loaders.
+
+    The optional method module_repr(module) may be defined to provide a
+    repr for a module when appropriate (see PEP 420). The __repr__() method on
+    the module type will use the method as appropriate.
+
+    """
 
     @abc.abstractmethod
     def load_module(self, fullname):
         """Abstract method which when implemented should load a module.
-        The fullname is a str."""
+        The fullname is a str.
-        raise NotImplementedError
+
+        ImportError is raised on failure.
+        """
+        raise ImportError
 
-    @abc.abstractmethod
     def module_repr(self, module):
-        """Abstract method which when implemented calculates and returns the
+        """Return a module's repr.
-        given module's repr."""
+
+        Used by the module type when implemented without raising an exception.
+        """
         raise NotImplementedError
 
 
@@ -119,7 +126,7 @@ class ResourceLoader(Loader):
     def get_data(self, path):
         """Abstract method which when implemented should return the bytes for
         the specified path.  The path must be a str."""
-        raise NotImplementedError
+        raise IOError
 
 
 class InspectLoader(Loader):
@@ -134,20 +141,29 @@ class InspectLoader(Loader):
     @abc.abstractmethod
     def is_package(self, fullname):
         """Abstract method which when implemented should return whether the
-        module is a package.  The fullname is a str.  Returns a bool."""
+        module is a package.  The fullname is a str.  Returns a bool.
-        raise NotImplementedError
+
+        Raises ImportError is the module cannot be found.
+        """
+        raise ImportError
 
     @abc.abstractmethod
     def get_code(self, fullname):
         """Abstract method which when implemented should return the code object
-        for the module.  The fullname is a str.  Returns a types.CodeType."""
+        for the module.  The fullname is a str.  Returns a types.CodeType.
-        raise NotImplementedError
+
+        Raises ImportError if the module cannot be found.
+        """
+        raise ImportError
 
     @abc.abstractmethod
     def get_source(self, fullname):
         """Abstract method which should return the source code for the
-        module.  The fullname is a str.  Returns a str."""
+        module.  The fullname is a str.  Returns a str.
-        raise NotImplementedError
+
+        Raises ImportError if the module cannot be found.
+        """
+        raise ImportError
 
 _register(InspectLoader, machinery.BuiltinImporter, machinery.FrozenImporter,
             machinery.ExtensionFileLoader)
@@ -165,8 +181,11 @@ class ExecutionLoader(InspectLoader):
     @abc.abstractmethod
     def get_filename(self, fullname):
         """Abstract method which should return the value that __file__ is to be
-        set to."""
+        set to.
-        raise NotImplementedError
+
+        Raises ImportError if the module cannot be found.
+        """
+        raise ImportError
 
 
 class FileLoader(_bootstrap.FileLoader, ResourceLoader, ExecutionLoader):
@@ -198,7 +217,7 @@ class SourceLoader(_bootstrap.SourceLoader, ResourceLoader, ExecutionLoader):
     def path_mtime(self, path):
         """Return the (int) modification time for the path (str)."""
         if self.path_stats.__func__ is SourceLoader.path_stats:
-            raise NotImplementedError
+            raise IOError
         return int(self.path_stats(path)['mtime'])
 
     def path_stats(self, path):
@@ -209,7 +228,7 @@ class SourceLoader(_bootstrap.SourceLoader, ResourceLoader, ExecutionLoader):
         - 'size' (optional) is the size in bytes of the source code.
         """
         if self.path_mtime.__func__ is SourceLoader.path_mtime:
-            raise NotImplementedError
+            raise IOError
         return {'mtime': self.path_mtime(path)}
 
     def set_data(self, path, data):
@@ -220,8 +239,6 @@ class SourceLoader(_bootstrap.SourceLoader, ResourceLoader, ExecutionLoader):
         Any needed intermediary directories are to be created. If for some
         reason the file cannot be written because of permissions, fail
         silently.
-
         """
-        raise NotImplementedError
 
 _register(SourceLoader, machinery.SourceFileLoader)

Lib/test/test_importlib/source/test_abc_loader.py

@@ -1,410 +0,0 @@
-import importlib
-from importlib import abc
-
-from .. import abc as testing_abc
-from .. import util
-from . import util as source_util
-
-import imp
-import inspect
-import io
-import marshal
-import os
-import sys
-import types
-import unittest
-import warnings
-
-
-class SourceOnlyLoaderMock(abc.SourceLoader):
-
-    # Globals that should be defined for all modules.
-    source = (b"_ = '::'.join([__name__, __file__, __cached__, __package__, "
-              b"repr(__loader__)])")
-
-    def __init__(self, path):
-        self.path = path
-
-    def get_data(self, path):
-        assert self.path == path
-        return self.source
-
-    def get_filename(self, fullname):
-        return self.path
-
-    def module_repr(self, module):
-        return '<module>'
-
-
-class SourceLoaderMock(SourceOnlyLoaderMock):
-
-    source_mtime = 1
-
-    def __init__(self, path, magic=imp.get_magic()):
-        super().__init__(path)
-        self.bytecode_path = imp.cache_from_source(self.path)
-        self.source_size = len(self.source)
-        data = bytearray(magic)
-        data.extend(importlib._w_long(self.source_mtime))
-        data.extend(importlib._w_long(self.source_size))
-        code_object = compile(self.source, self.path, 'exec',
-                                dont_inherit=True)
-        data.extend(marshal.dumps(code_object))
-        self.bytecode = bytes(data)
-        self.written = {}
-
-    def get_data(self, path):
-        if path == self.path:
-            return super().get_data(path)
-        elif path == self.bytecode_path:
-            return self.bytecode
-        else:
-            raise OSError
-
-    def path_stats(self, path):
-        assert path == self.path
-        return {'mtime': self.source_mtime, 'size': self.source_size}
-
-    def set_data(self, path, data):
-        self.written[path] = bytes(data)
-        return path == self.bytecode_path
-
-
-def raise_ImportError(*args, **kwargs):
-    raise ImportError
-
-
-class SourceLoaderTestHarness(unittest.TestCase):
-
-    def setUp(self, *, is_package=True, **kwargs):
-        self.package = 'pkg'
-        if is_package:
-            self.path = os.path.join(self.package, '__init__.py')
-            self.name = self.package
-        else:
-            module_name = 'mod'
-            self.path = os.path.join(self.package, '.'.join(['mod', 'py']))
-            self.name = '.'.join([self.package, module_name])
-        self.cached = imp.cache_from_source(self.path)
-        self.loader = self.loader_mock(self.path, **kwargs)
-
-    def verify_module(self, module):
-        self.assertEqual(module.__name__, self.name)
-        self.assertEqual(module.__file__, self.path)
-        self.assertEqual(module.__cached__, self.cached)
-        self.assertEqual(module.__package__, self.package)
-        self.assertEqual(module.__loader__, self.loader)
-        values = module._.split('::')
-        self.assertEqual(values[0], self.name)
-        self.assertEqual(values[1], self.path)
-        self.assertEqual(values[2], self.cached)
-        self.assertEqual(values[3], self.package)
-        self.assertEqual(values[4], repr(self.loader))
-
-    def verify_code(self, code_object):
-        module = imp.new_module(self.name)
-        module.__file__ = self.path
-        module.__cached__ = self.cached
-        module.__package__ = self.package
-        module.__loader__ = self.loader
-        module.__path__ = []
-        exec(code_object, module.__dict__)
-        self.verify_module(module)
-
-
-class SourceOnlyLoaderTests(SourceLoaderTestHarness):
-
-    """Test importlib.abc.SourceLoader for source-only loading.
-
-    Reload testing is subsumed by the tests for
-    importlib.util.module_for_loader.
-
-    """
-
-    loader_mock = SourceOnlyLoaderMock
-
-    def test_get_source(self):
-        # Verify the source code is returned as a string.
-        # If an OSError is raised by get_data then raise ImportError.
-        expected_source = self.loader.source.decode('utf-8')
-        self.assertEqual(self.loader.get_source(self.name), expected_source)
-        def raise_OSError(path):
-            raise OSError
-        self.loader.get_data = raise_OSError
-        with self.assertRaises(ImportError) as cm:
-            self.loader.get_source(self.name)
-        self.assertEqual(cm.exception.name, self.name)
-
-    def test_is_package(self):
-        # Properly detect when loading a package.
-        self.setUp(is_package=False)
-        self.assertFalse(self.loader.is_package(self.name))
-        self.setUp(is_package=True)
-        self.assertTrue(self.loader.is_package(self.name))
-        self.assertFalse(self.loader.is_package(self.name + '.__init__'))
-
-    def test_get_code(self):
-        # Verify the code object is created.
-        code_object = self.loader.get_code(self.name)
-        self.verify_code(code_object)
-
-    def test_source_to_code(self):
-        # Verify the compiled code object.
-        code = self.loader.source_to_code(self.loader.source, self.path)
-        self.verify_code(code)
-
-    def test_load_module(self):
-        # Loading a module should set __name__, __loader__, __package__,
-        # __path__ (for packages), __file__, and __cached__.
-        # The module should also be put into sys.modules.
-        with util.uncache(self.name):
-            module = self.loader.load_module(self.name)
-            self.verify_module(module)
-            self.assertEqual(module.__path__, [os.path.dirname(self.path)])
-            self.assertIn(self.name, sys.modules)
-
-    def test_package_settings(self):
-        # __package__ needs to be set, while __path__ is set on if the module
-        # is a package.
-        # Testing the values for a package are covered by test_load_module.
-        self.setUp(is_package=False)
-        with util.uncache(self.name):
-            module = self.loader.load_module(self.name)
-            self.verify_module(module)
-            self.assertTrue(not hasattr(module, '__path__'))
-
-    def test_get_source_encoding(self):
-        # Source is considered encoded in UTF-8 by default unless otherwise
-        # specified by an encoding line.
-        source = "_ = 'ü'"
-        self.loader.source = source.encode('utf-8')
-        returned_source = self.loader.get_source(self.name)
-        self.assertEqual(returned_source, source)
-        source = "# coding: latin-1\n_ = ü"
-        self.loader.source = source.encode('latin-1')
-        returned_source = self.loader.get_source(self.name)
-        self.assertEqual(returned_source, source)
-
-
-@unittest.skipIf(sys.dont_write_bytecode, "sys.dont_write_bytecode is true")
-class SourceLoaderBytecodeTests(SourceLoaderTestHarness):
-
-    """Test importlib.abc.SourceLoader's use of bytecode.
-
-    Source-only testing handled by SourceOnlyLoaderTests.
-
-    """
-
-    loader_mock = SourceLoaderMock
-
-    def verify_code(self, code_object, *, bytecode_written=False):
-        super().verify_code(code_object)
-        if bytecode_written:
-            self.assertIn(self.cached, self.loader.written)
-            data = bytearray(imp.get_magic())
-            data.extend(importlib._w_long(self.loader.source_mtime))
-            data.extend(importlib._w_long(self.loader.source_size))
-            data.extend(marshal.dumps(code_object))
-            self.assertEqual(self.loader.written[self.cached], bytes(data))
-
-    def test_code_with_everything(self):
-        # When everything should work.
-        code_object = self.loader.get_code(self.name)
-        self.verify_code(code_object)
-
-    def test_no_bytecode(self):
-        # If no bytecode exists then move on to the source.
-        self.loader.bytecode_path = "<does not exist>"
-        # Sanity check
-        with self.assertRaises(OSError):
-            bytecode_path = imp.cache_from_source(self.path)
-            self.loader.get_data(bytecode_path)
-        code_object = self.loader.get_code(self.name)
-        self.verify_code(code_object, bytecode_written=True)
-
-    def test_code_bad_timestamp(self):
-        # Bytecode is only used when the timestamp matches the source EXACTLY.
-        for source_mtime in (0, 2):
-            assert source_mtime != self.loader.source_mtime
-            original = self.loader.source_mtime
-            self.loader.source_mtime = source_mtime
-            # If bytecode is used then EOFError would be raised by marshal.
-            self.loader.bytecode = self.loader.bytecode[8:]
-            code_object = self.loader.get_code(self.name)
-            self.verify_code(code_object, bytecode_written=True)
-            self.loader.source_mtime = original
-
-    def test_code_bad_magic(self):
-        # Skip over bytecode with a bad magic number.
-        self.setUp(magic=b'0000')
-        # If bytecode is used then EOFError would be raised by marshal.
-        self.loader.bytecode = self.loader.bytecode[8:]
-        code_object = self.loader.get_code(self.name)
-        self.verify_code(code_object, bytecode_written=True)
-
-    def test_dont_write_bytecode(self):
-        # Bytecode is not written if sys.dont_write_bytecode is true.
-        # Can assume it is false already thanks to the skipIf class decorator.
-        try:
-            sys.dont_write_bytecode = True
-            self.loader.bytecode_path = "<does not exist>"
-            code_object = self.loader.get_code(self.name)
-            self.assertNotIn(self.cached, self.loader.written)
-        finally:
-            sys.dont_write_bytecode = False
-
-    def test_no_set_data(self):
-        # If set_data is not defined, one can still read bytecode.
-        self.setUp(magic=b'0000')
-        original_set_data = self.loader.__class__.set_data
-        try:
-            del self.loader.__class__.set_data
-            code_object = self.loader.get_code(self.name)
-            self.verify_code(code_object)
-        finally:
-            self.loader.__class__.set_data = original_set_data
-
-    def test_set_data_raises_exceptions(self):
-        # Raising NotImplementedError or OSError is okay for set_data.
-        def raise_exception(exc):
-            def closure(*args, **kwargs):
-                raise exc
-            return closure
-
-        self.setUp(magic=b'0000')
-        self.loader.set_data = raise_exception(NotImplementedError)
-        code_object = self.loader.get_code(self.name)
-        self.verify_code(code_object)
-
-
-class SourceLoaderGetSourceTests(unittest.TestCase):
-
-    """Tests for importlib.abc.SourceLoader.get_source()."""
-
-    def test_default_encoding(self):
-        # Should have no problems with UTF-8 text.
-        name = 'mod'
-        mock = SourceOnlyLoaderMock('mod.file')
-        source = 'x = "ü"'
-        mock.source = source.encode('utf-8')
-        returned_source = mock.get_source(name)
-        self.assertEqual(returned_source, source)
-
-    def test_decoded_source(self):
-        # Decoding should work.
-        name = 'mod'
-        mock = SourceOnlyLoaderMock("mod.file")
-        source = "# coding: Latin-1\nx='ü'"
-        assert source.encode('latin-1') != source.encode('utf-8')
-        mock.source = source.encode('latin-1')
-        returned_source = mock.get_source(name)
-        self.assertEqual(returned_source, source)
-
-    def test_universal_newlines(self):
-        # PEP 302 says universal newlines should be used.
-        name = 'mod'
-        mock = SourceOnlyLoaderMock('mod.file')
-        source = "x = 42\r\ny = -13\r\n"
-        mock.source = source.encode('utf-8')
-        expect = io.IncrementalNewlineDecoder(None, True).decode(source)
-        self.assertEqual(mock.get_source(name), expect)
-
-
-class AbstractMethodImplTests(unittest.TestCase):
-
-    """Test the concrete abstractmethod implementations."""
-
-    class MetaPathFinder(abc.MetaPathFinder):
-        def find_module(self, fullname, path):
-            super().find_module(fullname, path)
-
-    class PathEntryFinder(abc.PathEntryFinder):
-        def find_module(self, _):
-            super().find_module(_)
-
-        def find_loader(self, _):
-            super().find_loader(_)
-
-    class Finder(abc.Finder):
-        def find_module(self, fullname, path):
-            super().find_module(fullname, path)
-
-    class Loader(abc.Loader):
-        def load_module(self, fullname):
-            super().load_module(fullname)
-
-        def module_repr(self, module):
-            super().module_repr(module)
-
-    class ResourceLoader(Loader, abc.ResourceLoader):
-        def get_data(self, _):
-            super().get_data(_)
-
-    class InspectLoader(Loader, abc.InspectLoader):
-        def is_package(self, _):
-            super().is_package(_)
-
-        def get_code(self, _):
-            super().get_code(_)
-
-        def get_source(self, _):
-            super().get_source(_)
-
-    class ExecutionLoader(InspectLoader, abc.ExecutionLoader):
-        def get_filename(self, _):
-            super().get_filename(_)
-
-    class SourceLoader(ResourceLoader, ExecutionLoader, abc.SourceLoader):
-        pass
-
-    def raises_NotImplementedError(self, ins, *args):
-        for method_name in args:
-            method = getattr(ins, method_name)
-            arg_count = len(inspect.getfullargspec(method)[0]) - 1
-            args = [''] * arg_count
-            try:
-                method(*args)
-            except NotImplementedError:
-                pass
-            else:
-                msg = "{}.{} did not raise NotImplementedError"
-                self.fail(msg.format(ins.__class__.__name__, method_name))
-
-    def test_Loader(self):
-        self.raises_NotImplementedError(self.Loader(), 'load_module')
-
-    # XXX misplaced; should be somewhere else
-    def test_Finder(self):
-        self.raises_NotImplementedError(self.Finder(), 'find_module')
-
-    def test_ResourceLoader(self):
-        self.raises_NotImplementedError(self.ResourceLoader(), 'load_module',
-                                        'get_data')
-
-    def test_InspectLoader(self):
-        self.raises_NotImplementedError(self.InspectLoader(), 'load_module',
-                                        'is_package', 'get_code', 'get_source')
-
-    def test_ExecutionLoader(self):
-        self.raises_NotImplementedError(self.ExecutionLoader(), 'load_module',
-                                        'is_package', 'get_code', 'get_source',
-                                        'get_filename')
-
-    def test_SourceLoader(self):
-        ins = self.SourceLoader()
-        # Required abstractmethods.
-        self.raises_NotImplementedError(ins, 'get_filename', 'get_data')
-        # Optional abstractmethods.
-        self.raises_NotImplementedError(ins, 'path_stats', 'set_data')
-
-
-def test_main():
-    from test.support import run_unittest
-    run_unittest(SourceOnlyLoaderTests,
-                 SourceLoaderBytecodeTests,
-                 SourceLoaderGetSourceTests,
-                 AbstractMethodImplTests)
-
-
-if __name__ == '__main__':
-    test_main()

Lib/test/test_importlib/test_abc.py

@@ -1,9 +1,18 @@
+import importlib
 from importlib import abc
 from importlib import machinery
+
+import imp
 import inspect
+import io
+import marshal
+import os
+import sys
 import unittest
 
+from . import util
 
+##### Inheritance
 class InheritanceTests:
 
     """Test that the specified class is a subclass/superclass of the expected
@@ -72,16 +81,422 @@ class SourceLoader(InheritanceTests, unittest.TestCase):
     subclasses = [machinery.SourceFileLoader]
 
 
-def test_main():
+##### Default semantics
-    from test.support import run_unittest
+class MetaPathFinderSubclass(abc.MetaPathFinder):
-    classes = []
+
-    for class_ in globals().values():
+    def find_module(self, fullname, path):
-        if (inspect.isclass(class_) and
+        return super().find_module(fullname, path)
-                issubclass(class_, unittest.TestCase) and
+
-                issubclass(class_, InheritanceTests)):
+
-            classes.append(class_)
+class MetaPathFinderDefaultsTests(unittest.TestCase):
-    run_unittest(*classes)
+
+    ins = MetaPathFinderSubclass()
+
+    def test_find_module(self):
+        # Default should return None.
+        self.assertIsNone(self.ins.find_module('something', None))
+
+    def test_invalidate_caches(self):
+        # Calling the method is a no-op.
+        self.ins.invalidate_caches()
+
+
+class PathEntryFinderSubclass(abc.PathEntryFinder):
+
+    def find_loader(self, fullname):
+        return super().find_loader(fullname)
+
+
+class PathEntryFinderDefaultsTests(unittest.TestCase):
+
+    ins = PathEntryFinderSubclass()
+
+    def test_find_loader(self):
+        self.assertEqual((None, []), self.ins.find_loader('something'))
+
+    def find_module(self):
+        self.assertEqual(None, self.ins.find_module('something'))
+
+    def test_invalidate_caches(self):
+        # Should be a no-op.
+        self.ins.invalidate_caches()
+
+
+class LoaderSubclass(abc.Loader):
+
+    def load_module(self, fullname):
+        return super().load_module(fullname)
+
+
+class LoaderDefaultsTests(unittest.TestCase):
+
+    ins = LoaderSubclass()
+
+    def test_load_module(self):
+        with self.assertRaises(ImportError):
+            self.ins.load_module('something')
+
+    def test_module_repr(self):
+        mod = imp.new_module('blah')
+        with self.assertRaises(NotImplementedError):
+            self.ins.module_repr(mod)
+        original_repr = repr(mod)
+        mod.__loader__ = self.ins
+        # Should still return a proper repr.
+        self.assertTrue(repr(mod))
+
+
+class ResourceLoaderSubclass(LoaderSubclass, abc.ResourceLoader):
+
+    def get_data(self, path):
+        return super().get_data(path)
+
+
+class ResourceLoaderDefaultsTests(unittest.TestCase):
+
+    ins = ResourceLoaderSubclass()
+
+    def test_get_data(self):
+        with self.assertRaises(IOError):
+            self.ins.get_data('/some/path')
+
+
+class InspectLoaderSubclass(LoaderSubclass, abc.InspectLoader):
+
+    def is_package(self, fullname):
+        return super().is_package(fullname)
+
+    def get_code(self, fullname):
+        return super().get_code(fullname)
+
+    def get_source(self, fullname):
+        return super().get_source(fullname)
+
+
+class InspectLoaderDefaultsTests(unittest.TestCase):
+
+    ins = InspectLoaderSubclass()
+
+    def test_is_package(self):
+        with self.assertRaises(ImportError):
+            self.ins.is_package('blah')
+
+    def test_get_code(self):
+        with self.assertRaises(ImportError):
+            self.ins.get_code('blah')
+
+    def test_get_source(self):
+        with self.assertRaises(ImportError):
+            self.ins.get_source('blah')
+
+
+class ExecutionLoaderSubclass(InspectLoaderSubclass, abc.ExecutionLoader):
+
+    def get_filename(self, fullname):
+        return super().get_filename(fullname)
+
+
+class ExecutionLoaderDefaultsTests(unittest.TestCase):
+
+    ins = ExecutionLoaderSubclass()
+
+    def test_get_filename(self):
+        with self.assertRaises(ImportError):
+            self.ins.get_filename('blah')
+
+
+##### SourceLoader
+class SourceOnlyLoaderMock(abc.SourceLoader):
+
+    # Globals that should be defined for all modules.
+    source = (b"_ = '::'.join([__name__, __file__, __cached__, __package__, "
+              b"repr(__loader__)])")
+
+    def __init__(self, path):
+        self.path = path
+
+    def get_data(self, path):
+        if path != self.path:
+            raise IOError
+        return self.source
+
+    def get_filename(self, fullname):
+        return self.path
+
+    def module_repr(self, module):
+        return '<module>'
+
+
+class SourceLoaderMock(SourceOnlyLoaderMock):
+
+    source_mtime = 1
+
+    def __init__(self, path, magic=imp.get_magic()):
+        super().__init__(path)
+        self.bytecode_path = imp.cache_from_source(self.path)
+        self.source_size = len(self.source)
+        data = bytearray(magic)
+        data.extend(importlib._w_long(self.source_mtime))
+        data.extend(importlib._w_long(self.source_size))
+        code_object = compile(self.source, self.path, 'exec',
+                                dont_inherit=True)
+        data.extend(marshal.dumps(code_object))
+        self.bytecode = bytes(data)
+        self.written = {}
+
+    def get_data(self, path):
+        if path == self.path:
+            return super().get_data(path)
+        elif path == self.bytecode_path:
+            return self.bytecode
+        else:
+            raise OSError
+
+    def path_stats(self, path):
+        if path != self.path:
+            raise IOError
+        return {'mtime': self.source_mtime, 'size': self.source_size}
+
+    def set_data(self, path, data):
+        self.written[path] = bytes(data)
+        return path == self.bytecode_path
+
+
+class SourceLoaderTestHarness(unittest.TestCase):
+
+    def setUp(self, *, is_package=True, **kwargs):
+        self.package = 'pkg'
+        if is_package:
+            self.path = os.path.join(self.package, '__init__.py')
+            self.name = self.package
+        else:
+            module_name = 'mod'
+            self.path = os.path.join(self.package, '.'.join(['mod', 'py']))
+            self.name = '.'.join([self.package, module_name])
+        self.cached = imp.cache_from_source(self.path)
+        self.loader = self.loader_mock(self.path, **kwargs)
+
+    def verify_module(self, module):
+        self.assertEqual(module.__name__, self.name)
+        self.assertEqual(module.__file__, self.path)
+        self.assertEqual(module.__cached__, self.cached)
+        self.assertEqual(module.__package__, self.package)
+        self.assertEqual(module.__loader__, self.loader)
+        values = module._.split('::')
+        self.assertEqual(values[0], self.name)
+        self.assertEqual(values[1], self.path)
+        self.assertEqual(values[2], self.cached)
+        self.assertEqual(values[3], self.package)
+        self.assertEqual(values[4], repr(self.loader))
+
+    def verify_code(self, code_object):
+        module = imp.new_module(self.name)
+        module.__file__ = self.path
+        module.__cached__ = self.cached
+        module.__package__ = self.package
+        module.__loader__ = self.loader
+        module.__path__ = []
+        exec(code_object, module.__dict__)
+        self.verify_module(module)
+
+
+class SourceOnlyLoaderTests(SourceLoaderTestHarness):
+
+    """Test importlib.abc.SourceLoader for source-only loading.
+
+    Reload testing is subsumed by the tests for
+    importlib.util.module_for_loader.
+
+    """
+
+    loader_mock = SourceOnlyLoaderMock
+
+    def test_get_source(self):
+        # Verify the source code is returned as a string.
+        # If an OSError is raised by get_data then raise ImportError.
+        expected_source = self.loader.source.decode('utf-8')
+        self.assertEqual(self.loader.get_source(self.name), expected_source)
+        def raise_OSError(path):
+            raise OSError
+        self.loader.get_data = raise_OSError
+        with self.assertRaises(ImportError) as cm:
+            self.loader.get_source(self.name)
+        self.assertEqual(cm.exception.name, self.name)
+
+    def test_is_package(self):
+        # Properly detect when loading a package.
+        self.setUp(is_package=False)
+        self.assertFalse(self.loader.is_package(self.name))
+        self.setUp(is_package=True)
+        self.assertTrue(self.loader.is_package(self.name))
+        self.assertFalse(self.loader.is_package(self.name + '.__init__'))
+
+    def test_get_code(self):
+        # Verify the code object is created.
+        code_object = self.loader.get_code(self.name)
+        self.verify_code(code_object)
+
+    def test_source_to_code(self):
+        # Verify the compiled code object.
+        code = self.loader.source_to_code(self.loader.source, self.path)
+        self.verify_code(code)
+
+    def test_load_module(self):
+        # Loading a module should set __name__, __loader__, __package__,
+        # __path__ (for packages), __file__, and __cached__.
+        # The module should also be put into sys.modules.
+        with util.uncache(self.name):
+            module = self.loader.load_module(self.name)
+            self.verify_module(module)
+            self.assertEqual(module.__path__, [os.path.dirname(self.path)])
+            self.assertIn(self.name, sys.modules)
+
+    def test_package_settings(self):
+        # __package__ needs to be set, while __path__ is set on if the module
+        # is a package.
+        # Testing the values for a package are covered by test_load_module.
+        self.setUp(is_package=False)
+        with util.uncache(self.name):
+            module = self.loader.load_module(self.name)
+            self.verify_module(module)
+            self.assertTrue(not hasattr(module, '__path__'))
+
+    def test_get_source_encoding(self):
+        # Source is considered encoded in UTF-8 by default unless otherwise
+        # specified by an encoding line.
+        source = "_ = 'ü'"
+        self.loader.source = source.encode('utf-8')
+        returned_source = self.loader.get_source(self.name)
+        self.assertEqual(returned_source, source)
+        source = "# coding: latin-1\n_ = ü"
+        self.loader.source = source.encode('latin-1')
+        returned_source = self.loader.get_source(self.name)
+        self.assertEqual(returned_source, source)
+
+
+@unittest.skipIf(sys.dont_write_bytecode, "sys.dont_write_bytecode is true")
+class SourceLoaderBytecodeTests(SourceLoaderTestHarness):
+
+    """Test importlib.abc.SourceLoader's use of bytecode.
+
+    Source-only testing handled by SourceOnlyLoaderTests.
+
+    """
+
+    loader_mock = SourceLoaderMock
+
+    def verify_code(self, code_object, *, bytecode_written=False):
+        super().verify_code(code_object)
+        if bytecode_written:
+            self.assertIn(self.cached, self.loader.written)
+            data = bytearray(imp.get_magic())
+            data.extend(importlib._w_long(self.loader.source_mtime))
+            data.extend(importlib._w_long(self.loader.source_size))
+            data.extend(marshal.dumps(code_object))
+            self.assertEqual(self.loader.written[self.cached], bytes(data))
+
+    def test_code_with_everything(self):
+        # When everything should work.
+        code_object = self.loader.get_code(self.name)
+        self.verify_code(code_object)
+
+    def test_no_bytecode(self):
+        # If no bytecode exists then move on to the source.
+        self.loader.bytecode_path = "<does not exist>"
+        # Sanity check
+        with self.assertRaises(OSError):
+            bytecode_path = imp.cache_from_source(self.path)
+            self.loader.get_data(bytecode_path)
+        code_object = self.loader.get_code(self.name)
+        self.verify_code(code_object, bytecode_written=True)
+
+    def test_code_bad_timestamp(self):
+        # Bytecode is only used when the timestamp matches the source EXACTLY.
+        for source_mtime in (0, 2):
+            assert source_mtime != self.loader.source_mtime
+            original = self.loader.source_mtime
+            self.loader.source_mtime = source_mtime
+            # If bytecode is used then EOFError would be raised by marshal.
+            self.loader.bytecode = self.loader.bytecode[8:]
+            code_object = self.loader.get_code(self.name)
+            self.verify_code(code_object, bytecode_written=True)
+            self.loader.source_mtime = original
+
+    def test_code_bad_magic(self):
+        # Skip over bytecode with a bad magic number.
+        self.setUp(magic=b'0000')
+        # If bytecode is used then EOFError would be raised by marshal.
+        self.loader.bytecode = self.loader.bytecode[8:]
+        code_object = self.loader.get_code(self.name)
+        self.verify_code(code_object, bytecode_written=True)
+
+    def test_dont_write_bytecode(self):
+        # Bytecode is not written if sys.dont_write_bytecode is true.
+        # Can assume it is false already thanks to the skipIf class decorator.
+        try:
+            sys.dont_write_bytecode = True
+            self.loader.bytecode_path = "<does not exist>"
+            code_object = self.loader.get_code(self.name)
+            self.assertNotIn(self.cached, self.loader.written)
+        finally:
+            sys.dont_write_bytecode = False
+
+    def test_no_set_data(self):
+        # If set_data is not defined, one can still read bytecode.
+        self.setUp(magic=b'0000')
+        original_set_data = self.loader.__class__.set_data
+        try:
+            del self.loader.__class__.set_data
+            code_object = self.loader.get_code(self.name)
+            self.verify_code(code_object)
+        finally:
+            self.loader.__class__.set_data = original_set_data
+
+    def test_set_data_raises_exceptions(self):
+        # Raising NotImplementedError or OSError is okay for set_data.
+        def raise_exception(exc):
+            def closure(*args, **kwargs):
+                raise exc
+            return closure
+
+        self.setUp(magic=b'0000')
+        self.loader.set_data = raise_exception(NotImplementedError)
+        code_object = self.loader.get_code(self.name)
+        self.verify_code(code_object)
+
+
+class SourceLoaderGetSourceTests(unittest.TestCase):
+
+    """Tests for importlib.abc.SourceLoader.get_source()."""
+
+    def test_default_encoding(self):
+        # Should have no problems with UTF-8 text.
+        name = 'mod'
+        mock = SourceOnlyLoaderMock('mod.file')
+        source = 'x = "ü"'
+        mock.source = source.encode('utf-8')
+        returned_source = mock.get_source(name)
+        self.assertEqual(returned_source, source)
+
+    def test_decoded_source(self):
+        # Decoding should work.
+        name = 'mod'
+        mock = SourceOnlyLoaderMock("mod.file")
+        source = "# coding: Latin-1\nx='ü'"
+        assert source.encode('latin-1') != source.encode('utf-8')
+        mock.source = source.encode('latin-1')
+        returned_source = mock.get_source(name)
+        self.assertEqual(returned_source, source)
+
+    def test_universal_newlines(self):
+        # PEP 302 says universal newlines should be used.
+        name = 'mod'
+        mock = SourceOnlyLoaderMock('mod.file')
+        source = "x = 42\r\ny = -13\r\n"
+        mock.source = source.encode('utf-8')
+        expect = io.IncrementalNewlineDecoder(None, True).decode(source)
+        self.assertEqual(mock.get_source(name), expect)
 
 
 if __name__ == '__main__':
-    test_main()
+    unittest.main()

Misc/NEWS

@@ -30,6 +30,13 @@ Core and Builtins
 Library
 -------
 
+- Issue #17093: Make the ABCs in importlib.abc provide default values or raise
+  reasonable exceptions for their methods to make them more amenable to super()
+  calls.
+
+- Issue #17566: Make importlib.abc.Loader.module_repr() optional instead of an
+  abstractmethod and raising NotImplementedError so as to be ignored by default.
+
 - Issue #17678: Remove the use of deprecated method in http/cookiejar.py.
   Changing the usage of get_origin_req_host() to origin_req_host.