Fix incorrect classification of property docstrings in Numpy-style (#5498)

Author: DanielNoord

ChangeLog

@@ -42,6 +42,11 @@ Release date: TBA
 
   Closes #5323
 
+* Fixed incorrect classification of Numpy-style docstring as Google-style docstring for
+  docstrings with property setter documentation.
+  Docstring classification is now based on the highest amount of matched sections instead
+  of the order in which the docstring styles were tried.
+
 * Fixed detection of ``arguments-differ`` when superclass static
   methods lacked a ``@staticmethod`` decorator.
 

doc/whatsnew/2.13.rst

@@ -19,6 +19,13 @@ Extensions
 
 * Pyreverse - add output in mermaid-js format and html which is an mermaid js diagram with html boilerplate
 
+* ``DocstringParameterChecker``
+
+  * Fixed incorrect classification of Numpy-style docstring as Google-style docstring for
+    docstrings with property setter documentation.
+    Docstring classification is now based on the highest amount of matched sections instead
+    of the order in which the docstring styles were tried.
+
 Other Changes
 =============
 

pylint/extensions/_check_docs_utils.py

@@ -172,19 +172,20 @@ def possible_exc_types(node):
         return set()
 
 
-def docstringify(docstring, default_type="default"):
+def docstringify(docstring: str, default_type: str = "default") -> "Docstring":
+    best_match = (0, DOCSTRING_TYPES.get(default_type, Docstring)(docstring))
     for docstring_type in (
         SphinxDocstring,
         EpytextDocstring,
         GoogleDocstring,
         NumpyDocstring,
     ):
         instance = docstring_type(docstring)
-        if instance.is_valid():
-            return instance
+        matching_sections = instance.matching_sections()
+        if matching_sections > best_match[0]:
+            best_match = (matching_sections, instance)
 
-    docstring_type = DOCSTRING_TYPES.get(default_type, Docstring)
-    return docstring_type(docstring)
+    return best_match[1]
 
 
 class Docstring:
@@ -210,8 +211,9 @@ def __init__(self, doc):
     def __repr__(self) -> str:
         return f"<{self.__class__.__name__}:'''{self.doc}'''>"
 
-    def is_valid(self):
-        return False
+    def matching_sections(self) -> int:
+        """Returns the number of matching docstring sections"""
+        return 0
 
     def exceptions(self):
         return set()
@@ -322,13 +324,17 @@ class SphinxDocstring(Docstring):
 
     supports_yields = False
 
-    def is_valid(self):
-        return bool(
-            self.re_param_in_docstring.search(self.doc)
-            or self.re_raise_in_docstring.search(self.doc)
-            or self.re_rtype_in_docstring.search(self.doc)
-            or self.re_returns_in_docstring.search(self.doc)
-            or self.re_property_type_in_docstring.search(self.doc)
+    def matching_sections(self) -> int:
+        """Returns the number of matching docstring sections"""
+        return sum(
+            bool(i)
+            for i in (
+                self.re_param_in_docstring.search(self.doc),
+                self.re_raise_in_docstring.search(self.doc),
+                self.re_rtype_in_docstring.search(self.doc),
+                self.re_returns_in_docstring.search(self.doc),
+                self.re_property_type_in_docstring.search(self.doc),
+            )
         )
 
     def exceptions(self):
@@ -526,13 +532,17 @@ class GoogleDocstring(Docstring):
 
     supports_yields = True
 
-    def is_valid(self):
-        return bool(
-            self.re_param_section.search(self.doc)
-            or self.re_raise_section.search(self.doc)
-            or self.re_returns_section.search(self.doc)
-            or self.re_yields_section.search(self.doc)
-            or self.re_property_returns_line.search(self._first_line())
+    def matching_sections(self) -> int:
+        """Returns the number of matching docstring sections"""
+        return sum(
+            bool(i)
+            for i in (
+                self.re_param_section.search(self.doc),
+                self.re_raise_section.search(self.doc),
+                self.re_returns_section.search(self.doc),
+                self.re_yields_section.search(self.doc),
+                self.re_property_returns_line.search(self._first_line()),
+            )
         )
 
     def has_params(self):

pylint/extensions/docparams.py

@@ -307,7 +307,7 @@ def visit_raise(self, node: nodes.Raise) -> None:
                 func_node = property_
 
         doc = utils.docstringify(func_node.doc, self.config.default_docstring_type)
-        if not doc.is_valid():
+        if not doc.matching_sections():
             if doc.doc:
                 self._handle_no_raise_doc(expected_excs, func_node)
             return

tests/extensions/test_check_docs.py

@@ -184,42 +184,6 @@ def do_something(): #@
         with self.assertNoMessages():
             self.checker.visit_functiondef(node)
 
-    def test_finds_missing_raises_from_setter_numpy(self) -> None:
-        """Example of a setter having missing raises documentation in
-        the Numpy style docstring of the property
-        """
-        property_node, node = astroid.extract_node(
-            """
-        class Foo(object):
-            @property
-            def foo(self): #@
-                '''int: docstring
-
-                Include a "Raises" section so that this is identified
-                as a Numpy docstring and not a Google docstring.
-
-                Raises
-                ------
-                RuntimeError
-                    Always
-                '''
-                raise RuntimeError()
-                return 10
-
-            @foo.setter
-            def foo(self, value):
-                raise AttributeError() #@
-        """
-        )
-        with self.assertAddsMessages(
-            MessageTest(
-                msg_id="missing-raises-doc",
-                node=property_node,
-                args=("AttributeError",),
-            )
-        ):
-            self.checker.visit_raise(node)
-
     def test_finds_missing_raises_from_setter_numpy_2(self) -> None:
         """Example of a setter having missing raises documentation in
         its own Numpy style docstring of the property

tests/functional/ext/docparams/raise/missing_raises_doc_Numpy.py

@@ -1,6 +1,7 @@
 """Tests for missing-raises-doc and missing-raises-type-doc for Numpy style docstrings"""
 # pylint: disable=function-redefined, invalid-name, undefined-variable, missing-function-docstring
 # pylint: disable=unused-argument, try-except-raise, import-outside-toplevel
+# pylint: disable=too-few-public-methods, disallowed-name
 
 
 def test_find_missing_numpy_raises(self):  # [missing-raises-doc]
@@ -129,3 +130,30 @@ def test_find_invalid_missing_numpy_attr_raises(self):
     from re import error
 
     raise error("hi")
+
+
+class Foo:
+    """test_finds_missing_raises_from_setter_numpy
+    Example of a setter having missing raises documentation in
+    the Numpy style docstring of the property
+    """
+
+    @property
+    def foo(self):  # [missing-raises-doc]
+        """int: docstring
+
+        Include a "Raises" section so that this is identified
+        as a Numpy docstring and not a Google docstring.
+
+        Raises
+        ------
+        RuntimeError
+            Always
+        """
+        raise RuntimeError()
+        return 10  # [unreachable]
+
+    @foo.setter
+    def foo(self, value):
+        print(self)
+        raise AttributeError()

tests/functional/ext/docparams/raise/missing_raises_doc_Numpy.txt

@@ -1,6 +1,8 @@
-missing-raises-doc:6:0:15:25:test_find_missing_numpy_raises:"""RuntimeError"" not documented as being raised":UNDEFINED
-unreachable:15:4:15:25:test_find_missing_numpy_raises:Unreachable code:UNDEFINED
-unreachable:29:4:29:25:test_find_all_numpy_raises:Unreachable code:UNDEFINED
-missing-raises-doc:32:0:45:25:test_find_rethrown_numpy_raises:"""RuntimeError"" not documented as being raised":UNDEFINED
-missing-raises-doc:48:0:61:25:test_find_rethrown_numpy_multiple_raises:"""RuntimeError, ValueError"" not documented as being raised":UNDEFINED
-missing-raises-doc:106:0:116:21:test_find_valid_missing_numpy_attr_raises:"""error"" not documented as being raised":UNDEFINED
+missing-raises-doc:7:0:16:25:test_find_missing_numpy_raises:"""RuntimeError"" not documented as being raised":UNDEFINED
+unreachable:16:4:16:25:test_find_missing_numpy_raises:Unreachable code:UNDEFINED
+unreachable:30:4:30:25:test_find_all_numpy_raises:Unreachable code:UNDEFINED
+missing-raises-doc:33:0:46:25:test_find_rethrown_numpy_raises:"""RuntimeError"" not documented as being raised":UNDEFINED
+missing-raises-doc:49:0:62:25:test_find_rethrown_numpy_multiple_raises:"""RuntimeError, ValueError"" not documented as being raised":UNDEFINED
+missing-raises-doc:107:0:117:21:test_find_valid_missing_numpy_attr_raises:"""error"" not documented as being raised":UNDEFINED
+missing-raises-doc:142:4:154:17:Foo.foo:"""AttributeError"" not documented as being raised":UNDEFINED
+unreachable:154:8:154:17:Foo.foo:Unreachable code:UNDEFINED