From 6f2e374496b7f893ec5678ff3940addf49eb75c5 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Mon, 23 May 2022 16:06:27 -0400 Subject: [PATCH 01/13] Add a test --- pydoctor/test/test_astbuilder.py | 67 ++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index c2f219206..d3593ec7c 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2075,3 +2075,70 @@ class j: pass assert system.allobjects['top._impl'].resolveName('f') == system.allobjects['top'].contents['f'] assert system.allobjects['_impl2'].resolveName('i') == system.allobjects['top'].contents['i'] assert all(n in system.allobjects['top'].contents for n in ['f', 'g', 'h', 'i', 'j']) + +@systemcls_param +def test_module_level_attributes_and_aliases(systemcls: Type[model.System]) -> None: + """ + Currently, the first analyzed assigment wins, basically. I believe further logic should be added + such that definitions in the orelse clause of the Try node is processed before the + except handlers. This way could define our aliases both there and in the body of the + Try node and fall back to what's defnied in the handlers if the names doesn't exist yet. + """ + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString(''' + ssl = 1 + ''', modname='twisted.internet') + builder.addModuleString(''' + try: + from twisted.internet import ssl as _ssl + # The first analyzed assigment to an alias wins. + ssl = _ssl + # For classic variables, the rules are the same. + var = 1 + # For constants, the rules are still the same. + VAR = 1 + # Looks like a constant, but should be treated like an alias + 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 constant + 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.CONSTANT + + # Test looks like constant but actually an alias. + 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 + From 0c50517f4e2aec570ce84f0321f35b1183072d85 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Mon, 23 May 2022 20:20:32 -0400 Subject: [PATCH 02/13] Fix bug in deprecate.py extension. --- pydoctor/extensions/deprecate.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pydoctor/extensions/deprecate.py b/pydoctor/extensions/deprecate.py index 90ffb87d3..fe2cc28cb 100644 --- a/pydoctor/extensions/deprecate.py +++ b/pydoctor/extensions/deprecate.py @@ -60,7 +60,8 @@ def depart_ClassDef(self, node:ast.ClassDef) -> None: except KeyError: # Classes inside functions are ignored. return - assert isinstance(cls, model.Class) + if not isinstance(cls, model.Class): + return getDeprecated(cls, node.decorator_list) def depart_FunctionDef(self, node:ast.FunctionDef) -> None: @@ -73,7 +74,8 @@ def depart_FunctionDef(self, node:ast.FunctionDef) -> None: except KeyError: # Inner functions are ignored. return - assert isinstance(func, (model.Function, model.Attribute)) + if not isinstance(func, (model.Function, model.Attribute)): + return getDeprecated(func, node.decorator_list) _incremental_Version_signature = inspect.signature(Version) From 7d408730d7620a5440f8c9323a0a783a9f3dfbee Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Mon, 23 May 2022 20:59:12 -0400 Subject: [PATCH 03/13] Since the new NodeVisitor class, all the node are not visited as before. The older visitor relied on generic_visit() to be recursive. Where the provided generic_visit() is not recursive anymore, and moreover not called automatically when visiting unknow nodes! So what I'm saying here is that since #576 have been merged, we're not visiting the statements inside the 'orelse' field of Try and If nodes, same goes for 'finalbody' and 'handlers'. This commit fixes that issue. The rationale is now the following: All statements in the 'orelse' block of IF nodes and statements in the except handlers of TRY nodes that would override a name already defined in the main 'body' (or TRY 'orelse' or 'finalbody') are ignored. Meaning that in the context of the code below, 'ssl' would resolve to 'twisted.internet.ssl': try: from twisted.internet import ssl as _ssl except ImportError: ssl = None else: ssl = _ssl --- pydoctor/astbuilder.py | 78 +++++++++++++- pydoctor/test/test_astbuilder.py | 168 ++++++++++++++++++++++++++++++- 2 files changed, 241 insertions(+), 5 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index a1e2d4831..05d1ae1eb 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -1,6 +1,7 @@ """Convert ASTs into L{pydoctor.model.Documentable} instances.""" import ast +import contextlib import sys from attr import attrs, attrib from functools import partial @@ -201,7 +202,39 @@ def __init__(self, builder: 'ASTBuilder', module: model.Module): self.builder = builder self.system = builder.system self.module = module + self._override_guard_state: Tuple[bool, model.Documentable, Sequence[str]] = \ + (False, cast(model.Documentable, None), []) + + @contextlib.contextmanager + def override_guard(self) -> Iterator[None]: + """ + Returns a context manager that will make the builder ignore any extraneous + assigments to existing names within the same context. + + @note: The list of existing names is genrated at the moment of + calling the function. + """ + current = self.builder.current + 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 = (True, current, self._list_names(current)) + yield + self._override_guard_state = ignore_override_init + + def _list_names(self, ob: model.Documentable) -> List[str]: + """ + List the names currently defined in a class/module. + """ + names = list(ob.contents) + if isinstance(ob, model.CanContainImportsDocumentable): + names.extend(ob._localNameToFullName_map) + return names + def _name_in_override_guard(self, ob: model.Documentable, name:str) -> bool: + return self._override_guard_state[0] is True \ + and self._override_guard_state[1] is ob \ + and name in self._override_guard_state[2] def visit_If(self, node: ast.If) -> None: if isinstance(node.test, ast.Compare): @@ -210,6 +243,28 @@ def visit_If(self, node: ast.If) -> None: # whatever is declared in them cannot be imported # and thus is not part of the API raise self.SkipNode() + + def depart_If(self, node: ast.If) -> None: + # At this point the body of the Try 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 @@ -227,6 +282,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._name_in_override_guard(parent, node.name): + raise self.SkipNode() rawbases = [] bases = [] @@ -328,6 +386,11 @@ def visit_ImportFrom(self, node: ast.ImportFrom) -> None: def _importAll(self, modname: str) -> None: """Handle a C{from import *} statement.""" + # Always ignore import * in override guard + if self._override_guard_state[0]: + self.builder.warning("ignored import * from", modname) + return + mod = self.system.getProcessedModule(modname) if mod is None: # We don't have any information about the module, so we don't know @@ -420,7 +483,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._name_in_override_guard(current, asname): + continue + if mod is not None and self._handleReExport(exports, orgname, asname, mod) is True: continue @@ -449,9 +516,13 @@ def visit_Import(self, node: ast.Import) -> None: str(self.builder.current)) return _localNameToFullName = self.builder.current._localNameToFullName_map + current = self.builder.current for al in node.names: fullname, asname = al.name, al.asname if asname is not None: + # Ignore in override guard + if self._name_in_override_guard(current, asname): + continue _localNameToFullName[asname] = fullname @@ -729,6 +800,8 @@ def _handleAssignment(self, if isinstance(targetNode, ast.Name): target = targetNode.id scope = self.builder.current + if self._name_in_override_guard(scope, target): + return if isinstance(scope, model.Module): self._handleAssignmentInModule(target, annotation, expr, lineno) elif isinstance(scope, model.Class): @@ -788,6 +861,9 @@ def _handleFunctionDef(self, parent = self.builder.current if isinstance(parent, model.Function): raise self.SkipNode() + # Ignore in override guard + if self._name_in_override_guard(parent, node.name): + raise self.SkipNode() lineno = node.lineno diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index d3593ec7c..f8900425b 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2092,13 +2092,11 @@ def test_module_level_attributes_and_aliases(systemcls: Type[model.System]) -> N builder.addModuleString(''' try: from twisted.internet import ssl as _ssl - # The first analyzed assigment to an alias wins. + # The names defined in the body of the if block wins over the + # names defined in the except handler ssl = _ssl - # For classic variables, the rules are the same. var = 1 - # For constants, the rules are still the same. VAR = 1 - # Looks like a constant, but should be treated like an alias ALIAS = _ssl except ImportError: ssl = None @@ -2142,3 +2140,165 @@ def test_module_level_attributes_and_aliases(systemcls: Type[model.System]) -> N 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 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 + 'var 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 constant + 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.CONSTANT + + # Test looks like constant but actually an alias. + 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_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 not 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' not in mod._localNameToFullName_map From ae6582f56c64b7d20f49e12c24ee7124ab76a6b2 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Mon, 23 May 2022 21:05:14 -0400 Subject: [PATCH 04/13] fix tests --- pydoctor/test/test_astbuilder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index f8900425b..8a4ef4e85 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2174,7 +2174,7 @@ def func(): class klass: 'klass doc' var2 = 1 - 'var doc' + 'var2 doc' else: # these definition will be ignored since they are # alreade definied in the body of the If block. From 3bd4407168b69c79f8cd07e63dfc122690537667 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Mon, 23 May 2022 21:50:26 -0400 Subject: [PATCH 05/13] Move _name_in_override_guard() such that it's covered by the tests. --- pydoctor/astbuilder.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 05d1ae1eb..809afccd1 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -519,10 +519,11 @@ def visit_Import(self, node: ast.Import) -> None: current = self.builder.current for al in node.names: fullname, asname = al.name, al.asname + # Ignore in override guard + if self._name_in_override_guard(current, asname or fullname): + continue if asname is not None: - # Ignore in override guard - if self._name_in_override_guard(current, asname): - continue + # Do not create an alias with the same name as value _localNameToFullName[asname] = fullname From 861342273d2ea907de108350e10e98cd2ff62ced Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Mon, 23 May 2022 21:51:33 -0400 Subject: [PATCH 06/13] Fix typo --- pydoctor/astbuilder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 809afccd1..52626ac84 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -211,7 +211,7 @@ def override_guard(self) -> Iterator[None]: Returns a context manager that will make the builder ignore any extraneous assigments to existing names within the same context. - @note: The list of existing names is genrated at the moment of + @note: The list of existing names is generated at the moment of calling the function. """ current = self.builder.current From 9a027226ff4754aca76803e964dc77a5b7cccb09 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Mon, 23 May 2022 22:43:39 -0400 Subject: [PATCH 07/13] Ignore Try.handlers blocks If.orelse blocks in functions/methods. --- pydoctor/astbuilder.py | 12 ++++++++++- pydoctor/test/test_astbuilder.py | 34 +++++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 52626ac84..55c7910aa 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -246,6 +246,12 @@ def visit_If(self, node: ast.If) -> None: def depart_If(self, node: ast.If) -> None: # At this point the body of the Try node has already been visited + + # Visit the orelse with override guard only if the current context is a module or a class. + # If.orelse blocks in functions are currently ignored. + if not isinstance(self.builder.current, (model.Module, model.Class)): + return + # Visit the 'orelse' block of the If node, with override guard with self.override_guard(): for n in node.orelse: @@ -260,7 +266,11 @@ def depart_Try(self, node: ast.Try) -> None: for n in node.finalbody: self.walkabout(n) - # Visit the handlers with override guard + # Visit the handlers with override guard only if the current context is a module or a class. + # Try.handlers blocks in functions are currently ignored. + if not isinstance(self.builder.current, (model.Module, model.Class)): + return + with self.override_guard(): for h in node.handlers: for n in h.body: diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 8a4ef4e85..6ae41d8d6 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2232,7 +2232,39 @@ class var2: assert klass.docstring == 'klass doc' assert isinstance(var2, model.Attribute) assert var2.docstring == 'var2 doc' - + +@systemcls_param +def test_method_level_orelse_handlers_ignored(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: + # Since error is only defined in the except block in a function/method + # it will not be included in the documentation. + # It will be documented if one adds a '@var error:' field to the class docstring though. + self.error = True + finally: + self.name = text + if sys.version_info > (3,0): + pass + else: + # Idem for this instance attribute + self.legacy = 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('legacy') == None + assert K.resolveName('error') == None + assert K.resolveName('name') == K.contents['name'] + @systemcls_param def test_class_level_attributes_and_aliases_orelse(systemcls: Type[model.System]) -> None: system = systemcls() From 7ad998f8eaca6719b1219725b594e6ac2e006403 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Mon, 23 May 2022 23:36:05 -0400 Subject: [PATCH 08/13] Revert ignoring 'If.orelse' and 'Try.handler' in functions and methods. Properly add support for that to override_guard() function instead. --- pydoctor/astbuilder.py | 29 +++++++++-------- pydoctor/test/test_astbuilder.py | 56 +++++++++++++++++++++++++++----- 2 files changed, 64 insertions(+), 21 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 55c7910aa..bc4709acc 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -195,6 +195,17 @@ def extract_final_subscript(annotation: ast.Subscript) -> ast.expr: assert isinstance(ann_slice, ast.expr) return ann_slice +def getframe(ob:'model.Documentable') -> model.CanContainImportsDocumentable: + """ + Returns the first L{model.Class} or L{model.Module} starting + at C{ob} and going upward in the tree. + """ + if isinstance(ob, model.Inheritable): + return getframe(ob.parent) + else: + assert isinstance(ob, model.CanContainImportsDocumentable) + return ob + class ModuleVistor(NodeVisitor): def __init__(self, builder: 'ASTBuilder', module: model.Module): @@ -214,11 +225,11 @@ def override_guard(self) -> Iterator[None]: @note: The list of existing names is generated at the moment of calling the function. """ - current = self.builder.current + ctx = getframe(self.builder.current) 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 = (True, current, self._list_names(current)) + self._override_guard_state = (True, ctx, self._list_names(ctx)) yield self._override_guard_state = ignore_override_init @@ -246,12 +257,6 @@ def visit_If(self, node: ast.If) -> None: def depart_If(self, node: ast.If) -> None: # At this point the body of the Try node has already been visited - - # Visit the orelse with override guard only if the current context is a module or a class. - # If.orelse blocks in functions are currently ignored. - if not isinstance(self.builder.current, (model.Module, model.Class)): - return - # Visit the 'orelse' block of the If node, with override guard with self.override_guard(): for n in node.orelse: @@ -266,11 +271,7 @@ def depart_Try(self, node: ast.Try) -> None: for n in node.finalbody: self.walkabout(n) - # Visit the handlers with override guard only if the current context is a module or a class. - # Try.handlers blocks in functions are currently ignored. - if not isinstance(self.builder.current, (model.Module, model.Class)): - return - + # Visit the handlers with override guard with self.override_guard(): for h in node.handlers: for n in h.body: @@ -720,6 +721,8 @@ def _handleInstanceVar(self, return if not _maybeAttribute(cls, name): return + if self._name_in_override_guard(cls, name): + return # 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)) diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 6ae41d8d6..358427faf 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2234,7 +2234,7 @@ class var2: assert var2.docstring == 'var2 doc' @systemcls_param -def test_method_level_orelse_handlers_ignored(systemcls: Type[model.System]) -> None: +def test_method_level_orelse_handlers_use_case1(systemcls: Type[model.System]) -> None: system = systemcls() builder = system.systemBuilder(system) builder.addModuleString(''' @@ -2244,26 +2244,66 @@ def __init__(self, text): try: self.test() except: - # Since error is only defined in the except block in a function/method - # it will not be included in the documentation. - # It will be documented if one adds a '@var error:' field to the class docstring though. + # 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 - else: - # Idem for this instance attribute + 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') == None - assert K.resolveName('error') == None + 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) == 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: From bd90f2bcbb7274539247ad0c81fa2fde3dea7625 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Mon, 23 May 2022 23:42:03 -0400 Subject: [PATCH 09/13] Fix mypy --- pydoctor/test/test_astbuilder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 358427faf..30ece5f3e 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2273,7 +2273,7 @@ def __init__(self, text): s = K.contents['still_supported'] assert K.resolveName('still_supported') == s assert isinstance(s, model.Attribute) - assert ast.literal_eval(s.value) == True + assert ast.literal_eval(s.value or '') == True @systemcls_param def test_method_level_orelse_handlers_use_case2(systemcls: Type[model.System]) -> None: From 39ca9a3ed9a364fd899edffec9319d23311eaf3b Mon Sep 17 00:00:00 2001 From: tristanlatr <19967168+tristanlatr@users.noreply.github.com> Date: Tue, 24 May 2022 14:13:43 -0400 Subject: [PATCH 10/13] Apply suggestions from code review --- pydoctor/astbuilder.py | 7 ++++--- pydoctor/test/test_astbuilder.py | 5 +---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index bc4709acc..89171b54c 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -220,10 +220,10 @@ def __init__(self, builder: 'ASTBuilder', module: model.Module): def override_guard(self) -> Iterator[None]: """ Returns a context manager that will make the builder ignore any extraneous - assigments to existing names within the same context. + 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. + calling the function, such that new names defined inside these blocks follows the usual override rules. """ ctx = getframe(self.builder.current) ignore_override_init = self._override_guard_state @@ -243,6 +243,7 @@ def _list_names(self, ob: model.Documentable) -> List[str]: return names def _name_in_override_guard(self, ob: model.Documentable, name:str) -> bool: + """Should this name be ignored because it matches the override guard in the context of C{ob}?""" return self._override_guard_state[0] is True \ and self._override_guard_state[1] is ob \ and name in self._override_guard_state[2] @@ -399,7 +400,7 @@ def _importAll(self, modname: str) -> None: # Always ignore import * in override guard if self._override_guard_state[0]: - self.builder.warning("ignored import * from", modname) + self.builder.warning("ignored import *", modname) return mod = self.system.getProcessedModule(modname) diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 30ece5f3e..b9c85537b 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2079,10 +2079,7 @@ class j: pass @systemcls_param def test_module_level_attributes_and_aliases(systemcls: Type[model.System]) -> None: """ - Currently, the first analyzed assigment wins, basically. I believe further logic should be added - such that definitions in the orelse clause of the Try node is processed before the - except handlers. This way could define our aliases both there and in the body of the - Try node and fall back to what's defnied in the handlers if the names doesn't exist yet. + 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) From 56ef027305f2cfe1d4a64eb576a3c273b3dcc250 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Tue, 24 May 2022 15:05:32 -0400 Subject: [PATCH 11/13] Try fixing #174 --- pydoctor/astbuilder.py | 36 +++++++++++++++++++++++++++++--- pydoctor/astutils.py | 29 ++++++++++++++++++++++++- pydoctor/model.py | 9 ++++++++ pydoctor/test/test_astbuilder.py | 29 +++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 4 deletions(-) diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index bc4709acc..b4e543eaa 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -14,9 +14,9 @@ ) import astor -from pydoctor import epydoc2stan, model, node2stan +from pydoctor import epydoc2stan, model, node2stan, qnmatch from pydoctor.epydoc.markup._pyval_repr import colorize_inline_pyval -from pydoctor.astutils import bind_args, node2dottedname, node2fullname, is__name__equals__main__, NodeVisitor +from pydoctor.astutils import evaluate_If_test, bind_args, node2dottedname, node2fullname, is__name__equals__main__, NodeVisitor def parseFile(path: Path) -> ast.Module: """Parse the contents of a Python source file.""" @@ -206,6 +206,18 @@ def getframe(ob:'model.Documentable') -> model.CanContainImportsDocumentable: assert isinstance(ob, model.CanContainImportsDocumentable) return ob +def get_eval_if(ob: 'model.Documentable') -> Dict[str, Any]: + """ + Returns the If evaluation rules applicable in the context of the received object. + """ + global_eval_if = ob.system.eval_if + ob_eval_if: Dict[str, Any] = {} + for p in global_eval_if.keys(): + if qnmatch.qnmatch(ob.fullName(), p): + ob_eval_if.update(global_eval_if[p]) + return ob_eval_if + + class ModuleVistor(NodeVisitor): def __init__(self, builder: 'ASTBuilder', module: model.Module): @@ -253,7 +265,25 @@ 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() # we use SkipChildren() here to still call depart(). + + # 'ast.If' evaluation feature, do not visit one of the branches based + # on user customizations, else visit both branches as ususal. + if not self.system.eval_if: + return + current = self.builder.current + if_eval_in_this_context = get_eval_if(current) + if not if_eval_in_this_context: + return + + evaluated = evaluate_If_test(node, if_eval_in_this_context, current) + + if evaluated == False: + # this will only call depart(), and thus visit only on the else branch of the If. + raise self.SkipChildren() + if evaluated == True: + # this will skip the call to depart(), so the else branch will not be visited at all. + raise self.SkipDeparture() def depart_If(self, node: ast.If) -> None: # At this point the body of the Try node has already been visited diff --git a/pydoctor/astutils.py b/pydoctor/astutils.py index e5aa88446..b500df7a6 100644 --- a/pydoctor/astutils.py +++ b/pydoctor/astutils.py @@ -4,7 +4,7 @@ import sys from numbers import Number -from typing import Iterator, Optional, List, Iterable, Sequence, TYPE_CHECKING +from typing import Any, Dict, Iterator, Optional, List, Iterable, Sequence, TYPE_CHECKING from inspect import BoundArguments, Signature import ast @@ -155,3 +155,30 @@ def is_using_annotations(expr: Optional[ast.AST], if full_name in annotations: return True return False + +def evaluate_If_test(node:ast.If, eval_if:Dict[str, Any], ctx:'model.Documentable') -> Optional[bool]: + """ + Currently supports Ifs with dotted names only. + + Like:: + + if TYPE_CHECKING: + ... + if typing.TYPE_CHECKING: + ... + + Does not recognize stuff like:: + + if TYPE_CHECKING==True: + ... + if TYPE_CHECKING is not False: + ... + """ + _test = node.test + # Supports simple dotted names only for now. + # But if we were to add more support for comparators, + # this is where it should happend. + full_name = node2fullname(_test, ctx) + if full_name and full_name in eval_if: + return bool(eval_if[full_name]) + return None diff --git a/pydoctor/model.py b/pydoctor/model.py index f159d01e1..6f3246a1e 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -641,6 +641,15 @@ class System: Additional list of extensions to load alongside default extensions. """ + eval_if: Dict[str, Dict[str, Any]] = {} + """ + Custom C{ast.If.test} evaluations. + Example:: + + class System(model.System): + eval_if = {"**":{"typing.TYPE_CHECKING":False}} + """ + def __init__(self, options: Optional['Options'] = None): self.allobjects: Dict[str, Documentable] = {} self.rootobjects: List[_ModuleT] = [] diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index 30ece5f3e..c3a7730e0 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2374,3 +2374,32 @@ class klassfallback(thing): assert mod.expandName('seven') == 'six.seven' assert 'klass' not in mod._localNameToFullName_map assert 'crazy_var' not in mod._localNameToFullName_map + + +@systemcls_param +def test_if_TYPE_CHECKING_False(systemcls: Type[model.System]) -> None: + + src = ''' + from typing import TYPE_CHECKING + + if TYPE_CHECKING: + # Inform mypy of import shenanigans. + from klein.resource import _SpecialModuleObject + resource = _SpecialModuleObject() + else: + from klein import resource + + class NotInTheTYPE_CHECKING_False_Context: + if TYPE_CHECKING: + var = 2 + ''' + + class MySystem(systemcls): # type:ignore[valid-type, misc] + eval_if = {'complex_mod': {'typing.TYPE_CHECKING':False}} + + system = MySystem() + mod = fromText(src, system=system, modname='complex_mod') + assert '_SpecialModuleObject' not in mod._localNameToFullName_map + assert 'resource' in mod._localNameToFullName_map + assert mod.expandName('resource') == 'klein.resource' + assert 'var' in mod.contents['NotInTheTYPE_CHECKING_False_Context'].contents From 0ad23b85c8e300a62b4c8c2485176c84f4c28c7a Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Tue, 24 May 2022 16:58:35 -0400 Subject: [PATCH 12/13] Add documentation about the branch priorities --- docs/source/codedoc.rst | 42 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/docs/source/codedoc.rst b/docs/source/codedoc.rst index 090d9c26c..7ec93bca9 100644 --- a/docs/source/codedoc.rst +++ b/docs/source/codedoc.rst @@ -318,3 +318,45 @@ 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 "principal" branch do not get overriden by names defined in the except hanlders or ifs' else block. + +Meaning that in the context of the code below, ``ssl`` would resolve to ``twisted.internet.ssl``: + +.. code:: python + + try: + from twisted.internet import ssl as _ssl + except ImportError: + ssl = None + else: + ssl = _ssl + +Similarly, in the context of the code below, the first ``CapSys`` class will be +documented and the second one will be ignored. + +.. code:: python + + from typing import TYPE_CHECKING + if TYPE_CHECKING: + from typing import Protocol + class CapSys(Protocol): + def readouterr() -> Any: + ... + else: + class CapSys(object): # ignored + ... + +.. But sometimes pydoctor can be better off analysing the ``TYPE_CHECKING`` blocks and should +.. stick to the runtime version of the code instead. + +.. You can instrut pydoctor do to such things with a custom system class. + +.. .. code:: python + +.. class MySystem(model.System): +.. eval_if = {'my_mod':{'TYPE_CHECKING':False}} From 2e18b5f47de14e50388fbfbe9b297c365d62f95b Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Wed, 25 May 2022 00:00:15 -0400 Subject: [PATCH 13/13] Document the new 'if' evaluation feature. --- docs/source/codedoc.rst | 15 ++++----- docs/source/customize.rst | 64 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 10 deletions(-) diff --git a/docs/source/codedoc.rst b/docs/source/codedoc.rst index 7ec93bca9..94aba8e1a 100644 --- a/docs/source/codedoc.rst +++ b/docs/source/codedoc.rst @@ -319,6 +319,8 @@ The content of ``my_project/__init__.py`` includes:: __all__ = ("MyClass",) +.. _branch-priorities: + Branch priorities ----------------- @@ -351,12 +353,7 @@ documented and the second one will be ignored. class CapSys(object): # ignored ... -.. But sometimes pydoctor can be better off analysing the ``TYPE_CHECKING`` blocks and should -.. stick to the runtime version of the code instead. - -.. You can instrut pydoctor do to such things with a custom system class. - -.. .. code:: python - -.. class MySystem(model.System): -.. eval_if = {'my_mod':{'TYPE_CHECKING':False}} +But sometimes pydoctor can be better off analysing the ``TYPE_CHECKING`` blocks and should +stick to the runtime version of the code instead. +See the :ref:`Tweak ifs branch priorities ` section for more +informatons about how to handle this kind of corner cases. diff --git a/docs/source/customize.rst b/docs/source/customize.rst index 859f2571b..bcf867182 100644 --- a/docs/source/customize.rst +++ b/docs/source/customize.rst @@ -105,6 +105,8 @@ Privacy tweak examples .. note:: Quotation marks should be added around each rule to avoid shell expansions. Unless the arguments are passed directly to pydoctor, like in Sphinx's ``conf.py``, in this case you must not quote the privacy rules. +.. _use-custom-system-class: + Use a custom system class ------------------------- @@ -116,7 +118,8 @@ and pass your custom class dotted name with the following argument:: System class allows you to customize certain aspect of the system and configure the enabled extensions. If what you want to achieve has something to do with the state of some objects in the Documentable tree, it's very likely that you can do it without the need to override any system method, -by using the extension mechanism described below. +by using the extension mechanism described below. Configuring extenions and other forms of system customizations +does, nonetheless requires you to subclass the :py:class:`pydoctor.model.System` and override the required class variables. Brief on pydoctor extensions ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -218,6 +221,65 @@ if some want to write a custom :py:class:`pydoctor.sphinx.SphinxInventory`. If you feel like other users of the community might benefit from your extension as well, please don't hesitate to open a pull request adding your extension module to the package :py:mod:`pydoctor.extensions`. +.. _tweak-ifs-branch-priority: + +Tweak 'ifs' branch priority +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Sometimes pydoctor default :ref:`Branch priorities ` can be inconvenient. +Specifically, when dealing with ``TYPE_CHECKING``. +A common purpose of ``if TYPE_CHECKING:`` blocks is to contain imports that are only +necessary for the purpose of type annotations. These might be circular imports at runtime. +When these types are used to annotate arguments or return values, +they are relevant to pydoctor as well: either to extract type information +from annotations or to look up the fully qualified version of some ``L{name}``. + +In other cases, ``if TYPE_CHECKING:`` blocks are used to perform trickery +to make ``mypy`` accept code that is difficult to analyze. +In these cases, pydoctor can have better results by analysing the runtime version of the code instead. +For details, read `the comments in this Klein PR `_. + +Pydoctor has the ability to evaluate some simple name based :py:class:`ast.If` +condition in order to visit either the statements in main ``If.body`` or the ``If.orelse`` only. + +You can instrut pydoctor do to such things with a custom system class, +by overriding the class variable :py:attr:`pydoctor.model.System.eval_if`. +This variable should be a dict of strings to dict of strings to any value. +This mechanism currently supports simple name based 'Ifs' only. + +Meaning like:: + + if TYPE_CHECKING: + ... + if typing.TYPE_CHECKING: + ... + +Does not recognize 'If' expressions like:: + + if TYPE_CHECKING==True: + ... + if TYPE_CHECKING is not False: + ... + +The code below demonstrate an example usage. + +The assignment to :py:attr:`pydoctor.model.System.eval_if` declares two custom 'If' evaluation rules. +The first rule applies only to the 'Ifs' in ``my_mod`` and second one applies to all objects directly in the package ``my_mod._complex`` +Both rules makes the statements in ``if TYPE_CHECKING:`` blocks skipped to give the priority to what's defined in the ``else:`` blocks. + +.. code:: python + + class MySystem(model.System): + eval_if = { + 'my_mod':{'TYPE_CHECKING':False}, + 'my_mod._complex.*':{'TYPE_CHECKING':False}, + } + +.. note:: See :py:mod:`pydoctor.qnmatch` for more informations regarding the pattern syntax. + +Then use the ``--system-class`` option as described in :ref:`Use a custom system class ` section. + + Use a custom writer class -------------------------