Validate docstring examples (#101)

- Fix examples in docstrings
- Add pylint docstring validator

Author: Marenz

noxfile.py

@@ -33,6 +33,7 @@ def pylint(session: nox.Session) -> None:
         ".[docs]",
         "pylint",
         "pytest",
+        "sybil",
         "nox",
         "async-solipsism",
         "hypothesis",
@@ -95,6 +96,8 @@ def pytest(session: nox.Session) -> None:
         "pytest-asyncio",
         "async-solipsism",
         "hypothesis",
+        "sybil",
+        "pylint",
     )
     session.install("-e", ".")
     session.run(

src/conftest.py

@@ -0,0 +1,210 @@
+# License: MIT
+# Copyright © 2023 Frequenz Energy-as-a-Service GmbH
+
+"""Pytest plugin to validate docstring code examples.
+
+Code examples are often wrapped in triple backticks (```) within our docstrings.
+This plugin extracts these code examples and validates them using pylint.
+"""
+
+from __future__ import annotations
+
+import ast
+import os
+import subprocess
+from pathlib import Path
+
+from sybil import Example, Sybil
+from sybil.evaluators.python import pad
+from sybil.parsers.abstract.lexers import textwrap
+from sybil.parsers.myst import CodeBlockParser
+
+PYLINT_DISABLE_COMMENT = (
+    "# pylint: {}=unused-import,wildcard-import,unused-wildcard-import"
+)
+
+FORMAT_STRING = """
+# Generated auto-imports for code example
+{disable_pylint}
+{imports}
+{enable_pylint}
+
+{code}"""
+
+
+def get_import_statements(code: str) -> list[str]:
+    """Get all import statements from a given code string.
+
+    Args:
+        code: The code to extract import statements from.
+
+    Returns:
+        A list of import statements.
+    """
+    tree = ast.parse(code)
+    import_statements = []
+
+    for node in ast.walk(tree):
+        if isinstance(node, (ast.Import, ast.ImportFrom)):
+            import_statement = ast.get_source_segment(code, node)
+            import_statements.append(import_statement)
+
+    return import_statements
+
+
+def path_to_import_statement(path: Path) -> str:
+    """Convert a path to a Python file to an import statement.
+
+    Args:
+        path: The path to convert.
+
+    Returns:
+        The import statement.
+
+    Raises:
+        ValueError: If the path does not point to a Python file.
+    """
+    # Make the path relative to the present working directory
+    if path.is_absolute():
+        path = path.relative_to(Path.cwd())
+
+    # Check if the path is a Python file
+    if path.suffix != ".py":
+        raise ValueError("Path must point to a Python file (.py)")
+
+    # Remove 'src' prefix if present
+    parts = path.parts
+    if parts[0] == "src":
+        parts = parts[1:]
+
+    # Remove the '.py' extension and join parts with '.'
+    module_path = ".".join(parts)[:-3]
+
+    # Create the import statement
+    import_statement = f"from {module_path} import *"
+    return import_statement
+
+
+class CustomPythonCodeBlockParser(CodeBlockParser):
+    """Code block parser that validates extracted code examples using pylint.
+
+    This parser is a modified version of the default Python code block parser
+    from the Sybil library.
+    It uses pylint to validate the extracted code examples.
+
+    All code examples are preceded by the original file's import statements as
+    well as an wildcard import of the file itself.
+    This allows us to use the code examples as if they were part of the original
+    file.
+
+    Additionally, the code example is padded with empty lines to make sure the
+    line numbers are correct.
+
+    Pylint warnings which are unimportant for code examples are disabled.
+    """
+
+    def __init__(self):
+        """Initialize the parser."""
+        super().__init__("python")
+
+    def evaluate(self, example: Example) -> None | str:
+        """Validate the extracted code example using pylint.
+
+        Args:
+            example: The extracted code example.
+
+        Returns:
+            None if the code example is valid, otherwise the pylint output.
+        """
+        # Get the import statements for the original file
+        import_header = get_import_statements(example.document.text)
+        # Add a wildcard import of the original file
+        import_header.append(
+            path_to_import_statement(Path(os.path.relpath(example.path)))
+        )
+        imports_code = "\n".join(import_header)
+
+        # Dedent the code example
+        # There is also example.parsed that is already prepared, but it has
+        # empty lines stripped and thus fucks up the line numbers.
+        example_code = textwrap.dedent(
+            example.document.text[example.start : example.end]
+        )
+        # Remove first line (the line with the triple backticks)
+        example_code = example_code[example_code.find("\n") + 1 :]
+
+        example_with_imports = FORMAT_STRING.format(
+            disable_pylint=PYLINT_DISABLE_COMMENT.format("disable"),
+            imports=imports_code,
+            enable_pylint=PYLINT_DISABLE_COMMENT.format("enable"),
+            code=example_code,
+        )
+
+        # Make sure the line numbers are correct
+        source = pad(
+            example_with_imports,
+            example.line - imports_code.count("\n") - FORMAT_STRING.count("\n"),
+        )
+
+        # pylint disable parameters
+        pylint_disable_params = [
+            "missing-module-docstring",
+            "missing-class-docstring",
+            "missing-function-docstring",
+            "reimported",
+            "unused-variable",
+            "no-name-in-module",
+            "await-outside-async",
+        ]
+
+        response = validate_with_pylint(source, example.path, pylint_disable_params)
+
+        if len(response) > 0:
+            response_concats = "\n".join(response)
+            return (
+                f"Pylint validation failed for code example:\n"
+                f"{example_with_imports}\nOutput: {response_concats}"
+            )
+
+        return None
+
+
+def validate_with_pylint(
+    code_example: str, path: str, disable_params: list[str]
+) -> list[str]:
+    """Validate a code example using pylint.
+
+    Args:
+        code_example: The code example to validate.
+        path: The path to the original file.
+        disable_params: The pylint disable parameters.
+
+    Returns:
+        A list of pylint messages.
+    """
+    try:
+        pylint_command = [
+            "pylint",
+            "--disable",
+            ",".join(disable_params),
+            "--from-stdin",
+            path,
+        ]
+
+        subprocess.run(
+            pylint_command,
+            input=code_example,
+            text=True,
+            capture_output=True,
+            check=True,
+        )
+    except subprocess.CalledProcessError as exception:
+        return exception.output.splitlines()
+
+    return []
+
+
+pytest_collect_file = Sybil(
+    parsers=[CustomPythonCodeBlockParser()],
+    patterns=["*.py"],
+).pytest()

src/frequenz/channels/util/_merge.py

@@ -20,6 +20,13 @@ class Merge(Receiver[T]):
         stream, by using `Merge` like this:
 
         ```python
+        from frequenz.channels import Broadcast
+
+        channel1 = Broadcast[int]("input-chan-1")
+        channel2 = Broadcast[int]("input-chan-2")
+        receiver1 = channel1.new_receiver()
+        receiver2 = channel2.new_receiver()
+
         merge = Merge(receiver1, receiver2)
         while msg := await merge.receive():
             # do something with msg

src/frequenz/channels/util/_select.py

@@ -72,6 +72,13 @@ class Select:
         simultaneously wait on, this can be done with:
 
         ```python
+        from frequenz.channels import Broadcast
+
+        channel1 = Broadcast[int]("input-chan-1")
+        channel2 = Broadcast[int]("input-chan-2")
+        receiver1 = channel1.new_receiver()
+        receiver2 = channel2.new_receiver()
+
         select = Select(name1 = receiver1, name2 = receiver2)
         while await select.ready():
             if msg := select.name1:

src/frequenz/channels/util/_timer.py

@@ -287,43 +287,61 @@ class Timer(Receiver[timedelta]):
         with other receivers, and even start it (semi) manually:
 
         ```python
+        import logging
+        from frequenz.channels.util import Select
+        from frequenz.channels import Broadcast
+
+        timer = Timer.timeout(timedelta(seconds=1.0), auto_start=False)
+        chan = Broadcast[int]("input-chan")
+        receiver1 = chan.new_receiver()
+
         timer = Timer.timeout(timedelta(seconds=1.0), auto_start=False)
         # Do some other initialization, the timer will start automatically if
         # a message is awaited (or manually via `reset()`).
         select = Select(bat_1=receiver1, timer=timer)
         while await select.ready():
             if msg := select.bat_1:
                 if val := msg.inner:
-                    process_data(val)
+                    battery_soc = val
                 else:
-                    logging.warn("battery channel closed")
+                    logging.warning("battery channel closed")
             elif drift := select.timer:
                 # Print some regular battery data
-                print(f"Battery is charged at {battery.soc}%")
-                if stop_logging:
-                    timer.stop()
-                elif start_logging:
-                    timer.reset()
+                print(f"Battery is charged at {battery_soc}%")
         ```
 
     Example: Timeout example
         ```python
+        import logging
+        from frequenz.channels.util import Select
+        from frequenz.channels import Broadcast
+
+        def process_data(data: int):
+            logging.info("Processing data: %d", data)
+
+        def do_heavy_processing(data: int):
+            logging.info("Heavy processing data: %d", data)
+
         timer = Timer.timeout(timedelta(seconds=1.0), auto_start=False)
+        chan1 = Broadcast[int]("input-chan-1")
+        chan2 = Broadcast[int]("input-chan-2")
+        receiver1 = chan1.new_receiver()
+        receiver2 = chan2.new_receiver()
         select = Select(bat_1=receiver1, heavy_process=receiver2, timeout=timer)
         while await select.ready():
             if msg := select.bat_1:
                 if val := msg.inner:
                     process_data(val)
                     timer.reset()
                 else:
-                    logging.warn("battery channel closed")
+                    logging.warning("battery channel closed")
             if msg := select.heavy_process:
                 if val := msg.inner:
                     do_heavy_processing(val)
                 else:
-                    logging.warn("processing channel closed")
+                    logging.warning("processing channel closed")
             elif drift := select.timeout:
-                logging.warn("No data received in time")
+                logging.warning("No data received in time")
         ```
 
         In this case `do_heavy_processing` might take 2 seconds, and we don't