From cdc9e1c5ee92a4c621314a9d9c6c465bfdd2ad92 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timoth=C3=A9e=20Mazzucotelli?= <dev@pawamoy.fr>
Date: Mon, 11 Mar 2024 18:13:38 +0100
Subject: [PATCH] refactor: Warn (debug) when a submodule shadows a member with
 the same name

Issue-124: https://github.com/mkdocstrings/griffe/issues/124
---
 src/griffe/loader.py | 21 +++++++++++++--------
 tests/test_loader.py | 24 +++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/src/griffe/loader.py b/src/griffe/loader.py
index 19e4f250..5200e9fd 100644
--- a/src/griffe/loader.py
+++ b/src/griffe/loader.py
@@ -387,7 +387,7 @@ def expand_wildcards(
                 )
                 # Special case: we avoid overwriting a submodule with an alias pointing to it.
                 # Griffe suffers from this design flaw where an object cannot store both
-                # a submodule and a member of the same name, while this poses no issue in Python.
+                # a submodule and a member of the same name, while this poses (almost) no issue in Python.
                 # We at least prevent this case where a submodule is overwritten by an imported version of itself.
                 if already_present:
                     prev_member = obj.get_member(new_member.name)
@@ -580,17 +580,22 @@ def _load_submodule(self, module: Module, subparts: tuple[str, ...], subpath: Pa
             return
         submodule_name = subparts[-1]
         try:
-            parent_module.set_member(
+            submodule = self._load_module(
                 submodule_name,
-                self._load_module(
-                    submodule_name,
-                    subpath,
-                    submodules=False,
-                    parent=parent_module,
-                ),
+                subpath,
+                submodules=False,
+                parent=parent_module,
             )
         except LoadingError as error:
             logger.debug(str(error))
+        else:
+            if submodule_name in parent_module.members:
+                logger.debug(
+                    f"Submodule '{submodule.path}' is shadowing the member at the same path. "
+                    "We recommend renaming the member or the submodule (for example prefixing it with `_`), "
+                    "see https://mkdocstrings.github.io/griffe/best_practices/#avoid-member-submodule-name-shadowing.",
+                )
+            parent_module.set_member(submodule_name, submodule)
 
     def _create_module(self, module_name: str, module_path: Path | list[Path]) -> Module:
         return Module(
diff --git a/tests/test_loader.py b/tests/test_loader.py
index 8802420a..93a0dd61 100644
--- a/tests/test_loader.py
+++ b/tests/test_loader.py
@@ -10,7 +10,7 @@
 
 from griffe.expressions import ExprName
 from griffe.loader import GriffeLoader
-from griffe.tests import temporary_pyfile, temporary_pypackage
+from griffe.tests import temporary_pyfile, temporary_pypackage, temporary_visited_package
 
 if TYPE_CHECKING:
     from pathlib import Path
@@ -399,3 +399,25 @@ def test_loading_stubs_only_packages(tmp_path: Path, namespace: bool) -> None:
         assert "b" in top_module.members
     assert "a" in top_module["module"].members
     assert "b" in top_module["module"].members
+
+
+@pytest.mark.parametrize(
+    "init",
+    [
+        "from package.thing import thing",
+        "thing = False",
+    ],
+)
+def test_submodule_shadowing_member(init: str, caplog: pytest.LogCaptureFixture) -> None:
+    """Warn when a submodule shadows a member of the same name.
+
+    Parameters:
+        init: Contents of the top-level init module.
+    """
+    caplog.set_level(logging.DEBUG)
+    with temporary_visited_package(
+        "package",
+        {"__init__.py": init, "thing.py": "thing = True"},
+        init=True,
+    ):
+        assert "shadowing" in caplog.text