From f82317a00333e1b8971625f14e4452e93e9840ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Mazzucotelli?= Date: Sun, 11 Aug 2024 16:57:17 +0200 Subject: [PATCH] feat: Warn when an object coming from a sibling, parent or external module instead of the current module or a submodule is exported Implemented as a DEBUG log for now, waiting for a logging configuration system. Issue-249: https://github.com/mkdocstrings/griffe/issues/249 Related-to-PR-251: https://github.com/mkdocstrings/griffe/pull/251 --- src/_griffe/loader.py | 21 ++++++++++++++++++++- tests/test_loader.py | 25 +++++++++++++++++++++++-- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/_griffe/loader.py b/src/_griffe/loader.py index 1f68f8e0..80f803c4 100644 --- a/src/_griffe/loader.py +++ b/src/_griffe/loader.py @@ -13,7 +13,13 @@ from _griffe.agents.visitor import visit from _griffe.collections import LinesCollection, ModulesCollection from _griffe.enumerations import Kind -from _griffe.exceptions import AliasResolutionError, CyclicAliasError, LoadingError, UnimportableModuleError +from _griffe.exceptions import ( + AliasResolutionError, + CyclicAliasError, + LoadingError, + NameResolutionError, + UnimportableModuleError, +) from _griffe.expressions import ExprName from _griffe.extensions.base import Extensions, load_extensions from _griffe.finder import ModuleFinder, NamespacePackage, Package @@ -292,6 +298,7 @@ def expand_exports(self, module: Module, seen: set | None = None) -> None: seen.add(module.path) if module.exports is None: return + expanded = set() for export in module.exports: # It's a name: we resolve it, get the module it comes from, @@ -311,9 +318,21 @@ def expand_exports(self, module: Module, seen: set | None = None) -> None: _logger.warning(f"Unsupported item in {module.path}.__all__: {export} (use strings only)") # It's a string, simply add it to the current exports. else: + with suppress(NameResolutionError): + if not module.resolve(export).startswith(module.path): + # NOTE: This won't work for built-in attributes during inspection, + # since their canonical module cannot be determined. + _logger.debug( + f"Name `{export}` exported by module `{module.path}` doesn't come from this module or from a submodule.", + ) expanded.add(export) module.exports = expanded + # Make sure to expand exports in all modules. + for submodule in module.modules.values(): + if not submodule.is_alias and submodule.path not in seen: + self.expand_exports(submodule, seen) + def expand_wildcards( self, obj: Object, diff --git a/tests/test_loader.py b/tests/test_loader.py index 0fdfa533..ef6486a7 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -8,7 +8,7 @@ import pytest -from griffe import ExprName, GriffeLoader, temporary_pyfile, temporary_pypackage, temporary_visited_package +from griffe import ExprName, GriffeLoader, load, temporary_pyfile, temporary_pypackage, temporary_visited_package from tests.helpers import clear_sys_modules if TYPE_CHECKING: @@ -101,7 +101,7 @@ def test_load_data_from_stubs() -> None: code = ''' from typing import List, Literal, Optional, Protocol, Set, Tuple, Union - __all__ = 'RustNotify', 'WatchfilesRustInternalError' + __all__ = ['RustNotify'] class AbstractEvent(Protocol): def is_set(self) -> bool: ... @@ -484,3 +484,24 @@ def test_not_calling_package_loaded_hook_on_something_else_than_package() -> Non alias: Alias = loader.load("pkg.L") assert alias.is_alias assert not alias.resolved + + +@pytest.mark.parametrize("force_inspection", [True, False]) +def test_warning_on_objects_from_non_submodules_being_exported( + caplog: pytest.LogCaptureFixture, + force_inspection: bool, +) -> None: + """Warn when objects from non-submodules are exported.""" + with temporary_pypackage( + "pkg", + { + "__init__.py": "from typing import List\nfrom pkg import moda, modb\n__all__ = ['List']", + "moda.py": "class Thing: ...", + "modb.py": "from pkg.moda import Thing\n\n__all__ = ['Thing']", + }, + ) as pkg: + with caplog.at_level(logging.DEBUG, logger="griffe"): + load("pkg", search_paths=[pkg.tmpdir], force_inspection=force_inspection, resolve_aliases=True) + messages = [record.message for record in caplog.records] + assert any("Name `List` exported by module" in msg for msg in messages) + assert any("Name `Thing` exported by module" in msg for msg in messages)