Skip to content

Commit

Permalink
Fix importing of namespace packages and built-in modules
Browse files Browse the repository at this point in the history
* Namespace packages commonly don't use `_NamespaceLoader`, and it is
  an implementation detail in any case. The loader is then set to None.
  There is no need to inspect the loader anyway. Everything we need is
  on the `spec`. The unit test had a "work-around" for this, but this
  does not happen in real usage.

* There are more built-in modules than just "builtins". The full list is
  available in `sys.builtin_module_names`[1]. However, we don't need to
  handle them specially.

* It is also hard to know if a standard library module is built as an
  extension or a built-in. For example, in my Ubuntu 22.04's build of
  python 3.10 both `_pickle` and `_elementtree` modules are built-in[2].
  Use a third-party module (`ujson`) for tests, which is guaranteed to
  be an extension module.

* pydantic's `__init__` is no longer an extension module, so use mypyc
  instead, which we already have through mypy.

[1] https://docs.python.org/3/library/sys.html#sys.builtin_module_names
[2] https://github.com/python/cpython/blob/519b2ae22b54760475bbf62b9558d453c703f9c6/PC/config.c#L87
  • Loading branch information
eltoder committed Mar 21, 2024
1 parent 2c2561c commit 30f57c0
Show file tree
Hide file tree
Showing 7 changed files with 124 additions and 163 deletions.
219 changes: 93 additions & 126 deletions poetry.lock

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@ tomli = {version = ">=0.2.6,<3.0.0", python = "<3.11"}
[tool.poetry.dev-dependencies]
flake8 = "^7.0"
isort = "^5.13.2"
mypy = "^1.9"
mypy = {version = "^1.9", extras = ["mypyc"]}
pytest = "^8.1.1"
black = "^24"
pytest-cov = "^4.1.0"
pytest-mock = "^3.12.0"
typing_extensions = ">=4.1,<5"
# Only actually needed as an example library whose __init__ is an extension.
pydantic = "<3"
# Used as an example of an extension module.
ujson = "^5.9.0"

[tool.poetry.scripts]
slotscheck = "slotscheck.cli:root"
Expand Down
31 changes: 8 additions & 23 deletions src/slotscheck/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@
import pkgutil
from dataclasses import dataclass, field, replace
from functools import partial, reduce
from importlib._bootstrap_external import ( # type: ignore[import-not-found]
_NamespaceLoader,
)
from importlib.abc import FileLoader
from importlib.machinery import ExtensionFileLoader
from importlib.util import find_spec
from inspect import isclass
from itertools import chain, takewhile
Expand Down Expand Up @@ -149,7 +144,7 @@ def merge(self, other: ModuleTree) -> ModuleTree:
class UnexpectedImportLocation(Exception):
module: ModuleName
expected: AbsPath
actual: AbsPath
actual: Optional[AbsPath]


@add_slots
Expand All @@ -170,26 +165,16 @@ def module_tree(
return FailedImport(module, e)
if spec is None:
raise ModuleNotFoundError(f"No module named '{module}'", name=module)
loader = spec.loader
*namespaces, name = module.split(".")
location: AbsPath
location = Path(spec.origin) if spec.has_location and spec.origin else None
tree: ModuleTree
if isinstance(loader, (FileLoader, ExtensionFileLoader)):
assert isinstance(loader.path, str) # type: ignore[union-attr]
location = Path(loader.path) # type: ignore[union-attr]
tree = (
_package(name, location.parent)
if loader.is_package(module)
else Module(name)
)
elif isinstance(loader, _NamespaceLoader):
assert len(loader._path._path) == 1
location = Path(loader._path._path[0])
tree = _package(name, location)
elif module == "builtins":
return Module(module)
if spec.submodule_search_locations is None:
tree = Module(name)
else:
raise NotImplementedError(f"Unsupported module loader type: {loader}")
assert len(spec.submodule_search_locations) == 1
pkg_location = Path(spec.submodule_search_locations[0])
location = location or pkg_location
tree = _package(name, pkg_location)

if expected_location and location != expected_location:
raise UnexpectedImportLocation(module, expected_location, location)
Expand Down
1 change: 1 addition & 0 deletions tests/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# This file is needed for the overrides in mypy.ini to work.
1 change: 1 addition & 0 deletions tests/examples/gc.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# This module is shadowed by the builtin module.
4 changes: 2 additions & 2 deletions tests/src/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ def test_builtins(runner: CliRunner):


def test_extension(runner: CliRunner):
result = runner.invoke(cli, ["-m", "_pickle"])
result = runner.invoke(cli, ["-m", "ujson"])
assert result.exit_code == 0
assert result.output == ("All OK!\nScanned 1 module(s), 5 class(es).\n")
assert result.output == ("All OK!\nScanned 1 module(s), 1 class(es).\n")


def test_success_verbose(runner: CliRunner):
Expand Down
25 changes: 16 additions & 9 deletions tests/src/test_discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,27 +221,26 @@ def test_namespaced(self):
"namespaced", make_pkg("module", Module("foo"), Module("bla"))
)

def test_implicitly_namspaced_submodule(self):
def test_implicitly_namespaced_submodule(self):
assert module_tree("implicitly_namespaced.module", None) == make_pkg(
"implicitly_namespaced",
make_pkg("module", Module("foo"), Module("bla")),
)

def test_namespace_loader(self):
import implicitly_namespaced.module # type: ignore # noqa

def test_implicitly_namespaced(self):
assert module_tree("implicitly_namespaced", None) == make_pkg(
"implicitly_namespaced",
Module("version"),
make_pkg("module", Module("foo"), Module("bla")),
make_pkg("another", Module("foo")),
)

def test_builtin(self):
assert module_tree("builtins", None) == Module("builtins")
@pytest.mark.parametrize("module", ["_ast", "builtins", "gc", "sys"])
def test_builtin(self, module: str):
assert module_tree(module, None) == Module(module)

def test_extension(self):
assert module_tree("_elementtree", None) == Module("_elementtree")
assert module_tree("ujson", None) == Module("ujson")

def test_import_causes_base_exception_no_strict_imports(self, mocker):
assert module_tree(
Expand All @@ -255,9 +254,9 @@ def test_import_error(self, mocker):
) == FailedImport("broken.submodule", mocker.ANY)

def test_extension_package(self):
tree = module_tree("pydantic", None)
tree = module_tree("mypyc", None)
assert isinstance(tree, Package)
assert len(tree.content) > 20
assert len(tree.content) > 10

def test_module(self):
assert module_tree(
Expand All @@ -282,6 +281,14 @@ def test_unexpected_location(self):
EXAMPLES_DIR / "module_misc/a/b/c.py",
)

def test_shadowed_by_builtin(self):
with pytest.raises(UnexpectedImportLocation) as exc:
module_tree("gc", expected_location=EXAMPLES_DIR / "gc.py")

assert exc.value == UnexpectedImportLocation(
"gc", EXAMPLES_DIR / "gc.py", None
)

def test_pyc_file(self):
assert module_tree("compiled", None) == make_pkg(
"compiled", Module("foo"), Module("bar")
Expand Down

0 comments on commit 30f57c0

Please sign in to comment.