From 96b9e0525933f27f26e831e909666a45a9c9b54f Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Tue, 11 Jul 2023 00:42:35 -0400 Subject: [PATCH 01/23] Fix #295 --- pydoctor/astbuilder.py | 3 ++ pydoctor/epydoc2stan.py | 87 ++++++++++++++++++++++++++++++- pydoctor/linker.py | 22 ++++++-- pydoctor/model.py | 23 ++++++-- pydoctor/node2stan.py | 39 ++++++++------ pydoctor/test/test_epydoc2stan.py | 78 +++++++++++++++++++++++++++ 6 files changed, 227 insertions(+), 25 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index e1fc18004..a9199c922 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -176,6 +176,7 @@ def visit_Module(self, node: ast.Module) -> None: def depart_Module(self, node: ast.Module) -> None: self.builder.pop(self.module) + epydoc2stan.transform_parsed_names(self.module) def visit_ClassDef(self, node: ast.ClassDef) -> None: # Ignore classes within functions. @@ -1072,6 +1073,8 @@ def _annotation_for_elements(sequence: Iterable[object]) -> Optional[ast.expr]: class ASTBuilder: """ Keeps tracks of the state of the AST build, creates documentable and adds objects to the system. + + One ASTBuilder instance is only suitable to build one Module. """ ModuleVistor = ModuleVistor diff --git a/pydoctor/epydoc2stan.py b/pydoctor/epydoc2stan.py index fa34e94be..cfb4457fc 100644 --- a/pydoctor/epydoc2stan.py +++ b/pydoctor/epydoc2stan.py @@ -4,6 +4,8 @@ from collections import defaultdict import enum +import inspect +from itertools import chain from typing import ( TYPE_CHECKING, Any, Callable, ClassVar, DefaultDict, Dict, Generator, Iterator, List, Mapping, Optional, Sequence, Tuple, Union, @@ -12,8 +14,11 @@ import re import attr +from docutils.transforms import Transform +from docutils import nodes from pydoctor import model, linker, node2stan +from pydoctor.node2stan import parse_reference from pydoctor.astutils import is_none_literal from pydoctor.epydoc.markup import Field as EpydocField, ParseError, get_parser_by_name, processtypes from twisted.web.template import Tag, tags @@ -884,7 +889,10 @@ def get_parsed_type(obj: model.Documentable) -> Optional[ParsedDocstring]: # Only Attribute instances have the 'annotation' attribute. annotation: Optional[ast.expr] = getattr(obj, 'annotation', None) if annotation is not None: - return colorize_inline_pyval(annotation) + parsed_type = colorize_inline_pyval(annotation) + # cache parsed_type attribute + obj.parsed_type = parsed_type + return parsed_type return None @@ -1143,3 +1151,80 @@ def populate_constructors_extra_info(cls:model.Class) -> None: extra_epytext += '`%s <%s>`' % (short_text, c.fullName()) cls.extra_info.append(parse_docstring(cls, extra_epytext, cls, 'restructuredtext', section='constructor extra')) + +class _ReferenceTransform(Transform): + + def __init__(self, document:nodes.document, ctx:'model.Documentable'): + super().__init__(document) + self.ctx = ctx + + def apply(self): + ctx = self.ctx + module = self.ctx.module + for node in self.document.findall(nodes.title_reference): + _, target = parse_reference(node) + if target == node.attributes.get('refuri', target): + name, *rest = target.split('.') + # Only apply transformation to non-ambigous names, + # because we don't know if we're dealing with an annotation + # or an interpreted, so we must go with the conservative approach. + if ((module.isNameDefined(name) and + not ctx.isNameDefined(name, only_locals=True)) + or (ctx.isNameDefined(name, only_locals=True) and + not module.isNameDefined(name))): + + node.attributes['refuri'] = '.'.join(chain( + ctx._localNameToFullName(name).split('.'), rest)) + +def _apply_reference_transform(doc:ParsedDocstring, ctx:'model.Documentable') -> None: + """ + Runs L{_ReferenceTransform} on the underlying docutils document. + No-op if L{to_node} raises L{NotImplementedError}. + """ + try: + document = doc.to_node() + except NotImplementedError: + return + else: + _ReferenceTransform(document, ctx).apply() + +def transform_parsed_names(node:'model.Module') -> None: + """ + Walk this module's content and apply in-place transformations to the + L{ParsedDocstring} instances that olds L{obj_reference} or L{title_reference} nodes. + + Fixing "Lookup of name in annotation fails on reparented object #295". + The fix is not 100% complete at the moment: attribute values and decorators + are not handled. + """ + from pydoctor import model, astbuilder + # resolve names early when possible + for ob in model.walk(node): + # resolve names in parsed_docstring, do not forget field bodies + if ob.parsed_docstring: + _apply_reference_transform(ob.parsed_docstring, ob) + for f in ob.parsed_docstring.fields: + _apply_reference_transform(f.body(), ob) + if isinstance(ob, model.Function): + if ob.signature: + for p in ob.signature.parameters.values(): + ann = p.annotation if p.annotation is not inspect.Parameter.empty else None + if isinstance(ann, astbuilder._ValueFormatter): + _apply_reference_transform(ann._colorized, ob) + default = p.default if p.default is not inspect.Parameter.empty else None + if isinstance(default, astbuilder._ValueFormatter): + _apply_reference_transform(default._colorized, ob) + # TODO: resolve function's annotations, they are currently presented twice + # we can only change signature, annotations in param table must be handled by + # introducing attribute parsed_annotations + elif isinstance(ob, model.Attribute): + # resolve attribute annotation with parsed_type attribute + parsed_type = get_parsed_type(ob) + if parsed_type: + _apply_reference_transform(parsed_type, ob) + # TODO: resolve parsed_value + # TODO: resolve parsed_decorators + elif isinstance(ob, model.Class): + # TODO: resolve parsed_class_signature + # TODO: resolve parsed_decorators + pass diff --git a/pydoctor/linker.py b/pydoctor/linker.py index c2430a4b2..8f0814591 100644 --- a/pydoctor/linker.py +++ b/pydoctor/linker.py @@ -133,9 +133,13 @@ def look_for_intersphinx(self, name: str) -> Optional[str]: def link_to(self, identifier: str, label: "Flattenable") -> Tag: fullID = self.obj.expandName(identifier) - target = self.obj.system.objForFullName(fullID) - if target is not None: - return taglink(target, self.page_url, label) + try: + target = self.obj.system.find_object(fullID) + except LookupError: + pass + else: + if target is not None: + return taglink(target, self.page_url, label) url = self.look_for_intersphinx(fullID) if url is not None: @@ -184,8 +188,18 @@ def _resolve_identifier_xref(self, if target is not None: return target - # Check if the fullID exists in an intersphinx inventory. fullID = self.obj.expandName(identifier) + + # Try fetching the name with it's outdated fullname + try: + target = self.obj.system.find_object(fullID) + except LookupError: + pass + else: + if target is not None: + return target + + # Check if the fullID exists in an intersphinx inventory. target_url = self.look_for_intersphinx(fullID) if not target_url: # FIXME: https://github.com/twisted/pydoctor/issues/125 diff --git a/pydoctor/model.py b/pydoctor/model.py index 8d0d6c2ba..be84dda66 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -111,6 +111,19 @@ class DocumentableKind(Enum): PROPERTY = 150 VARIABLE = 100 +def walk(node:'Documentable') -> Iterator['Documentable']: + """ + Recursively yield all descendant nodes in the tree starting at *node* + (including *node* itself), in no specified order. This is useful if you + only want to modify nodes in place and don't care about the context. + """ + from collections import deque + todo = deque([node]) + while todo: + node = todo.popleft() + todo.extend(node.contents.values()) + yield node + class Documentable: """An object that can be documented. @@ -271,7 +284,7 @@ def _handle_reparenting_post(self) -> None: def _localNameToFullName(self, name: str) -> str: raise NotImplementedError(self._localNameToFullName) - def isNameDefined(self, name:str) -> bool: + def isNameDefined(self, name:str, only_locals:bool=False) -> bool: """ Is the given name defined in the globals/locals of self-context? Only the first name of a dotted name is checked. @@ -411,13 +424,13 @@ def setup(self) -> None: super().setup() self._localNameToFullName_map: Dict[str, str] = {} - def isNameDefined(self, name: str) -> bool: + def isNameDefined(self, name: str, only_locals:bool=False) -> bool: name = name.split('.')[0] if name in self.contents: return True if name in self._localNameToFullName_map: return True - if not isinstance(self, Module): + if not isinstance(self, Module) and not only_locals: return self.module.isNameDefined(name) else: return False @@ -811,8 +824,8 @@ def docsources(self) -> Iterator[Documentable]: def _localNameToFullName(self, name: str) -> str: return self.parent._localNameToFullName(name) - def isNameDefined(self, name: str) -> bool: - return self.parent.isNameDefined(name) + def isNameDefined(self, name: str, only_locals:bool=False) -> bool: + return self.parent.isNameDefined(name, only_locals=only_locals) class Function(Inheritable): kind = DocumentableKind.FUNCTION diff --git a/pydoctor/node2stan.py b/pydoctor/node2stan.py index 6acab4e91..cb9fbee58 100644 --- a/pydoctor/node2stan.py +++ b/pydoctor/node2stan.py @@ -3,7 +3,7 @@ """ import re import optparse -from typing import Any, Callable, ClassVar, Iterable, List, Optional, Union, TYPE_CHECKING +from typing import Any, Callable, ClassVar, Iterable, List, Optional, Sequence, Tuple, Union, TYPE_CHECKING from docutils.writers import html4css1 from docutils import nodes, frontend, __version_info__ as docutils_version_info @@ -52,6 +52,25 @@ def gettext(node: Union[nodes.Node, List[nodes.Node]]) -> List[str]: filtered.extend(gettext(child)) return filtered +def parse_reference(node:nodes.title_reference) -> Tuple[Union[str, Sequence[nodes.Node]], str]: + """ + Split a reference into (label, target). + """ + label: "Flattenable" + if 'refuri' in node.attributes: + # Epytext parsed or manually constructed nodes. + label, target = node.children, node.attributes['refuri'] + else: + # RST parsed. + m = _TARGET_RE.match(node.astext()) + if m: + label, target = m.groups() + else: + label = target = node.astext() + # Support linking to functions and methods with () at the end + if target.endswith('()'): + target = target[:len(target)-2] + return label, target _TARGET_RE = re.compile(r'^(.*?)\s*<(?:URI:|URL:)?([^<>]+)>$') _VALID_IDENTIFIER_RE = re.compile('[^0-9a-zA-Z_]') @@ -105,22 +124,12 @@ def visit_obj_reference(self, node: nodes.Node) -> None: self._handle_reference(node, link_func=self._linker.link_to) def _handle_reference(self, node: nodes.Node, link_func: Callable[[str, "Flattenable"], "Flattenable"]) -> None: + node_label, target = parse_reference(node) label: "Flattenable" - if 'refuri' in node.attributes: - # Epytext parsed or manually constructed nodes. - label, target = node2stan(node.children, self._linker), node.attributes['refuri'] + if not isinstance(node_label, str): + label = node2stan(node_label, self._linker) else: - # RST parsed. - m = _TARGET_RE.match(node.astext()) - if m: - label, target = m.groups() - else: - label = target = node.astext() - - # Support linking to functions and methods with () at the end - if target.endswith('()'): - target = target[:len(target)-2] - + label = node_label self.body.append(flatten(link_func(target, label))) raise nodes.SkipNode() diff --git a/pydoctor/test/test_epydoc2stan.py b/pydoctor/test/test_epydoc2stan.py index 4d4a8b521..80d0dd238 100644 --- a/pydoctor/test/test_epydoc2stan.py +++ b/pydoctor/test/test_epydoc2stan.py @@ -2106,3 +2106,81 @@ def func(): assert docstring2html(mod.contents['func'], docformat='plaintext') == expected captured = capsys.readouterr().out assert captured == '' + +def test_parsed_names_partially_resolved_early() -> None: + """ + Test for issue #295 + + Annotations are first locally resolved when we reach the end of the module, + then again when we actually resolve the name when generating the stan for the annotation. + """ + typing = '''\ + List = ClassVar = TypeVar = object() + ''' + + base = '''\ + import ast + class Vis(ast.NodeVisitor): + ... + ''' + src = '''\ + from typing import List + import typing as t + + from .base import Vis + + class Cls(Vis, t.Generic['_T']): + """ + L{Cls} + """ + clsvar:List[str] + clsvar2:t.ClassVar[List[str]] + + def __init__(self, a:'_T'): + self._a:'_T' = a + + C = Cls + _T = t.TypeVar('_T') + unknow: i|None|list + ann:Cls + ''' + + top = '''\ + # the order matters here + from .src import C, Cls, Vis + __all__ = ['Cls', 'C', 'Vis'] + ''' + + system = model.System() + builder = system.systemBuilder(system) + builder.addModuleString(top, 'top', is_package=True) + builder.addModuleString(base, 'base', 'top') + builder.addModuleString(src, 'src', 'top') + builder.addModuleString(typing, 'typing') + builder.buildModules() + + Cls = system.allobjects['top.Cls'] + clsvar = Cls.contents['clsvar'] + clsvar2 = Cls.contents['clsvar2'] + a = Cls.contents['_a'] + assert clsvar.expandName('typing.List')=='typing.List' + assert '' in clsvar.parsed_type.to_node().pformat() + assert 'href="typing.html#List"' in flatten(clsvar.parsed_type.to_stan(clsvar.docstring_linker)) + assert 'href="typing.html#ClassVar"' in flatten(clsvar2.parsed_type.to_stan(clsvar2.docstring_linker)) + assert 'href="top.src.html#_T"' in flatten(a.parsed_type.to_stan(clsvar.docstring_linker)) + + # the reparenting/alias issue + ann = system.allobjects['top.src.ann'] + assert 'href="top.Cls.html"' in flatten(ann.parsed_type.to_stan(ann.docstring_linker)) + assert 'href="top.Cls.html"' in flatten(Cls.parsed_docstring.to_stan(Cls.docstring_linker)) + + unknow = system.allobjects['top.src.unknow'] + assert flatten_text(unknow.parsed_type.to_stan(unknow.docstring_linker)) == 'i|None|list' + + + + # TODO: test the __init__ signature and the class bases + + # TODO: Fix two new twisted warnings: + # twisted/internet/_sslverify.py:330: Cannot find link target for "twisted.internet.ssl.DN", resolved from "twisted.internet._sslverify.DistinguishedName" + # twisted/internet/_sslverify.py:347: Cannot find link target for "twisted.internet.ssl.DN", resolved from "twisted.internet._sslverify.DistinguishedName" \ No newline at end of file From 6550919743a2fb0ed08bd0ac99919da9cd379894 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Tue, 11 Jul 2023 17:40:37 -0400 Subject: [PATCH 02/23] Fix the _ReferenceTransform for annotations. Also enamble msg(once=True) when using report(). --- pydoctor/epydoc2stan.py | 37 ++++++++++++++++---------- pydoctor/linker.py | 26 ++++++++++--------- pydoctor/model.py | 6 +++-- pydoctor/test/test_epydoc2stan.py | 43 +++++++++++++++++++------------ 4 files changed, 67 insertions(+), 45 deletions(-) diff --git a/pydoctor/epydoc2stan.py b/pydoctor/epydoc2stan.py index cfb4457fc..3f9141265 100644 --- a/pydoctor/epydoc2stan.py +++ b/pydoctor/epydoc2stan.py @@ -1154,9 +1154,11 @@ def populate_constructors_extra_info(cls:model.Class) -> None: class _ReferenceTransform(Transform): - def __init__(self, document:nodes.document, ctx:'model.Documentable'): + def __init__(self, document:nodes.document, + ctx:'model.Documentable', is_annotation:bool): super().__init__(document) self.ctx = ctx + self.is_annotation = is_annotation def apply(self): ctx = self.ctx @@ -1165,18 +1167,25 @@ def apply(self): _, target = parse_reference(node) if target == node.attributes.get('refuri', target): name, *rest = target.split('.') - # Only apply transformation to non-ambigous names, - # because we don't know if we're dealing with an annotation - # or an interpreted, so we must go with the conservative approach. - if ((module.isNameDefined(name) and - not ctx.isNameDefined(name, only_locals=True)) - or (ctx.isNameDefined(name, only_locals=True) and - not module.isNameDefined(name))): + + # kindda duplicate a little part of the annotation linker logic here, + # there are no simple way of doing it otherwise at the moment. + # Once all presented parsed elements are stored as Documentable attributes + # we might be able to simply use that and drop the use of the annotation linker, + # but for now this will to the trick: + lookup_context = ctx + if self.is_annotation and ctx is not module and module.isNameDefined(name, + only_locals=True) and ctx.isNameDefined(name, only_locals=True): + # If we're dealing with an annotation, give precedence to the module's + # lookup (wrt PEP 563) + lookup_context = module + linker.warn_ambiguous_annotation(module, ctx, target) - node.attributes['refuri'] = '.'.join(chain( - ctx._localNameToFullName(name).split('.'), rest)) + node.attributes['refuri'] = '.'.join(chain( + lookup_context._localNameToFullName(name).split('.'), rest)) -def _apply_reference_transform(doc:ParsedDocstring, ctx:'model.Documentable') -> None: +def _apply_reference_transform(doc:ParsedDocstring, ctx:'model.Documentable', + is_annotation:bool=False) -> None: """ Runs L{_ReferenceTransform} on the underlying docutils document. No-op if L{to_node} raises L{NotImplementedError}. @@ -1186,7 +1195,7 @@ def _apply_reference_transform(doc:ParsedDocstring, ctx:'model.Documentable') -> except NotImplementedError: return else: - _ReferenceTransform(document, ctx).apply() + _ReferenceTransform(document, ctx, is_annotation).apply() def transform_parsed_names(node:'model.Module') -> None: """ @@ -1210,7 +1219,7 @@ def transform_parsed_names(node:'model.Module') -> None: for p in ob.signature.parameters.values(): ann = p.annotation if p.annotation is not inspect.Parameter.empty else None if isinstance(ann, astbuilder._ValueFormatter): - _apply_reference_transform(ann._colorized, ob) + _apply_reference_transform(ann._colorized, ob, is_annotation=True) default = p.default if p.default is not inspect.Parameter.empty else None if isinstance(default, astbuilder._ValueFormatter): _apply_reference_transform(default._colorized, ob) @@ -1221,7 +1230,7 @@ def transform_parsed_names(node:'model.Module') -> None: # resolve attribute annotation with parsed_type attribute parsed_type = get_parsed_type(ob) if parsed_type: - _apply_reference_transform(parsed_type, ob) + _apply_reference_transform(parsed_type, ob, is_annotation=True) # TODO: resolve parsed_value # TODO: resolve parsed_decorators elif isinstance(ob, model.Class): diff --git a/pydoctor/linker.py b/pydoctor/linker.py index 8f0814591..95162f005 100644 --- a/pydoctor/linker.py +++ b/pydoctor/linker.py @@ -252,6 +252,19 @@ def _resolve_identifier_xref(self, self.reporting_obj.report(message, 'resolve_identifier_xref', lineno) raise LookupError(identifier) +def warn_ambiguous_annotation(mod:'model.Documentable', + obj:'model.Documentable', + target:str) -> None: + # report a low-level message about ambiguous annotation + mod_ann = mod.expandName(target) + obj_ann = obj.expandName(target) + if mod_ann != obj_ann: + obj.report( + f'ambiguous annotation {target!r}, could be interpreted as ' + f'{obj_ann!r} instead of {mod_ann!r}', section='annotation', + thresh=1 + ) + class _AnnotationLinker(DocstringLinker): """ Specialized linker to resolve annotations attached to the given L{Documentable}. @@ -271,21 +284,10 @@ def __init__(self, obj:'model.Documentable') -> None: @property def obj(self) -> 'model.Documentable': return self._obj - - def warn_ambiguous_annotation(self, target:str) -> None: - # report a low-level message about ambiguous annotation - mod_ann = self._module.expandName(target) - obj_ann = self._scope.expandName(target) - if mod_ann != obj_ann: - self.obj.report( - f'ambiguous annotation {target!r}, could be interpreted as ' - f'{obj_ann!r} instead of {mod_ann!r}', section='annotation', - thresh=1 - ) def link_to(self, target: str, label: "Flattenable") -> Tag: if self._module.isNameDefined(target): - self.warn_ambiguous_annotation(target) + warn_ambiguous_annotation(self._module, self._obj, target) return self._module_linker.link_to(target, label) elif self._scope.isNameDefined(target): return self._scope_linker.link_to(target, label) diff --git a/pydoctor/model.py b/pydoctor/model.py index be84dda66..1e0bae1f9 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -380,7 +380,8 @@ def module(self) -> 'Module': def report(self, descr: str, section: str = 'parsing', lineno_offset: int = 0, thresh:int=-1) -> None: """ - Log an error or warning about this documentable object. + Log an error or warning about this documentable object. + A reported message will only be printed once. @param descr: The error/warning string @param section: What the warning is about. @@ -405,7 +406,8 @@ def report(self, descr: str, section: str = 'parsing', lineno_offset: int = 0, t self.system.msg( section, f'{self.description}:{linenumber}: {descr}', - thresh=thresh) + # some warnings can be reported more that once. + thresh=thresh, once=True) @property def docstring_linker(self) -> 'linker.DocstringLinker': diff --git a/pydoctor/test/test_epydoc2stan.py b/pydoctor/test/test_epydoc2stan.py index 80d0dd238..fb80272ca 100644 --- a/pydoctor/test/test_epydoc2stan.py +++ b/pydoctor/test/test_epydoc2stan.py @@ -1029,21 +1029,22 @@ class Klass: mod.parsed_docstring.get_summary().to_stan(mod.docstring_linker) # type:ignore warnings = ['test:2: Cannot find link target for "thing.notfound" (you can link to external docs with --intersphinx)'] - if linkercls is linker._EpydocLinker: - warnings = warnings * 2 assert capsys.readouterr().out.strip().splitlines() == warnings - + + # reset warnings + mod.system.once_msgs = set() + # This is wrong: Klass.parsed_docstring.to_stan(mod.docstring_linker) # type:ignore Klass.parsed_docstring.get_summary().to_stan(mod.docstring_linker) # type:ignore # Because the warnings will be reported on line 2 warnings = ['test:2: Cannot find link target for "thing.notfound" (you can link to external docs with --intersphinx)'] - warnings = warnings * 2 assert capsys.readouterr().out.strip().splitlines() == warnings - # assert capsys.readouterr().out == '' + # reset warnings + mod.system.once_msgs = set() # Reset stan and summary, because they are supposed to be cached. Klass.parsed_docstring._stan = None # type:ignore @@ -1054,9 +1055,7 @@ class Klass: Klass.parsed_docstring.to_stan(mod.docstring_linker) # type:ignore Klass.parsed_docstring.get_summary().to_stan(mod.docstring_linker) # type:ignore - warnings = ['test:5: Cannot find link target for "thing.notfound" (you can link to external docs with --intersphinx)'] - warnings = warnings * 2 - + warnings = ['test:5: Cannot find link target for "thing.notfound" (you can link to external docs with --intersphinx)'] assert capsys.readouterr().out.strip().splitlines() == warnings def test_EpydocLinker_look_for_intersphinx_no_link() -> None: @@ -1291,8 +1290,6 @@ def test_EpydocLinker_warnings(capsys: CapSys) -> None: # The rationale about xref warnings is to warn when the target cannot be found. assert captured == ('module:3: Cannot find link target for "notfound"' - '\nmodule:3: Cannot find link target for "notfound"' - '\nmodule:5: Cannot find link target for "notfound"' '\nmodule:5: Cannot find link target for "notfound"\n') assert 'href="index.html#base"' in summary2html(mod) @@ -1999,7 +1996,6 @@ def f(self, x:typ) -> typ: assert capsys.readouterr().out == """\ m:5: ambiguous annotation 'typ', could be interpreted as 'm.C.typ' instead of 'm.typ' -m:5: ambiguous annotation 'typ', could be interpreted as 'm.C.typ' instead of 'm.typ' m:7: ambiguous annotation 'typ', could be interpreted as 'm.C.typ' instead of 'm.typ' """ @@ -2177,10 +2173,23 @@ def __init__(self, a:'_T'): unknow = system.allobjects['top.src.unknow'] assert flatten_text(unknow.parsed_type.to_stan(unknow.docstring_linker)) == 'i|None|list' - - - # TODO: test the __init__ signature and the class bases + # test the __init__ signature + assert 'href="top.src.html#_T"' in flatten(format_signature(Cls.contents['__init__'])) - # TODO: Fix two new twisted warnings: - # twisted/internet/_sslverify.py:330: Cannot find link target for "twisted.internet.ssl.DN", resolved from "twisted.internet._sslverify.DistinguishedName" - # twisted/internet/_sslverify.py:347: Cannot find link target for "twisted.internet.ssl.DN", resolved from "twisted.internet._sslverify.DistinguishedName" \ No newline at end of file +def test_reparented_ambiguous_annotation_confusion() -> None: + """ + Like L{test_top_level_type_alias_wins_over_class_level} but with reparented class. + """ + src = ''' + typ = object() + class C: + typ = int|str + var: typ + ''' + system = model.System() + builder = system.systemBuilder(system) + builder.addModuleString(src, modname='_m') + builder.addModuleString('from _m import C; __all__=["C"]', 'm') + builder.buildModules() + var = system.allobjects['m.C.var'] + assert 'href="_m.html#typ"' in flatten(var.parsed_type.to_stan(var.docstring_linker)) From 34e2c5433522757d0c2699c65d66d50d8c34d71b Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Tue, 11 Jul 2023 17:51:56 -0400 Subject: [PATCH 03/23] Fix mypy --- pydoctor/epydoc2stan.py | 4 +++- pydoctor/node2stan.py | 2 +- pydoctor/templatewriter/__init__.py | 2 +- pydoctor/test/test_epydoc2stan.py | 18 +++++++++--------- pydoctor/test/test_sphinx.py | 2 +- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/pydoctor/epydoc2stan.py b/pydoctor/epydoc2stan.py index 3f9141265..e074866ea 100644 --- a/pydoctor/epydoc2stan.py +++ b/pydoctor/epydoc2stan.py @@ -1160,7 +1160,7 @@ def __init__(self, document:nodes.document, self.ctx = ctx self.is_annotation = is_annotation - def apply(self): + def apply(self) -> None: ctx = self.ctx module = self.ctx.module for node in self.document.findall(nodes.title_reference): @@ -1237,3 +1237,5 @@ def transform_parsed_names(node:'model.Module') -> None: # TODO: resolve parsed_class_signature # TODO: resolve parsed_decorators pass + +# do one test with parsed type docstrings \ No newline at end of file diff --git a/pydoctor/node2stan.py b/pydoctor/node2stan.py index cb9fbee58..b5ae44ec6 100644 --- a/pydoctor/node2stan.py +++ b/pydoctor/node2stan.py @@ -56,7 +56,7 @@ def parse_reference(node:nodes.title_reference) -> Tuple[Union[str, Sequence[nod """ Split a reference into (label, target). """ - label: "Flattenable" + label: Union[str, Sequence[nodes.Node]] if 'refuri' in node.attributes: # Epytext parsed or manually constructed nodes. label, target = node.children, node.attributes['refuri'] diff --git a/pydoctor/templatewriter/__init__.py b/pydoctor/templatewriter/__init__.py index ffaff07a1..3ae033261 100644 --- a/pydoctor/templatewriter/__init__.py +++ b/pydoctor/templatewriter/__init__.py @@ -39,7 +39,7 @@ def parse_xml(text: str) -> minidom.Document: """ try: # TODO: submit a PR to typeshed to add a return type for parseString() - return cast(minidom.Document, minidom.parseString(text)) + return minidom.parseString(text) except Exception as e: raise ValueError(f"Failed to parse template as XML: {e}") from e diff --git a/pydoctor/test/test_epydoc2stan.py b/pydoctor/test/test_epydoc2stan.py index fb80272ca..3a33dccc3 100644 --- a/pydoctor/test/test_epydoc2stan.py +++ b/pydoctor/test/test_epydoc2stan.py @@ -2160,21 +2160,21 @@ def __init__(self, a:'_T'): clsvar2 = Cls.contents['clsvar2'] a = Cls.contents['_a'] assert clsvar.expandName('typing.List')=='typing.List' - assert '' in clsvar.parsed_type.to_node().pformat() - assert 'href="typing.html#List"' in flatten(clsvar.parsed_type.to_stan(clsvar.docstring_linker)) - assert 'href="typing.html#ClassVar"' in flatten(clsvar2.parsed_type.to_stan(clsvar2.docstring_linker)) - assert 'href="top.src.html#_T"' in flatten(a.parsed_type.to_stan(clsvar.docstring_linker)) + assert '' in clsvar.parsed_type.to_node().pformat() #type: ignore + assert 'href="typing.html#List"' in flatten(clsvar.parsed_type.to_stan(clsvar.docstring_linker)) #type: ignore + assert 'href="typing.html#ClassVar"' in flatten(clsvar2.parsed_type.to_stan(clsvar2.docstring_linker)) #type: ignore + assert 'href="top.src.html#_T"' in flatten(a.parsed_type.to_stan(clsvar.docstring_linker)) #type: ignore # the reparenting/alias issue ann = system.allobjects['top.src.ann'] - assert 'href="top.Cls.html"' in flatten(ann.parsed_type.to_stan(ann.docstring_linker)) - assert 'href="top.Cls.html"' in flatten(Cls.parsed_docstring.to_stan(Cls.docstring_linker)) + assert 'href="top.Cls.html"' in flatten(ann.parsed_type.to_stan(ann.docstring_linker)) #type: ignore + assert 'href="top.Cls.html"' in flatten(Cls.parsed_docstring.to_stan(Cls.docstring_linker)) #type: ignore unknow = system.allobjects['top.src.unknow'] - assert flatten_text(unknow.parsed_type.to_stan(unknow.docstring_linker)) == 'i|None|list' + assert flatten_text(unknow.parsed_type.to_stan(unknow.docstring_linker)) == 'i|None|list' #type: ignore # test the __init__ signature - assert 'href="top.src.html#_T"' in flatten(format_signature(Cls.contents['__init__'])) + assert 'href="top.src.html#_T"' in flatten(format_signature(Cls.contents['__init__'])) #type: ignore def test_reparented_ambiguous_annotation_confusion() -> None: """ @@ -2192,4 +2192,4 @@ class C: builder.addModuleString('from _m import C; __all__=["C"]', 'm') builder.buildModules() var = system.allobjects['m.C.var'] - assert 'href="_m.html#typ"' in flatten(var.parsed_type.to_stan(var.docstring_linker)) + assert 'href="_m.html#typ"' in flatten(var.parsed_type.to_stan(var.docstring_linker)) #type: ignore diff --git a/pydoctor/test/test_sphinx.py b/pydoctor/test/test_sphinx.py index d05274c04..2f87e7850 100644 --- a/pydoctor/test/test_sphinx.py +++ b/pydoctor/test/test_sphinx.py @@ -110,7 +110,7 @@ def test_generate_empty_functional() -> None: @contextmanager def openFileForWriting(path: str) -> Iterator[io.BytesIO]: yield output - inv_writer._openFileForWriting = openFileForWriting # type: ignore[assignment] + inv_writer._openFileForWriting = openFileForWriting # type:ignore inv_writer.generate(subjects=[], basepath='base-path') From ef41a2fba5cd1685ea7e09bd9e93e4579e6bb8ab Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Tue, 11 Jul 2023 17:52:52 -0400 Subject: [PATCH 04/23] fix pyflakes --- pydoctor/templatewriter/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/templatewriter/__init__.py b/pydoctor/templatewriter/__init__.py index 3ae033261..e81da8934 100644 --- a/pydoctor/templatewriter/__init__.py +++ b/pydoctor/templatewriter/__init__.py @@ -1,5 +1,5 @@ """Render pydoctor data as HTML.""" -from typing import Any, Iterable, Iterator, Optional, Union, cast, TYPE_CHECKING +from typing import Any, Iterable, Iterator, Optional, Union, TYPE_CHECKING if TYPE_CHECKING: from typing_extensions import Protocol, runtime_checkable else: From bd8af401901abab41591176b9c7cc92e90f6ba79 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Tue, 11 Jul 2023 18:15:20 -0400 Subject: [PATCH 05/23] fix ref --- pydoctor/epydoc2stan.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/epydoc2stan.py b/pydoctor/epydoc2stan.py index e074866ea..0bd2ce13c 100644 --- a/pydoctor/epydoc2stan.py +++ b/pydoctor/epydoc2stan.py @@ -1200,7 +1200,7 @@ def _apply_reference_transform(doc:ParsedDocstring, ctx:'model.Documentable', def transform_parsed_names(node:'model.Module') -> None: """ Walk this module's content and apply in-place transformations to the - L{ParsedDocstring} instances that olds L{obj_reference} or L{title_reference} nodes. + L{ParsedDocstring} instances that olds L{obj_reference} or L{nodes.title_reference} nodes. Fixing "Lookup of name in annotation fails on reparented object #295". The fix is not 100% complete at the moment: attribute values and decorators From bd69087992fbe15d711036de5734dda401fc04bf Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Wed, 12 Jul 2023 13:07:43 -0400 Subject: [PATCH 06/23] Fix issue regarding builtin names. Introduce the rawtarget attribute as well. --- pydoctor/epydoc2stan.py | 33 ++++++++++++--- pydoctor/sphinx.py | 6 +++ pydoctor/test/test_epydoc2stan.py | 68 ++++++++++++++++++++++++++++++- 3 files changed, 101 insertions(+), 6 deletions(-) diff --git a/pydoctor/epydoc2stan.py b/pydoctor/epydoc2stan.py index 0bd2ce13c..ad812ed6c 100644 --- a/pydoctor/epydoc2stan.py +++ b/pydoctor/epydoc2stan.py @@ -5,6 +5,7 @@ from collections import defaultdict import enum import inspect +import builtins from itertools import chain from typing import ( TYPE_CHECKING, Any, Callable, ClassVar, DefaultDict, Dict, Generator, @@ -1152,6 +1153,8 @@ def populate_constructors_extra_info(cls:model.Class) -> None: cls.extra_info.append(parse_docstring(cls, extra_epytext, cls, 'restructuredtext', section='constructor extra')) +_builtin_names = set(dir(builtins)) + class _ReferenceTransform(Transform): def __init__(self, document:nodes.document, @@ -1165,14 +1168,34 @@ def apply(self) -> None: module = self.ctx.module for node in self.document.findall(nodes.title_reference): _, target = parse_reference(node) - if target == node.attributes.get('refuri', target): + + # we're setting two attributes here: 'refuri' and 'rawtarget'. + # 'refuri' might already be created by the colorizer or docstring parser, + # but 'rawtarget' is only created from within this transform, so we can + # use that information to ensure this process is only ever applied once + # per title_reference element. + attribs = node.attributes + if target == attribs.get('refuri', target) and 'rawtarget' not in attribs: + # save the raw target name + attribs['rawtarget'] = target + name, *rest = target.split('.') + is_name_defined = ctx.isNameDefined(name) + # check if it's a non-shadowed builtins + if not is_name_defined and name in _builtin_names: + # transform bare builtin name into builtins. + attribs['refuri'] = '.'.join(('builtins', name, *rest)) + return + # no-op for unbound name + if not is_name_defined: + attribs['refuri'] = target + return # kindda duplicate a little part of the annotation linker logic here, # there are no simple way of doing it otherwise at the moment. # Once all presented parsed elements are stored as Documentable attributes # we might be able to simply use that and drop the use of the annotation linker, - # but for now this will to the trick: + # but for now this will do the trick: lookup_context = ctx if self.is_annotation and ctx is not module and module.isNameDefined(name, only_locals=True) and ctx.isNameDefined(name, only_locals=True): @@ -1180,9 +1203,9 @@ def apply(self) -> None: # lookup (wrt PEP 563) lookup_context = module linker.warn_ambiguous_annotation(module, ctx, target) - - node.attributes['refuri'] = '.'.join(chain( - lookup_context._localNameToFullName(name).split('.'), rest)) + # save pre-resolved refuri + attribs['refuri'] = '.'.join(chain(lookup_context.expandName(name).split('.'), rest)) + def _apply_reference_transform(doc:ParsedDocstring, ctx:'model.Documentable', is_annotation:bool=False) -> None: diff --git a/pydoctor/sphinx.py b/pydoctor/sphinx.py index 6e1a8ebad..2cc4862b9 100644 --- a/pydoctor/sphinx.py +++ b/pydoctor/sphinx.py @@ -135,6 +135,12 @@ def getLink(self, name: str) -> Optional[str]: """ Return link for `name` or None if no link is found. """ + # special casing the 'builtins' module because our name resolving + # replaces bare builtins names with builtins. in order not to confuse + # them with objects in the system when reparenting. + if name.startswith('builtins.'): + name = name[len('builtins.'):] + base_url, relative_link = self._links.get(name, (None, None)) if not relative_link: return None diff --git a/pydoctor/test/test_epydoc2stan.py b/pydoctor/test/test_epydoc2stan.py index 3a33dccc3..2c8181422 100644 --- a/pydoctor/test/test_epydoc2stan.py +++ b/pydoctor/test/test_epydoc2stan.py @@ -1187,6 +1187,43 @@ def test_EpydocLinker_resolve_identifier_xref_intersphinx_link_not_found(capsys: assert expected == captured +def test_EpydocLinker_link_not_found_show_original(capsys: CapSys) -> None: + n = '' + m = '''\ + from n import Stuff + S = Stuff + ''' + src = '''\ + """ + L{S} + """ + class Cls: + """ + L{Stuff } + """ + from m import S + ''' + system = model.System() + builder = system.systemBuilder(system) + builder.addModuleString(n, 'n') + builder.addModuleString(m, 'm') + builder.addModuleString(src, 'src') + builder.buildModules() + docstring2html(system.allobjects['src']) + captured = capsys.readouterr().out + # TODO: shoud say resolved from "S" + expected = ( + 'src:2: Cannot find link target for "n.Stuff", resolved from "m.S"\n' + ) + assert expected == captured + + docstring2html(system.allobjects['src.Cls']) + captured = capsys.readouterr().out + expected = ( + 'src:6: Cannot find link target for "n.Stuff", resolved from "m.S"\n' + ) + assert expected == captured + class InMemoryInventory: """ A simple inventory implementation which has an in-memory API link mapping. @@ -1412,6 +1449,8 @@ def __init__(self) -> None: self.requests: List[str] = [] def link_to(self, target: str, label: "Flattenable") -> Tag: + if target.startswith('builtins.'): + target = target[len('builtins.'):] self.requests.append(target) return tags.transparent(label) @@ -2160,7 +2199,7 @@ def __init__(self, a:'_T'): clsvar2 = Cls.contents['clsvar2'] a = Cls.contents['_a'] assert clsvar.expandName('typing.List')=='typing.List' - assert '' in clsvar.parsed_type.to_node().pformat() #type: ignore + assert 'refuri="typing.List"' in clsvar.parsed_type.to_node().pformat() #type: ignore assert 'href="typing.html#List"' in flatten(clsvar.parsed_type.to_stan(clsvar.docstring_linker)) #type: ignore assert 'href="typing.html#ClassVar"' in flatten(clsvar2.parsed_type.to_stan(clsvar2.docstring_linker)) #type: ignore assert 'href="top.src.html#_T"' in flatten(a.parsed_type.to_stan(clsvar.docstring_linker)) #type: ignore @@ -2193,3 +2232,30 @@ class C: builder.buildModules() var = system.allobjects['m.C.var'] assert 'href="_m.html#typ"' in flatten(var.parsed_type.to_stan(var.docstring_linker)) #type: ignore + +def test_reparented_builtins_confusion() -> None: + """ + - builtin links are resolved as such even when the new parent + declares a name shadowing a builtin. + """ + src = ''' + class C: + var: list + C = print('one') + ''' + top = ''' + list = object + print = partial(print, flush=True) + + from src import C + __all__=["C"] + ''' + system = model.System() + builder = system.systemBuilder(system) + builder.addModuleString(src, modname='src') + builder.addModuleString(top, modname='top') + builder.buildModules() + clsvar = system.allobjects['top.C.var'] + + assert 'refuri="builtins.list"' in clsvar.parsed_type.to_node().pformat() #type: ignore + # does not work for constant values at the moment From 0fd9982ba900b53c50a449a97d7cf08789432827 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Wed, 12 Jul 2023 13:27:34 -0400 Subject: [PATCH 07/23] Fix big loop issue --- pydoctor/epydoc2stan.py | 81 ++++++++++++++++--------------- pydoctor/test/test_epydoc2stan.py | 18 +++++++ 2 files changed, 59 insertions(+), 40 deletions(-) diff --git a/pydoctor/epydoc2stan.py b/pydoctor/epydoc2stan.py index ad812ed6c..d0407f3b2 100644 --- a/pydoctor/epydoc2stan.py +++ b/pydoctor/epydoc2stan.py @@ -1161,50 +1161,51 @@ def __init__(self, document:nodes.document, ctx:'model.Documentable', is_annotation:bool): super().__init__(document) self.ctx = ctx + self.module = ctx.module self.is_annotation = is_annotation - def apply(self) -> None: + def _transform(self, node:nodes.title_reference) -> None: ctx = self.ctx - module = self.ctx.module + module = self.module + _, target = parse_reference(node) + # we're setting two attributes here: 'refuri' and 'rawtarget'. + # 'refuri' might already be created by the colorizer or docstring parser, + # but 'rawtarget' is only created from within this transform, so we can + # use that information to ensure this process is only ever applied once + # per title_reference element. + attribs = node.attributes + if target == attribs.get('refuri', target) and 'rawtarget' not in attribs: + # save the raw target name + attribs['rawtarget'] = target + name, *rest = target.split('.') + is_name_defined = ctx.isNameDefined(name) + # check if it's a non-shadowed builtins + if not is_name_defined and name in _builtin_names: + # transform bare builtin name into builtins. + attribs['refuri'] = '.'.join(('builtins', name, *rest)) + return + # no-op for unbound name + if not is_name_defined: + attribs['refuri'] = target + return + # kindda duplicate a little part of the annotation linker logic here, + # there are no simple way of doing it otherwise at the moment. + # Once all presented parsed elements are stored as Documentable attributes + # we might be able to simply use that and drop the use of the annotation linker, + # but for now this will do the trick: + lookup_context = ctx + if self.is_annotation and ctx is not module and module.isNameDefined(name, + only_locals=True) and ctx.isNameDefined(name, only_locals=True): + # If we're dealing with an annotation, give precedence to the module's + # lookup (wrt PEP 563) + lookup_context = module + linker.warn_ambiguous_annotation(module, ctx, target) + # save pre-resolved refuri + attribs['refuri'] = '.'.join(chain(lookup_context.expandName(name).split('.'), rest)) + + def apply(self) -> None: for node in self.document.findall(nodes.title_reference): - _, target = parse_reference(node) - - # we're setting two attributes here: 'refuri' and 'rawtarget'. - # 'refuri' might already be created by the colorizer or docstring parser, - # but 'rawtarget' is only created from within this transform, so we can - # use that information to ensure this process is only ever applied once - # per title_reference element. - attribs = node.attributes - if target == attribs.get('refuri', target) and 'rawtarget' not in attribs: - # save the raw target name - attribs['rawtarget'] = target - - name, *rest = target.split('.') - is_name_defined = ctx.isNameDefined(name) - # check if it's a non-shadowed builtins - if not is_name_defined and name in _builtin_names: - # transform bare builtin name into builtins. - attribs['refuri'] = '.'.join(('builtins', name, *rest)) - return - # no-op for unbound name - if not is_name_defined: - attribs['refuri'] = target - return - - # kindda duplicate a little part of the annotation linker logic here, - # there are no simple way of doing it otherwise at the moment. - # Once all presented parsed elements are stored as Documentable attributes - # we might be able to simply use that and drop the use of the annotation linker, - # but for now this will do the trick: - lookup_context = ctx - if self.is_annotation and ctx is not module and module.isNameDefined(name, - only_locals=True) and ctx.isNameDefined(name, only_locals=True): - # If we're dealing with an annotation, give precedence to the module's - # lookup (wrt PEP 563) - lookup_context = module - linker.warn_ambiguous_annotation(module, ctx, target) - # save pre-resolved refuri - attribs['refuri'] = '.'.join(chain(lookup_context.expandName(name).split('.'), rest)) + self._transform(node) def _apply_reference_transform(doc:ParsedDocstring, ctx:'model.Documentable', diff --git a/pydoctor/test/test_epydoc2stan.py b/pydoctor/test/test_epydoc2stan.py index 2c8181422..eebef6aad 100644 --- a/pydoctor/test/test_epydoc2stan.py +++ b/pydoctor/test/test_epydoc2stan.py @@ -2259,3 +2259,21 @@ class C: assert 'refuri="builtins.list"' in clsvar.parsed_type.to_node().pformat() #type: ignore # does not work for constant values at the moment + +def test_link_resolving_unbound_names() -> None: + """ + - unbdound names are not touched, and does not stop the process. + """ + src = ''' + class C: + var: unknown|list + ''' + system = model.System() + builder = system.systemBuilder(system) + builder.addModuleString(src, modname='src') + builder.buildModules() + clsvar = system.allobjects['src.C.var'] + + assert 'refuri="builtins.list"' in clsvar.parsed_type.to_node().pformat() #type: ignore + assert 'refuri="unknown"' in clsvar.parsed_type.to_node().pformat() #type: ignore + # does not work for constant values at the moment \ No newline at end of file From b84f24c4b98111ee7d0a24616f97c8383a164ae0 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Wed, 12 Jul 2023 15:14:58 -0400 Subject: [PATCH 08/23] Better test coverage for the linker --- pydoctor/test/test_epydoc2stan.py | 55 +++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/pydoctor/test/test_epydoc2stan.py b/pydoctor/test/test_epydoc2stan.py index eebef6aad..8d802354e 100644 --- a/pydoctor/test/test_epydoc2stan.py +++ b/pydoctor/test/test_epydoc2stan.py @@ -1359,6 +1359,61 @@ class C: assert 'href="#var"' in url assert not capsys.readouterr().out +def test_EpydocLinker_xref_look_for_name_multiple_candidates(capsys:CapSys) -> None: + """ + When the linker use look_for_name(), if 'identifier' refers to more than one object, it complains. + """ + system = model.System() + builder = system.systemBuilder(system) + builder.addModuleString('class C:...', modname='_one') + builder.addModuleString('class C:...', modname='_two') + builder.addModuleString('"L{C}"', modname='top') + builder.buildModules() + docstring2html(system.allobjects['top']) + assert capsys.readouterr().out == ( + 'top:1: ambiguous ref to C, could be _one.C, _two.C\n' + 'top:1: Cannot find link target for "C"\n') + +def test_EpydocLinker_xref_look_for_name_into_uncle_objects(capsys:CapSys) -> None: + """ + The linker walk up the object tree and see if 'identifier' refers to an + object in an "uncle" object. + """ + system = model.System() + builder = system.systemBuilder(system) + builder.addModuleString('', modname='pack', is_package=True) + builder.addModuleString('class C:...', modname='mod2', parent_name='pack') + builder.addModuleString('class I:\n var=1;"L{C}"', modname='mod1', parent_name='pack') + builder.buildModules() + assert 'href="pack.mod2.C.html"' in docstring2html(system.allobjects['pack.mod1.I.var']) + assert capsys.readouterr().out == '' + +def test_EpydocLinker_xref_look_for_name_into_all_modules(capsys:CapSys) -> None: + """ + The linker examine every module and package in the system and see if 'identifier' + names an object in each one. + """ + system = model.System() + builder = system.systemBuilder(system) + builder.addModuleString('class C:...', modname='_one') + builder.addModuleString('"L{C}"', modname='top') + builder.buildModules() + assert 'href="_one.C.html"' in docstring2html(system.allobjects['top']) + assert capsys.readouterr().out == '' + +def test_EpydocLinker_xref_walk_up_the_object_tree(capsys:CapSys) -> None: + """ + The linker walks up the object tree and see if 'identifier' refers + to an object by Python name resolution in each context. + """ + system = model.System() + builder = system.systemBuilder(system) + builder.addModuleString('class C:...', modname='pack', is_package=True) + builder.addModuleString('class I:\n var=1;"L{C}"', modname='mod1', parent_name='pack') + builder.buildModules() + assert 'href="pack.C.html"' in docstring2html(system.allobjects['pack.mod1.I.var']) + assert capsys.readouterr().out == '' + def test_xref_not_found_epytext(capsys: CapSys) -> None: """ When a link in an epytext docstring cannot be resolved, the reference From 31144e50c694dafd8bd119f68cfb19e10e38a496 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 13 Jul 2023 12:27:11 -0400 Subject: [PATCH 09/23] Introduce Class.parsed_bases, Function.parsed_decorators, Function.parsed_annotations, FunctionOverload.parsed_decorators, Attribute.parsed_decorators, Attribute.parsed_value. Fixes links in presentation of these components. --- pydoctor/astbuilder.py | 8 +- pydoctor/epydoc/markup/_types.py | 1 + pydoctor/epydoc2stan.py | 150 ++++++++++++++++------ pydoctor/model.py | 9 +- pydoctor/templatewriter/pages/__init__.py | 31 +---- pydoctor/test/test_epydoc2stan.py | 24 +++- 6 files changed, 150 insertions(+), 73 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index a9199c922..d663cfeee 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -981,12 +981,12 @@ class _ValueFormatter: """ def __init__(self, value: ast.expr, ctx: model.Documentable): - self._colorized = colorize_inline_pyval(value) + self.parsed = colorize_inline_pyval(value) """ The colorized value as L{ParsedDocstring}. """ - self._linker = ctx.docstring_linker + self.linker = ctx.docstring_linker """ Linker. """ @@ -999,7 +999,7 @@ def __repr__(self) -> str: # Using node2stan.node2html instead of flatten(to_stan()). # This avoids calling flatten() twice, # but potential XML parser errors caused by XMLString needs to be handled later. - return ''.join(node2stan.node2html(self._colorized.to_node(), self._linker)) + return ''.join(node2stan.node2html(self.parsed.to_node(), self.linker)) class _AnnotationValueFormatter(_ValueFormatter): """ @@ -1007,7 +1007,7 @@ class _AnnotationValueFormatter(_ValueFormatter): """ def __init__(self, value: ast.expr, ctx: model.Function): super().__init__(value, ctx) - self._linker = linker._AnnotationLinker(ctx) + self.linker = linker._AnnotationLinker(ctx) def __repr__(self) -> str: """ diff --git a/pydoctor/epydoc/markup/_types.py b/pydoctor/epydoc/markup/_types.py index b221ccb87..8b5f00f6f 100644 --- a/pydoctor/epydoc/markup/_types.py +++ b/pydoctor/epydoc/markup/_types.py @@ -13,6 +13,7 @@ from docutils import nodes from twisted.web.template import Tag, tags +# TODO: this class should support to_node() like others. class ParsedTypeDocstring(TypeDocstring, ParsedDocstring): """ Add L{ParsedDocstring} interface on top of L{TypeDocstring} and diff --git a/pydoctor/epydoc2stan.py b/pydoctor/epydoc2stan.py index d0407f3b2..1da415e46 100644 --- a/pydoctor/epydoc2stan.py +++ b/pydoctor/epydoc2stan.py @@ -4,12 +4,13 @@ from collections import defaultdict import enum +from functools import partial import inspect import builtins from itertools import chain from typing import ( TYPE_CHECKING, Any, Callable, ClassVar, DefaultDict, Dict, Generator, - Iterator, List, Mapping, Optional, Sequence, Tuple, Union, + Iterator, List, Mapping, Optional, Sequence, Tuple, TypeVar, Union, ) import ast import re @@ -271,18 +272,19 @@ def __init__(self, obj: model.Documentable): self.sinces: List[Field] = [] self.unknowns: DefaultDict[str, List[FieldDesc]] = defaultdict(list) - def set_param_types_from_annotations( - self, annotations: Mapping[str, Optional[ast.expr]] - ) -> None: + def set_param_types_from_annotations(self) -> None: + if not isinstance(self.obj, model.Function): + return + annotations = self.obj.annotations _linker = linker._AnnotationLinker(self.obj) formatted_annotations = { - name: None if value is None - else ParamType(safe_to_stan(colorize_inline_pyval(value), _linker, + name: None if parsed_annotation is None + else ParamType(safe_to_stan(parsed_annotation, _linker, self.obj, fallback=colorized_pyval_fallback, section='annotation', report=False), # don't spam the log, invalid annotation are going to be reported when the signature gets colorized origin=FieldOrigin.FROM_AST) - for name, value in annotations.items() + for name, parsed_annotation in get_parsed_annotations(self.obj).items() } ret_type = formatted_annotations.pop('return', None) @@ -799,8 +801,7 @@ def format_docstring(obj: model.Documentable) -> Tag: ret(unwrap_docstring_stan(stan)) fh = FieldHandler(obj) - if isinstance(obj, model.Function): - fh.set_param_types_from_annotations(obj.annotations) + fh.set_param_types_from_annotations() if source is not None: assert obj.parsed_docstring is not None, "ensure_parsed_docstring() did not do it's job" for field in obj.parsed_docstring.fields: @@ -879,23 +880,87 @@ def type2stan(obj: model.Documentable) -> Optional[Tag]: return safe_to_stan(parsed_type, _linker, obj, fallback=colorized_pyval_fallback, section='annotation') +_T = TypeVar('_T') +def _memoize(o:object, attrname:str, getter:Callable[[], _T]) -> _T: + parsed = getattr(o, attrname, None) + if parsed is not None: + return parsed #type:ignore + parsed = getter() + setattr(o, attrname, parsed) + return parsed + def get_parsed_type(obj: model.Documentable) -> Optional[ParsedDocstring]: """ Get the type of this attribute as parsed docstring. """ - parsed_type = obj.parsed_type - if parsed_type is not None: - return parsed_type - - # Only Attribute instances have the 'annotation' attribute. - annotation: Optional[ast.expr] = getattr(obj, 'annotation', None) - if annotation is not None: - parsed_type = colorize_inline_pyval(annotation) - # cache parsed_type attribute - obj.parsed_type = parsed_type - return parsed_type + def _get_parsed_type() -> Optional[ParsedDocstring]: + annotation = getattr(obj, 'annotation', None) + if annotation is not None: + v = colorize_inline_pyval(annotation) + reportWarnings(obj, v.warnings, section='colorize annotation') + return v + return None + return _memoize(obj, 'parsed_type', _get_parsed_type) - return None +def get_parsed_decorators(obj: Union[model.Attribute, model.Function, + model.FunctionOverload]) -> Optional[Sequence[ParsedDocstring]]: + """ + Get the decorators of this function as parsed docstring. + """ + def _get_parsed_decorators() -> Optional[Sequence[ParsedDocstring]]: + v = [colorize_inline_pyval(dec) for dec in obj.decorators] if \ + obj.decorators is not None else None + for c in v or (): + reportWarnings(obj, c.warnings, section='colorize decorators') + return v + return _memoize(obj, 'parsed_decorators', _get_parsed_decorators) + +def get_parsed_value(obj:model.Attribute) -> Optional[ParsedDocstring]: + """ + Get the value of this constant as parsed docstring. + """ + def _get_parsed_value() -> Optional[ParsedDocstring]: + v = colorize_pyval(obj.value, + linelen=obj.system.options.pyvalreprlinelen, + maxlines=obj.system.options.pyvalreprmaxlines) if obj.value is not None else None + # Report eventual warnings. + reportWarnings(obj, v.warnings, section='colorize constant') + return v + return _memoize(obj, 'parsed_value', _get_parsed_value) + +def get_parsed_annotations(obj:model.Function) -> Mapping[str, Optional[ParsedDocstring]]: + """ + Get the annotations of this function as dict from str to parsed docstring. + """ + def _get_parsed_annotations() -> Mapping[str, Optional[ParsedDocstring]]: + return {name:colorize_inline_pyval(ann) if ann else None for \ + (name, ann) in obj.annotations.items()} + # do not warn here + return _memoize(obj, 'parsed_annotations', _get_parsed_annotations) + +def get_parsed_bases(obj:model.Class) -> Sequence[ParsedDocstring]: + """ + Get the bases of this class as a seqeunce of parsed docstrings. + """ + def _get_parsed_bases() -> Sequence[ParsedDocstring]: + r = [] + for (str_base, base_node), base_obj in zip(obj.rawbases, obj.baseobjects): + # Make sure we bypass the linker’s resolver process for base object, + # because it has been resolved already (with two passes). + # Otherwise, since the class declaration wins over the imported names, + # a class with the same name as a base class confused pydoctor and it would link + # to it self: https://github.com/twisted/pydoctor/issues/662 + refmap = None + if base_obj is not None: + refmap = {str_base:base_obj.fullName()} + + # link to external class, using the colorizer here + # to link to classes with generics (subscripts and other AST expr). + p = colorize_inline_pyval(base_node, refmap=refmap) + r.append(p) + reportWarnings(obj, p.warnings, section='colorize bases') + return r + return _memoize(obj, 'parsed_bases', _get_parsed_bases) def format_toc(obj: model.Documentable) -> Optional[Tag]: # Load the parsed_docstring if it's not already done. @@ -990,23 +1055,19 @@ def colorized_pyval_fallback(_: List[ParseError], doc:ParsedDocstring, __:model. return Tag('code')(node2stan.gettext(doc.to_node())) def _format_constant_value(obj: model.Attribute) -> Iterator["Flattenable"]: - + doc = get_parsed_value(obj) + if doc is None: + return + # yield the table title, "Value" row = tags.tr(class_="fieldStart") row(tags.td(class_="fieldName")("Value")) # yield the first row. yield row - doc = colorize_pyval(obj.value, - linelen=obj.system.options.pyvalreprlinelen, - maxlines=obj.system.options.pyvalreprmaxlines) - value_repr = safe_to_stan(doc, obj.docstring_linker, obj, fallback=colorized_pyval_fallback, section='rendering of constant') - # Report eventual warnings. It warns when a regex failed to parse. - reportWarnings(obj, doc.warnings, section='colorize constant') - # yield the value repr. row = tags.tr() row(tags.td(tags.pre(class_='constant-value')(value_repr))) @@ -1243,23 +1304,34 @@ def transform_parsed_names(node:'model.Module') -> None: for p in ob.signature.parameters.values(): ann = p.annotation if p.annotation is not inspect.Parameter.empty else None if isinstance(ann, astbuilder._ValueFormatter): - _apply_reference_transform(ann._colorized, ob, is_annotation=True) + _apply_reference_transform(ann.parsed, ob, is_annotation=True) default = p.default if p.default is not inspect.Parameter.empty else None if isinstance(default, astbuilder._ValueFormatter): - _apply_reference_transform(default._colorized, ob) - # TODO: resolve function's annotations, they are currently presented twice - # we can only change signature, annotations in param table must be handled by - # introducing attribute parsed_annotations + _apply_reference_transform(default.parsed, ob) + for _,ann in get_parsed_annotations(ob).items(): + if ann: + _apply_reference_transform(ann, ob, is_annotation=True) + for dec in get_parsed_decorators(ob) or (): + if dec: + _apply_reference_transform(dec, ob) + for overload in ob.overloads: + for dec in get_parsed_decorators(overload) or (): + if dec: + _apply_reference_transform(dec, ob) elif isinstance(ob, model.Attribute): # resolve attribute annotation with parsed_type attribute parsed_type = get_parsed_type(ob) if parsed_type: _apply_reference_transform(parsed_type, ob, is_annotation=True) - # TODO: resolve parsed_value - # TODO: resolve parsed_decorators + if ob.kind in ob.system.show_attr_value: + parsed_value = get_parsed_value(ob) + if parsed_value: + _apply_reference_transform(parsed_value, ob) + for dec in get_parsed_decorators(ob) or (): + if dec: + _apply_reference_transform(dec, ob) elif isinstance(ob, model.Class): - # TODO: resolve parsed_class_signature - # TODO: resolve parsed_decorators - pass + for base in get_parsed_bases(ob): + _apply_reference_transform(base, ob) # do one test with parsed type docstrings \ No newline at end of file diff --git a/pydoctor/model.py b/pydoctor/model.py index 1e0bae1f9..a52232dff 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -646,7 +646,9 @@ def setup(self) -> None: """ self._initialbases: List[str] = [] self._initialbaseobjects: List[Optional['Class']] = [] - + + self.parsed_bases:Optional[List[ParsedDocstring]] = None + def _init_mro(self) -> None: """ Compute the correct value of the method resolution order returned by L{mro()}. @@ -843,6 +845,8 @@ def setup(self) -> None: self.kind = DocumentableKind.METHOD self.signature = None self.overloads = [] + self.parsed_decorators:Optional[Sequence[ParsedDocstring]] = None + self.parsed_annotations:Optional[Dict[str, Optional[ParsedDocstring]]] = None @attr.s(auto_attribs=True) class FunctionOverload: @@ -852,6 +856,7 @@ class FunctionOverload: primary: Function signature: Signature decorators: Sequence[ast.expr] + parsed_decorators:Optional[Sequence[ParsedDocstring]] = None class Attribute(Inheritable): kind: Optional[DocumentableKind] = DocumentableKind.ATTRIBUTE @@ -863,6 +868,8 @@ class Attribute(Inheritable): None value means the value is not initialized at the current point of the the process. """ + parsed_decorators:Optional[Sequence[ParsedDocstring]] = None + parsed_value:Optional[ParsedDocstring] = None # Work around the attributes of the same name within the System class. _ModuleT = Module diff --git a/pydoctor/templatewriter/pages/__init__.py b/pydoctor/templatewriter/pages/__init__.py index 272239605..68fd5425f 100644 --- a/pydoctor/templatewriter/pages/__init__.py +++ b/pydoctor/templatewriter/pages/__init__.py @@ -50,7 +50,8 @@ def format_decorators(obj: Union[model.Function, model.Attribute, model.Function # 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 - for dec in obj.decorators or (): + for dec, doc in zip(obj.decorators or (), + epydoc2stan.get_parsed_decorators(obj) or ()): if isinstance(dec, ast.Call): fn = node2fullname(dec.func, documentable_obj) # We don't want to show the deprecated decorator; @@ -58,15 +59,9 @@ def format_decorators(obj: Union[model.Function, model.Attribute, model.Function if fn in ("twisted.python.deprecate.deprecated", "twisted.python.deprecate.deprecatedProperty"): break - - # Colorize decorators! - doc = colorize_inline_pyval(dec) stan = epydoc2stan.safe_to_stan(doc, documentable_obj.docstring_linker, documentable_obj, fallback=epydoc2stan.colorized_pyval_fallback, section='rendering of decorators') - - # Report eventual warnings. It warns when we can't colorize the expression for some reason. - epydoc2stan.reportWarnings(documentable_obj, doc.warnings, section='colorize decorator') yield '@', stan.children, tags.br() def format_signature(func: Union[model.Function, model.FunctionOverload]) -> "Flattenable": @@ -90,29 +85,17 @@ def format_class_signature(cls: model.Class) -> "Flattenable": """ r: List["Flattenable"] = [] # the linker will only be used to resolve the generic arguments of the base classes, - # it won't actually resolve the base classes (see comment few lines below). + # it won't actually resolve the base classes (see comment in epydoc2stan.get_parsed_bases). # this is why we're using the annotation linker. _linker = linker._AnnotationLinker(cls) - if cls.rawbases: + parsed_bases = epydoc2stan.get_parsed_bases(cls) + if parsed_bases: r.append('(') - for idx, ((str_base, base_node), base_obj) in enumerate(zip(cls.rawbases, cls.baseobjects)): + for idx, parsed_base in enumerate(parsed_bases): if idx != 0: r.append(', ') - - # Make sure we bypass the linker’s resolver process for base object, - # because it has been resolved already (with two passes). - # Otherwise, since the class declaration wins over the imported names, - # a class with the same name as a base class confused pydoctor and it would link - # to it self: https://github.com/twisted/pydoctor/issues/662 - - refmap = None - if base_obj is not None: - refmap = {str_base:base_obj.fullName()} - - # link to external class, using the colorizer here - # to link to classes with generics (subscripts and other AST expr). - stan = epydoc2stan.safe_to_stan(colorize_inline_pyval(base_node, refmap=refmap), _linker, cls, + stan = epydoc2stan.safe_to_stan(parsed_base, _linker, cls, fallback=epydoc2stan.colorized_pyval_fallback, section='rendering of class signature') r.extend(stan.children) diff --git a/pydoctor/test/test_epydoc2stan.py b/pydoctor/test/test_epydoc2stan.py index 8d802354e..ea72bc824 100644 --- a/pydoctor/test/test_epydoc2stan.py +++ b/pydoctor/test/test_epydoc2stan.py @@ -2294,13 +2294,15 @@ def test_reparented_builtins_confusion() -> None: declares a name shadowing a builtin. """ src = ''' - class C: + class C(int): var: list C = print('one') + @stuff(auto=object) + def __init__(self, v:bytes=bytes): + "L{str}" ''' top = ''' - list = object - print = partial(print, flush=True) + list = object = int = print = str = bytes = True from src import C __all__=["C"] @@ -2311,9 +2313,17 @@ class C: builder.addModuleString(top, modname='top') builder.buildModules() clsvar = system.allobjects['top.C.var'] + C = system.allobjects['top.C'] + Ci = system.allobjects['top.C.C'] + __init__ = system.allobjects['top.C.__init__'] assert 'refuri="builtins.list"' in clsvar.parsed_type.to_node().pformat() #type: ignore - # does not work for constant values at the moment + assert 'refuri="builtins.print"' in Ci.parsed_value.to_node().pformat() #type: ignore + assert 'refuri="builtins.int"' in C.parsed_bases[0].to_node().pformat() #type: ignore + assert 'refuri="builtins.object"' in __init__.parsed_decorators[0].to_node().pformat() #type: ignore + assert 'refuri="builtins.bytes"' in __init__.signature.parameters['v'].default.parsed.to_node().pformat() #type: ignore + assert 'refuri="builtins.bytes"' in __init__.signature.parameters['v'].annotation.parsed.to_node().pformat() #type: ignore + assert 'refuri="builtins.bytes"' in __init__.parsed_annotations['v'].to_node().pformat() #type: ignore def test_link_resolving_unbound_names() -> None: """ @@ -2331,4 +2341,8 @@ class C: assert 'refuri="builtins.list"' in clsvar.parsed_type.to_node().pformat() #type: ignore assert 'refuri="unknown"' in clsvar.parsed_type.to_node().pformat() #type: ignore - # does not work for constant values at the moment \ No newline at end of file + # does not work for constant values at the moment + +# what to do with inherited documentation of reparented class attribute part of an +# import cycle? We can't set the value of parsed_docstring from the astbuilder because +# we havnen't resolved the mro yet. \ No newline at end of file From e39b77198685202070899fb1cc43410fb2be1c7a Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 13 Jul 2023 13:59:29 -0400 Subject: [PATCH 10/23] Add tests --- pydoctor/test/test_epydoc2stan.py | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/pydoctor/test/test_epydoc2stan.py b/pydoctor/test/test_epydoc2stan.py index ea72bc824..7bfaf4511 100644 --- a/pydoctor/test/test_epydoc2stan.py +++ b/pydoctor/test/test_epydoc2stan.py @@ -2324,6 +2324,8 @@ def __init__(self, v:bytes=bytes): assert 'refuri="builtins.bytes"' in __init__.signature.parameters['v'].default.parsed.to_node().pformat() #type: ignore assert 'refuri="builtins.bytes"' in __init__.signature.parameters['v'].annotation.parsed.to_node().pformat() #type: ignore assert 'refuri="builtins.bytes"' in __init__.parsed_annotations['v'].to_node().pformat() #type: ignore + assert __init__.parsed_docstring is None # should not be none, actually :/ + # assert 'refuri="builtins.bytes"' in __init__.parsed_docstring.to_node().pformat() #type: ignore def test_link_resolving_unbound_names() -> None: """ @@ -2343,6 +2345,29 @@ class C: assert 'refuri="unknown"' in clsvar.parsed_type.to_node().pformat() #type: ignore # does not work for constant values at the moment +def test_reference_transform_in_type_docstring() -> None: + """ + It will fail with ParsedTypeDocstring at the moment. + """ + src = ''' + __docformat__='google' + class C: + """ + Args: + a (list): the list + """ + ''' + system = model.System() + builder = system.systemBuilder(system) + builder.addModuleString(src, modname='src') + builder.addModuleString('from src import C;__all__=["C"];list=True', modname='top') + builder.buildModules() + clsvar = system.allobjects['top.C'] + + with pytest.raises(NotImplementedError): + assert 'refuri="builtins.list"' in clsvar.parsed_docstring.fields[1].body().to_node().pformat() #type: ignore + # what to do with inherited documentation of reparented class attribute part of an # import cycle? We can't set the value of parsed_docstring from the astbuilder because -# we havnen't resolved the mro yet. \ No newline at end of file +# we havnen't resolved the mro yet. + From f0f694d4d05d4a142cbed89ff420970d80e9fabd Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 13 Jul 2023 14:02:31 -0400 Subject: [PATCH 11/23] Fix mypy --- pydoctor/epydoc2stan.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pydoctor/epydoc2stan.py b/pydoctor/epydoc2stan.py index 1da415e46..a85c9ed69 100644 --- a/pydoctor/epydoc2stan.py +++ b/pydoctor/epydoc2stan.py @@ -910,8 +910,10 @@ def get_parsed_decorators(obj: Union[model.Attribute, model.Function, def _get_parsed_decorators() -> Optional[Sequence[ParsedDocstring]]: v = [colorize_inline_pyval(dec) for dec in obj.decorators] if \ obj.decorators is not None else None + documentable_obj = obj if not isinstance(obj, model.FunctionOverload) else obj.primary for c in v or (): - reportWarnings(obj, c.warnings, section='colorize decorators') + if c: + reportWarnings(documentable_obj, c.warnings, section='colorize decorators') return v return _memoize(obj, 'parsed_decorators', _get_parsed_decorators) From dd2cf17a911a27b64417f073506082654df760bb Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 13 Jul 2023 14:06:15 -0400 Subject: [PATCH 12/23] fix static checks --- pydoctor/epydoc2stan.py | 5 ++--- pydoctor/templatewriter/pages/__init__.py | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/pydoctor/epydoc2stan.py b/pydoctor/epydoc2stan.py index a85c9ed69..6d7371966 100644 --- a/pydoctor/epydoc2stan.py +++ b/pydoctor/epydoc2stan.py @@ -4,7 +4,6 @@ from collections import defaultdict import enum -from functools import partial import inspect import builtins from itertools import chain @@ -12,7 +11,6 @@ TYPE_CHECKING, Any, Callable, ClassVar, DefaultDict, Dict, Generator, Iterator, List, Mapping, Optional, Sequence, Tuple, TypeVar, Union, ) -import ast import re import attr @@ -926,7 +924,8 @@ def _get_parsed_value() -> Optional[ParsedDocstring]: linelen=obj.system.options.pyvalreprlinelen, maxlines=obj.system.options.pyvalreprmaxlines) if obj.value is not None else None # Report eventual warnings. - reportWarnings(obj, v.warnings, section='colorize constant') + if v: + reportWarnings(obj, v.warnings, section='colorize constant') return v return _memoize(obj, 'parsed_value', _get_parsed_value) diff --git a/pydoctor/templatewriter/pages/__init__.py b/pydoctor/templatewriter/pages/__init__.py index 68fd5425f..77c737b65 100644 --- a/pydoctor/templatewriter/pages/__init__.py +++ b/pydoctor/templatewriter/pages/__init__.py @@ -17,7 +17,6 @@ from pydoctor.templatewriter import util, TemplateLookup, TemplateElement from pydoctor.templatewriter.pages.table import ChildTable from pydoctor.templatewriter.pages.sidebar import SideBar -from pydoctor.epydoc.markup._pyval_repr import colorize_inline_pyval if TYPE_CHECKING: from typing_extensions import Final From 190b5a75ba3cc0ec8fc7a1a62f880ecbf3bdf8fa Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 29 Sep 2023 13:29:05 -0400 Subject: [PATCH 13/23] - Parse module imports and store them into Module.imports attribute. - Call handleDuplicate when re-exporting process introduces a new duplicate object. --- pydoctor/astbuilder.py | 65 +++++++++----- pydoctor/model.py | 36 +++++++- pydoctor/test/test_astbuilder.py | 148 +++++++++++++++++++++++++++++++ 3 files changed, 224 insertions(+), 25 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 4ace10bc4..2b0c5a705 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -166,6 +166,21 @@ def extract_final_subscript(annotation: ast.Subscript) -> ast.expr: assert isinstance(ann_slice, ast.expr) return ann_slice +def getPublicNames(mod:'model.Module') -> Collection[str]: + """ + Get all names to import when wildcardm importing the given module: + use __all__ if available, otherwise take all names that are not private. + """ + names = mod.all + if names is None: + names = [ + name + for name in chain(mod.contents.keys(), + mod._localNameToFullName_map.keys()) + if not name.startswith('_') + ] + return names + class ModuleVistor(NodeVisitor): def __init__(self, builder: 'ASTBuilder', module: model.Module): @@ -325,32 +340,23 @@ def visit_ImportFrom(self, node: ast.ImportFrom) -> None: assert modname is not None if node.names[0].name == '*': - self._importAll(modname) + self._importAll(modname, linenumber=node.lineno) else: - self._importNames(modname, node.names) + self._importNames(modname, node.names, linenumber=node.lineno) - def _importAll(self, modname: str) -> None: + def _importAll(self, modname: str, linenumber:int) -> None: """Handle a C{from import *} statement.""" + ctx = self.builder.current + if isinstance(ctx, model.Module): + ctx.imports.append(model.Import('*', modname, linenumber=linenumber, orgname='*')) mod = self.system.getProcessedModule(modname) if mod is None: # We don't have any information about the module, so we don't know # what names to import. - self.builder.current.report(f"import * from unknown {modname}", thresh=1) + ctx.report(f"import * from unknown {modname}", thresh=1) return - - self.builder.current.report(f"import * from {modname}", thresh=1) - - # Get names to import: use __all__ if available, otherwise take all - # names that are not private. - names = mod.all - if names is None: - names = [ - name - for name in chain(mod.contents.keys(), - mod._localNameToFullName_map.keys()) - if not name.startswith('_') - ] + ctx.report(f"import * from {modname}", thresh=1) # Fetch names to export. exports = self._getCurrentModuleExports() @@ -359,7 +365,7 @@ def _importAll(self, modname: str) -> None: assert isinstance(self.builder.current, model.CanContainImportsDocumentable) _localNameToFullName = self.builder.current._localNameToFullName_map expandName = mod.expandName - for name in names: + for name in getPublicNames(mod): if self._handleReExport(exports, name, name, mod) is True: continue @@ -394,8 +400,8 @@ def _handleReExport(self, curr_mod_exports:Collection[str], # So we use content.get first to resolve non-alias names. ob = origin_module.contents.get(origin_name) or origin_module.resolveName(origin_name) if ob is None: - current.report("cannot resolve re-exported name :" - f'{modname}.{origin_name}', thresh=1) + current.report("cannot resolve re-exported name: " + f"'{modname}.{origin_name}'", thresh=1) else: if origin_module.all is None or origin_name not in origin_module.all: self.system.msg( @@ -408,7 +414,7 @@ def _handleReExport(self, curr_mod_exports:Collection[str], return True return False - def _importNames(self, modname: str, names: Iterable[ast.alias]) -> None: + def _importNames(self, modname: str, names: Iterable[ast.alias], linenumber:int) -> None: """Handle a C{from import } statement.""" # Process the module we're importing from. @@ -420,6 +426,8 @@ def _importNames(self, modname: str, names: Iterable[ast.alias]) -> None: current = self.builder.current assert isinstance(current, model.CanContainImportsDocumentable) _localNameToFullName = current._localNameToFullName_map + is_module = isinstance(current, model.Module) + for al in names: orgname, asname = al.name, al.asname if asname is None: @@ -434,6 +442,10 @@ def _importNames(self, modname: str, names: Iterable[ast.alias]) -> None: self.system.getProcessedModule(f'{modname}.{orgname}') _localNameToFullName[asname] = f'{modname}.{orgname}' + if is_module: + cast(model.Module, + current).imports.append(model.Import(asname, modname, + orgname=orgname, linenumber=linenumber)) def visit_Import(self, node: ast.Import) -> None: """Process an import statement. @@ -448,16 +460,23 @@ def visit_Import(self, node: ast.Import) -> None: (dotted_name, as_name) where as_name is None if there was no 'as foo' part of the statement. """ - if not isinstance(self.builder.current, model.CanContainImportsDocumentable): + ctx = self.builder.current + if not isinstance(ctx, model.CanContainImportsDocumentable): # processing import statement in odd context return - _localNameToFullName = self.builder.current._localNameToFullName_map + _localNameToFullName = ctx._localNameToFullName_map + is_module = isinstance(ctx, model.Module) + for al in node.names: targetname, asname = al.name, al.asname if asname is None: # we're keeping track of all defined names asname = targetname = targetname.split('.')[0] _localNameToFullName[asname] = targetname + if is_module: + cast(model.Module, + ctx).imports.append(model.Import(asname, targetname, + linenumber=node.lineno)) def _handleOldSchoolMethodDecoration(self, target: str, expr: Optional[ast.expr]) -> bool: if not isinstance(expr, ast.Call): diff --git a/pydoctor/model.py b/pydoctor/model.py index 41bb4c10f..4c588eb7f 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -242,14 +242,31 @@ def docsources(self) -> Iterator['Documentable']: def reparent(self, new_parent: 'Module', new_name: str) -> None: + """ + Move this documentable to a new location. + """ + + old_name = self.name + new_contents = new_parent.contents + + # issue warnings + if new_name in new_contents: + + self.system.handleDuplicate(new_contents[new_name]) + self.report(f"introduced by re-exporting {self} into {new_parent}" + '' if new_name==old_name else f' as {new_name!r}', thresh=1) + # this code attempts to preserve "rather a lot" of # invariants assumed by various bits of pydoctor # and that are of course not written down anywhere # :/ - self._handle_reparenting_pre() + # Basically we maintain at least 2 references for each object in the system + # one in it's parent.contents dict and one in allobject dict. The later has been proven + # not to be necessary, but it speeds-up name resolving. + self._handle_reparenting_pre() # but why do we call this method twice? old_parent = self.parent assert isinstance(old_parent, CanContainImportsDocumentable) - old_name = self.name + self.parent = self.parentMod = new_parent self.name = new_name self._handle_reparenting_post() @@ -421,7 +438,19 @@ def isNameDefined(self, name: str) -> bool: return self.module.isNameDefined(name) else: return False + +@attr.s(auto_attribs=True, slots=True) +class Import: + """ + An imported name. + @note: One L{Import} instance is created for each + name bound in the C{import} statement. + """ + name:str + orgmodule:str + linenumber:int + orgname:Optional[str]=None class Module(CanContainImportsDocumentable): kind = DocumentableKind.MODULE @@ -457,6 +486,8 @@ def setup(self) -> None: self._docformat: Optional[str] = None + self.imports: List[Import] = [] + def _localNameToFullName(self, name: str) -> str: if name in self.contents: o: Documentable = self.contents[name] @@ -1448,6 +1479,7 @@ def fetchIntersphinxInventories(self, cache: CacheT) -> None: self.intersphinx.update(cache, url) def defaultPostProcess(system:'System') -> None: + for cls in system.objectsOfType(Class): # Initiate the MROs cls._init_mro() diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index b32a474f7..dfb7ac58e 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2720,3 +2720,151 @@ def test_typealias_unstring(systemcls: Type[model.System]) -> None: # there is not Constant nodes in the type alias anymore next(n for n in ast.walk(typealias.value) if isinstance(n, ast.Constant)) +@systemcls_param +def test_module_imports(systemcls: Type[model.System]) -> None: + code = ''' + import mod2 + import pack.subpack + import pack.subpack as a + from mod2 import _k as k, _l as l, _m as m + from pack.subpack.stuff import C + from x import * + ''' + expected = [('mod2','mod2', None), + ('pack','pack', None), + ('a','pack.subpack', None), + ('k','mod2','_k'), + ('l','mod2','_l'), + ('m','mod2','_m'), + ('C','pack.subpack.stuff','C'), + ('*','x', '*')] + mod = fromText(code, systemcls=systemcls) + + assert len(expected)==len(mod.imports) + for i, exp in zip(mod.imports, expected): + assert isinstance(i, model.Import) + + expected_name, expected_orgmodule, expected_orgname = exp + assert i.name == expected_name + assert i.orgmodule == expected_orgmodule + assert i.orgname == expected_orgname + +@systemcls_param +def test_module_relative_imports(systemcls: Type[model.System]) -> None: + code = ''' + from ..mod2 import bar as b + from .pack import foo + ''' + expected = [('b','top.mod2','bar'), + ('foo','top.subpack.pack','foo'),] + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString('', modname='top', is_package=True) + builder.addModuleString('', modname='subpack', parent_name='top', is_package=True) + builder.addModuleString(code, modname='other', parent_name='top.subpack') + builder.buildModules() + mod = system.allobjects['top.subpack.other'] + assert isinstance(mod, model.Module) + assert len(expected)==len(mod.imports) + for i, exp in zip(mod.imports, expected): + assert isinstance(i, model.Import) + + expected_name, expected_orgmodule, expected_orgname = exp + assert i.name == expected_name + assert i.orgmodule == expected_orgmodule + assert i.orgname == expected_orgname + +@systemcls_param +def test_module_relative_package_imports(systemcls: Type[model.System]) -> None: + code = ''' + from ...mod2 import bar as b + from .pack import foo + ''' + expected = [('b','top.mod2','bar'), + ('foo','top.subpack.other.pack','foo'),] + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString('', modname='top', is_package=True) + builder.addModuleString('', modname='subpack', parent_name='top', is_package=True) + builder.addModuleString(code, modname='other', parent_name='top.subpack', is_package=True) + builder.buildModules() + mod = system.allobjects['top.subpack.other'] + assert isinstance(mod, model.Module) + assert len(expected)==len(mod.imports) + for i, exp in zip(mod.imports, expected): + assert isinstance(i, model.Import) + + expected_name, expected_orgmodule, expected_orgname = exp + assert i.name == expected_name + assert i.orgmodule == expected_orgmodule + assert i.orgname == expected_orgname + +@systemcls_param +def test_allobjects_mapping_reparented_confusion(systemcls: Type[model.System], capsys:CapSys) -> None: + """ + When reparenting, it takes care to handle duplicte objects with system.handleDuplicate. + """ + src1 = '''\ + class mything: + "reparented" + class stuff: + do = object() + ''' + mything_src = '''\ + class stuff: + "doc" + def do(x:int):... + ''' + pack = 'from ._src import mything; __all__=["mything"]' + + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString(pack, 'pack', is_package=True) + builder.addModuleString(src1, '_src', parent_name='pack') + builder.addModuleString(mything_src, 'mything', parent_name='pack') + builder.buildModules() + + assert [(o.name,o.kind) for o in + system.allobjects['pack'].contents.values()] == [('_src', model.DocumentableKind.MODULE), + # ('mything 0', model.DocumentableKind.MODULE), + ('mything', model.DocumentableKind.CLASS)] + + assert system.allobjects['pack.mything'].docstring == "reparented" + + assert system.allobjects['pack.mything.stuff'].docstring == None + assert system.allobjects['pack.mything.stuff.do'].kind == model.DocumentableKind.CLASS_VARIABLE + + assert system.allobjects['pack.mything 0.stuff'].docstring == "doc" + assert system.allobjects['pack.mything 0.stuff'].kind == model.DocumentableKind.CLASS + assert system.allobjects['pack.mything 0.stuff.do'].kind == model.DocumentableKind.METHOD + + assert capsys.readouterr().out == ( + "moving 'pack._src.mything' into 'pack'\n" + "pack.mything:???: duplicate Module 'pack.mything'\n" + "pack._src:1: introduced by re-exporting Class 'pack._src.mything' into Package 'pack'\n" + ) + +@systemcls_param +def test_cannot_resolve_reparented(systemcls: Type[model.System], capsys:CapSys) -> None: + """ + When reparenting, it warns when the reparented target cannot be found + """ + src1 = '''\ + class Cls:... + ''' + mything_src = '''\ + class Slc:... + ''' + pack = 'from ._src2 import Slc;from ._src1 import Cls; __all__=["Cls", "Slc"]' + + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString(pack, 'pack', is_package=True) + builder.addModuleString(src1, '_src0', parent_name='pack') + builder.addModuleString(mything_src, '_src1', parent_name='pack') + builder.buildModules() + + assert list(system.allobjects['pack'].contents) == ['_src0', '_src1'] + + assert capsys.readouterr().out == (#"pack:1: cannot resolve origin module of re-exported name: 'Slc'from origin module 'pack._src2'\n" + "pack:???: cannot resolve re-exported name: 'pack._src1.Cls'\n") \ No newline at end of file From cd764149a5bd1ca6ab3159ef0387afc8bdcacd42 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 29 Sep 2023 20:32:09 -0400 Subject: [PATCH 14/23] Move the re exporting to post processing --- pydoctor/astbuilder.py | 183 ++++++++++++++++++++----------- pydoctor/model.py | 47 +------- pydoctor/test/test_astbuilder.py | 4 +- 3 files changed, 128 insertions(+), 106 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 2b0c5a705..a4ad88679 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -166,7 +166,14 @@ def extract_final_subscript(annotation: ast.Subscript) -> ast.expr: assert isinstance(ann_slice, ast.expr) return ann_slice -def getPublicNames(mod:'model.Module') -> Collection[str]: +def getModuleExports(mod: model.Module) -> Collection[str]: + # Fetch names to export. + exports = mod.all + if exports is None: + exports = [] + return exports + +def getPublicNames(mod: model.Module) -> Collection[str]: """ Get all names to import when wildcardm importing the given module: use __all__ if available, otherwise take all names that are not private. @@ -181,6 +188,112 @@ def getPublicNames(mod:'model.Module') -> Collection[str]: ] return names +# post-processes + +def _handleReExport(new_parent: 'model.Module', + origin_name: str, + as_name: str, + origin_module: model.Module, + linenumber: int) -> None: + """ + Move re-exported objects into module C{new_parent}. + """ + modname = origin_module.fullName() + + # In case of duplicates names, we can't rely on resolveName, + # So we use content.get first to resolve non-alias names. + ob = origin_module.contents.get(origin_name) or origin_module.resolveName(origin_name) + if ob is None: + new_parent.report("cannot resolve re-exported name: " + f'\'{modname}.{origin_name}\'', + lineno_offset=linenumber) + else: + if origin_module.all is None or origin_name not in origin_module.all: + if as_name != ob.name: + new_parent.system.msg( + "astbuilder", + f"moving {ob.fullName()!r} into {new_parent.fullName()!r} as {as_name!r}") + else: + new_parent.system.msg( + "astbuilder", + f"moving {ob.fullName()!r} into {new_parent.fullName()!r}") + ob.reparent(new_parent, as_name) + else: + new_parent.system.msg( + "astbuilder", + f"not moving {ob.fullName()} into {new_parent.fullName()}, " + f"because {origin_name!r} is already exported in {modname}.__all__") + +def processReExports(system: model.System) -> None: + for mod in tuple(system.objectsOfType(model.Module)): + exports = getModuleExports(mod) + for imported_name in mod.imports: + local_name = imported_name.name + orgname = imported_name.orgname + orgmodule = imported_name.orgmodule + if local_name != '*' and (not orgname or local_name not in exports): + continue + + origin = system.modules.get(orgmodule) + if origin is None: + if orgmodule.split('.', 1)[0] in system.root_names: + msg = f"cannot resolve origin module of re-exported name: {orgname or local_name!r}" + if orgname and local_name!=orgname: + msg += f" as {local_name!r}" + msg += f"from origin module {imported_name.orgmodule!r}" + mod.report(msg, lineno_offset=imported_name.linenumber) + elif isinstance(origin, model.Module): + if local_name != '*': + if orgname: + # only 'import from' statements can be used in re-exporting currently. + _handleReExport(mod, orgname, local_name, origin, + linenumber=imported_name.linenumber) + else: + for n in getPublicNames(origin): + if n in exports: + _handleReExport(mod, n, n, origin, + linenumber=imported_name.linenumber) + else: + msg = f"origin module {imported_name.orgmodule!r} should be Module, got {type(origin).__name__}" + mod.report(msg, lineno_offset=imported_name.linenumber) + + +def postProcessClasses(system: model.System) -> None: + for cls in system.objectsOfType(model.Class): + # Initiate the MROs + cls._init_mro() + # Lookup of constructors + cls._init_constructors() + + # Compute subclasses + for b in cls.baseobjects: + if b is not None: + b.subclasses.append(cls) + + # Checking whether the class is an exception + if model.is_exception(cls): + cls.kind = model.DocumentableKind.EXCEPTION + +def postProcessAttributes(system:model.System) -> None: + for attrib in system.objectsOfType(model.Attribute): + _inherits_instance_variable_kind(attrib) + +def _inherits_instance_variable_kind(attr: model.Attribute) -> None: + """ + If any of the inherited members of a class variable is an instance variable, + then the subclass' class variable become an instance variable as well. + """ + if attr.kind is not model.DocumentableKind.CLASS_VARIABLE: + return + docsources = attr.docsources() + next(docsources) + for inherited in docsources: + if inherited.kind is model.DocumentableKind.INSTANCE_VARIABLE: + attr.kind = model.DocumentableKind.INSTANCE_VARIABLE + break + +# main ast visitor + class ModuleVistor(NodeVisitor): def __init__(self, builder: 'ASTBuilder', module: model.Module): @@ -346,10 +459,11 @@ def visit_ImportFrom(self, node: ast.ImportFrom) -> None: def _importAll(self, modname: str, linenumber:int) -> None: """Handle a C{from import *} statement.""" - ctx = self.builder.current if isinstance(ctx, model.Module): - ctx.imports.append(model.Import('*', modname, linenumber=linenumber, orgname='*')) + ctx.imports.append(model.Import('*', modname, + linenumber=linenumber, orgname='*')) + mod = self.system.getProcessedModule(modname) if mod is None: # We don't have any information about the module, so we don't know @@ -358,71 +472,19 @@ def _importAll(self, modname: str, linenumber:int) -> None: return ctx.report(f"import * from {modname}", thresh=1) - # Fetch names to export. - exports = self._getCurrentModuleExports() - # Add imported names to our module namespace. assert isinstance(self.builder.current, model.CanContainImportsDocumentable) _localNameToFullName = self.builder.current._localNameToFullName_map expandName = mod.expandName for name in getPublicNames(mod): - - if self._handleReExport(exports, name, name, mod) is True: - continue - _localNameToFullName[name] = expandName(name) - def _getCurrentModuleExports(self) -> Collection[str]: - # Fetch names to export. - current = self.builder.current - if isinstance(current, model.Module): - exports = current.all - if exports is None: - exports = [] - else: - # Don't export names imported inside classes or functions. - exports = [] - return exports - - def _handleReExport(self, curr_mod_exports:Collection[str], - origin_name:str, as_name:str, - origin_module:model.Module) -> bool: - """ - Move re-exported objects into current module. - - @returns: True if the imported name has been sucessfully re-exported. - """ - # Move re-exported objects into current module. - current = self.builder.current - modname = origin_module.fullName() - if as_name in curr_mod_exports: - # In case of duplicates names, we can't rely on resolveName, - # So we use content.get first to resolve non-alias names. - ob = origin_module.contents.get(origin_name) or origin_module.resolveName(origin_name) - if ob is None: - current.report("cannot resolve re-exported name: " - f"'{modname}.{origin_name}'", thresh=1) - else: - if origin_module.all is None or origin_name not in origin_module.all: - self.system.msg( - "astbuilder", - "moving %r into %r" % (ob.fullName(), current.fullName()) - ) - # Must be a Module since the exports is set to an empty list if it's not. - assert isinstance(current, model.Module) - ob.reparent(current, as_name) - return True - return False - def _importNames(self, modname: str, names: Iterable[ast.alias], linenumber:int) -> None: """Handle a C{from import } statement.""" # Process the module we're importing from. mod = self.system.getProcessedModule(modname) - # Fetch names to export. - exports = self._getCurrentModuleExports() - current = self.builder.current assert isinstance(current, model.CanContainImportsDocumentable) _localNameToFullName = current._localNameToFullName_map @@ -433,19 +495,16 @@ def _importNames(self, modname: str, names: Iterable[ast.alias], linenumber:int) if asname is None: asname = orgname - if mod is not None and self._handleReExport(exports, orgname, asname, mod) is True: - continue - # If we're importing from a package, make sure imported modules # are processed (getProcessedModule() ignores non-modules). if isinstance(mod, model.Package): self.system.getProcessedModule(f'{modname}.{orgname}') - + _localNameToFullName[asname] = f'{modname}.{orgname}' if is_module: cast(model.Module, current).imports.append(model.Import(asname, modname, - orgname=orgname, linenumber=linenumber)) + orgname=orgname, linenumber=linenumber)) def visit_Import(self, node: ast.Import) -> None: """Process an import statement. @@ -1311,4 +1370,6 @@ def parseDocformat(node: ast.Assign, mod: model.Module) -> None: def setup_pydoctor_extension(r:extensions.ExtRegistrar) -> None: r.register_astbuilder_visitor(TypeAliasVisitorExt) - r.register_post_processor(model.defaultPostProcess, priority=200) + r.register_post_processor(processReExports, priority=250) + r.register_post_processor(postProcessClasses, priority=200) + r.register_post_processor(postProcessAttributes, priority=200) diff --git a/pydoctor/model.py b/pydoctor/model.py index 4c588eb7f..5173a3dc2 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -52,13 +52,6 @@ # Functions 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 @@ -251,7 +244,6 @@ def reparent(self, new_parent: 'Module', new_name: str) -> None: # issue warnings if new_name in new_contents: - self.system.handleDuplicate(new_contents[new_name]) self.report(f"introduced by re-exporting {self} into {new_parent}" '' if new_name==old_name else f' as {new_name!r}', thresh=1) @@ -945,6 +937,7 @@ class System: """ def __init__(self, options: Optional['Options'] = None): + self.modules: Dict[str, Module] = {} self.allobjects: Dict[str, Documentable] = {} self.rootobjects: List[_ModuleT] = [] @@ -1165,7 +1158,9 @@ def privacyClass(self, ob: Documentable) -> PrivacyClass: def addObject(self, obj: Documentable) -> None: """Add C{object} to the system.""" - + if isinstance(obj, _ModuleT): + # we already handled duplication of modules. + self.modules[obj.fullName()] = obj if obj.parent: obj.parent.contents[obj.name] = obj elif isinstance(obj, _ModuleT): @@ -1478,40 +1473,6 @@ def fetchIntersphinxInventories(self, cache: CacheT) -> None: for url in self.options.intersphinx: self.intersphinx.update(cache, url) -def defaultPostProcess(system:'System') -> None: - - for cls in system.objectsOfType(Class): - # Initiate the MROs - cls._init_mro() - # Lookup of constructors - cls._init_constructors() - - # Compute subclasses - for b in cls.baseobjects: - if b is not None: - b.subclasses.append(cls) - - # Checking whether the class is an exception - if is_exception(cls): - cls.kind = DocumentableKind.EXCEPTION - - for attrib in system.objectsOfType(Attribute): - _inherits_instance_variable_kind(attrib) - -def _inherits_instance_variable_kind(attr: Attribute) -> None: - """ - If any of the inherited members of a class variable is an instance variable, - then the subclass' class variable become an instance variable as well. - """ - if attr.kind is not DocumentableKind.CLASS_VARIABLE: - return - docsources = attr.docsources() - next(docsources) - for inherited in docsources: - if inherited.kind is DocumentableKind.INSTANCE_VARIABLE: - attr.kind = DocumentableKind.INSTANCE_VARIABLE - break - def get_docstring( obj: Documentable ) -> Tuple[Optional[str], Optional[Documentable]]: diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index dfb7ac58e..7636a21e1 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2866,5 +2866,5 @@ class Slc:... assert list(system.allobjects['pack'].contents) == ['_src0', '_src1'] - assert capsys.readouterr().out == (#"pack:1: cannot resolve origin module of re-exported name: 'Slc'from origin module 'pack._src2'\n" - "pack:???: cannot resolve re-exported name: 'pack._src1.Cls'\n") \ No newline at end of file + assert capsys.readouterr().out == ("pack:1: cannot resolve origin module of re-exported name: 'Slc'from origin module 'pack._src2'\n" + "pack:1: cannot resolve re-exported name: 'pack._src1.Cls'\n") \ No newline at end of file From a3ad4eb2ccc14db52f10c515d3279b83540c2d85 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 29 Sep 2023 20:41:19 -0400 Subject: [PATCH 15/23] Fix mypy and simplify --- pydoctor/astbuilder.py | 24 ++++++++++-------------- pydoctor/model.py | 1 - 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index a4ad88679..dce2bd3a4 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -242,21 +242,17 @@ def processReExports(system: model.System) -> None: msg += f" as {local_name!r}" msg += f"from origin module {imported_name.orgmodule!r}" mod.report(msg, lineno_offset=imported_name.linenumber) - elif isinstance(origin, model.Module): - if local_name != '*': - if orgname: - # only 'import from' statements can be used in re-exporting currently. - _handleReExport(mod, orgname, local_name, origin, - linenumber=imported_name.linenumber) - else: - for n in getPublicNames(origin): - if n in exports: - _handleReExport(mod, n, n, origin, - linenumber=imported_name.linenumber) + elif local_name != '*': + if orgname: + # only 'import from' statements can be used in re-exporting currently. + _handleReExport(mod, orgname, local_name, origin, + linenumber=imported_name.linenumber) else: - msg = f"origin module {imported_name.orgmodule!r} should be Module, got {type(origin).__name__}" - mod.report(msg, lineno_offset=imported_name.linenumber) - + for n in getPublicNames(origin): + if n in exports: + _handleReExport(mod, n, n, origin, + linenumber=imported_name.linenumber) + def postProcessClasses(system: model.System) -> None: for cls in system.objectsOfType(model.Class): diff --git a/pydoctor/model.py b/pydoctor/model.py index 5173a3dc2..ee762fe97 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -12,7 +12,6 @@ from collections import defaultdict import datetime import importlib -import platform import sys import textwrap import types From 8d7b7483b44e7b4a2815e5357747078793d733f6 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Sat, 30 Sep 2023 19:26:39 -0400 Subject: [PATCH 16/23] Fix issue #184. Introduce. Module.elsewhere_contents --- .gitignore | 2 + docs/tests/test_twisted_docs.py | 7 +- pydoctor/astbuilder.py | 204 +++++++++++++++------- pydoctor/model.py | 7 + pydoctor/templatewriter/pages/__init__.py | 49 ++++-- pydoctor/templatewriter/pages/table.py | 37 +++- pydoctor/test/test_astbuilder.py | 116 +++++++++++- pydoctor/test/test_packages.py | 6 +- pydoctor/test/test_templatewriter.py | 59 +++++++ 9 files changed, 397 insertions(+), 90 deletions(-) diff --git a/.gitignore b/.gitignore index cfa1c1303..0d881acd0 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,5 @@ _trial_temp/ apidocs/ *.egg-info .eggs +__pycache__ +.hypothesis \ No newline at end of file diff --git a/docs/tests/test_twisted_docs.py b/docs/tests/test_twisted_docs.py index 339802219..2a5af1386 100644 --- a/docs/tests/test_twisted_docs.py +++ b/docs/tests/test_twisted_docs.py @@ -25,10 +25,10 @@ def test_IPAddress_implementations() -> None: assert all(impl in page for impl in show_up), page # Test for https://github.com/twisted/pydoctor/issues/505 -def test_web_template_api() -> None: +def test_some_apis() -> None: """ This test ensures all important members of the twisted.web.template - module are documented at the right place + module are documented at the right place, and other APIs exist as well. """ exists = ['twisted.web.template.Tag.html', @@ -39,7 +39,8 @@ def test_web_template_api() -> None: 'twisted.web.template.TagLoader.html', 'twisted.web.template.XMLString.html', 'twisted.web.template.XMLFile.html', - 'twisted.web.template.Element.html',] + 'twisted.web.template.Element.html', + 'twisted.internet.ssl.DistinguishedName.html'] for e in exists: assert (BASE_DIR / e).exists(), f"{e} not found" diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index dce2bd3a4..e3038e381 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -1,6 +1,7 @@ """Convert ASTs into L{pydoctor.model.Documentable} instances.""" import ast +from collections import defaultdict import sys from functools import partial @@ -12,14 +13,14 @@ Type, TypeVar, Union, cast ) +import attr import astor from pydoctor import epydoc2stan, model, node2stan, extensions, linker 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, - is__name__equals__main__, unstring_annotation, iterassign, extract_docstring_linenum, infer_type, get_parents, + is__name__equals__main__, unstring_annotation, iterassign, extract_docstring_linenum, get_parents, infer_type, NodeVisitor, Parentage) - def parseFile(path: Path) -> ast.Module: """Parse the contents of a Python source file.""" with open(path, 'rb') as f: @@ -166,7 +167,53 @@ def extract_final_subscript(annotation: ast.Subscript) -> ast.expr: assert isinstance(ann_slice, ast.expr) return ann_slice -def getModuleExports(mod: model.Module) -> Collection[str]: +def _resolveReExportTarget(origin_module:model.Module, origin_name:str, + new_parent:model.Module, linenumber:int) -> Optional[model.Documentable]: + # In case of duplicates names, we can't rely on resolveName, + # So we use content.get first to resolve non-alias names. + ob = origin_module.contents.get(origin_name) or origin_module.resolveName(origin_name) + if ob is None: + new_parent.report("cannot resolve re-exported name: " + f'\'{origin_module.fullName()}.{origin_name}\'', lineno_offset=linenumber) + return ob + +def _handleReExport(info:'ReExport', elsewhere:Collection['ReExport']) -> None: + """ + Move re-exported objects into module C{new_parent}. + """ + new_parent = info.new_parent + target = info.target + as_name = info.as_name + target_parent = target.parent + assert isinstance(target_parent, model.Module) + + if as_name != target.name: + new_parent.system.msg( + "astbuilder", + f"moving {target.fullName()!r} into {new_parent.fullName()!r} as {as_name!r}") + else: + new_parent.system.msg( + "astbuilder", + f"moving {target.fullName()!r} into {new_parent.fullName()!r}") + + # Remember that this name is re-exported + target_parent.elsewhere_contents[target.name] = target + for e in elsewhere: + new_parent.system.msg( + "astbuilder", + f"also available at '{e.new_parent.fullName()}.{e.as_name}'") + e.new_parent.elsewhere_contents[e.as_name] = target + + target.reparent(new_parent, as_name) + + # if origin_module.all is None or origin_name not in origin_module.all: + # else: + # new_parent.system.msg( + # "astbuilder", + # f"not moving {target.fullName()} into {new_parent.fullName()}, " + # f"because {origin_name!r} is already exported in {modname}.__all__") + +def getModuleExports(mod:'model.Module') -> Collection[str]: # Fetch names to export. exports = mod.all if exports is None: @@ -188,44 +235,36 @@ def getPublicNames(mod: model.Module) -> Collection[str]: ] return names -# post-processes - -def _handleReExport(new_parent: 'model.Module', - origin_name: str, - as_name: str, - origin_module: model.Module, - linenumber: int) -> None: - """ - Move re-exported objects into module C{new_parent}. - """ - modname = origin_module.fullName() - - # In case of duplicates names, we can't rely on resolveName, - # So we use content.get first to resolve non-alias names. - ob = origin_module.contents.get(origin_name) or origin_module.resolveName(origin_name) - if ob is None: - new_parent.report("cannot resolve re-exported name: " - f'\'{modname}.{origin_name}\'', - lineno_offset=linenumber) - else: - if origin_module.all is None or origin_name not in origin_module.all: - if as_name != ob.name: - new_parent.system.msg( - "astbuilder", - f"moving {ob.fullName()!r} into {new_parent.fullName()!r} as {as_name!r}") - else: - new_parent.system.msg( - "astbuilder", - f"moving {ob.fullName()!r} into {new_parent.fullName()!r}") - ob.reparent(new_parent, as_name) - else: - new_parent.system.msg( - "astbuilder", - f"not moving {ob.fullName()} into {new_parent.fullName()}, " - f"because {origin_name!r} is already exported in {modname}.__all__") +@attr.s(auto_attribs=True, slots=True) +class ReExport: + new_parent: model.Module + as_name: str + origin_module: model.Module + target: model.Documentable + + +def _maybeExistingNameOverridesImport(mod:model.Module, local_name:str, + imp:model.Import, target:model.Documentable) -> bool: + if local_name in mod.contents: + existing = mod.contents[local_name] + # The imported name already exists in the locals, we test the linenumbers to + # know whether the import should override the local name. We could do better if + # integrate with better static analysis like def-use chains. + if (not isinstance(existing, model.Module) and # modules are always shadowed by members + mod.contents[local_name].linenumber > imp.linenumber): + mod.report(f"not moving {target.fullName()} into {mod.fullName()}, " + f"because {local_name!r} is defined at line {existing.linenumber}", + lineno_offset=imp.linenumber, + thresh=-1) + return True + return False -def processReExports(system: model.System) -> None: - for mod in tuple(system.objectsOfType(model.Module)): +def processReExports(system:'model.System') -> None: + # first gather all export infos, clean them up + # and apply them at the end. + reexports: List[ReExport] = [] + + for mod in system.objectsOfType(model.Module): exports = getModuleExports(mod) for imported_name in mod.imports: local_name = imported_name.name @@ -233,26 +272,55 @@ def processReExports(system: model.System) -> None: orgmodule = imported_name.orgmodule if local_name != '*' and (not orgname or local_name not in exports): continue - - origin = system.modules.get(orgmodule) - if origin is None: - if orgmodule.split('.', 1)[0] in system.root_names: - msg = f"cannot resolve origin module of re-exported name: {orgname or local_name!r}" - if orgname and local_name!=orgname: - msg += f" as {local_name!r}" - msg += f"from origin module {imported_name.orgmodule!r}" - mod.report(msg, lineno_offset=imported_name.linenumber) - elif local_name != '*': - if orgname: + origin = system.modules.get(orgmodule) or system.allobjects.get(orgmodule) + if isinstance(origin, model.Module): + if local_name != '*': # only 'import from' statements can be used in re-exporting currently. - _handleReExport(mod, orgname, local_name, origin, - linenumber=imported_name.linenumber) - else: - for n in getPublicNames(origin): - if n in exports: - _handleReExport(mod, n, n, origin, - linenumber=imported_name.linenumber) - + if orgname: + target = _resolveReExportTarget(origin, orgname, + mod, imported_name.linenumber) + if target: + if _maybeExistingNameOverridesImport(mod, local_name, imported_name, target): + continue + reexports.append( + ReExport(mod, local_name, origin, target) + ) + else: + for n in getPublicNames(origin): + if n in exports: + target = _resolveReExportTarget(origin, n, mod, imported_name.linenumber) + if target: + if _maybeExistingNameOverridesImport(mod, n, imported_name, target): + continue + reexports.append( + ReExport(mod, n, origin, target) + ) + elif orgmodule.split('.', 1)[0] in system.root_names: + msg = f"cannot resolve origin module of re-exported name: {orgname or local_name!r}" + if orgname and local_name!=orgname: + msg += f" as {local_name!r}" + msg += f"from origin module {imported_name.orgmodule!r}" + mod.report(msg, lineno_offset=imported_name.linenumber) + + exports_per_target:Dict[model.Documentable, List[ReExport]] = defaultdict(list) + for r in reexports: + exports_per_target[r.target].append(r) + + for target, _exports in exports_per_target.items(): + elsewhere = [] + assert len(_exports) > 0 + if len(_exports) > 1: + # when an object has several re-exports, the module with the lowest number + # of dot in it's name is choosen, if there is an equality, the longer local name + # if choosen + # TODO: move this into a system method + # TODO: do not move objects inside a private module + # TODO: do not move objects when they are listed in __all__ of a public module + _exports.sort(key=lambda r:(r.new_parent.fullName().count('.'), -len(r.as_name))) + elsewhere.extend(_exports[1:]) + + reexport = _exports[0] + _handleReExport(reexport, elsewhere) def postProcessClasses(system: model.System) -> None: for cls in system.objectsOfType(model.Class): @@ -464,15 +532,22 @@ def _importAll(self, modname: str, linenumber:int) -> None: if mod is None: # We don't have any information about the module, so we don't know # what names to import. - ctx.report(f"import * from unknown {modname}", thresh=1) + ctx.report(f"import * from unknown module {modname!r}", thresh=1, lineno_offset=linenumber) return - ctx.report(f"import * from {modname}", thresh=1) + + if mod.state is model.ProcessingState.PROCESSING: + ctx.report(f"import * from partially processed module {modname!r}", + thresh=1, lineno_offset=linenumber) + + # Get names to import: use __all__ if available, otherwise take all + # names that are not private. + names = getPublicNames(mod) # Add imported names to our module namespace. - assert isinstance(self.builder.current, model.CanContainImportsDocumentable) - _localNameToFullName = self.builder.current._localNameToFullName_map + assert isinstance(ctx, model.CanContainImportsDocumentable) + _localNameToFullName = ctx._localNameToFullName_map expandName = mod.expandName - for name in getPublicNames(mod): + for name in names: _localNameToFullName[name] = expandName(name) def _importNames(self, modname: str, names: Iterable[ast.alias], linenumber:int) -> None: @@ -485,7 +560,6 @@ def _importNames(self, modname: str, names: Iterable[ast.alias], linenumber:int) assert isinstance(current, model.CanContainImportsDocumentable) _localNameToFullName = current._localNameToFullName_map is_module = isinstance(current, model.Module) - for al in names: orgname, asname = al.name, al.asname if asname is None: diff --git a/pydoctor/model.py b/pydoctor/model.py index ee762fe97..b6823cee9 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -478,6 +478,13 @@ def setup(self) -> None: self._docformat: Optional[str] = None self.imports: List[Import] = [] + self.elsewhere_contents: Dict[str, 'Documentable'] = {} + """ + When pydoctor re-export objects, it leaves references to object in this dict + so they can still be listed in childtable of origin modules. This attribute belongs + to the "view model" part of Documentable interface and should only be used to present + links to these objects, nor to do any name resolving. + """ def _localNameToFullName(self, name: str) -> str: if name in self.contents: diff --git a/pydoctor/templatewriter/pages/__init__.py b/pydoctor/templatewriter/pages/__init__.py index 272239605..6d73fbffa 100644 --- a/pydoctor/templatewriter/pages/__init__.py +++ b/pydoctor/templatewriter/pages/__init__.py @@ -1,7 +1,8 @@ """The classes that turn L{Documentable} instances into objects we can render.""" +from itertools import chain from typing import ( - TYPE_CHECKING, Dict, Iterator, List, Optional, Mapping, Sequence, + TYPE_CHECKING, Callable, Dict, Iterator, List, Optional, Mapping, Sequence, Tuple, Type, Union ) import ast @@ -296,11 +297,20 @@ def extras(self) -> List["Flattenable"]: def docstring(self) -> "Flattenable": return self.docgetter.get(self.ob) + + def _childtable_objects_order(self, + v:Union[model.Documentable, Tuple[str, model.Documentable]]) -> Tuple[int, int, str]: + if isinstance(v, model.Documentable): + return util.objects_order(v) + else: + name, o = v + i,j,_ = util.objects_order(o) + return (i,j, f'{self.ob.fullName()}.{name}'.lower()) - def children(self) -> Sequence[model.Documentable]: + def children(self) -> Sequence[Union[model.Documentable, Tuple[str, model.Documentable]]]: return sorted( (o for o in self.ob.contents.values() if o.isVisible), - key=util.objects_order) + key=self._childtable_objects_order) def packageInitTable(self) -> "Flattenable": return () @@ -380,7 +390,6 @@ def slot_map(self) -> Dict[str, "Flattenable"]: ) return slot_map - class ModulePage(CommonPage): ob: model.Module @@ -393,17 +402,35 @@ def extras(self) -> List["Flattenable"]: r.extend(super().extras()) return r + + def _iter_reexported_members(self, predicate: Optional[Callable[[model.Documentable], bool]]=None) -> Iterator[Tuple[str, model.Documentable]]: + if not predicate: + predicate = lambda v:True + return ((n,o) for n,o in self.ob.elsewhere_contents.items() if o.isVisible and predicate(o)) + def children(self) -> Sequence[Union[model.Documentable, Tuple[str, model.Documentable]]]: + return sorted(chain( + super().children(), self._iter_reexported_members()), + key=self._childtable_objects_order) -class PackagePage(ModulePage): - def children(self) -> Sequence[model.Documentable]: - return sorted(self.ob.submodules(), key=objects_order) - def packageInitTable(self) -> "Flattenable": - children = sorted( - (o for o in self.ob.contents.values() +class PackagePage(ModulePage): + def children(self) -> Sequence[Union[model.Documentable, Tuple[str, model.Documentable]]]: + return sorted(chain(self.ob.submodules(), + self._iter_reexported_members( + predicate=lambda o: isinstance(o, model.Module))), + key=self._childtable_objects_order) + + def initTableChildren(self) -> Sequence[Union[model.Documentable, Tuple[str, model.Documentable]]]: + return sorted( + chain((o for o in self.ob.contents.values() if not isinstance(o, model.Module) and o.isVisible), - key=util.objects_order) + self._iter_reexported_members( + predicate=lambda o: not isinstance(o, model.Module))), + key=self._childtable_objects_order) + + def packageInitTable(self) -> "Flattenable": + children = self.initTableChildren() if children: loader = ChildTable.lookup_loader(self.template_lookup) return [ diff --git a/pydoctor/templatewriter/pages/table.py b/pydoctor/templatewriter/pages/table.py index 0099eb7df..ed6076121 100644 --- a/pydoctor/templatewriter/pages/table.py +++ b/pydoctor/templatewriter/pages/table.py @@ -1,10 +1,10 @@ -from typing import TYPE_CHECKING, Collection +from typing import TYPE_CHECKING, Collection, Optional, Tuple, Union, overload from twisted.web.iweb import ITemplateLoader from twisted.web.template import Element, Tag, TagLoader, renderer, tags from pydoctor import epydoc2stan -from pydoctor.model import Documentable, Function +from pydoctor.model import Documentable, Function, Class from pydoctor.templatewriter import TemplateElement, util if TYPE_CHECKING: @@ -18,16 +18,18 @@ def __init__(self, docgetter: util.DocGetter, ob: Documentable, child: Documentable, + as_name:Optional[str] ): super().__init__(loader) self.docgetter = docgetter self.ob = ob self.child = child + self.as_name = as_name @renderer def class_(self, request: object, tag: Tag) -> "Flattenable": class_ = util.css_class(self.child) - if self.child.parent is not self.ob: + if isinstance(self.ob, Class) and self.child.parent is not self.ob: class_ = 'base' + class_ return class_ @@ -47,8 +49,9 @@ def kind(self, request: object, tag: Tag) -> Tag: @renderer def name(self, request: object, tag: Tag) -> Tag: return tag.clear()(tags.code( - epydoc2stan.taglink(self.child, self.ob.url, epydoc2stan.insert_break_points(self.child.name)) - )) + epydoc2stan.taglink(self.child, self.ob.url, + epydoc2stan.insert_break_points( + self.as_name or self.child.name)))) @renderer def summaryDoc(self, request: object, tag: Tag) -> Tag: @@ -61,11 +64,28 @@ class ChildTable(TemplateElement): filename = 'table.html' + # not really a legit usage of overload, but mypy made me do it. + @overload def __init__(self, docgetter: util.DocGetter, ob: Documentable, children: Collection[Documentable], loader: ITemplateLoader, + ):... + @overload + def __init__(self, + docgetter: util.DocGetter, + ob: Documentable, + children: Collection[Union[Documentable, + Tuple[str, Documentable]]], + loader: ITemplateLoader, + ):... + def __init__(self, + docgetter: util.DocGetter, + ob: Documentable, + children: Collection[Union[Documentable, + Tuple[str, Documentable]]], + loader: ITemplateLoader, ): super().__init__(loader) self.children = children @@ -85,7 +105,8 @@ def rows(self, request: object, tag: Tag) -> "Flattenable": TagLoader(tag), self.docgetter, self.ob, - child) + child=child if isinstance(child, Documentable) else child[1], + as_name=None if isinstance(child, Documentable) else child[0]) for child in self.children - if child.isVisible - ] + if (child if isinstance(child, Documentable) else child[1]).isVisible + ] diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 7636a21e1..5668e986e 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2867,4 +2867,118 @@ class Slc:... assert list(system.allobjects['pack'].contents) == ['_src0', '_src1'] assert capsys.readouterr().out == ("pack:1: cannot resolve origin module of re-exported name: 'Slc'from origin module 'pack._src2'\n" - "pack:1: cannot resolve re-exported name: 'pack._src1.Cls'\n") \ No newline at end of file + "pack:1: cannot resolve re-exported name: 'pack._src1.Cls'\n") + +@systemcls_param +def test_reparenting_from_module_that_defines__all__(systemcls: Type[model.System], capsys:CapSys) -> None: + """ + Even if a module defined it's own __all__ attribute, + we can reparent it's direct children to a new module, + but only if it's a private module. + """ + src = '''\ + class cls:... + __all__ = ['cls'] + ''' + pack = 'from ._src import cls; __all__=["cls"]' + + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString(pack, 'pack', is_package=True) + builder.addModuleString(src, '_src', parent_name='pack') + builder.buildModules() + assert capsys.readouterr().out == "moving 'pack._src.cls' into 'pack'\n" + assert system.allobjects['pack.cls'] is system.allobjects['pack._src'].elsewhere_contents['cls'] # type:ignore + +@systemcls_param +def test_do_not_reparent_to_existing_name(systemcls: Type[model.System], capsys:CapSys) -> None: + """ + Pydoctor will not re-export a name that is + shadowed by a local by the same name. + """ + src1 = '''\ + class Cls:... + ''' + src2 = '''\ + class Slc:... + ''' + pack = '''\ + class Slc:... + from ._src1 import Slc + from ._src import Cls + class Cls:... + __all__=["Cls", "Slc"] + ''' + + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString(pack, 'pack', is_package=True) + builder.addModuleString(src1, '_src', parent_name='pack') + builder.addModuleString(src2, '_src1', parent_name='pack') + builder.buildModules() + + assert capsys.readouterr().out == ("pack:3: not moving pack._src.Cls into pack, because 'Cls' is defined at line 4\n" + "moving 'pack._src1.Slc' into 'pack'\n" + "pack:1: duplicate Class 'pack.Slc'\n" + "pack._src1:1: introduced by re-exporting Class 'pack._src1.Slc' into Package 'pack'\n") + + assert system.allobjects['pack.Slc'] is system.allobjects['pack._src1'].elsewhere_contents['Slc'] # type:ignore + +@systemcls_param +def test_multiple_re_exports(systemcls: Type[model.System], capsys:CapSys) -> None: + """ + Pydoctor will re-export a name to the module with + the lowest amount of dots in it's fullname. + """ + src = '''\ + class Cls:... + ''' + subpack = '''\ + from pack.subpack.src import Cls + __all__=['Cls'] + ''' + pack = '''\ + from pack.subpack import Cls + __all__=["Cls"] + ''' + + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString(pack, 'pack', is_package=True) + builder.addModuleString(subpack, 'subpack', is_package=True, parent_name='pack') + builder.addModuleString(src, 'src', parent_name='pack.subpack') + builder.buildModules() + + assert capsys.readouterr().out == ("moving 'pack.subpack.src.Cls' into 'pack'\n" + "also available at 'pack.subpack.Cls'\n") + + assert system.allobjects['pack.Cls'] is system.allobjects['pack.subpack'].elsewhere_contents['Cls'] # type:ignore + assert system.allobjects['pack.Cls'] is system.allobjects['pack.subpack.src'].elsewhere_contents['Cls'] # type:ignore + +@systemcls_param +def test_multiple_re_exports_alias(systemcls: Type[model.System], capsys:CapSys) -> None: + """ + The case of twisted.internet.ssl.DistinguishedName/DN + """ + src = '''\ + class DistinguishedName:... + DN = DistinguishedName + ''' + subpack = '' + pack = ''' + from pack.subpack.src import DN, DistinguishedName as DisName + __all__=['DN', 'DisName'] + ''' + + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString(pack, 'pack', is_package=True) + builder.addModuleString(subpack, 'subpack', is_package=True, parent_name='pack') + builder.addModuleString(src, 'src', parent_name='pack.subpack') + builder.buildModules() + + assert capsys.readouterr().out == ("moving 'pack.subpack.src.DistinguishedName' into 'pack' as 'DisName'\n" + "also available at 'pack.DN'\n") + + assert system.allobjects['pack.DisName'] is system.allobjects['pack'].elsewhere_contents['DN'] # type:ignore + assert system.allobjects['pack.DisName'] is system.allobjects['pack.subpack.src'].elsewhere_contents['DistinguishedName'] # type:ignore \ No newline at end of file diff --git a/pydoctor/test/test_packages.py b/pydoctor/test/test_packages.py index fe16a9991..8223b4943 100644 --- a/pydoctor/test/test_packages.py +++ b/pydoctor/test/test_packages.py @@ -57,8 +57,10 @@ def test_allgames() -> None: assert isinstance(mod1, model.Module) mod2 = system.allobjects['allgames.mod2'] assert isinstance(mod2, model.Module) - # InSourceAll is not moved into mod2, but NotInSourceAll is. - assert 'InSourceAll' in mod1.contents + # changed in 2023 + # InSourceAll is moved into mod2 as well as NotInSourceAll. + assert 'InSourceAll' not in mod1.contents + assert 'InSourceAll' in mod2.contents assert 'NotInSourceAll' in mod2.contents # Source paths must be unaffected by the move, so that error messages # point to the right source code. diff --git a/pydoctor/test/test_templatewriter.py b/pydoctor/test/test_templatewriter.py index 64a331474..3f53094f9 100644 --- a/pydoctor/test/test_templatewriter.py +++ b/pydoctor/test/test_templatewriter.py @@ -13,6 +13,7 @@ TemplateLookup, Template, HtmlTemplate, UnsupportedTemplateVersion, OverrideTemplateNotAllowed) +from pydoctor.templatewriter.pages import PackagePage, ModulePage from pydoctor.templatewriter.pages.table import ChildTable from pydoctor.templatewriter.pages.attributechild import AttributeChild from pydoctor.templatewriter.summary import isClassNodePrivate, isPrivate, moduleSummary, ClassIndexPage @@ -816,3 +817,61 @@ class Stuff(socket): index = flatten(ClassIndexPage(mod.system, TemplateLookup(template_dir))) assert 'href="https://docs.python.org/3/library/socket.html#socket.socket"' in index +def test_multiple_re_exports_documented_elsewhere_renders() -> None: + """ + Pydoctor will leave links from the origin module. + """ + src = '''\ + class Cls:... + ''' + subpack = '''\ + from pack.subpack.src import Cls + __all__=['Cls'] + ''' + pack = '''\ + from pack.subpack import Cls + __all__=["Cls"] + ''' + + system = model.System() + builder = system.systemBuilder(system) + builder.addModuleString(pack, 'pack', is_package=True) + builder.addModuleString(subpack, 'subpack', is_package=True, parent_name='pack') + builder.addModuleString(src, 'src', parent_name='pack.subpack') + builder.buildModules() + + subpackpage = PackagePage(system.allobjects['pack.subpack'], TemplateLookup(template_dir)) + srcpage = ModulePage(system.allobjects['pack.subpack.src'], TemplateLookup(template_dir)) + assert len(subpackpage.children())==1 + assert ('Cls', system.allobjects['pack.Cls']) in subpackpage.initTableChildren() + assert ('Cls', system.allobjects['pack.Cls']) in srcpage.children() + + assert system.allobjects['pack.Cls'].url in flatten(subpackpage) + assert system.allobjects['pack.Cls'].url in flatten(srcpage) + +@systemcls_param +def test_multiple_re_exports_alias_renders_asname(systemcls: Type[model.System], capsys:CapSys) -> None: + """ + The case of twisted.internet.ssl.DistinguishedName/DN + """ + src = '''\ + class DistinguishedName:... + DN = DistinguishedName + ''' + pack = ''' + from pack.subpack.src import DN, DistinguishedName + __all__=['DN', 'DistinguishedName'] + ''' + + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString(pack, 'pack', is_package=True) + builder.addModuleString('', 'subpack', is_package=True, parent_name='pack') + builder.addModuleString(src, 'src', parent_name='pack.subpack') + builder.buildModules() + + subpackpage = PackagePage(system.allobjects['pack'], TemplateLookup(template_dir)) + html = flatten(subpackpage) + + assert system.allobjects['pack.DistinguishedName'].url in html + assert 'DN' in html From d5d176a0a37dee3349761b36f9b1d27cffc44123 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Sat, 30 Sep 2023 19:34:35 -0400 Subject: [PATCH 17/23] Do not reparent stuff listed in __all__ of a public module. Give priority to public modules in the re-export sorting. --- pydoctor/astbuilder.py | 65 +++++++++++++++++++++----------- pydoctor/test/test_astbuilder.py | 45 +++++++++++++++++----- pydoctor/test/test_packages.py | 7 ++-- 3 files changed, 81 insertions(+), 36 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index e3038e381..55320fd3d 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -2,6 +2,7 @@ import ast from collections import defaultdict +from statistics import mean import sys from functools import partial @@ -186,32 +187,32 @@ def _handleReExport(info:'ReExport', elsewhere:Collection['ReExport']) -> None: as_name = info.as_name target_parent = target.parent assert isinstance(target_parent, model.Module) + + # Remember that this name is re-exported + target_parent.elsewhere_contents[target.name] = target + + extra_msg = '' + + for e in elsewhere: + e.new_parent.elsewhere_contents[e.as_name] = target + if not extra_msg: + extra_msg += ', also available at ' + extra_msg += f"'{e.new_parent.fullName()}.{e.as_name}'" + else: + extra_msg += f" and '{e.new_parent.fullName()}.{e.as_name}'" + if as_name != target.name: new_parent.system.msg( "astbuilder", - f"moving {target.fullName()!r} into {new_parent.fullName()!r} as {as_name!r}") + f"moving {target.fullName()!r} into {new_parent.fullName()!r} as {as_name!r}{extra_msg}") else: new_parent.system.msg( "astbuilder", - f"moving {target.fullName()!r} into {new_parent.fullName()!r}") - - # Remember that this name is re-exported - target_parent.elsewhere_contents[target.name] = target - for e in elsewhere: - new_parent.system.msg( - "astbuilder", - f"also available at '{e.new_parent.fullName()}.{e.as_name}'") - e.new_parent.elsewhere_contents[e.as_name] = target - + f"moving {target.fullName()!r} into {new_parent.fullName()!r}{extra_msg}") + target.reparent(new_parent, as_name) - # if origin_module.all is None or origin_name not in origin_module.all: - # else: - # new_parent.system.msg( - # "astbuilder", - # f"not moving {target.fullName()} into {new_parent.fullName()}, " - # f"because {origin_name!r} is already exported in {modname}.__all__") def getModuleExports(mod:'model.Module') -> Collection[str]: # Fetch names to export. @@ -242,6 +243,11 @@ class ReExport: origin_module: model.Module target: model.Documentable +def _exports_order(r:ReExport) -> object: + return (-r.new_parent.privacyClass.value, + r.new_parent.fullName().count('.'), + -len(r.as_name)) + def _maybeExistingNameOverridesImport(mod:model.Module, local_name:str, imp:model.Import, target:model.Documentable) -> bool: @@ -308,15 +314,28 @@ def processReExports(system:'model.System') -> None: for target, _exports in exports_per_target.items(): elsewhere = [] + + if isinstance(target.parent, model.Module) and target.parent.all is not None \ + and target.name in target.parent.all \ + and target.parent.privacyClass is model.PrivacyClass.PUBLIC: + + target.system.msg( + "astbuilder", + f"not moving {target.fullName()} into {' or '.join(repr(e.new_parent.fullName()) for e in _exports)}, " + f"because {target.name!r} is already exported in public module {target.parent.fullName()!r}") + + for e in _exports: + e.new_parent.elsewhere_contents[e.as_name] = target + + continue + assert len(_exports) > 0 if len(_exports) > 1: - # when an object has several re-exports, the module with the lowest number + # when an object has several re-exports, the public module with the lowest number # of dot in it's name is choosen, if there is an equality, the longer local name - # if choosen - # TODO: move this into a system method - # TODO: do not move objects inside a private module - # TODO: do not move objects when they are listed in __all__ of a public module - _exports.sort(key=lambda r:(r.new_parent.fullName().count('.'), -len(r.as_name))) + # is choosen + + _exports.sort(key=_exports_order) elsewhere.extend(_exports[1:]) reexport = _exports[0] diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 5668e986e..fad7fa761 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2873,21 +2873,48 @@ class Slc:... def test_reparenting_from_module_that_defines__all__(systemcls: Type[model.System], capsys:CapSys) -> None: """ Even if a module defined it's own __all__ attribute, - we can reparent it's direct children to a new module, - but only if it's a private module. + we can reparent it's direct children to a new module + but only when the origin module has a lower privacy class + (i.e reparenting from a private module to a plublic module), otherwise the name stays there. """ - src = '''\ + _src = '''\ class cls:... - __all__ = ['cls'] + class cls3:... + class cls4:... + __all__ = ['cls', 'cls3', 'cls4'] + ''' + src = ''' + class cls2:... + __all__ = ['cls2'] + ''' + pack = '''\ + from ._src import cls + from .src import cls2 + __all__=["cls","cls2"] + ''' + subpack = '''\ + from .._src import cls3 + __all__=["cls3"] + ''' + private = '''\ + from pack._src import cls3, cls4 + __all__ = ['cls3', 'cls4'] ''' - pack = 'from ._src import cls; __all__=["cls"]' system = systemcls() builder = system.systemBuilder(system) + builder.addModuleString(private, '_private') builder.addModuleString(pack, 'pack', is_package=True) - builder.addModuleString(src, '_src', parent_name='pack') + builder.addModuleString(subpack, 'subpack', parent_name='pack', is_package=True) + builder.addModuleString(_src, '_src', parent_name='pack') + builder.addModuleString(src, 'src', parent_name='pack') builder.buildModules() - assert capsys.readouterr().out == "moving 'pack._src.cls' into 'pack'\n" + assert capsys.readouterr().out == ( + "moving 'pack._src.cls3' into 'pack.subpack', also available at '_private.cls3'\n" + "moving 'pack._src.cls4' into '_private'\n" + "moving 'pack._src.cls' into 'pack'\n" + "not moving pack.src.cls2 into 'pack', because 'cls2' is already exported in public module 'pack.src'\n") + assert system.allobjects['pack.cls'] is system.allobjects['pack._src'].elsewhere_contents['cls'] # type:ignore @systemcls_param @@ -2949,7 +2976,7 @@ class Cls:... builder.addModuleString(src, 'src', parent_name='pack.subpack') builder.buildModules() - assert capsys.readouterr().out == ("moving 'pack.subpack.src.Cls' into 'pack'\n" + assert capsys.readouterr().out == ("moving 'pack.subpack.src.Cls' into 'pack', " "also available at 'pack.subpack.Cls'\n") assert system.allobjects['pack.Cls'] is system.allobjects['pack.subpack'].elsewhere_contents['Cls'] # type:ignore @@ -2977,7 +3004,7 @@ class DistinguishedName:... builder.addModuleString(src, 'src', parent_name='pack.subpack') builder.buildModules() - assert capsys.readouterr().out == ("moving 'pack.subpack.src.DistinguishedName' into 'pack' as 'DisName'\n" + assert capsys.readouterr().out == ("moving 'pack.subpack.src.DistinguishedName' into 'pack' as 'DisName', " "also available at 'pack.DN'\n") assert system.allobjects['pack.DisName'] is system.allobjects['pack'].elsewhere_contents['DN'] # type:ignore diff --git a/pydoctor/test/test_packages.py b/pydoctor/test/test_packages.py index 8223b4943..37034c655 100644 --- a/pydoctor/test/test_packages.py +++ b/pydoctor/test/test_packages.py @@ -57,10 +57,9 @@ def test_allgames() -> None: assert isinstance(mod1, model.Module) mod2 = system.allobjects['allgames.mod2'] assert isinstance(mod2, model.Module) - # changed in 2023 - # InSourceAll is moved into mod2 as well as NotInSourceAll. - assert 'InSourceAll' not in mod1.contents - assert 'InSourceAll' in mod2.contents + # InSourceAll is not moved into mod2 because it's defined in __all__ and module is public. + assert 'InSourceAll' in mod1.contents + assert 'InSourceAll' not in mod2.contents assert 'NotInSourceAll' in mod2.contents # Source paths must be unaffected by the move, so that error messages # point to the right source code. From 434a46073473908adf08345d5e9bf1529305bbc1 Mon Sep 17 00:00:00 2001 From: tristanlatr <19967168+tristanlatr@users.noreply.github.com> Date: Sat, 30 Sep 2023 19:39:41 -0400 Subject: [PATCH 18/23] Fix space --- pydoctor/astbuilder.py | 2 +- pydoctor/test/test_astbuilder.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 55320fd3d..1d7da427f 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -305,7 +305,7 @@ def processReExports(system:'model.System') -> None: msg = f"cannot resolve origin module of re-exported name: {orgname or local_name!r}" if orgname and local_name!=orgname: msg += f" as {local_name!r}" - msg += f"from origin module {imported_name.orgmodule!r}" + msg += f" from origin module {imported_name.orgmodule!r}" mod.report(msg, lineno_offset=imported_name.linenumber) exports_per_target:Dict[model.Documentable, List[ReExport]] = defaultdict(list) diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index fad7fa761..53c5aab93 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2866,7 +2866,7 @@ class Slc:... assert list(system.allobjects['pack'].contents) == ['_src0', '_src1'] - assert capsys.readouterr().out == ("pack:1: cannot resolve origin module of re-exported name: 'Slc'from origin module 'pack._src2'\n" + assert capsys.readouterr().out == ("pack:1: cannot resolve origin module of re-exported name: 'Slc' from origin module 'pack._src2'\n" "pack:1: cannot resolve re-exported name: 'pack._src1.Cls'\n") @systemcls_param From 0bf2334149618ec6b943a65ed284df7810ddf315 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Sun, 1 Oct 2023 17:56:42 -0400 Subject: [PATCH 19/23] Fix typo --- pydoctor/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index b6823cee9..b8b4fddb9 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -483,7 +483,7 @@ def setup(self) -> None: When pydoctor re-export objects, it leaves references to object in this dict so they can still be listed in childtable of origin modules. This attribute belongs to the "view model" part of Documentable interface and should only be used to present - links to these objects, nor to do any name resolving. + links to these objects. Not to do name resolving. """ def _localNameToFullName(self, name: str) -> str: From f28c265565f5d59479fcb7ffe4fa3098da19732c Mon Sep 17 00:00:00 2001 From: tristanlatr <19967168+tristanlatr@users.noreply.github.com> Date: Thu, 12 Oct 2023 22:18:52 -0300 Subject: [PATCH 20/23] Update pydoctor/astbuilder.py --- pydoctor/astbuilder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 410f62088..8fd7e7c5f 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -244,7 +244,7 @@ class ReExport: origin_module: model.Module target: model.Documentable -def _exports_order(r:ReExport) -> object: +def _exports_order(r:ReExport) -> tuple[int, int, int]: return (-r.new_parent.privacyClass.value, r.new_parent.fullName().count('.'), -len(r.as_name)) From a70b8b202d0ffd90f844ca655fa696ebc9bba85c Mon Sep 17 00:00:00 2001 From: tristanlatr <19967168+tristanlatr@users.noreply.github.com> Date: Sun, 15 Oct 2023 13:39:47 -0300 Subject: [PATCH 21/23] Update pydoctor/astbuilder.py --- pydoctor/astbuilder.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 8fd7e7c5f..64e4d8879 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -3,7 +3,6 @@ import ast from collections import defaultdict -from statistics import mean import sys from functools import partial From 1c4343be87a78e2e9a684d3bd70c648907a274cf Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Wed, 1 Nov 2023 13:51:15 -0400 Subject: [PATCH 22/23] It's possible to re-export stuff from a class. --- pydoctor/astbuilder.py | 7 ++--- pydoctor/model.py | 18 +++++++----- pydoctor/templatewriter/pages/__init__.py | 2 +- pydoctor/test/test_astbuilder.py | 34 +++++++++++++++++++---- 4 files changed, 43 insertions(+), 18 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 64e4d8879..58a513ef6 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -186,15 +186,14 @@ def _handleReExport(info:'ReExport', elsewhere:Collection['ReExport']) -> None: target = info.target as_name = info.as_name target_parent = target.parent - assert isinstance(target_parent, model.Module) # Remember that this name is re-exported - target_parent.elsewhere_contents[target.name] = target + target_parent.exported[target.name] = target extra_msg = '' for e in elsewhere: - e.new_parent.elsewhere_contents[e.as_name] = target + e.new_parent.exported[e.as_name] = target if not extra_msg: extra_msg += ', also available at ' @@ -325,7 +324,7 @@ def processReExports(system:'model.System') -> None: f"because {target.name!r} is already exported in public module {target.parent.fullName()!r}") for e in _exports: - e.new_parent.elsewhere_contents[e.as_name] = target + e.new_parent.exported[e.as_name] = target continue diff --git a/pydoctor/model.py b/pydoctor/model.py index d32cf4189..4d4746ef9 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -419,6 +419,17 @@ class CanContainImportsDocumentable(Documentable): def setup(self) -> None: super().setup() self._localNameToFullName_map: Dict[str, str] = {} + """ + Mapping from local names to fullnames: Powers name resolving. + """ + + self.exported: Dict[str, 'Documentable'] = {} + """ + When pydoctor re-export objects, it leaves references to object in this dict + so they can still be listed in childtable of origin modules or classes. This attribute belongs + to the "view model" part of Documentable interface and should only be used to present + links to these objects. Not to do name resolving. + """ def isNameDefined(self, name: str) -> bool: name = name.split('.')[0] @@ -479,13 +490,6 @@ def setup(self) -> None: self._docformat: Optional[str] = None self.imports: List[Import] = [] - self.elsewhere_contents: Dict[str, 'Documentable'] = {} - """ - When pydoctor re-export objects, it leaves references to object in this dict - so they can still be listed in childtable of origin modules. This attribute belongs - to the "view model" part of Documentable interface and should only be used to present - links to these objects. Not to do name resolving. - """ def _localNameToFullName(self, name: str) -> str: if name in self.contents: diff --git a/pydoctor/templatewriter/pages/__init__.py b/pydoctor/templatewriter/pages/__init__.py index 33b838d7f..3a33f27f1 100644 --- a/pydoctor/templatewriter/pages/__init__.py +++ b/pydoctor/templatewriter/pages/__init__.py @@ -407,7 +407,7 @@ def extras(self) -> List["Flattenable"]: def _iter_reexported_members(self, predicate: Optional[Callable[[model.Documentable], bool]]=None) -> Iterator[Tuple[str, model.Documentable]]: if not predicate: predicate = lambda v:True - return ((n,o) for n,o in self.ob.elsewhere_contents.items() if o.isVisible and predicate(o)) + return ((n,o) for n,o in self.ob.exported.items() if o.isVisible and predicate(o)) def children(self) -> Sequence[Union[model.Documentable, Tuple[str, model.Documentable]]]: return sorted(chain( diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 53c5aab93..067e62a36 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2915,7 +2915,7 @@ class cls2:... "moving 'pack._src.cls' into 'pack'\n" "not moving pack.src.cls2 into 'pack', because 'cls2' is already exported in public module 'pack.src'\n") - assert system.allobjects['pack.cls'] is system.allobjects['pack._src'].elsewhere_contents['cls'] # type:ignore + assert system.allobjects['pack.cls'] is system.allobjects['pack._src'].exported['cls'] # type:ignore @systemcls_param def test_do_not_reparent_to_existing_name(systemcls: Type[model.System], capsys:CapSys) -> None: @@ -2949,7 +2949,7 @@ class Cls:... "pack:1: duplicate Class 'pack.Slc'\n" "pack._src1:1: introduced by re-exporting Class 'pack._src1.Slc' into Package 'pack'\n") - assert system.allobjects['pack.Slc'] is system.allobjects['pack._src1'].elsewhere_contents['Slc'] # type:ignore + assert system.allobjects['pack.Slc'] is system.allobjects['pack._src1'].exported['Slc'] # type:ignore @systemcls_param def test_multiple_re_exports(systemcls: Type[model.System], capsys:CapSys) -> None: @@ -2979,8 +2979,8 @@ class Cls:... assert capsys.readouterr().out == ("moving 'pack.subpack.src.Cls' into 'pack', " "also available at 'pack.subpack.Cls'\n") - assert system.allobjects['pack.Cls'] is system.allobjects['pack.subpack'].elsewhere_contents['Cls'] # type:ignore - assert system.allobjects['pack.Cls'] is system.allobjects['pack.subpack.src'].elsewhere_contents['Cls'] # type:ignore + assert system.allobjects['pack.Cls'] is system.allobjects['pack.subpack'].exported['Cls'] # type:ignore + assert system.allobjects['pack.Cls'] is system.allobjects['pack.subpack.src'].exported['Cls'] # type:ignore @systemcls_param def test_multiple_re_exports_alias(systemcls: Type[model.System], capsys:CapSys) -> None: @@ -3007,5 +3007,27 @@ class DistinguishedName:... assert capsys.readouterr().out == ("moving 'pack.subpack.src.DistinguishedName' into 'pack' as 'DisName', " "also available at 'pack.DN'\n") - assert system.allobjects['pack.DisName'] is system.allobjects['pack'].elsewhere_contents['DN'] # type:ignore - assert system.allobjects['pack.DisName'] is system.allobjects['pack.subpack.src'].elsewhere_contents['DistinguishedName'] # type:ignore \ No newline at end of file + assert system.allobjects['pack.DisName'] is system.allobjects['pack'].exported['DN'] # type:ignore + assert system.allobjects['pack.DisName'] is system.allobjects['pack.subpack.src'].exported['DistinguishedName'] # type:ignore + +@systemcls_param +def test_re_export_method(systemcls: Type[model.System], capsys:CapSys) -> None: + src = '''\ + class Thing: + def method(self):... + method = Thing.method + ''' + subpack = '' + pack = ''' + from pack.subpack.src import method + __all__=['method'] + ''' + + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString(pack, 'pack', is_package=True) + builder.addModuleString(subpack, 'subpack', is_package=True, parent_name='pack') + builder.addModuleString(src, 'src', parent_name='pack.subpack') + builder.buildModules() + assert capsys.readouterr().out == "moving 'pack.subpack.src.Thing.method' into 'pack'\n" + From 011ebcfa851aaab04399eced76cbdfac1b7bdf9b Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Tue, 2 Apr 2024 21:36:52 -0400 Subject: [PATCH 23/23] Fix mypy --- pydoctor/astbuilder.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 7c99d9be6..124f7f065 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -186,6 +186,10 @@ def _handleReExport(info:'ReExport', elsewhere:Collection['ReExport']) -> None: as_name = info.as_name target_parent = target.parent + if target_parent is None: + # TODO: warn because we can't reparent a root module + return None + # Remember that this name is re-exported target_parent.exported[target.name] = target