bpo-43672: raise ImportWarning when calling find_loader() (GH-25119)

Doc/reference/import.rst

@@ -904,7 +904,8 @@ a list containing the portion.
    ``find_loader()`` in preference to ``find_module()``.
 
 .. versionchanged:: 3.10
-    Calls to :meth:`~importlib.abc.PathEntryFinder.find_module` by the import
+    Calls to :meth:`~importlib.abc.PathEntryFinder.find_module` and
+    :meth:`~importlib.abc.PathEntryFinder.find_loader` by the import
     system will raise :exc:`ImportWarning`.
 
 

Doc/whatsnew/3.10.rst

@@ -1050,7 +1050,13 @@ Deprecated
   :meth:`importlib.abc.PathEntryFinder.find_spec`
   are preferred, respectively. You can use
   :func:`importlib.util.spec_from_loader` to help in porting.
-  (Contributed by Brett Cannon in  :issue:`42134`.)
+  (Contributed by Brett Cannon in :issue:`42134`.)
+
+* The use of :meth:`importlib.abc.PathEntryFinder.find_loader` by the import
+  system now triggers an :exc:`ImportWarning` as
+  :meth:`importlib.abc.PathEntryFinder.find_spec` is preferred. You can use
+  :func:`importlib.util.spec_from_loader` to help in porting.
+  (Contributed by Brett Cannon in :issue:`43672`.)
 
 * The import system now uses the ``__spec__`` attribute on modules before
   falling back on :meth:`~importlib.abc.Loader.module_repr` for a module's

Lib/importlib/_bootstrap_external.py

@@ -1323,10 +1323,13 @@ class PathFinder:
         # This would be a good place for a DeprecationWarning if
         # we ended up going that route.
         if hasattr(finder, 'find_loader'):
+            msg = (f"{_bootstrap._object_name(finder)}.find_spec() not found; "
+                    "falling back to find_loader()")
+            _warnings.warn(msg, ImportWarning)
             loader, portions = finder.find_loader(fullname)
         else:
             msg = (f"{_bootstrap._object_name(finder)}.find_spec() not found; "
-                           "falling back to find_module()")
+                    "falling back to find_module()")
             _warnings.warn(msg, ImportWarning)
             loader = finder.find_module(fullname)
             portions = []

Lib/test/test_importlib/extension/test_loader.py

@@ -89,8 +89,7 @@ class LoaderTests(abc.LoaderTests):
  ) = util.test_both(LoaderTests, machinery=machinery)
 
 class MultiPhaseExtensionModuleTests(abc.LoaderTests):
-    """Test loading extension modules with multi-phase initialization (PEP 489)
-    """
+    # Test loading extension modules with multi-phase initialization (PEP 489).
 
     def setUp(self):
         self.name = '_testmultiphase'
@@ -101,13 +100,13 @@ class MultiPhaseExtensionModuleTests(abc.LoaderTests):
             self.name, self.spec.origin)
 
     def load_module(self):
-        '''Load the module from the test extension'''
+        # Load the module from the test extension.
         with warnings.catch_warnings():
             warnings.simplefilter("ignore", DeprecationWarning)
             return self.loader.load_module(self.name)
 
     def load_module_by_name(self, fullname):
-        '''Load a module from the test extension by name'''
+        # Load a module from the test extension by name.
         origin = self.spec.origin
         loader = self.machinery.ExtensionFileLoader(fullname, origin)
         spec = importlib.util.spec_from_loader(fullname, loader)
@@ -125,7 +124,7 @@ class MultiPhaseExtensionModuleTests(abc.LoaderTests):
     test_state_after_failure = None
 
     def test_module(self):
-        '''Test loading an extension module'''
+        # Test loading an extension module.
         with util.uncache(self.name):
             module = self.load_module()
             for attr, value in [('__name__', self.name),
@@ -139,7 +138,7 @@ class MultiPhaseExtensionModuleTests(abc.LoaderTests):
                                   self.machinery.ExtensionFileLoader)
 
     def test_functionality(self):
-        '''Test basic functionality of stuff defined in an extension module'''
+        # Test basic functionality of stuff defined in an extension module.
         with util.uncache(self.name):
             module = self.load_module()
             self.assertIsInstance(module, types.ModuleType)
@@ -159,7 +158,7 @@ class MultiPhaseExtensionModuleTests(abc.LoaderTests):
             self.assertEqual(module.str_const, 'something different')
 
     def test_reload(self):
-        '''Test that reload didn't re-set the module's attributes'''
+        # Test that reload didn't re-set the module's attributes.
         with util.uncache(self.name):
             module = self.load_module()
             ex_class = module.Example
@@ -167,7 +166,7 @@ class MultiPhaseExtensionModuleTests(abc.LoaderTests):
             self.assertIs(ex_class, module.Example)
 
     def test_try_registration(self):
-        '''Assert that the PyState_{Find,Add,Remove}Module C API doesn't work'''
+        # Assert that the PyState_{Find,Add,Remove}Module C API doesn't work.
         module = self.load_module()
         with self.subTest('PyState_FindModule'):
             self.assertEqual(module.call_state_registration_func(0), None)
@@ -179,14 +178,14 @@ class MultiPhaseExtensionModuleTests(abc.LoaderTests):
                 module.call_state_registration_func(2)
 
     def test_load_submodule(self):
-        '''Test loading a simulated submodule'''
+        # Test loading a simulated submodule.
         module = self.load_module_by_name('pkg.' + self.name)
         self.assertIsInstance(module, types.ModuleType)
         self.assertEqual(module.__name__, 'pkg.' + self.name)
         self.assertEqual(module.str_const, 'something different')
 
     def test_load_short_name(self):
-        '''Test loading module with a one-character name'''
+        # Test loading module with a one-character name.
         module = self.load_module_by_name('x')
         self.assertIsInstance(module, types.ModuleType)
         self.assertEqual(module.__name__, 'x')
@@ -194,27 +193,27 @@ class MultiPhaseExtensionModuleTests(abc.LoaderTests):
         self.assertNotIn('x', sys.modules)
 
     def test_load_twice(self):
-        '''Test that 2 loads result in 2 module objects'''
+        # Test that 2 loads result in 2 module objects.
         module1 = self.load_module_by_name(self.name)
         module2 = self.load_module_by_name(self.name)
         self.assertIsNot(module1, module2)
 
     def test_unloadable(self):
-        '''Test nonexistent module'''
+        # Test nonexistent module.
         name = 'asdfjkl;'
         with self.assertRaises(ImportError) as cm:
             self.load_module_by_name(name)
         self.assertEqual(cm.exception.name, name)
 
     def test_unloadable_nonascii(self):
-        '''Test behavior with nonexistent module with non-ASCII name'''
+        # Test behavior with nonexistent module with non-ASCII name.
         name = 'fo\xf3'
         with self.assertRaises(ImportError) as cm:
             self.load_module_by_name(name)
         self.assertEqual(cm.exception.name, name)
 
     def test_nonmodule(self):
-        '''Test returning a non-module object from create works'''
+        # Test returning a non-module object from create works.
         name = self.name + '_nonmodule'
         mod = self.load_module_by_name(name)
         self.assertNotEqual(type(mod), type(unittest))
@@ -222,7 +221,7 @@ class MultiPhaseExtensionModuleTests(abc.LoaderTests):
 
     # issue 27782
     def test_nonmodule_with_methods(self):
-        '''Test creating a non-module object with methods defined'''
+        # Test creating a non-module object with methods defined.
         name = self.name + '_nonmodule_with_methods'
         mod = self.load_module_by_name(name)
         self.assertNotEqual(type(mod), type(unittest))
@@ -230,14 +229,14 @@ class MultiPhaseExtensionModuleTests(abc.LoaderTests):
         self.assertEqual(mod.bar(10, 1), 9)
 
     def test_null_slots(self):
-        '''Test that NULL slots aren't a problem'''
+        # Test that NULL slots aren't a problem.
         name = self.name + '_null_slots'
         module = self.load_module_by_name(name)
         self.assertIsInstance(module, types.ModuleType)
         self.assertEqual(module.__name__, name)
 
     def test_bad_modules(self):
-        '''Test SystemError is raised for misbehaving extensions'''
+        # Test SystemError is raised for misbehaving extensions.
         for name_base in [
                 'bad_slot_large',
                 'bad_slot_negative',
@@ -261,9 +260,9 @@ class MultiPhaseExtensionModuleTests(abc.LoaderTests):
                     self.load_module_by_name(name)
 
     def test_nonascii(self):
-        '''Test that modules with non-ASCII names can be loaded'''
+        # Test that modules with non-ASCII names can be loaded.
         # punycode behaves slightly differently in some-ASCII and no-ASCII
-        # cases, so test both
+        # cases, so test both.
         cases = [
             (self.name + '_zkou\u0161ka_na\u010dten\xed', 'Czech'),
             ('\uff3f\u30a4\u30f3\u30dd\u30fc\u30c8\u30c6\u30b9\u30c8',

Lib/test/test_importlib/import_/test_path.py

@@ -143,12 +143,16 @@ class FinderTests:
                 return self.loader, self.portions
         path = 'testing path'
         with util.import_state(path_importer_cache={path: TestFinder()}):
-            self.assertIsNone(
+            with warnings.catch_warnings():
+                warnings.simplefilter("ignore", ImportWarning)
+                self.assertIsNone(
                     self.machinery.PathFinder.find_spec('whatever', [path]))
         success_finder = TestFinder()
         success_finder.loader = __loader__
         with util.import_state(path_importer_cache={path: success_finder}):
-            spec = self.machinery.PathFinder.find_spec('whatever', [path])
+            with warnings.catch_warnings():
+                warnings.simplefilter("ignore", ImportWarning)
+                spec = self.machinery.PathFinder.find_spec('whatever', [path])
         self.assertEqual(spec.loader, __loader__)
 
     def test_finder_with_find_spec(self):

Lib/test/test_importlib/test_abc.py

@@ -221,13 +221,13 @@ class LoaderDefaultsTests(ABCTestHarness):
     def test_module_repr(self):
         mod = types.ModuleType('blah')
         with warnings.catch_warnings():
-            warnings.simplefilter("ignore")
+            warnings.simplefilter("ignore", DeprecationWarning)
             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))
+            original_repr = repr(mod)
+            mod.__loader__ = self.ins
+            # Should still return a proper repr.
+            self.assertTrue(repr(mod))
 
 
 (Frozen_LDefaultTests,

Lib/test/test_importlib/test_main.py

@@ -36,12 +36,10 @@ class BasicTests(fixtures.DistInfoPkg, unittest.TestCase):
             Distribution.from_name('does-not-exist')
 
     def test_package_not_found_mentions_metadata(self):
-        """
-        When a package is not found, that could indicate that the
-        packgae is not installed or that it is installed without
-        metadata. Ensure the exception mentions metadata to help
-        guide users toward the cause. See #124.
-        """
+        # When a package is not found, that could indicate that the
+        # packgae is not installed or that it is installed without
+        # metadata. Ensure the exception mentions metadata to help
+        # guide users toward the cause. See #124.
         with self.assertRaises(PackageNotFoundError) as ctx:
             Distribution.from_name('does-not-exist')
 
@@ -90,10 +88,8 @@ class NameNormalizationTests(fixtures.OnSysPath, fixtures.SiteDir, unittest.Test
         return 'my-pkg'
 
     def test_dashes_in_dist_name_found_as_underscores(self):
-        """
-        For a package with a dash in the name, the dist-info metadata
-        uses underscores in the name. Ensure the metadata loads.
-        """
+        # For a package with a dash in the name, the dist-info metadata
+        # uses underscores in the name. Ensure the metadata loads.
         pkg_name = self.pkg_with_dashes(self.site_dir)
         assert version(pkg_name) == '1.0'
 
@@ -111,9 +107,7 @@ class NameNormalizationTests(fixtures.OnSysPath, fixtures.SiteDir, unittest.Test
         return 'CherryPy'
 
     def test_dist_name_found_as_any_case(self):
-        """
-        Ensure the metadata loads when queried with any case.
-        """
+        # Ensure the metadata loads when queried with any case.
         pkg_name = self.pkg_with_mixed_case(self.site_dir)
         assert version(pkg_name) == '1.0'
         assert version(pkg_name.lower()) == '1.0'
@@ -241,13 +235,11 @@ class TestEntryPoints(unittest.TestCase):
         assert "'name'" in repr(self.ep)
 
     def test_hashable(self):
-        """EntryPoints should be hashable"""
+        # EntryPoints should be hashable.
         hash(self.ep)
 
     def test_json_dump(self):
-        """
-        json should not expect to be able to dump an EntryPoint
-        """
+        # json should not expect to be able to dump an EntryPoint.
         with self.assertRaises(Exception):
             with warnings.catch_warnings(record=True):
                 json.dumps(self.ep)
@@ -259,9 +251,7 @@ class TestEntryPoints(unittest.TestCase):
         assert self.ep.attr is None
 
     def test_sortable(self):
-        """
-        EntryPoint objects are sortable, but result is undefined.
-        """
+        # EntryPoint objects are sortable, but result is undefined.
         sorted(
             [
                 EntryPoint('b', 'val', 'group'),
@@ -274,10 +264,8 @@ class FileSystem(
     fixtures.OnSysPath, fixtures.SiteDir, fixtures.FileBuilder, unittest.TestCase
 ):
     def test_unicode_dir_on_sys_path(self):
-        """
-        Ensure a Unicode subdirectory of a directory on sys.path
-        does not crash.
-        """
+        # Ensure a Unicode subdirectory of a directory on sys.path
+        # does not crash.
         fixtures.build_files(
             {self.unicode_filename(): {}},
             prefix=self.site_dir,

Lib/test/test_importlib/test_metadata_api.py

@@ -81,10 +81,8 @@ class APITests(
             self.assertEqual(ep.dist.version, "1.0.0")
 
     def test_entry_points_unique_packages(self):
-        """
-        Entry points should only be exposed for the first package
-        on sys.path with a given name.
-        """
+        # Entry points should only be exposed for the first package
+        # on sys.path with a given name.
         alt_site_dir = self.fixtures.enter_context(fixtures.tempdir())
         self.fixtures.enter_context(self.add_sys_path(alt_site_dir))
         alt_pkg = {
@@ -116,11 +114,9 @@ class APITests(
         assert entry_points(group='missing') == ()
 
     def test_entry_points_dict_construction(self):
-        """
-        Prior versions of entry_points() returned simple lists and
-        allowed casting those lists into maps by name using ``dict()``.
-        Capture this now deprecated use-case.
-        """
+        # Prior versions of entry_points() returned simple lists and
+        # allowed casting those lists into maps by name using ``dict()``.
+        # Capture this now deprecated use-case.
         with warnings.catch_warnings(record=True) as caught:
             warnings.filterwarnings("default", category=DeprecationWarning)
             eps = dict(entry_points(group='entries'))
@@ -134,11 +130,9 @@ class APITests(
         assert "Construction of dict of EntryPoints is deprecated" in str(expected)
 
     def test_entry_points_groups_getitem(self):
-        """
-        Prior versions of entry_points() returned a dict. Ensure
-        that callers using '.__getitem__()' are supported but warned to
-        migrate.
-        """
+        # Prior versions of entry_points() returned a dict. Ensure
+        # that callers using '.__getitem__()' are supported but warned to
+        # migrate.
         with warnings.catch_warnings(record=True):
             entry_points()['entries'] == entry_points(group='entries')
 
@@ -146,11 +140,9 @@ class APITests(
                 entry_points()['missing']
 
     def test_entry_points_groups_get(self):
-        """
-        Prior versions of entry_points() returned a dict. Ensure
-        that callers using '.get()' are supported but warned to
-        migrate.
-        """
+        # Prior versions of entry_points() returned a dict. Ensure
+        # that callers using '.get()' are supported but warned to
+        # migrate.
         with warnings.catch_warnings(record=True):
             entry_points().get('missing', 'default') == 'default'
             entry_points().get('entries', 'default') == entry_points()['entries']
@@ -259,7 +251,7 @@ class OffSysPathTests(fixtures.DistInfoPkgOffPath, unittest.TestCase):
         assert any(dist.metadata['Name'] == 'distinfo-pkg' for dist in dists)
 
     def test_distribution_at_pathlib(self):
-        """Demonstrate how to load metadata direct from a directory."""
+        # Demonstrate how to load metadata direct from a directory.
         dist_info_path = self.site_dir / 'distinfo_pkg-1.0.0.dist-info'
         dist = Distribution.at(dist_info_path)
         assert dist.version == '1.0.0'

Lib/test/test_importlib/test_path.py

@@ -29,11 +29,9 @@ class PathDiskTests(PathTests, unittest.TestCase):
     data = data01
 
     def test_natural_path(self):
-        """
-        Guarantee the internal implementation detail that
-        file-system-backed resources do not get the tempdir
-        treatment.
-        """
+        # Guarantee the internal implementation detail that
+        # file-system-backed resources do not get the tempdir
+        # treatment.
         with resources.path(self.data, 'utf-8.file') as path:
             assert 'data' in str(path)
 

Lib/test/test_importlib/test_reader.py

@@ -60,7 +60,6 @@ class MultiplexedPathTest(unittest.TestCase):
             path.open()
 
     def test_join_path(self):
-        print('test_join_path')
         prefix = os.path.abspath(os.path.join(__file__, '..'))
         data01 = os.path.join(prefix, 'data01')
         path = MultiplexedPath(self.folder, data01)

Lib/test/test_importlib/test_util.py

@@ -845,22 +845,20 @@ class MagicNumberTests(unittest.TestCase):
         'only applies to candidate or final python release levels'
     )
     def test_magic_number(self):
-        """
-        Each python minor release should generally have a MAGIC_NUMBER
-        that does not change once the release reaches candidate status.
-
-        Once a release reaches candidate status, the value of the constant
-        EXPECTED_MAGIC_NUMBER in this test should be changed.
-        This test will then check that the actual MAGIC_NUMBER matches
-        the expected value for the release.
-
-        In exceptional cases, it may be required to change the MAGIC_NUMBER
-        for a maintenance release. In this case the change should be
-        discussed in python-dev. If a change is required, community
-        stakeholders such as OS package maintainers must be notified
-        in advance. Such exceptional releases will then require an
-        adjustment to this test case.
-        """
+        # Each python minor release should generally have a MAGIC_NUMBER
+        # that does not change once the release reaches candidate status.
+
+        # Once a release reaches candidate status, the value of the constant
+        # EXPECTED_MAGIC_NUMBER in this test should be changed.
+        # This test will then check that the actual MAGIC_NUMBER matches
+        # the expected value for the release.
+
+        # In exceptional cases, it may be required to change the MAGIC_NUMBER
+        # for a maintenance release. In this case the change should be
+        # discussed in python-dev. If a change is required, community
+        # stakeholders such as OS package maintainers must be notified
+        # in advance. Such exceptional releases will then require an
+        # adjustment to this test case.
         EXPECTED_MAGIC_NUMBER = 3413
         actual = int.from_bytes(importlib.util.MAGIC_NUMBER[:2], 'little')
 

Misc/NEWS.d/next/Core and Builtins/2021-03-31-09-12-54.bpo-43672.jTT5uG.rst

@@ -0,0 +1 @@
+Raise ImportWarning when calling find_loader().