From cbf39764474529b03eea206801fb8f456a2da9b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Sun, 12 Dec 2021 22:14:03 +0100 Subject: Fix incorrect classification of property docstrings in Numpy-style (#5498) --- ChangeLog | 5 +++ doc/whatsnew/2.13.rst | 7 +++ pylint/extensions/_check_docs_utils.py | 52 +++++++++++++--------- pylint/extensions/docparams.py | 2 +- tests/extensions/test_check_docs.py | 36 --------------- .../docparams/raise/missing_raises_doc_Numpy.py | 28 ++++++++++++ .../docparams/raise/missing_raises_doc_Numpy.txt | 14 +++--- 7 files changed, 80 insertions(+), 64 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9bc848b59..738d8acda 100644 --- a/ChangeLog +++ b/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. diff --git a/doc/whatsnew/2.13.rst b/doc/whatsnew/2.13.rst index ec1ea0af5..001b6b976 100644 --- a/doc/whatsnew/2.13.rst +++ b/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 ============= diff --git a/pylint/extensions/_check_docs_utils.py b/pylint/extensions/_check_docs_utils.py index 27c2a755f..bfab0e1e4 100644 --- a/pylint/extensions/_check_docs_utils.py +++ b/pylint/extensions/_check_docs_utils.py @@ -172,7 +172,8 @@ 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, @@ -180,11 +181,11 @@ def docstringify(docstring, default_type="default"): 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 @@ class Docstring: 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): diff --git a/pylint/extensions/docparams.py b/pylint/extensions/docparams.py index 92e598c5d..e7b293988 100644 --- a/pylint/extensions/docparams.py +++ b/pylint/extensions/docparams.py @@ -307,7 +307,7 @@ class DocstringParameterChecker(BaseChecker): 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 diff --git a/tests/extensions/test_check_docs.py b/tests/extensions/test_check_docs.py index b0eeeb483..37a5e62bc 100644 --- a/tests/extensions/test_check_docs.py +++ b/tests/extensions/test_check_docs.py @@ -184,42 +184,6 @@ class TestParamDocChecker(CheckerTestCase): 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 diff --git a/tests/functional/ext/docparams/raise/missing_raises_doc_Numpy.py b/tests/functional/ext/docparams/raise/missing_raises_doc_Numpy.py index e4a9308bd..dc42940ea 100644 --- a/tests/functional/ext/docparams/raise/missing_raises_doc_Numpy.py +++ b/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() diff --git a/tests/functional/ext/docparams/raise/missing_raises_doc_Numpy.txt b/tests/functional/ext/docparams/raise/missing_raises_doc_Numpy.txt index b6573e38a..69dcffb21 100644 --- a/tests/functional/ext/docparams/raise/missing_raises_doc_Numpy.txt +++ b/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 -- cgit v1.2.1