Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions _bmad-output/implementation-artifacts/sprint-status.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# generated: 2026-03-07
# generated: 2026-03-24
# project: docvet
# project_key: NOKEY
# tracking_system: both
Expand Down Expand Up @@ -33,7 +33,7 @@
# - SM typically creates next story after previous one is 'done' to incorporate learnings
# - Dev moves story to 'review', then runs code-review (fresh context, different LLM recommended)

generated: 2026-03-07
generated: 2026-03-24
project: docvet
project_key: NOKEY
tracking_system: both
Expand Down Expand Up @@ -268,7 +268,7 @@ development_status:
epic-32: in-progress
32-1-fix-feasibility-spike: done
32-2-fix-core-section-scaffolding-engine: done
32-3-fix-cli-wiring-subcommand-dry-run-and-discovery: review
32-3-fix-cli-wiring-subcommand-dry-run-and-discovery: done
32-4-inline-suppression-comments: backlog
epic-32-retrospective: optional

Expand Down
17 changes: 12 additions & 5 deletions src/docvet/checks/enrichment/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
deprecation notices), ``_reverse`` (extra Raises/Yields/Returns), and
``_late_rules`` (trivial docstrings, return types, init params,
scaffold-incomplete). This module retains section parsing, shared
constants, and the ``check_enrichment`` dispatch orchestrator.
constants, the ``_should_skip_reverse_check`` guard, and the
``check_enrichment`` dispatch orchestrator.

Supports both Google-style and Sphinx/RST docstring conventions via the
``style`` parameter on :func:`check_enrichment`. NumPy-style section
Expand Down Expand Up @@ -253,6 +254,7 @@ def _extract_section_content(docstring: str, section_name: str) -> str | None:
# ---------------------------------------------------------------------------

from ._forward import ( # noqa: E402
_ABSTRACT_DECORATORS, # noqa: F401 – re-exported for tests
_build_node_index,
_check_missing_other_parameters,
_check_missing_raises,
Expand All @@ -261,10 +263,12 @@ def _extract_section_content(docstring: str, section_name: str) -> str | None:
_check_missing_warns,
_check_missing_yields,
_extract_exception_name, # noqa: F401 – re-exported for submodules
_has_decorator,
_has_decorator, # noqa: F401 – re-exported for submodules
_is_abstract,
_is_meaningful_return, # noqa: F401 – re-exported for submodules
_is_property, # noqa: F401 – re-exported for submodules
_is_stub_function,
_is_stub_statement, # noqa: F401 – re-exported for tests
_is_warn_call, # noqa: F401 – re-exported for submodules
_NodeT,
)
Expand All @@ -277,16 +281,19 @@ def _should_skip_reverse_check(node: _NodeT) -> bool:
intended behaviour for implementers, not current implementation.
This aligns with ruff DOC202/403/502 and pydoclint skip logic.

Skips when the node is ``@abstractmethod`` or a stub function
(body is ``pass``, ``...``, or ``raise NotImplementedError``).
Skips when the node has any abstract decorator (``@abstractmethod``
and deprecated variants ``@abstractclassmethod``,
``@abstractstaticmethod``, ``@abstractproperty``) or is a stub
function (body is ``pass``, ``...``, ``raise NotImplementedError``,
or docstring-only).

Args:
node: The AST node for the symbol.

Returns:
``True`` when reverse checks should be skipped for this node.
"""
if _has_decorator(node, "abstractmethod"):
if _is_abstract(node):
return True
if _is_stub_function(node):
return True
Expand Down
98 changes: 73 additions & 25 deletions src/docvet/checks/enrichment/_forward.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
Detects missing docstring sections (Raises, Returns, Yields, Receives,
Warns, Other Parameters) by walking the function's AST subtree. Each
check function follows the uniform five-parameter dispatch signature
used by ``_RULE_DISPATCH`` in the enrichment orchestrator.
used by ``_RULE_DISPATCH`` in the enrichment orchestrator. Also provides
shared helpers for stub detection (``_is_stub_function``,
``_is_stub_statement``) and abstract decorator detection
(``_is_abstract``, ``_ABSTRACT_DECORATORS``).

See Also:
[`docvet.checks.enrichment`][]: Orchestrator and dispatch table.
Expand Down Expand Up @@ -222,14 +225,70 @@ def _has_decorator(node: _NodeT, name: str) -> bool:
return False


_ABSTRACT_DECORATORS = frozenset(
{
"abstractmethod",
"abstractclassmethod",
"abstractstaticmethod",
"abstractproperty",
}
)


def _is_abstract(node: _NodeT) -> bool:
"""Check whether a function has any abstract decorator.

Detects ``@abstractmethod`` and its deprecated predecessors
(``@abstractclassmethod``, ``@abstractstaticmethod``,
``@abstractproperty``). Aligned with ruff's ``is_abstract()``
in ``crates/ruff_python_semantic/src/analyze/visibility.rs``.

Args:
node: The function or class AST node to inspect.

Returns:
``True`` when the node has any abstract decorator.
"""
return any(_has_decorator(node, name) for name in _ABSTRACT_DECORATORS)


def _is_stub_statement(stmt: ast.stmt) -> bool:
"""Check whether a single AST statement is a stub-like statement.

Recognises ``pass``, ``...`` (Ellipsis), ``raise NotImplementedError``,
and string literals (extra docstrings). Aligned with ruff's ``is_stub()``
in ``crates/ruff_python_semantic/src/analyze/function_type.rs``.

Args:
stmt: The AST statement node to inspect.

Returns:
``True`` when the statement is a stub-like statement.
"""
if isinstance(stmt, ast.Pass):
return True
if isinstance(stmt, ast.Expr) and isinstance(stmt.value, ast.Constant):
# Ellipsis (...) or string literal (extra docstring).
return stmt.value.value is ... or isinstance(stmt.value.value, str)
if isinstance(stmt, ast.Raise) and isinstance(stmt.exc, (ast.Name, ast.Call)):
exc = stmt.exc.func if isinstance(stmt.exc, ast.Call) else stmt.exc
return isinstance(exc, ast.Name) and exc.id == "NotImplementedError"
return False


def _is_stub_function(node: _NodeT) -> bool:
"""Check whether a function body is a stub (no real implementation).

Stubs are single-statement bodies consisting of ``pass``,
``...`` (Ellipsis), or ``raise NotImplementedError``. A leading
docstring ``Expr(Constant(str))`` is stripped before evaluating
body length so the helper works for both documented and
undocumented functions.
Stubs are bodies consisting entirely of stub-like statements:
``pass``, ``...`` (Ellipsis), ``raise NotImplementedError``, or
string literals. A leading docstring ``Expr(Constant(str))`` is
stripped before evaluating so the helper works for both documented
and undocumented functions. A docstring-only body (empty after
stripping) is also a stub — this covers ``typing.Protocol`` methods
and interface contracts.

Multi-statement stub bodies (e.g. ``pass; ...``) are accepted,
aligning with ruff's ``is_stub()`` semantics.

Args:
node: The function AST node to inspect.
Expand All @@ -246,21 +305,9 @@ def _is_stub_function(node: _NodeT) -> bool:
and isinstance(body[0].value.value, str)
):
body = body[1:]
if len(body) != 1:
return False
stmt = body[0]
if isinstance(stmt, ast.Pass):
return True
if (
isinstance(stmt, ast.Expr)
and isinstance(stmt.value, ast.Constant)
and stmt.value.value is ...
):
return True
if isinstance(stmt, ast.Raise) and isinstance(stmt.exc, (ast.Name, ast.Call)):
exc = stmt.exc.func if isinstance(stmt.exc, ast.Call) else stmt.exc
return isinstance(exc, ast.Name) and exc.id == "NotImplementedError"
return False
if not body:
return True # Docstring-only body is a stub.
return all(_is_stub_statement(stmt) for stmt in body)


def _should_skip_returns_check(
Expand All @@ -273,8 +320,9 @@ def _should_skip_returns_check(
Centralises guard clauses for :func:`_check_missing_returns` to keep
the main function focused on AST walking. Skips when the symbol
already has a ``Returns:`` section, is a dunder method,
``@property``/``@cached_property``, ``@abstractmethod``,
``@overload``, or a stub function.
``@property``/``@cached_property``, any abstract decorator
(``@abstractmethod`` and deprecated variants), ``@overload``, or a
stub function.

Args:
symbol: The documented symbol to inspect.
Expand All @@ -292,8 +340,8 @@ def _should_skip_returns_check(
# @property and @cached_property methods
if _is_property(node):
return True
# @abstractmethod (interface contracts, not implementations)
if _has_decorator(node, "abstractmethod"):
# Abstract methods (interface contracts, not implementations)
if _is_abstract(node):
return True
# Stub functions (pass, ..., raise NotImplementedError)
if _is_stub_function(node):
Expand Down
162 changes: 162 additions & 0 deletions tests/unit/checks/test_missing_returns.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
from docvet.checks.enrichment import (
_build_node_index,
_check_missing_returns,
_is_abstract,
_is_stub_function,
_is_stub_statement,
_parse_sections,
check_enrichment,
)
Expand Down Expand Up @@ -645,3 +647,163 @@ def test_is_stub_function_documented_stub_with_raise():
func_node = tree.body[0]
assert isinstance(func_node, ast.FunctionDef)
assert _is_stub_function(func_node) is True


# ---------------------------------------------------------------------------
# _is_stub_function: docstring-only body (#387)
# ---------------------------------------------------------------------------


def test_is_stub_function_docstring_only_body():
"""Docstring-only body is a stub (Protocol methods, #387)."""
tree = ast.parse('def f():\n """Interface contract."""\n')
func_node = tree.body[0]
assert isinstance(func_node, ast.FunctionDef)
assert _is_stub_function(func_node) is True


def test_is_stub_function_docstring_only_method_in_protocol():
"""Protocol method with docstring-only body is a stub (#387)."""
source = (
"from typing import Protocol\n"
"class P(Protocol):\n"
" def compute(self, x: int) -> float:\n"
' """Compute a value.\n\n'
" Returns:\n"
" The computed float result.\n"
' """\n'
)
tree = ast.parse(source)
class_node = tree.body[1]
assert isinstance(class_node, ast.ClassDef)
func_node = class_node.body[0]
assert isinstance(func_node, ast.FunctionDef)
assert _is_stub_function(func_node) is True


# ---------------------------------------------------------------------------
# _is_stub_function: multi-statement stub bodies (#388)
# ---------------------------------------------------------------------------


def test_is_stub_function_multi_pass():
"""Multiple pass statements should be recognized as a stub (#388)."""
tree = ast.parse("def f():\n pass\n pass\n")
func_node = tree.body[0]
assert isinstance(func_node, ast.FunctionDef)
assert _is_stub_function(func_node) is True


def test_is_stub_function_pass_and_ellipsis():
"""Pass + ellipsis combination is a stub (#388)."""
tree = ast.parse("def f():\n pass\n ...\n")
func_node = tree.body[0]
assert isinstance(func_node, ast.FunctionDef)
assert _is_stub_function(func_node) is True


def test_is_stub_function_documented_multi_statement_stub():
"""Docstring + pass + ellipsis is a stub (#388)."""
tree = ast.parse('def f():\n """Doc."""\n pass\n ...\n')
func_node = tree.body[0]
assert isinstance(func_node, ast.FunctionDef)
assert _is_stub_function(func_node) is True


def test_is_stub_function_mixed_stub_and_real_not_stub():
"""Pass + real statement is NOT a stub."""
tree = ast.parse("def f():\n pass\n return 1\n")
func_node = tree.body[0]
assert isinstance(func_node, ast.FunctionDef)
assert _is_stub_function(func_node) is False


# ---------------------------------------------------------------------------
# _is_stub_statement helper tests
# ---------------------------------------------------------------------------


@pytest.mark.parametrize(
("code", "expected"),
[
("pass", True),
("...", True),
("raise NotImplementedError", True),
("raise NotImplementedError('msg')", True),
('"extra docstring"', True),
("x = 1", False),
("return 42", False),
],
ids=[
"pass",
"ellipsis",
"raise-NIE",
"raise-NIE-msg",
"string-literal",
"assign",
"return",
],
)
def test_is_stub_statement(code, expected):
"""Direct test of _is_stub_statement helper."""
tree = ast.parse(code)
stmt = tree.body[0]
assert _is_stub_statement(stmt) is expected


# ---------------------------------------------------------------------------
# _is_abstract helper tests (#389)
# ---------------------------------------------------------------------------


@pytest.mark.parametrize(
("decorator", "expected"),
[
("abstractmethod", True),
("abstractclassmethod", True),
("abstractstaticmethod", True),
("abstractproperty", True),
("staticmethod", False),
("classmethod", False),
("property", False),
],
ids=[
"abstractmethod",
"abstractclassmethod",
"abstractstaticmethod",
"abstractproperty",
"staticmethod",
"classmethod",
"property",
],
)
def test_is_abstract(decorator, expected):
"""_is_abstract detects all abstract decorators (#389)."""
tree = ast.parse(f"@{decorator}\ndef f():\n pass\n")
func_node = tree.body[0]
assert isinstance(func_node, ast.FunctionDef)
assert _is_abstract(func_node) is expected


def test_is_abstract_qualified_decorator():
"""_is_abstract detects abc.abstractmethod (qualified form)."""
tree = ast.parse("import abc\n@abc.abstractmethod\ndef f():\n pass\n")
func_node = tree.body[1]
assert isinstance(func_node, ast.FunctionDef)
assert _is_abstract(func_node) is True


def test_missing_returns_skips_abstractclassmethod():
"""missing-returns skips @abstractclassmethod (#389)."""
source = (
"import abc\n"
"class C(abc.ABC):\n"
" @abc.abstractclassmethod\n"
" def create(cls) -> 'C':\n"
' """Create instance."""\n'
" return cls()\n"
)
tree = ast.parse(source)
config = EnrichmentConfig()
findings = check_enrichment(source, tree, config, "test.py")
assert not any(f.rule == "missing-returns" for f in findings)
Loading
Loading