fix: Insert the right directory in front of import paths before inspe…

…cting a module (dynamically imported)

src/griffe/agents/inspector.py

@@ -82,9 +82,6 @@ def inspect(
     Returns:
         The module, with its members populated.
     """
-    import_paths = list(import_paths) if import_paths else []
-    if filepath and filepath.parent not in import_paths:
-        import_paths.insert(0, filepath.parent)
     return Inspector(
         module_name,
         filepath,
@@ -173,12 +170,34 @@ def get_module(self, import_paths: Sequence[str | Path] | None = None) -> Module
         import_path = self.module_name
         if self.parent is not None:
             import_path = f"{self.parent.path}.{import_path}"
+
+        # Make sure `import_paths` is a list, in case we want to `insert` into it.
+        import_paths = list(import_paths or ())
+
+        # If the thing we want to import has a filepath,
+        # we make sure to insert the right parent directory
+        # at the front of our list of import paths.
+        # We do this by counting the number of dots `.` in the import path,
+        # corresponding to slashes `/` in the filesystem,
+        # and go up in the file tree the same number of times.
+        if self.filepath:
+            parent_path = self.filepath.parent
+            for _ in range(import_path.count(".")):
+                parent_path = parent_path.parent
+            if parent_path not in import_paths:
+                import_paths.insert(0, parent_path)
+
         value = dynamic_import(import_path, import_paths)
-        parent = None
+
+        # We successfully imported the given object,
+        # and we now create the object tree with all the necessary nodes,
+        # from the root of the package to this leaf object.
+        parent_node = None
         if self.parent is not None:
             for part in self.parent.path.split("."):
-                parent = ObjectNode(None, name=part, parent=parent)
-        module_node = ObjectNode(value, self.module_name, parent=parent)
+                parent_node = ObjectNode(None, name=part, parent=parent_node)
+        module_node = ObjectNode(value, self.module_name, parent=parent_node)
+
         self.inspect(module_node)
         return self.current.module
 

src/griffe/importer.py

@@ -11,6 +11,10 @@
     from pathlib import Path
 
 
+def _error_details(error: BaseException, objpath: str) -> str:
+    return f"With sys.path = {sys.path!r}, accessing {objpath!r} raises {error.__class__.__name__}: {error}"
+
+
 @contextmanager
 def sys_path(*paths: str | Path) -> Iterator[None]:
     """Redefine `sys.path` temporarily.
@@ -39,9 +43,39 @@ def dynamic_import(import_path: str, import_paths: Sequence[str | Path] | None =
     It can be a module, class, method, function, attribute,
     nested arbitrarily.
 
+    It works like this:
+
+    - for a given object path `a.b.x.y`
+    - it tries to import `a.b.x.y` as a module (with `importlib.import_module`)
+    - if it fails, it tries again with `a.b.x`, storing `y`
+    - then `a.b`, storing `x.y`
+    - then `a`, storing `b.x.y`
+    - if nothing worked, it raises an error
+    - if one of the iteration worked, it moves on, and...
+    - it tries to get the remaining (stored) parts with `getattr`
+    - for example it gets `b` from `a`, then `x` from `b`, etc.
+    - if a single attribute access fails, it raises an error
+    - if everything worked, it returns the last obtained attribute
+
+    Since the function potentially tries multiple things before succeeding,
+    all errors happening along the way are recorded, and re-emitted with
+    an `ImportError` when it fails, to let users know what was tried.
+
+    IMPORTANT: The paths given through the `import_paths` parameter are used
+    to temporarily patch `sys.path`: this function is therefore not thread-safe.
+
+    IMPORTANT: The paths given as `import_paths` must be *correct*.
+    The contents of `sys.path` must be consistent to what a user of the imported code
+    would expect. Given a set of paths, if the import fails for a user, it will fail here too,
+    with potentially unintuitive errors. If we wanted to make this function more robust,
+    we could add a loop to "roll the window" of given paths, shifting them to the left
+    (for example: `("/a/a", "/a/b", "/a/c/")`, then `("/a/b", "/a/c", "/a/a/")`,
+    then `("/a/c", "/a/a", "/a/b/")`), to make sure each entry is given highest priority
+    at least once, maintaining relative order, but we deem this unncessary for now.
+
     Parameters:
         import_path: The path of the object to import.
-        import_paths: The paths to import the object from.
+        import_paths: The (sys) paths to import the object from.
 
     Raises:
         ModuleNotFoundError: When the object's module could not be found.
@@ -55,36 +89,34 @@ def dynamic_import(import_path: str, import_paths: Sequence[str | Path] | None =
     errors = []
 
     with sys_path(*(import_paths or ())):
-        while True:
+        while module_parts:
             module_path = ".".join(module_parts)
             try:
                 module = import_module(module_path)
-            except ModuleNotFoundError as error:
-                if len(module_parts) == 1:
-                    raise
-                errors.append(f"{error.__class__.__name__}: {error}")
-                object_parts.insert(0, module_parts.pop(-1))
-            except (Exception, BaseException) as error:
+            except BaseException as error:  # noqa: BLE001
                 # pyo3's PanicException can only be caught with BaseException.
-                # We do want to catch base exceptions anyway (exit, interrupt, etc.),
-                errors.append(f"{error.__class__.__name__}: {error}")
-                raise ImportError("\n".join(errors)) from error
+                # We do want to catch base exceptions anyway (exit, interrupt, etc.).
+                errors.append(_error_details(error, module_path))
+                object_parts.insert(0, module_parts.pop(-1))
             else:
                 break
+        else:
+            raise ImportError("; ".join(errors))
+
+        # Sometimes extra dependencies are not installed,
+        # so importing the leaf module fails with a ModuleNotFoundError,
+        # or later `getattr` triggers additional code that fails.
+        # In these cases, and for consistency, we always re-raise an ImportError
+        # instead of an any other exception (it's called "dynamic import" after all).
+        # See https://github.com/mkdocstrings/mkdocstrings/issues/380
+        value = module
+        for part in object_parts:
+            try:
+                value = getattr(value, part)
+            except BaseException as error:  # noqa: BLE001
+                errors.append(_error_details(error, module_path + ":" + ".".join(object_parts)))
+                raise ImportError("; ".join(errors))  # noqa: B904,TRY200
 
-    # Sometimes extra dependencies are not installed,
-    # and therefore we aren't able to import the leaf module,
-    # so we end up with its parent instead, on which we can't
-    # get the attribute either. In that case we re-raise an
-    # ImportError for consistency.
-    # See https://github.com/mkdocstrings/mkdocstrings/issues/380
-
-    value = module
-    for part in object_parts:
-        try:
-            value = getattr(value, part)
-        except AttributeError as error:
-            raise ImportError("\n".join(errors)) from error
     return value
 
 

tests/test_git.py

@@ -97,7 +97,7 @@ def test_load_git_errors(git_repo: Path) -> None:
     with pytest.raises(RuntimeError, match="Could not create git worktre"):
         load_git(MODULE_NAME, ref="invalid-tag", repo=git_repo)
 
-    with pytest.raises(ModuleNotFoundError, match="No module named 'not_a_real_module'"):
+    with pytest.raises(ImportError, match="ModuleNotFoundError: No module named 'not_a_real_module'"):
         load_git("not_a_real_module", ref="v0.2.0", repo=git_repo)
 
 

tests/test_inspector.py

@@ -2,12 +2,12 @@
 
 from __future__ import annotations
 
-from contextlib import suppress
 from pathlib import Path
 
 import pytest
 
 from griffe.agents.inspector import inspect
+import sys
 from griffe.tests import temporary_inspected_module, temporary_pypackage
 
 
@@ -54,9 +54,10 @@ def test_missing_dependency() -> None:
     with temporary_pypackage("package", ["module.py"]) as tmp_package:
         filepath = Path(tmp_package.path, "module.py")
         filepath.write_text("import missing")
-        with pytest.raises(ImportError):  # noqa: SIM117
-            with suppress(ModuleNotFoundError):
-                inspect("package.module", filepath=filepath, import_paths=[tmp_package.tmpdir])
+        with pytest.raises(ImportError, match="ModuleNotFoundError: No module named 'missing'"):
+            inspect("package.module", filepath=filepath, import_paths=[tmp_package.tmpdir])
+    sys.modules.pop("package", None)
+    sys.modules.pop("package.module", None)
 
 
 def test_inspect_properties_as_attributes() -> None:
@@ -95,3 +96,11 @@ def test_inspecting_parameters_with_functions_as_default_values() -> None:
     with temporary_inspected_module("def func(): ...\ndef other_func(f=func): ...") as module:
         default = module["other_func"].parameters["f"].default
     assert default == "func"
+
+
+def test_inspecting_package_and_module_with_same_names() -> None:
+    """Package and module having same name shouldn't cause issues."""
+    with temporary_pypackage("package", {"package.py": "a = 0"}) as tmp_package:
+        inspect("package.package", filepath=Path(tmp_package.path, "package.py"), import_paths=[tmp_package.tmpdir])
+    sys.modules.pop("package", None)
+    sys.modules.pop("package.package", None)

tests/test_stdlib.py

@@ -33,7 +33,7 @@ def test_fuzzing_on_stdlib() -> None:
 
     loader = GriffeLoader()
     for package in stblib_packages:
-        with suppress(ModuleNotFoundError):
+        with suppress(ImportError):
             loader.load(package)
 
     loader.resolve_aliases(implicit=True, external=True)