Skip to content

Commit

Permalink
feat: Warn when an object coming from a sibling, parent or external m…
Browse files Browse the repository at this point in the history
…odule 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: #249
Related-to-PR-251: #251
  • Loading branch information
pawamoy committed Aug 11, 2024
1 parent 309c6e3 commit f82317a
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 3 deletions.
21 changes: 20 additions & 1 deletion src/_griffe/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
25 changes: 23 additions & 2 deletions tests/test_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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: ...
Expand Down Expand Up @@ -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)

0 comments on commit f82317a

Please # to comment.