Fix raise/return detection for nested functions

CHANGELOG.md

@@ -1,10 +1,14 @@
 # Change Log
 
-## Unreleased
+## [0.0.15] - 2023-07-10
 
 - Fixed
   - Fixed false positive `DOC402` when `yield` statements are in a block within
-    a child function
+    a nested function
+  - Fixed false positives when `return` and `raise` statements are in a block
+    within a nested function
+- Full diff
+  - https://github.com/jsh9/pydoclint/compare/0.0.15...0.0.15
 
 ## [0.0.14] - 2023-07-05
 

pydoclint/utils/generic.py

@@ -49,6 +49,9 @@ def getFunctionId(node: FuncOrAsyncFuncDef) -> Tuple[int, int, str]:
     """
     Get unique identifier of a function def. We also need line and
     column number because different function can have identical names.
+
+    Note: this function is no longer used by the actual code, but it is
+    still used in unit tests. That's why we did not remove it.
     """
     return node.lineno, node.col_offset, node.name
 

pydoclint/utils/return_yield_raise.py

@@ -1,10 +1,10 @@
 import ast
-from typing import Dict, Tuple, Type, Union
+from typing import Callable, Dict, Tuple, Type
 
 from pydoclint.utils import walk
 from pydoclint.utils.annotation import unparseAnnotation
 from pydoclint.utils.astTypes import BlockType, FuncOrAsyncFuncDef
-from pydoclint.utils.generic import getFunctionId, stringStartsWith
+from pydoclint.utils.generic import stringStartsWith
 
 ReturnType = Type[ast.Return]
 ExprType = Type[ast.Expr]
@@ -23,6 +23,10 @@ def isReturnAnnotationNone(node: FuncOrAsyncFuncDef) -> bool:
     return _isNone(node.returns)
 
 
+def _isNone(node: ast.AST) -> bool:
+    return isinstance(node, ast.Constant) and node.value is None
+
+
 def hasGeneratorAsReturnAnnotation(node: FuncOrAsyncFuncDef) -> bool:
     """Check whether the function node has a 'Generator' return annotation"""
     if node.returns is None:
@@ -46,73 +50,84 @@ def hasIteratorOrIterableAsReturnAnnotation(node: FuncOrAsyncFuncDef) -> bool:
 
 def hasYieldStatements(node: FuncOrAsyncFuncDef) -> bool:
     """Check whether the function node has any yield statements"""
-    childLine: int = -999
-    foundYieldStmt: bool = False
-
-    # key: child lineno, value: (parent lineno, is parent a function?)
-    familyTree: Dict[int, Tuple[int, bool]] = {}
-
-    for child, parent in walk.walk(node):
-        childLine = _getLineNum(child)
-        parentLine = _getLineNum(parent)
-        if childLine != -1 and parentLine != -1 and childLine != parentLine:
-            isFunction = isinstance(parent, FuncOrAsyncFunc)
-            familyTree[childLine] = (parentLine, isFunction)
-
-        if isinstance(child, ast.Expr) and isinstance(
-            child.value, (ast.Yield, ast.YieldFrom)
-        ):
-            if isinstance(parent, (ast.AsyncFunctionDef, ast.FunctionDef)):
-                foundYieldStmt = True
-                break
-
-            if isinstance(parent, BlockType):
-                foundYieldStmt = True
-                break
-
-    if foundYieldStmt:
-        parentFuncLineNum = _lookupParentFunc(familyTree, childLine)
 
-        # We consider this `yield` a valid one only when its parent function
-        # is indeed `node`
-        if parentFuncLineNum == node.lineno:
-            return True
+    def isThisNodeAYieldStmt(node_: ast.AST) -> bool:
+        return isinstance(node_, ast.Expr) and isinstance(
+            node_.value, (ast.Yield, ast.YieldFrom)
+        )
 
-    return False
+    return _hasExpectedStatements(node, isThisNodeAYieldStmt)
 
 
 def hasReturnStatements(node: FuncOrAsyncFuncDef) -> bool:
     """Check whether the function node has any return statements"""
-    return _hasReturnOrRaiseStatements(node, expectedNodeType=ast.Return)
+
+    def isThisNodeAReturnStmt(node_: ast.AST) -> bool:
+        return isinstance(node_, ast.Return)
+
+    return _hasExpectedStatements(node, isThisNodeAReturnStmt)
 
 
 def hasRaiseStatements(node: FuncOrAsyncFuncDef) -> bool:
     """Check whether the function node has any raise statements"""
-    return _hasReturnOrRaiseStatements(node, expectedNodeType=ast.Raise)
 
+    def isThisNodeARaiseStmt(node_: ast.AST) -> bool:
+        return isinstance(node_, ast.Raise)
+
+    return _hasExpectedStatements(node, isThisNodeARaiseStmt)
 
-def _hasReturnOrRaiseStatements(
+
+def _hasExpectedStatements(
         node: FuncOrAsyncFuncDef,
-        expectedNodeType: Union[Type[ast.Return], Type[ast.Raise]],
+        isThisNodeAnExpectedStmt: Callable[[ast.AST], bool],
 ) -> bool:
-    thisId = getFunctionId(node)
+    """
+    Check whether the node contains an expected statement (return, yield, or
+    raise).
+    """
+    childLineNum: int = -999
+    foundReturnOrRaiseStmt: bool = False
+
+    # key: child lineno, value: (parent lineno, is parent a function?)
+    familyTree: Dict[int, Tuple[int, bool]] = {}
+
     for child, parent in walk.walk(node):
-        if isinstance(child, expectedNodeType):
+        childLineNum = _updateFamilyTree(child, parent, familyTree)
+
+        if isThisNodeAnExpectedStmt(child):
             if isinstance(parent, (ast.AsyncFunctionDef, ast.FunctionDef)):
-                # Only return True if the parent is `node` (in other words,
-                # this statement doesn't come from a child function of `node`)
-                parentId = getFunctionId(parent)
-                if thisId == parentId:
-                    return True
+                foundReturnOrRaiseStmt = True
+                break
 
             if isinstance(parent, BlockType):
-                return True
+                foundReturnOrRaiseStmt = True
+                break
 
-    return False
+    return _confirmThisStmtIsNotWithinNestedFunc(
+        foundStatementTemp=foundReturnOrRaiseStmt,
+        familyTree=familyTree,
+        lineNumOfStatement=childLineNum,
+        lineNumOfThisNode=node.lineno,
+    )
 
 
-def _isNone(node: ast.AST) -> bool:
-    return isinstance(node, ast.Constant) and node.value is None
+def _updateFamilyTree(
+        child: ast.AST,
+        parent: ast.AST,
+        familyTree: Dict[int, Tuple[int, bool]],
+) -> int:
+    """
+    Structure of `familyTree`:
+        Key: line number of child node
+        Value: (line number of parent node, whether this parent is a function)
+    """
+    childLine = _getLineNum(child)
+    parentLine = _getLineNum(parent)
+    if childLine != -1 and parentLine != -1 and childLine != parentLine:
+        isFunction = isinstance(parent, FuncOrAsyncFunc)
+        familyTree[childLine] = (parentLine, isFunction)
+
+    return childLine
 
 
 def _getLineNum(node: ast.AST) -> int:
@@ -124,14 +139,43 @@ def _getLineNum(node: ast.AST) -> int:
     return lineNum
 
 
+def _confirmThisStmtIsNotWithinNestedFunc(
+        foundStatementTemp: bool,
+        familyTree: Dict[int, Tuple[int, bool]],
+        lineNumOfStatement: int,
+        lineNumOfThisNode: int,
+) -> bool:
+    """
+    Check whether we REALLY found the expected statement (return, yield,
+    or raise).
+
+    Returns True if this statement is not within a nested function of `node`.
+    Returns False if otherwise.
+
+    We do this by checking whether the line number of the parent function
+    of the statement is the same as the line number of the node.
+    """
+    if not foundStatementTemp:
+        return False
+
+    parentFuncLineNum = _lookupParentFunc(familyTree, lineNumOfStatement)
+    return parentFuncLineNum == lineNumOfThisNode
+
+
 def _lookupParentFunc(
         familyLine: Dict[int, Tuple[int, bool]],
-        lineNum: int,
+        lineNumOfChildNode: int,
 ) -> int:
-    if lineNum not in familyLine:
+    """
+    Look up the parent function of the given child node.
+
+    Recursion is used in this function, because the key-val pairs in
+    `familyLine` only records immediate child-parent mapping.
+    """
+    if lineNumOfChildNode not in familyLine:
         return -999
 
-    parentLineNum, isParentAFunction = familyLine[lineNum]
+    parentLineNum, isParentAFunction = familyLine[lineNumOfChildNode]
 
     if isParentAFunction:
         return parentLineNum

pydoclint/visitor.py

@@ -546,7 +546,7 @@ def checkReturns(  # noqa: C901
                         msg += str([retArgType])
                         violations.append(v203.appendMoreMsg(moreMsg=msg))
                 else:
-                    if returnAnno.annotation != '':
+                    if bool(returnAnno.annotation):  # not empty str or not None
                         msg = 'Return annotation has 1 type(s); docstring'
                         msg += ' return section has 0 type(s).'
                         violations.append(v203.appendMoreMsg(moreMsg=msg))

setup.cfg

@@ -1,6 +1,6 @@
 [metadata]
 name = pydoclint
-version = 0.0.14
+version = 0.0.15
 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/google/raises/cases.py

@@ -93,3 +93,53 @@ def func8(self) -> None:
             open('nonexistent')
         except FileNotFoundError:
             raise
+
+    def func9a(self, arg0) -> None:
+        """
+        There should be DOC502 for this outer method.
+
+        Args:
+            arg0: Arg 0
+
+        Raises:
+            FileNotFoundError: If the file does not exist.
+        """
+
+        def inner9a(arg1) -> None:
+            """
+            There should be DOC501 for this inner method
+
+            Args:
+                arg1: Arg 1
+            """
+            try:
+                open('nonexistent')
+            except FileNotFoundError:
+                raise
+
+        print(arg0)
+
+    def func9b(self, arg0) -> None:
+        """
+        There should not be any violations in this outer method.
+
+        Args:
+            arg0: Arg 0
+        """
+
+        def inner9a(arg1) -> None:
+            """
+            There should not be any violations in this inner method.
+
+            Args:
+                arg1: Arg1
+
+            Raises:
+                FileNotFoundError: If the file does not exist.
+            """
+            try:
+                open('nonexistent')
+            except FileNotFoundError:
+                raise
+
+        print(arg0)

tests/data/google/returns/classmethod.py

@@ -187,3 +187,55 @@ def func95(cls) -> Tuple[Dict[MyClass1, 'MyClass2'], List['MyClass3']]:
             Tuple[Dict["MyClass1", MyClass2], List['MyClass3']]: Something
         """
         print(1)
+
+    @classmethod
+    def func101(cls, arg0: float):
+        """
+        Expected violations: DOC202, DOC203
+
+        Args:
+            arg0 (float): Arg 0
+
+        Returns:
+            bool: Return value
+        """
+
+        def inner101(arg1: str) -> bool:
+            """
+            Expected violations: DOC201, DOC203
+
+            Args:
+                arg1 (str): Arg 1
+            """
+            if arg1 > 'a':
+                return True
+            else:
+                return False
+
+        print(2)
+
+    @classmethod
+    def func102(cls, arg0: float):
+        """
+        There should not be any violations
+
+        Args:
+            arg0 (float): Arg 0
+        """
+
+        def inner102(arg1: str) -> bool:
+            """
+            There should not be any violations
+
+            Args:
+                arg1 (str): Arg 1
+
+            Returns:
+                bool: Return value
+            """
+            if arg1 > 'a':
+                return True
+            else:
+                return False
+
+        print(2)