From 190b5a75ba3cc0ec8fc7a1a62f880ecbf3bdf8fa Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 29 Sep 2023 13:29:05 -0400 Subject: [PATCH 01/14] - 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 02/14] 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 03/14] 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 04/14] 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 05/14] 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 06/14] 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 07/14] 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 08/14] 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 09/14] 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 10/14] 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 11/14] 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 From 797eb0cbda632637681677db202ba38f469dc72e Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Wed, 3 Apr 2024 10:01:32 -0400 Subject: [PATCH 12/14] Try using System.find_object() in System.expandName() so it can better follow aliases. --- pydoctor/model.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index 4bec57cf6..e9dd9bfa8 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -351,7 +351,12 @@ class E: break nxt = self.system.objForFullName(full_name) if nxt is None: - break + try: + nxt = self.system.find_object(full_name) + except (RecursionError, LookupError): + break + if nxt is None: + break obj = nxt return '.'.join([full_name] + parts[i + 1:]) From 5df63e83d6e917332af950452a1b188fd5765829 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Wed, 3 Apr 2024 15:48:12 -0400 Subject: [PATCH 13/14] Replace System.find_object by System.objForFullName and drop find_object() symbol. The new objForFullName() implementation relies on old find_object() code. --- pydoctor/extensions/zopeinterface.py | 2 +- pydoctor/model.py | 32 +++++++++++++++------------- pydoctor/test/test_astbuilder.py | 25 ++++++++++++++++++++++ pydoctor/test/test_packages.py | 6 +++--- 4 files changed, 46 insertions(+), 19 deletions(-) diff --git a/pydoctor/extensions/zopeinterface.py b/pydoctor/extensions/zopeinterface.py index a321ae975..19ea64651 100644 --- a/pydoctor/extensions/zopeinterface.py +++ b/pydoctor/extensions/zopeinterface.py @@ -110,7 +110,7 @@ def _handle_implemented( for idx, iface_name in enumerate(implementer.implements_directly): try: - iface = implementer.system.find_object(iface_name) + iface = implementer.system.objForFullName(iface_name, raise_missing=True) except LookupError: implementer.report( 'Interface "%s" not found' % iface_name, diff --git a/pydoctor/model.py b/pydoctor/model.py index e9dd9bfa8..1090189b1 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -349,14 +349,12 @@ class E: # should probably either return None or raise LookupError. full_name = f'{obj.fullName()}.{p}' break - nxt = self.system.objForFullName(full_name) + try: + nxt = self.system.objForFullName(full_name) + except RecursionError: + break if nxt is None: - try: - nxt = self.system.find_object(full_name) - except (RecursionError, LookupError): - break - if nxt is None: - break + break obj = nxt return '.'.join([full_name] + parts[i + 1:]) @@ -1118,14 +1116,15 @@ def msg(self, self.needsnl = False print('') - def objForFullName(self, fullName: str) -> Optional[Documentable]: - return self.allobjects.get(fullName) + def objForFullName(self, full_name: str, raise_missing:bool=False) -> Optional[Documentable]: + # return self.allobjects.get(fullName) - def find_object(self, full_name: str) -> Optional[Documentable]: - """Look up an object using a potentially outdated full name. + # def find_object(self, full_name: str) -> Optional[Documentable]: + """Look up an object using a full name. + Works with potentially outdated full anmes as well. A name can become outdated if the object is reparented: - L{objForFullName()} will only be able to find it under its new name, + L{System.allobjects} only contains its new name, but we might still have references to the old name. @param full_name: The fully qualified name of the object. @@ -1134,7 +1133,7 @@ def find_object(self, full_name: str) -> Optional[Documentable]: @raise LookupError: If the object is not found, while its name does match one of the roots of this system. """ - obj = self.objForFullName(full_name) + obj = self.allobjects.get(full_name) if obj is not None: return obj @@ -1143,10 +1142,13 @@ def find_object(self, full_name: str) -> Optional[Documentable]: name_parts = full_name.split('.', 1) for root_obj in self.rootobjects: if root_obj.name == name_parts[0]: - obj = self.objForFullName(root_obj.expandName(name_parts[1])) + obj = self.allobjects.get(root_obj.expandName(name_parts[1])) if obj is not None: return obj - raise LookupError(full_name) + if raise_missing: + raise LookupError(full_name) + else: + break return None diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 18600ff1f..9d00bed14 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -3030,3 +3030,28 @@ def method(self):... builder.buildModules() assert capsys.readouterr().out == "moving 'pack.subpack.src.Thing.method' into 'pack'\n" +@systemcls_param +def test_multiple_reexports_with_aliases(systemcls: Type[model.System], capsys:CapSys) -> None: + # the attr.s() case + _make = ''' + """ + Link to L{attr.s}. + """ + + def attrs():... + ''' + top = ''' + from ._make import attrs + s = attributes = attrs + __all__ = ['s', 'attributes', 'attrs'] + ''' + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString(top, 'attr', is_package=True) + builder.addModuleString(_make, '_make', parent_name='attr') + builder.buildModules() + + make_mod = system.allobjects['attr._make'] + epydoc2stan.ensure_parsed_docstring(make_mod) + to_html(make_mod.parsed_docstring, make_mod.docstring_linker) + assert 'Cannot find link target' not in capsys.readouterr().out diff --git a/pydoctor/test/test_packages.py b/pydoctor/test/test_packages.py index 37034c655..c8b6913a3 100644 --- a/pydoctor/test/test_packages.py +++ b/pydoctor/test/test_packages.py @@ -142,18 +142,18 @@ def test_reparenting_follows_aliases() -> None: assert mything._localNameToFullName('MyClass') == 'reparenting_follows_aliases.main.MyClass' assert myotherthing._localNameToFullName('MyClass') == 'reparenting_follows_aliases._mything.MyClass' - system.find_object('reparenting_follows_aliases._mything.MyClass') == klass + system.objForFullName('reparenting_follows_aliases._mything.MyClass') == klass # This part of the test cannot pass for now since we don't recursively resolve aliases. # See https://github.com/twisted/pydoctor/pull/414 and https://github.com/twisted/pydoctor/issues/430 try: - assert system.find_object('reparenting_follows_aliases._myotherthing.MyClass') == klass + assert system.objForFullName('reparenting_follows_aliases._myotherthing.MyClass') == klass assert myotherthing.resolveName('MyClass') == klass assert mything.resolveName('MyClass') == klass assert top.resolveName('_myotherthing.MyClass') == klass assert top.resolveName('_mything.MyClass') == klass - except (AssertionError, LookupError): + except AssertionError: return else: raise AssertionError("Congratulation!") From 43276e0518ebc219b11a541a6c2410510e5e37eb Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Wed, 3 Apr 2024 17:22:51 -0400 Subject: [PATCH 14/14] Better docs --- pydoctor/model.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index 1090189b1..0585ba834 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -1117,9 +1117,6 @@ def msg(self, print('') def objForFullName(self, full_name: str, raise_missing:bool=False) -> Optional[Documentable]: - # return self.allobjects.get(fullName) - - # def find_object(self, full_name: str) -> Optional[Documentable]: """Look up an object using a full name. Works with potentially outdated full anmes as well. @@ -1131,7 +1128,7 @@ def objForFullName(self, full_name: str, raise_missing:bool=False) -> Optional[D @return: The object, or L{None} if the name is external (it does not match any of the roots of this system). @raise LookupError: If the object is not found, while its name does - match one of the roots of this system. + match one of the roots of this system and C{raise_missing=True}. """ obj = self.allobjects.get(full_name) if obj is not None: