From bd7ca7b57834131b5174ae1c272a79d39e1e9aff Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 5 Aug 2022 00:12:50 -0400 Subject: [PATCH 01/47] Initial attempt at fixing #587: Unify properties getter, setters and deleters under a single documentation entry. --- pydoctor/astbuilder.py | 99 +++++++------- pydoctor/model.py | 120 ++++++++++++++++- pydoctor/templatewriter/pages/__init__.py | 2 +- .../templatewriter/pages/attributechild.py | 19 ++- .../templatewriter/pages/functionchild.py | 17 ++- pydoctor/test/test_astbuilder.py | 47 ++++++- pydoctor/test/test_templatewriter.py | 125 ++++++++++++++++++ pydoctor/themes/base/attribute-child.html | 1 + 8 files changed, 374 insertions(+), 56 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 5ffb4add2..bb011046e 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -723,6 +723,8 @@ def _handleFunctionDef(self, is_property = False is_classmethod = False is_staticmethod = False + property_info: Optional[model.PropertyInfo] = None + if isinstance(parent, model.Class) and node.decorator_list: for d in node.decorator_list: if isinstance(d, ast.Call): @@ -738,18 +740,37 @@ def _handleFunctionDef(self, elif deco_name == ['staticmethod']: is_staticmethod = True elif len(deco_name) >= 2 and deco_name[-1] in ('setter', 'deleter'): - # Rename the setter/deleter, so it doesn't replace - # the property object. - func_name = '.'.join(deco_name[-2:]) - - if is_property: - # handle property and skip child nodes. - attr = self._handlePropertyDef(node, docstring, lineno) - if is_classmethod: - attr.report(f'{attr.fullName()} is both property and classmethod') - if is_staticmethod: - attr.report(f'{attr.fullName()} is both property and staticmethod') - raise self.SkipNode() + if len(deco_name)==2: + # Setters and deleters must have the same name as the property function + if deco_name[0]==func_name: + property_getter = parent.contents.get(func_name) + + if property_getter is not None: + # Rename the setter/deleter such that + # it does not replace the property getter. + + func_name = '.'.join(deco_name) + + if not isinstance(property_getter, model.Function): + # Can't make sens of decorator ending in .setter/.deleter :/ + # The property setter/deleter is not targeting a function + # We still rename it because it overrides something and it maches + # the rules to be a property. Maybe it's actually targetting a callable + # implemented as a __call__ method or a lamda function. Is it even valid python? + continue + if property_getter._property_info is None: + # Probably an unsupported type of property + continue + + # We have an actual python property: + # Store property info object + property_info = property_getter._property_info + + else: + # Can't make sens of decorator ending in .setter/.deleter :/ + # The decorator is a dotted name of three parts or more, like 'Person.name.setter'. + # Don't do anything special with it, i.e do not rename it. + continue func = self.builder.pushFunction(func_name, lineno) func.is_async = is_async @@ -806,6 +827,25 @@ def add_arg(name: str, kind: Any, default: Optional[ast.expr]) -> None: func.signature = signature func.annotations = self._annotations_from_function(node) + + + if is_property: + # Init PropertyInfo object when visiting the getter. + func._property_info = model.PropertyInfo() + + if is_classmethod: + func.report(f'{func.fullName()} is both property and classmethod') + if is_staticmethod: + func.report(f'{func.fullName()} is both property and staticmethod') + + # Store property functions to be handled later. + if property_info is not None: + if func_name.endswith('.deleter'): + property_info.deleter = func + elif func_name.endswith('.setter'): + property_info.setter = func + else: + assert False def depart_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: self.builder.popFunction() @@ -813,41 +853,6 @@ def depart_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: def depart_FunctionDef(self, node: ast.FunctionDef) -> None: self.builder.popFunction() - def _handlePropertyDef(self, - node: Union[ast.AsyncFunctionDef, ast.FunctionDef], - docstring: Optional[ast.Str], - lineno: int - ) -> model.Attribute: - - attr = self.builder.addAttribute(name=node.name, kind=model.DocumentableKind.PROPERTY, parent=self.builder.current) - attr.setLineNumber(lineno) - - if docstring is not None: - attr.setDocstring(docstring) - assert attr.docstring is not None - pdoc = epydoc2stan.parse_docstring(attr, attr.docstring, attr) - other_fields = [] - for field in pdoc.fields: - tag = field.tag() - if tag == 'return': - if not pdoc.has_body: - pdoc = field.body() - # Avoid format_summary() going back to the original - # empty-body docstring. - attr.docstring = '' - elif tag == 'rtype': - attr.parsed_type = field.body() - else: - other_fields.append(field) - pdoc.fields = other_fields - attr.parsed_docstring = pdoc - - if node.returns is not None: - attr.annotation = self._unstring_annotation(node.returns) - attr.decorators = node.decorator_list - - return attr - def _annotations_from_function( self, func: Union[ast.AsyncFunctionDef, ast.FunctionDef] ) -> Mapping[str, Optional[ast.expr]]: diff --git a/pydoctor/model.py b/pydoctor/model.py index 5b4053814..9cedd0e78 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -29,6 +29,8 @@ from pydoctor.epydoc.markup import ParsedDocstring from pydoctor.sphinx import CacheT, SphinxInventory +import attr + if TYPE_CHECKING: from typing_extensions import Literal from pydoctor.astbuilder import ASTBuilder, DocumentableT @@ -674,6 +676,11 @@ def docsources(self) -> Iterator[Documentable]: def _localNameToFullName(self, name: str) -> str: return self.parent._localNameToFullName(name) +@attr.s(auto_attribs=True) +class PropertyInfo: + setter: Optional['Function'] = None + deleter: Optional['Function'] = None + class Function(Inheritable): kind = DocumentableKind.FUNCTION is_async: bool @@ -681,11 +688,101 @@ class Function(Inheritable): decorators: Optional[Sequence[ast.expr]] signature: Optional[Signature] + # Property handling is special: This attribute is used in the processing step only. + _property_info: Optional[PropertyInfo] = None + def setup(self) -> None: super().setup() if isinstance(self.parent, Class): self.kind = DocumentableKind.METHOD +def init_property(getter: 'Function', + setter: Optional['Function'], + deleter: Optional['Function'], + ) -> None: + """ + Create a L{Attribute} that replaces the property + functions in the documentable tree. + """ + + # avoid cyclic import + from pydoctor import epydoc2stan + + system = getter.system + + # Create an Attribute object for the property + attr = system.Attribute(name=getter.name, system=system, parent=getter.parent) + + attr.parentMod = getter.parentMod + attr.kind = DocumentableKind.PROPERTY + attr.setLineNumber(getter.linenumber) + attr.docstring = getter.docstring + attr.annotation = getter.annotations.get('return') + attr.decorators = getter.decorators + attr.extra_info = getter.extra_info + + # Parse docstring now. + if epydoc2stan.ensure_parsed_docstring(getter): + + pdoc = getter.parsed_docstring + assert pdoc is not None + + other_fields = [] + # process fields such that :returns: clause docs takes the whole docs + # if no global description is written. + for field in pdoc.fields: + tag = field.tag() + if tag == 'return': + if not pdoc.has_body: + pdoc = field.body() + elif tag == 'rtype': + attr.parsed_type = field.body() + else: + other_fields.append(field) + pdoc.fields = other_fields + + # Set the new attribute parsed docstring + attr.parsed_docstring = pdoc + + # We recognize 3 types of properties: + # - read-only + # - read-write + # - read-write-delete + # read-delete-only is not useful to be supported + + def get_property_permission_text(write:bool, delete:bool) -> str: + if not write: + return "This property is *read-only*." + if delete: + return "This property is *readable*, *writable* and *deletable*." + else: + return "This property is *readable* and *writable*." + + parsed_info = epydoc2stan.parse_docstring( + obj=getter, + doc=get_property_permission_text( + write=setter is not None, + delete=deleter is not None), + source=getter, + markup='restructuredtext', + section='property permission text',) + + attr.extra_info.append(parsed_info) + + if setter: + del setter.parent.contents[setter.name] + system._remove(setter) + attr.property_setter = setter + + if deleter: + del deleter.parent.contents[deleter.name] + system._remove(deleter) + attr.property_deleter = deleter + + del getter.parent.contents[getter.name] + system._remove(getter) + system.addObject(attr) + class Attribute(Inheritable): kind: Optional[DocumentableKind] = DocumentableKind.ATTRIBUTE annotation: Optional[ast.expr] @@ -694,8 +791,18 @@ class Attribute(Inheritable): """ The value of the assignment expression. - None value means the value is not initialized at the current point of the the process. + None value means the value is not initialized + at the current point of the the process. + Or maybe it can be that the attribute is a property. + """ + + property_setter: Optional[Function] = None """ + The property setter L{Function}, is any defined. + Only applicable if L{kind} is L{DocumentableKind.PROPERTY} + """ + property_deleter: Optional[Function] = None + """Idem for the deleter.""" # Work around the attributes of the same name within the System class. _ModuleT = Module @@ -1280,7 +1387,16 @@ def postProcess(self) -> None: for b in cls.baseobjects: if b is not None: b.subclasses.append(cls) - + + # Machup property functions into an Attribute. + # Use list() to avoid error "dictionary changed size during iteration" + # Because we are indeed transforming the tree as + # well as the mapping that contains all the objects. + for func in list(self.objectsOfType(Function)): + if func._property_info is not None: + init_property(func, + setter=func._property_info.setter, + deleter=func._property_info.deleter) for post_processor in self._post_processors: post_processor(self) diff --git a/pydoctor/templatewriter/pages/__init__.py b/pydoctor/templatewriter/pages/__init__.py index 828dda33b..c79bfa387 100644 --- a/pydoctor/templatewriter/pages/__init__.py +++ b/pydoctor/templatewriter/pages/__init__.py @@ -256,7 +256,7 @@ def childlist(self) -> List[Union["AttributeChild", "FunctionChild"]]: if isinstance(c, model.Function): r.append(FunctionChild(self.docgetter, c, self.objectExtras(c), func_loader)) elif isinstance(c, model.Attribute): - r.append(AttributeChild(self.docgetter, c, self.objectExtras(c), attr_loader)) + r.append(AttributeChild(self.docgetter, c, self.objectExtras(c), attr_loader, func_loader)) else: assert False, type(c) return r diff --git a/pydoctor/templatewriter/pages/attributechild.py b/pydoctor/templatewriter/pages/attributechild.py index dc08ca6a7..c8f81cbcf 100644 --- a/pydoctor/templatewriter/pages/attributechild.py +++ b/pydoctor/templatewriter/pages/attributechild.py @@ -20,12 +20,14 @@ def __init__(self, docgetter: util.DocGetter, ob: Attribute, extras: List[Tag], - loader: ITemplateLoader + loader: ITemplateLoader, + funcLoader: ITemplateLoader, ): super().__init__(loader) self.docgetter = docgetter self.ob = ob self._functionExtras = extras + self._funcLoader = funcLoader @renderer def class_(self, request: object, tag: Tag) -> "Flattenable": @@ -80,3 +82,18 @@ def constantValue(self, request: object, tag: Tag) -> "Flattenable": return tag.clear() # Attribute is a constant (with a value), then display it's value return epydoc2stan.format_constant_value(self.ob) + + @renderer + def propertyInfo(self, request: object, tag: Tag) -> "Flattenable": + # Property info consist in nested function child elements that + # formats the setter and deleter docs of the property. + r = [] + if self.ob.kind is DocumentableKind.PROPERTY: + from pydoctor.templatewriter.pages.functionchild import FunctionChild + + assert isinstance(self.ob, Attribute) + + for func in [f for f in (self.ob.property_setter, self.ob.property_deleter) if f]: + r.append(FunctionChild(self.docgetter, func, extras=[], + loader=self._funcLoader, silent_undoc=True)) + return r diff --git a/pydoctor/templatewriter/pages/functionchild.py b/pydoctor/templatewriter/pages/functionchild.py index 0766cd1d9..56177d951 100644 --- a/pydoctor/templatewriter/pages/functionchild.py +++ b/pydoctor/templatewriter/pages/functionchild.py @@ -4,6 +4,7 @@ from twisted.web.template import Tag, renderer, tags from pydoctor.model import Function +from pydoctor.epydoc2stan import get_docstring from pydoctor.templatewriter import TemplateElement, util from pydoctor.templatewriter.pages import format_decorators, format_signature @@ -19,12 +20,14 @@ def __init__(self, docgetter: util.DocGetter, ob: Function, extras: List[Tag], - loader: ITemplateLoader + loader: ITemplateLoader, + silent_undoc:bool=False, ): super().__init__(loader) self.docgetter = docgetter self.ob = ob self._functionExtras = extras + self._silent_undoc = silent_undoc @renderer def class_(self, request: object, tag: Tag) -> "Flattenable": @@ -75,5 +78,13 @@ def objectExtras(self, request: object, tag: Tag) -> List[Tag]: @renderer def functionBody(self, request: object, tag: Tag) -> "Flattenable": - return self.docgetter.get(self.ob) - + # Default behaviour + if not self._silent_undoc: + return self.docgetter.get(self.ob) + + # If the function is not documented, do not even show 'Undocumented' + doc, _ = get_docstring(self.ob) + if doc: + return self.docgetter.get(self.ob) + else: + return () diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 58cd0b2f8..395eaaf8b 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -1534,12 +1534,12 @@ def prop(self): assert getter.kind is model.DocumentableKind.PROPERTY assert getter.docstring == """Getter.""" - setter = C.contents['prop.setter'] + setter = getter.property_setter assert isinstance(setter, model.Function) assert setter.kind is model.DocumentableKind.METHOD assert setter.docstring == """Setter.""" - deleter = C.contents['prop.deleter'] + deleter = getter.property_deleter assert isinstance(deleter, model.Function) assert deleter.kind is model.DocumentableKind.METHOD assert deleter.docstring == """Deleter.""" @@ -1971,3 +1971,46 @@ class j: pass assert system.allobjects['top._impl'].resolveName('f') == system.allobjects['top'].contents['f'] assert system.allobjects['_impl2'].resolveName('i') == system.allobjects['top'].contents['i'] assert all(n in system.allobjects['top'].contents for n in ['f', 'g', 'h', 'i', 'j']) + +@systemcls_param +def test_instance_var_override(systemcls: Type[model.System]) -> None: + src = ''' + class A: + _data=None + def data(self): + if self._data is None: + self._data = Data(self) + return self._data + ''' + mod = fromText(src, modname='test', systemcls=systemcls) + assert mod.contents['A'].contents['data'].kind is model.DocumentableKind.METHOD + assert mod.contents['A'].contents['_data'].kind is model.DocumentableKind.INSTANCE_VARIABLE + + src = ''' + class A: + def data(self): + if self._data is None: + self._data = Data(self) + return self._data + _data=None + ''' + mod = fromText(src, modname='test', systemcls=systemcls) + assert mod.contents['A'].contents['data'].kind is model.DocumentableKind.METHOD + assert mod.contents['A'].contents['_data'].kind is model.DocumentableKind.INSTANCE_VARIABLE + +@systemcls_param +def test_instance_var_override_in_property(systemcls: Type[model.System]) -> None: + src = ''' + class A: + _data=None + @property + def data(self): + if self._data is None: + self._data = Data(self) + return self._data + ''' + + mod = fromText(src, modname='propt', systemcls=systemcls) + assert mod.contents['A'].contents['data'].kind is model.DocumentableKind.PROPERTY + assert mod.contents['A'].contents['_data'].kind is model.DocumentableKind.INSTANCE_VARIABLE + diff --git a/pydoctor/test/test_templatewriter.py b/pydoctor/test/test_templatewriter.py index ed9d0f736..c5c2182af 100644 --- a/pydoctor/test/test_templatewriter.py +++ b/pydoctor/test/test_templatewriter.py @@ -13,6 +13,8 @@ HtmlTemplate, UnsupportedTemplateVersion, OverrideTemplateNotAllowed) from pydoctor.templatewriter.pages.table import ChildTable +from pydoctor.templatewriter.pages.attributechild import AttributeChild +from pydoctor.templatewriter.pages.functionchild import FunctionChild from pydoctor.templatewriter.summary import isClassNodePrivate, isPrivate, moduleSummary from pydoctor.test.test_astbuilder import fromText from pydoctor.test.test_packages import processPackage, testpackages @@ -53,6 +55,13 @@ def getHTMLOf(ob: model.Documentable) -> str: wr._writeDocsForOne(ob, f) return f.getvalue().decode() +def getHTMLOfAttribute(ob: model.Documentable) -> str: + assert isinstance(ob, model.Attribute) + tlookup = TemplateLookup(template_dir) + stan = AttributeChild(util.DocGetter(), ob, [], + AttributeChild.lookup_loader(tlookup), + FunctionChild.lookup_loader(tlookup)) + return flatten(stan) def test_sidebar() -> None: src = ''' @@ -626,3 +635,119 @@ def test_objects_order_mixed_modules_and_packages() -> None: assert names == ['aaa', 'aba', 'bbb'] +def test_property_getter_setter_docs() -> None: + + src1 = ''' + class A: + @property + def data(self): + "getter doc" + @data.setter + def data(self): + "setter doc" + @data.deleter + def data(self): + "deleter doc" + ''' + mod1 = fromText(src1, modname='propt') + + # We can only see the property object, the functions got removed. + assert list(mod1.contents['A'].contents)==['data'] + assert not list(mod1.system.objectsOfType(model.Function)) + attr = mod1.contents['A'].contents['data'] + + html = getHTMLOfAttribute(attr) + assert all([part in html for part in ['getter doc','setter doc','deleter doc']]), html + +def test_property_getter_setter_no_undocumented() -> None: + + src2 = ''' + class A: + @property + def data(self): + """ + @returns: the data + """ + @data.setter + def data(self, data): + """ + @param data: the new data + """ + @data.deleter + def data(self): + ... + ''' + mod2 = fromText(src2, modname='propt') + attr = mod2.contents['A'].contents['data'] + + html = getHTMLOfAttribute(attr) + # asserts that no 'Undocumented' shows up! + assert 'Undocumented' not in html + + # This variant does not test anything special: + src3 = ''' + class A: + @property + def data(self): + """ + Get the data. + @returns: the data + """ + @data.setter + def data(self, data): + """ + Sets the data. + @param data: the new value + """ + @data.deleter + def data(self): + "Deletes the data" + ''' + +def test_property_getter_inherits_docs() -> None: + + src4 = ''' + class Base: + data: int + "getter docs" + class A(Base): + @property + def data(self): # inherits docs + return 0 + ''' + mod = fromText(src4, modname='propt') + attr = mod.contents['A'].contents['data'] + + html = getHTMLOfAttribute(attr) + assert 'getter docs' in html + + src5 = ''' + class Base: + @property + def data(self): + "getter docs" + @data.setter + def data(self): + "setter docs" + @data.deleter + def data(self): + "deleter docs" + + class A(Base): + # Inherits docs, but not for setter and deleters, + # This is because overriding the property name also overrides + # the setters and deleters, so they should be given explicit docs, or nothing. + @property + def data(self): # inherits docs + return 0 + @data.deleter + def data(self): # doesn't inherits docs + pass + ''' + + mod = fromText(src5, modname='propt') + attr = mod.contents['A'].contents['data'] + + html = getHTMLOfAttribute(attr) + assert 'getter docs' in html + assert 'deleter docs' not in html diff --git a/pydoctor/themes/base/attribute-child.html b/pydoctor/themes/base/attribute-child.html index 847090c3a..806d6231a 100644 --- a/pydoctor/themes/base/attribute-child.html +++ b/pydoctor/themes/base/attribute-child.html @@ -28,5 +28,6 @@ Value of the attribute if it's a constant. + From 5758a3748cf777ce0988aaa913207b5f49bf99d2 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 5 Aug 2022 00:17:52 -0400 Subject: [PATCH 02/47] More property demo --- docs/epytext_demo/demo_epytext_module.py | 12 ++++++++++-- .../demo_restructuredtext_module.py | 12 ++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/docs/epytext_demo/demo_epytext_module.py b/docs/epytext_demo/demo_epytext_module.py index 2ad9833d9..ac3bef5b6 100644 --- a/docs/epytext_demo/demo_epytext_module.py +++ b/docs/epytext_demo/demo_epytext_module.py @@ -137,13 +137,14 @@ def read_and_write(self) -> int: @read_and_write.setter def read_and_write(self, value: int) -> None: """ - This is a docstring for setter. + This is a docstring for setter. + Their are usually not explicitely documented though. """ @property def read_and_write_delete(self) -> int: """ - This is a read-write-delete property. + This is the docstring of the property. """ return 1 @@ -158,6 +159,13 @@ def read_and_write_delete(self) -> None: """ This is a docstring for deleter. """ + + @property + def undoc_prop(self) -> bytes: + """This property has a docstring only on the setter.""" + @undoc_prop.setter + def undoc_prop(self, p) -> None: # type:ignore + ... class IContact(zope.interface.Interface): diff --git a/docs/restructuredtext_demo/demo_restructuredtext_module.py b/docs/restructuredtext_demo/demo_restructuredtext_module.py index b47c9c148..612636c5c 100644 --- a/docs/restructuredtext_demo/demo_restructuredtext_module.py +++ b/docs/restructuredtext_demo/demo_restructuredtext_module.py @@ -153,13 +153,14 @@ def read_and_write(self) -> int: @read_and_write.setter def read_and_write(self, value: int) -> None: """ - This is a docstring for setter. + This is a docstring for setter. + Their are usually not explicitely documented though. """ @property def read_and_write_delete(self) -> int: """ - This is a read-write-delete property. + This is the docstring of the property. """ return 1 @@ -174,6 +175,13 @@ def read_and_write_delete(self) -> None: """ This is a docstring for deleter. """ + + @property + def undoc_prop(self) -> bytes: + """This property has a docstring only on the setter.""" + @undoc_prop.setter + def undoc_prop(self, p) -> None: # type:ignore + ... class IContact(zope.interface.Interface): """ From eb8f49e6a10f843ae6f4120f43a1952f6ed75c17 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 5 Aug 2022 00:17:57 -0400 Subject: [PATCH 03/47] Fix test --- docs/tests/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/tests/test.py b/docs/tests/test.py index b2d139656..d213379ea 100644 --- a/docs/tests/test.py +++ b/docs/tests/test.py @@ -185,7 +185,7 @@ def test_search(query:str, expected:List[str], order_is_important:bool=True) -> ['pydoctor.model.Class', 'pydoctor.factory.Factory.Class', 'pydoctor.model.DocumentableKind.CLASS', - 'pydoctor.model.System.Class']) + 'pydoctor.model.System.Class'], order_is_important=False) to_stan_results = [ 'pydoctor.epydoc.markup.ParsedDocstring.to_stan', From 51935e6ba11325acd4ca0a0fac52d6f084f2fc88 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 11 Aug 2022 13:39:54 -0400 Subject: [PATCH 04/47] Better understanding properties --- pydoctor/astbuilder.py | 231 +++++++++++++----- pydoctor/astutils.py | 10 +- pydoctor/model.py | 136 +++++++---- pydoctor/test/test_astbuilder.py | 135 +++++++++- .../test/test_twisted_python_deprecate.py | 10 +- 5 files changed, 421 insertions(+), 101 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index bb011046e..8398819e4 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -1,6 +1,7 @@ """Convert ASTs into L{pydoctor.model.Documentable} instances.""" import ast +import enum import sys from functools import partial @@ -13,7 +14,9 @@ ) import astor +import attr from pydoctor import epydoc2stan, model, node2stan +from pydoctor import astutils from pydoctor.epydoc.markup._pyval_repr import colorize_inline_pyval from pydoctor.astutils import NodeVisitor, node2dottedname, node2fullname, is__name__equals__main__, is_using_typing_final @@ -100,6 +103,66 @@ def extract_final_subscript(annotation: ast.Subscript) -> ast.expr: assert isinstance(ann_slice, ast.expr) return ann_slice +def is_property_def_decorator(dottedname:List[str], ctx:model.Documentable) -> bool: + if dottedname[-1].endswith('property') or dottedname[-1].endswith('Property'): + # TODO: Support property subclasses. + return True + return False + +def looks_like_property_func_decorator(deco_name:List[str], ctx:model.Documentable) -> bool: + if len(deco_name) >= 2 and deco_name[-1] in ('getter' ,'setter', 'deleter'): + return True + return False + +def get_inherited_property(_property_decorator:ast.expr, _parent: model.Documentable) -> Optional[model.Attribute]: + """ + Fetch the inherited property that this new decorator overrides. + None if it doesn't exist. + """ + if not get_property_function_kind(_property_decorator): + return None + (deco_name,_), = astutils.iter_decorator_list((_property_decorator,)) + assert deco_name is not None + + _property_name = deco_name[:-1] + + if len(_property_name) <= 1 or _property_name[-1] in _parent.contents: + # the property already exist + return None + + # property_def can be a getter/setter/deleter + _cls = _parent.resolveName('.'.join(_property_name[:-1])) + if _cls is None or not isinstance(_cls, model.Class): + # Can't make sens of property decorator + # the property decorator is pointing to something external OR + # not found in the system yet because of cyclic imports + # OR to something else than a class :/ + # Don't rename it. + return None + + # The class on which the property is defined (_cls) does not have + # to be in the MRO of the parent + property_def = _cls.find(_property_name[-1]) + if not isinstance(property_def, model.Attribute): + return None + + return property_def + +def get_property_function_kind(_property_decorator:ast.expr) -> Optional[model.PropertyFunctionKind]: + """ + What kind of property function this decorator declares? + None if we can't make sens of the decorator. + """ + (deco_name,_), = astutils.iter_decorator_list((_property_decorator,)) + if deco_name: + if deco_name[-1] == 'setter': + return model.PropertyFunctionKind.SETTER + if deco_name[-1] == 'getter': + return model.PropertyFunctionKind.GETTER + if deco_name[-1] == 'deleter': + return model.PropertyFunctionKind.DELETER + return None + class ModuleVistor(NodeVisitor): def __init__(self, builder: 'ASTBuilder', module: model.Module): @@ -720,57 +783,106 @@ def _handleFunctionDef(self, func_name = node.name # determine the function's kind - is_property = False + is_classmethod = False is_staticmethod = False - property_info: Optional[model.PropertyInfo] = None - + + is_property = False # True if is_property_def_decorator() + has_property_decorator = False # True if looks_like_property_func_decorator() + property_decorator: Optional[ast.expr] = None + if isinstance(parent, model.Class) and node.decorator_list: - for d in node.decorator_list: - if isinstance(d, ast.Call): - deco_name = node2dottedname(d.func) - else: - deco_name = node2dottedname(d) + for deco_name,decnode in astutils.iter_decorator_list(node.decorator_list): if deco_name is None: continue - if deco_name[-1].endswith('property') or deco_name[-1].endswith('Property'): + if is_property_def_decorator(deco_name, parent): is_property = True + property_decorator = decnode elif deco_name == ['classmethod']: is_classmethod = True elif deco_name == ['staticmethod']: is_staticmethod = True - elif len(deco_name) >= 2 and deco_name[-1] in ('setter', 'deleter'): - if len(deco_name)==2: - # Setters and deleters must have the same name as the property function - if deco_name[0]==func_name: - property_getter = parent.contents.get(func_name) - - if property_getter is not None: - # Rename the setter/deleter such that - # it does not replace the property getter. - - func_name = '.'.join(deco_name) - - if not isinstance(property_getter, model.Function): - # Can't make sens of decorator ending in .setter/.deleter :/ - # The property setter/deleter is not targeting a function - # We still rename it because it overrides something and it maches - # the rules to be a property. Maybe it's actually targetting a callable - # implemented as a __call__ method or a lamda function. Is it even valid python? - continue - if property_getter._property_info is None: - # Probably an unsupported type of property - continue - - # We have an actual python property: - # Store property info object - property_info = property_getter._property_info - - else: - # Can't make sens of decorator ending in .setter/.deleter :/ - # The decorator is a dotted name of three parts or more, like 'Person.name.setter'. - # Don't do anything special with it, i.e do not rename it. - continue + # Pre-handle property elements + elif looks_like_property_func_decorator(deco_name, parent): + # Setters and deleters should have the same name as the property function, + # otherwise ignore it. + # This pollutes the namespace unnecessarily and is generally not recommended. + # Therefore it makes sense to stick to a single name, + # which is consistent with the former property definition. + if not deco_name[-2] == func_name: + continue + + # Rename the setter/deleter, so it doesn't replace + # the property object. + + func_name = '.'.join(deco_name[-2:]) + has_property_decorator = True + property_decorator = decnode + + prop: Optional[model.Attribute] = None + prop_func_kind: Optional[model.PropertyFunctionKind] = None + is_new_property: bool = is_property + + if is_property and has_property_decorator: + # The function has both @property and @name.getter/setter/delter decorators + pass + + elif is_property: + prop = self.builder.addAttribute(node.name, + kind=model.DocumentableKind.PROPERTY, + parent=parent) + prop.setLineNumber(lineno) + prop.decorators = node.decorator_list + prop_func_kind = model.PropertyFunctionKind.GETTER + # rename func, this might create conflict if some overrides the .getter + func_name = node.name+'.getter' + + elif has_property_decorator: + assert property_decorator is not None + + prop_func_kind = get_property_function_kind(property_decorator) + inherited_property = get_inherited_property(property_decorator, parent) + + if inherited_property: + if inherited_property._property_info: + prop = self.builder.addAttribute(node.name, + kind=model.DocumentableKind.PROPERTY, + parent=parent) + prop.setLineNumber(lineno) + prop.decorators = node.decorator_list + # copy property info + prop._property_info = model.PropertyInfo( + **attr.asdict(inherited_property._property_info)) + is_new_property = True + + elif not prop_func_kind: + # should never go there since the deocrator should looks_like_property_func_decorator() + pass + else: + # fetch property info to add this info to it + maybe_prop = self.builder.current.contents.get(node.name) + if not maybe_prop: + # can't find property + pass + elif not isinstance(maybe_prop, model.Attribute): + # object is not a Attribute + prop = None + elif not maybe_prop._property_info: + # Attribute is not a property + prop = None + else: + prop = maybe_prop + + # Check if this property function is overriding a previously defined + # property function on the same scope before pushing the new function + # If it does override something, delete it before handleDuplicate() trigger a unseless warning. + + if prop is not None and not is_new_property \ + and func_name in parent.contents: + self.system._remove(parent.contents[func_name]) + del parent.contents[func_name] + + # Push and analyse function func = self.builder.pushFunction(func_name, lineno) func.is_async = is_async @@ -829,23 +941,29 @@ def add_arg(name: str, kind: Any, default: Optional[ast.expr]) -> None: func.annotations = self._annotations_from_function(node) - if is_property: - # Init PropertyInfo object when visiting the getter. - func._property_info = model.PropertyInfo() + if is_property or has_property_decorator: + report_on = prop or func if is_classmethod: - func.report(f'{func.fullName()} is both property and classmethod') + report_on.report(f'{report_on.fullName()} is both property and classmethod') if is_staticmethod: - func.report(f'{func.fullName()} is both property and staticmethod') - - # Store property functions to be handled later. - if property_info is not None: - if func_name.endswith('.deleter'): - property_info.deleter = func - elif func_name.endswith('.setter'): - property_info.setter = func - else: - assert False + report_on.report(f'{report_on.fullName()} is both property and staticmethod') + + assert property_decorator is not None + + # TODO: maybe deleter this attribute + func.property_decorator = property_decorator + + if prop is not None and prop_func_kind is not None: + # Store the fact that this function implements one of the getter/setter/deleter + # of the property 'prop'. + assert prop._property_info is not None + prop._property_info.set(prop_func_kind, func) + + # Store the fact that this function declares a + # new property vs adding new functionality on top of getter + if is_new_property: + prop._property_info.declaration = func def depart_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: self.builder.popFunction() @@ -1134,6 +1252,9 @@ def addAttribute(self, parentMod = self.currentMod attr = system.Attribute(system, name, parent) attr.kind = kind + if kind is model.DocumentableKind.PROPERTY: + # init property info if this attribute is a property + attr._property_info = model.PropertyInfo() attr.parentMod = parentMod system.addObject(attr) self.currentAttr = attr diff --git a/pydoctor/astutils.py b/pydoctor/astutils.py index e5aa88446..b3e0d5fbd 100644 --- a/pydoctor/astutils.py +++ b/pydoctor/astutils.py @@ -4,7 +4,7 @@ import sys from numbers import Number -from typing import Iterator, Optional, List, Iterable, Sequence, TYPE_CHECKING +from typing import Iterator, Optional, List, Iterable, Sequence, TYPE_CHECKING, Tuple, Union from inspect import BoundArguments, Signature import ast @@ -155,3 +155,11 @@ def is_using_annotations(expr: Optional[ast.AST], if full_name in annotations: return True return False + +def iter_decorator_list(decorator_list:Iterable[ast.expr]) -> Iterator[Tuple[Optional[List[str]], ast.expr]]: + for d in decorator_list: + if isinstance(d, ast.Call): + deco_name = node2dottedname(d.func) + else: + deco_name = node2dottedname(d) + yield deco_name,d diff --git a/pydoctor/model.py b/pydoctor/model.py index 9cedd0e78..f503bf90a 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -676,10 +676,40 @@ def docsources(self) -> Iterator[Documentable]: def _localNameToFullName(self, name: str) -> str: return self.parent._localNameToFullName(name) +class PropertyFunctionKind(Enum): + GETTER = 1 + SETTER = 2 + DELETER = 3 + @attr.s(auto_attribs=True) class PropertyInfo: + declaration:'Function' = NotImplemented + """ + The function that initially declared this property with C{@property} decorator or by overriding + the a property function with C{@A.name.setter/getter/deleter} from another class. + """ + getter:Optional['Function'] = None + """ + The getter. Generally the same as L{declaration} if it has not been overriden. + """ setter: Optional['Function'] = None + """ + None if it has not been set with C{@name.setter} decorator. + """ deleter: Optional['Function'] = None + """ + None if it has not been set with C{@name.deleter} decorator. + """ + + def set(self, kind:PropertyFunctionKind, func:'Function') -> None: + if kind is PropertyFunctionKind.GETTER: + self.getter = func + elif kind is PropertyFunctionKind.SETTER: + self.setter = func + elif kind is PropertyFunctionKind.DELETER: + self.deleter = func + else: + assert False class Function(Inheritable): kind = DocumentableKind.FUNCTION @@ -689,37 +719,45 @@ class Function(Inheritable): signature: Optional[Signature] # Property handling is special: This attribute is used in the processing step only. - _property_info: Optional[PropertyInfo] = None - + property_decorator: Optional[ast.expr] = None + """ + A property decorator like C{@property} or C{@name.setter} or C{@BaseClass.name.getter} / etc. + + C{None} if this function is not decorated with any kind of property decorator. + """ + def setup(self) -> None: super().setup() if isinstance(self.parent, Class): self.kind = DocumentableKind.METHOD -def init_property(getter: 'Function', - setter: Optional['Function'], - deleter: Optional['Function'], - ) -> None: +def init_property(attr:'Attribute') -> Iterator['Function']: """ - Create a L{Attribute} that replaces the property + Initiates the L{Attribute} that replaces the property functions in the documentable tree. + + Returns the functions to remove from the tree. If the property matchup fails """ + info = attr._property_info + assert info is not None + + getter = info.getter + setter = info.setter + deleter = info.deleter + + if getter is None: + # The getter should never be None + return () # avoid cyclic import from pydoctor import epydoc2stan - - system = getter.system - - # Create an Attribute object for the property - attr = system.Attribute(name=getter.name, system=system, parent=getter.parent) - attr.parentMod = getter.parentMod - attr.kind = DocumentableKind.PROPERTY - attr.setLineNumber(getter.linenumber) + # Setup Attribute object for the property attr.docstring = getter.docstring attr.annotation = getter.annotations.get('return') attr.decorators = getter.decorators - attr.extra_info = getter.extra_info + + attr.extra_info.extend(getter.extra_info) # Parse docstring now. if epydoc2stan.ensure_parsed_docstring(getter): @@ -739,6 +777,7 @@ def init_property(getter: 'Function', attr.parsed_type = field.body() else: other_fields.append(field) + pdoc.fields = other_fields # Set the new attribute parsed docstring @@ -759,34 +798,29 @@ def get_property_permission_text(write:bool, delete:bool) -> str: return "This property is *readable* and *writable*." parsed_info = epydoc2stan.parse_docstring( - obj=getter, + obj=info.declaration, doc=get_property_permission_text( write=setter is not None, delete=deleter is not None), - source=getter, + source=info.declaration, markup='restructuredtext', section='property permission text',) + # TODO: Add inheritence info to getter/setter/deleters attr.extra_info.append(parsed_info) + # Yield the objects to remove from the Documentable tree + yield getter if setter: - del setter.parent.contents[setter.name] - system._remove(setter) - attr.property_setter = setter - + yield setter if deleter: - del deleter.parent.contents[deleter.name] - system._remove(deleter) - attr.property_deleter = deleter + yield deleter - del getter.parent.contents[getter.name] - system._remove(getter) - system.addObject(attr) class Attribute(Inheritable): kind: Optional[DocumentableKind] = DocumentableKind.ATTRIBUTE annotation: Optional[ast.expr] - decorators: Optional[Sequence[ast.expr]] = None + decorators: Optional[Sequence[ast.expr]] = None # decorators are used only if the attribute is a property value: Optional[ast.expr] = None """ The value of the assignment expression. @@ -796,13 +830,24 @@ class Attribute(Inheritable): Or maybe it can be that the attribute is a property. """ - property_setter: Optional[Function] = None - """ - The property setter L{Function}, is any defined. - Only applicable if L{kind} is L{DocumentableKind.PROPERTY} - """ - property_deleter: Optional[Function] = None - """Idem for the deleter.""" + _property_info:Optional[PropertyInfo] = None + + @property + def property_setter(self) -> Optional[Function]: + """ + The property setter L{Function}, is any defined. + Only applicable if L{kind} is L{DocumentableKind.PROPERTY} + """ + if self._property_info: + return self._property_info.setter + return None + + @property + def property_deleter(self) -> Optional[Function]: + """Idem for the deleter.""" + if self._property_info: + return self._property_info.deleter + return None # Work around the attributes of the same name within the System class. _ModuleT = Module @@ -1295,11 +1340,11 @@ class C: if something: def meth(self): implementation 1 - else: + if somethinglelse: def meth(self): implementation 2 - The default is that the second definition "wins". + The default rule is that the last definition "wins". """ i = 0 fullName = obj.fullName() @@ -1392,11 +1437,16 @@ def postProcess(self) -> None: # Use list() to avoid error "dictionary changed size during iteration" # Because we are indeed transforming the tree as # well as the mapping that contains all the objects. - for func in list(self.objectsOfType(Function)): - if func._property_info is not None: - init_property(func, - setter=func._property_info.setter, - deleter=func._property_info.deleter) + to_delete: List[Documentable] = [] + for attr in list(self.objectsOfType(Attribute)): + if attr._property_info is not None: + to_delete.extend(init_property(attr)) + for obj in set(to_delete): + self._remove(obj) + assert '.' in obj.name + assert obj.parent is not None + if obj.name in obj.parent.contents: + del obj.parent.contents[obj.name] for post_processor in self._post_processors: post_processor(self) diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 395eaaf8b..7b83690d4 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -5,7 +5,7 @@ import astor -from pydoctor import astbuilder, model +from pydoctor import astbuilder, model, node2stan from pydoctor.epydoc.markup import DocstringLinker, ParsedDocstring from pydoctor.stanutils import flatten, html2stan, flatten_text from pydoctor.epydoc.markup.epytext import Element, ParsedEpytextDocstring @@ -2014,3 +2014,136 @@ def data(self): assert mod.contents['A'].contents['data'].kind is model.DocumentableKind.PROPERTY assert mod.contents['A'].contents['_data'].kind is model.DocumentableKind.INSTANCE_VARIABLE +@systemcls_param +def test_property_inherited(systemcls: Type[model.System], capsys: CapSys) -> None: + # source from cpython test_property.py + src = ''' + class BaseClass(object): + @property + def spam(self): + """BaseClass.getter""" + pass + @spam.setter + def spam(self, value): + """BaseClass.setter""" + pass + @spam.deleter + def spam(self): + """BaseClass.setter""" + pass + + class SubClass(BaseClass): + # inherited property + @BaseClass.spam.getter + def spam(self): + """SubClass.getter""" + pass + + class SubClass2(BaseClass): + # inherited property + @BaseClass.spam.setter + def spam(self): + """SubClass.setter""" + pass + + class SubClass3(BaseClass): + # inherited property + @BaseClass.spam.deleter + def spam(self): + """SubClass.deleter""" + pass + ''' + mod = fromText(src, modname='mod', systemcls=systemcls) + assert not capsys.readouterr().out + + spam0 = mod.contents['BaseClass'].contents['spam'] + spam1 = mod.contents['SubClass'].contents['spam'] + spam2 = mod.contents['SubClass2'].contents['spam'] + spam3 = mod.contents['SubClass3'].contents['spam'] + + assert list(mod.contents['BaseClass'].contents) == \ + list(mod.contents['SubClass'].contents) == \ + list(mod.contents['SubClass2'].contents) == \ + list(mod.contents['SubClass3'].contents) == ['spam'] + + assert isinstance(spam0, model.Attribute) + assert isinstance(spam1, model.Attribute) + assert isinstance(spam2, model.Attribute) + assert isinstance(spam3, model.Attribute) + + assert spam0.kind is model.DocumentableKind.PROPERTY + assert spam1.kind is model.DocumentableKind.PROPERTY + assert spam2.kind is model.DocumentableKind.PROPERTY + assert spam3.kind is model.DocumentableKind.PROPERTY + + assert isinstance(spam0.property_setter, model.Function) + assert isinstance(spam1.property_setter, model.Function) + assert isinstance(spam2.property_setter, model.Function) + assert isinstance(spam3.property_setter, model.Function) + + assert isinstance(spam0.property_deleter, model.Function) + assert isinstance(spam1.property_deleter, model.Function) + assert isinstance(spam2.property_deleter, model.Function) + assert isinstance(spam3.property_deleter, model.Function) + + assert spam0._property_info.getter.fullName() == 'mod.BaseClass.spam.getter' + assert spam0._property_info.setter.fullName() == 'mod.BaseClass.spam.setter' + assert spam0._property_info.deleter.fullName() == 'mod.BaseClass.spam.deleter' + + assert spam1._property_info.getter.fullName() == 'mod.SubClass.spam.getter' + assert spam1._property_info.setter.fullName() == 'mod.BaseClass.spam.setter' + assert spam1._property_info.deleter.fullName() == 'mod.BaseClass.spam.deleter' + + assert spam2._property_info.getter.fullName() == 'mod.BaseClass.spam.getter' + assert spam2._property_info.setter.fullName() == 'mod.SubClass2.spam.setter' + assert spam2._property_info.deleter.fullName() == 'mod.BaseClass.spam.deleter' + + assert spam3._property_info.getter.fullName() == 'mod.BaseClass.spam.getter' + assert spam3._property_info.setter.fullName() == 'mod.BaseClass.spam.setter' + assert spam3._property_info.deleter.fullName() == 'mod.SubClass3.spam.deleter' + + assert spam1._property_info.getter is not spam0._property_info.getter + +@systemcls_param +def test_property_old_school(systemcls: Type[model.System], capsys: CapSys) -> None: + """ + We don't support it for now. + """ + src = ''' + class PropertyDocBase(object): + _spam = 1 + def _get_spam(self): + return self._spam + # Old school property + spam = property(_get_spam, doc="spam spam spam") + class PropertyDocSub(PropertyDocBase): + @PropertyDocBase.spam.getter + def spam(self): + # The docstring is ignored since the property is defined with old school manner + """The decorator does not use this doc string""" + return self._spam + ''' + +@systemcls_param +def test_property_getter_override(systemcls: Type[model.System], capsys: CapSys) -> None: + + src = ''' + class PropertyNewGetter(object): + + @property + def spam(self): + """original docstring""" + return 1 + @spam.getter + def spam(self): + # This overrides the old docstring. + """new docstring""" + return 8 + ''' + mod = fromText(src, modname='mod', systemcls=systemcls) + assert not capsys.readouterr().out + assert node2stan.gettext( + mod.contents['PropertyNewGetter'].contents['spam']\ + .parsed_docstring.to_node()) == 'new docstring' + +# TODO: Add corner cases test diff --git a/pydoctor/test/test_twisted_python_deprecate.py b/pydoctor/test/test_twisted_python_deprecate.py index 2a42e97b4..58c9e00a5 100644 --- a/pydoctor/test/test_twisted_python_deprecate.py +++ b/pydoctor/test/test_twisted_python_deprecate.py @@ -2,7 +2,7 @@ import re from typing import Type -from pydoctor import model +from pydoctor import model, node2stan from pydoctor.stanutils import flatten_text, html2stan from pydoctor.test import CapSys, test_templatewriter from pydoctor.test.test_astbuilder import fromText, DeprecateSystem @@ -79,6 +79,14 @@ class stuff: ... name='Baz', package='Twisted', version=r'14\.2\.3', replacement='stuff' ), class_html_text, re.DOTALL), class_html_text + foom_deprecated_property = _class.contents['foom'] + assert isinstance(foom_deprecated_property, model.Attribute) + info_text = [' '.join(node2stan.gettext(i.to_node())) for i in foom_deprecated_property.extra_info] + assert len(info_text) == 2, info_text + assert info_text == [ + 'Deprecated since version NEXT: foom was deprecated in Twisted NEXT; please use faam instead.', + 'This property is read-only .', ] + assert re.match(_html_template_with_replacement.format( name='foom', package='Twisted', version=r'NEXT', replacement='faam' ), class_html_text, re.DOTALL), class_html_text From 11ac56124f5aa9e88ee83480806a050f511f7feb Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 11 Aug 2022 15:09:00 -0400 Subject: [PATCH 05/47] fix test --- pydoctor/test/test_astbuilder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 7b83690d4..43a482adb 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2144,6 +2144,6 @@ def spam(self): assert not capsys.readouterr().out assert node2stan.gettext( mod.contents['PropertyNewGetter'].contents['spam']\ - .parsed_docstring.to_node()) == 'new docstring' + .parsed_docstring.to_node()) == ['new docstring'] # TODO: Add corner cases test From 4c812acc4280a5d9c1c0ecf5cbb1d9771af422ea Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 12 Aug 2022 10:54:59 -0400 Subject: [PATCH 06/47] More tests --- pydoctor/test/test_astbuilder.py | 159 ++++++++++++++++++++++++++++++- 1 file changed, 155 insertions(+), 4 deletions(-) diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 43a482adb..4f38b9b05 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2016,6 +2016,9 @@ def data(self): @systemcls_param def test_property_inherited(systemcls: Type[model.System], capsys: CapSys) -> None: + """ + Properties can be inherited. + """ # source from cpython test_property.py src = ''' class BaseClass(object): @@ -2126,7 +2129,10 @@ def spam(self): @systemcls_param def test_property_getter_override(systemcls: Type[model.System], capsys: CapSys) -> None: - + """ + A function that explicitely overides a property getter will override the docstring as well. + But not the line number. + """ src = ''' class PropertyNewGetter(object): @@ -2142,8 +2148,153 @@ def spam(self): ''' mod = fromText(src, modname='mod', systemcls=systemcls) assert not capsys.readouterr().out - assert node2stan.gettext( - mod.contents['PropertyNewGetter'].contents['spam']\ - .parsed_docstring.to_node()) == ['new docstring'] + attr = mod.contents['PropertyNewGetter'].contents['spam'] + # the parsed_docstring attribute gets initiated in post-processing + assert node2stan.gettext(attr.parsed_docstring.to_node()) == ['new docstring'] + assert attr.linenumber == 4 # TODO: Add corner cases test + +@systemcls_param +def test_property_corner_cases(systemcls: Type[model.System], capsys: CapSys) -> None: + """ + Property handling can be quite complex, there are many corner cases. + """ + + base_mod = ''' + # two modules form a cyclic import, + # so this class might not be understood well by pydoctor. + # Since the cycles can be arbitrarly complex, we just don't + # go in the details of resolving them. + from src import BaseClass + + class System: + pass + + class SubClass(BaseClass): + # cyclic inherited property + @BaseClass.spam.getter + def spam(self): + pass + ''' + + src_mod = ''' + from typing import TYPE_CHECKING + if TYPE_CHECKING: + from mod import System + + class BaseClass(object): + system: 'System' + + @property + def spam(self): + """BaseClass.getter""" + pass + @spam.setter + def spam(self, value): + """BaseClass.setter""" + pass + @spam.deleter + def spam(self): + """BaseClass.setter""" + pass + + class SubClass(BaseClass): + # inherited property + @BaseClass.spam.getter + def spam(self): + """SubClass.getter""" + pass + + # this class is valid, even if not very clear + class NotSubClass: + # does not need to explicitely subclass SubClass2 + @SubClass.spam.setter # Valid once! + def spam(self, v): + pass + @spam.getter + def spam(self): # inherits docs + pass + + class InvalidClass: + @SubClass.spam.setter # Valid once! + def spam(self, v): + pass + @SubClass.spam.getter # Not valid if there is already a property defined ! + def spam(self): + pass + + class InvalidClass2: + @notfound.getter + def notfound(self): + pass + + class InvalidClass3: + @property + @notfound.getter + def notfound(self): + pass + + class InvalidClass4: + @InvalidClass3.nhaaa.getter + def notfound(self): + pass + + class InvalidClass5: + @InvalidClass2.notfound.getter + def notfound(self): + pass + ''' + + system = systemcls() + builder = system.systemBuilder(system) + + # modules are processed in the order they are discovered/added + # to the builder, so by adding 'src_mod' fist, we're sure + # the other module won't be fully processed at the time we're + # processing 'src_mod', because imports triggers processing of + # imported modules, except in the case of cycles. + builder.addModuleString(src_mod, modname='src') + builder.addModuleString(base_mod, modname='mod') + builder.buildModules() + + assert not capsys.readouterr().out + + mod = system.allobjects['mod'] + src = system.allobjects['src'] + + # Pydoctor doesn't understand this property because it's using an + # import cycle. So the older behaviour applies: + # only renaming the method + assert list(mod.contents['SubClass'].contents) == ['spam.getter'] + assert mod.contents['SubClass'].contents['spam.getter'].kind is model.DocumentableKind.METHOD + + assert list(src.contents['SubClass'].contents) == ['spam'] + assert list(src.contents['NotSubClass'].contents) == ['spam'] + + spam0 = src.contents['SubClass'].contents['spam'] + spam1 = src.contents['NotSubClass'].contents['spam'] + + assert list(src.contents['InvalidClass'].contents) == ['spam', 'spam.getter'] + + spam2 = src.contents['InvalidClass'].contents['spam'] + spam2_bogus = src.contents['InvalidClass'].contents['spam.getter'] + + notfound = src.contents['InvalidClass2'].contents['notfound.getter'] + notfound_both = src.contents['InvalidClass3'].contents['notfound.getter'] + + notfound_alt = src.contents['InvalidClass4'].contents['notfound'] + not_a_property = src.contents['InvalidClass5'].contents['notfound.getter'] + + assert spam0.kind is model.DocumentableKind.PROPERTY + assert spam1.kind is model.DocumentableKind.PROPERTY + assert spam2.kind is model.DocumentableKind.PROPERTY + assert spam2_bogus.kind is model.DocumentableKind.METHOD + assert notfound.kind is model.DocumentableKind.METHOD + assert notfound_both.kind is model.DocumentableKind.METHOD + assert notfound_alt.kind is model.DocumentableKind.METHOD + assert not_a_property.kind is model.DocumentableKind.METHOD + + + + From 6935be1e0509e9fc81e4339b876408104ba1dba7 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 12 Aug 2022 10:55:41 -0400 Subject: [PATCH 07/47] Don't use list() to create a list of attribute to initiate. --- pydoctor/model.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index f503bf90a..eab96c372 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -733,8 +733,7 @@ def setup(self) -> None: def init_property(attr:'Attribute') -> Iterator['Function']: """ - Initiates the L{Attribute} that replaces the property - functions in the documentable tree. + Initiates the L{Attribute} that represent the property in the tree. Returns the functions to remove from the tree. If the property matchup fails """ @@ -1433,12 +1432,10 @@ def postProcess(self) -> None: if b is not None: b.subclasses.append(cls) - # Machup property functions into an Attribute. - # Use list() to avoid error "dictionary changed size during iteration" - # Because we are indeed transforming the tree as - # well as the mapping that contains all the objects. + # Machup property functions into an Attribute that already exist. + # We are transforming the tree at the end only. to_delete: List[Documentable] = [] - for attr in list(self.objectsOfType(Attribute)): + for attr in self.objectsOfType(Attribute): if attr._property_info is not None: to_delete.extend(init_property(attr)) for obj in set(to_delete): From 32554af62727929fcd250b9605f854afc0a60310 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 12 Aug 2022 10:56:12 -0400 Subject: [PATCH 08/47] Better checks before adding new inherited elements to property --- pydoctor/astbuilder.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 8398819e4..807f16271 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -1,7 +1,6 @@ """Convert ASTs into L{pydoctor.model.Documentable} instances.""" import ast -import enum import sys from functools import partial @@ -840,11 +839,13 @@ def _handleFunctionDef(self, elif has_property_decorator: assert property_decorator is not None + (deco_name,_), = astutils.iter_decorator_list((property_decorator,)) prop_func_kind = get_property_function_kind(property_decorator) inherited_property = get_inherited_property(property_decorator, parent) - if inherited_property: - if inherited_property._property_info: + # Looks like inherited property + if len(deco_name)>2: + if inherited_property and inherited_property._property_info: prop = self.builder.addAttribute(node.name, kind=model.DocumentableKind.PROPERTY, parent=parent) @@ -941,20 +942,19 @@ def add_arg(name: str, kind: Any, default: Optional[ast.expr]) -> None: func.annotations = self._annotations_from_function(node) - if is_property or has_property_decorator: - report_on = prop or func + if prop is not None: if is_classmethod: - report_on.report(f'{report_on.fullName()} is both property and classmethod') + prop.report(f'{prop.fullName()} is both property and classmethod') if is_staticmethod: - report_on.report(f'{report_on.fullName()} is both property and staticmethod') + prop.report(f'{prop.fullName()} is both property and staticmethod') assert property_decorator is not None # TODO: maybe deleter this attribute func.property_decorator = property_decorator - if prop is not None and prop_func_kind is not None: + if prop_func_kind is not None: # Store the fact that this function implements one of the getter/setter/deleter # of the property 'prop'. assert prop._property_info is not None From b3c6d01b0bdd3281e5bb3e04ad42e5dde922c54a Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Tue, 16 Aug 2022 11:08:59 -0400 Subject: [PATCH 09/47] Add overriden properties to the RST demo. --- .../demo_restructuredtext_module.py | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/docs/restructuredtext_demo/demo_restructuredtext_module.py b/docs/restructuredtext_demo/demo_restructuredtext_module.py index 612636c5c..23e7028dd 100644 --- a/docs/restructuredtext_demo/demo_restructuredtext_module.py +++ b/docs/restructuredtext_demo/demo_restructuredtext_module.py @@ -105,6 +105,22 @@ def _private_inside_private(self) -> List[str]: :rtype: `list` """ return [] + + @property + def isPrivate(self) -> bool: + """Whether this class is private""" + return True + @isPrivate.setter + def isPrivate(self, v) -> bool: + raise NotImplemented() + + @property + def isPublic(self) -> bool: + """Whether this class is public""" + return False + @isPublic.setter + def isPublic(self, v) -> bool: + raise NotImplemented() @@ -182,6 +198,14 @@ def undoc_prop(self) -> bytes: @undoc_prop.setter def undoc_prop(self, p) -> None: # type:ignore ... + + @property + def isPrivate(self) -> bool: + return False + + @_PrivateClass.isPublic.setter + def isPublic(self, v): + self._v = v class IContact(zope.interface.Interface): """ From 4fb19c29c0330abb3ac1a322ce7f094c116184f2 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 18 Nov 2022 00:49:47 -0500 Subject: [PATCH 10/47] fix import --- pydoctor/templatewriter/pages/attributechild.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/templatewriter/pages/attributechild.py b/pydoctor/templatewriter/pages/attributechild.py index f92a997b7..35421549f 100644 --- a/pydoctor/templatewriter/pages/attributechild.py +++ b/pydoctor/templatewriter/pages/attributechild.py @@ -3,7 +3,7 @@ from twisted.web.iweb import ITemplateLoader from twisted.web.template import Tag, renderer, tags -from pydoctor.model import Attribute +from pydoctor.model import Attribute, DocumentableKind from pydoctor import epydoc2stan from pydoctor.templatewriter import TemplateElement, util from pydoctor.templatewriter.pages import format_decorators From a3f2519fd936f39fa909bcd0adbcacdb2726afdb Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 18 Nov 2022 00:49:57 -0500 Subject: [PATCH 11/47] add assert --- pydoctor/astbuilder.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 6761914ca..1f45c8576 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -890,6 +890,7 @@ def _handleFunctionDef(self, inherited_property = get_inherited_property(property_decorator, parent) # Looks like inherited property + assert deco_name if len(deco_name)>2: if inherited_property and inherited_property._property_info: prop = self.builder.addAttribute(node.name, From bdd449fa0f559d88bb92dc3218d12424e6d324c4 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 18 Nov 2022 10:26:22 -0500 Subject: [PATCH 12/47] fix mypy --- pydoctor/test/test_astbuilder.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index d13e4aebd..8771db732 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2127,23 +2127,23 @@ def spam(self): assert isinstance(spam2.property_deleter, model.Function) assert isinstance(spam3.property_deleter, model.Function) - assert spam0._property_info.getter.fullName() == 'mod.BaseClass.spam.getter' - assert spam0._property_info.setter.fullName() == 'mod.BaseClass.spam.setter' - assert spam0._property_info.deleter.fullName() == 'mod.BaseClass.spam.deleter' + assert spam0._property_info.getter.fullName() == 'mod.BaseClass.spam.getter' # type:ignore + assert spam0._property_info.setter.fullName() == 'mod.BaseClass.spam.setter' # type:ignore + assert spam0._property_info.deleter.fullName() == 'mod.BaseClass.spam.deleter' # type:ignore - assert spam1._property_info.getter.fullName() == 'mod.SubClass.spam.getter' - assert spam1._property_info.setter.fullName() == 'mod.BaseClass.spam.setter' - assert spam1._property_info.deleter.fullName() == 'mod.BaseClass.spam.deleter' + assert spam1._property_info.getter.fullName() == 'mod.SubClass.spam.getter' # type:ignore + assert spam1._property_info.setter.fullName() == 'mod.BaseClass.spam.setter' # type:ignore + assert spam1._property_info.deleter.fullName() == 'mod.BaseClass.spam.deleter' # type:ignore - assert spam2._property_info.getter.fullName() == 'mod.BaseClass.spam.getter' - assert spam2._property_info.setter.fullName() == 'mod.SubClass2.spam.setter' - assert spam2._property_info.deleter.fullName() == 'mod.BaseClass.spam.deleter' + assert spam2._property_info.getter.fullName() == 'mod.BaseClass.spam.getter' # type:ignore + assert spam2._property_info.setter.fullName() == 'mod.SubClass2.spam.setter' # type:ignore + assert spam2._property_info.deleter.fullName() == 'mod.BaseClass.spam.deleter' # type:ignore - assert spam3._property_info.getter.fullName() == 'mod.BaseClass.spam.getter' - assert spam3._property_info.setter.fullName() == 'mod.BaseClass.spam.setter' - assert spam3._property_info.deleter.fullName() == 'mod.SubClass3.spam.deleter' + assert spam3._property_info.getter.fullName() == 'mod.BaseClass.spam.getter' # type:ignore + assert spam3._property_info.setter.fullName() == 'mod.BaseClass.spam.setter' # type:ignore + assert spam3._property_info.deleter.fullName() == 'mod.SubClass3.spam.deleter' # type:ignore - assert spam1._property_info.getter is not spam0._property_info.getter + assert spam1._property_info.getter is not spam0._property_info.getter # type:ignore @systemcls_param def test_property_old_school(systemcls: Type[model.System], capsys: CapSys) -> None: @@ -2188,7 +2188,7 @@ def spam(self): assert not capsys.readouterr().out attr = mod.contents['PropertyNewGetter'].contents['spam'] # the parsed_docstring attribute gets initiated in post-processing - assert node2stan.gettext(attr.parsed_docstring.to_node()) == ['new docstring'] + assert node2stan.gettext(attr.parsed_docstring.to_node()) == ['new docstring'] # type:ignore assert attr.linenumber == 4 @systemcls_param From 67ec35896927391c4884657aaa3cbd93199b0b67 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 18 Nov 2022 12:00:52 -0500 Subject: [PATCH 13/47] simplify the code a little bit --- pydoctor/astbuilder.py | 107 ++++++++++++++----------------- pydoctor/model.py | 3 +- pydoctor/test/test_astbuilder.py | 27 ++++++++ 3 files changed, 75 insertions(+), 62 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index bf4689661..4cca3650e 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -149,27 +149,32 @@ def extract_final_subscript(annotation: ast.Subscript) -> ast.expr: return ann_slice def is_property_def_decorator(dottedname:List[str], ctx:model.Documentable) -> bool: - if dottedname[-1].endswith('property') or dottedname[-1].endswith('Property'): + """ + Whether the last element of the list of names finishes by "property" or "Property". + """ + if len(dottedname) >= 1 and (dottedname[-1].endswith('property') or dottedname[-1].endswith('Property')): # TODO: Support property subclasses. return True return False -def looks_like_property_func_decorator(deco_name:List[str], ctx:model.Documentable) -> bool: - if len(deco_name) >= 2 and deco_name[-1] in ('getter' ,'setter', 'deleter'): +def looks_like_property_func_decorator(dottedname:List[str], ctx:model.Documentable) -> bool: + """ + Whether the last element of the list of names is "getter", "setter" or "deleter". + Won't match C{['geter']}, because we require the list to have a least 2 elements. + """ + if len(dottedname) >= 2 and dottedname[-1] in ('getter' ,'setter', 'deleter'): return True return False -def get_inherited_property(_property_decorator:ast.expr, _parent: model.Documentable) -> Optional[model.Attribute]: +def get_inherited_property(dottedname:List[str], _parent: model.Documentable) -> Optional[model.Attribute]: """ Fetch the inherited property that this new decorator overrides. None if it doesn't exist. """ - if not get_property_function_kind(_property_decorator): + if not get_property_function_kind(dottedname): return None - (deco_name,_), = astutils.iter_decorator_list((_property_decorator,)) - assert deco_name is not None - _property_name = deco_name[:-1] + _property_name = dottedname[:-1] if len(_property_name) <= 1 or _property_name[-1] in _parent.contents: # the property already exist @@ -193,18 +198,17 @@ def get_inherited_property(_property_decorator:ast.expr, _parent: model.Document return property_def -def get_property_function_kind(_property_decorator:ast.expr) -> Optional[model.PropertyFunctionKind]: +def get_property_function_kind(dottedname:List[str]) -> Optional[model.PropertyFunctionKind]: """ What kind of property function this decorator declares? None if we can't make sens of the decorator. """ - (deco_name,_), = astutils.iter_decorator_list((_property_decorator,)) - if deco_name: - if deco_name[-1] == 'setter': + if dottedname: + if dottedname[-1] == 'setter': return model.PropertyFunctionKind.SETTER - if deco_name[-1] == 'getter': + if dottedname[-1] == 'getter': return model.PropertyFunctionKind.GETTER - if deco_name[-1] == 'deleter': + if dottedname[-1] == 'deleter': return model.PropertyFunctionKind.DELETER return None @@ -832,16 +836,15 @@ def _handleFunctionDef(self, is_staticmethod = False is_overload_func = False - is_property = False # True if is_property_def_decorator() - has_property_decorator = False # True if looks_like_property_func_decorator() - property_decorator: Optional[ast.expr] = None + is_property = False + property_decorator_dottedname: Optional[List[str]] = None if node.decorator_list: - for deco_name,decnode in astutils.iter_decorator_list(node.decorator_list): + for deco_name, _ in astutils.iter_decorator_list(node.decorator_list): if deco_name is None: continue if isinstance(parent, model.Class): - if deco_name[-1].endswith('property') or deco_name[-1].endswith('Property'): + if is_property_def_decorator(deco_name, parent): is_property = True elif deco_name == ['classmethod']: is_classmethod = True @@ -861,8 +864,7 @@ def _handleFunctionDef(self, # the property object. func_name = '.'.join(deco_name[-2:]) - has_property_decorator = True - property_decorator = decnode + property_decorator_dottedname = deco_name # Determine if the function is decorated with overload if parent.expandName('.'.join(deco_name)) in ('typing.overload', 'typing_extensions.overload'): @@ -872,31 +874,14 @@ def _handleFunctionDef(self, prop: Optional[model.Attribute] = None prop_func_kind: Optional[model.PropertyFunctionKind] = None is_new_property: bool = is_property - - if is_property and has_property_decorator: - # The function has both @property and @name.getter/setter/delter decorators - pass - elif is_property: - prop = self.builder.addAttribute(node.name, - kind=model.DocumentableKind.PROPERTY, - parent=parent) - prop.setLineNumber(lineno) - prop.decorators = node.decorator_list - prop_func_kind = model.PropertyFunctionKind.GETTER - # rename func, this might create conflict if some overrides the .getter - func_name = node.name+'.getter' - - elif has_property_decorator: - assert property_decorator is not None - - (deco_name,_), = astutils.iter_decorator_list((property_decorator,)) - prop_func_kind = get_property_function_kind(property_decorator) - inherited_property = get_inherited_property(property_decorator, parent) + if property_decorator_dottedname is not None: + prop_func_kind = get_property_function_kind(property_decorator_dottedname) + # Looks like inherited property - assert deco_name - if len(deco_name)>2: + if len(property_decorator_dottedname)>2: + inherited_property = get_inherited_property(property_decorator_dottedname, parent) if inherited_property and inherited_property._property_info: prop = self.builder.addAttribute(node.name, kind=model.DocumentableKind.PROPERTY, @@ -908,9 +893,6 @@ def _handleFunctionDef(self, **attr.asdict(inherited_property._property_info)) is_new_property = True - elif not prop_func_kind: - # should never go there since the deocrator should looks_like_property_func_decorator() - pass else: # fetch property info to add this info to it maybe_prop = self.builder.current.contents.get(node.name) @@ -926,12 +908,21 @@ def _handleFunctionDef(self, else: prop = maybe_prop + elif is_property: + prop = self.builder.addAttribute(node.name, + kind=model.DocumentableKind.PROPERTY, + parent=parent) + prop.setLineNumber(lineno) + prop.decorators = node.decorator_list + prop_func_kind = model.PropertyFunctionKind.GETTER + # rename func, this might create conflict if some overrides the .getter + func_name = node.name+'.getter' + # Check if this property function is overriding a previously defined # property function on the same scope before pushing the new function # If it does override something, delete it before handleDuplicate() trigger a unseless warning. - if prop is not None and not is_new_property \ - and func_name in parent.contents: + and func_name in parent.contents: self.system._remove(parent.contents[func_name]) del parent.contents[func_name] @@ -1015,6 +1006,14 @@ def add_arg(name: str, kind: Any, default: Optional[ast.expr]) -> None: func.report(f'{func.fullName()} has invalid parameters: {ex}') signature = Signature() + func.annotations = annotations + + # Only set main function signature if it is a non-overload + if is_overload_func: + func.overloads.append(model.FunctionOverload(primary=func, signature=signature, decorators=node.decorator_list)) + else: + func.signature = signature + if prop is not None: if is_classmethod: @@ -1022,11 +1021,6 @@ def add_arg(name: str, kind: Any, default: Optional[ast.expr]) -> None: if is_staticmethod: prop.report(f'{prop.fullName()} is both property and staticmethod') - # assert property_decorator is not None it actually can be None - - # # TODO: maybe deleter this attribute - # func.property_decorator = property_decorator - if prop_func_kind is not None: # Store the fact that this function implements one of the getter/setter/deleter # of the property 'prop'. @@ -1038,13 +1032,6 @@ def add_arg(name: str, kind: Any, default: Optional[ast.expr]) -> None: if is_new_property: prop._property_info.declaration = func - func.annotations = annotations - - # Only set main function signature if it is a non-overload - if is_overload_func: - func.overloads.append(model.FunctionOverload(primary=func, signature=signature, decorators=node.decorator_list)) - else: - func.signature = signature def depart_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: self.builder.popFunction() diff --git a/pydoctor/model.py b/pydoctor/model.py index e7de8b49a..7831c3e2b 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -716,7 +716,7 @@ class PropertyInfo: declaration:'Function' = NotImplemented """ The function that initially declared this property with C{@property} decorator or by overriding - the a property function with C{@A.name.setter/getter/deleter} from another class. + the property function with C{@A.name.setter/getter/deleter} from another class. """ getter:Optional['Function'] = None """ @@ -1490,7 +1490,6 @@ def postProcess(self) -> None: to_delete.extend(init_property(attr)) for obj in set(to_delete): self._remove(obj) - assert '.' in obj.name assert obj.parent is not None if obj.name in obj.parent.contents: del obj.parent.contents[obj.name] diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 8771db732..840c665e7 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2472,3 +2472,30 @@ def test_prepend_package_real_path(systemcls: Type[model.System]) -> None: finally: systemcls.systemBuilder = _builderT_init +@systemcls_param +def test_property_other_decorators(systemcls: Type[model.System]) -> None: + + src = ''' + class BaseClass(object): + @property + @deprecatedProperty + def spam(self): + """BaseClass.getter""" + pass + @spam.setter + @deprecatedSetProperty + def spam(self, value): + """BaseClass.setter""" + pass + @deprecatedDelProperty + @spam.deleter + def spam(self): + """BaseClass.setter""" + pass + ''' + + mod = fromText(src, systemcls=systemcls) + p = mod.contents['BaseClass'].contents['spam'] + assert isinstance(p, model.Attribute) + assert isinstance(p.property_setter, model.Function) + assert isinstance(p.property_deleter, model.Function) \ No newline at end of file From dbf0477c01cb1ff66f1e6761b60103b26d1f3249 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 18 Nov 2022 13:43:16 -0500 Subject: [PATCH 14/47] simplify again --- pydoctor/astbuilder.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 4cca3650e..9163281db 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -918,14 +918,6 @@ def _handleFunctionDef(self, # rename func, this might create conflict if some overrides the .getter func_name = node.name+'.getter' - # Check if this property function is overriding a previously defined - # property function on the same scope before pushing the new function - # If it does override something, delete it before handleDuplicate() trigger a unseless warning. - if prop is not None and not is_new_property \ - and func_name in parent.contents: - self.system._remove(parent.contents[func_name]) - del parent.contents[func_name] - # Push and analyse function # Check if it's a new func or exists with an overload @@ -941,6 +933,12 @@ def _handleFunctionDef(self, # Do not recreate function object, just re-push it self.builder.push(existing_func, lineno) func = existing_func + elif isinstance(existing_func, model.Function) and prop is not None and not is_new_property: + # Check if this property function is overriding a previously defined + # property function on the same scope before pushing the new function + # If it does override something, just re-push the function, do not override it. + self.builder.push(existing_func, lineno) + func = existing_func else: func = self.builder.pushFunction(func_name, lineno) From e7c73a5c25be764f15ccccb77b9a01ec03e31059 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 18 Nov 2022 13:55:40 -0500 Subject: [PATCH 15/47] Simplify again, no need to tell the users the property is read-write if we show the .setter function. It must be writable. --- pydoctor/model.py | 26 +------------------ .../test/test_twisted_python_deprecate.py | 5 +--- 2 files changed, 2 insertions(+), 29 deletions(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index 7831c3e2b..65a570774 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -823,32 +823,8 @@ def init_property(attr:'Attribute') -> Iterator['Function']: # Set the new attribute parsed docstring attr.parsed_docstring = pdoc - - # We recognize 3 types of properties: - # - read-only - # - read-write - # - read-write-delete - # read-delete-only is not useful to be supported - - def get_property_permission_text(write:bool, delete:bool) -> str: - if not write: - return "This property is *read-only*." - if delete: - return "This property is *readable*, *writable* and *deletable*." - else: - return "This property is *readable* and *writable*." - - parsed_info = epydoc2stan.parse_docstring( - obj=info.declaration, - doc=get_property_permission_text( - write=setter is not None, - delete=deleter is not None), - source=info.declaration, - markup='restructuredtext', - section='property permission text',) - + # TODO: Add inheritence info to getter/setter/deleters - attr.extra_info.append(parsed_info) # Yield the objects to remove from the Documentable tree yield getter diff --git a/pydoctor/test/test_twisted_python_deprecate.py b/pydoctor/test/test_twisted_python_deprecate.py index 58c9e00a5..f3d2c44fe 100644 --- a/pydoctor/test/test_twisted_python_deprecate.py +++ b/pydoctor/test/test_twisted_python_deprecate.py @@ -82,10 +82,7 @@ class stuff: ... foom_deprecated_property = _class.contents['foom'] assert isinstance(foom_deprecated_property, model.Attribute) info_text = [' '.join(node2stan.gettext(i.to_node())) for i in foom_deprecated_property.extra_info] - assert len(info_text) == 2, info_text - assert info_text == [ - 'Deprecated since version NEXT: foom was deprecated in Twisted NEXT; please use faam instead.', - 'This property is read-only .', ] + assert 'Deprecated since version NEXT: foom was deprecated in Twisted NEXT; please use faam instead.' in info_text assert re.match(_html_template_with_replacement.format( name='foom', package='Twisted', version=r'NEXT', replacement='faam' From d0842247e0ca8043e3c72fced613a78c63f9cb75 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 18 Nov 2022 16:31:07 -0500 Subject: [PATCH 16/47] Simplify more --- pydoctor/astbuilder.py | 50 +++++++++++++++--------------------------- pydoctor/model.py | 10 +++------ 2 files changed, 21 insertions(+), 39 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 9163281db..2fff0d696 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -871,7 +871,7 @@ def _handleFunctionDef(self, is_overload_func = True - prop: Optional[model.Attribute] = None + func_property: Optional[model.Attribute] = None prop_func_kind: Optional[model.PropertyFunctionKind] = None is_new_property: bool = is_property @@ -883,39 +883,30 @@ def _handleFunctionDef(self, if len(property_decorator_dottedname)>2: inherited_property = get_inherited_property(property_decorator_dottedname, parent) if inherited_property and inherited_property._property_info: - prop = self.builder.addAttribute(node.name, + func_property = self.builder.addAttribute(node.name, kind=model.DocumentableKind.PROPERTY, parent=parent) - prop.setLineNumber(lineno) - prop.decorators = node.decorator_list + func_property.setLineNumber(lineno) + func_property.decorators = node.decorator_list # copy property info - prop._property_info = model.PropertyInfo( + func_property._property_info = model.PropertyInfo( **attr.asdict(inherited_property._property_info)) is_new_property = True else: # fetch property info to add this info to it maybe_prop = self.builder.current.contents.get(node.name) - if not maybe_prop: - # can't find property - pass - elif not isinstance(maybe_prop, model.Attribute): - # object is not a Attribute - prop = None - elif not maybe_prop._property_info: - # Attribute is not a property - prop = None - else: - prop = maybe_prop + if isinstance(maybe_prop, model.Attribute) and maybe_prop._property_info: + func_property = maybe_prop elif is_property: - prop = self.builder.addAttribute(node.name, + func_property = self.builder.addAttribute(node.name, kind=model.DocumentableKind.PROPERTY, parent=parent) - prop.setLineNumber(lineno) - prop.decorators = node.decorator_list + func_property.setLineNumber(lineno) + func_property.decorators = node.decorator_list prop_func_kind = model.PropertyFunctionKind.GETTER - # rename func, this might create conflict if some overrides the .getter + # rename func X.getter func_name = node.name+'.getter' # Push and analyse function @@ -933,13 +924,14 @@ def _handleFunctionDef(self, # Do not recreate function object, just re-push it self.builder.push(existing_func, lineno) func = existing_func - elif isinstance(existing_func, model.Function) and prop is not None and not is_new_property: + elif isinstance(existing_func, model.Function) and func_property is not None and not is_new_property: # Check if this property function is overriding a previously defined # property function on the same scope before pushing the new function # If it does override something, just re-push the function, do not override it. self.builder.push(existing_func, lineno) func = existing_func else: + # create new function func = self.builder.pushFunction(func_name, lineno) func.is_async = is_async @@ -1012,23 +1004,17 @@ def add_arg(name: str, kind: Any, default: Optional[ast.expr]) -> None: else: func.signature = signature - if prop is not None: + if func_property is not None: if is_classmethod: - prop.report(f'{prop.fullName()} is both property and classmethod') + func_property.report(f'{func_property.fullName()} is both property and classmethod') if is_staticmethod: - prop.report(f'{prop.fullName()} is both property and staticmethod') + func_property.report(f'{func_property.fullName()} is both property and staticmethod') if prop_func_kind is not None: # Store the fact that this function implements one of the getter/setter/deleter - # of the property 'prop'. - assert prop._property_info is not None - prop._property_info.set(prop_func_kind, func) - - # Store the fact that this function declares a - # new property vs adding new functionality on top of getter - if is_new_property: - prop._property_info.declaration = func + assert func_property._property_info is not None + func_property._property_info.set(prop_func_kind, func) def depart_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: diff --git a/pydoctor/model.py b/pydoctor/model.py index 65a570774..4b887a2e2 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -713,14 +713,10 @@ class PropertyFunctionKind(Enum): @attr.s(auto_attribs=True) class PropertyInfo: - declaration:'Function' = NotImplemented - """ - The function that initially declared this property with C{@property} decorator or by overriding - the property function with C{@A.name.setter/getter/deleter} from another class. - """ + getter:Optional['Function'] = None """ - The getter. Generally the same as L{declaration} if it has not been overriden. + The getter. """ setter: Optional['Function'] = None """ @@ -824,7 +820,7 @@ def init_property(attr:'Attribute') -> Iterator['Function']: # Set the new attribute parsed docstring attr.parsed_docstring = pdoc - # TODO: Add inheritence info to getter/setter/deleters + # maybe TODO: Add inheritence info to getter/setter/deleters # Yield the objects to remove from the Documentable tree yield getter From 111b669a3b43d586a46db5b67560aec5ced0e0ad Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Sat, 19 Nov 2022 12:44:45 -0500 Subject: [PATCH 17/47] remove commented code --- pydoctor/model.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index 4b887a2e2..84304a126 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -745,14 +745,6 @@ class Function(Inheritable): signature: Optional[Signature] overloads: List['FunctionOverload'] - # # Property handling is special: This attribute is used in the processing step only. - # property_decorator: Optional[ast.expr] = None - # """ - # A property decorator like C{@property} or C{@name.setter} or C{@BaseClass.name.getter} / etc. - - # C{None} if this function is not decorated with any kind of property decorator. - # """ - def setup(self) -> None: super().setup() if isinstance(self.parent, Class): From 991d0a491be0aa5910ba0127aadb96a199a977d5 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Sat, 19 Nov 2022 13:29:01 -0500 Subject: [PATCH 18/47] Try to simplify more --- pydoctor/astbuilder.py | 29 ++++++----- pydoctor/model.py | 51 ++++++++++--------- .../templatewriter/pages/attributechild.py | 2 +- pydoctor/test/test_astbuilder.py | 50 +++++++++--------- 4 files changed, 69 insertions(+), 63 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 2fff0d696..976a14328 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -166,7 +166,7 @@ def looks_like_property_func_decorator(dottedname:List[str], ctx:model.Documenta return True return False -def get_inherited_property(dottedname:List[str], _parent: model.Documentable) -> Optional[model.Attribute]: +def get_inherited_property(dottedname:List[str], _parent: model.Documentable) -> Optional[model.PropertyDef]: """ Fetch the inherited property that this new decorator overrides. None if it doesn't exist. @@ -180,7 +180,7 @@ def get_inherited_property(dottedname:List[str], _parent: model.Documentable) -> # the property already exist return None - # property_def can be a getter/setter/deleter + # attr can be a getter/setter/deleter _cls = _parent.resolveName('.'.join(_property_name[:-1])) if _cls is None or not isinstance(_cls, model.Class): # Can't make sens of property decorator @@ -192,11 +192,14 @@ def get_inherited_property(dottedname:List[str], _parent: model.Documentable) -> # The class on which the property is defined (_cls) does not have # to be in the MRO of the parent - property_def = _cls.find(_property_name[-1]) - if not isinstance(property_def, model.Attribute): + attr_def = _cls.find(_property_name[-1]) + if not isinstance(attr_def, model.Attribute): return None - return property_def + if not attr_def.kind is model.DocumentableKind.PROPERTY: + return None + + return attr_def.property_def def get_property_function_kind(dottedname:List[str]) -> Optional[model.PropertyFunctionKind]: """ @@ -882,21 +885,20 @@ def _handleFunctionDef(self, # Looks like inherited property if len(property_decorator_dottedname)>2: inherited_property = get_inherited_property(property_decorator_dottedname, parent) - if inherited_property and inherited_property._property_info: + if inherited_property: func_property = self.builder.addAttribute(node.name, kind=model.DocumentableKind.PROPERTY, parent=parent) func_property.setLineNumber(lineno) func_property.decorators = node.decorator_list # copy property info - func_property._property_info = model.PropertyInfo( - **attr.asdict(inherited_property._property_info)) + func_property.property_def = inherited_property.clone() is_new_property = True else: # fetch property info to add this info to it maybe_prop = self.builder.current.contents.get(node.name) - if isinstance(maybe_prop, model.Attribute) and maybe_prop._property_info: + if isinstance(maybe_prop, model.Attribute) and maybe_prop.property_def: func_property = maybe_prop elif is_property: @@ -1011,10 +1013,9 @@ def add_arg(name: str, kind: Any, default: Optional[ast.expr]) -> None: if is_staticmethod: func_property.report(f'{func_property.fullName()} is both property and staticmethod') - if prop_func_kind is not None: - # Store the fact that this function implements one of the getter/setter/deleter - assert func_property._property_info is not None - func_property._property_info.set(prop_func_kind, func) + assert prop_func_kind is not None + # Store the fact that this function implements one of the getter/setter/deleter + func_property.property_def.set(prop_func_kind, func) def depart_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: @@ -1251,7 +1252,7 @@ def addAttribute(self, attr.kind = kind if kind is model.DocumentableKind.PROPERTY: # init property info if this attribute is a property - attr._property_info = model.PropertyInfo() + attr.property_def = model.PropertyDef() attr.parentMod = parentMod system.addObject(attr) self.currentAttr = attr diff --git a/pydoctor/model.py b/pydoctor/model.py index 84304a126..c25d44f7d 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -20,7 +20,7 @@ from inspect import signature, Signature from pathlib import Path from typing import ( - TYPE_CHECKING, Any, Callable, Collection, Dict, Iterator, List, Mapping, + TYPE_CHECKING, Any, Callable, Collection, Dict, Iterable, Iterator, List, Mapping, Optional, Sequence, Set, Tuple, Type, TypeVar, Union, cast, overload ) from urllib.parse import quote @@ -45,13 +45,17 @@ # # this was misguided. the tree structure is important, to be sure, # but the arrangement of the tree is far from arbitrary and there is -# at least some code that now relies on this. so here's a list: +# at least some code that now relies on this (the reparenting process +# implicitely relies on this, don't try to nest objects under Documentable +# that are not supposed to hold other objects!). +# +# Here's a list: # -# Packages can contain Packages and Modules +# Packages can contain Packages and Modules Functions and Classes # Modules can contain Functions and Classes # Classes can contain Functions (in this case they get called Methods) and # Classes -# Functions can't contain anything. +# Functions and Atributes can't contain anything. _string_lineno_is_end = sys.version_info < (3,8) \ @@ -712,7 +716,7 @@ class PropertyFunctionKind(Enum): DELETER = 3 @attr.s(auto_attribs=True) -class PropertyInfo: +class PropertyDef: getter:Optional['Function'] = None """ @@ -727,7 +731,7 @@ class PropertyInfo: None if it has not been set with C{@name.deleter} decorator. """ - def set(self, kind:PropertyFunctionKind, func:'Function') -> None: + def set(self, kind: PropertyFunctionKind, func:'Function') -> None: if kind is PropertyFunctionKind.GETTER: self.getter = func elif kind is PropertyFunctionKind.SETTER: @@ -737,6 +741,9 @@ def set(self, kind:PropertyFunctionKind, func:'Function') -> None: else: assert False + def clone(self) -> 'PropertyDef': + return PropertyDef(**attr.asdict(self)) + class Function(Inheritable): kind = DocumentableKind.FUNCTION is_async: bool @@ -767,7 +774,7 @@ def init_property(attr:'Attribute') -> Iterator['Function']: Returns the functions to remove from the tree. If the property matchup fails """ - info = attr._property_info + info = attr._property_def assert info is not None getter = info.getter @@ -835,24 +842,22 @@ class Attribute(Inheritable): Or maybe it can be that the attribute is a property. """ - _property_info:Optional[PropertyInfo] = None - + _property_def:Optional[PropertyDef] = None + @property - def property_setter(self) -> Optional[Function]: + def property_def(self) -> PropertyDef: """ - The property setter L{Function}, is any defined. - Only applicable if L{kind} is L{DocumentableKind.PROPERTY} + Access to this atribute is allowed only on PROPERTY objects. """ - if self._property_info: - return self._property_info.setter - return None - - @property - def property_deleter(self) -> Optional[Function]: - """Idem for the deleter.""" - if self._property_info: - return self._property_info.deleter - return None + assert self.kind is DocumentableKind.PROPERTY + assert self._property_def is not None + return self._property_def + + @property_def.setter + def property_def(self, v:PropertyDef) -> None: + assert self.kind is DocumentableKind.PROPERTY + self._property_def = v + # Work around the attributes of the same name within the System class. _ModuleT = Module @@ -1450,7 +1455,7 @@ def postProcess(self) -> None: # We are transforming the tree at the end only. to_delete: List[Documentable] = [] for attr in self.objectsOfType(Attribute): - if attr._property_info is not None: + if attr.property_def is not None: to_delete.extend(init_property(attr)) for obj in set(to_delete): self._remove(obj) diff --git a/pydoctor/templatewriter/pages/attributechild.py b/pydoctor/templatewriter/pages/attributechild.py index 35421549f..aeee61d2a 100644 --- a/pydoctor/templatewriter/pages/attributechild.py +++ b/pydoctor/templatewriter/pages/attributechild.py @@ -93,7 +93,7 @@ def propertyInfo(self, request: object, tag: Tag) -> "Flattenable": assert isinstance(self.ob, Attribute) - for func in [f for f in (self.ob.property_setter, self.ob.property_deleter) if f]: + for func in [f for f in (self.ob.property_def.setter, self.ob.property_def.deleter) if f]: r.append(FunctionChild(self.docgetter, func, extras=[], loader=self._funcLoader, silent_undoc=True)) return r diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 840c665e7..1e1ced0f1 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -1535,12 +1535,12 @@ def prop(self): assert getter.kind is model.DocumentableKind.PROPERTY assert getter.docstring == """Getter.""" - setter = getter.property_setter + setter = getter.property_def.setter assert isinstance(setter, model.Function) assert setter.kind is model.DocumentableKind.METHOD assert setter.docstring == """Setter.""" - deleter = getter.property_deleter + deleter = getter.property_def.deleter assert isinstance(deleter, model.Function) assert deleter.kind is model.DocumentableKind.METHOD assert deleter.docstring == """Deleter.""" @@ -2117,33 +2117,33 @@ def spam(self): assert spam2.kind is model.DocumentableKind.PROPERTY assert spam3.kind is model.DocumentableKind.PROPERTY - assert isinstance(spam0.property_setter, model.Function) - assert isinstance(spam1.property_setter, model.Function) - assert isinstance(spam2.property_setter, model.Function) - assert isinstance(spam3.property_setter, model.Function) + assert isinstance(spam0.property_def.setter, model.Function) + assert isinstance(spam1.property_def.setter, model.Function) + assert isinstance(spam2.property_def.setter, model.Function) + assert isinstance(spam3.property_def.setter, model.Function) - assert isinstance(spam0.property_deleter, model.Function) - assert isinstance(spam1.property_deleter, model.Function) - assert isinstance(spam2.property_deleter, model.Function) - assert isinstance(spam3.property_deleter, model.Function) + assert isinstance(spam0.property_def.deleter, model.Function) + assert isinstance(spam1.property_def.deleter, model.Function) + assert isinstance(spam2.property_def.deleter, model.Function) + assert isinstance(spam3.property_def.deleter, model.Function) - assert spam0._property_info.getter.fullName() == 'mod.BaseClass.spam.getter' # type:ignore - assert spam0._property_info.setter.fullName() == 'mod.BaseClass.spam.setter' # type:ignore - assert spam0._property_info.deleter.fullName() == 'mod.BaseClass.spam.deleter' # type:ignore + assert spam0.property_def.getter.fullName() == 'mod.BaseClass.spam.getter' #type:ignore[union-attr] + assert spam0.property_def.setter.fullName() == 'mod.BaseClass.spam.setter' + assert spam0.property_def.deleter.fullName() == 'mod.BaseClass.spam.deleter' - assert spam1._property_info.getter.fullName() == 'mod.SubClass.spam.getter' # type:ignore - assert spam1._property_info.setter.fullName() == 'mod.BaseClass.spam.setter' # type:ignore - assert spam1._property_info.deleter.fullName() == 'mod.BaseClass.spam.deleter' # type:ignore + assert spam1.property_def.getter.fullName() == 'mod.SubClass.spam.getter' #type:ignore[union-attr] + assert spam1.property_def.setter.fullName() == 'mod.BaseClass.spam.setter' + assert spam1.property_def.deleter.fullName() == 'mod.BaseClass.spam.deleter' - assert spam2._property_info.getter.fullName() == 'mod.BaseClass.spam.getter' # type:ignore - assert spam2._property_info.setter.fullName() == 'mod.SubClass2.spam.setter' # type:ignore - assert spam2._property_info.deleter.fullName() == 'mod.BaseClass.spam.deleter' # type:ignore + assert spam2.property_def.getter.fullName() == 'mod.BaseClass.spam.getter' #type:ignore[union-attr] + assert spam2.property_def.setter.fullName() == 'mod.SubClass2.spam.setter' + assert spam2.property_def.deleter.fullName() == 'mod.BaseClass.spam.deleter' - assert spam3._property_info.getter.fullName() == 'mod.BaseClass.spam.getter' # type:ignore - assert spam3._property_info.setter.fullName() == 'mod.BaseClass.spam.setter' # type:ignore - assert spam3._property_info.deleter.fullName() == 'mod.SubClass3.spam.deleter' # type:ignore + assert spam3.property_def.getter.fullName() == 'mod.BaseClass.spam.getter' #type:ignore[union-attr] + assert spam3.property_def.setter.fullName() == 'mod.BaseClass.spam.setter' + assert spam3.property_def.deleter.fullName() == 'mod.SubClass3.spam.deleter' - assert spam1._property_info.getter is not spam0._property_info.getter # type:ignore + assert spam1.property_def.getter is not spam0.property_def.getter @systemcls_param def test_property_old_school(systemcls: Type[model.System], capsys: CapSys) -> None: @@ -2497,5 +2497,5 @@ def spam(self): mod = fromText(src, systemcls=systemcls) p = mod.contents['BaseClass'].contents['spam'] assert isinstance(p, model.Attribute) - assert isinstance(p.property_setter, model.Function) - assert isinstance(p.property_deleter, model.Function) \ No newline at end of file + assert isinstance(p.property_def.setter, model.Function) + assert isinstance(p.property_def.deleter, model.Function) \ No newline at end of file From b6ab83d2d4215ce4a6c4aa35d8fa6817ee24e874 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Sat, 19 Nov 2022 13:43:28 -0500 Subject: [PATCH 19/47] Refactors --- pydoctor/model.py | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index c25d44f7d..2eaf90031 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -768,6 +768,20 @@ class FunctionOverload: signature: Signature decorators: Sequence[ast.expr] +def init_properties(system:'System') -> None: + # Machup property functions into an Attribute that already exist. + # We are transforming the tree at the end only. + to_prune: List[Documentable] = [] + for attr in system.objectsOfType(Attribute): + if attr.kind is DocumentableKind.PROPERTY: + to_prune.extend(init_property(attr)) + + for obj in set(to_prune): + system._remove(obj) + assert obj.parent is not None + if obj.name in obj.parent.contents: + del obj.parent.contents[obj.name] + def init_property(attr:'Attribute') -> Iterator['Function']: """ Initiates the L{Attribute} that represent the property in the tree. @@ -849,13 +863,13 @@ def property_def(self) -> PropertyDef: """ Access to this atribute is allowed only on PROPERTY objects. """ - assert self.kind is DocumentableKind.PROPERTY - assert self._property_def is not None + assert self.kind is DocumentableKind.PROPERTY, "property_def must be used on PROPERTY objects only" + assert self._property_def is not None, "inconsistent property_def" return self._property_def @property_def.setter def property_def(self, v:PropertyDef) -> None: - assert self.kind is DocumentableKind.PROPERTY + assert self.kind is DocumentableKind.PROPERTY, "property_def must be used on PROPERTY objects only" self._property_def = v @@ -1451,18 +1465,7 @@ def postProcess(self) -> None: if is_exception(cls): cls.kind = DocumentableKind.EXCEPTION - # Machup property functions into an Attribute that already exist. - # We are transforming the tree at the end only. - to_delete: List[Documentable] = [] - for attr in self.objectsOfType(Attribute): - if attr.property_def is not None: - to_delete.extend(init_property(attr)) - for obj in set(to_delete): - self._remove(obj) - assert obj.parent is not None - if obj.name in obj.parent.contents: - del obj.parent.contents[obj.name] - + init_properties(self) for post_processor in self._post_processors: post_processor(self) From d3d5d51081b85d5f6a99872ba4aa85c7acc3397c Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Wed, 5 Apr 2023 22:44:24 -0400 Subject: [PATCH 20/47] Simplify --- pydoctor/astbuilder.py | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index d13d39f3e..65663286b 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -148,7 +148,7 @@ def extract_final_subscript(annotation: ast.Subscript) -> ast.expr: assert isinstance(ann_slice, ast.expr) return ann_slice -def is_property_def_decorator(dottedname:List[str], ctx:model.Documentable) -> bool: +def _is_property_def_decorator(dottedname:List[str], ctx:model.Documentable) -> bool: """ Whether the last element of the list of names finishes by "property" or "Property". """ @@ -157,21 +157,13 @@ def is_property_def_decorator(dottedname:List[str], ctx:model.Documentable) -> b return True return False -def looks_like_property_func_decorator(dottedname:List[str], ctx:model.Documentable) -> bool: - """ - Whether the last element of the list of names is "getter", "setter" or "deleter". - Won't match C{['geter']}, because we require the list to have a least 2 elements. - """ - if len(dottedname) >= 2 and dottedname[-1] in ('getter' ,'setter', 'deleter'): - return True - return False -def get_inherited_property(dottedname:List[str], _parent: model.Documentable) -> Optional[model.PropertyDef]: +def _get_inherited_property(dottedname:List[str], _parent: model.Documentable) -> Optional[model.PropertyDef]: """ Fetch the inherited property that this new decorator overrides. None if it doesn't exist. """ - if not get_property_function_kind(dottedname): + if not _get_property_function_kind(dottedname): return None _property_name = dottedname[:-1] @@ -201,12 +193,12 @@ def get_inherited_property(dottedname:List[str], _parent: model.Documentable) -> return attr_def.property_def -def get_property_function_kind(dottedname:List[str]) -> Optional[model.PropertyFunctionKind]: +def _get_property_function_kind(dottedname:List[str]) -> Optional[model.PropertyFunctionKind]: """ What kind of property function this decorator declares? None if we can't make sens of the decorator. """ - if dottedname: + if len(dottedname) >= 2: if dottedname[-1] == 'setter': return model.PropertyFunctionKind.SETTER if dottedname[-1] == 'getter': @@ -840,21 +832,21 @@ def _handleFunctionDef(self, is_overload_func = False is_property = False - property_decorator_dottedname: Optional[List[str]] = None + property_deco: Optional[List[str]] = None if node.decorator_list: for deco_name, _ in astutils.iter_decorator_list(node.decorator_list): if deco_name is None: continue if isinstance(parent, model.Class): - if is_property_def_decorator(deco_name, parent): + if _is_property_def_decorator(deco_name, parent): is_property = True elif deco_name == ['classmethod']: is_classmethod = True elif deco_name == ['staticmethod']: is_staticmethod = True # Pre-handle property elements - elif looks_like_property_func_decorator(deco_name, parent): + elif _get_property_function_kind(deco_name): # Setters and deleters should have the same name as the property function, # otherwise ignore it. # This pollutes the namespace unnecessarily and is generally not recommended. @@ -867,7 +859,7 @@ def _handleFunctionDef(self, # the property object. func_name = '.'.join(deco_name[-2:]) - property_decorator_dottedname = deco_name + property_deco = deco_name # Determine if the function is decorated with overload if parent.expandName('.'.join(deco_name)) in ('typing.overload', 'typing_extensions.overload'): @@ -878,13 +870,13 @@ def _handleFunctionDef(self, prop_func_kind: Optional[model.PropertyFunctionKind] = None is_new_property: bool = is_property - if property_decorator_dottedname is not None: + if property_deco is not None: - prop_func_kind = get_property_function_kind(property_decorator_dottedname) + prop_func_kind = _get_property_function_kind(property_deco) # Looks like inherited property - if len(property_decorator_dottedname)>2: - inherited_property = get_inherited_property(property_decorator_dottedname, parent) + if len(property_deco)>2: + inherited_property = _get_inherited_property(property_deco, parent) if inherited_property: func_property = self.builder.addAttribute(node.name, kind=model.DocumentableKind.PROPERTY, From 346b40da5291b0f93e1a590a2b0ae2dfe021a713 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 6 Apr 2023 12:43:30 -0400 Subject: [PATCH 21/47] Use impossible import cycle in test --- pydoctor/test/test_astbuilder.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index efda02db4..a7ca214d3 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2215,9 +2215,7 @@ def spam(self): ''' src_mod = ''' - from typing import TYPE_CHECKING - if TYPE_CHECKING: - from mod import System + from mod import System class BaseClass(object): system: 'System' From 063ded208f3be3c0978d1f6cc20266b07a4adbc6 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 6 Apr 2023 16:15:57 -0400 Subject: [PATCH 22/47] Minor refactor --- pydoctor/astbuilder.py | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 65663286b..1eabd176a 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -802,6 +802,14 @@ def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: def visit_FunctionDef(self, node: ast.FunctionDef) -> None: self._handleFunctionDef(node, is_async=False) + def _addProperty(self, node, parent, lineno,): + attribute = self.builder.addAttribute(node.name, + kind=model.DocumentableKind.PROPERTY, + parent=parent) + attribute.setLineNumber(lineno) + attribute.decorators = node.decorator_list + return attribute + def _handleFunctionDef(self, node: Union[ast.AsyncFunctionDef, ast.FunctionDef], is_async: bool @@ -876,29 +884,21 @@ def _handleFunctionDef(self, # Looks like inherited property if len(property_deco)>2: - inherited_property = _get_inherited_property(property_deco, parent) - if inherited_property: - func_property = self.builder.addAttribute(node.name, - kind=model.DocumentableKind.PROPERTY, - parent=parent) - func_property.setLineNumber(lineno) - func_property.decorators = node.decorator_list - # copy property info - func_property.property_def = inherited_property.clone() + _inherited_property = _get_inherited_property(property_deco, parent) + if _inherited_property: + func_property = self._addProperty(node, parent, lineno) + # copy property defs info + func_property.property_def = _inherited_property.clone() is_new_property = True else: # fetch property info to add this info to it - maybe_prop = self.builder.current.contents.get(node.name) - if isinstance(maybe_prop, model.Attribute) and maybe_prop.property_def: - func_property = maybe_prop + _maybe_prop = self.builder.current.contents.get(node.name) + if isinstance(_maybe_prop, model.Attribute) and _maybe_prop.property_def: + func_property = _maybe_prop elif is_property: - func_property = self.builder.addAttribute(node.name, - kind=model.DocumentableKind.PROPERTY, - parent=parent) - func_property.setLineNumber(lineno) - func_property.decorators = node.decorator_list + func_property = self._addProperty(node, parent, lineno) prop_func_kind = model.PropertyFunctionKind.GETTER # rename func X.getter func_name = node.name+'.getter' @@ -913,7 +913,8 @@ def _handleFunctionDef(self, # which we do not allow. This also ensures that func will have # properties set for the primary function and not overloads. if existing_func.signature and is_overload_func: - existing_func.report(f'{existing_func.fullName()} overload appeared after primary function', lineno_offset=lineno-existing_func.linenumber) + existing_func.report(f'{existing_func.fullName()} overload appeared after primary function', + lineno_offset=lineno-existing_func.linenumber) raise self.SkipNode() # Do not recreate function object, just re-push it self.builder.push(existing_func, lineno) @@ -1006,7 +1007,7 @@ def add_arg(name: str, kind: Any, default: Optional[ast.expr]) -> None: func_property.report(f'{func_property.fullName()} is both property and staticmethod') assert prop_func_kind is not None - # Store the fact that this function implements one of the getter/setter/deleter + # Save the fact that this function implements one of the getter/setter/deleter func_property.property_def.set(prop_func_kind, func) From 9dccd4fd93c3e063ca8d340f7c81e9cafbf6edf8 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 6 Apr 2023 16:20:31 -0400 Subject: [PATCH 23/47] Minor refactors --- pydoctor/astbuilder.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 1eabd176a..327ce1ac8 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -174,7 +174,7 @@ def _get_inherited_property(dottedname:List[str], _parent: model.Documentable) - # attr can be a getter/setter/deleter _cls = _parent.resolveName('.'.join(_property_name[:-1])) - if _cls is None or not isinstance(_cls, model.Class): + if not isinstance(_cls, model.Class): # Can't make sens of property decorator # the property decorator is pointing to something external OR # not found in the system yet because of cyclic imports @@ -802,7 +802,8 @@ def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: def visit_FunctionDef(self, node: ast.FunctionDef) -> None: self._handleFunctionDef(node, is_async=False) - def _addProperty(self, node, parent, lineno,): + def _addProperty(self, node:Union[ast.AsyncFunctionDef, ast.FunctionDef], + parent:model.Documentable, lineno:int,) -> model.Attribute: attribute = self.builder.addAttribute(node.name, kind=model.DocumentableKind.PROPERTY, parent=parent) From a1529eec5c023f207eedf8666fcb1d071c605e92 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 6 Apr 2023 16:42:16 -0400 Subject: [PATCH 24/47] Fix import --- pydoctor/templatewriter/pages/functionchild.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pydoctor/templatewriter/pages/functionchild.py b/pydoctor/templatewriter/pages/functionchild.py index cbc444a68..f3dc3add3 100644 --- a/pydoctor/templatewriter/pages/functionchild.py +++ b/pydoctor/templatewriter/pages/functionchild.py @@ -3,8 +3,7 @@ from twisted.web.iweb import ITemplateLoader from twisted.web.template import Tag, renderer -from pydoctor.model import Function -from pydoctor.epydoc2stan import get_docstring +from pydoctor.model import Function, get_docstring from pydoctor.templatewriter import TemplateElement, util from pydoctor.templatewriter.pages import format_decorators, format_function_def, format_overloads From 6203a04275176b811bea0ddd24e026379bc5e516 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 6 Apr 2023 16:53:12 -0400 Subject: [PATCH 25/47] Fix mypy --- docs/epytext_demo/demo_epytext_module.py | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/epytext_demo/demo_epytext_module.py b/docs/epytext_demo/demo_epytext_module.py index c85372517..6eb771951 100644 --- a/docs/epytext_demo/demo_epytext_module.py +++ b/docs/epytext_demo/demo_epytext_module.py @@ -188,6 +188,7 @@ def read_and_write_delete(self) -> None: @property def undoc_prop(self) -> bytes: """This property has a docstring only on the setter.""" + return b'' @undoc_prop.setter def undoc_prop(self, p) -> None: # type:ignore ... From 272e7dbbeb6504e0c40fd0c29256fd931a349fe2 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 6 Apr 2023 16:53:26 -0400 Subject: [PATCH 26/47] Fix crash :/ --- pydoctor/astbuilder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 327ce1ac8..4045312ce 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -895,7 +895,7 @@ def _handleFunctionDef(self, else: # fetch property info to add this info to it _maybe_prop = self.builder.current.contents.get(node.name) - if isinstance(_maybe_prop, model.Attribute) and _maybe_prop.property_def: + if isinstance(_maybe_prop, model.Attribute) and _maybe_prop.kind is model.DocumentableKind.PROPERTY: func_property = _maybe_prop elif is_property: From 88cb4330bee674a156c8131b4fb9e396d2ee4cba Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 6 Apr 2023 17:01:59 -0400 Subject: [PATCH 27/47] Fix pyflakes --- pydoctor/astbuilder.py | 1 - pydoctor/model.py | 33 ++++++++++------------------ pydoctor/test/test_astbuilder.py | 2 +- pydoctor/test/test_templatewriter.py | 20 ----------------- 4 files changed, 13 insertions(+), 43 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 4045312ce..36e134f4c 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -13,7 +13,6 @@ ) import astor -import attr from pydoctor import epydoc2stan, model, node2stan, extensions, astutils from pydoctor.epydoc.markup._pyval_repr import colorize_inline_pyval from pydoctor.astutils import (is_none_literal, is_typing_annotation, is_using_annotations, is_using_typing_final, node2dottedname, node2fullname, diff --git a/pydoctor/model.py b/pydoctor/model.py index 64cac7d67..33f037376 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -8,11 +8,9 @@ import abc import ast -import attr from collections import defaultdict import datetime import importlib -import platform import sys import textwrap import types @@ -20,7 +18,7 @@ from inspect import signature, Signature from pathlib import Path from typing import ( - TYPE_CHECKING, Any, Callable, Collection, Dict, Iterable, Iterator, List, Mapping, + TYPE_CHECKING, Any, Callable, Collection, Dict, Iterator, List, Mapping, Optional, Sequence, Set, Tuple, Type, TypeVar, Union, cast, overload ) from urllib.parse import quote @@ -58,13 +56,6 @@ # Functions and Atributes can't contain anything. -_string_lineno_is_end = sys.version_info < (3,8) \ - and platform.python_implementation() != 'PyPy' -"""True iff the 'lineno' attribute of an AST string node points to the last -line in the string, rather than the first line. -""" - - class DocLocation(Enum): OWN_PAGE = 1 PARENT_PAGE = 2 @@ -860,9 +851,9 @@ def init_properties(system:'System') -> None: # Machup property functions into an Attribute that already exist. # We are transforming the tree at the end only. to_prune: List[Documentable] = [] - for attr in system.objectsOfType(Attribute): - if attr.kind is DocumentableKind.PROPERTY: - to_prune.extend(init_property(attr)) + for attrib in system.objectsOfType(Attribute): + if attrib.kind is DocumentableKind.PROPERTY: + to_prune.extend(init_property(attrib)) for obj in set(to_prune): system._remove(obj) @@ -870,13 +861,13 @@ def init_properties(system:'System') -> None: if obj.name in obj.parent.contents: del obj.parent.contents[obj.name] -def init_property(attr:'Attribute') -> Iterator['Function']: +def init_property(attrib:'Attribute') -> Iterator['Function']: """ Initiates the L{Attribute} that represent the property in the tree. Returns the functions to remove from the tree. If the property matchup fails """ - info = attr._property_def + info = attrib._property_def assert info is not None getter = info.getter @@ -891,11 +882,11 @@ def init_property(attr:'Attribute') -> Iterator['Function']: from pydoctor import epydoc2stan # Setup Attribute object for the property - attr.docstring = getter.docstring - attr.annotation = getter.annotations.get('return') - attr.decorators = getter.decorators + attrib.docstring = getter.docstring + attrib.annotation = getter.annotations.get('return') + attrib.decorators = getter.decorators - attr.extra_info.extend(getter.extra_info) + attrib.extra_info.extend(getter.extra_info) # Parse docstring now. if epydoc2stan.ensure_parsed_docstring(getter): @@ -912,14 +903,14 @@ def init_property(attr:'Attribute') -> Iterator['Function']: if not pdoc.has_body: pdoc = field.body() elif tag == 'rtype': - attr.parsed_type = field.body() + attrib.parsed_type = field.body() else: other_fields.append(field) pdoc.fields = other_fields # Set the new attribute parsed docstring - attr.parsed_docstring = pdoc + attrib.parsed_docstring = pdoc # maybe TODO: Add inheritence info to getter/setter/deleters diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 21aad0906..4228d502a 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2150,7 +2150,7 @@ def test_property_old_school(systemcls: Type[model.System], capsys: CapSys) -> N """ We don't support it for now. """ - src = ''' + ''' class PropertyDocBase(object): _spam = 1 def _get_spam(self): diff --git a/pydoctor/test/test_templatewriter.py b/pydoctor/test/test_templatewriter.py index c20e0e24d..3a0a21806 100644 --- a/pydoctor/test/test_templatewriter.py +++ b/pydoctor/test/test_templatewriter.py @@ -710,26 +710,6 @@ def data(self): # asserts that no 'Undocumented' shows up! assert 'Undocumented' not in html - # This variant does not test anything special: - src3 = ''' - class A: - @property - def data(self): - """ - Get the data. - @returns: the data - """ - @data.setter - def data(self, data): - """ - Sets the data. - @param data: the new value - """ - @data.deleter - def data(self): - "Deletes the data" - ''' - def test_property_getter_inherits_docs() -> None: src4 = ''' From 5acd371e4b7ba9888832573fd898c30f56b4c27d Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 7 Apr 2023 13:25:08 -0400 Subject: [PATCH 28/47] Create a legit model object for Properties. --- pydoctor/astbuilder.py | 41 +++++++++++-------- pydoctor/factory.py | 7 ++++ pydoctor/model.py | 33 ++++++--------- .../templatewriter/pages/attributechild.py | 4 +- pydoctor/test/test_astbuilder.py | 15 +++---- 5 files changed, 53 insertions(+), 47 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 36e134f4c..1357506bc 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -160,7 +160,8 @@ def _is_property_def_decorator(dottedname:List[str], ctx:model.Documentable) -> def _get_inherited_property(dottedname:List[str], _parent: model.Documentable) -> Optional[model.PropertyDef]: """ Fetch the inherited property that this new decorator overrides. - None if it doesn't exist. + Returns C{None} if it doesn't exist in the inherited members or if it's already definied in the locals. + The dottedname must have at least three elements, else return C{None}. """ if not _get_property_function_kind(dottedname): return None @@ -181,21 +182,21 @@ def _get_inherited_property(dottedname:List[str], _parent: model.Documentable) - # Don't rename it. return None - # The class on which the property is defined (_cls) does not have + # note: the class on which the property is defined (_cls) does not have # to be in the MRO of the parent attr_def = _cls.find(_property_name[-1]) - if not isinstance(attr_def, model.Attribute): - return None - if not attr_def.kind is model.DocumentableKind.PROPERTY: + if not isinstance(attr_def, model.Property): return None - + return attr_def.property_def def _get_property_function_kind(dottedname:List[str]) -> Optional[model.PropertyFunctionKind]: """ What kind of property function this decorator declares? None if we can't make sens of the decorator. + + @note: the dottedname must have at least two elements. """ if len(dottedname) >= 2: if dottedname[-1] == 'setter': @@ -802,12 +803,11 @@ def visit_FunctionDef(self, node: ast.FunctionDef) -> None: self._handleFunctionDef(node, is_async=False) def _addProperty(self, node:Union[ast.AsyncFunctionDef, ast.FunctionDef], - parent:model.Documentable, lineno:int,) -> model.Attribute: - attribute = self.builder.addAttribute(node.name, - kind=model.DocumentableKind.PROPERTY, - parent=parent) + parent:model.Documentable, lineno:int,) -> model.Property: + attribute = self.builder.addProperty(node.name, parent=parent) attribute.setLineNumber(lineno) attribute.decorators = node.decorator_list + assert isinstance(attribute, model.Property) return attribute def _handleFunctionDef(self, @@ -870,11 +870,12 @@ def _handleFunctionDef(self, property_deco = deco_name # Determine if the function is decorated with overload - if parent.expandName('.'.join(deco_name)) in ('typing.overload', 'typing_extensions.overload'): + if parent.expandName('.'.join(deco_name)) in ('typing.overload', + 'typing_extensions.overload'): is_overload_func = True - func_property: Optional[model.Attribute] = None + func_property: Optional[model.Property] = None prop_func_kind: Optional[model.PropertyFunctionKind] = None is_new_property: bool = is_property @@ -894,7 +895,7 @@ def _handleFunctionDef(self, else: # fetch property info to add this info to it _maybe_prop = self.builder.current.contents.get(node.name) - if isinstance(_maybe_prop, model.Attribute) and _maybe_prop.kind is model.DocumentableKind.PROPERTY: + if isinstance(_maybe_prop, model.Property): func_property = _maybe_prop elif is_property: @@ -1241,16 +1242,22 @@ def addAttribute(self, """ system = self.system parentMod = self.currentMod - attr = system.Attribute(system, name, parent) - attr.kind = kind if kind is model.DocumentableKind.PROPERTY: - # init property info if this attribute is a property - attr.property_def = model.PropertyDef() + attr:model.Attribute = system.Property(system, name, parent) + else: + attr = system.Attribute(system, name, parent) + attr.kind = kind attr.parentMod = parentMod system.addObject(attr) self.currentAttr = attr return attr + def addProperty(self, + name: str, parent: model.Documentable + ) -> model.Property: + return cast(model.Property, self.addAttribute(name, + model.DocumentableKind.PROPERTY, + parent)) def processModuleAST(self, mod_ast: ast.Module, mod: model.Module) -> None: diff --git a/pydoctor/factory.py b/pydoctor/factory.py index e56fb44a9..eded7d153 100644 --- a/pydoctor/factory.py +++ b/pydoctor/factory.py @@ -69,6 +69,7 @@ def __init__(self) -> None: 'Module': model.Module, 'Package': model.Package, 'Attribute': model.Attribute, + 'Property': model.Property, } super().__init__(bases=_bases) @@ -112,3 +113,9 @@ def Attribute(self) -> Type['model.Attribute']: data = self.get_class('Attribute') assert issubclass(data, self.model.Attribute) return data + + @property + def Property(self) -> Type['model.Property']: + data = self.get_class('Property') + assert issubclass(data, self.model.Property) + return data diff --git a/pydoctor/model.py b/pydoctor/model.py index 33f037376..ec8b82cd1 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -851,9 +851,8 @@ def init_properties(system:'System') -> None: # Machup property functions into an Attribute that already exist. # We are transforming the tree at the end only. to_prune: List[Documentable] = [] - for attrib in system.objectsOfType(Attribute): - if attrib.kind is DocumentableKind.PROPERTY: - to_prune.extend(init_property(attrib)) + for attrib in system.objectsOfType(Property): + to_prune.extend(init_property(attrib)) for obj in set(to_prune): system._remove(obj) @@ -861,13 +860,13 @@ def init_properties(system:'System') -> None: if obj.name in obj.parent.contents: del obj.parent.contents[obj.name] -def init_property(attrib:'Attribute') -> Iterator['Function']: +def init_property(attrib:'Property') -> Iterator['Function']: """ Initiates the L{Attribute} that represent the property in the tree. Returns the functions to remove from the tree. If the property matchup fails """ - info = attrib._property_def + info = attrib.property_def assert info is not None getter = info.getter @@ -935,22 +934,11 @@ class Attribute(Inheritable): Or maybe it can be that the attribute is a property. """ - _property_def:Optional[PropertyDef] = None - - @property - def property_def(self) -> PropertyDef: - """ - Access to this atribute is allowed only on PROPERTY objects. - """ - assert self.kind is DocumentableKind.PROPERTY, "property_def must be used on PROPERTY objects only" - assert self._property_def is not None, "inconsistent property_def" - return self._property_def - - @property_def.setter - def property_def(self, v:PropertyDef) -> None: - assert self.kind is DocumentableKind.PROPERTY, "property_def must be used on PROPERTY objects only" - self._property_def = v - +class Property(Attribute): + __class__ = Attribute #type:ignore + def __init__(self, *args:Any, **kwargs:Any): + super().__init__(*args, **kwargs) + self.property_def = PropertyDef() # Work around the attributes of the same name within the System class. _ModuleT = Module @@ -1092,6 +1080,9 @@ def Package(self) -> Type['Package']: @property def Attribute(self) -> Type['Attribute']: return self._factory.Attribute + @property + def Property(self) -> Type['Property']: + return self._factory.Property @property def sourcebase(self) -> Optional[str]: diff --git a/pydoctor/templatewriter/pages/attributechild.py b/pydoctor/templatewriter/pages/attributechild.py index 7ada23c9d..4707ef69e 100644 --- a/pydoctor/templatewriter/pages/attributechild.py +++ b/pydoctor/templatewriter/pages/attributechild.py @@ -3,7 +3,7 @@ from twisted.web.iweb import ITemplateLoader from twisted.web.template import Tag, renderer, tags -from pydoctor.model import Attribute, DocumentableKind +from pydoctor.model import Attribute, Property, DocumentableKind from pydoctor import epydoc2stan from pydoctor.templatewriter import TemplateElement, util from pydoctor.templatewriter.pages import format_decorators @@ -91,7 +91,7 @@ def propertyInfo(self, request: object, tag: Tag) -> "Flattenable": if self.ob.kind is DocumentableKind.PROPERTY: from pydoctor.templatewriter.pages.functionchild import FunctionChild - assert isinstance(self.ob, Attribute) + assert isinstance(self.ob, Property) for func in [f for f in (self.ob.property_def.setter, self.ob.property_def.deleter) if f]: r.append(FunctionChild(self.docgetter, func, extras=[], diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 4228d502a..6d80ece01 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -1531,8 +1531,9 @@ def prop(self): C = mod.contents['C'] getter = C.contents['prop'] - assert isinstance(getter, model.Attribute) - assert getter.kind is model.DocumentableKind.PROPERTY + assert isinstance(getter, model.Property) + assert getter.__class__ is model.Attribute # type:ignore + assert getter.docstring == """Getter.""" setter = getter.property_def.setter @@ -2107,10 +2108,10 @@ def spam(self): list(mod.contents['SubClass2'].contents) == \ list(mod.contents['SubClass3'].contents) == ['spam'] - assert isinstance(spam0, model.Attribute) - assert isinstance(spam1, model.Attribute) - assert isinstance(spam2, model.Attribute) - assert isinstance(spam3, model.Attribute) + assert isinstance(spam0, model.Property) + assert isinstance(spam1, model.Property) + assert isinstance(spam2, model.Property) + assert isinstance(spam3, model.Property) assert spam0.kind is model.DocumentableKind.PROPERTY assert spam1.kind is model.DocumentableKind.PROPERTY @@ -2499,7 +2500,7 @@ def spam(self): mod = fromText(src, systemcls=systemcls) p = mod.contents['BaseClass'].contents['spam'] - assert isinstance(p, model.Attribute) + assert isinstance(p, model.Property) assert isinstance(p.property_def.setter, model.Function) assert isinstance(p.property_def.deleter, model.Function) From ceacac0520c31e488c8e15180a3f29368a4b2e9a Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 7 Apr 2023 14:19:27 -0400 Subject: [PATCH 29/47] Refoctors to have a more simple model. --- pydoctor/astbuilder.py | 33 ++++---- pydoctor/model.py | 77 ++++++++----------- .../templatewriter/pages/attributechild.py | 2 +- pydoctor/test/test_astbuilder.py | 50 ++++++------ 4 files changed, 78 insertions(+), 84 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 1357506bc..41af2810d 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -147,7 +147,7 @@ def extract_final_subscript(annotation: ast.Subscript) -> ast.expr: assert isinstance(ann_slice, ast.expr) return ann_slice -def _is_property_def_decorator(dottedname:List[str], ctx:model.Documentable) -> bool: +def _is_property_decorator(dottedname:Sequence[str], ctx:model.Documentable) -> bool: """ Whether the last element of the list of names finishes by "property" or "Property". """ @@ -157,12 +157,14 @@ def _is_property_def_decorator(dottedname:List[str], ctx:model.Documentable) -> return False -def _get_inherited_property(dottedname:List[str], _parent: model.Documentable) -> Optional[model.PropertyDef]: +def _get_inherited_property(dottedname:Sequence[str], _parent: model.Documentable) -> Optional[model.Property]: """ Fetch the inherited property that this new decorator overrides. Returns C{None} if it doesn't exist in the inherited members or if it's already definied in the locals. The dottedname must have at least three elements, else return C{None}. """ + # TODO: It would be best if this job was done in post-processing... + if not _get_property_function_kind(dottedname): return None @@ -179,7 +181,6 @@ def _get_inherited_property(dottedname:List[str], _parent: model.Documentable) - # the property decorator is pointing to something external OR # not found in the system yet because of cyclic imports # OR to something else than a class :/ - # Don't rename it. return None # note: the class on which the property is defined (_cls) does not have @@ -189,9 +190,9 @@ def _get_inherited_property(dottedname:List[str], _parent: model.Documentable) - if not isinstance(attr_def, model.Property): return None - return attr_def.property_def + return attr_def -def _get_property_function_kind(dottedname:List[str]) -> Optional[model.PropertyFunctionKind]: +def _get_property_function_kind(dottedname:Sequence[str]) -> Optional[model.Property.Kind]: """ What kind of property function this decorator declares? None if we can't make sens of the decorator. @@ -200,11 +201,11 @@ def _get_property_function_kind(dottedname:List[str]) -> Optional[model.Property """ if len(dottedname) >= 2: if dottedname[-1] == 'setter': - return model.PropertyFunctionKind.SETTER + return model.Property.Kind.SETTER if dottedname[-1] == 'getter': - return model.PropertyFunctionKind.GETTER + return model.Property.Kind.GETTER if dottedname[-1] == 'deleter': - return model.PropertyFunctionKind.DELETER + return model.Property.Kind.DELETER return None class ModuleVistor(NodeVisitor): @@ -847,7 +848,7 @@ def _handleFunctionDef(self, if deco_name is None: continue if isinstance(parent, model.Class): - if _is_property_def_decorator(deco_name, parent): + if _is_property_decorator(deco_name, parent): is_property = True elif deco_name == ['classmethod']: is_classmethod = True @@ -876,7 +877,7 @@ def _handleFunctionDef(self, func_property: Optional[model.Property] = None - prop_func_kind: Optional[model.PropertyFunctionKind] = None + prop_func_kind: Optional[model.Property.Kind] = None is_new_property: bool = is_property if property_deco is not None: @@ -889,7 +890,9 @@ def _handleFunctionDef(self, if _inherited_property: func_property = self._addProperty(node, parent, lineno) # copy property defs info - func_property.property_def = _inherited_property.clone() + func_property.getter = _inherited_property.getter + func_property.setter = _inherited_property.setter + func_property.deleter = _inherited_property.deleter is_new_property = True else: @@ -900,9 +903,9 @@ def _handleFunctionDef(self, elif is_property: func_property = self._addProperty(node, parent, lineno) - prop_func_kind = model.PropertyFunctionKind.GETTER - # rename func X.getter - func_name = node.name+'.getter' + prop_func_kind = model.Property.Kind.GETTER + # Rename the getter as well + func_name = node.name + '.getter' # Push and analyse function @@ -1009,7 +1012,7 @@ def add_arg(name: str, kind: Any, default: Optional[ast.expr]) -> None: assert prop_func_kind is not None # Save the fact that this function implements one of the getter/setter/deleter - func_property.property_def.set(prop_func_kind, func) + func_property.store_function(prop_func_kind, func) def depart_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: diff --git a/pydoctor/model.py b/pydoctor/model.py index ec8b82cd1..5e10beeea 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -789,40 +789,6 @@ def docsources(self) -> Iterator[Documentable]: def _localNameToFullName(self, name: str) -> str: return self.parent._localNameToFullName(name) -class PropertyFunctionKind(Enum): - GETTER = 1 - SETTER = 2 - DELETER = 3 - -@attr.s(auto_attribs=True) -class PropertyDef: - - getter:Optional['Function'] = None - """ - The getter. - """ - setter: Optional['Function'] = None - """ - None if it has not been set with C{@name.setter} decorator. - """ - deleter: Optional['Function'] = None - """ - None if it has not been set with C{@name.deleter} decorator. - """ - - def set(self, kind: PropertyFunctionKind, func:'Function') -> None: - if kind is PropertyFunctionKind.GETTER: - self.getter = func - elif kind is PropertyFunctionKind.SETTER: - self.setter = func - elif kind is PropertyFunctionKind.DELETER: - self.deleter = func - else: - assert False - - def clone(self) -> 'PropertyDef': - return PropertyDef(**attr.asdict(self)) - class Function(Inheritable): kind = DocumentableKind.FUNCTION is_async: bool @@ -866,12 +832,9 @@ def init_property(attrib:'Property') -> Iterator['Function']: Returns the functions to remove from the tree. If the property matchup fails """ - info = attrib.property_def - assert info is not None - - getter = info.getter - setter = info.setter - deleter = info.deleter + getter = attrib.getter + setter = attrib.setter + deleter = attrib.deleter if getter is None: # The getter should never be None @@ -935,10 +898,38 @@ class Attribute(Inheritable): """ class Property(Attribute): + # for the templatewriter, property are just like attributes __class__ = Attribute #type:ignore - def __init__(self, *args:Any, **kwargs:Any): - super().__init__(*args, **kwargs) - self.property_def = PropertyDef() + + class Kind(Enum): + GETTER = 1 + SETTER = 2 + DELETER = 3 + + def setup(self) -> None: + super().setup() + self.getter:Optional['Function'] = None + """ + The getter. + """ + self.setter: Optional['Function'] = None + """ + None if it has not been set with C{@.setter} decorator. + """ + self.deleter: Optional['Function'] = None + """ + None if it has not been set with C{@.deleter} decorator. + """ + + def store_function(self, kind: 'Property.Kind', func:'Function') -> None: + if kind is Property.Kind.GETTER: + self.getter = func + elif kind is Property.Kind.SETTER: + self.setter = func + elif kind is Property.Kind.DELETER: + self.deleter = func + else: + assert False # Work around the attributes of the same name within the System class. _ModuleT = Module diff --git a/pydoctor/templatewriter/pages/attributechild.py b/pydoctor/templatewriter/pages/attributechild.py index 4707ef69e..cceaab191 100644 --- a/pydoctor/templatewriter/pages/attributechild.py +++ b/pydoctor/templatewriter/pages/attributechild.py @@ -93,7 +93,7 @@ def propertyInfo(self, request: object, tag: Tag) -> "Flattenable": assert isinstance(self.ob, Property) - for func in [f for f in (self.ob.property_def.setter, self.ob.property_def.deleter) if f]: + for func in [f for f in (self.ob.setter, self.ob.deleter) if f]: r.append(FunctionChild(self.docgetter, func, extras=[], loader=self._funcLoader, silent_undoc=True)) return r diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 6d80ece01..4d56e71d7 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -1536,12 +1536,12 @@ def prop(self): assert getter.docstring == """Getter.""" - setter = getter.property_def.setter + setter = getter.setter assert isinstance(setter, model.Function) assert setter.kind is model.DocumentableKind.METHOD assert setter.docstring == """Setter.""" - deleter = getter.property_def.deleter + deleter = getter.deleter assert isinstance(deleter, model.Function) assert deleter.kind is model.DocumentableKind.METHOD assert deleter.docstring == """Deleter.""" @@ -2118,33 +2118,33 @@ def spam(self): assert spam2.kind is model.DocumentableKind.PROPERTY assert spam3.kind is model.DocumentableKind.PROPERTY - assert isinstance(spam0.property_def.setter, model.Function) - assert isinstance(spam1.property_def.setter, model.Function) - assert isinstance(spam2.property_def.setter, model.Function) - assert isinstance(spam3.property_def.setter, model.Function) + assert isinstance(spam0.setter, model.Function) + assert isinstance(spam1.setter, model.Function) + assert isinstance(spam2.setter, model.Function) + assert isinstance(spam3.setter, model.Function) - assert isinstance(spam0.property_def.deleter, model.Function) - assert isinstance(spam1.property_def.deleter, model.Function) - assert isinstance(spam2.property_def.deleter, model.Function) - assert isinstance(spam3.property_def.deleter, model.Function) + assert isinstance(spam0.deleter, model.Function) + assert isinstance(spam1.deleter, model.Function) + assert isinstance(spam2.deleter, model.Function) + assert isinstance(spam3.deleter, model.Function) - assert spam0.property_def.getter.fullName() == 'mod.BaseClass.spam.getter' #type:ignore[union-attr] - assert spam0.property_def.setter.fullName() == 'mod.BaseClass.spam.setter' - assert spam0.property_def.deleter.fullName() == 'mod.BaseClass.spam.deleter' + assert spam0.getter.fullName() == 'mod.BaseClass.spam.getter' #type:ignore[union-attr] + assert spam0.setter.fullName() == 'mod.BaseClass.spam.setter' + assert spam0.deleter.fullName() == 'mod.BaseClass.spam.deleter' - assert spam1.property_def.getter.fullName() == 'mod.SubClass.spam.getter' #type:ignore[union-attr] - assert spam1.property_def.setter.fullName() == 'mod.BaseClass.spam.setter' - assert spam1.property_def.deleter.fullName() == 'mod.BaseClass.spam.deleter' + assert spam1.getter.fullName() == 'mod.SubClass.spam.getter' #type:ignore[union-attr] + assert spam1.setter.fullName() == 'mod.BaseClass.spam.setter' + assert spam1.deleter.fullName() == 'mod.BaseClass.spam.deleter' - assert spam2.property_def.getter.fullName() == 'mod.BaseClass.spam.getter' #type:ignore[union-attr] - assert spam2.property_def.setter.fullName() == 'mod.SubClass2.spam.setter' - assert spam2.property_def.deleter.fullName() == 'mod.BaseClass.spam.deleter' + assert spam2.getter.fullName() == 'mod.BaseClass.spam.getter' #type:ignore[union-attr] + assert spam2.setter.fullName() == 'mod.SubClass2.spam.setter' + assert spam2.deleter.fullName() == 'mod.BaseClass.spam.deleter' - assert spam3.property_def.getter.fullName() == 'mod.BaseClass.spam.getter' #type:ignore[union-attr] - assert spam3.property_def.setter.fullName() == 'mod.BaseClass.spam.setter' - assert spam3.property_def.deleter.fullName() == 'mod.SubClass3.spam.deleter' + assert spam3.getter.fullName() == 'mod.BaseClass.spam.getter' #type:ignore[union-attr] + assert spam3.setter.fullName() == 'mod.BaseClass.spam.setter' + assert spam3.deleter.fullName() == 'mod.SubClass3.spam.deleter' - assert spam1.property_def.getter is not spam0.property_def.getter + assert spam1.getter is not spam0.getter @systemcls_param def test_property_old_school(systemcls: Type[model.System], capsys: CapSys) -> None: @@ -2501,8 +2501,8 @@ def spam(self): mod = fromText(src, systemcls=systemcls) p = mod.contents['BaseClass'].contents['spam'] assert isinstance(p, model.Property) - assert isinstance(p.property_def.setter, model.Function) - assert isinstance(p.property_def.deleter, model.Function) + assert isinstance(p.setter, model.Function) + assert isinstance(p.deleter, model.Function) @systemcls_param def test_crash_type_inference_unhashable_type(systemcls: Type[model.System], capsys:CapSys) -> None: From 91c34bf2fe9a726e708f624256ee78eef5d2eac0 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 7 Apr 2023 14:35:06 -0400 Subject: [PATCH 30/47] Minor comment --- pydoctor/astbuilder.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 41af2810d..1957ccc55 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -875,6 +875,7 @@ def _handleFunctionDef(self, 'typing_extensions.overload'): is_overload_func = True + # Determine if this function is a property of some kind func_property: Optional[model.Property] = None prop_func_kind: Optional[model.Property.Kind] = None From a45ebe2c2d7ebccaf48d9bca2102287bc0cf48b4 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Sun, 23 Apr 2023 11:48:43 -0400 Subject: [PATCH 31/47] Add Property model to the extension system. --- pydoctor/extensions/__init__.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pydoctor/extensions/__init__.py b/pydoctor/extensions/__init__.py index 0d8816934..69222d55c 100644 --- a/pydoctor/extensions/__init__.py +++ b/pydoctor/extensions/__init__.py @@ -30,11 +30,13 @@ class FunctionMixin: """Base class for mixins applied to L{model.Function} objects.""" class AttributeMixin: """Base class for mixins applied to L{model.Attribute} objects.""" -class DocumentableMixin(ModuleMixin, ClassMixin, FunctionMixin, AttributeMixin): +class PropertyMixin: + """Base class for mixins applied to L{model.Property} objects.""" +class DocumentableMixin(ModuleMixin, ClassMixin, FunctionMixin, AttributeMixin, PropertyMixin): """Base class for mixins applied to all L{model.Documentable} objects.""" class CanContainImportsDocumentableMixin(PackageMixin, ModuleMixin, ClassMixin): """Base class for mixins applied to L{model.Class}, L{model.Module} and L{model.Package} objects.""" -class InheritableMixin(FunctionMixin, AttributeMixin): +class InheritableMixin(FunctionMixin, AttributeMixin, PropertyMixin): """Base class for mixins applied to L{model.Function} and L{model.Attribute} objects.""" MixinT = Union[ClassMixin, ModuleMixin, PackageMixin, FunctionMixin, AttributeMixin] @@ -85,6 +87,7 @@ def _get_setup_extension_func_from_module(module: str) -> Callable[['ExtRegistra PackageMixin: 'Package', FunctionMixin: 'Function', AttributeMixin: 'Attribute', + PropertyMixin: 'Property', } def _get_mixins(*mixins: Type[MixinT]) -> Dict[str, List[Type[MixinT]]]: From ff6a662be43cea63d6883d54225cdd039e0864e7 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Sun, 23 Apr 2023 12:34:21 -0400 Subject: [PATCH 32/47] Simplify implementation --- pydoctor/astbuilder.py | 87 +++++++++++++++++------------------------- 1 file changed, 36 insertions(+), 51 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 1957ccc55..219a24a0c 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -157,7 +157,7 @@ def _is_property_decorator(dottedname:Sequence[str], ctx:model.Documentable) -> return False -def _get_inherited_property(dottedname:Sequence[str], _parent: model.Documentable) -> Optional[model.Property]: +def _get_inherited_property(dottedname:Sequence[str], parent: model.Documentable) -> Optional[model.Property]: """ Fetch the inherited property that this new decorator overrides. Returns C{None} if it doesn't exist in the inherited members or if it's already definied in the locals. @@ -165,27 +165,16 @@ def _get_inherited_property(dottedname:Sequence[str], _parent: model.Documentabl """ # TODO: It would be best if this job was done in post-processing... - if not _get_property_function_kind(dottedname): - return None - - _property_name = dottedname[:-1] + property_name = dottedname[:-1] - if len(_property_name) <= 1 or _property_name[-1] in _parent.contents: + if len(property_name) <= 1 or property_name[-1] in parent.contents: # the property already exist return None # attr can be a getter/setter/deleter - _cls = _parent.resolveName('.'.join(_property_name[:-1])) - if not isinstance(_cls, model.Class): - # Can't make sens of property decorator - # the property decorator is pointing to something external OR - # not found in the system yet because of cyclic imports - # OR to something else than a class :/ - return None - - # note: the class on which the property is defined (_cls) does not have + # note: the class on which the property is defined does not have # to be in the MRO of the parent - attr_def = _cls.find(_property_name[-1]) + attr_def = parent.resolveName('.'.join(property_name)) if not isinstance(attr_def, model.Property): return None @@ -842,7 +831,8 @@ def _handleFunctionDef(self, is_property = False property_deco: Optional[List[str]] = None - + property_function_kind: Optional[model.Property.Kind] = None + if node.decorator_list: for deco_name, _ in astutils.iter_decorator_list(node.decorator_list): if deco_name is None: @@ -855,20 +845,21 @@ def _handleFunctionDef(self, elif deco_name == ['staticmethod']: is_staticmethod = True # Pre-handle property elements - elif _get_property_function_kind(deco_name): - # Setters and deleters should have the same name as the property function, - # otherwise ignore it. - # This pollutes the namespace unnecessarily and is generally not recommended. - # Therefore it makes sense to stick to a single name, - # which is consistent with the former property definition. - if not deco_name[-2] == func_name: - continue - - # Rename the setter/deleter, so it doesn't replace - # the property object. - - func_name = '.'.join(deco_name[-2:]) - property_deco = deco_name + else: + property_function_kind = _get_property_function_kind(deco_name) + if property_function_kind: + # Setters and deleters should have the same name as the property function, + # otherwise ignore it. + # This pollutes the namespace unnecessarily and is generally not recommended. + # Therefore it makes sense to stick to a single name, + # which is consistent with the former property definition. + if not deco_name[-2] == func_name: + continue + + # Rename the setter/deleter, so it doesn't replace + # the property object. + func_name = '.'.join(deco_name[-2:]) + property_deco = deco_name # Determine if the function is decorated with overload if parent.expandName('.'.join(deco_name)) in ('typing.overload', @@ -876,35 +867,29 @@ def _handleFunctionDef(self, is_overload_func = True # Determine if this function is a property of some kind - - func_property: Optional[model.Property] = None - prop_func_kind: Optional[model.Property.Kind] = None + property_model: Optional[model.Property] = None is_new_property: bool = is_property if property_deco is not None: - - prop_func_kind = _get_property_function_kind(property_deco) - # Looks like inherited property if len(property_deco)>2: _inherited_property = _get_inherited_property(property_deco, parent) if _inherited_property: - func_property = self._addProperty(node, parent, lineno) + property_model = self._addProperty(node, parent, lineno) # copy property defs info - func_property.getter = _inherited_property.getter - func_property.setter = _inherited_property.setter - func_property.deleter = _inherited_property.deleter + property_model.getter = _inherited_property.getter + property_model.setter = _inherited_property.setter + property_model.deleter = _inherited_property.deleter is_new_property = True - else: # fetch property info to add this info to it _maybe_prop = self.builder.current.contents.get(node.name) if isinstance(_maybe_prop, model.Property): - func_property = _maybe_prop + property_model = _maybe_prop elif is_property: - func_property = self._addProperty(node, parent, lineno) - prop_func_kind = model.Property.Kind.GETTER + property_model = self._addProperty(node, parent, lineno) + property_function_kind = model.Property.Kind.GETTER # Rename the getter as well func_name = node.name + '.getter' @@ -924,7 +909,7 @@ def _handleFunctionDef(self, # Do not recreate function object, just re-push it self.builder.push(existing_func, lineno) func = existing_func - elif isinstance(existing_func, model.Function) and func_property is not None and not is_new_property: + elif isinstance(existing_func, model.Function) and property_model is not None and not is_new_property: # Check if this property function is overriding a previously defined # property function on the same scope before pushing the new function # If it does override something, just re-push the function, do not override it. @@ -1004,16 +989,16 @@ def add_arg(name: str, kind: Any, default: Optional[ast.expr]) -> None: else: func.signature = signature - if func_property is not None: + if property_model is not None: if is_classmethod: - func_property.report(f'{func_property.fullName()} is both property and classmethod') + property_model.report(f'{property_model.fullName()} is both property and classmethod') if is_staticmethod: - func_property.report(f'{func_property.fullName()} is both property and staticmethod') + property_model.report(f'{property_model.fullName()} is both property and staticmethod') - assert prop_func_kind is not None + assert property_function_kind is not None # Save the fact that this function implements one of the getter/setter/deleter - func_property.store_function(prop_func_kind, func) + property_model.store_function(property_function_kind, func) def depart_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: From 2532a069bfd259fd70f4b92be01742a4539e71f6 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Sun, 23 Apr 2023 13:33:40 -0400 Subject: [PATCH 33/47] Simplify propertyInfo renderer --- pydoctor/templatewriter/pages/attributechild.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pydoctor/templatewriter/pages/attributechild.py b/pydoctor/templatewriter/pages/attributechild.py index cceaab191..d1a7f3c27 100644 --- a/pydoctor/templatewriter/pages/attributechild.py +++ b/pydoctor/templatewriter/pages/attributechild.py @@ -88,11 +88,8 @@ def propertyInfo(self, request: object, tag: Tag) -> "Flattenable": # Property info consist in nested function child elements that # formats the setter and deleter docs of the property. r = [] - if self.ob.kind is DocumentableKind.PROPERTY: + if isinstance(self.ob, Property): from pydoctor.templatewriter.pages.functionchild import FunctionChild - - assert isinstance(self.ob, Property) - for func in [f for f in (self.ob.setter, self.ob.deleter) if f]: r.append(FunctionChild(self.docgetter, func, extras=[], loader=self._funcLoader, silent_undoc=True)) From f2632a77c3612ec66c7525cda6605dfcc25dbc93 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Sun, 23 Apr 2023 15:11:30 -0400 Subject: [PATCH 34/47] add missing PropertyMixin to mixinT --- pydoctor/extensions/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/extensions/__init__.py b/pydoctor/extensions/__init__.py index 69222d55c..e9e1af888 100644 --- a/pydoctor/extensions/__init__.py +++ b/pydoctor/extensions/__init__.py @@ -39,7 +39,7 @@ class CanContainImportsDocumentableMixin(PackageMixin, ModuleMixin, ClassMixin): class InheritableMixin(FunctionMixin, AttributeMixin, PropertyMixin): """Base class for mixins applied to L{model.Function} and L{model.Attribute} objects.""" -MixinT = Union[ClassMixin, ModuleMixin, PackageMixin, FunctionMixin, AttributeMixin] +MixinT = Union[ClassMixin, ModuleMixin, PackageMixin, FunctionMixin, AttributeMixin, PropertyMixin] def _importlib_resources_contents(package: str) -> Iterable[str]: """Return an iterable of entries in C{package}. From 4383240365c35adde607d339ea401ca78029c0a6 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Mon, 24 Apr 2023 00:18:03 -0400 Subject: [PATCH 35/47] Refacfors --- pydoctor/astbuilder.py | 15 +++---- pydoctor/model.py | 11 +++-- pydoctor/templatewriter/pages/__init__.py | 15 ++++--- .../templatewriter/pages/attributechild.py | 42 +++++++++++++------ pydoctor/test/test_astbuilder.py | 2 - pydoctor/test/test_templatewriter.py | 12 ++++-- 6 files changed, 58 insertions(+), 39 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 219a24a0c..362dbf730 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -794,9 +794,10 @@ def visit_FunctionDef(self, node: ast.FunctionDef) -> None: def _addProperty(self, node:Union[ast.AsyncFunctionDef, ast.FunctionDef], parent:model.Documentable, lineno:int,) -> model.Property: - attribute = self.builder.addProperty(node.name, parent=parent) + attribute = self.builder.addAttribute(node.name, + model.DocumentableKind.PROPERTY, + parent) attribute.setLineNumber(lineno) - attribute.decorators = node.decorator_list assert isinstance(attribute, model.Property) return attribute @@ -1233,21 +1234,15 @@ def addAttribute(self, parentMod = self.currentMod if kind is model.DocumentableKind.PROPERTY: attr:model.Attribute = system.Property(system, name, parent) + assert attr.kind is model.DocumentableKind.PROPERTY else: attr = system.Attribute(system, name, parent) - attr.kind = kind + attr.kind = kind attr.parentMod = parentMod system.addObject(attr) self.currentAttr = attr return attr - def addProperty(self, - name: str, parent: model.Documentable - ) -> model.Property: - return cast(model.Property, self.addAttribute(name, - model.DocumentableKind.PROPERTY, - parent)) - def processModuleAST(self, mod_ast: ast.Module, mod: model.Module) -> None: for name, node in findModuleLevelAssign(mod_ast): diff --git a/pydoctor/model.py b/pydoctor/model.py index 5e10beeea..82850f859 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -846,7 +846,6 @@ def init_property(attrib:'Property') -> Iterator['Function']: # Setup Attribute object for the property attrib.docstring = getter.docstring attrib.annotation = getter.annotations.get('return') - attrib.decorators = getter.decorators attrib.extra_info.extend(getter.extra_info) @@ -887,7 +886,6 @@ def init_property(attrib:'Property') -> Iterator['Function']: class Attribute(Inheritable): kind: Optional[DocumentableKind] = DocumentableKind.ATTRIBUTE annotation: Optional[ast.expr] - decorators: Optional[Sequence[ast.expr]] = None # decorators are used only if the attribute is a property value: Optional[ast.expr] = None """ The value of the assignment expression. @@ -898,13 +896,18 @@ class Attribute(Inheritable): """ class Property(Attribute): - # for the templatewriter, property are just like attributes - __class__ = Attribute #type:ignore + kind: DocumentableKind = DocumentableKind.PROPERTY class Kind(Enum): GETTER = 1 SETTER = 2 DELETER = 3 + + @property + def decorators(self) -> Optional[Sequence[ast.expr]]: + if self.getter: + return self.getter.decorators + return None def setup(self) -> None: super().setup() diff --git a/pydoctor/templatewriter/pages/__init__.py b/pydoctor/templatewriter/pages/__init__.py index 12b6f749a..1fe9bff36 100644 --- a/pydoctor/templatewriter/pages/__init__.py +++ b/pydoctor/templatewriter/pages/__init__.py @@ -22,7 +22,7 @@ if TYPE_CHECKING: from typing_extensions import Final from twisted.web.template import Flattenable - from pydoctor.templatewriter.pages.attributechild import AttributeChild + from pydoctor.templatewriter.pages.attributechild import AttributeChild, PropertyChild from pydoctor.templatewriter.pages.functionchild import FunctionChild @@ -45,7 +45,7 @@ def map_kind(kind: model.DocumentableKind) -> model.DocumentableKind: return (-o.privacyClass.value, -map_kind(o.kind).value if o.kind else 0, o.fullName().lower()) -def format_decorators(obj: Union[model.Function, model.Attribute, model.FunctionOverload]) -> Iterator["Flattenable"]: +def format_decorators(obj: Union[model.Function, model.Property, model.FunctionOverload]) -> Iterator["Flattenable"]: # Since we use this function to colorize the FunctionOverload decorators and it's not an actual Documentable subclass, we use the overload's # primary function for parts that requires an interface to Documentable methods or attributes documentable_obj = obj if not isinstance(obj, model.FunctionOverload) else obj.primary @@ -287,19 +287,22 @@ def methods(self) -> Sequence[model.Documentable]: key=util.objects_order) def childlist(self) -> List[Union["AttributeChild", "FunctionChild"]]: - from pydoctor.templatewriter.pages.attributechild import AttributeChild + from pydoctor.templatewriter.pages.attributechild import AttributeChild, PropertyChild from pydoctor.templatewriter.pages.functionchild import FunctionChild - r: List[Union["AttributeChild", "FunctionChild"]] = [] + r: List[Union["PropertyChild", "AttributeChild", "FunctionChild"]] = [] func_loader = FunctionChild.lookup_loader(self.template_lookup) attr_loader = AttributeChild.lookup_loader(self.template_lookup) + property_loader = PropertyChild.lookup_loader(self.template_lookup) for c in self.methods(): - if isinstance(c, model.Function): + if isinstance(c, model.Property): + r.append(PropertyChild(self.docgetter, c, self.objectExtras(c), property_loader, func_loader)) + elif isinstance(c, model.Function): r.append(FunctionChild(self.docgetter, c, self.objectExtras(c), func_loader)) elif isinstance(c, model.Attribute): - r.append(AttributeChild(self.docgetter, c, self.objectExtras(c), attr_loader, func_loader)) + r.append(AttributeChild(self.docgetter, c, self.objectExtras(c), attr_loader)) else: assert False, type(c) return r diff --git a/pydoctor/templatewriter/pages/attributechild.py b/pydoctor/templatewriter/pages/attributechild.py index d1a7f3c27..64ed9b8c1 100644 --- a/pydoctor/templatewriter/pages/attributechild.py +++ b/pydoctor/templatewriter/pages/attributechild.py @@ -3,7 +3,7 @@ from twisted.web.iweb import ITemplateLoader from twisted.web.template import Tag, renderer, tags -from pydoctor.model import Attribute, Property, DocumentableKind +from pydoctor.model import Attribute, Property from pydoctor import epydoc2stan from pydoctor.templatewriter import TemplateElement, util from pydoctor.templatewriter.pages import format_decorators @@ -21,13 +21,11 @@ def __init__(self, ob: Attribute, extras: List["Flattenable"], loader: ITemplateLoader, - funcLoader: ITemplateLoader, ): super().__init__(loader) self.docgetter = docgetter self.ob = ob self._functionExtras = extras - self._funcLoader = funcLoader @renderer def class_(self, request: object, tag: Tag) -> "Flattenable": @@ -49,10 +47,6 @@ def anchorHref(self, request: object, tag: Tag) -> str: name = self.shortFunctionAnchor(request, tag) return f'#{name}' - @renderer - def decorator(self, request: object, tag: Tag) -> "Flattenable": - return list(format_decorators(self.ob)) - @renderer def attribute(self, request: object, tag: Tag) -> "Flattenable": attr: List["Flattenable"] = [tags.span(self.ob.name, class_='py-defname')] @@ -83,14 +77,36 @@ def constantValue(self, request: object, tag: Tag) -> "Flattenable": # Attribute is a constant/type alias (with a value), then display it's value return epydoc2stan.format_constant_value(self.ob) + @renderer + def decorator(self, request: object, tag: Tag) -> "Flattenable": + return () + + @renderer + def propertyInfo(self, request: object, tag: Tag) -> "Flattenable": + return () + +class PropertyChild(AttributeChild): + ob: Property + + def __init__(self, docgetter: util.DocGetter, + ob: Property, + extras: List['Flattenable'], + loader: ITemplateLoader, + funcLoader: ITemplateLoader): + super().__init__(docgetter, ob, extras, loader) + self._funcLoader = funcLoader + + @renderer + def decorator(self, request: object, tag: Tag) -> "Flattenable": + return list(format_decorators(self.ob)) + @renderer def propertyInfo(self, request: object, tag: Tag) -> "Flattenable": # Property info consist in nested function child elements that # formats the setter and deleter docs of the property. r = [] - if isinstance(self.ob, Property): - from pydoctor.templatewriter.pages.functionchild import FunctionChild - for func in [f for f in (self.ob.setter, self.ob.deleter) if f]: - r.append(FunctionChild(self.docgetter, func, extras=[], - loader=self._funcLoader, silent_undoc=True)) - return r + from pydoctor.templatewriter.pages.functionchild import FunctionChild + for func in [f for f in (self.ob.setter, self.ob.deleter) if f]: + r.append(FunctionChild(self.docgetter, func, extras=[], + loader=self._funcLoader, silent_undoc=True)) + return r \ No newline at end of file diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 4d56e71d7..c2e9cefa2 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -1532,8 +1532,6 @@ def prop(self): getter = C.contents['prop'] assert isinstance(getter, model.Property) - assert getter.__class__ is model.Attribute # type:ignore - assert getter.docstring == """Getter.""" setter = getter.setter diff --git a/pydoctor/test/test_templatewriter.py b/pydoctor/test/test_templatewriter.py index 3a0a21806..60c9a7f98 100644 --- a/pydoctor/test/test_templatewriter.py +++ b/pydoctor/test/test_templatewriter.py @@ -14,7 +14,7 @@ HtmlTemplate, UnsupportedTemplateVersion, OverrideTemplateNotAllowed) from pydoctor.templatewriter.pages.table import ChildTable -from pydoctor.templatewriter.pages.attributechild import AttributeChild +from pydoctor.templatewriter.pages.attributechild import AttributeChild, PropertyChild from pydoctor.templatewriter.pages.functionchild import FunctionChild from pydoctor.templatewriter.summary import isClassNodePrivate, isPrivate, moduleSummary from pydoctor.test.test_astbuilder import fromText, systemcls_param @@ -60,9 +60,13 @@ def getHTMLOf(ob: model.Documentable) -> str: def getHTMLOfAttribute(ob: model.Documentable) -> str: assert isinstance(ob, model.Attribute) tlookup = TemplateLookup(template_dir) - stan = AttributeChild(util.DocGetter(), ob, [], - AttributeChild.lookup_loader(tlookup), - FunctionChild.lookup_loader(tlookup)) + if isinstance(ob, model.Property): + stan = PropertyChild(util.DocGetter(), ob, [], + PropertyChild.lookup_loader(tlookup), + FunctionChild.lookup_loader(tlookup)) + else: + stan = AttributeChild(util.DocGetter(), ob, [], + AttributeChild.lookup_loader(tlookup)) return flatten(stan) def test_sidebar() -> None: From 21a047aa260ee9396932a9e2e85c5d421639ca75 Mon Sep 17 00:00:00 2001 From: tristanlatr <19967168+tristanlatr@users.noreply.github.com> Date: Sun, 4 Jun 2023 19:35:32 -0400 Subject: [PATCH 36/47] Update docs/epytext_demo/demo_epytext_module.py --- docs/epytext_demo/demo_epytext_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/epytext_demo/demo_epytext_module.py b/docs/epytext_demo/demo_epytext_module.py index 6eb771951..e293a67db 100644 --- a/docs/epytext_demo/demo_epytext_module.py +++ b/docs/epytext_demo/demo_epytext_module.py @@ -187,7 +187,7 @@ def read_and_write_delete(self) -> None: @property def undoc_prop(self) -> bytes: - """This property has a docstring only on the setter.""" + """This property has a docstring only on the getter.""" return b'' @undoc_prop.setter def undoc_prop(self, p) -> None: # type:ignore From 4f6b591ff1aa5aa02e6a06769ed744e5721b0f33 Mon Sep 17 00:00:00 2001 From: tristanlatr <19967168+tristanlatr@users.noreply.github.com> Date: Sun, 4 Jun 2023 19:35:48 -0400 Subject: [PATCH 37/47] Update docs/restructuredtext_demo/demo_restructuredtext_module.py --- docs/restructuredtext_demo/demo_restructuredtext_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/restructuredtext_demo/demo_restructuredtext_module.py b/docs/restructuredtext_demo/demo_restructuredtext_module.py index a760f0554..0ddfd4944 100644 --- a/docs/restructuredtext_demo/demo_restructuredtext_module.py +++ b/docs/restructuredtext_demo/demo_restructuredtext_module.py @@ -219,7 +219,7 @@ def read_and_write_delete(self) -> None: @property def undoc_prop(self) -> bytes: - """This property has a docstring only on the setter.""" + """This property has a docstring only on the getter.""" @undoc_prop.setter def undoc_prop(self, p) -> None: # type:ignore ... From de678455290bdbf7c935000182b56890e3141f8f Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Mon, 5 Jun 2023 09:27:32 -0400 Subject: [PATCH 38/47] Remove inherited property support. Add support for old school property() --- pydoctor/astbuilder.py | 90 ++++++++++----------- pydoctor/model.py | 80 +++++++++--------- pydoctor/test/test_astbuilder.py | 135 ++++++++++++++++--------------- 3 files changed, 153 insertions(+), 152 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 44048bfd0..0cf313b5b 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -4,7 +4,7 @@ import sys from functools import partial -from inspect import Parameter, Signature +from inspect import Parameter, Signature, signature from itertools import chain from pathlib import Path from typing import ( @@ -30,6 +30,7 @@ def parseFile(path: Path) -> ast.Module: else: _parse = ast.parse +_property_signature = signature(property) def _maybeAttribute(cls: model.Class, name: str) -> bool: """Check whether a name is a potential attribute of the given class. @@ -156,31 +157,6 @@ def _is_property_decorator(dottedname:Sequence[str], ctx:model.Documentable) -> return True return False - -def _get_inherited_property(dottedname:Sequence[str], parent: model.Documentable) -> Optional[model.Property]: - """ - Fetch the inherited property that this new decorator overrides. - Returns C{None} if it doesn't exist in the inherited members or if it's already definied in the locals. - The dottedname must have at least three elements, else return C{None}. - """ - # TODO: It would be best if this job was done in post-processing... - - property_name = dottedname[:-1] - - if len(property_name) <= 1 or property_name[-1] in parent.contents: - # the property already exist - return None - - # attr can be a getter/setter/deleter - # note: the class on which the property is defined does not have - # to be in the MRO of the parent - attr_def = parent.resolveName('.'.join(property_name)) - - if not isinstance(attr_def, model.Property): - return None - - return attr_def - def _get_property_function_kind(dottedname:Sequence[str]) -> Optional[model.Property.Kind]: """ What kind of property function this decorator declares? @@ -483,6 +459,8 @@ def _handleOldSchoolMethodDecoration(self, target: str, expr: Optional[ast.expr] if not isinstance(func, ast.Name): return False func_name = func.id + if func_name == 'property': + return self._handleOldSchoolPropertyDecoration(target, expr) args = expr.args if len(args) != 1: return False @@ -503,6 +481,34 @@ def _handleOldSchoolMethodDecoration(self, target: str, expr: Optional[ast.expr] return True return False + def _handleOldSchoolPropertyDecoration(self, target: str, expr: ast.Call) -> bool: + try: + bound_args = astutils.bind_args(_property_signature, expr) + except: + return False + + cls = self.builder.current + + getter = node2dottedname(bound_args.arguments.get('fget')) + setter = node2dottedname(bound_args.arguments.get('fset')) + deleter = node2dottedname(bound_args.arguments.get('fdel')) + doc = bound_args.arguments.get('doc') + + attr = self._addProperty(target, cls, expr.lineno) + if getter: + attr.getter = cls.contents.get(getter[0]) + if setter: + attr.setter = cls.contents.get(setter[0]) + if deleter: + attr.deleter = cls.contents.get(deleter[0]) + + if doc: + if attr.getter and attr.getter.docstring: + attr.report('Proerty docstring is overriden by property() call "doc" argument.') + attr.setDocstring(doc) + + return True + def _warnsConstantAssigmentOverride(self, obj: model.Attribute, lineno_offset: int) -> None: obj.report(f'Assignment to constant "{obj.name}" overrides previous assignment ' f'at line {obj.linenumber}, the original value will not be part of the docs.', @@ -793,9 +799,8 @@ def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: def visit_FunctionDef(self, node: ast.FunctionDef) -> None: self._handleFunctionDef(node, is_async=False) - def _addProperty(self, node:Union[ast.AsyncFunctionDef, ast.FunctionDef], - parent:model.Documentable, lineno:int,) -> model.Property: - attribute = self.builder.addAttribute(node.name, + def _addProperty(self, name:str, parent:model.Documentable, lineno:int,) -> model.Property: + attribute = self.builder.addAttribute(name, model.DocumentableKind.PROPERTY, parent) attribute.setLineNumber(lineno) @@ -846,8 +851,8 @@ def _handleFunctionDef(self, is_classmethod = True elif deco_name == ['staticmethod']: is_staticmethod = True - # Pre-handle property elements else: + # Pre-handle property elements property_function_kind = _get_property_function_kind(deco_name) if property_function_kind: # Setters and deleters should have the same name as the property function, @@ -872,27 +877,22 @@ def _handleFunctionDef(self, property_model: Optional[model.Property] = None is_new_property: bool = is_property - if property_deco is not None: - # Looks like inherited property - if len(property_deco)>2: - _inherited_property = _get_inherited_property(property_deco, parent) - if _inherited_property: - property_model = self._addProperty(node, parent, lineno) - # copy property defs info - property_model.getter = _inherited_property.getter - property_model.setter = _inherited_property.setter - property_model.deleter = _inherited_property.deleter - is_new_property = True - else: - # fetch property info to add this info to it + if property_deco: + if len(property_deco)==2: + # This is a property getter or deleter + # We don't support non local properties definitions (when len(property_deco)>2) + # I've only saw this in cpython test cases. _maybe_prop = self.builder.current.contents.get(node.name) if isinstance(_maybe_prop, model.Property): property_model = _maybe_prop + # We don't report warnings if we can't figure out the property model. elif is_property: - property_model = self._addProperty(node, parent, lineno) + # This is a new property definition + property_model = self._addProperty(node.name, parent, lineno) property_function_kind = model.Property.Kind.GETTER - # Rename the getter as well + # Rename the getter function as well, since both the Property and the Function will + # live side by side until properties are post-processed. func_name = node.name + '.getter' # Push and analyse function diff --git a/pydoctor/model.py b/pydoctor/model.py index 7cefb5031..045ec0717 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -837,13 +837,13 @@ class FunctionOverload: decorators: Sequence[ast.expr] def init_properties(system:'System') -> None: - # Machup property functions into an Attribute that already exist. - # We are transforming the tree at the end only. - to_prune: List[Documentable] = [] + # Machup property Functons into the Property object, + # and remove them from the tree. + to_prune: Set[Documentable] = set() for attrib in system.objectsOfType(Property): - to_prune.extend(init_property(attrib)) + to_prune.update(init_property(attrib)) - for obj in set(to_prune): + for obj in to_prune: system._remove(obj) assert obj.parent is not None if obj.name in obj.parent.contents: @@ -851,54 +851,56 @@ def init_properties(system:'System') -> None: def init_property(attrib:'Property') -> Iterator['Function']: """ - Initiates the L{Attribute} that represent the property in the tree. + Initiates the L{Property} that represent the property in the tree. - Returns the functions to remove from the tree. If the property matchup fails + Returns the functions to remove from the tree. """ getter = attrib.getter setter = attrib.setter deleter = attrib.deleter if getter is None: - # The getter should never be None + # The getter should never be None, + # but it can execpitinally happend when + # one uses the property() call alone. return () # avoid cyclic import from pydoctor import epydoc2stan - # Setup Attribute object for the property - attrib.docstring = getter.docstring - attrib.annotation = getter.annotations.get('return') + # Setup Attribute object for the property + if not attrib.docstring: + attrib.docstring = getter.docstring + attrib.docstring_lineno = getter.docstring_lineno + attrib.annotation = getter.annotations.get('return') attrib.extra_info.extend(getter.extra_info) # Parse docstring now. if epydoc2stan.ensure_parsed_docstring(getter): - pdoc = getter.parsed_docstring - assert pdoc is not None + parsed_doc = getter.parsed_docstring + assert parsed_doc is not None other_fields = [] # process fields such that :returns: clause docs takes the whole docs # if no global description is written. - for field in pdoc.fields: + for field in parsed_doc.fields: tag = field.tag() if tag == 'return': - if not pdoc.has_body: - pdoc = field.body() + if not parsed_doc.has_body: + parsed_doc = field.body() elif tag == 'rtype': attrib.parsed_type = field.body() else: other_fields.append(field) - pdoc.fields = other_fields + parsed_doc.fields = other_fields # Set the new attribute parsed docstring - attrib.parsed_docstring = pdoc - - # maybe TODO: Add inheritence info to getter/setter/deleters + attrib.parsed_docstring = parsed_doc - # Yield the objects to remove from the Documentable tree + # Yields the objects to remove from the Documentable tree yield getter if setter: yield setter @@ -911,16 +913,25 @@ class Attribute(Inheritable): annotation: Optional[ast.expr] value: Optional[ast.expr] = None """ - The value of the assignment expression. - - None value means the value is not initialized - at the current point of the the process. - Or maybe it can be that the attribute is a property. + The value of the assignment expression. """ class Property(Attribute): kind: DocumentableKind = DocumentableKind.PROPERTY - + + getter:Optional['Function'] = None + """ + The getter. + """ + setter: Optional['Function'] = None + """ + None if it has not been set with C{@.setter} decorator. + """ + deleter: Optional['Function'] = None + """ + None if it has not been set with C{@.deleter} decorator. + """ + class Kind(Enum): GETTER = 1 SETTER = 2 @@ -932,21 +943,6 @@ def decorators(self) -> Optional[Sequence[ast.expr]]: return self.getter.decorators return None - def setup(self) -> None: - super().setup() - self.getter:Optional['Function'] = None - """ - The getter. - """ - self.setter: Optional['Function'] = None - """ - None if it has not been set with C{@.setter} decorator. - """ - self.deleter: Optional['Function'] = None - """ - None if it has not been set with C{@.deleter} decorator. - """ - def store_function(self, kind: 'Property.Kind', func:'Function') -> None: if kind is Property.Kind.GETTER: self.getter = func diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 1a09b48ca..b62312193 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2055,9 +2055,10 @@ def data(self): assert mod.contents['A'].contents['_data'].kind is model.DocumentableKind.INSTANCE_VARIABLE @systemcls_param -def test_property_inherited(systemcls: Type[model.System], capsys: CapSys) -> None: +def test_property_inherited_not_supported(systemcls: Type[model.System], capsys: CapSys) -> None: """ - Properties can be inherited. + Properties can be inherited, but we don't support it for now :/ + Nevertheless, the test case exist for future developments. """ # source from cpython test_property.py src = ''' @@ -2088,84 +2089,86 @@ class SubClass2(BaseClass): def spam(self): """SubClass.setter""" pass - - class SubClass3(BaseClass): - # inherited property - @BaseClass.spam.deleter - def spam(self): - """SubClass.deleter""" - pass ''' mod = fromText(src, modname='mod', systemcls=systemcls) assert not capsys.readouterr().out spam0 = mod.contents['BaseClass'].contents['spam'] - spam1 = mod.contents['SubClass'].contents['spam'] - spam2 = mod.contents['SubClass2'].contents['spam'] - spam3 = mod.contents['SubClass3'].contents['spam'] + spam1 = mod.contents['SubClass'].contents['spam.getter'] + spam2 = mod.contents['SubClass2'].contents['spam.setter'] - assert list(mod.contents['BaseClass'].contents) == \ - list(mod.contents['SubClass'].contents) == \ - list(mod.contents['SubClass2'].contents) == \ - list(mod.contents['SubClass3'].contents) == ['spam'] + assert list(mod.contents['BaseClass'].contents) == ['spam'] + assert list(mod.contents['SubClass'].contents) == ['spam.getter'] + assert list(mod.contents['SubClass2'].contents) == ['spam.setter'] assert isinstance(spam0, model.Property) - assert isinstance(spam1, model.Property) - assert isinstance(spam2, model.Property) - assert isinstance(spam3, model.Property) + assert isinstance(spam1, model.Function) + assert isinstance(spam2, model.Function) assert spam0.kind is model.DocumentableKind.PROPERTY - assert spam1.kind is model.DocumentableKind.PROPERTY - assert spam2.kind is model.DocumentableKind.PROPERTY - assert spam3.kind is model.DocumentableKind.PROPERTY + assert spam1.kind is model.DocumentableKind.METHOD + assert spam2.kind is model.DocumentableKind.METHOD assert isinstance(spam0.setter, model.Function) - assert isinstance(spam1.setter, model.Function) - assert isinstance(spam2.setter, model.Function) - assert isinstance(spam3.setter, model.Function) assert isinstance(spam0.deleter, model.Function) - assert isinstance(spam1.deleter, model.Function) - assert isinstance(spam2.deleter, model.Function) - assert isinstance(spam3.deleter, model.Function) assert spam0.getter.fullName() == 'mod.BaseClass.spam.getter' #type:ignore[union-attr] - assert spam0.setter.fullName() == 'mod.BaseClass.spam.setter' - assert spam0.deleter.fullName() == 'mod.BaseClass.spam.deleter' - - assert spam1.getter.fullName() == 'mod.SubClass.spam.getter' #type:ignore[union-attr] - assert spam1.setter.fullName() == 'mod.BaseClass.spam.setter' - assert spam1.deleter.fullName() == 'mod.BaseClass.spam.deleter' - - assert spam2.getter.fullName() == 'mod.BaseClass.spam.getter' #type:ignore[union-attr] - assert spam2.setter.fullName() == 'mod.SubClass2.spam.setter' - assert spam2.deleter.fullName() == 'mod.BaseClass.spam.deleter' - - assert spam3.getter.fullName() == 'mod.BaseClass.spam.getter' #type:ignore[union-attr] - assert spam3.setter.fullName() == 'mod.BaseClass.spam.setter' - assert spam3.deleter.fullName() == 'mod.SubClass3.spam.deleter' - - assert spam1.getter is not spam0.getter @systemcls_param def test_property_old_school(systemcls: Type[model.System], capsys: CapSys) -> None: """ - We don't support it for now. + """ - ''' + src = ''' class PropertyDocBase(object): _spam = 1 def _get_spam(self): return self._spam # Old school property spam = property(_get_spam, doc="spam spam spam") - class PropertyDocSub(PropertyDocBase): - @PropertyDocBase.spam.getter - def spam(self): - # The docstring is ignored since the property is defined with old school manner - """The decorator does not use this doc string""" - return self._spam + ''' + mod = fromText(src, modname='mod', systemcls=systemcls) + assert not capsys.readouterr().out + + spam = mod.resolveName('PropertyDocBase.spam') + assert isinstance(spam, model.Property) + assert spam.docstring == "spam spam spam" + +@systemcls_param +def test_property_old_school_keywords(systemcls:Type[model.System], capsys:CapSys) -> None: + src = ''' + class Circle: + def __init__(self, radius): + self._radius = radius + + def _get_radius(self): + # should not have docs here, + # since there is doc parameter to the property() call. + return self._radius + + def _set_radius(self, value): + "setter docs" + self._radius = value + + def _del_radius(self): + "deleter docs" + del self._radius + + radius = property( + fget=_get_radius, + fset=_set_radius, + fdel=_del_radius, + doc="The radius property." + ) + ''' + mod = fromText(src, modname='mod', systemcls=systemcls) + assert not capsys.readouterr().out + + spam = mod.resolveName('Circle.radius') + assert isinstance(spam, model.Property) + assert spam.docstring == "The radius property." @systemcls_param def test_property_getter_override(systemcls: Type[model.System], capsys: CapSys) -> None: @@ -2194,9 +2197,11 @@ def spam(self): assert attr.linenumber == 4 @systemcls_param -def test_property_corner_cases(systemcls: Type[model.System], capsys: CapSys) -> None: +def test_property_not_supported_or_corner_cases(systemcls: Type[model.System], capsys: CapSys) -> None: """ Property handling can be quite complex, there are many corner cases. + But the general rule is that when a function ressemble a property function, + it's renamed such that it ends with .setter or .deleter. """ base_mod = ''' @@ -2299,21 +2304,21 @@ def notfound(self): mod = system.allobjects['mod'] src = system.allobjects['src'] - # Pydoctor doesn't understand this property because it's using an - # import cycle. So the older behaviour applies: - # only renaming the method + # Pydoctor doesn't understand this property because it's inherited. + # And plus it belong in a import cycle. So the older behaviour applies: + # only renaming the methods assert list(mod.contents['SubClass'].contents) == ['spam.getter'] assert mod.contents['SubClass'].contents['spam.getter'].kind is model.DocumentableKind.METHOD - assert list(src.contents['SubClass'].contents) == ['spam'] - assert list(src.contents['NotSubClass'].contents) == ['spam'] + assert list(src.contents['SubClass'].contents) == ['spam.getter'] + assert list(src.contents['NotSubClass'].contents) == ['spam.setter','spam.getter'] - spam0 = src.contents['SubClass'].contents['spam'] - spam1 = src.contents['NotSubClass'].contents['spam'] + spam0 = src.contents['SubClass'].contents['spam.getter'] + spam1 = src.contents['NotSubClass'].contents['spam.getter'] - assert list(src.contents['InvalidClass'].contents) == ['spam', 'spam.getter'] + assert list(src.contents['InvalidClass'].contents) == ['spam.setter', 'spam.getter'] - spam2 = src.contents['InvalidClass'].contents['spam'] + spam2 = src.contents['InvalidClass'].contents['spam.setter'] spam2_bogus = src.contents['InvalidClass'].contents['spam.getter'] notfound = src.contents['InvalidClass2'].contents['notfound.getter'] @@ -2322,9 +2327,9 @@ def notfound(self): notfound_alt = src.contents['InvalidClass4'].contents['notfound'] not_a_property = src.contents['InvalidClass5'].contents['notfound.getter'] - assert spam0.kind is model.DocumentableKind.PROPERTY - assert spam1.kind is model.DocumentableKind.PROPERTY - assert spam2.kind is model.DocumentableKind.PROPERTY + assert spam0.kind is model.DocumentableKind.METHOD + assert spam1.kind is model.DocumentableKind.METHOD + assert spam2.kind is model.DocumentableKind.METHOD assert spam2_bogus.kind is model.DocumentableKind.METHOD assert notfound.kind is model.DocumentableKind.METHOD assert notfound_both.kind is model.DocumentableKind.METHOD From 4b8a0ff4b87b88b5104eb3efa4e4378854405b99 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Mon, 5 Jun 2023 10:49:12 -0400 Subject: [PATCH 39/47] Avoid calling 'signature(property)', it fails on some python versions. --- pydoctor/astbuilder.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 0cf313b5b..3a19171d6 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -4,7 +4,7 @@ import sys from functools import partial -from inspect import Parameter, Signature, signature +from inspect import Parameter, Signature from itertools import chain from pathlib import Path from typing import ( @@ -30,7 +30,10 @@ def parseFile(path: Path) -> ast.Module: else: _parse = ast.parse -_property_signature = signature(property) +_property_signature = Signature((Parameter('fget', Parameter.POSITIONAL_OR_KEYWORD, default=None), + Parameter('fset', Parameter.POSITIONAL_OR_KEYWORD, default=None), + Parameter('fdel', Parameter.POSITIONAL_OR_KEYWORD, default=None), + Parameter('doc', Parameter.POSITIONAL_OR_KEYWORD, default=None))) def _maybeAttribute(cls: model.Class, name: str) -> bool: """Check whether a name is a potential attribute of the given class. From c63545195bfc092b455b080f508769c8ca46bd78 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Mon, 5 Jun 2023 11:28:12 -0400 Subject: [PATCH 40/47] Trigger warnings when a docstring is beeing overriden. --- pydoctor/astbuilder.py | 21 ++++++++++++--------- pydoctor/model.py | 20 ++++++++++++++------ pydoctor/test/test_astbuilder.py | 24 ++++++++++++++++++++++-- 3 files changed, 48 insertions(+), 17 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 3a19171d6..fdccc26e3 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -498,18 +498,21 @@ def _handleOldSchoolPropertyDecoration(self, target: str, expr: ast.Call) -> boo doc = bound_args.arguments.get('doc') attr = self._addProperty(target, cls, expr.lineno) - if getter: - attr.getter = cls.contents.get(getter[0]) - if setter: - attr.setter = cls.contents.get(setter[0]) - if deleter: - attr.deleter = cls.contents.get(deleter[0]) + for definition, prop_kind in zip((getter, setter, deleter), + (model.Property.Kind.GETTER, + model.Property.Kind.SETTER, + model.Property.Kind.DELETER)): + if not len(definition or ())==1: + continue + fn = cls.contents.get(definition[0]) + if fn is None: + continue + attr.store_function(prop_kind, fn) if doc: - if attr.getter and attr.getter.docstring: - attr.report('Proerty docstring is overriden by property() call "doc" argument.') - attr.setDocstring(doc) + attr.getter.setDocstring(doc) + self.builder.currentAttr = attr return True def _warnsConstantAssigmentOverride(self, obj: model.Attribute, lineno_offset: int) -> None: diff --git a/pydoctor/model.py b/pydoctor/model.py index 045ec0717..43e8c44a5 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -157,6 +157,16 @@ def setup(self) -> None: def setDocstring(self, node: ast.Str) -> None: lineno, doc = astutils.extract_docstring(node) + self._setDocstringValue(doc, lineno) + + def _setDocstringValue(self, doc:str, lineno:int) -> None: + if self.docstring: + msg = 'Existing docstring' + if self.docstring_lineno: + msg += f' at line {self.docstring_lineno}' + msg += f' is overriden by docstring at line {lineno}' + self.report(msg) + self.docstring = doc self.docstring_lineno = lineno @@ -869,11 +879,9 @@ def init_property(attrib:'Property') -> Iterator['Function']: from pydoctor import epydoc2stan # Setup Attribute object for the property - if not attrib.docstring: - attrib.docstring = getter.docstring - attrib.docstring_lineno = getter.docstring_lineno - - attrib.annotation = getter.annotations.get('return') + attrib._setDocstringValue(getter.docstring, getter.docstring_lineno) + if not attrib.annotation: + attrib.annotation = getter.annotations.get('return') attrib.extra_info.extend(getter.extra_info) # Parse docstring now. @@ -910,7 +918,7 @@ def init_property(attrib:'Property') -> Iterator['Function']: class Attribute(Inheritable): kind: Optional[DocumentableKind] = DocumentableKind.ATTRIBUTE - annotation: Optional[ast.expr] + annotation: Optional[ast.expr] = None value: Optional[ast.expr] = None """ The value of the assignment expression. diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index b62312193..c14bc7e67 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2118,7 +2118,7 @@ def spam(self): @systemcls_param def test_property_old_school(systemcls: Type[model.System], capsys: CapSys) -> None: """ - + Old school property() decorator is recognized. """ src = ''' class PropertyDocBase(object): @@ -2190,12 +2190,32 @@ def spam(self): return 8 ''' mod = fromText(src, modname='mod', systemcls=systemcls) - assert not capsys.readouterr().out + assert capsys.readouterr().out == 'mod:4: Existing docstring at line 6 is overriden by docstring at line 11\n' attr = mod.contents['PropertyNewGetter'].contents['spam'] # the parsed_docstring attribute gets initiated in post-processing assert node2stan.gettext(attr.parsed_docstring.to_node()) == ['new docstring'] # type:ignore assert attr.linenumber == 4 +@systemcls_param +def test_mutilple_docstrings_on_property(systemcls: Type[model.System], capsys: CapSys) -> None: + """ + When pydoctor encounters multiple places where the docstring is defined, it reports a warning. + This can only happend for properties at the moment. + """ + src = ''' + class PropertyOld(object): + def _get_spam(self): + "First doc" + # Old school property + spam = property(_get_spam, doc="Second doc") + "Third doc" + ''' + mod = fromText(src, systemcls=systemcls) + spam = mod.resolveName('PropertyOld.spam') + spam.docstring == "Second doc" + assert capsys.readouterr().out == (':3: Existing docstring at line 4 is overriden by docstring at line 6\n' + ':6: Existing docstring at line 7 is overriden by docstring at line 6\n') + @systemcls_param def test_property_not_supported_or_corner_cases(systemcls: Type[model.System], capsys: CapSys) -> None: """ From 198f91d68f33d43a0b9bddb60a93e5fea79d4a1a Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Mon, 5 Jun 2023 12:02:58 -0400 Subject: [PATCH 41/47] Fix mypy and few refactors --- pydoctor/astbuilder.py | 26 ++++++++++++---------- pydoctor/model.py | 4 +++- pydoctor/test/test_astbuilder.py | 32 ++++++++++++++++++++++++++++ pydoctor/test/test_templatewriter.py | 2 +- 4 files changed, 51 insertions(+), 13 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index fdccc26e3..eccbe0d2a 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -492,25 +492,29 @@ def _handleOldSchoolPropertyDecoration(self, target: str, expr: ast.Call) -> boo cls = self.builder.current - getter = node2dottedname(bound_args.arguments.get('fget')) - setter = node2dottedname(bound_args.arguments.get('fset')) - deleter = node2dottedname(bound_args.arguments.get('fdel')) - doc = bound_args.arguments.get('doc') - attr = self._addProperty(target, cls, expr.lineno) - for definition, prop_kind in zip((getter, setter, deleter), + for arg, prop_kind in zip(('fget', 'fset', 'fdel'), (model.Property.Kind.GETTER, model.Property.Kind.SETTER, - model.Property.Kind.DELETER)): + model.Property.Kind.DELETER), + ): + definition = node2dottedname(bound_args.arguments.get(arg)) if not len(definition or ())==1: - continue - fn = cls.contents.get(definition[0]) - if fn is None: + continue + # the subscript indexing is safe since we've checked len(x)==1 + fn = cls.contents.get(definition[0]) # type:ignore[index] + if not isinstance(fn, model.Function): continue attr.store_function(prop_kind, fn) + doc = bound_args.arguments.get('doc') if doc: - attr.getter.setDocstring(doc) + if attr.getter: + # the warning message in case of overriden docstrings makes + # more sens when relative to the getter docstring. so use that when available. + attr.getter.setDocstring(doc) + else: + attr.setDocstring(doc) self.builder.currentAttr = attr return True diff --git a/pydoctor/model.py b/pydoctor/model.py index 43e8c44a5..f56c1875b 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -879,7 +879,9 @@ def init_property(attrib:'Property') -> Iterator['Function']: from pydoctor import epydoc2stan # Setup Attribute object for the property - attrib._setDocstringValue(getter.docstring, getter.docstring_lineno) + if getter.docstring: + attrib._setDocstringValue(getter.docstring, + getter.docstring_lineno) if not attrib.annotation: attrib.annotation = getter.annotations.get('return') attrib.extra_info.extend(getter.extra_info) diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index c14bc7e67..474a0f7f9 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2170,6 +2170,37 @@ def _del_radius(self): assert isinstance(spam, model.Property) assert spam.docstring == "The radius property." + +@systemcls_param +def test_property_call_alone(systemcls:Type[model.System], capsys:CapSys) -> None: + """ + property() can be used whiout any getters or setters, or with lambda functions. + """ + src = ''' + class PropertyBase: + spam = property() + class PropertyDocBase: + spam = property(doc="spam spam spam") + class PropertyLambdaBase: + spam = property(fget=lambda self:self._spam) + ''' + mod = fromText(src, modname='mod', systemcls=systemcls) + assert not capsys.readouterr().out + + spam1 = mod.contents['PropertyBase'].contents['spam'] + spam2 = mod.contents['PropertyDocBase'].contents['spam'] + spam3 = mod.contents['PropertyLambdaBase'].contents['spam'] + + assert isinstance(spam1, model.Property) + assert isinstance(spam2, model.Property) + assert isinstance(spam3, model.Property) + + assert spam2.docstring == "spam spam spam" + + assert spam1.getter is None + assert spam2.getter is None + assert spam3.getter is None + @systemcls_param def test_property_getter_override(systemcls: Type[model.System], capsys: CapSys) -> None: """ @@ -2212,6 +2243,7 @@ def _get_spam(self): ''' mod = fromText(src, systemcls=systemcls) spam = mod.resolveName('PropertyOld.spam') + assert isinstance(spam, model.Property) spam.docstring == "Second doc" assert capsys.readouterr().out == (':3: Existing docstring at line 4 is overriden by docstring at line 6\n' ':6: Existing docstring at line 7 is overriden by docstring at line 6\n') diff --git a/pydoctor/test/test_templatewriter.py b/pydoctor/test/test_templatewriter.py index 60c9a7f98..a765a7f21 100644 --- a/pydoctor/test/test_templatewriter.py +++ b/pydoctor/test/test_templatewriter.py @@ -61,7 +61,7 @@ def getHTMLOfAttribute(ob: model.Documentable) -> str: assert isinstance(ob, model.Attribute) tlookup = TemplateLookup(template_dir) if isinstance(ob, model.Property): - stan = PropertyChild(util.DocGetter(), ob, [], + stan: "Flattenable" = PropertyChild(util.DocGetter(), ob, [], PropertyChild.lookup_loader(tlookup), FunctionChild.lookup_loader(tlookup)) else: From d2e143b40bbac8473a3c438df4c1cb591dd6b449 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Mon, 5 Jun 2023 12:04:02 -0400 Subject: [PATCH 42/47] Typo --- pydoctor/test/test_astbuilder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 474a0f7f9..ef77a1d36 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2174,7 +2174,7 @@ def _del_radius(self): @systemcls_param def test_property_call_alone(systemcls:Type[model.System], capsys:CapSys) -> None: """ - property() can be used whiout any getters or setters, or with lambda functions. + property() can be used without any getters or setters, or with lambda functions. """ src = ''' class PropertyBase: From d7bdff5b5dca6e79e1a350d8c0683a97911fb2ac Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Wed, 17 Jan 2024 14:57:38 -0500 Subject: [PATCH 43/47] Re-add support for inherited property support. --- pydoctor/astbuilder.py | 71 ++++++++------- pydoctor/test/test_astbuilder.py | 146 +++++++++++++++++-------------- 2 files changed, 121 insertions(+), 96 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 06ccb0d89..e98edd2f8 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -180,6 +180,31 @@ def _is_property_decorator(dottedname:Sequence[str], ctx:model.Documentable) -> return True return False + +def _get_inherited_property(dottedname:Sequence[str], parent: model.Documentable) -> Optional[model.Property]: + """ + Fetch the inherited property that this new decorator overrides. + Returns C{None} if it doesn't exist in the inherited members or if it's already definied in the locals. + The dottedname must have at least three elements, else return C{None}. + """ + # TODO: It would be best if this job was done in post-processing... + + property_name = dottedname[:-1] + + if len(property_name) <= 1 or property_name[-1] in parent.contents: + # the property already exist + return None + + # attr can be a getter/setter/deleter + # note: the class on which the property is defined does not have + # to be in the MRO of the parent + attr_def = parent.resolveName('.'.join(property_name)) + + if not isinstance(attr_def, model.Property): + return None + + return attr_def + def _get_property_function_kind(dottedname:Sequence[str]) -> Optional[model.Property.Kind]: """ What kind of property function this decorator declares? @@ -542,7 +567,7 @@ def _handleOldSchoolPropertyDecoration(self, target: str, expr: ast.Call) -> boo attr.store_function(prop_kind, fn) doc = bound_args.arguments.get('doc') - if doc: + if isinstance(doc, astutils.Str): if attr.getter: # the warning message in case of overriden docstrings makes # more sens when relative to the getter docstring. so use that when available. @@ -553,29 +578,6 @@ def _handleOldSchoolPropertyDecoration(self, target: str, expr: ast.Call) -> boo self.builder.currentAttr = attr return True - def _warnsConstantAssigmentOverride(self, obj: model.Attribute, lineno_offset: int) -> None: - obj.report(f'Assignment to constant "{obj.name}" overrides previous assignment ' - f'at line {obj.linenumber}, the original value will not be part of the docs.', - section='ast', lineno_offset=lineno_offset) - - def _warnsConstantReAssigmentInInstance(self, obj: model.Attribute, lineno_offset: int = 0) -> None: - obj.report(f'Assignment to constant "{obj.name}" inside an instance is ignored, this value will not be part of the docs.', - section='ast', lineno_offset=lineno_offset) - - def _handleConstant(self, obj: model.Attribute, value: Optional[ast.expr], lineno: int) -> None: - """Must be called after obj.setLineNumber() to have the right line number in the warning.""" - - if is_attribute_overridden(obj, value): - - if obj.kind in (model.DocumentableKind.CONSTANT, - model.DocumentableKind.VARIABLE, - model.DocumentableKind.CLASS_VARIABLE): - # Module/Class level warning, regular override. - self._warnsConstantAssigmentOverride(obj=obj, lineno_offset=lineno-obj.linenumber) - else: - # Instance level warning caught at the time of the constant detection. - self._warnsConstantReAssigmentInInstance(obj) - @classmethod def _handleConstant(cls, obj:model.Attribute, annotation:Optional[ast.expr], @@ -877,7 +879,7 @@ def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: def visit_FunctionDef(self, node: ast.FunctionDef) -> None: self._handleFunctionDef(node, is_async=False) - def _addProperty(self, name:str, parent:model.Documentable, lineno:int,) -> model.Property: + def _addProperty(self, name: str, parent:model.Documentable, lineno:int,) -> model.Property: attribute = self.builder.addAttribute(name, model.DocumentableKind.PROPERTY, parent) @@ -951,15 +953,22 @@ def _handleFunctionDef(self, property_model: Optional[model.Property] = None is_new_property: bool = is_property - if property_deco: - if len(property_deco)==2: - # This is a property getter or deleter - # We don't support non local properties definitions (when len(property_deco)>2) - # I've only saw this in cpython test cases. + if property_deco is not None: + # Looks like inherited property + if len(property_deco)>2: + _inherited_property = _get_inherited_property(property_deco, parent) + if _inherited_property: + property_model = self._addProperty(node.name, parent, lineno) + # copy property defs info + property_model.getter = _inherited_property.getter + property_model.setter = _inherited_property.setter + property_model.deleter = _inherited_property.deleter + is_new_property = True + else: + # fetch property info to add this info to it _maybe_prop = self.builder.current.contents.get(node.name) if isinstance(_maybe_prop, model.Property): property_model = _maybe_prop - # We don't report warnings if we can't figure out the property model. elif is_property: # This is a new property definition diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index af419709a..9c29ebb52 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2079,10 +2079,9 @@ def data(self): assert mod.contents['A'].contents['_data'].kind is model.DocumentableKind.INSTANCE_VARIABLE @systemcls_param -def test_property_inherited_not_supported(systemcls: Type[model.System], capsys: CapSys) -> None: +def test_property_inherited(systemcls: Type[model.System], capsys: CapSys) -> None: """ - Properties can be inherited, but we don't support it for now :/ - Nevertheless, the test case exist for future developments. + Properties can be inherited. """ # source from cpython test_property.py src = ''' @@ -2113,37 +2112,71 @@ class SubClass2(BaseClass): def spam(self): """SubClass.setter""" pass + + class SubClass3(BaseClass): + # inherited property + @BaseClass.spam.deleter + def spam(self): + """SubClass.deleter""" + pass ''' mod = fromText(src, modname='mod', systemcls=systemcls) assert not capsys.readouterr().out spam0 = mod.contents['BaseClass'].contents['spam'] - spam1 = mod.contents['SubClass'].contents['spam.getter'] - spam2 = mod.contents['SubClass2'].contents['spam.setter'] + spam1 = mod.contents['SubClass'].contents['spam'] + spam2 = mod.contents['SubClass2'].contents['spam'] + spam3 = mod.contents['SubClass3'].contents['spam'] - assert list(mod.contents['BaseClass'].contents) == ['spam'] - assert list(mod.contents['SubClass'].contents) == ['spam.getter'] - assert list(mod.contents['SubClass2'].contents) == ['spam.setter'] + assert list(mod.contents['BaseClass'].contents) == \ + list(mod.contents['SubClass'].contents) == \ + list(mod.contents['SubClass2'].contents) == \ + list(mod.contents['SubClass3'].contents) == ['spam'] assert isinstance(spam0, model.Property) - assert isinstance(spam1, model.Function) - assert isinstance(spam2, model.Function) + assert isinstance(spam1, model.Property) + assert isinstance(spam2, model.Property) + assert isinstance(spam3, model.Property) assert spam0.kind is model.DocumentableKind.PROPERTY - assert spam1.kind is model.DocumentableKind.METHOD - assert spam2.kind is model.DocumentableKind.METHOD + assert spam1.kind is model.DocumentableKind.PROPERTY + assert spam2.kind is model.DocumentableKind.PROPERTY + assert spam3.kind is model.DocumentableKind.PROPERTY assert isinstance(spam0.setter, model.Function) + assert isinstance(spam1.setter, model.Function) + assert isinstance(spam2.setter, model.Function) + assert isinstance(spam3.setter, model.Function) assert isinstance(spam0.deleter, model.Function) + assert isinstance(spam1.deleter, model.Function) + assert isinstance(spam2.deleter, model.Function) + assert isinstance(spam3.deleter, model.Function) assert spam0.getter.fullName() == 'mod.BaseClass.spam.getter' #type:ignore[union-attr] + assert spam0.setter.fullName() == 'mod.BaseClass.spam.setter' + assert spam0.deleter.fullName() == 'mod.BaseClass.spam.deleter' + + assert spam1.getter.fullName() == 'mod.SubClass.spam.getter' #type:ignore[union-attr] + assert spam1.setter.fullName() == 'mod.BaseClass.spam.setter' + assert spam1.deleter.fullName() == 'mod.BaseClass.spam.deleter' + + assert spam2.getter.fullName() == 'mod.BaseClass.spam.getter' #type:ignore[union-attr] + assert spam2.setter.fullName() == 'mod.SubClass2.spam.setter' + assert spam2.deleter.fullName() == 'mod.BaseClass.spam.deleter' + + assert spam3.getter.fullName() == 'mod.BaseClass.spam.getter' #type:ignore[union-attr] + assert spam3.setter.fullName() == 'mod.BaseClass.spam.setter' + assert spam3.deleter.fullName() == 'mod.SubClass3.spam.deleter' + + assert spam1.getter is not spam0.getter @systemcls_param def test_property_old_school(systemcls: Type[model.System], capsys: CapSys) -> None: """ Old school property() decorator is recognized. """ + src = ''' class PropertyDocBase(object): _spam = 1 @@ -2151,49 +2184,34 @@ def _get_spam(self): return self._spam # Old school property spam = property(_get_spam, doc="spam spam spam") - + class PropertyDocSub(PropertyDocBase): + @PropertyDocBase.spam.getter + def spam(self): + # The docstring is ignored since the property is defined with old school manner + """The decorator does not use this doc string""" + return self._spam ''' - mod = fromText(src, modname='mod', systemcls=systemcls) - assert not capsys.readouterr().out - - spam = mod.resolveName('PropertyDocBase.spam') - assert isinstance(spam, model.Property) - assert spam.docstring == "spam spam spam" + mod = fromText(src, modname='t', systemcls=systemcls) + assert capsys.readouterr().out == ('') + s = mod.resolveName('PropertyDocBase.spam') + s2 = mod.resolveName('PropertyDocSub.spam') + assert isinstance(s, model.Property) + assert isinstance(s2, model.Property) @systemcls_param -def test_property_old_school_keywords(systemcls:Type[model.System], capsys:CapSys) -> None: +def test_property_old_school_doc_is_not_str_crash(systemcls: Type[model.System], capsys: CapSys) -> None: + """ + It does not crash when the doc argument is not a string. + """ src = ''' - class Circle: - def __init__(self, radius): - self._radius = radius - - def _get_radius(self): - # should not have docs here, - # since there is doc parameter to the property() call. - return self._radius - - def _set_radius(self, value): - "setter docs" - self._radius = value - - def _del_radius(self): - "deleter docs" - del self._radius - - radius = property( - fget=_get_radius, - fset=_set_radius, - fdel=_del_radius, - doc="The radius property." - ) + class PropertyDocBase(object): + def _get_spam(self):... + spam = property(_get_spam, doc=None) ''' - mod = fromText(src, modname='mod', systemcls=systemcls) - assert not capsys.readouterr().out - - spam = mod.resolveName('Circle.radius') - assert isinstance(spam, model.Property) - assert spam.docstring == "The radius property." - + mod = fromText(src, modname='t', systemcls=systemcls) + assert capsys.readouterr().out == ('') + s = mod.resolveName('PropertyDocBase.spam') + assert isinstance(s, model.Property) @systemcls_param def test_property_call_alone(systemcls:Type[model.System], capsys:CapSys) -> None: @@ -2273,11 +2291,9 @@ def _get_spam(self): ':6: Existing docstring at line 7 is overriden by docstring at line 6\n') @systemcls_param -def test_property_not_supported_or_corner_cases(systemcls: Type[model.System], capsys: CapSys) -> None: +def test_property_corner_cases(systemcls: Type[model.System], capsys: CapSys) -> None: """ Property handling can be quite complex, there are many corner cases. - But the general rule is that when a function ressemble a property function, - it's renamed such that it ends with .setter or .deleter. """ base_mod = ''' @@ -2380,21 +2396,21 @@ def notfound(self): mod = system.allobjects['mod'] src = system.allobjects['src'] - # Pydoctor doesn't understand this property because it's inherited. - # And plus it belong in a import cycle. So the older behaviour applies: - # only renaming the methods + # Pydoctor doesn't understand this property because it's using an + # import cycle. So the older behaviour applies: + # only renaming the method assert list(mod.contents['SubClass'].contents) == ['spam.getter'] assert mod.contents['SubClass'].contents['spam.getter'].kind is model.DocumentableKind.METHOD - assert list(src.contents['SubClass'].contents) == ['spam.getter'] - assert list(src.contents['NotSubClass'].contents) == ['spam.setter','spam.getter'] + assert list(src.contents['SubClass'].contents) == ['spam'] + assert list(src.contents['NotSubClass'].contents) == ['spam'] - spam0 = src.contents['SubClass'].contents['spam.getter'] - spam1 = src.contents['NotSubClass'].contents['spam.getter'] + spam0 = src.contents['SubClass'].contents['spam'] + spam1 = src.contents['NotSubClass'].contents['spam'] - assert list(src.contents['InvalidClass'].contents) == ['spam.setter', 'spam.getter'] + assert list(src.contents['InvalidClass'].contents) == ['spam', 'spam.getter'] - spam2 = src.contents['InvalidClass'].contents['spam.setter'] + spam2 = src.contents['InvalidClass'].contents['spam'] spam2_bogus = src.contents['InvalidClass'].contents['spam.getter'] notfound = src.contents['InvalidClass2'].contents['notfound.getter'] @@ -2403,9 +2419,9 @@ def notfound(self): notfound_alt = src.contents['InvalidClass4'].contents['notfound'] not_a_property = src.contents['InvalidClass5'].contents['notfound.getter'] - assert spam0.kind is model.DocumentableKind.METHOD - assert spam1.kind is model.DocumentableKind.METHOD - assert spam2.kind is model.DocumentableKind.METHOD + assert spam0.kind is model.DocumentableKind.PROPERTY + assert spam1.kind is model.DocumentableKind.PROPERTY + assert spam2.kind is model.DocumentableKind.PROPERTY assert spam2_bogus.kind is model.DocumentableKind.METHOD assert notfound.kind is model.DocumentableKind.METHOD assert notfound_both.kind is model.DocumentableKind.METHOD From af08b4b91a9f687a229b0e7d37782bd43944d84b Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 18 Jan 2024 14:37:35 -0500 Subject: [PATCH 44/47] Fix pyflakes --- pydoctor/templatewriter/pages/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/templatewriter/pages/__init__.py b/pydoctor/templatewriter/pages/__init__.py index a816ac8ee..66ed21c70 100644 --- a/pydoctor/templatewriter/pages/__init__.py +++ b/pydoctor/templatewriter/pages/__init__.py @@ -23,7 +23,7 @@ if TYPE_CHECKING: from typing_extensions import Final from twisted.web.template import Flattenable - from pydoctor.templatewriter.pages.attributechild import AttributeChild, PropertyChild + from pydoctor.templatewriter.pages.attributechild import AttributeChild from pydoctor.templatewriter.pages.functionchild import FunctionChild From 58c941bed8da5aa7ca96c08f9596a0d43d70e64f Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 18 Jan 2024 18:08:42 -0500 Subject: [PATCH 45/47] Minor refactors --- pydoctor/astbuilder.py | 61 +++++++++++++++++++++++++++--------------- 1 file changed, 39 insertions(+), 22 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index e98edd2f8..abb0fe046 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -205,21 +205,33 @@ def _get_inherited_property(dottedname:Sequence[str], parent: model.Documentable return attr_def -def _get_property_function_kind(dottedname:Sequence[str]) -> Optional[model.Property.Kind]: +def _get_property_function_kind(dottedname:Sequence[str]) -> model.Property.Kind: """ What kind of property function this decorator declares? None if we can't make sens of the decorator. - @note: the dottedname must have at least two elements. + Returns a L{Property.Kind} instance only if the given dotted name ends + with C{setter}, C{deleter} or C{getter} + + + @note: The dottedname must have at least two elements. """ if len(dottedname) >= 2: - if dottedname[-1] == 'setter': + last = dottedname[-1] + if last == 'setter': return model.Property.Kind.SETTER - if dottedname[-1] == 'getter': + if last == 'getter': return model.Property.Kind.GETTER - if dottedname[-1] == 'deleter': + if last == 'deleter': return model.Property.Kind.DELETER - return None + raise ValueError(f'This does not look like a property function decorator: {dottedname}') + +def _is_property_function(dottedname:Sequence[str]) -> bool: + try: + _get_property_function_kind(dottedname) + except ValueError: + return False + return True class ModuleVistor(NodeVisitor): @@ -906,21 +918,25 @@ def _handleFunctionDef(self, doc_node = get_docstring_node(node) func_name = node.name - # determine the function's kind - + # Whether the function is derorated with @classmethod is_classmethod = False + # Whether the function is derorated with @staticmethod is_staticmethod = False + # Whether the function is derorated with @typing.overload is_overload_func = False - + # Whether the function is derorated with @property is_property = False - property_deco: Optional[List[str]] = None - property_function_kind: Optional[model.Property.Kind] = None + # If the function is decorated with some @stuff.setter/deleter/getter property decorator: + # stores the dotted name of the expression. + property_func_deco: Optional[List[str]] = None + parent_is_cls = isinstance(parent, model.Class) if node.decorator_list: for deco_name, _ in astutils.iter_decorator_list(node.decorator_list): if deco_name is None: continue - if isinstance(parent, model.Class): + # determine the function's kind + if parent_is_cls: if _is_property_decorator(deco_name, parent): is_property = True elif deco_name == ['classmethod']: @@ -929,8 +945,7 @@ def _handleFunctionDef(self, is_staticmethod = True else: # Pre-handle property elements - property_function_kind = _get_property_function_kind(deco_name) - if property_function_kind: + if _is_property_function(deco_name): # Setters and deleters should have the same name as the property function, # otherwise ignore it. # This pollutes the namespace unnecessarily and is generally not recommended. @@ -942,7 +957,7 @@ def _handleFunctionDef(self, # Rename the setter/deleter, so it doesn't replace # the property object. func_name = '.'.join(deco_name[-2:]) - property_deco = deco_name + property_func_deco = deco_name # Determine if the function is decorated with overload if parent.expandName('.'.join(deco_name)) in ('typing.overload', @@ -950,13 +965,14 @@ def _handleFunctionDef(self, is_overload_func = True # Determine if this function is a property of some kind + property_func_kind: Optional[model.Property.Kind] = None property_model: Optional[model.Property] = None is_new_property: bool = is_property - if property_deco is not None: + if property_func_deco is not None: # Looks like inherited property - if len(property_deco)>2: - _inherited_property = _get_inherited_property(property_deco, parent) + if len(property_func_deco)>2: + _inherited_property = _get_inherited_property(property_func_deco, parent) if _inherited_property: property_model = self._addProperty(node.name, parent, lineno) # copy property defs info @@ -969,11 +985,12 @@ def _handleFunctionDef(self, _maybe_prop = self.builder.current.contents.get(node.name) if isinstance(_maybe_prop, model.Property): property_model = _maybe_prop + property_func_kind = _get_property_function_kind(property_func_deco) elif is_property: # This is a new property definition property_model = self._addProperty(node.name, parent, lineno) - property_function_kind = model.Property.Kind.GETTER + property_func_kind = model.Property.Kind.GETTER # Rename the getter function as well, since both the Property and the Function will # live side by side until properties are post-processed. func_name = node.name + '.getter' @@ -1075,6 +1092,7 @@ def add_arg(name: str, kind: Any, default: Optional[ast.expr]) -> None: else: func.signature = signature + # For properties, save the fact that this function implements one of the getter/setter/deleter if property_model is not None: if is_classmethod: @@ -1082,9 +1100,8 @@ def add_arg(name: str, kind: Any, default: Optional[ast.expr]) -> None: if is_staticmethod: property_model.report(f'{property_model.fullName()} is both property and staticmethod') - assert property_function_kind is not None - # Save the fact that this function implements one of the getter/setter/deleter - property_model.store_function(property_function_kind, func) + assert property_func_kind is not None + property_model.store_function(property_func_kind, func) def depart_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: From 1bb7bed7f1bf381095816d3dc8e9aecd1f421a5e Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Sun, 21 Jan 2024 13:01:52 -0500 Subject: [PATCH 46/47] Make sure we don't remove property functions when the property is created with the property() call. Simpler message when a docstring is getting overriden. --- pydoctor/astbuilder.py | 16 +++++- pydoctor/model.py | 94 ++++++++++++++++---------------- pydoctor/test/test_astbuilder.py | 54 ++++++++++++++---- 3 files changed, 105 insertions(+), 59 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index abb0fe046..1745968d4 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -570,10 +570,9 @@ def _handleOldSchoolPropertyDecoration(self, target: str, expr: ast.Call) -> boo model.Property.Kind.DELETER), ): definition = node2dottedname(bound_args.arguments.get(arg)) - if not len(definition or ())==1: + if not definition: continue - # the subscript indexing is safe since we've checked len(x)==1 - fn = cls.contents.get(definition[0]) # type:ignore[index] + fn = cls.resolveName('.'.join(definition)) if not isinstance(fn, model.Function): continue attr.store_function(prop_kind, fn) @@ -979,6 +978,17 @@ def _handleFunctionDef(self, property_model.getter = _inherited_property.getter property_model.setter = _inherited_property.setter property_model.deleter = _inherited_property.deleter + # Manually inherits documentation if not explicit in the definition. + if doc_node is None: + property_model.docstring = _inherited_property.docstring + # We can transfert the line numbers only if the two prperties are in the same module. + # This ignores reparenting, but it's ok because the reparenting will soon hapen in post-process. + if _inherited_property.module is property_model.module: + property_model.docstring_lineno = _inherited_property.docstring_lineno + else: + # TODO: Wrap the docstring info into a new class Docstring. + # so we can transfert the filename information as well. + property_model.docstring_lineno = -1 is_new_property = True else: # fetch property info to add this info to it diff --git a/pydoctor/model.py b/pydoctor/model.py index b90ac220d..3cd600809 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -165,8 +165,8 @@ def _setDocstringValue(self, doc:str, lineno:int) -> None: msg = 'Existing docstring' if self.docstring_lineno: msg += f' at line {self.docstring_lineno}' - msg += f' is overriden by docstring at line {lineno}' - self.report(msg) + msg += f' is overriden' + self.report(msg, 'docstring', lineno_offset=lineno-self.docstring_lineno) self.docstring = doc self.docstring_lineno = lineno @@ -865,57 +865,59 @@ def init_property(attrib:'Property') -> Iterator['Function']: Returns the functions to remove from the tree. """ + # avoid cyclic import + from pydoctor import epydoc2stan + getter = attrib.getter setter = attrib.setter deleter = attrib.deleter - if getter is None: + if getter is not None: # The getter should never be None, # but it can execpitinally happend when - # one uses the property() call alone. - return () - - # avoid cyclic import - from pydoctor import epydoc2stan - - # Setup Attribute object for the property - if getter.docstring: - attrib._setDocstringValue(getter.docstring, - getter.docstring_lineno) - if not attrib.annotation: - attrib.annotation = getter.annotations.get('return') - attrib.extra_info.extend(getter.extra_info) - - # Parse docstring now. - if epydoc2stan.ensure_parsed_docstring(getter): - - parsed_doc = getter.parsed_docstring - assert parsed_doc is not None - - other_fields = [] - # process fields such that :returns: clause docs takes the whole docs - # if no global description is written. - for field in parsed_doc.fields: - tag = field.tag() - if tag == 'return': - if not parsed_doc.has_body: - parsed_doc = field.body() - elif tag == 'rtype': - attrib.parsed_type = field.body() - else: - other_fields.append(field) - - parsed_doc.fields = other_fields + # one uses the property() call alone and dynamically sets the getter. + + # Setup Attribute object for the property + if getter.docstring: + attrib._setDocstringValue(getter.docstring, + getter.docstring_lineno) + if not attrib.annotation: + attrib.annotation = getter.annotations.get('return') + attrib.extra_info.extend(getter.extra_info) + + # Parse docstring now. + if epydoc2stan.ensure_parsed_docstring(getter): - # Set the new attribute parsed docstring - attrib.parsed_docstring = parsed_doc - - # Yields the objects to remove from the Documentable tree - yield getter - if setter: - yield setter - if deleter: - yield deleter + parsed_doc = getter.parsed_docstring + assert parsed_doc is not None + + other_fields = [] + # process fields such that :returns: clause docs takes the whole docs + # if no global description is written. + for field in parsed_doc.fields: + tag = field.tag() + if tag == 'return': + if not parsed_doc.has_body: + parsed_doc = field.body() + elif tag == 'rtype': + attrib.parsed_type = field.body() + else: + other_fields.append(field) + + parsed_doc.fields = other_fields + + # Set the new attribute parsed docstring + attrib.parsed_docstring = parsed_doc + + # Yields the objects to remove from the Documentable tree. + # Ensures we delete only the function decorated with @stuff.getter/setter/deleter; + # We know these functions will be renamed with the function kind suffix. + for fn, expected_name in zip([getter, setter, deleter], + [f'{attrib.name}.getter', + f'{attrib.name}.setter', + f'{attrib.name}.deleter']): + if fn and fn.name == expected_name and fn.parent is attrib.parent: + yield fn class Attribute(Inheritable): diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 9c29ebb52..68228d7f4 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2176,27 +2176,58 @@ def test_property_old_school(systemcls: Type[model.System], capsys: CapSys) -> N """ Old school property() decorator is recognized. """ - + src0 = ''' + def get_spam2(self): + ... + class PropertyDocBase0(object): + @property + def spam3(self): + "spam3 docs" + ''' src = ''' + import t0 class PropertyDocBase(object): _spam = 1 - def _get_spam(self): + def get_spam(self): return self._spam # Old school property - spam = property(_get_spam, doc="spam spam spam") + spam = property(get_spam, doc="spam spam spam") + spam2 = property(t0.get_spam2, doc="spam2 spam2 spam2") class PropertyDocSub(PropertyDocBase): @PropertyDocBase.spam.getter def spam(self): - # The docstring is ignored since the property is defined with old school manner - """The decorator does not use this doc string""" + "This docstring overrides the other one in the property() call" return self._spam + @t0.PropertyDocBase0.spam3.getter + def spam3(self): + "This docstring overrides the other one in module t0" + ''' - mod = fromText(src, modname='t', systemcls=systemcls) - assert capsys.readouterr().out == ('') + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString(src0, modname='t0') + builder.addModuleString(src, modname='t') + builder.buildModules() + + mod = system.allobjects['t'] s = mod.resolveName('PropertyDocBase.spam') s2 = mod.resolveName('PropertyDocSub.spam') assert isinstance(s, model.Property) assert isinstance(s2, model.Property) + # The get_spam() function has not been removed form the tree. + assert mod.resolveName('PropertyDocSub.get_spam') + assert s.docstring == "spam spam spam" + assert s2.docstring == "This docstring overrides the other one in the property() call" + spam2 = mod.resolveName('PropertyDocBase.spam2') + assert isinstance(spam2, model.Property) + assert spam2.getter + assert spam2.getter.fullName() == 't0.get_spam2' + + spam3 = mod.resolveName('PropertyDocSub.spam3') + assert spam3.docstring == "This docstring overrides the other one in module t0" + + assert system.allobjects['t0.get_spam2'] + assert capsys.readouterr().out == ('') @systemcls_param def test_property_old_school_doc_is_not_str_crash(systemcls: Type[model.System], capsys: CapSys) -> None: @@ -2263,7 +2294,7 @@ def spam(self): return 8 ''' mod = fromText(src, modname='mod', systemcls=systemcls) - assert capsys.readouterr().out == 'mod:4: Existing docstring at line 6 is overriden by docstring at line 11\n' + assert capsys.readouterr().out == 'mod:11: Existing docstring at line 6 is overriden\n' attr = mod.contents['PropertyNewGetter'].contents['spam'] # the parsed_docstring attribute gets initiated in post-processing assert node2stan.gettext(attr.parsed_docstring.to_node()) == ['new docstring'] # type:ignore @@ -2282,13 +2313,16 @@ def _get_spam(self): # Old school property spam = property(_get_spam, doc="Second doc") "Third doc" + spam2 = property(fset=None, doc='spam2 docs') + "ignored" ''' mod = fromText(src, systemcls=systemcls) spam = mod.resolveName('PropertyOld.spam') assert isinstance(spam, model.Property) spam.docstring == "Second doc" - assert capsys.readouterr().out == (':3: Existing docstring at line 4 is overriden by docstring at line 6\n' - ':6: Existing docstring at line 7 is overriden by docstring at line 6\n') + assert capsys.readouterr().out == (':6: Existing docstring at line 4 is overriden\n' + ':9: Existing docstring at line 8 is overriden\n' + ':6: Existing docstring at line 7 is overriden\n') @systemcls_param def test_property_corner_cases(systemcls: Type[model.System], capsys: CapSys) -> None: From dec3483cff1c2eb6120de3146aa5edaed8fff789 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 21 Jun 2024 11:56:21 -0400 Subject: [PATCH 47/47] Factor out some logic of the function def handler that is becoming too large. --- pydoctor/astbuilder.py | 178 ++++++++++++++++++++++++----------------- 1 file changed, 104 insertions(+), 74 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 97b91c618..cc1268574 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -181,7 +181,7 @@ def _is_property_decorator(dottedname:Sequence[str], ctx:model.Documentable) -> return False -def _get_inherited_property(dottedname:Sequence[str], parent: model.Documentable) -> Optional[model.Property]: +def _fetch_property(deconame:Sequence[str], parent: model.Documentable) -> Optional[model.Property]: """ Fetch the inherited property that this new decorator overrides. Returns C{None} if it doesn't exist in the inherited members or if it's already definied in the locals. @@ -189,7 +189,7 @@ def _get_inherited_property(dottedname:Sequence[str], parent: model.Documentable """ # TODO: It would be best if this job was done in post-processing... - property_name = dottedname[:-1] + property_name = deconame[:-1] if len(property_name) <= 1 or property_name[-1] in parent.contents: # the property already exist @@ -919,88 +919,38 @@ def _handleFunctionDef(self, doc_node = get_docstring_node(node) func_name = node.name - # Whether the function is derorated with @classmethod - is_classmethod = False - # Whether the function is derorated with @staticmethod - is_staticmethod = False - # Whether the function is derorated with @typing.overload - is_overload_func = False - # Whether the function is derorated with @property - is_property = False - # If the function is decorated with some @stuff.setter/deleter/getter property decorator: - # stores the dotted name of the expression. - property_func_deco: Optional[List[str]] = None - - parent_is_cls = isinstance(parent, model.Class) - if node.decorator_list: - for deco_name, _ in astutils.iter_decorator_list(node.decorator_list): - if deco_name is None: - continue - # determine the function's kind - if parent_is_cls: - if _is_property_decorator(deco_name, parent): - is_property = True - elif deco_name == ['classmethod']: - is_classmethod = True - elif deco_name == ['staticmethod']: - is_staticmethod = True - else: - # Pre-handle property elements - if _is_property_function(deco_name): - # Setters and deleters should have the same name as the property function, - # otherwise ignore it. - # This pollutes the namespace unnecessarily and is generally not recommended. - # Therefore it makes sense to stick to a single name, - # which is consistent with the former property definition. - if not deco_name[-2] == func_name: - continue - - # Rename the setter/deleter, so it doesn't replace - # the property object. - func_name = '.'.join(deco_name[-2:]) - property_func_deco = deco_name - - # Determine if the function is decorated with overload - if parent.expandName('.'.join(deco_name)) in ('typing.overload', - 'typing_extensions.overload'): - is_overload_func = True + (is_classmethod, is_staticmethod, + is_overload_func, is_property, + property_deconame) = self._getFunctionKinds(node, parent) + + if property_deconame: + # Rename the setter/deleter, so it doesn't replace + # the property object. + func_name = '.'.join(property_deconame[-2:]) - # Determine if this function is a property of some kind + # Determine if this function is a property of some kind and process it property_func_kind: Optional[model.Property.Kind] = None property_model: Optional[model.Property] = None is_new_property: bool = is_property - if property_func_deco is not None: - # Looks like inherited property - if len(property_func_deco)>2: - _inherited_property = _get_inherited_property(property_func_deco, parent) - if _inherited_property: - property_model = self._addProperty(node.name, parent, lineno) - # copy property defs info - property_model.getter = _inherited_property.getter - property_model.setter = _inherited_property.setter - property_model.deleter = _inherited_property.deleter - # Manually inherits documentation if not explicit in the definition. - if doc_node is None: - property_model.docstring = _inherited_property.docstring - # We can transfert the line numbers only if the two prperties are in the same module. - # This ignores reparenting, but it's ok because the reparenting will soon hapen in post-process. - if _inherited_property.module is property_model.module: - property_model.docstring_lineno = _inherited_property.docstring_lineno - else: - # TODO: Wrap the docstring info into a new class Docstring. - # so we can transfert the filename information as well. - property_model.docstring_lineno = -1 + if property_deconame is not None: + # Process property @name.setter/deleter/getter decorated function + if len(property_deconame)>2: + # Looks like inherited property + base_property = _fetch_property(property_deconame, parent) + if base_property: + property_model = self._addInheritedProperty(base_property, node.name, parent, lineno, + copy_docstring=doc_node is None) is_new_property = True else: # fetch property info to add this info to it - _maybe_prop = self.builder.current.contents.get(node.name) - if isinstance(_maybe_prop, model.Property): - property_model = _maybe_prop - property_func_kind = _get_property_function_kind(property_func_deco) + maybe_property = self.builder.current.contents.get(node.name) + if isinstance(maybe_property, model.Property): + property_model = maybe_property + property_func_kind = _get_property_function_kind(property_deconame) elif is_property: - # This is a new property definition + # This is a new @property definition property_model = self._addProperty(node.name, parent, lineno) property_func_kind = model.Property.Kind.GETTER # Rename the getter function as well, since both the Property and the Function will @@ -1023,6 +973,7 @@ def _handleFunctionDef(self, # Do not recreate function object, just re-push it self.builder.push(existing_func, lineno) func = existing_func + elif isinstance(existing_func, model.Function) and property_model is not None and not is_new_property: # Check if this property function is overriding a previously defined # property function on the same scope before pushing the new function @@ -1114,6 +1065,85 @@ def add_arg(name: str, kind: Any, default: Optional[ast.expr]) -> None: assert property_func_kind is not None property_model.store_function(property_func_kind, func) + + def _getFunctionKinds(self, node: ast.FunctionDef | ast.AsyncFunctionDef, + parent: model.Documentable) -> tuple[bool, bool, bool, bool, List[str] | None]: + """ + Returns a tuple with the following values: + + - Whether the function is derorated with @classmethod + - Whether the function is derorated with @staticmethod + - Whether the function is derorated with @typing.overload + - Whether the function is derorated with @property + - The property decorator name as list of string if the function + is decorated with some @stuff.setter/deleter/getter otherwise None. + """ + is_classmethod = False + is_staticmethod = False + is_overload_func = False + is_property = False + property_deconame: List[str] | None = None + + parent_is_cls = isinstance(parent, model.Class) + if node.decorator_list: + for deco_name, _ in astutils.iter_decorator_list(node.decorator_list): + if deco_name is None: + continue + # determine the function's kind + if parent_is_cls: + if _is_property_decorator(deco_name, parent): + is_property = True + elif deco_name == ['classmethod']: + is_classmethod = True + elif deco_name == ['staticmethod']: + is_staticmethod = True + else: + # Pre-handle property elements + if _is_property_function(deco_name): + # Setters and deleters should have the same name as the property function, + # otherwise ignore it. + # This pollutes the namespace unnecessarily and is generally not recommended. + # Therefore it makes sense to stick to a single name, + # which is consistent with the former property definition. + if not deco_name[-2] == node.name: + continue + + property_deconame = deco_name + + # Determine if the function is decorated with overload + if parent.expandName('.'.join(deco_name)) in ('typing.overload', + 'typing_extensions.overload'): + is_overload_func = True + + return (is_classmethod, is_staticmethod, + is_overload_func, is_property, + property_deconame) + + def _addInheritedProperty(self, + base_property: model.Property, + name: str, + parent: model.Documentable, + lineno: int, + copy_docstring: bool, ): + property_model = self._addProperty(name, parent, lineno) + # copy property defs info + property_model.getter = base_property.getter + property_model.setter = base_property.setter + property_model.deleter = base_property.deleter + + # Manually inherits documentation if not explicit in the definition. + if copy_docstring: + property_model.docstring = base_property.docstring + # We can transfert the line numbers only if the two properties are in the same module. + # This ignores reparenting, but it's ok because the reparenting will soon hapen in post-process. + if base_property.module is property_model.module: + property_model.docstring_lineno = base_property.docstring_lineno + else: + # TODO: Wrap the docstring info into a new class Docstring. + # so we can transfert the filename information as well. + pass + + return property_model def depart_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: