From f8e80c9fd6bd6acc5f481494200052d4ea464608 Mon Sep 17 00:00:00 2001 From: tristanlatr <19967168+tristanlatr@users.noreply.github.com> Date: Sat, 9 Sep 2023 23:07:15 -0400 Subject: [PATCH] Explicit annotation wins over inferred type (#691) * Fix #690 --- README.rst | 1 + pydoctor/astbuilder.py | 100 ++++++++++-------------------- pydoctor/astutils.py | 56 ++++++++++++++++- pydoctor/extensions/attrs.py | 2 +- pydoctor/model.py | 12 ++-- pydoctor/test/test_astbuilder.py | 103 ++++++++++++++++++++++++++++++- 6 files changed, 195 insertions(+), 79 deletions(-) diff --git a/README.rst b/README.rst index 1c71c6ccd..feb4e91ac 100644 --- a/README.rst +++ b/README.rst @@ -82,6 +82,7 @@ in development scope when possible, when impossible, the theoretical runtime scopes are used. A warning can be reported when an annotation name is ambiguous (can be resolved to different names depending on the scope context) with option ``-v``. +* Ensure that explicit annotation are honored when there are multiple declarations of the same name. * Use stricter verification before marking an attribute as constant: - instance variables are never marked as constant - a variable that has several definitions will not be marked as constant diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index 621cf8cc5..3622cb4f8 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -16,9 +16,10 @@ 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, get_parents, + is__name__equals__main__, unstring_annotation, iterassign, extract_docstring_linenum, infer_type, get_parents, NodeVisitor, Parentage) + def parseFile(path: Path) -> ast.Module: """Parse the contents of a Python source file.""" with open(path, 'rb') as f: @@ -173,6 +174,17 @@ def __init__(self, builder: 'ASTBuilder', module: model.Module): self.system = builder.system self.module = module + def _infer_attr_annotations(self, scope: model.Documentable) -> None: + # Infer annotation when leaving scope so explicit + # annotations take precedence. + for attrib in scope.contents.values(): + if not isinstance(attrib, model.Attribute): + continue + # If this attribute has not explicit annotation, + # infer its type from it's ast expression. + if attrib.annotation is None and attrib.value is not None: + # do not override explicit annotation + attrib.annotation = infer_type(attrib.value) def visit_If(self, node: ast.If) -> None: if isinstance(node.test, ast.Compare): @@ -192,6 +204,7 @@ def visit_Module(self, node: ast.Module) -> None: epydoc2stan.extract_fields(self.module) def depart_Module(self, node: ast.Module) -> None: + self._infer_attr_annotations(self.builder.current) self.builder.pop(self.module) def visit_ClassDef(self, node: ast.ClassDef) -> None: @@ -275,6 +288,7 @@ def visit_ClassDef(self, node: ast.ClassDef) -> None: def depart_ClassDef(self, node: ast.ClassDef) -> None: + self._infer_attr_annotations(self.builder.current) self.builder.popClass() @@ -495,14 +509,22 @@ def _tweakConstantAnnotation(obj: model.Attribute, annotation:Optional[ast.expr] annotation = extract_final_subscript(annotation) except ValueError as e: obj.report(str(e), section='ast', lineno_offset=lineno-obj.linenumber) - obj.annotation = _infer_type(value) if value else None + obj.annotation = infer_type(value) if value else None else: # Will not display as "Final[str]" but rather only "str" obj.annotation = annotation else: # Just plain "Final" annotation. # Simply ignore it because it's duplication of information. - obj.annotation = _infer_type(value) if value else None + obj.annotation = infer_type(value) if value else None + + @staticmethod + def _setAttributeAnnotation(obj: model.Attribute, + annotation: Optional[ast.expr],) -> None: + if annotation is not None: + # TODO: What to do when an attribute has several explicit annotations? + # (mypy reports a warning in these kind of cases) + obj.annotation = annotation @staticmethod def _storeAttrValue(obj:model.Attribute, new_value:Optional[ast.expr], @@ -558,11 +580,9 @@ def _handleModuleVar(self, if not isinstance(obj, model.Attribute): return - - if annotation is None and expr is not None: - annotation = _infer_type(expr) - obj.annotation = annotation + self._setAttributeAnnotation(obj, annotation) + obj.setLineNumber(lineno) self._handleConstant(obj, annotation, expr, lineno, @@ -605,11 +625,8 @@ def _handleClassVar(self, if obj.kind is None: obj.kind = model.DocumentableKind.CLASS_VARIABLE - if expr is not None: - if annotation is None: - annotation = _infer_type(expr) - - obj.annotation = annotation + self._setAttributeAnnotation(obj, annotation) + obj.setLineNumber(lineno) self._handleConstant(obj, annotation, expr, lineno, @@ -637,10 +654,8 @@ def _handleInstanceVar(self, if obj is None: obj = self.builder.addAttribute(name=name, kind=None, parent=cls) - if annotation is None and expr is not None: - annotation = _infer_type(expr) - - obj.annotation = annotation + self._setAttributeAnnotation(obj, annotation) + obj.setLineNumber(lineno) # undonditionnaly set the kind to ivar obj.kind = model.DocumentableKind.INSTANCE_VARIABLE @@ -1043,59 +1058,6 @@ def __repr__(self) -> str: """ return '%s' % super().__repr__() - -def _infer_type(expr: ast.expr) -> Optional[ast.expr]: - """Infer an expression's type. - @param expr: The expression's AST. - @return: A type annotation, or None if the expression has no obvious type. - """ - try: - value: object = ast.literal_eval(expr) - except (ValueError, TypeError): - return None - else: - ann = _annotation_for_value(value) - if ann is None: - return None - else: - return ast.fix_missing_locations(ast.copy_location(ann, expr)) - -def _annotation_for_value(value: object) -> Optional[ast.expr]: - if value is None: - return None - name = type(value).__name__ - if isinstance(value, (dict, list, set, tuple)): - ann_elem = _annotation_for_elements(value) - if isinstance(value, dict): - ann_value = _annotation_for_elements(value.values()) - if ann_value is None: - ann_elem = None - elif ann_elem is not None: - ann_elem = ast.Tuple(elts=[ann_elem, ann_value]) - if ann_elem is not None: - if name == 'tuple': - ann_elem = ast.Tuple(elts=[ann_elem, ast.Ellipsis()]) - return ast.Subscript(value=ast.Name(id=name), - slice=ast.Index(value=ann_elem)) - return ast.Name(id=name) - -def _annotation_for_elements(sequence: Iterable[object]) -> Optional[ast.expr]: - names = set() - for elem in sequence: - ann = _annotation_for_value(elem) - if isinstance(ann, ast.Name): - names.add(ann.id) - else: - # Nested sequences are too complex. - return None - if len(names) == 1: - name = names.pop() - return ast.Name(id=name) - else: - # Empty sequence or no uniform type. - return None - - DocumentableT = TypeVar('DocumentableT', bound=model.Documentable) class ASTBuilder: diff --git a/pydoctor/astutils.py b/pydoctor/astutils.py index 8c9154d41..36fa38293 100644 --- a/pydoctor/astutils.py +++ b/pydoctor/astutils.py @@ -404,6 +404,59 @@ def extract_docstring(node: ast.Str) -> Tuple[int, str]: lineno = extract_docstring_linenum(node) return lineno, inspect.cleandoc(node.s) + +def infer_type(expr: ast.expr) -> Optional[ast.expr]: + """Infer a literal expression's type. + @param expr: The expression's AST. + @return: A type annotation, or None if the expression has no obvious type. + """ + try: + value: object = ast.literal_eval(expr) + except (ValueError, TypeError): + return None + else: + ann = _annotation_for_value(value) + if ann is None: + return None + else: + return ast.fix_missing_locations(ast.copy_location(ann, expr)) + +def _annotation_for_value(value: object) -> Optional[ast.expr]: + if value is None: + return None + name = type(value).__name__ + if isinstance(value, (dict, list, set, tuple)): + ann_elem = _annotation_for_elements(value) + if isinstance(value, dict): + ann_value = _annotation_for_elements(value.values()) + if ann_value is None: + ann_elem = None + elif ann_elem is not None: + ann_elem = ast.Tuple(elts=[ann_elem, ann_value]) + if ann_elem is not None: + if name == 'tuple': + ann_elem = ast.Tuple(elts=[ann_elem, ast.Ellipsis()]) + return ast.Subscript(value=ast.Name(id=name), + slice=ast.Index(value=ann_elem)) + return ast.Name(id=name) + +def _annotation_for_elements(sequence: Iterable[object]) -> Optional[ast.expr]: + names = set() + for elem in sequence: + ann = _annotation_for_value(elem) + if isinstance(ann, ast.Name): + names.add(ann.id) + else: + # Nested sequences are too complex. + return None + if len(names) == 1: + name = names.pop() + return ast.Name(id=name) + else: + # Empty sequence or no uniform type. + return None + + class Parentage(ast.NodeTransformer): """ Add C{parent} attribute to ast nodes instances. @@ -429,4 +482,5 @@ def _yield_parents(n:Optional[ast.AST]) -> Iterator[ast.AST]: yield n p = cast(ast.AST, getattr(n, 'parent', None)) yield from _yield_parents(p) - yield from _yield_parents(getattr(node, 'parent', None)) \ No newline at end of file + yield from _yield_parents(getattr(node, 'parent', None)) + diff --git a/pydoctor/extensions/attrs.py b/pydoctor/extensions/attrs.py index 50dccc7b6..364b41e22 100644 --- a/pydoctor/extensions/attrs.py +++ b/pydoctor/extensions/attrs.py @@ -108,7 +108,7 @@ def annotation_from_attrib( return astutils.unstring_annotation(typ, ctx) default = args.arguments.get('default') if default is not None: - return astbuilder._infer_type(default) + return astutils.infer_type(default) return None class ModuleVisitor(extensions.ModuleVisitorExt): diff --git a/pydoctor/model.py b/pydoctor/model.py index 310529fe0..7a720f5e1 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -840,7 +840,7 @@ class FunctionOverload: class Attribute(Inheritable): kind: Optional[DocumentableKind] = DocumentableKind.ATTRIBUTE - annotation: Optional[ast.expr] + annotation: Optional[ast.expr] = None decorators: Optional[Sequence[ast.expr]] = None value: Optional[ast.expr] = None """ @@ -1436,21 +1436,19 @@ def postProcess(self) -> None: without the risk of drawing incorrect conclusions because modules were not fully processed yet. """ - - # default post-processing includes: - # - Processing of subclasses - # - MRO computing. - # - Lookup of constructors - # - Checking whether the class is an exception for cls in self.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 diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index a12df406c..8edba83aa 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -2421,6 +2421,108 @@ def __init__(self): mod = fromText(src, systemcls=systemcls) assert getConstructorsText(mod.contents['Animal']) == "Animal()" +@systemcls_param +def test_explicit_annotation_wins_over_inferred_type(systemcls: Type[model.System]) -> None: + """ + Explicit annotations are the preffered way of presenting the type of an attribute. + """ + src = '''\ + class Stuff(object): + thing: List[Tuple[Thing, ...]] + def __init__(self): + self.thing = [] + ''' + mod = fromText(src, systemcls=systemcls, modname='mod') + thing = mod.system.allobjects['mod.Stuff.thing'] + assert flatten_text(epydoc2stan.type2stan(thing)) == "List[Tuple[Thing, ...]]" #type:ignore + + src = '''\ + class Stuff(object): + thing = [] + def __init__(self): + self.thing: List[Tuple[Thing, ...]] = [] + ''' + mod = fromText(src, systemcls=systemcls, modname='mod') + thing = mod.system.allobjects['mod.Stuff.thing'] + assert flatten_text(epydoc2stan.type2stan(thing)) == "List[Tuple[Thing, ...]]" #type:ignore + +@systemcls_param +def test_explicit_inherited_annotation_looses_over_inferred_type(systemcls: Type[model.System]) -> None: + """ + Annotation are of inherited. + """ + src = '''\ + class _Stuff(object): + thing: List[Tuple[Thing, ...]] + class Stuff(_Stuff): + def __init__(self): + self.thing = [] + ''' + mod = fromText(src, systemcls=systemcls, modname='mod') + thing = mod.system.allobjects['mod.Stuff.thing'] + assert flatten_text(epydoc2stan.type2stan(thing)) == "list" #type:ignore + +@systemcls_param +def test_inferred_type_override(systemcls: Type[model.System]) -> None: + """ + The last visited value will be used to infer the type annotation + of an unnanotated attribute. + """ + src = '''\ + class Stuff(object): + thing = 1 + def __init__(self): + self.thing = (1,2) + ''' + mod = fromText(src, systemcls=systemcls, modname='mod') + thing = mod.system.allobjects['mod.Stuff.thing'] + assert flatten_text(epydoc2stan.type2stan(thing)) == "tuple[int, ...]" #type:ignore + +@systemcls_param +def test_inferred_type_is_not_propagated_to_subclasses(systemcls: Type[model.System]) -> None: + """ + Inferred type annotation should not be propagated to subclasses. + """ + src = '''\ + class _Stuff(object): + def __init__(self): + self.thing = [] + class Stuff(_Stuff): + def __init__(self, thing): + self.thing = thing + ''' + mod = fromText(src, systemcls=systemcls, modname='mod') + thing = mod.system.allobjects['mod.Stuff.thing'] + assert epydoc2stan.type2stan(thing) is None + + +@systemcls_param +def test_inherited_type_is_not_propagated_to_subclasses(systemcls: Type[model.System]) -> None: + """ + We can't repliably propage the annotations from one class to it's subclass because of + issue https://github.com/twisted/pydoctor/issues/295. + """ + src1 = '''\ + class _s:... + class _Stuff(object): + def __init__(self): + self.thing:_s = [] + ''' + src2 = '''\ + from base import _Stuff, _s + class Stuff(_Stuff): + def __init__(self, thing): + self.thing = thing + __all__=['Stuff', '_s'] + ''' + system = systemcls() + builder = system.systemBuilder(system) + builder.addModuleString(src1, 'base') + builder.addModuleString(src2, 'mod') + builder.buildModules() + thing = system.allobjects['mod.Stuff.thing'] + assert epydoc2stan.type2stan(thing) is None + @systemcls_param def test_augmented_assignment(systemcls: Type[model.System]) -> None: mod = fromText(''' @@ -2530,7 +2632,6 @@ class c: assert 'var' not in mod.contents assert 'var' not in mod.contents['c'].contents - @systemcls_param def test_typealias_unstring(systemcls: Type[model.System]) -> None: """