From bed13fbef079e8d8e05b32cc606c9b823045cc3a Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 15 Mar 2019 13:46:01 -0700 Subject: [PATCH 1/4] Explicitly check if an attribute on a spec exists --- Lib/pyclbr.py | 7 ++++--- Lib/test/test_pyclbr.py | 24 ++++++++++++++++++++++-- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/Lib/pyclbr.py b/Lib/pyclbr.py index 2c798df233b8ebd..93be269c74ebc34 100644 --- a/Lib/pyclbr.py +++ b/Lib/pyclbr.py @@ -162,15 +162,16 @@ def _readmodule(module, path, inpackage=None): spec = importlib.util._find_spec_from_path(fullmodule, search_path) _modules[fullmodule] = tree # Is module a package? - if spec.submodule_search_locations is not None: + if getattr(spec, 'submodule_search_locations', None) is not None: tree['__path__'] = spec.submodule_search_locations try: source = spec.loader.get_source(fullmodule) - if source is None: - return tree except (AttributeError, ImportError): # If module is not Python source, we cannot do anything. return tree + else: + if source is None: + return tree fname = spec.loader.get_filename(fullmodule) return _create_tree(fullmodule, path, fname, source, tree, inpackage) diff --git a/Lib/test/test_pyclbr.py b/Lib/test/test_pyclbr.py index 9e970d9df041b85..467c0c0b52ed710 100644 --- a/Lib/test/test_pyclbr.py +++ b/Lib/test/test_pyclbr.py @@ -10,6 +10,7 @@ import pyclbr from unittest import TestCase, main as unittest_main from test import support +from test.test_importlib import util as test_importlib_util from functools import partial StaticMethodType = type(staticmethod(lambda: None)) @@ -235,11 +236,30 @@ def test_others(self): cm('email.parser') cm('test.test_pyclbr') - def test_issue_14798(self): + +class ReadmoduleTests(TestCase): + + def setUp(self): + self._modules = pyclbr._modules.copy() + + def tearDown(self): + pyclbr._modules = self._modules + + + def test_dotted_name_not_a_package(self): # test ImportError is raised when the first part of a dotted name is - # not a package + # not a package. + # + # Issue #14798. self.assertRaises(ImportError, pyclbr.readmodule_ex, 'asyncore.foo') + def test_module_has_no_spec(self): + module_name = "justnone" + assert module_name not in pyclbr._modules + with test_importlib_util.uncache(module_name): + sys.modules[module_name] = None # Lacks a __spec__. + self.assertFalse(pyclbr.readmodule_ex(module_name)) + if __name__ == "__main__": unittest_main() From d8715145b3795ad1d6885d83c0a07eeeca608f40 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 15 Mar 2019 14:01:40 -0700 Subject: [PATCH 2/4] Test against a non-existent module --- Lib/pyclbr.py | 2 ++ Lib/test/test_pyclbr.py | 6 +++--- .../next/Library/2019-03-15-13-54-07.bpo-36298.amEVK2.rst | 2 ++ 3 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2019-03-15-13-54-07.bpo-36298.amEVK2.rst diff --git a/Lib/pyclbr.py b/Lib/pyclbr.py index 93be269c74ebc34..b966206ed027522 100644 --- a/Lib/pyclbr.py +++ b/Lib/pyclbr.py @@ -160,6 +160,8 @@ def _readmodule(module, path, inpackage=None): else: search_path = path + sys.path spec = importlib.util._find_spec_from_path(fullmodule, search_path) + if spec is None: + raise ModuleNotFoundError(f"no module named {fullmodule!r}", name=fullmodule) _modules[fullmodule] = tree # Is module a package? if getattr(spec, 'submodule_search_locations', None) is not None: diff --git a/Lib/test/test_pyclbr.py b/Lib/test/test_pyclbr.py index 467c0c0b52ed710..839c58f0fde5be1 100644 --- a/Lib/test/test_pyclbr.py +++ b/Lib/test/test_pyclbr.py @@ -254,11 +254,11 @@ def test_dotted_name_not_a_package(self): self.assertRaises(ImportError, pyclbr.readmodule_ex, 'asyncore.foo') def test_module_has_no_spec(self): - module_name = "justnone" + module_name = "doesnotexist" assert module_name not in pyclbr._modules with test_importlib_util.uncache(module_name): - sys.modules[module_name] = None # Lacks a __spec__. - self.assertFalse(pyclbr.readmodule_ex(module_name)) + with self.assertRaises(ModuleNotFoundError): + pyclbr.readmodule_ex(module_name) if __name__ == "__main__": diff --git a/Misc/NEWS.d/next/Library/2019-03-15-13-54-07.bpo-36298.amEVK2.rst b/Misc/NEWS.d/next/Library/2019-03-15-13-54-07.bpo-36298.amEVK2.rst new file mode 100644 index 000000000000000..8501739c309e90a --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-03-15-13-54-07.bpo-36298.amEVK2.rst @@ -0,0 +1,2 @@ +Raise an ImportError in pyclbr when no module spec is found for a module. +Thanks to 'mental' for the bug report. From 9c52322610a783505fb15b45f1e4624687224828 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 15 Mar 2019 14:06:01 -0700 Subject: [PATCH 3/4] Be more accurate about the exception raised --- .../next/Library/2019-03-15-13-54-07.bpo-36298.amEVK2.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2019-03-15-13-54-07.bpo-36298.amEVK2.rst b/Misc/NEWS.d/next/Library/2019-03-15-13-54-07.bpo-36298.amEVK2.rst index 8501739c309e90a..14be079ddb92a2f 100644 --- a/Misc/NEWS.d/next/Library/2019-03-15-13-54-07.bpo-36298.amEVK2.rst +++ b/Misc/NEWS.d/next/Library/2019-03-15-13-54-07.bpo-36298.amEVK2.rst @@ -1,2 +1,2 @@ -Raise an ImportError in pyclbr when no module spec is found for a module. +Raise ModuleNotFoundError in pyclbr when a module can't be found. Thanks to 'mental' for the bug report. From 56293ad3585cea0e92cb29c7d5a2c1e9d3ba1123 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 22 Mar 2019 14:58:40 -0700 Subject: [PATCH 4/4] Revert a change because the spec is expected to be fully-conforming --- Lib/pyclbr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/pyclbr.py b/Lib/pyclbr.py index b966206ed027522..8fd0523b7e3b96b 100644 --- a/Lib/pyclbr.py +++ b/Lib/pyclbr.py @@ -164,7 +164,7 @@ def _readmodule(module, path, inpackage=None): raise ModuleNotFoundError(f"no module named {fullmodule!r}", name=fullmodule) _modules[fullmodule] = tree # Is module a package? - if getattr(spec, 'submodule_search_locations', None) is not None: + if spec.submodule_search_locations is not None: tree['__path__'] = spec.submodule_search_locations try: source = spec.loader.get_source(fullmodule)