Fix false positive DOC203 in property methods (#115)

Author: jsh9

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Change Log
 
+## [0.3.9] - 2024-01-16
+
+- Fixed
+
+  - False positive violation `DOC203` when there is no docstring return section
+    for methods with `@property` decorator
+
+- Full diff
+  - https://github.com/jsh9/pydoclint/compare/0.3.8...0.3.9
+
 ## [0.3.8] - 2023-10-20
 
 - Fixed

pydoclint/utils/generic.py

@@ -92,19 +92,6 @@ def detectMethodType(node: ast.FunctionDef) -> MethodType:
     return MethodType.INSTANCE_METHOD
 
 
-def checkIsAbstractMethod(node: ast.FunctionDef) -> bool:
-    """Check whether `node` is an abstract method"""
-    if len(node.decorator_list) == 0:
-        return False
-
-    for decorator in node.decorator_list:
-        if isinstance(decorator, ast.Name):
-            if decorator.id == 'abstractmethod':
-                return True
-
-    return False
-
-
 def getDocstring(node: ClassOrFunctionDef) -> str:
     """Get docstring from a class definition or a function definition"""
     docstring_: Optional[str] = ast.get_docstring(node)
@@ -153,17 +140,6 @@ def getNodeName(node: ast.AST) -> str:
     return node.name if 'name' in node.__dict__ else ''
 
 
-def isPropertyMethod(node: FuncOrAsyncFuncDef) -> bool:
-    """Check whether a function has `@property` as its last decorator"""
-    return (
-        isinstance(node.decorator_list, list)
-        and len(node.decorator_list) > 0
-        and isinstance(node.decorator_list[-1], ast.Name)
-        and hasattr(node.decorator_list[-1], 'id')
-        and node.decorator_list[-1].id == 'property'
-    )
-
-
 def stringStartsWith(string: str, substrings: Tuple[str, ...]) -> bool:
     """Check whether the string starts with any of the substrings"""
     for substring in substrings:

pydoclint/utils/special_methods.py

@@ -0,0 +1,32 @@
+import ast
+
+from pydoclint.utils.astTypes import FuncOrAsyncFuncDef
+
+
+def checkIsAbstractMethod(node: FuncOrAsyncFuncDef) -> bool:
+    """Check whether `node` is an abstract method"""
+    return checkMethodContainsSpecifiedDecorator(node, 'abstractmethod')
+
+
+def checkIsPropertyMethod(node: FuncOrAsyncFuncDef) -> bool:
+    """Check whether `node` is a method with @property decorator"""
+    return checkMethodContainsSpecifiedDecorator(node, 'property')
+
+
+def checkMethodContainsSpecifiedDecorator(
+        node: FuncOrAsyncFuncDef,
+        decorator: str,
+) -> bool:
+    """Check whether a method is decorated by the specified decorator"""
+    return (
+        isinstance(node.decorator_list, list)
+        and len(node.decorator_list) > 0
+        and any(
+            (  # noqa: PAR001
+                isinstance(_, ast.Name)
+                and hasattr(node.decorator_list[-1], 'id')
+                and _.id == decorator
+            )
+            for _ in node.decorator_list
+        )
+    )

pydoclint/visitor.py

@@ -6,12 +6,10 @@
 from pydoclint.utils.astTypes import FuncOrAsyncFuncDef
 from pydoclint.utils.doc import Doc
 from pydoclint.utils.generic import (
-    checkIsAbstractMethod,
     collectFuncArgs,
     detectMethodType,
     generateMsgPrefix,
     getDocstring,
-    isPropertyMethod,
 )
 from pydoclint.utils.internal_error import InternalError
 from pydoclint.utils.method_type import MethodType
@@ -27,6 +25,10 @@
     isReturnAnnotationNone,
     isReturnAnnotationNoReturn,
 )
+from pydoclint.utils.special_methods import (
+    checkIsAbstractMethod,
+    checkIsPropertyMethod,
+)
 from pydoclint.utils.violation import Violation
 from pydoclint.utils.visitor_helper import (
     checkReturnTypesForViolations,
@@ -485,11 +487,12 @@ def checkReturns(  # noqa: C901
         hasGenAsRetAnno: bool = hasGeneratorAsReturnAnnotation(node)
         onlyHasYieldStmt: bool = hasYieldStmt and not hasReturnStmt
         hasIterAsRetAnno: bool = hasIteratorOrIterableAsReturnAnnotation(node)
+        isPropertyMethod: bool = checkIsPropertyMethod(node)
 
         docstringHasReturnSection: bool = doc.hasReturnsSection
 
         violations: List[Violation] = []
-        if not docstringHasReturnSection and not isPropertyMethod(node):
+        if not docstringHasReturnSection and not isPropertyMethod:
             if (
                 # fmt: off
                 not (onlyHasYieldStmt and hasIterAsRetAnno)
@@ -541,6 +544,12 @@ def checkReturns(  # noqa: C901
                 # to check for DOC203 violations.
                 return violations
 
+            if returnSec == [] and isPropertyMethod:
+                # No need to check return type for methods with "@property"
+                # decorator. This is because it's OK for @property methods
+                # to have no return section in the docstring.
+                return violations
+
             checkReturnTypesForViolations(
                 style=self.style,
                 returnAnnotation=returnAnno,

setup.cfg

@@ -1,6 +1,6 @@
 [metadata]
 name = pydoclint
-version = 0.3.8
+version = 0.3.9
 description = A Python docstring linter that checks arguments, returns, yields, and raises sections
 long_description = file: README.md
 long_description_content_type = text/markdown

tests/data/common/property_method.py

@@ -0,0 +1,12 @@
+class MyClass:
+    data: float = 2.1
+
+    @property
+    def something(self) -> float:
+        """
+        Some property.
+
+        It's OK to have no return section in this method, because this
+        is a "property method" and is intended to be used as an attribute.
+        """
+        return self.data

tests/test_main.py

@@ -900,6 +900,16 @@ def testAbstractMethod(style: str, checkReturnTypes: bool) -> None:
     assert list(map(str, violations)) == expected
 
 
+@pytest.mark.parametrize('style', ['google', 'numpy', 'sphinx'])
+def testNoReturnSectionInPropertyMethod(style: str) -> None:
+    violations = _checkFile(
+        filename=DATA_DIR / 'common/property_method.py',
+        style=style,
+        skipCheckingShortDocstrings=False,
+    )
+    assert len(violations) == 0
+
+
 @pytest.mark.parametrize(
     'style, argTypeHintsInDocstring, argTypeHintsInSignature',
     itertools.product(

tests/utils/test_generic.py

@@ -1,13 +1,9 @@
 import ast
-from typing import List, Optional
+from typing import List
 
 import pytest
 
-from pydoclint.utils.generic import (
-    collectFuncArgs,
-    isPropertyMethod,
-    stripQuotes,
-)
+from pydoclint.utils.generic import collectFuncArgs, stripQuotes
 
 src1 = """
 def func1(
@@ -76,55 +72,6 @@ def testCollectFuncArgs(src: str, expected: List[str]) -> None:
     assert [_.arg for _ in out] == expected
 
 
-srcProperty1 = """
-class A:
-    def method1(self):
-        pass
-"""
-
-srcProperty2 = """
-class A:
-    @property
-    def method1(self):
-        pass
-"""
-
-srcProperty3 = """
-# pydoclint only does static code analysis in order to achieve fast speed.
-# If users rename built-in decorator names (such as `property`), pydoclint
-# will not recognize it.
-
-hello_world = property
-
-class A:
-    @hello_world
-    def method1(self):
-        pass
-"""
-
-
-@pytest.mark.parametrize(
-    'src, expected',
-    [
-        (srcProperty1, False),
-        (srcProperty2, True),
-        (srcProperty3, False),
-    ],
-)
-def testIsPropertyMethod(src: str, expected: bool) -> None:
-    def getMethod1(tree_: ast.AST) -> Optional[ast.FunctionDef]:
-        for node_ in ast.walk(tree_):
-            if isinstance(node_, ast.FunctionDef) and node_.name == 'method1':
-                return node_
-
-        return None
-
-    tree = ast.parse(src)
-    node = getMethod1(tree)
-    result = isPropertyMethod(node)
-    assert result == expected
-
-
 @pytest.mark.parametrize(
     'string, expected',
     [

tests/utils/test_special_methods.py

@@ -0,0 +1,72 @@
+import ast
+from typing import Optional
+
+import pytest
+
+from pydoclint.utils.special_methods import (
+    checkMethodContainsSpecifiedDecorator,
+)
+
+src1 = """
+class A:
+    def method1(self):
+        pass
+"""
+
+src2 = """
+class A:
+    @property
+    def method1(self):
+        pass
+"""
+
+src3 = """
+class A:
+    @hello
+    @world
+    @property
+    @morning
+    def method1(self):
+        pass
+"""
+
+src4 = """
+# pydoclint only does static code analysis in order to achieve fast speed.
+# If users rename built-in decorator names (such as `property`), pydoclint
+# will not recognize it.
+
+hello_world = property
+
+class A:
+    @hello_world
+    def method1(self):
+        pass
+"""
+
+
+@pytest.mark.parametrize(
+    'src, decorator, expected',
+    [
+        (src1, 'something', False),
+        (src2, 'property', True),
+        (src3, 'property', True),
+        (src4, 'hello_world', True),
+        (src4, 'property', False),
+    ],
+)
+def testCheckMethodContainsSpecifiedDecorator(
+        src: str,
+        decorator: str,
+        expected: bool,
+) -> None:
+    def getMethod1(tree_: ast.AST) -> Optional[ast.FunctionDef]:
+        for node_ in ast.walk(tree_):
+            if isinstance(node_, ast.FunctionDef) and node_.name == 'method1':
+                return node_
+
+        return None
+
+    tree = ast.parse(src)
+    node = getMethod1(tree)
+    result = checkMethodContainsSpecifiedDecorator(node, decorator=decorator)
+    assert result == expected