From 265ebd04d2e4914e59cc6e5cdf2c4064d41b13b3 Mon Sep 17 00:00:00 2001 From: jsh9 <25124332+jsh9@users.noreply.github.com> Date: Mon, 10 Jul 2023 02:53:45 -0700 Subject: [PATCH 1/5] Fix raise/return detection for nested functions --- pydoclint/utils/return_yield_raise.py | 95 ++++++++++++++++------- pydoclint/visitor.py | 2 +- tests/data/google/raises/cases.py | 50 ++++++++++++ tests/data/google/returns/classmethod.py | 52 +++++++++++++ tests/data/google/returns/function.py | 52 +++++++++++++ tests/data/google/returns/method.py | 50 ++++++++++++ tests/data/google/returns/staticmethod.py | 52 +++++++++++++ tests/data/numpy/raises/cases.py | 62 +++++++++++++++ tests/data/numpy/returns/classmethod.py | 64 +++++++++++++++ tests/data/numpy/returns/function.py | 64 +++++++++++++++ tests/data/numpy/returns/method.py | 62 +++++++++++++++ tests/data/numpy/returns/staticmethod.py | 64 +++++++++++++++ tests/test_main.py | 16 ++++ 13 files changed, 657 insertions(+), 28 deletions(-) diff --git a/pydoclint/utils/return_yield_raise.py b/pydoclint/utils/return_yield_raise.py index f2587a9..ef3f656 100644 --- a/pydoclint/utils/return_yield_raise.py +++ b/pydoclint/utils/return_yield_raise.py @@ -4,7 +4,7 @@ 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] @@ -46,39 +46,32 @@ 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 + childLineNum: int = -999 + foundYieldStmtTemp: bool = False # "temp" b/c it may be false positive # 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) + childLineNum = _updateFamilyTree(child, parent, familyTree) if isinstance(child, ast.Expr) and isinstance( child.value, (ast.Yield, ast.YieldFrom) ): if isinstance(parent, (ast.AsyncFunctionDef, ast.FunctionDef)): - foundYieldStmt = True + foundYieldStmtTemp = True break if isinstance(parent, BlockType): - foundYieldStmt = True + foundYieldStmtTemp = 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 - - return False + return _foundExpectedStatement( + foundStatementTemp=foundYieldStmtTemp, + familyTree=familyTree, + lineNumOfStatement=childLineNum, + lineNumOfThisNode=node.lineno, + ) def hasReturnStatements(node: FuncOrAsyncFuncDef) -> bool: @@ -95,26 +88,50 @@ def _hasReturnOrRaiseStatements( node: FuncOrAsyncFuncDef, expectedNodeType: Union[Type[ast.Return], Type[ast.Raise]], ) -> bool: - thisId = getFunctionId(node) + 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): + childLineNum = _updateFamilyTree(child, parent, familyTree) + if isinstance(child, expectedNodeType): 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 _foundExpectedStatement( + 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: + 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: try: lineNum = node.lineno @@ -124,6 +141,30 @@ def _getLineNum(node: ast.AST) -> int: return lineNum +def _foundExpectedStatement( + foundStatementTemp: bool, + familyTree: Dict[int, Tuple[int, bool]], + lineNumOfStatement: int, + lineNumOfThisNode: int, +) -> bool: + """ + Check whether we REALLY found the expected statment (return, yield, + or raise). + + 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 so, we know that this statement is an immediate child of the node. + If not, we know that this statement is a child of a nested function 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, diff --git a/pydoclint/visitor.py b/pydoclint/visitor.py index f5fd345..b3ee2eb 100644 --- a/pydoclint/visitor.py +++ b/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)) diff --git a/tests/data/google/raises/cases.py b/tests/data/google/raises/cases.py index 76b49c6..21b0565 100644 --- a/tests/data/google/raises/cases.py +++ b/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) diff --git a/tests/data/google/returns/classmethod.py b/tests/data/google/returns/classmethod.py index 30d83dd..ca38b68 100644 --- a/tests/data/google/returns/classmethod.py +++ b/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) diff --git a/tests/data/google/returns/function.py b/tests/data/google/returns/function.py index c2edb71..58f557c 100644 --- a/tests/data/google/returns/function.py +++ b/tests/data/google/returns/function.py @@ -182,3 +182,55 @@ def func95() -> Tuple[Dict[MyClass1, 'MyClass2'], List['MyClass3']]: Tuple[Dict["MyClass1", MyClass2], List['MyClass3']]: Something """ print(1) + + +def func101(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) + + +def func102(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) diff --git a/tests/data/google/returns/method.py b/tests/data/google/returns/method.py index b14ece1..eee413b 100644 --- a/tests/data/google/returns/method.py +++ b/tests/data/google/returns/method.py @@ -167,3 +167,53 @@ def func95(self) -> Tuple[Dict[MyClass1, 'MyClass2'], List['MyClass3']]: Tuple[Dict["MyClass1", MyClass2], List['MyClass3']]: Something """ print(1) + + def func101(self, 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) + + def func102(self, 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) diff --git a/tests/data/google/returns/staticmethod.py b/tests/data/google/returns/staticmethod.py index 5f17c3c..941b656 100644 --- a/tests/data/google/returns/staticmethod.py +++ b/tests/data/google/returns/staticmethod.py @@ -187,3 +187,55 @@ def func95() -> Tuple[Dict[MyClass1, 'MyClass2'], List['MyClass3']]: Tuple[Dict["MyClass1", MyClass2], List['MyClass3']]: Something """ print(1) + + @staticmethod + def func101(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) + + @staticmethod + def func102(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) diff --git a/tests/data/numpy/raises/cases.py b/tests/data/numpy/raises/cases.py index fff55f3..5982582 100644 --- a/tests/data/numpy/raises/cases.py +++ b/tests/data/numpy/raises/cases.py @@ -116,3 +116,65 @@ def func8(self) -> None: open('nonexistent') except FileNotFoundError: raise + + def func9a(self, arg0) -> None: + """ + There should be DOC502 for this outer method. + + Parameters + ---------- + arg0 : + Arg 0 + + Raises + ------ + FileNotFoundError + If the file does not exist. + """ + + def inner9a(arg1) -> None: + """ + There should be DOC501 for this inner method + + Parameters + ---------- + 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. + + Parameters + ---------- + arg0 + Arg 0 + """ + + def inner9a(arg1) -> None: + """ + There should not be any violations in this inner method. + + Parameters + ---------- + arg1 + Arg1 + + Raises + ------ + FileNotFoundError + If the file does not exist. + """ + try: + open('nonexistent') + except FileNotFoundError: + raise + + print(arg0) diff --git a/tests/data/numpy/returns/classmethod.py b/tests/data/numpy/returns/classmethod.py index 8db01c9..210e22a 100644 --- a/tests/data/numpy/returns/classmethod.py +++ b/tests/data/numpy/returns/classmethod.py @@ -215,3 +215,67 @@ def func95(cls) -> Tuple[Dict[MyClass1, 'MyClass2'], List['MyClass3']]: The return value """ print(1) + + @classmethod + def func101(cls, arg0: float): + """ + Expected violations: DOC202, DOC203 + + Parameters + ---------- + arg0 : float + Arg 0 + + Returns + ------- + bool + Return value + """ + + def inner101(arg1: str) -> bool: + """ + Expected violations: DOC201, DOC203 + + Parameters + ---------- + 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 + + Parameters + ---------- + arg0 : float + Arg 0 + """ + + def inner102(arg1: str) -> bool: + """ + There should not be any violations + + Parameters + ---------- + arg1 : str + Arg 1 + + Returns + ------- + bool + Return value + """ + if arg1 > 'a': + return True + else: + return False + + print(2) diff --git a/tests/data/numpy/returns/function.py b/tests/data/numpy/returns/function.py index c5a07f7..427f679 100644 --- a/tests/data/numpy/returns/function.py +++ b/tests/data/numpy/returns/function.py @@ -214,3 +214,67 @@ def func95() -> Tuple[Dict[MyClass1, 'MyClass2'], List['MyClass3']]: The return value """ print(1) + + +def func101(arg0: float): + """ + Expected violations: DOC202, DOC203 + + Parameters + ---------- + arg0 : float + Arg 0 + + Returns + ------- + bool + Return value + """ + + def inner101(arg1: str) -> bool: + """ + Expected violations: DOC201, DOC203 + + Parameters + ---------- + arg1 : str + Arg 1 + """ + if arg1 > 'a': + return True + else: + return False + + print(2) + + +def func102(arg0: float): + """ + There should not be any violations + + Parameters + ---------- + arg0 : float + Arg 0 + """ + + def inner102(arg1: str) -> bool: + """ + There should not be any violations + + Parameters + ---------- + arg1 : str + Arg 1 + + Returns + ------- + bool + Return value + """ + if arg1 > 'a': + return True + else: + return False + + print(2) diff --git a/tests/data/numpy/returns/method.py b/tests/data/numpy/returns/method.py index 8dbdb6a..58bfa9e 100644 --- a/tests/data/numpy/returns/method.py +++ b/tests/data/numpy/returns/method.py @@ -195,3 +195,65 @@ def func95(self) -> Tuple[Dict[MyClass1, 'MyClass2'], List['MyClass3']]: The return value """ print(1) + + def func101(self, arg0: float): + """ + Expected violations: DOC202, DOC203 + + Parameters + ---------- + arg0 : float + Arg 0 + + Returns + ------- + bool + Return value + """ + + def inner101(arg1: str) -> bool: + """ + Expected violations: DOC201, DOC203 + + Parameters + ---------- + arg1 : str + Arg 1 + """ + if arg1 > 'a': + return True + else: + return False + + print(2) + + def func102(self, arg0: float): + """ + There should not be any violations + + Parameters + ---------- + arg0 : float + Arg 0 + """ + + def inner102(arg1: str) -> bool: + """ + There should not be any violations + + Parameters + ---------- + arg1 : str + Arg 1 + + Returns + ------- + bool + Return value + """ + if arg1 > 'a': + return True + else: + return False + + print(2) diff --git a/tests/data/numpy/returns/staticmethod.py b/tests/data/numpy/returns/staticmethod.py index 9451d7a..4ace32c 100644 --- a/tests/data/numpy/returns/staticmethod.py +++ b/tests/data/numpy/returns/staticmethod.py @@ -215,3 +215,67 @@ def func95() -> Tuple[Dict[MyClass1, 'MyClass2'], List['MyClass3']]: The return value """ print(1) + + @staticmethod + def func101(arg0: float): + """ + Expected violations: DOC202, DOC203 + + Parameters + ---------- + arg0 : float + Arg 0 + + Returns + ------- + bool + Return value + """ + + def inner101(arg1: str) -> bool: + """ + Expected violations: DOC201, DOC203 + + Parameters + ---------- + arg1 : str + Arg 1 + """ + if arg1 > 'a': + return True + else: + return False + + print(2) + + @staticmethod + def func102(arg0: float): + """ + There should not be any violations + + Parameters + ---------- + arg0 : float + Arg 0 + """ + + def inner102(arg1: str) -> bool: + """ + There should not be any violations + + Parameters + ---------- + arg1 : str + Arg 1 + + Returns + ------- + bool + Return value + """ + if arg1 > 'a': + return True + else: + return False + + print(2) diff --git a/tests/test_main.py b/tests/test_main.py index add6e63..4975d7b 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -191,6 +191,18 @@ def testReturns(style: str, filename: str) -> None: "docstring return section types: ['int']" ) + expectedViolations.extend([ + 'DOC202: Method `MyClass.func101` has a return section in docstring, but ' + 'there are no return statements or annotations ', + 'DOC203: Method `MyClass.func101` return type(s) in docstring not consistent ' + 'with the return annotation. Return annotation has 0 type(s); docstring ' + 'return section has 1 type(s).', + 'DOC201: Function `inner101` does not have a return section in docstring ', + 'DOC203: Function `inner101` return type(s) in docstring not consistent with ' + 'the return annotation. Return annotation has 1 type(s); docstring return ' + 'section has 0 type(s).', + ]) + expectedViolationsCopy = copy.deepcopy(expectedViolations) if filename == 'function.py': _tweakViolationMsgForFunctions(expectedViolationsCopy) @@ -450,6 +462,10 @@ def testRaises(style: str, skipRaisesCheck: bool) -> None: 'are not "raise" statements in the body ', 'DOC502: Method `B.func7` has a "Raises" section in the docstring, but there ' 'are not "raise" statements in the body ', + 'DOC502: Method `B.func9a` has a "Raises" section in the docstring, but there ' + 'are not "raise" statements in the body ', + 'DOC501: Function `inner9a` has "raise" statements, but the docstring does ' + 'not have a "Raises" section ', ] expected1 = [] expected = expected1 if skipRaisesCheck else expected0 From fdf672abb4918a2fd7c04b74c9df5140cd9e7e30 Mon Sep 17 00:00:00 2001 From: jsh9 <25124332+jsh9@users.noreply.github.com> Date: Mon, 10 Jul 2023 03:05:56 -0700 Subject: [PATCH 2/5] Reduce code duplication --- pydoclint/utils/return_yield_raise.py | 49 +++++++++++---------------- 1 file changed, 19 insertions(+), 30 deletions(-) diff --git a/pydoclint/utils/return_yield_raise.py b/pydoclint/utils/return_yield_raise.py index ef3f656..aaacb1b 100644 --- a/pydoclint/utils/return_yield_raise.py +++ b/pydoclint/utils/return_yield_raise.py @@ -1,5 +1,5 @@ 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 @@ -46,47 +46,36 @@ def hasIteratorOrIterableAsReturnAnnotation(node: FuncOrAsyncFuncDef) -> bool: def hasYieldStatements(node: FuncOrAsyncFuncDef) -> bool: """Check whether the function node has any yield statements""" - childLineNum: int = -999 - foundYieldStmtTemp: bool = False # "temp" b/c it may be false positive - # key: child lineno, value: (parent lineno, is parent a function?) - familyTree: Dict[int, Tuple[int, bool]] = {} + def isThisNodeAYieldStmt(node_: ast.AST) -> bool: + return isinstance(node_, ast.Expr) and isinstance( + node_.value, (ast.Yield, ast.YieldFrom) + ) - for child, parent in walk.walk(node): - childLineNum = _updateFamilyTree(child, parent, familyTree) - - if isinstance(child, ast.Expr) and isinstance( - child.value, (ast.Yield, ast.YieldFrom) - ): - if isinstance(parent, (ast.AsyncFunctionDef, ast.FunctionDef)): - foundYieldStmtTemp = True - break - - if isinstance(parent, BlockType): - foundYieldStmtTemp = True - break - - return _foundExpectedStatement( - foundStatementTemp=foundYieldStmtTemp, - familyTree=familyTree, - lineNumOfStatement=childLineNum, - lineNumOfThisNode=node.lineno, - ) + 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: childLineNum: int = -999 foundReturnOrRaiseStmt: bool = False @@ -97,7 +86,7 @@ def _hasReturnOrRaiseStatements( for child, parent in walk.walk(node): childLineNum = _updateFamilyTree(child, parent, familyTree) - if isinstance(child, expectedNodeType): + if isThisNodeAnExpectedStmt(child): if isinstance(parent, (ast.AsyncFunctionDef, ast.FunctionDef)): foundReturnOrRaiseStmt = True break From 223bc7b4433ea829d119df01e91cbd74098cc4f1 Mon Sep 17 00:00:00 2001 From: jsh9 <25124332+jsh9@users.noreply.github.com> Date: Mon, 10 Jul 2023 03:08:47 -0700 Subject: [PATCH 3/5] Rename function; update docstring --- pydoclint/utils/return_yield_raise.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/pydoclint/utils/return_yield_raise.py b/pydoclint/utils/return_yield_raise.py index aaacb1b..d99e1b9 100644 --- a/pydoclint/utils/return_yield_raise.py +++ b/pydoclint/utils/return_yield_raise.py @@ -95,7 +95,7 @@ def _hasExpectedStatements( foundReturnOrRaiseStmt = True break - return _foundExpectedStatement( + return _confirmThisStmtIsNotWithinNestedFunc( foundStatementTemp=foundReturnOrRaiseStmt, familyTree=familyTree, lineNumOfStatement=childLineNum, @@ -130,22 +130,21 @@ def _getLineNum(node: ast.AST) -> int: return lineNum -def _foundExpectedStatement( +def _confirmThisStmtIsNotWithinNestedFunc( foundStatementTemp: bool, familyTree: Dict[int, Tuple[int, bool]], lineNumOfStatement: int, lineNumOfThisNode: int, ) -> bool: """ - Check whether we REALLY found the expected statment (return, yield, + 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 so, we know that this statement is an immediate child of the node. - If not, we know that this statement is a child of a nested function of - the node. """ if not foundStatementTemp: return False From 2ecd4ffa7c76fd1232aa20cbade23a0313acc7aa Mon Sep 17 00:00:00 2001 From: jsh9 <25124332+jsh9@users.noreply.github.com> Date: Mon, 10 Jul 2023 03:10:27 -0700 Subject: [PATCH 4/5] Update changelog; bump version --- CHANGELOG.md | 8 ++++++-- setup.cfg | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 414a85c..f15bdad 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/setup.cfg b/setup.cfg index 071bb77..1e53848 100644 --- a/setup.cfg +++ b/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 From 211c8cc47f02a5c1653f3ee7454425e02a9aa658 Mon Sep 17 00:00:00 2001 From: jsh9 <25124332+jsh9@users.noreply.github.com> Date: Mon, 10 Jul 2023 03:21:40 -0700 Subject: [PATCH 5/5] Update comments; rename variable --- pydoclint/utils/generic.py | 3 +++ pydoclint/utils/return_yield_raise.py | 29 ++++++++++++++++++++------- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/pydoclint/utils/generic.py b/pydoclint/utils/generic.py index 9697dde..b5fa12d 100644 --- a/pydoclint/utils/generic.py +++ b/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 diff --git a/pydoclint/utils/return_yield_raise.py b/pydoclint/utils/return_yield_raise.py index d99e1b9..185a64c 100644 --- a/pydoclint/utils/return_yield_raise.py +++ b/pydoclint/utils/return_yield_raise.py @@ -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: @@ -77,6 +81,10 @@ def _hasExpectedStatements( node: FuncOrAsyncFuncDef, isThisNodeAnExpectedStmt: Callable[[ast.AST], bool], ) -> bool: + """ + Check whether the node contains an expected statement (return, yield, or + raise). + """ childLineNum: int = -999 foundReturnOrRaiseStmt: bool = False @@ -103,15 +111,16 @@ def _hasExpectedStatements( ) -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: @@ -155,12 +164,18 @@ def _confirmThisStmtIsNotWithinNestedFunc( 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