Improve class attr comparison logic (#154)

CHANGELOG.md

@@ -1,5 +1,16 @@
 # Change Log
 
+## [0.5.5] - 2024-07-15
+
+- Fixed
+
+  - Fixed a bug where `a = b = c = 1` style cannot be properly parsed
+    (https://github.com/jsh9/pydoclint/issues/151)
+
+- Changed
+  - Changed the default of `--treat-property-methods-as-class-attributes` to
+    `False` to restore backward compatibility
+
 ## [0.5.4] - 2024-07-14
 
 - Added

docs/config_options.md

@@ -26,7 +26,7 @@ page:
 - [13. `--ignore-underscore-args` (shortform: `-iua`, default: `True`)](#13---ignore-underscore-args-shortform--iua-default-true)
 - [14. `--check-class-attributes` (shortform: `-cca`, default: `True`)](#14---check-class-attributes-shortform--cca-default-true)
 - [15. `--should-document-private-class-attributes` (shortform: `-sdpca`, default: `False`)](#15---should-document-private-class-attributes-shortform--sdpca-default-false)
-- [16. `--treat-property-methods-as-class-attributes` (shortform: `-tpmaca`, default: `True`)](#16---treat-property-methods-as-class-attributes-shortform--tpmaca-default-true)
+- [16. `--treat-property-methods-as-class-attributes` (shortform: `-tpmaca`, default: `False`)](#16---treat-property-methods-as-class-attributes-shortform--tpmaca-default-false)
 - [17. `--baseline`](#17---baseline)
 - [18. `--generate-baseline` (default: `False`)](#18---generate-baseline-default-false)
 - [19. `--show-filenames-in-every-violation-message` (shortform: `-sfn`, default: `False`)](#19---show-filenames-in-every-violation-message-shortform--sfn-default-false)
@@ -194,13 +194,12 @@ for more instructions.
 If True, private class attributes (those that start with leading `_`) should be
 documented. If False, they should not be documented.
 
-## 16. `--treat-property-methods-as-class-attributes` (shortform: `-tpmaca`, default: `True`)
+## 16. `--treat-property-methods-as-class-attributes` (shortform: `-tpmaca`, default: `False`)
 
 If True, treat `@property` methods as class properties. This means that they
 need to be documented in the "Attributes" section of the class docstring, and
 there cannot be any docstring under the @property methods. This option is only
-effective when --check-class-attributes is True. We recommend setting both this
-option and --check-class-attributes to True.
+effective when --check-class-attributes is True.
 
 ## 17. `--baseline`
 

pydoclint/flake8_entry.py

@@ -183,7 +183,7 @@ def add_options(cls, parser):  # noqa: D102
             '-tpmaca',
             '--treat-property-methods-as-class-attributes',
             action='store',
-            default='True',
+            default='False',
             parse_from_config=True,
             help=(
                 'If True, treat @property methods as class properties. This means'

pydoclint/main.py

@@ -223,7 +223,7 @@ def validateStyleValue(
     '--treat-property-methods-as-class-attributes',
     type=bool,
     show_default=True,
-    default=True,
+    default=False,
     help=(
         'If True, treat @property methods as class properties. This means'
         ' that they need to be documented in the "Attributes" section of'
@@ -527,7 +527,7 @@ def _checkPaths(
         ignoreUnderscoreArgs: bool = True,
         checkClassAttributes: bool = True,
         shouldDocumentPrivateClassAttributes: bool = False,
-        treatPropertyMethodsAsClassAttributes: bool = True,
+        treatPropertyMethodsAsClassAttributes: bool = False,
         requireReturnSectionWhenReturningNothing: bool = False,
         requireYieldSectionWhenYieldingNothing: bool = False,
         quiet: bool = False,
@@ -606,7 +606,7 @@ def _checkFile(
         ignoreUnderscoreArgs: bool = True,
         checkClassAttributes: bool = True,
         shouldDocumentPrivateClassAttributes: bool = False,
-        treatPropertyMethodsAsClassAttributes: bool = True,
+        treatPropertyMethodsAsClassAttributes: bool = False,
         requireReturnSectionWhenReturningNothing: bool = False,
         requireYieldSectionWhenYieldingNothing: bool = False,
 ) -> List[Violation]:

pydoclint/utils/arg.py

@@ -88,25 +88,6 @@ def fromAstArg(cls, astArg: ast.arg) -> 'Arg':
         typeHint: str = '' if anno is None else unparseAnnotation(anno)
         return Arg(name=astArg.arg, typeHint=typeHint)
 
-    @classmethod
-    def fromAstAssignWithNonTupleTarget(cls, astAssign: ast.Assign) -> 'Arg':
-        """
-        Construct an Arg object from a Python ast.Assign object whose
-        "target" field is an ast.Name rather than an ast.Tuple.
-        """
-        if len(astAssign.targets) != 1:
-            raise InternalError(
-                f'astAssign.targets has length {len(astAssign.targets)}'
-            )
-
-        if not isinstance(astAssign.targets[0], ast.Name):  # not a tuple
-            raise InternalError(
-                f'astAssign.targets[0] is of type {type(astAssign.targets[0])}'
-                ' instead of ast.Name'
-            )
-
-        return Arg(name=astAssign.targets[0].id, typeHint='')
-
     @classmethod
     def fromAstAnnAssign(cls, astAnnAssign: ast.AnnAssign) -> 'Arg':
         """Construct an Arg object from a Python ast.AnnAssign object"""
@@ -224,32 +205,26 @@ def fromDocstringAttr(
         return ArgList(infoList=infoList)
 
     @classmethod
-    def fromAstAssignWithTupleTarget(cls, astAssign: ast.Assign) -> 'ArgList':
-        """
-        Construct an ArgList from a Python ast.Assign object whose
-        "target" field is an ast.Tuple rather than an ast.Name.
-        """
-        if len(astAssign.targets) != 1:
-            raise InternalError(
-                f'astAssign.targets has length {len(astAssign.targets)}'
-            )
-
-        if not isinstance(astAssign.targets[0], ast.Tuple):
-            raise InternalError(
-                f'astAssign.targets[0] is of type {type(astAssign.targets[0])}'
-                ' instead of ast.Tuple'
-            )
-
-        infoList = []
-        for i, item in enumerate(astAssign.targets[0].elts):
-            if not isinstance(item, ast.Name):
+    def fromAstAssign(cls, astAssign: ast.Assign) -> 'ArgList':
+        """Construct an ArgList from variable declaration/assignment"""
+        infoList: List[Arg] = []
+        for i, target in enumerate(astAssign.targets):
+            if isinstance(target, ast.Tuple):  # such as `a, b = c, d = 1, 2`
+                for j, item in enumerate(target.elts):
+                    if not isinstance(item, ast.Name):
+                        raise InternalError(
+                            f'astAssign.targets[{i}].elts[{j}] is of'
+                            f' type {type(item)} instead of ast.Name'
+                        )
+
+                    infoList.append(Arg(name=item.id, typeHint=''))
+            elif isinstance(target, ast.Name):  # such as `a = 1` or `a = b = 2`
+                infoList.append(Arg(name=target.id, typeHint=''))
+            else:
                 raise InternalError(
-                    f'astAssign.targets[0].elts[{i}] is of type {type(item)}'
-                    ' instead of ast.Name'
+                    f'astAssign.targets[{i}] is of type {type(target)}'
                 )
 
-            infoList.append(Arg(name=item.id, typeHint=''))
-
         return ArgList(infoList=infoList)
 
     def contains(self, arg: Arg) -> bool:

pydoclint/utils/visitor_helper.py

@@ -1,11 +1,10 @@
 """Helper functions to classes/methods in visitor.py"""
 import ast
 import sys
-from typing import List, Optional, Set, Union
+from typing import List, Optional, Set
 
 from pydoclint.utils.annotation import unparseAnnotation
 from pydoclint.utils.arg import Arg, ArgList
-from pydoclint.utils.astTypes import FuncOrAsyncFuncDef
 from pydoclint.utils.doc import Doc
 from pydoclint.utils.generic import (
     appendArgsToCheckToV105,
@@ -42,14 +41,11 @@ def checkClassAttributesAgainstClassDocstring(
         treatPropertyMethodsAsClassAttributes: bool,
 ) -> None:
     """Check class attribute list against the attribute list in docstring"""
-    classAttributes = _collectClassAttributes(
+    actualArgs: ArgList = extractClassAttributesFromNode(
         node=node,
         shouldDocumentPrivateClassAttributes=(
             shouldDocumentPrivateClassAttributes
         ),
-    )
-    actualArgs: ArgList = _convertClassAttributesIntoArgList(
-        classAttrs=classAttributes,
         treatPropertyMethodsAsClassAttrs=treatPropertyMethodsAsClassAttributes,
     )
 
@@ -124,71 +120,62 @@ def checkClassAttributesAgainstClassDocstring(
     )
 
 
-def _collectClassAttributes(
+def extractClassAttributesFromNode(
         *,
         node: ast.ClassDef,
         shouldDocumentPrivateClassAttributes: bool,
-) -> List[Union[ast.Assign, ast.AnnAssign, FuncOrAsyncFuncDef]]:
-    if 'body' not in node.__dict__ or len(node.body) == 0:
-        return []
-
-    attributes: List[Union[ast.Assign, ast.AnnAssign]] = []
-    for item in node.body:
-        # Notes:
-        #   - ast.Assign are something like "attr1 = 1.5"
-        #   - ast.AnnAssign are something like "attr2: float = 1.5"
-        if isinstance(item, (ast.Assign, ast.AnnAssign)):
-            classAttrName: str = _getClassAttrName(item)
-            if shouldDocumentPrivateClassAttributes:
-                attributes.append(item)
-            else:
-                if not classAttrName.startswith('_'):
-                    attributes.append(item)
-
-        if isinstance(
-            item, (ast.AsyncFunctionDef, ast.FunctionDef)
-        ) and checkIsPropertyMethod(item):
-            attributes.append(item)
-
-    return attributes
-
-
-def _getClassAttrName(attrItem: Union[ast.Assign, ast.AnnAssign]) -> str:
-    if isinstance(attrItem, ast.Assign):
-        return attrItem.targets[0].id
-
-    if isinstance(attrItem, ast.AnnAssign):
-        return attrItem.target.id
-
-    raise InternalError(f'Unrecognized attrItem type: {type(attrItem)}')
-
-
-def _convertClassAttributesIntoArgList(
-        *,
-        classAttrs: List[Union[ast.Assign, ast.AnnAssign, FuncOrAsyncFuncDef]],
         treatPropertyMethodsAsClassAttrs: bool,
 ) -> ArgList:
+    """
+    Extract class attributes from an AST node.
+
+    Parameters
+    ----------
+    node : ast.ClassDef
+        The class definition
+    shouldDocumentPrivateClassAttributes : bool
+        Whether we should document private class attributes.  If ``True``,
+        private class attributes will be included in the return value.
+    treatPropertyMethodsAsClassAttrs : bool
+        Whether we'd like to treat property methods as class attributes.
+        If ``True``, property methods will be included in the return value.
+
+    Returns
+    -------
+    ArgList
+        The argument list
+
+    Raises
+    ------
+    InternalError
+        When the length of item.targets is 0
+    """
+    if 'body' not in node.__dict__ or len(node.body) == 0:
+        return ArgList([])
+
     atl: List[Arg] = []
-    for attr in classAttrs:
-        if isinstance(attr, ast.AnnAssign):
-            atl.append(Arg.fromAstAnnAssign(attr))
-        elif isinstance(attr, ast.Assign):
-            if isinstance(attr.targets[0], ast.Tuple):
-                atl.extend(ArgList.fromAstAssignWithTupleTarget(attr).infoList)
-            else:
-                atl.append(Arg.fromAstAssignWithNonTupleTarget(attr))
-        elif isinstance(attr, (ast.AsyncFunctionDef, ast.FunctionDef)):
-            if treatPropertyMethodsAsClassAttrs:
+    for itm in node.body:
+        if isinstance(itm, ast.AnnAssign):  # with type hints ("a: int = 1")
+            atl.append(Arg.fromAstAnnAssign(itm))
+        elif isinstance(itm, ast.Assign):  # no type hints
+            if not isinstance(itm.targets, list) or len(itm.targets) == 0:
+                raise InternalError(
+                    '`item.targets` needs to be a list of length > 0.'
+                    f' Instead, it is {itm.targets}'
+                )
+
+            atl.extend(ArgList.fromAstAssign(itm).infoList)
+        elif isinstance(itm, (ast.AsyncFunctionDef, ast.FunctionDef)):
+            if treatPropertyMethodsAsClassAttrs and checkIsPropertyMethod(itm):
                 atl.append(
                     Arg(
-                        name=attr.name,
-                        typeHint=unparseAnnotation(attr.returns),
+                        name=itm.name,
+                        typeHint=unparseAnnotation(itm.returns),
                     )
                 )
-        else:
-            raise InternalError(
-                f'Unknown type of class attribute: {type(attr)}'
-            )
+
+    if not shouldDocumentPrivateClassAttributes:
+        atl = [_ for _ in atl if not _.name.startswith('_')]
 
     return ArgList(infoList=atl)
 

setup.cfg

@@ -1,6 +1,6 @@
 [metadata]
 name = pydoclint
-version = 0.5.4
+version = 0.5.5
 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/edge_cases/12_property_methods_as_class_attr/google.py

@@ -4,6 +4,7 @@ class House:
 
     Attributes:
         price (float): House price
+        _privateProperty (str): A private property
 
     Args:
         price_0 (float): House price
@@ -27,3 +28,7 @@ def price(self, new_price):
     @price.deleter
     def price(self):
         del self._price
+
+    @property
+    def _privateProperty(self) -> str:
+        return 'secret'

tests/data/edge_cases/13_class_attr_assignments/google.py

@@ -0,0 +1,26 @@
+class MyClass:
+    """
+    My Class
+
+    Attributes:
+        a1: attr 1
+        a2: attr 2
+        a3: attr 3
+        a4: attr 4
+        a5: attr 5
+        a6: attr 6
+        a7: attr 7
+        a8: attr 8
+        a9: attr 9
+        a10: attr 10
+        a11: attr 11
+        a12: attr 12
+        a13: attr 13
+        a14: attr 14
+        a15: attr 15
+    """
+
+    a1, a2, a3 = 1, 2, 3
+    a4 = a5 = a6 = 3
+    a7 = a8 = a9 = a6 == 2
+    a10, a11 = a12, a13 = a14, a15 = 5, 6