refactor: Be more strict when parsing sections in Google docstrings

Issue #204: #204

Author: pawamoy

docs/docstrings.md

@@ -46,9 +46,55 @@ section identifier: optional section title
 
 All sections identifiers are case-insensitive.
 All sections support multiple lines in descriptions,
-as well as blank lines.
+as well as blank lines. The first line must not be blank.
+Each section must be separated from contents above by a blank line.
 
-Some sections also support documenting multiple items.
+❌ This is **invalid** and will be parsed as regular markup:
+
+```python
+Some text.
+Note: # (1)!
+    Some information.
+
+    Blank lines allowed.
+```
+
+1. Missing blank line above.
+
+❌ This is **invalid** and will be parsed as regular markup:
+
+```python
+Some text.
+
+Note: # (1)!
+
+    Some information.
+
+    Blank lines allowed.
+```
+
+1. Extraneous blank line below.
+
+✅ This is **valid** and will parsed as a text section followed by a note admonition:
+
+```python
+Some text.
+
+Note:
+    Some information.
+
+    Blank lines allowed.
+```
+
+Find out possibly invalid section syntax by grepping for "reasons" in Griffe debug logs:
+
+```bash
+griffe dump -Ldebug -o/dev/null \
+    -fdgoogle -D '{"strict_sections": true}' \
+    your_package 2>&1 | grep reasons
+```
+
+Some sections support documenting multiple items (attributes, parameters, etc.).
 When multiple items are supported, each item description can
 use multiple lines, and continuation lines must be indented once
 more so that the parser is able to differentiate items.

src/griffe/docstrings/google.py

@@ -37,6 +37,7 @@
 )
 from griffe.docstrings.utils import parse_annotation, warning
 from griffe.expressions import ExprName
+from griffe.logger import LogLevel
 
 if TYPE_CHECKING:
     from typing import Any, Literal, Pattern
@@ -245,12 +246,7 @@ def _read_parameters_section(
     **options: Any,
 ) -> tuple[DocstringSectionParameters | None, int]:
     parameters, new_offset = _read_parameters(docstring, offset=offset, **options)
-
-    if parameters:
-        return DocstringSectionParameters(parameters), new_offset
-
-    _warn(docstring, new_offset, f"Empty parameters section at line {offset}")
-    return None, new_offset
+    return DocstringSectionParameters(parameters), new_offset
 
 
 def _read_other_parameters_section(
@@ -261,12 +257,7 @@ def _read_other_parameters_section(
     **options: Any,
 ) -> tuple[DocstringSectionOtherParameters | None, int]:
     parameters, new_offset = _read_parameters(docstring, offset=offset, warn_unknown_params=False, **options)
-
-    if parameters:
-        return DocstringSectionOtherParameters(parameters), new_offset
-
-    _warn(docstring, new_offset, f"Empty other parameters section at line {offset}")
-    return None, new_offset
+    return DocstringSectionOtherParameters(parameters), new_offset
 
 
 def _read_attributes_section(
@@ -302,11 +293,7 @@ def _read_attributes_section(
 
         attributes.append(DocstringAttribute(name=name, annotation=annotation, description=description))
 
-    if attributes:
-        return DocstringSectionAttributes(attributes), new_offset
-
-    _warn(docstring, new_offset, f"Empty attributes section at line {offset}")
-    return None, new_offset
+    return DocstringSectionAttributes(attributes), new_offset
 
 
 def _read_functions_section(
@@ -337,11 +324,7 @@ def _read_functions_section(
 
         functions.append(DocstringFunction(name=name, annotation=signature, description=description))
 
-    if functions:
-        return DocstringSectionFunctions(functions), new_offset
-
-    _warn(docstring, new_offset, f"Empty functions/methods section at line {offset}")
-    return None, new_offset
+    return DocstringSectionFunctions(functions), new_offset
 
 
 def _read_classes_section(
@@ -372,11 +355,7 @@ def _read_classes_section(
 
         classes.append(DocstringClass(name=name, annotation=signature, description=description))
 
-    if classes:
-        return DocstringSectionClasses(classes), new_offset
-
-    _warn(docstring, new_offset, f"Empty classes section at line {offset}")
-    return None, new_offset
+    return DocstringSectionClasses(classes), new_offset
 
 
 def _read_modules_section(
@@ -397,11 +376,7 @@ def _read_modules_section(
         description = "\n".join([description.lstrip(), *module_lines[1:]]).rstrip("\n")
         modules.append(DocstringModule(name=name, description=description))
 
-    if modules:
-        return DocstringSectionModules(modules), new_offset
-
-    _warn(docstring, new_offset, f"Empty modules section at line {offset}")
-    return None, new_offset
+    return DocstringSectionModules(modules), new_offset
 
 
 def _read_raises_section(
@@ -425,11 +400,7 @@ def _read_raises_section(
             annotation = parse_annotation(annotation, docstring)
             exceptions.append(DocstringRaise(annotation=annotation, description=description))
 
-    if exceptions:
-        return DocstringSectionRaises(exceptions), new_offset
-
-    _warn(docstring, new_offset, f"Empty exceptions section at line {offset}")
-    return None, new_offset
+    return DocstringSectionRaises(exceptions), new_offset
 
 
 def _read_warns_section(
@@ -450,11 +421,7 @@ def _read_warns_section(
             description = "\n".join([description.lstrip(), *warning_lines[1:]]).rstrip("\n")
             warns.append(DocstringWarn(annotation=annotation, description=description))
 
-    if warns:
-        return DocstringSectionWarns(warns), new_offset
-
-    _warn(docstring, new_offset, f"Empty warns section at line {offset}")
-    return None, new_offset
+    return DocstringSectionWarns(warns), new_offset
 
 
 def _read_returns_section(
@@ -516,11 +483,7 @@ def _read_returns_section(
 
         returns.append(DocstringReturn(name=name or "", annotation=annotation, description=description))
 
-    if returns:
-        return DocstringSectionReturns(returns), new_offset
-
-    _warn(docstring, new_offset, f"Empty returns section at line {offset}")
-    return None, new_offset
+    return DocstringSectionReturns(returns), new_offset
 
 
 def _read_yields_section(
@@ -567,11 +530,7 @@ def _read_yields_section(
 
         yields.append(DocstringYield(name=name or "", annotation=annotation, description=description))
 
-    if yields:
-        return DocstringSectionYields(yields), new_offset
-
-    _warn(docstring, new_offset, f"Empty yields section at line {offset}")
-    return None, new_offset
+    return DocstringSectionYields(yields), new_offset
 
 
 def _read_receives_section(
@@ -614,11 +573,7 @@ def _read_receives_section(
 
         receives.append(DocstringReceive(name=name or "", annotation=annotation, description=description))
 
-    if receives:
-        return DocstringSectionReceives(receives), new_offset
-
-    _warn(docstring, new_offset, f"Empty receives section at line {offset}")
-    return None, new_offset
+    return DocstringSectionReceives(receives), new_offset
 
 
 def _read_examples_section(
@@ -677,11 +632,7 @@ def _read_examples_section(
     elif current_example:
         sub_sections.append((DocstringSectionKind.examples, "\n".join(current_example)))
 
-    if sub_sections:
-        return DocstringSectionExamples(sub_sections), new_offset
-
-    _warn(docstring, new_offset, f"Empty examples section at line {offset}")
-    return None, new_offset
+    return DocstringSectionExamples(sub_sections), new_offset
 
 
 def _read_deprecated_section(
@@ -692,11 +643,6 @@ def _read_deprecated_section(
 ) -> tuple[DocstringSectionDeprecated | None, int]:
     text, new_offset = _read_block(docstring, offset=offset, **options)
 
-    # early exit if there is no text in the yield section
-    if not text:
-        _warn(docstring, new_offset, f"Empty deprecated section at line {offset}")
-        return None, new_offset
-
     # check the presence of a name and description, separated by a semi-colon
     try:
         version, text = text.split(":", 1)
@@ -733,6 +679,8 @@ def _is_empty_line(line: str) -> bool:
     DocstringSectionKind.deprecated: _read_deprecated_section,
 }
 
+_sentinel = object()
+
 
 def parse(
     docstring: Docstring,
@@ -800,7 +748,41 @@ def parse(
             groups = match.groupdict()
             title = groups["title"]
             admonition_type = groups["type"]
-            if admonition_type.lower() in _section_kind:
+            is_section = admonition_type.lower() in _section_kind
+
+            has_previous_line = offset > 0
+            blank_line_above = not has_previous_line or _is_empty_line(lines[offset - 1])
+            has_next_line = offset < len(lines) - 1
+            has_next_lines = offset < len(lines) - 2
+            blank_line_below = has_next_line and _is_empty_line(lines[offset + 1])
+            blank_lines_below = has_next_lines and _is_empty_line(lines[offset + 2])
+            indented_line_below = has_next_line and not blank_line_below and lines[offset + 1].startswith(" ")
+            indented_lines_below = has_next_lines and not blank_lines_below and lines[offset + 2].startswith(" ")
+            if not (indented_line_below or indented_lines_below):
+                # Do not warn when there are no contents,
+                # this is most probably not a section or admonition.
+                current_section.append(lines[offset])
+                offset += 1
+                continue
+            reasons = []
+            kind = "section" if is_section else "admonition"
+            if (indented_line_below or indented_lines_below) and not blank_line_above:
+                reasons.append(f"Missing blank line above {kind}")
+            if indented_lines_below and blank_line_below:
+                reasons.append(f"Extraneous blank line below {kind} title")
+            if reasons:
+                reasons_string = "; ".join(reasons)
+                _warn(
+                    docstring,
+                    offset,
+                    f"Possible {kind} skipped, reasons: {reasons_string}",
+                    LogLevel.debug,
+                )
+                current_section.append(lines[offset])
+                offset += 1
+                continue
+
+            if is_section:
                 if current_section:
                     if any(current_section):
                         sections.append(DocstringSectionText("\n".join(current_section).rstrip("\n")))

src/griffe/docstrings/utils.py

@@ -4,7 +4,7 @@
 
 from ast import PyCF_ONLY_AST
 from contextlib import suppress
-from typing import TYPE_CHECKING, Callable
+from typing import TYPE_CHECKING, Protocol
 
 from griffe.agents.nodes import safe_get_annotation
 from griffe.logger import LogLevel, get_logger
@@ -14,7 +14,12 @@
     from griffe.expressions import Expr
 
 
-def warning(name: str) -> Callable[[Docstring, int, str], None]:
+class WarningCallable(Protocol):
+    def __call__(self, docstring: Docstring, offset: int, message: str, log_level: LogLevel = ...) -> None:
+        ...
+
+
+def warning(name: str) -> WarningCallable:
     """Create and return a warn function.
 
     Parameters:
@@ -32,12 +37,13 @@ def warning(name: str) -> Callable[[Docstring, int, str], None]:
     """
     logger = get_logger(name)
 
-    def warn(docstring: Docstring, offset: int, message: str) -> None:
+    def warn(docstring: Docstring, offset: int, message: str, log_level: LogLevel = LogLevel.warning) -> None:
         try:
             prefix = docstring.parent.relative_filepath  # type: ignore[union-attr]
         except (AttributeError, ValueError):
             prefix = "<module>"
-        logger.warning(f"{prefix}:{(docstring.lineno or 0)+offset}: {message}")
+        log = getattr(logger, log_level.value)
+        log(f"{prefix}:{(docstring.lineno or 0)+offset}: {message}")
 
     return warn
 

tests/conftest.py

@@ -1,5 +1 @@
 """Configuration for the pytest test suite."""
-
-from __future__ import annotations
-
-pytest_plugins = ["griffe.tests"]

tests/test_docstrings/helpers.py

@@ -6,6 +6,7 @@
 
 from griffe.dataclasses import Attribute, Class, Docstring, Function, Module
 from griffe.docstrings.dataclasses import DocstringAttribute, DocstringElement, DocstringParameter, DocstringSection
+from griffe.logger import LogLevel
 
 if TYPE_CHECKING:
     from types import ModuleType
@@ -53,7 +54,7 @@ def parse(docstring: str, parent: ParentType | None = None, **parser_opts: Any)
             docstring_object.parent = parent
             parent.docstring = docstring_object
         warnings = []
-        parser_module._warn = lambda _docstring, _offset, message: warnings.append(message)  # type: ignore[attr-defined]
+        parser_module._warn = lambda _docstring, _offset, message, log_level=LogLevel.warning: warnings.append(message)  # type: ignore[attr-defined]
         sections = parser_module.parse(docstring_object, **parser_opts)
         return sections, warnings