Fix issue #184

(Wip, still a couple of problems)

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"
 

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,6 +13,7 @@
     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
@@ -149,36 +151,51 @@ def extract_final_subscript(annotation: ast.Subscript) -> ast.expr:
         assert isinstance(ann_slice, ast.expr)
         return ann_slice
 
-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()
-
+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'\'{modname}.{origin_name}\'', lineno_offset=linenumber)
+                                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:
-        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__")
+        new_parent.system.msg(
+            "astbuilder",
+            f"moving {target.fullName()!r} into {new_parent.fullName()!r}")
+
+    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.
@@ -198,7 +215,35 @@ def getPublicNames(mod:'model.Module') -> Collection[str]:
             ]
     return names
 
+@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:
+    # 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:
@@ -210,22 +255,53 @@ def processReExports(system:'model.System') -> None:
             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.
                     if orgname:
-                        # only 'import from' statements can be used in re-exporting currently.
-                        _handleReExport(mod, orgname, local_name, origin, 
-                                        linenumber=imported_name.linenumber)
+                        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:
-                            _handleReExport(mod, n, n, origin,
-                                            linenumber=imported_name.linenumber)
+                            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)
+
 class ModuleVistor(NodeVisitor):
 
     def __init__(self, builder: 'ASTBuilder', module: model.Module):

pydoctor/model.py

@@ -549,6 +549,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, not to do any name resolving.
+        """
 
     def _localNameToFullName(self, name: str) -> str:
         if name in self.contents:

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 [

pydoctor/templatewriter/pages/table.py

@@ -1,10 +1,10 @@
-from typing import TYPE_CHECKING, Collection
+from typing import TYPE_CHECKING, Collection, Optional, Tuple, Union
 
 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:
@@ -64,7 +67,7 @@ class ChildTable(TemplateElement):
     def __init__(self,
             docgetter: util.DocGetter,
             ob: Documentable,
-            children: Collection[Documentable],
+            children: Collection[Union[Documentable, Tuple[str, Documentable]]],
             loader: ITemplateLoader,
             ):
         super().__init__(loader)
@@ -85,7 +88,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
+                ]