diff --git a/README.rst b/README.rst index 581a1a42d..64b30e004 100644 --- a/README.rst +++ b/README.rst @@ -74,6 +74,11 @@ in development ^^^^^^^^^^^^^^ * Drop Python 3.7 and support Python 3.13. +* Improve collection of objects: + - Document objects declared in the ``else`` block of 'if' statements (previously they were ignored). + - Document objects declared in ``finalbody`` and ``else`` block of 'try' statements (previously they were ignored). + - Objects declared in the ``else`` block of if statements and in the ``handlers`` of 'try' statements + are ignored if a concurrent object is declared before (`more infos on branch priorities `_). * Trigger a warning when several docstrings are detected for the same object. * Improve typing of docutils related code. * Run unit tests on all supported combinations of Python versions and platforms, including PyPy for Windows. Previously, tests where ran on all supported Python version for Linux, but not for MacOS and Windows. diff --git a/docs/source/codedoc.rst b/docs/source/codedoc.rst index 06fa4325d..a8e3d7602 100644 --- a/docs/source/codedoc.rst +++ b/docs/source/codedoc.rst @@ -333,3 +333,54 @@ The content of ``my_project/__init__.py`` includes:: from .core._impl import MyClass __all__ = ("MyClass",) + +Branch priorities +----------------- + +When pydoctor deals with try/except/else or if/else block, it makes sure that the names defined in +the main flow has precedence over the definitions in ``except`` handlers or ``else`` blocks. + +Meaning that in the context of the code below, ``ssl`` would resolve to ``twisted.internet.ssl``: + +.. code:: python + + try: + # main flow + from twisted.internet import ssl as _ssl + except ImportError: + # exceptional flow + ssl = None # ignored since 'ssl' is defined in the main flow below. + var = True # not ignored since 'var' is not defined anywhere else. + else: + # main flow + ssl = _ssl + +Similarly, in the context of the code below, the ``CapSys`` protocol under the ``TYPE_CHECKING`` block will be +documented and the runtime version will be ignored. + +.. code:: python + + from typing import TYPE_CHECKING + if TYPE_CHECKING: + # main flow + from typing import Protocol + class CapSys(Protocol): + def readouterr() -> Any: + ... + else: + # secondary flow + class CapSys(object): # ignored since 'CapSys' is defined in the main flow above. + ... + +But sometimes pydoctor can be better off analysing the ``TYPE_CHECKING`` blocks and should +stick to the runtime version of the code instead. +For these case, you might want to inverse the condition of if statement: + + .. code:: python + + if not TYPE_CHECKING: + # main flow + from ._implementation import Thing + else: + # secondary flow + from ._typing import Thing # ignored since 'Thing' is defined in the main flow above. diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 129c76bbe..aa35626cc 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -2,15 +2,15 @@ from __future__ import annotations import ast +import contextlib import sys from functools import partial from inspect import Parameter, Signature -from itertools import chain from pathlib import Path from typing import ( Any, Callable, Collection, Dict, Iterable, Iterator, List, Mapping, Optional, Sequence, Tuple, - Type, TypeVar, Union, cast + Type, TypeVar, Union, Set, cast ) from pydoctor import epydoc2stan, model, node2stan, extensions, linker @@ -176,6 +176,35 @@ def __init__(self, builder: 'ASTBuilder', module: model.Module): self.builder = builder self.system = builder.system self.module = module + self._override_guard_state: Tuple[Optional[model.Documentable], Set[str]] = (None, set()) + + @contextlib.contextmanager + def override_guard(self) -> Iterator[None]: + """ + Returns a context manager that will make the builder ignore any new + assigments to existing names within the same context. Currently used to visit C{If.orelse} and C{Try.handlers}. + + @note: The list of existing names is generated at the moment of + calling the function, such that new names defined inside these blocks follows the usual override rules. + """ + ctx = self.builder.current + while not isinstance(ctx, model.CanContainImportsDocumentable): + assert ctx.parent + ctx = ctx.parent + ignore_override_init = self._override_guard_state + # we list names only once to ignore new names added inside the block, + # they should be overriden as usual. + self._override_guard_state = (ctx, set(ctx.localNames())) + yield + self._override_guard_state = ignore_override_init + + def _ignore_name(self, ob: model.Documentable, name:str) -> bool: + """ + Should this C{name} be ignored because it matches + the override guard in the context of C{ob}? + """ + ctx, names = self._override_guard_state + return ctx is ob and name in names def _infer_attr_annotations(self, scope: model.Documentable) -> None: # Infer annotation when leaving scope so explicit @@ -195,7 +224,29 @@ def visit_If(self, node: ast.If) -> None: # skip if __name__ == '__main__': blocks since # whatever is declared in them cannot be imported # and thus is not part of the API - raise self.SkipNode() + raise self.SkipChildren() + + def depart_If(self, node: ast.If) -> None: + # At this point the body of the If node has already been visited + # Visit the 'orelse' block of the If node, with override guard + with self.override_guard(): + for n in node.orelse: + self.walkabout(n) + + def depart_Try(self, node: ast.Try) -> None: + # At this point the body of the Try node has already been visited + # Visit the 'orelse' and 'finalbody' blocks of the Try node. + + for n in node.orelse: + self.walkabout(n) + for n in node.finalbody: + self.walkabout(n) + + # Visit the handlers with override guard + with self.override_guard(): + for h in node.handlers: + for n in h.body: + self.walkabout(n) def visit_Module(self, node: ast.Module) -> None: assert self.module.docstring is None @@ -216,6 +267,9 @@ def visit_ClassDef(self, node: ast.ClassDef) -> None: parent = self.builder.current if isinstance(parent, model.Function): raise self.SkipNode() + # Ignore in override guard + if self._ignore_name(parent, node.name): + raise self.IgnoreNode() rawbases = [] initialbases = [] @@ -337,35 +391,37 @@ def visit_ImportFrom(self, node: ast.ImportFrom) -> None: def _importAll(self, modname: str) -> None: """Handle a C{from import *} statement.""" + current = self.builder.current + 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) + current.report(f"import * from unknown {modname}", thresh=1) return - self.builder.current.report(f"import * from {modname}", thresh=1) + 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('_') - ] + names = [ name for name in mod.localNames() + if not name.startswith('_') ] # 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 + assert isinstance(current, model.CanContainImportsDocumentable) + _localNameToFullName = current._localNameToFullName_map expandName = mod.expandName for name in names: + # # Ignore in override guard + if self._ignore_name(current, name): + continue + if self._handleReExport(exports, name, name, mod) is True: continue @@ -429,6 +485,11 @@ def _importNames(self, modname: str, names: Iterable[ast.alias]) -> None: orgname, asname = al.name, al.asname if asname is None: asname = orgname + + # Ignore in override guard + if self._ignore_name(current, asname): + continue + # If we're importing from a package, make sure imported modules # are processed (getProcessedModule() ignores non-modules). if isinstance(mod, model.Package): @@ -451,15 +512,20 @@ 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): + current = self.builder.current + if not isinstance(current, model.CanContainImportsDocumentable): # processing import statement in odd context return - _localNameToFullName = self.builder.current._localNameToFullName_map + _localNameToFullName = current._localNameToFullName_map + 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] + # Ignore in override guard + if self._ignore_name(current, asname): + continue _localNameToFullName[asname] = targetname def _handleOldSchoolMethodDecoration(self, target: str, expr: Optional[ast.expr]) -> bool: @@ -644,6 +710,8 @@ def _handleInstanceVar(self, raise IgnoreAssignment() if not _maybeAttribute(cls, name): raise IgnoreAssignment() + if self._ignore_name(cls, name): + raise IgnoreAssignment() # Class variables can only be Attribute, so it's OK to cast because we used _maybeAttribute() above. obj = cast(Optional[model.Attribute], cls.contents.get(name)) @@ -732,6 +800,8 @@ def _handleAssignment(self, if isinstance(targetNode, ast.Name): target = targetNode.id scope = self.builder.current + if self._ignore_name(scope, target): + raise IgnoreAssignment() if isinstance(scope, model.Module): self._handleAssignmentInModule(target, annotation, expr, lineno, augassign=augassign) elif isinstance(scope, model.Class): @@ -863,6 +933,9 @@ def _handleFunctionDef(self, parent = self.builder.current if isinstance(parent, model.Function): raise self.SkipNode() + # Ignore in override guard + if self._ignore_name(parent, node.name): + raise self.IgnoreNode() lineno = node.lineno @@ -909,7 +982,7 @@ def _handleFunctionDef(self, attr.report(f'{attr.fullName()} is both property and classmethod') if is_staticmethod: attr.report(f'{attr.fullName()} is both property and staticmethod') - raise self.SkipNode() + raise self.SkipNode() # visitor extensions will still be called. # Check if it's a new func or exists with an overload existing_func = parent.contents.get(func_name) @@ -920,7 +993,7 @@ def _handleFunctionDef(self, # properties set for the primary function and not overloads. if existing_func.signature and is_overload_func: existing_func.report(f'{existing_func.fullName()} overload appeared after primary function', lineno_offset=lineno-existing_func.linenumber) - raise self.SkipNode() + raise self.IgnoreNode() # Do not recreate function object, just re-push it self.builder.push(existing_func, lineno) func = existing_func diff --git a/pydoctor/extensions/deprecate.py b/pydoctor/extensions/deprecate.py index dbdbf602c..02c4fd8e8 100644 --- a/pydoctor/extensions/deprecate.py +++ b/pydoctor/extensions/deprecate.py @@ -55,25 +55,25 @@ def depart_ClassDef(self, node:ast.ClassDef) -> None: """ Called after a class definition is visited. """ + current = self.visitor.builder.current try: - cls = self.visitor.builder.current.contents[node.name] + cls = current.contents[node.name] except KeyError: # Classes inside functions are ignored. return - assert isinstance(cls, model.Class) getDeprecated(cls, node.decorator_list) def depart_FunctionDef(self, node:ast.FunctionDef) -> None: """ Called after a function definition is visited. """ + current = self.visitor.builder.current try: # Property or Function - func = self.visitor.builder.current.contents[node.name] + func = current.contents[node.name] except KeyError: # Inner functions are ignored. return - assert isinstance(func, (model.Function, model.Attribute)) getDeprecated(func, node.decorator_list) _incremental_Version_signature = inspect.signature(Version) diff --git a/pydoctor/model.py b/pydoctor/model.py index 425727ebb..9c78fc3d0 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -9,7 +9,7 @@ import abc import ast -import attr +from itertools import chain from collections import defaultdict import datetime import importlib @@ -26,6 +26,8 @@ ) from urllib.parse import quote +import attr + from pydoctor.options import Options from pydoctor import factory, qnmatch, utils, linker, astutils, mro from pydoctor.epydoc.markup import ParsedDocstring @@ -468,6 +470,9 @@ def isNameDefined(self, name: str) -> bool: else: return False + def localNames(self) -> Iterator[str]: + return chain(self.contents.keys(), + self._localNameToFullName_map.keys()) class Module(CanContainImportsDocumentable): kind = DocumentableKind.MODULE diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index e456eae5f..bf45246fb 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -1995,6 +1995,27 @@ def bar(): ''', modname='test', systemcls=systemcls) assert tuple(mod.contents) == ('foo', 'bar') +@systemcls_param +def test__name__equals__main__is_skipped_but_orelse_processes(systemcls: Type[model.System]) -> None: + """ + Code inside of C{if __name__ == '__main__'} should be skipped, but the else block should be processed. + """ + mod = fromText(''' + foo = True + if __name__ == '__main__': + var = True + def fun(): + pass + class Class: + pass + else: + class Very: + ... + def bar(): + pass + ''', modname='test', systemcls=systemcls) + assert tuple(mod.contents) == ('foo', 'Very', 'bar' ) + @systemcls_param def test_variable_named_like_current_module(systemcls: Type[model.System]) -> None: """ @@ -2061,6 +2082,302 @@ class j: pass assert system.allobjects['_impl2'].resolveName('i') == system.allobjects['top'].contents['i'] assert all(n in system.allobjects['top'].contents for n in ['f', 'g', 'h', 'i', 'j']) +@systemcls_param +def test_module_level_attributes_and_aliases(systemcls: Type[model.System]) -> None: + """ + Variables and aliases defined in the main body of a Try node will have priority over the names defined in the except handlers. + """ + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString(''' + ssl = 1 + ''', modname='twisted.internet') + builder.addModuleString(''' + try: + from twisted.internet import ssl as _ssl + # The names defined in the body of the if block wins over the + # names defined in the except handler + ssl = _ssl + var = 1 + VAR = 1 + ALIAS = _ssl + except ImportError: + ssl = None + var = 2 + VAR = 2 + ALIAS = None + ''', modname='mod') + builder.buildModules() + mod = system.allobjects['mod'] + + # Test alias + assert mod.expandName('ssl')=="twisted.internet.ssl" + assert mod.expandName('_ssl')=="twisted.internet.ssl" + s = mod.resolveName('ssl') + assert isinstance(s, model.Attribute) + assert s.value is not None + assert ast.literal_eval(s.value)==1 + assert s.kind == model.DocumentableKind.VARIABLE + + # Test variable + assert mod.expandName('var')=="mod.var" + v = mod.resolveName('var') + assert isinstance(v, model.Attribute) + assert v.value is not None + assert ast.literal_eval(v.value)==1 + assert v.kind == model.DocumentableKind.VARIABLE + + # Test variable 2 + assert mod.expandName('VAR')=="mod.VAR" + V = mod.resolveName('VAR') + assert isinstance(V, model.Attribute) + assert V.value is not None + assert ast.literal_eval(V.value)==1 + assert V.kind == model.DocumentableKind.VARIABLE + + # Test variable 3 + assert mod.expandName('ALIAS')=="twisted.internet.ssl" + s = mod.resolveName('ALIAS') + assert isinstance(s, model.Attribute) + assert s.value is not None + assert ast.literal_eval(s.value)==1 + assert s.kind == model.DocumentableKind.VARIABLE + +@systemcls_param +def test_module_level_attributes_and_aliases_orelse(systemcls: Type[model.System]) -> None: + """ + We visit the try orelse body and these names have priority over the names in the except handlers. + """ + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString(''' + ssl = 1 + ''', modname='twisted.internet') + builder.addModuleString(''' + try: + from twisted.internet import ssl as _ssl + except ImportError: + ssl = None + var = 2 + VAR = 2 + ALIAS = None + newname = 2 + else: + # The names defined in the orelse or finally block wins over the + # names defined in the except handler + ssl = _ssl + var = 1 + finally: + VAR = 1 + ALIAS = _ssl + + if sys.version_info > (3,7): + def func(): + 'func doc' + class klass: + 'klass doc' + var2 = 1 + 'var2 doc' + else: + # these definition will be ignored since they are + # alreade definied in the body of the If block. + func = None + 'not this one' + def klass(): + 'not this one' + class var2: + 'not this one' + ''', modname='mod') + builder.buildModules() + mod = system.allobjects['mod'] + + # Tes newname survives the override guard + assert 'newname' in mod.contents + + # Test alias + assert mod.expandName('ssl')=="twisted.internet.ssl" + assert mod.expandName('_ssl')=="twisted.internet.ssl" + s = mod.resolveName('ssl') + assert isinstance(s, model.Attribute) + assert s.value is not None + assert ast.literal_eval(s.value)==1 + assert s.kind == model.DocumentableKind.VARIABLE + + # Test variable + assert mod.expandName('var')=="mod.var" + v = mod.resolveName('var') + assert isinstance(v, model.Attribute) + assert v.value is not None + assert ast.literal_eval(v.value)==1 + assert v.kind == model.DocumentableKind.VARIABLE + + # Test variable 2 + assert mod.expandName('VAR')=="mod.VAR" + V = mod.resolveName('VAR') + assert isinstance(V, model.Attribute) + assert V.value is not None + assert ast.literal_eval(V.value)==1 + assert V.kind == model.DocumentableKind.VARIABLE + + # Test variable 3 + assert mod.expandName('ALIAS')=="twisted.internet.ssl" + s = mod.resolveName('ALIAS') + assert isinstance(s, model.Attribute) + assert s.value is not None + assert ast.literal_eval(s.value)==1 + assert s.kind == model.DocumentableKind.VARIABLE + + # Test if override guard + func, klass, var2 = mod.resolveName('func'), mod.resolveName('klass'), mod.resolveName('var2') + assert isinstance(func, model.Function) + assert func.docstring == 'func doc' + assert isinstance(klass, model.Class) + assert klass.docstring == 'klass doc' + assert isinstance(var2, model.Attribute) + assert var2.docstring == 'var2 doc' + +@systemcls_param +def test_method_level_orelse_handlers_use_case1(systemcls: Type[model.System]) -> None: + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString(''' + class K: + def test(self, ):... + def __init__(self, text): + try: + self.test() + except: + # Even if this attribute is only defined in the except block in a function/method + # it will be included in the documentation. + self.error = True + finally: + self.name = text + if sys.version_info > (3,0): + pass + elif sys.version_info > (2,6): + # Idem for these instance attributes + self.legacy = True + self.still_supported = True + else: + # This attribute is ignored, the same rules that applies + # at the module level applies here too. + # since it's already defined in the upper block If.body + # this assigment is ignored. + self.still_supported = False + ''', modname='mod') + builder.buildModules() + mod = system.allobjects['mod'] + assert isinstance(mod, model.Module) + K = mod.contents['K'] + assert isinstance(K, model.Class) + assert K.resolveName('legacy') == K.contents['legacy'] + assert K.resolveName('error') == K.contents['error'] + assert K.resolveName('name') == K.contents['name'] + s = K.contents['still_supported'] + assert K.resolveName('still_supported') == s + assert isinstance(s, model.Attribute) + assert ast.literal_eval(s.value or '') == True + +@systemcls_param +def test_method_level_orelse_handlers_use_case2(systemcls: Type[model.System]) -> None: + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString(''' + class K: + def __init__(self, d:dict, g:Iterator): + try: + next(g) + except StopIteration: + # this should be documented + self.data = d + else: + raise RuntimeError("the generator wasn't exhausted!") + finally: + if sys.version_info < (3,7): + raise RuntimeError("please upadate your python version to 3.7 al least!") + else: + # Idem for this instance attribute + self.ok = True + ''', modname='mod') + builder.buildModules() + mod = system.allobjects['mod'] + assert isinstance(mod, model.Module) + K = mod.contents['K'] + assert isinstance(K, model.Class) + assert K.resolveName('data') == K.contents['data'] + assert K.resolveName('ok') == K.contents['ok'] + + +@systemcls_param +def test_class_level_attributes_and_aliases_orelse(systemcls: Type[model.System]) -> None: + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString('crazy_var=2', modname='crazy') + builder.addModuleString(''' + if sys.version_info > (3,0): + thing = object + class klass(thing): + 'klass doc' + var2 = 3 + + # regular import + from crazy import crazy_var as cv + else: + # these imports will be ignored because the names + # have been defined in the body of the If block. + from six import t as thing + import klass + from crazy27 import crazy_var as cv + + # Wildcard imports are still processed + # in name override guard context + from crazy import * + + # this import is not ignored + from six import seven + + # this class is not ignored and will be part of the public docs. + class klassfallback(thing): + 'klassfallback doc' + var2 = 1 + # this overrides var2 + var2 = 2 + + # this is ignored because the name 'klass' + # has been defined in the body of the If block. + klass = klassfallback + 'ignored' + + var3 = 1 + # this overrides var3 + var3 = 2 + ''', modname='mod') + builder.buildModules() + mod = system.allobjects['mod'] + assert isinstance(mod, model.Module) + + klass, klassfallback, var2, var3 = \ + mod.resolveName('klass'), \ + mod.resolveName('klassfallback'), \ + mod.resolveName('klassfallback.var2'), \ + mod.resolveName('var3') + + assert isinstance(klass, model.Class) + assert isinstance(klassfallback, model.Class) + assert isinstance(var2, model.Attribute) + assert isinstance(var3, model.Attribute) + + assert klassfallback.docstring == 'klassfallback doc' + assert klass.docstring == 'klass doc' + assert ast.literal_eval(var2.value or '') == 2 + assert ast.literal_eval(var3.value or '') == 2 + + assert mod.expandName('cv') == 'crazy.crazy_var' + assert mod.expandName('thing') == 'object' + assert mod.expandName('seven') == 'six.seven' + assert 'klass' not in mod._localNameToFullName_map + assert 'crazy_var' in mod._localNameToFullName_map # from the wildcard + @systemcls_param def test_exception_kind(systemcls: Type[model.System], capsys: CapSys) -> None: """ @@ -2794,6 +3111,37 @@ class B2: # TODO: handle doc comments.x assert capsys.readouterr().out == ':18: Existing docstring at line 14 is overriden\n' +@systemcls_param +def test_import_all_inside_else_branch_is_processed(systemcls: Type[model.System], capsys: CapSys) -> None: + src1 = ''' + Callable = ... + ''' + + src2 = ''' + Callable = ... + TypeAlias = ... + ''' + + src0 = ''' + import sys + if sys.version_info > (3, 10): + from typing import * + else: + from typing_extensions import * + ''' + + system = systemcls() + builder = systemcls.systemBuilder(system) + builder.addModuleString(src0, 'main') + builder.addModuleString(src1, 'typing') + builder.addModuleString(src2, 'typing_extensions') + builder.buildModules() + # assert not capsys.readouterr().out + main = system.allobjects['main'] + assert list(main.localNames()) == ['sys', 'Callable', 'TypeAlias'] # type: ignore + assert main.expandName('Callable') == 'typing.Callable' + assert main.expandName('TypeAlias') == 'typing_extensions.TypeAlias' + @systemcls_param def test_inline_docstring_multiple_assigments(systemcls: Type[model.System], capsys: CapSys) -> None: # TODO: this currently does not support nested tuple assignments. @@ -2901,4 +3249,4 @@ def test_inline_docstring_at_wrong_place(systemcls: Type[model.System], capsys: assert not mod.contents['b'].docstring assert not mod.contents['c'].docstring assert not mod.contents['d'].docstring - assert not mod.contents['e'].docstring \ No newline at end of file + assert not mod.contents['e'].docstring diff --git a/pydoctor/test/test_attrs.py b/pydoctor/test/test_attrs.py index 1d01fd8d2..a13f24654 100644 --- a/pydoctor/test/test_attrs.py +++ b/pydoctor/test/test_attrs.py @@ -125,3 +125,24 @@ class C4: ... 'test:13: Unable to figure out value for "auto_attribs" argument to attr.s(), maybe too complex\n' 'test:16: Value for "auto_attribs" argument to attr.s() has type "int", expected "bool"\n' ) + +@attrs_systemcls_param +def test_attrs_class_else_branch(systemcls: Type[model.System]) -> None: + + mod = fromText(''' + import attr + foo = bar = lambda:False + + if foo(): + pass + else: + @attr.s + class C: + if bar(): + pass + else: + var = attr.ib() + ''', systemcls=systemcls) + + var = mod.contents['C'].contents['var'] + assert var.kind is model.DocumentableKind.INSTANCE_VARIABLE \ No newline at end of file diff --git a/pydoctor/test/test_twisted_python_deprecate.py b/pydoctor/test/test_twisted_python_deprecate.py index 641150ac0..44ddeaec5 100644 --- a/pydoctor/test/test_twisted_python_deprecate.py +++ b/pydoctor/test/test_twisted_python_deprecate.py @@ -184,3 +184,32 @@ class stuff: ... assert re.match(_html_template_with_replacement.format( name='foum', package='Twisted', version='NEXT', replacement='mod.Baz.faam' ), class_html_text, re.DOTALL), class_html_text + + +@twisted_deprecated_systemcls_param +def test_twisted_python_deprecate_else_branch(capsys: CapSys, systemcls: Type[model.System]) -> None: + """ + When @deprecated decorator is used within the else branch of a if block and the same name is defined + in the body branch, the name is not marked as deprecated. + """ + + mod = fromText(''' + if sys.version_info>(3.8): + def foo(): + ... + + class Bar: + ... + else: + from incremental import Version + @twisted.python.deprecate.deprecated(Version('python', 3, 8, 0), replacement='just use newer python version') + def foo(): + ... + @twisted.python.deprecate.deprecated(Version('python', 3, 8, 0), replacement='just use newer python version') + class Bar: + ... + ''', systemcls=systemcls) + + assert not capsys.readouterr().out + assert 'just use newer python version' not in test_templatewriter.getHTMLOf(mod.contents['foo']) + assert 'just use newer python version' not in test_templatewriter.getHTMLOf(mod.contents['Bar']) \ No newline at end of file diff --git a/pydoctor/test/test_visitor.py b/pydoctor/test/test_visitor.py index be4bfefdc..44d23e080 100644 --- a/pydoctor/test/test_visitor.py +++ b/pydoctor/test/test_visitor.py @@ -15,6 +15,8 @@ def dump(node: nodes.Node, text:str='') -> None: class DocutilsNodeVisitor(visitor.Visitor[nodes.Node]): def unknown_visit(self, ob: nodes.Node) -> None: pass + def unknown_departure(self, ob: nodes.Node) -> None: + pass @classmethod def get_children(cls, ob:nodes.Node) -> Iterable[nodes.Node]: @@ -134,7 +136,7 @@ def test_visitor(capsys:CapSys) -> None: ''') doc = parsed_doc.to_node() - MainVisitor(visitor.ExtList(TitleReferenceDumpAfter)).walk(doc) + MainVisitor(visitor.ExtList(TitleReferenceDumpAfter)).walkabout(doc) assert capsys.readouterr().out == r'''title_reference line: None, rawsource: `notfound` title_reference line: None, rawsource: `notfound` title_reference line: None, rawsource: `another link ` @@ -143,7 +145,7 @@ def test_visitor(capsys:CapSys) -> None: vis = MainVisitor() vis.extensions.add(ParagraphDump, TitleReferenceDumpAfter) - vis.walk(doc) + vis.walkabout(doc) assert capsys.readouterr().out == r'''paragraph line: 4, rawsource: Lorem ipsum `notfound`. title_reference line: None, rawsource: `notfound` paragraph line: 9, rawsource: Lorem ``ipsum`` diff --git a/pydoctor/visitor.py b/pydoctor/visitor.py index 9c086a68b..afa317457 100644 --- a/pydoctor/visitor.py +++ b/pydoctor/visitor.py @@ -6,6 +6,7 @@ from collections import defaultdict import enum import abc +from itertools import chain from typing import Dict, Generic, Iterable, List, Optional, Type, TypeVar T = TypeVar("T") @@ -53,7 +54,7 @@ class Visitor(_BaseVisitor[T], abc.ABC): Each class has corresponding methods, doing nothing by default; override individual methods for specific and useful behaviour. The `visit()` method is called by - `walk()` upon entering a object. `walkabout()` also calls + `walkabout()` upon entering a object, it also calls the `depart()` method before exiting a object. The generic methods call "``visit_`` + objet class name" or @@ -68,6 +69,7 @@ class Visitor(_BaseVisitor[T], abc.ABC): def __init__(self, extensions: Optional['ExtList[T]']=None) -> None: self.extensions: 'ExtList[T]' = extensions or ExtList() self.extensions.attach_visitor(self) + self._skipped_nodes: set[T] = set() @classmethod def get_children(cls, ob: T) -> Iterable[T]: @@ -78,7 +80,7 @@ class _TreePruningException(Exception): Base class for `Visitor`-related tree pruning exceptions. Raise subclasses from within ``visit_...`` or ``depart_...`` methods - called from `Visitor.walk()` and `Visitor.walkabout()` tree traversals to prune + called from `Visitor.walkabout()` tree traversals to prune the tree traversed. """ class SkipChildren(_TreePruningException): @@ -86,48 +88,15 @@ class SkipChildren(_TreePruningException): Do not visit any children of the current node. The current node's siblings and ``depart_...`` method are not affected. """ - class SkipSiblings(_TreePruningException): - """ - Do not visit any more siblings (to the right) of the current node. The - current node's children and its ``depart_...`` method are not affected. - """ class SkipNode(_TreePruningException): """ Do not visit the current node's children, and do not call the current - node's ``depart_...`` method. + node's ``depart_...`` method. The extensions will still be called. """ - class SkipDeparture(_TreePruningException): + class IgnoreNode(_TreePruningException): """ - Do not call the current node's ``depart_...`` method. The current node's - children and siblings are not affected. - """ - - def walk(self, ob: T) -> None: + Comletely stop visiting the current node, extensions will not be run on that node. """ - Traverse a tree of objects, calling the - `visit()` method of `visitor` when entering each - node. (The `walkabout()` method is similar, except it also - calls the `depart()` method before exiting each objects.) - - This tree traversal supports limited in-place tree - modifications. Replacing one node with one or more nodes is - OK, as is removing an element. However, if the node removed - or replaced occurs after the current node, the old node will - still be traversed, and any new nodes will not. - - :param ob: An object to walk. - """ - try: - self.visit(ob) - except (self.SkipChildren, self.SkipNode): - return - except self.SkipDeparture: - pass # not applicable; ignore - try: - for child in self.get_children(ob): - self.walk(child) - except self.SkipSiblings: - pass def visit(self, ob: T) -> None: """Extend the base visit with extensions. @@ -135,64 +104,62 @@ def visit(self, ob: T) -> None: Parameters: node: The node to visit. """ - for v in self.extensions.before_visit + self.extensions.outter_visit: + for v in chain(self.extensions.before_visit, self.extensions.outter_visit): v.visit(ob) pruning = None try: super().visit(ob) except self._TreePruningException as ex: + if isinstance(ex, self.IgnoreNode): + # this exception should be raised right away since it means + # not visiting the extension visitors. + raise pruning = ex - for v in self.extensions.after_visit + self.extensions.inner_visit: + for v in chain(self.extensions.after_visit, self.extensions.inner_visit): v.visit(ob) if pruning: raise pruning - def depart(self, ob: T, extensions_only:bool=False) -> None: + def depart(self, ob: T) -> None: """Extend the base depart with extensions.""" - for v in self.extensions.before_visit + self.extensions.inner_visit: + for v in chain(self.extensions.before_visit, self.extensions.inner_visit): v.depart(ob) - if not extensions_only: + if ob not in self._skipped_nodes: super().depart(ob) - for v in self.extensions.after_visit + self.extensions.outter_visit: + for v in chain(self.extensions.after_visit, self.extensions.outter_visit): v.depart(ob) def walkabout(self, ob: T) -> None: """ - Perform a tree traversal similarly to `walk()` (which - see), except also call the `depart()` method before exiting each node. + Perform a tree traversal, calling `visit()` method when entering a + node and the `depart()` method before exiting each node. Takes special care to handle L{_TreePruningException} the following way: - - If a L{SkipNode} or L{SkipDeparture} exception is raised inside the main visitor C{visit()} method, + - If a L{SkipNode} exception is raised inside the main visitor C{visit()} method, the C{depart_*} method on the extensions will still be called. :param ob: An object to walk. """ - call_depart = True - skip_node = False try: try: self.visit(ob) except self.SkipNode: - skip_node = True - call_depart = False - except self.SkipDeparture: - call_depart = False - if not skip_node: - try: - for child in self.get_children(ob): - self.walkabout(child) - except self.SkipSiblings: - pass + self._skipped_nodes.add(ob) + except self.IgnoreNode: + return + else: + for child in self.get_children(ob): + self.walkabout(child) except self.SkipChildren: pass - self.depart(ob, extensions_only=not call_depart) + self.depart(ob) # Adapted from https://github.com/pawamoy/griffe # Copyright (c) 2021, Timothée Mazzucotelli @@ -291,15 +258,6 @@ class VisitorExt(_BaseVisitor[T]): The node visitor extension base class, to inherit from. Subclasses must define the `when` class variable, and any custom ``visit_*`` methods. - - All `_TreePruningException` raised in the main `Visitor.visit()` method will be - delayed until extensions visitor ``visit()`` and ``depart()`` methods are run as well. - - Meaning: - - If the main module visitor raises `SkipNode`, the extension visitor set to run ``AFTER`` will still visit this node, but not it's children. - - If your extension visitor is set to run ``BEFORE`` the main visitor and it raises `SkipNode`, the main visitor will not visit this node. - - If a L{SkipNode} or L{SkipDeparture} exception is raised inside the main visitor C{visit()} method, - the C{depart_*} method on the extensions will still be called. See: `When` """