bpo-42137: have ModuleType.__repr__ prefer __spec__ over module_repr() (GH-24953)

This is to work towards the removal of the use of  module_repr() in Python 3.12 (documented as deprecated since 3.4).

Author: brettcannon

Doc/library/importlib.rst

@@ -208,7 +208,7 @@ Functions
    .. versionadded:: 3.4
    .. versionchanged:: 3.7
        :exc:`ModuleNotFoundError` is raised when the module being reloaded lacks
-       a :class:`ModuleSpec`.
+       a :class:`~importlib.machinery.ModuleSpec`.
 
 
 :mod:`importlib.abc` -- Abstract base classes related to import
@@ -1591,19 +1591,19 @@ an :term:`importer`.
 
 .. function:: spec_from_loader(name, loader, *, origin=None, is_package=None)
 
-   A factory function for creating a :class:`ModuleSpec` instance based
-   on a loader.  The parameters have the same meaning as they do for
-   ModuleSpec.  The function uses available :term:`loader` APIs, such as
+   A factory function for creating a :class:`~importlib.machinery.ModuleSpec`
+   instance based on a loader.  The parameters have the same meaning as they do
+   for ModuleSpec.  The function uses available :term:`loader` APIs, such as
    :meth:`InspectLoader.is_package`, to fill in any missing
    information on the spec.
 
    .. versionadded:: 3.4
 
 .. function:: spec_from_file_location(name, location, *, loader=None, submodule_search_locations=None)
 
-   A factory function for creating a :class:`ModuleSpec` instance based
-   on the path to a file.  Missing information will be filled in on the
-   spec by making use of loader APIs and by the implication that the
+   A factory function for creating a :class:`~importlib.machinery.ModuleSpec`
+   instance based on the path to a file.  Missing information will be filled in
+   on the spec by making use of loader APIs and by the implication that the
    module will be file-based.
 
    .. versionadded:: 3.4

Doc/whatsnew/3.10.rst

@@ -1004,6 +1004,12 @@ Deprecated
   :meth:`~importlib.abc.Loader.exec_module` is preferred.
   (Contributed by Brett Cannon in :issue:`26131`.)
 
+* The import system now uses the ``__spec__`` attribute on modules before
+  falling back on :meth:`~importlib.abc.Loader.module_repr` for a module's
+  ``__repr__()`` method. Removal of the use of ``module_repr()`` is scheduled
+  for Python 3.12.
+  (Contributed by Brett Cannon in :issue:`42137`.)
+
 * ``sqlite3.OptimizedUnicode`` has been undocumented and obsolete since Python
   3.3, when it was made an alias to :class:`str`.  It is now deprecated,
   scheduled for removal in Python 3.12.

Lib/importlib/_bootstrap.py

@@ -275,7 +275,7 @@ def _requires_frozen_wrapper(self, fullname):
 def _load_module_shim(self, fullname):
     """Load the specified module into sys.modules and return it.
 
-    This method is deprecated.  Use loader.exec_module instead.
+    This method is deprecated.  Use loader.exec_module() instead.
 
     """
     msg = ("the load_module() method is deprecated and slated for removal in "
@@ -292,24 +292,16 @@ def _load_module_shim(self, fullname):
 # Module specifications #######################################################
 
 def _module_repr(module):
-    # The implementation of ModuleType.__repr__().
+    """The implementation of ModuleType.__repr__()."""
     loader = getattr(module, '__loader__', None)
-    if hasattr(loader, 'module_repr'):
-        # As soon as BuiltinImporter, FrozenImporter, and NamespaceLoader
-        # drop their implementations for module_repr. we can add a
-        # deprecation warning here.
+    if spec := getattr(module, "__spec__", None):
+        return _module_repr_from_spec(spec)
+    elif hasattr(loader, 'module_repr'):
         try:
             return loader.module_repr(module)
         except Exception:
             pass
-    try:
-        spec = module.__spec__
-    except AttributeError:
-        pass
-    else:
-        if spec is not None:
-            return _module_repr_from_spec(spec)
-
+    # Fall through to a catch-all which always succeeds.
     # We could use module.__class__.__name__ instead of 'module' in the
     # various repr permutations.
     try:

Lib/test/test_importlib/frozen/test_loader.py

@@ -160,14 +160,6 @@ def test_module_repr(self):
             self.assertEqual(repr_str,
                              "<module '__hello__' (frozen)>")
 
-    def test_module_repr_indirect(self):
-        with warnings.catch_warnings():
-            warnings.simplefilter("ignore", DeprecationWarning)
-            with util.uncache('__hello__'), captured_stdout():
-                module = self.machinery.FrozenImporter.load_module('__hello__')
-            self.assertEqual(repr(module),
-                            "<module '__hello__' (frozen)>")
-
     # No way to trigger an error in a frozen module.
     test_state_after_failure = None
 

Lib/test/test_importlib/test_namespace_pkgs.py

@@ -82,7 +82,8 @@ def test_cant_import_other(self):
 
     def test_module_repr(self):
         import foo.one
-        self.assertEqual(repr(foo), "<module 'foo' (namespace)>")
+        self.assertEqual(foo.__spec__.loader.module_repr(foo),
+                         "<module 'foo' (namespace)>")
 
 
 class DynamicPathNamespacePackage(NamespacePackageTest):

Misc/NEWS.d/next/Core and Builtins/2021-03-20-19-54-47.bpo-42137.A8aQvj.rst

@@ -0,0 +1,2 @@
+The import system now prefers using ``__spec__`` for ``ModuleType.__repr__``
+over ``module_repr()``.