Skip to content

Commit

Permalink
refactor: Warn (debug) when a submodule shadows a member with the sam…
Browse files Browse the repository at this point in the history
…e name

Issue-124: #124
  • Loading branch information
pawamoy committed Mar 11, 2024
1 parent 781437e commit cdc9e1c
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 9 deletions.
21 changes: 13 additions & 8 deletions src/griffe/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down
24 changes: 23 additions & 1 deletion tests/test_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

0 comments on commit cdc9e1c

Please sign in to comment.