Skip to content

Commit

Permalink
Improve class attr comparison logic
Browse files Browse the repository at this point in the history
  • Loading branch information
@jsh9
jsh9 committed Jul 15, 2024
1 parent 0b750d5 commit c4b1ffc
Show file tree
Hide file tree
Showing 6 changed files with 303 additions and 106 deletions.
58 changes: 16 additions & 42 deletions pydoclint/utils/arg.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand Down Expand Up @@ -224,32 +205,25 @@ 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':
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:
Expand Down
105 changes: 44 additions & 61 deletions pydoclint/utils/visitor_helper.py
View file
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -42,14 +41,12 @@ 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,
)

Expand Down Expand Up @@ -124,71 +121,57 @@ 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
"""
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)

Expand Down
5 changes: 5 additions & 0 deletions tests/data/edge_cases/12_property_methods_as_class_attr/google.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ class House:
Attributes:
price (float): House price
_privateProperty (str): A private property
Args:
price_0 (float): House price
Expand All @@ -27,3 +28,7 @@ def price(self, new_price):
@price.deleter
def price(self):
del self._price

@property
def _privateProperty(self) -> str:
return 'secret'
26 changes: 26 additions & 0 deletions tests/data/edge_cases/13_class_attr_assignments/google.py
View file
Original file line number Diff line number Diff line change
@@ -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
52 changes: 49 additions & 3 deletions tests/test_main.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1332,6 +1332,7 @@ def testNonAscii() -> None:
'style': 'google',
'checkClassAttributes': True,
'treatPropertyMethodsAsClassAttributes': True,
'shouldDocumentPrivateClassAttributes': True,
},
[],
),
Expand All @@ -1341,15 +1342,51 @@ def testNonAscii() -> None:
'style': 'google',
'checkClassAttributes': True,
'treatPropertyMethodsAsClassAttributes': True,
'shouldDocumentPrivateClassAttributes': False,
},
[],
[
'DOC602: Class `House`: Class docstring contains more class attributes than '
'in actual class attributes. (Please read '
'https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to '
'correctly document class attributes.)',
'DOC603: Class `House`: Class docstring attributes are different from actual '
'class attributes. (Or could be other formatting issues: '
'https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). '
'Arguments in the docstring but not in the actual class attributes: '
'[_privateProperty: str]. (Please read '
'https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to '
'correctly document class attributes.)',
],
),
(
'12_property_methods_as_class_attr/google.py',
{
'style': 'google',
'checkClassAttributes': True,
'treatPropertyMethodsAsClassAttributes': False,
'shouldDocumentPrivateClassAttributes': True,
},
[
'DOC602: Class `House`: Class docstring contains more class attributes than '
'in actual class attributes. (Please read '
'https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to '
'correctly document class attributes.)',
'DOC603: Class `House`: Class docstring attributes are different from actual '
'class attributes. (Or could be other formatting issues: '
'https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). '
'Arguments in the docstring but not in the actual class attributes: '
'[_privateProperty: str, price: float]. (Please read '
'https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to '
'correctly document class attributes.)',
],
),
(
'12_property_methods_as_class_attr/google.py',
{
'style': 'google',
'checkClassAttributes': True,
'treatPropertyMethodsAsClassAttributes': False,
'shouldDocumentPrivateClassAttributes': False,
},
[
'DOC602: Class `House`: Class docstring contains more class attributes than '
Expand All @@ -1359,12 +1396,20 @@ def testNonAscii() -> None:
'DOC603: Class `House`: Class docstring attributes are different from actual '
'class attributes. (Or could be other formatting issues: '
'https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). '
'Arguments in the docstring but not in the actual class attributes: [price: '
'float]. (Please read '
'Arguments in the docstring but not in the actual class attributes: '
'[_privateProperty: str, price: float]. (Please read '
'https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to '
'correctly document class attributes.)',
],
),
(
'13_class_attr_assignments/google.py',
{
'style': 'google',
'checkClassAttributes': True,
},
[],
),
],
)
def testEdgeCases(
Expand All @@ -1389,6 +1434,7 @@ def testPlayground() -> None:
violations = _checkFile(
filename=DATA_DIR / 'playground.py',
style='google',
skipCheckingRaises=True,
)
expected = []
assert list(map(str, violations)) == expected
Loading

0 comments on commit c4b1ffc

Please sign in to comment.