Issue #12599: Be more strict in accepting None vs. a false-like object

in importlib.

Thanks to PJE for pointing out the issue and Nick Coghlan for filing
the bug.

3 files changed, 27 insertions, 4 deletions

diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py
index 983abd5..415c488 100644
--- a/Lib/importlib/_bootstrap.py
+++ b/Lib/importlib/_bootstrap.py
@@ -217,7 +217,7 @@ def module_for_loader(fxn):
     """
     def module_for_loader_wrapper(self, fullname, *args, **kwargs):
         module = sys.modules.get(fullname)
-        is_reload = bool(module)
+        is_reload = module is not None
         if not is_reload:
             # This must be done before open() is called as the 'io' module
             # implicitly imports 'locale' and would otherwise trigger an
@@ -711,7 +711,7 @@ class PathFinder:
         If 'hooks' is false then use sys.path_hooks.
 
         """
-        if not hooks:
+        if hooks is None:
             hooks = sys.path_hooks
         for hook in hooks:
             try:
@@ -753,7 +753,7 @@ class PathFinder:
     def find_module(cls, fullname, path=None):
         """Find the module on sys.path or 'path' based on sys.path_hooks and
         sys.path_importer_cache."""
-        if not path:
+        if path is None:
             path = sys.path
         for entry in path:
             try:
diff --git a/Lib/importlib/test/import_/test_path.py b/Lib/importlib/test/import_/test_path.py
index a211bdf..fe47717 100644
--- a/Lib/importlib/test/import_/test_path.py
+++ b/Lib/importlib/test/import_/test_path.py
@@ -42,6 +42,15 @@ class FinderTests(unittest.TestCase):
             loader = machinery.PathFinder.find_module(module, [path])
             self.assertTrue(loader is importer)
 
+    def test_empty_list(self):
+        # An empty list should not count as asking for sys.path.
+        module = 'module'
+        path = '<test path>'
+        importer = util.mock_modules(module)
+        with util.import_state(path_importer_cache={path: importer},
+                               path=[path]):
+            self.assertIsNone(machinery.PathFinder.find_module('module', []))
+
     def test_path_hooks(self):
         # Test that sys.path_hooks is used.
         # Test that sys.path_importer_cache is set.
diff --git a/Lib/importlib/test/test_util.py b/Lib/importlib/test/test_util.py
index c7cdad1..b035d65 100644
--- a/Lib/importlib/test/test_util.py
+++ b/Lib/importlib/test/test_util.py
@@ -65,9 +65,23 @@ class ModuleForLoaderTests(unittest.TestCase):
         self.assertEqual(wrapped.__name__, fxn.__name__)
         self.assertEqual(wrapped.__qualname__, fxn.__qualname__)
 
-class SetPackageTests(unittest.TestCase):
+    def test_false_module(self):
+        # If for some odd reason a module is considered false, still return it
+        # from sys.modules.
+        class FalseModule(types.ModuleType):
+            def __bool__(self): return False
+
+        name = 'mod'
+        module = FalseModule(name)
+        with test_util.uncache(name):
+            self.assertFalse(module)
+            sys.modules[name] = module
+            given = self.return_module(name)
+            self.assertTrue(given is module)
 
 
+class SetPackageTests(unittest.TestCase):
+
     """Tests for importlib.util.set_package."""
 
     def verify(self, module, expect):