From ac2b173bc8acd2d08f6b6ffe29dd8cda0b2c8814 Mon Sep 17 00:00:00 2001 From: Peter Kolbus Date: Sun, 22 Nov 2020 10:48:03 -0600 Subject: [PATCH] Remove dependency on imp. --- ChangeLog | 5 ++ astroid/interpreter/_import/spec.py | 101 +++++++++++++---------- astroid/modutils.py | 108 +++++++------------------ astroid/scoped_nodes.py | 1 + tests/testdata/python3/data/nonregr.py | 4 +- tests/unittest_brain.py | 6 +- tests/unittest_inference.py | 12 --- tests/unittest_modutils.py | 19 ++++- 8 files changed, 115 insertions(+), 141 deletions(-) diff --git a/ChangeLog b/ChangeLog index a5a0be55d2..cba68dfd3d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -55,6 +55,11 @@ Release Date: TBA * Reduce memory usage of astroid's module cache. +* Remove dependency on `imp`. + + Close #594 + Close #681 + What's New in astroid 2.4.3? ============================ Release Date: TBA diff --git a/astroid/interpreter/_import/spec.py b/astroid/interpreter/_import/spec.py index 26e22b5ddc..125f9a16d8 100644 --- a/astroid/interpreter/_import/spec.py +++ b/astroid/interpreter/_import/spec.py @@ -12,17 +12,11 @@ import collections import distutils import enum -import imp import os import sys import zipimport -try: - import importlib.machinery - - _HAS_MACHINERY = True -except ImportError: - _HAS_MACHINERY = False +import importlib.machinery try: from functools import lru_cache @@ -37,22 +31,6 @@ "PY_CODERESOURCE PY_COMPILED PY_FROZEN PY_RESOURCE " "PY_SOURCE PY_ZIPMODULE PY_NAMESPACE", ) -_ImpTypes = { - imp.C_BUILTIN: ModuleType.C_BUILTIN, - imp.C_EXTENSION: ModuleType.C_EXTENSION, - imp.PKG_DIRECTORY: ModuleType.PKG_DIRECTORY, - imp.PY_COMPILED: ModuleType.PY_COMPILED, - imp.PY_FROZEN: ModuleType.PY_FROZEN, - imp.PY_SOURCE: ModuleType.PY_SOURCE, -} -if hasattr(imp, "PY_RESOURCE"): - _ImpTypes[imp.PY_RESOURCE] = ModuleType.PY_RESOURCE -if hasattr(imp, "PY_CODERESOURCE"): - _ImpTypes[imp.PY_CODERESOURCE] = ModuleType.PY_CODERESOURCE - - -def _imp_type_to_module_type(imp_type): - return _ImpTypes[imp_type] _ModuleSpec = collections.namedtuple( @@ -114,26 +92,59 @@ def contribute_to_path(self, spec, processed): """Get a list of extra paths where this finder can search.""" -class ImpFinder(Finder): - """A finder based on the imp module.""" +class ImportlibFinder(Finder): + """A finder based on the importlib module.""" + + _SUFFIXES = ( + [(s, ModuleType.C_EXTENSION) for s in importlib.machinery.EXTENSION_SUFFIXES] + + [(s, ModuleType.PY_SOURCE) for s in importlib.machinery.SOURCE_SUFFIXES] + + [(s, ModuleType.PY_COMPILED) for s in importlib.machinery.BYTECODE_SUFFIXES] + ) def find_module(self, modname, module_parts, processed, submodule_path): + if not isinstance(modname, str): + raise TypeError("'modname' must be a str, not {}".format(type(modname))) if submodule_path is not None: submodule_path = list(submodule_path) - try: - stream, mp_filename, mp_desc = imp.find_module(modname, submodule_path) - except ImportError: - return None - - # Close resources. - if stream: - stream.close() - - return ModuleSpec( - name=modname, - location=mp_filename, - module_type=_imp_type_to_module_type(mp_desc[2]), - ) + else: + try: + spec = importlib.util.find_spec(modname) + if spec: + if spec.loader is importlib.machinery.BuiltinImporter: + return ModuleSpec( + name=modname, + location=None, + module_type=ModuleType.C_BUILTIN, + ) + if spec.loader is importlib.machinery.FrozenImporter: + return ModuleSpec( + name=modname, + location=None, + module_type=ModuleType.PY_FROZEN, + ) + except ValueError: + pass + submodule_path = sys.path + + for entry in submodule_path: + package_directory = os.path.join(entry, modname) + for suffix in [".py", importlib.machinery.BYTECODE_SUFFIXES[0]]: + package_file_name = "__init__" + suffix + file_path = os.path.join(package_directory, package_file_name) + if os.path.isfile(file_path): + return ModuleSpec( + name=modname, + location=package_directory, + module_type=ModuleType.PKG_DIRECTORY, + ) + for suffix, type_ in ImportlibFinder._SUFFIXES: + file_name = modname + suffix + file_path = os.path.join(entry, file_name) + if os.path.isfile(file_path): + return ModuleSpec( + name=modname, location=file_path, module_type=type_ + ) + return None def contribute_to_path(self, spec, processed): if spec.location is None: @@ -159,7 +170,7 @@ def contribute_to_path(self, spec, processed): return path -class ExplicitNamespacePackageFinder(ImpFinder): +class ExplicitNamespacePackageFinder(ImportlibFinder): """A finder for the explicit namespace packages, generated through pkg_resources.""" def find_module(self, modname, module_parts, processed, submodule_path): @@ -229,10 +240,12 @@ def contribute_to_path(self, spec, processed): return None -_SPEC_FINDERS = (ImpFinder, ZipFinder) -if _HAS_MACHINERY: - _SPEC_FINDERS += (PathSpecFinder,) -_SPEC_FINDERS += (ExplicitNamespacePackageFinder,) +_SPEC_FINDERS = ( + ImportlibFinder, + ZipFinder, + PathSpecFinder, + ExplicitNamespacePackageFinder, +) def _is_setuptools_namespace(location): diff --git a/astroid/modutils.py b/astroid/modutils.py index 4e6ed86bf8..4fe0bd5e7a 100644 --- a/astroid/modutils.py +++ b/astroid/modutils.py @@ -31,7 +31,7 @@ :type BUILTIN_MODULES: dict :var BUILTIN_MODULES: dictionary with builtin module names has key """ -import imp +import importlib.util import os import platform import sys @@ -44,7 +44,6 @@ # distutils is replaced by virtualenv with a module that does # weird path manipulations in order to get to the # real distutils module. -from typing import Optional, List from .interpreter._import import spec from .interpreter._import import util @@ -178,110 +177,53 @@ def _cache_normalize_path(path): return result -def load_module_from_name(dotted_name, path=None, use_sys=True): +def load_module_from_name(dotted_name): """Load a Python module from its name. :type dotted_name: str :param dotted_name: python name of a module or package - :type path: list or None - :param path: - optional list of path where the module or package should be - searched (use sys.path if nothing or None is given) - - :type use_sys: bool - :param use_sys: - boolean indicating whether the sys.modules dictionary should be - used or not - - :raise ImportError: if the module or package is not found :rtype: module :return: the loaded module """ - return load_module_from_modpath(dotted_name.split("."), path, use_sys) + try: + return sys.modules[dotted_name] + except KeyError: + pass + return importlib.import_module(dotted_name) -def load_module_from_modpath(parts, path: Optional[List[str]] = None, use_sys=1): + +def load_module_from_modpath(parts): """Load a python module from its split name. :type parts: list(str) or tuple(str) :param parts: python name of a module or package split on '.' - :param path: - Optional list of path where the module or package should be - searched (use sys.path if nothing or None is given) - - :type use_sys: bool - :param use_sys: - boolean indicating whether the sys.modules dictionary should be used or not - :raise ImportError: if the module or package is not found :rtype: module :return: the loaded module """ - if use_sys: - try: - return sys.modules[".".join(parts)] - except KeyError: - pass - modpath = [] - prevmodule = None - for part in parts: - modpath.append(part) - curname = ".".join(modpath) - module = None - if len(modpath) != len(parts): - # even with use_sys=False, should try to get outer packages from sys.modules - module = sys.modules.get(curname) - elif use_sys: - # because it may have been indirectly loaded through a parent - module = sys.modules.get(curname) - if module is None: - mp_file, mp_filename, mp_desc = imp.find_module(part, path) - module = imp.load_module(curname, mp_file, mp_filename, mp_desc) - # mp_file still needs to be closed. - if mp_file: - mp_file.close() - if prevmodule: - setattr(prevmodule, part, module) - _file = getattr(module, "__file__", "") - prevmodule = module - if not _file and util.is_namespace(curname): - continue - if not _file and len(modpath) != len(parts): - raise ImportError("no module in %s" % ".".join(parts[len(modpath) :])) - path = [os.path.dirname(_file)] - return module + return load_module_from_name(".".join(parts)) -def load_module_from_file( - filepath: str, path: Optional[List[str]] = None, use_sys=True -): +def load_module_from_file(filepath: str): """Load a Python module from it's path. :type filepath: str :param filepath: path to the python module or package - :param Optional[List[str]] path: - Optional list of path where the module or package should be - searched (use sys.path if nothing or None is given) - - :type use_sys: bool - :param use_sys: - boolean indicating whether the sys.modules dictionary should be - used or not - :raise ImportError: if the module or package is not found :rtype: module :return: the loaded module """ modpath = modpath_from_file(filepath) - return load_module_from_modpath(modpath, path, use_sys) + return load_module_from_modpath(modpath) def check_modpath_has_init(path, mod_path): @@ -418,7 +360,9 @@ def file_info_from_modpath(modpath, path=None, context_file=None): elif modpath == ["os", "path"]: # FIXME: currently ignoring search_path... return spec.ModuleSpec( - name="os.path", location=os.path.__file__, module_type=imp.PY_SOURCE + name="os.path", + location=os.path.__file__, + module_type=spec.ModuleType.PY_SOURCE, ) return _spec_from_modpath(modpath, path, context) @@ -614,16 +558,22 @@ def is_relative(modname, from_file): from_file = os.path.dirname(from_file) if from_file in sys.path: return False - try: - stream, _, _ = imp.find_module(modname.split(".")[0], [from_file]) - - # Close the stream to avoid ResourceWarnings. - if stream: - stream.close() - return True - except ImportError: + name = os.path.basename(from_file) + file_path = os.path.dirname(from_file) + parent_spec = importlib.util.find_spec(name, from_file) + while parent_spec is None and len(file_path) > 0: + name = os.path.basename(file_path) + "." + name + file_path = os.path.dirname(file_path) + parent_spec = importlib.util.find_spec(name, from_file) + + if parent_spec is None: return False + submodule_spec = importlib.util.find_spec( + name + "." + modname.split(".")[0], parent_spec.submodule_search_locations + ) + return submodule_spec is not None + # internal only functions ##################################################### diff --git a/astroid/scoped_nodes.py b/astroid/scoped_nodes.py index 5c94196689..15ef3ee954 100644 --- a/astroid/scoped_nodes.py +++ b/astroid/scoped_nodes.py @@ -1448,6 +1448,7 @@ def extra_decorators(self): decorators.append(assign.value) return decorators + # pylint: disable=invalid-overridden-method @decorators_mod.cachedproperty def type( self diff --git a/tests/testdata/python3/data/nonregr.py b/tests/testdata/python3/data/nonregr.py index 78765c8544..073135d233 100644 --- a/tests/testdata/python3/data/nonregr.py +++ b/tests/testdata/python3/data/nonregr.py @@ -16,9 +16,7 @@ def toto(value): print(v.get('yo')) -import imp -fp, mpath, desc = imp.find_module('optparse',a) -s_opt = imp.load_module('std_optparse', fp, mpath, desc) +import optparse as s_opt class OptionParser(s_opt.OptionParser): diff --git a/tests/unittest_brain.py b/tests/unittest_brain.py index c308ddd535..b42fa03ff1 100644 --- a/tests/unittest_brain.py +++ b/tests/unittest_brain.py @@ -540,8 +540,10 @@ def test_multiprocessing_manager(self): obj = next(module[attr].infer()) self.assertEqual(obj.qname(), "{}.{}".format(bases.BUILTINS, attr)) - array = next(module["array"].infer()) - self.assertEqual(array.qname(), "array.array") + # pypy's implementation of array.__spec__ return None. This causes problems for this inference. + if not hasattr(sys, "pypy_version_info"): + array = next(module["array"].infer()) + self.assertEqual(array.qname(), "array.array") manager = next(module["manager"].infer()) # Verify that we have these attributes diff --git a/tests/unittest_inference.py b/tests/unittest_inference.py index b7bc732d3f..23a131e4d9 100644 --- a/tests/unittest_inference.py +++ b/tests/unittest_inference.py @@ -541,18 +541,6 @@ class Warning(Warning): self.assertEqual(ancestor.root().name, BUILTINS) self.assertRaises(StopIteration, partial(next, ancestors)) - def test_qqch(self): - code = """ - from astroid.modutils import load_module_from_name - xxx = load_module_from_name('__pkginfo__') - """ - ast = parse(code, __name__) - xxx = ast["xxx"] - self.assertSetEqual( - {n.__class__ for n in xxx.inferred()}, - {nodes.Const, util.Uninferable.__class__}, - ) - def test_method_argument(self): code = ''' class ErudiEntitySchema: diff --git a/tests/unittest_modutils.py b/tests/unittest_modutils.py index b5c41bf09b..460bc93f2c 100644 --- a/tests/unittest_modutils.py +++ b/tests/unittest_modutils.py @@ -82,7 +82,7 @@ def test_knownValues_load_module_from_name_2(self): def test_raise_load_module_from_name_1(self): self.assertRaises( - ImportError, modutils.load_module_from_name, "os.path", use_sys=0 + ImportError, modutils.load_module_from_name, "_this_module_does_not_exist_" ) @@ -297,6 +297,23 @@ def test_knownValues_is_relative_1(self): def test_knownValues_is_relative_3(self): self.assertFalse(modutils.is_relative("astroid", astroid.__path__[0])) + def test_deep_relative(self): + self.assertTrue(modutils.is_relative("ElementTree", xml.etree.__path__[0])) + + def test_deep_relative2(self): + self.assertFalse(modutils.is_relative("ElementTree", xml.__path__[0])) + + def test_deep_relative3(self): + self.assertTrue(modutils.is_relative("etree.ElementTree", xml.__path__[0])) + + def test_deep_relative4(self): + self.assertTrue(modutils.is_relative("etree.gibberish", xml.__path__[0])) + + def test_is_relative_bad_path(self): + self.assertFalse( + modutils.is_relative("ElementTree", os.path.join(xml.__path__[0], "ftree")) + ) + class GetModuleFilesTest(unittest.TestCase): def test_get_module_files_1(self):