Skip to content

Commit

Permalink
fix: Reduce risk of recursion errors by excluding imported objects fr…
Browse files Browse the repository at this point in the history
…om `has_docstrings`, unless they're public

Issue-302: #302
  • Loading branch information
pawamoy committed Aug 10, 2024
1 parent 8a21f4d commit 9296ca7
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
22 changes: 20 additions & 2 deletions src/_griffe/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -444,12 +444,30 @@ def has_docstring(self) -> bool:
"""Whether this object has a docstring (empty or not)."""
return bool(self.docstring)

# NOTE: (pawamoy) I'm not happy with `has_docstrings`.
# It currently recurses into submodules, but that doesn't make sense
# if downstream projects use it to know if they should render an init module
# while not rendering submodules too: the property could tell them there are
# docstrings, but they could be in submodules, not in the init module.
# Maybe we should derive it into new properties: `has_local_docstrings`,
# `has_docstrings`, `has_public_docstrings`... Maybe we should make it a function?`
# For now it's used in mkdocstrings-python so we must be careful with changes.
@property
def has_docstrings(self) -> bool:
"""Whether this object or any of its members has a docstring (empty or not)."""
"""Whether this object or any of its members has a docstring (empty or not).
Inherited members are not considered. Imported members are not considered,
unless they are also public.
"""
if self.has_docstring:
return True
return any(member.has_docstrings for member in self.members.values())
for member in self.members.values():
try:
if (not member.is_imported or member.is_public) and member.has_docstrings:
return True
except AliasResolutionError:
continue
return False

def member_is_exported(self, member: Object | Alias, *, explicitely: bool = True) -> bool: # noqa: ARG002
"""Deprecated. Use [`member.is_exported`][griffe.Object.is_exported] instead."""
Expand Down
7 changes: 7 additions & 0 deletions tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
module_vtree,
temporary_inspected_module,
temporary_pypackage,
temporary_visited_module,
temporary_visited_package,
)

Expand All @@ -38,6 +39,12 @@ def test_submodule_exports() -> None:

def test_has_docstrings() -> None:
"""Assert the `.has_docstrings` method is recursive."""
with temporary_visited_module("class A:\n '''Hello.'''") as module:
assert module.has_docstrings


def test_has_docstrings_submodules() -> None:
"""Assert the `.has_docstrings` method descends into submodules."""
module = module_vtree("a.b.c.d")
module["b.c.d"].docstring = Docstring("Hello.")
assert module.has_docstrings
Expand Down

0 comments on commit 9296ca7

Please sign in to comment.