Constructor checking for AST validator (#622)

* fix(validate.py): Considers subclass nesting when checking GL08 constructor

Introduced GL08 checking for constructors (i.e. __init__) but didn't traverse the parent class heireacy when importing the __init__ parent from a module.

* test(validate.py): Added a test to check nested class docstring when checking for initialisation docstring

Test written due to bug missed in #592.

* fix(validate.py): Allows the validator to check AST constructor docstrings compliance with parent class

The abstract syntax tree doesn't provide any link to a parent class at node level, so special traversal is required to check constructor parent implementation of docstrings.

* test(test_validate_hook.py,-example_module.py): Wrote new example_module tests for AST (AbstractSyntaxTree) discovery of constructor method parent class.

* ci(test.yml): Added --pre option to prerelease job to ensure pre-release installation, and changed pytest invocation to use `numpydoc` instead of `.`, for consistency with `test` job

In response to discussion on #591

* refactor(tests): Remove `__init__.py` module status of `tests\hooks\` to match `tests\` directory

* ci(test.yml): Added explicit call to hook tests to see if included in pytest execution

* test(tests\hooks\test_validate_hook.py): Changed constructor validation to use AST ancestry, fixed hook tests and added Windows support for test_utils

* ci(test.yml): Added file existance check for hook tests

Workflows do not seem to be running hook pytests.

* ci(test.yml): Correct the workflow task name/version

* ci(test.yml): Added explicit pytest call to the hooks directory

* ci(test.yml): Removed file existance test, after explicit call to hooks verifies execution

* fix(validate.py): switched conditional GL08 check order to avoid property class failure for Validator.name

Solves Issue #638, though lacks property support for property class attributes.

* test(test_validate.py): add coverage for existing / expected functionality of property and dataclass objects

* test(test_validate_hook.py): modify test strings and remove unused os.sep replacement

Updates to make this PR compatible with the recent merge from #653

Author: mattgebert

.github/workflows/test.yml

@@ -48,7 +48,7 @@ jobs:
 
       - name: Run test suite
         run: |
-          pytest -v --pyargs numpydoc
+          pytest -v --pyargs .
 
       - name: Test coverage
         run: |

numpydoc/tests/hooks/__init__.py

@@ -28,4 +28,35 @@ def create(self):
 
 
 class NewClass:
-    pass
+    class GoodConstructor:
+        """
+        A nested class to test constructors via AST hook.
+
+        Implements constructor via class docstring.
+
+        Parameters
+        ----------
+        name : str
+            The name of the new class.
+        """
+
+        def __init__(self, name):
+            self.name = name
+
+    class BadConstructor:
+        """
+        A nested class to test constructors via AST hook.
+
+        Implements a bad constructor docstring despite having a good class docstring.
+
+        Parameters
+        ----------
+        name : str
+            The name of the new class.
+        """
+
+        def __init__(self, name):
+            """
+            A failing constructor implementation without parameters.
+            """
+            self.name = name

numpydoc/tests/hooks/example_module.py

@@ -62,6 +62,22 @@ def test_validate_hook(example_module, config, capsys):
         {example_module!s}:26: EX01 No examples section found
 
         {example_module!s}:30: GL08 The object does not have a docstring
+
+        {example_module!s}:31: SA01 See Also section not found
+
+        {example_module!s}:31: EX01 No examples section found
+
+        {example_module!s}:46: SA01 See Also section not found
+
+        {example_module!s}:46: EX01 No examples section found
+
+        {example_module!s}:58: ES01 No extended summary found
+
+        {example_module!s}:58: PR01 Parameters {{'name'}} not documented
+
+        {example_module!s}:58: SA01 See Also section not found
+
+        {example_module!s}:58: EX01 No examples section found
         """
     )
 
@@ -89,6 +105,8 @@ def test_validate_hook_with_ignore(example_module, capsys):
         {example_module!s}:26: SS05 Summary must start with infinitive verb, not third person (e.g. use "Generate" instead of "Generates")
 
         {example_module!s}:30: GL08 The object does not have a docstring
+
+        {example_module!s}:58: PR01 Parameters {{'name'}} not documented
         """
     )
 

numpydoc/tests/hooks/test_validate_hook.py

@@ -1,5 +1,6 @@
 import warnings
 from contextlib import nullcontext
+from dataclasses import dataclass
 from functools import cached_property, partial, wraps
 from inspect import getsourcefile, getsourcelines
 
@@ -1305,6 +1306,109 @@ def __init__(self, param1: int):
         pass
 
 
+class ConstructorDocumentedinEmbeddedClass:  # ignore Gl08, ES01
+    """
+    Class to test the initialisation behaviour of a embedded class.
+    """
+
+    class EmbeddedClass1:  # ignore GL08, ES01
+        """
+        An additional level for embedded class documentation checking.
+        """
+
+        class EmbeddedClass2:
+            """
+            This is an embedded class.
+
+            Extended summary.
+
+            Parameters
+            ----------
+            param1 : int
+                Description of param1.
+
+            See Also
+            --------
+            otherclass : A class that does something else.
+
+            Examples
+            --------
+            This is an example of how to use EmbeddedClass.
+            """
+
+            def __init__(self, param1: int) -> None:
+                pass
+
+
+class IncompleteConstructorDocumentedinEmbeddedClass:
+    """
+    Class to test the initialisation behaviour of a embedded class.
+    """
+
+    class EmbeddedClass1:
+        """
+        An additional level for embedded class documentation checking.
+        """
+
+        class EmbeddedClass2:
+            """
+            This is an embedded class.
+
+            Extended summary.
+
+            See Also
+            --------
+            otherclass : A class that does something else.
+
+            Examples
+            --------
+            This is an example of how to use EmbeddedClass.
+            """
+
+            def __init__(self, param1: int) -> None:
+                pass
+
+
+@dataclass
+class DataclassWithDocstring:
+    """
+    A class decorated by `dataclass`.
+
+    To check the functionality of `dataclass` objects do not break the Validator.
+    As param1 is not documented this class should also raise PR01.
+    """
+
+    param1: int
+
+
+class ClassWithPropertyObject:
+    """
+    A class with a `property`.
+
+    To check the functionality of `property` objects do not break the Validator.
+
+    Parameters
+    ----------
+    param1 : int
+        Description of param1.
+    """
+
+    def __init__(self, param1: int) -> None:
+        self._param1 = param1
+
+    @property
+    def param1(self) -> int:
+        """
+        Get the value of param1.
+
+        Returns
+        -------
+        int
+            The value of param1.
+        """
+        return self._param1
+
+
 class TestValidator:
     def _import_path(self, klass=None, func=None):
         """
@@ -1657,6 +1761,18 @@ def test_bad_docstrings(self, capsys, klass, func, msgs):
                 tuple(),
                 ("PR01"),  # Parameter not documented in class constructor
             ),
+            (
+                "ConstructorDocumentedinEmbeddedClass.EmbeddedClass1.EmbeddedClass2",
+                tuple(),
+                ("GL08",),
+                tuple(),
+            ),
+            (
+                "IncompleteConstructorDocumentedinEmbeddedClass.EmbeddedClass1.EmbeddedClass2",
+                ("GL08",),
+                tuple(),
+                ("PR01",),
+            ),
         ],
     )
     def test_constructor_docstrings(
@@ -1674,6 +1790,39 @@ def test_constructor_docstrings(
         for code in exc_init_codes:
             assert code not in " ".join(err[0] for err in result["errors"])
 
+        if klass == "ConstructorDocumentedinEmbeddedClass":
+            raise NotImplementedError(
+                "Test for embedded class constructor docstring not implemented yet."
+            )
+
+    def test_dataclass_object(self):
+        # Test validator methods complete execution on dataclass objects and methods
+        # Test case ought to be removed if dataclass objects properly supported.
+        result = validate_one(self._import_path(klass="DataclassWithDocstring"))
+        # Check codes match as expected for dataclass objects.
+        errs = ["ES01", "SA01", "EX01", "PR01"]
+        for error in result["errors"]:
+            assert error[0] in errs
+            errs.remove(error[0])
+
+        # Test initialisation method (usually undocumented in dataclass) raises any errors.
+        init_fn = self._import_path(klass="DataclassWithDocstring", func="__init__")
+        result = validate_one(init_fn)
+        # Check that __init__ raises GL08 when the class docstring doesn't document params.
+        assert result["errors"][0][0] == "GL08"
+
+    def test_property_object(self):
+        # Test validator methods complete execution on class property objects
+        # Test case ought to be removed if property objects properly supported.
+        result = validate_one(
+            self._import_path(klass="ClassWithPropertyObject", func="param1")
+        )
+        # Check codes match as expected for property objects.
+        errs = ["ES01", "SA01", "EX01"]
+        for error in result["errors"]:
+            assert error[0] in errs
+            errs.remove(error[0])
+
 
 def decorator(x):
     """Test decorator."""

numpydoc/tests/test_validate.py

@@ -572,6 +572,14 @@ def _check_desc(desc, code_no_desc, code_no_upper, code_no_period, **kwargs):
     return errs
 
 
+def _find_class_node(module_node: ast.AST, cls_name) -> ast.ClassDef:
+    #  Find the class node within a module, when checking constructor docstrings.
+    for node in ast.walk(module_node):
+        if isinstance(node, ast.ClassDef) and node.name == cls_name:
+            return node
+    raise ValueError(f"Could not find class node {cls_name}")
+
+
 def validate(obj_name, validator_cls=None, **validator_kwargs):
     """
     Validate the docstring.
@@ -639,20 +647,49 @@ def validate(obj_name, validator_cls=None, **validator_kwargs):
         report_GL08: bool = True
         # Check if the object is a class and has a docstring in the constructor
         # Also check if code_obj is defined, as undefined for the AstValidator in validate_docstrings.py.
-        if (
-            doc.name.endswith(".__init__")
-            and doc.is_function_or_method
-            and hasattr(doc, "code_obj")
-        ):
-            cls_name = doc.code_obj.__qualname__.split(".")[0]
-            cls = Validator._load_obj(f"{doc.code_obj.__module__}.{cls_name}")
-            # cls = Validator._load_obj(f"{doc.name[:-9]}.{cls_name}") ## Alternative
-            cls_doc = Validator(get_doc_object(cls))
+        if doc.is_function_or_method and doc.name.endswith(".__init__"):
+            # Import here at runtime to avoid circular import as
+            # AstValidator is a subclass of Validator class without `doc_obj` attribute.
+            from numpydoc.hooks.validate_docstrings import (
+                AstValidator,  # Support abstract syntax tree hook.
+            )
+
+            if hasattr(doc, "code_obj"):  # All Validator objects have this attr.
+                cls_name = ".".join(
+                    doc.code_obj.__qualname__.split(".")[:-1]
+                )  # Collect all class depths before the constructor.
+                cls = Validator._load_obj(f"{doc.code_obj.__module__}.{cls_name}")
+                # cls = Validator._load_obj(f"{doc.name[:-9]}.{cls_name}") ## Alternative
+                cls_doc = Validator(get_doc_object(cls))
+            elif isinstance(doc, AstValidator):  # Supports class traversal for ASTs.
+                ancestry = doc.ancestry
+                if len(ancestry) > 2:  # e.g. module.class.__init__
+                    parent = doc.ancestry[-1]  # Get the parent
+                    cls_name = ".".join(
+                        [
+                            getattr(node, "name", node.__module__)
+                            for node in doc.ancestry
+                        ]
+                    )
+                    cls_doc = AstValidator(
+                        ast_node=parent,
+                        filename=doc.source_file_name,
+                        obj_name=cls_name,
+                        ancestry=doc.ancestry[:-1],
+                    )
+                else:
+                    # Ignore edge case: __init__ functions that don't belong to a class.
+                    cls_doc = None
+            else:
+                raise TypeError(
+                    f"Cannot load {doc.name} as a usable Validator object (Validator does not have `doc_obj` attr or type `AstValidator`)."
+                )
 
             # Parameter_mismatches, PR01, PR02, PR03 are checked for the class docstring.
             # If cls_doc has PR01, PR02, PR03 errors, i.e. invalid class docstring,
             # then we also report missing constructor docstring, GL08.
-            report_GL08 = len(cls_doc.parameter_mismatches) > 0
+            if cls_doc:
+                report_GL08 = len(cls_doc.parameter_mismatches) > 0
 
         # Check if GL08 is to be ignored:
         if "GL08" in ignore_validation_comments: