diff --git a/README.rst b/README.rst index a47f1d6d1..757c2acdd 100644 --- a/README.rst +++ b/README.rst @@ -73,6 +73,9 @@ What's New? in development ^^^^^^^^^^^^^^ +* Fixes a bug that would cause a variable marked as `Final` not beeing considered as a constant if + it was declared under a control-flow block. + pydoctor 24.11.0 ^^^^^^^^^^^^^^^^ diff --git a/pydoctor/astbuilder.py b/pydoctor/astbuilder.py index aa35626cc..17c8907d3 100644 --- a/pydoctor/astbuilder.py +++ b/pydoctor/astbuilder.py @@ -88,9 +88,11 @@ def is_constant(obj: model.Attribute, @note: Must be called after setting obj.annotation to detect variables using Final. """ + if is_using_typing_final(annotation, obj): + return True if not is_attribute_overridden(obj, value) and value: if not any(isinstance(n, _CONTROL_FLOW_BLOCKS) for n in get_parents(value)): - return obj.name.isupper() or is_using_typing_final(annotation, obj) + return obj.name.isupper() return False class TypeAliasVisitorExt(extensions.ModuleVisitorExt): @@ -217,6 +219,14 @@ def _infer_attr_annotations(self, scope: model.Documentable) -> None: if attrib.annotation is None and attrib.value is not None: # do not override explicit annotation attrib.annotation = infer_type(attrib.value) + + def _tweak_constants_annotations(self, scope: model.Documentable) -> None: + # tweak constants annotations whenwe leave the scope so we can still + # check whether the annotation uses Final while we're visiting other nodes. + for attrib in scope.contents.values(): + if not isinstance(attrib, model.Attribute): + continue + self._tweak_constant_annotation(attrib) def visit_If(self, node: ast.If) -> None: if isinstance(node.test, ast.Compare): @@ -259,6 +269,7 @@ def visit_Module(self, node: ast.Module) -> None: epydoc2stan.extract_fields(self.module) def depart_Module(self, node: ast.Module) -> None: + self._tweak_constants_annotations(self.builder.current) self._infer_attr_annotations(self.builder.current) self.builder.pop(self.module) @@ -347,6 +358,7 @@ def visit_ClassDef(self, node: ast.ClassDef) -> None: def depart_ClassDef(self, node: ast.ClassDef) -> None: + self._tweak_constants_annotations(self.builder.current) self._infer_attr_annotations(self.builder.current) self.builder.popClass() @@ -563,29 +575,31 @@ def _handleConstant(cls, obj:model.Attribute, defaultKind:model.DocumentableKind) -> None: if is_constant(obj, annotation=annotation, value=value): obj.kind = model.DocumentableKind.CONSTANT - cls._tweakConstantAnnotation(obj=obj, annotation=annotation, - value=value, lineno=lineno) + # do not call tweak annotation just yet... elif obj.kind is model.DocumentableKind.CONSTANT: - obj.kind = defaultKind + # reset to the default kind only for attributes that were heuristically + # declared as constants + if not is_using_typing_final(obj.annotation, obj): + obj.kind = defaultKind @staticmethod - def _tweakConstantAnnotation(obj: model.Attribute, annotation:Optional[ast.expr], - value: Optional[ast.expr], lineno: int) -> None: + def _tweak_constant_annotation(obj: model.Attribute) -> None: # Display variables annotated with Final with the real type instead. + annotation = obj.annotation if is_using_typing_final(annotation, obj): if isinstance(annotation, ast.Subscript): try: 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.report(str(e), section='ast', lineno_offset=annotation.lineno-obj.linenumber) + obj.annotation = infer_type(obj.value) if obj.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(obj.value) if obj.value else None @staticmethod def _setAttributeAnnotation(obj: model.Attribute, diff --git a/pydoctor/test/test_astbuilder.py b/pydoctor/test/test_astbuilder.py index bf45246fb..5678753f4 100644 --- a/pydoctor/test/test_astbuilder.py +++ b/pydoctor/test/test_astbuilder.py @@ -1953,7 +1953,7 @@ def test_not_a_constant_module(systemcls: Type[model.System], capsys:CapSys) -> THING = 'EN' OTHER = 1 OTHER += 1 - E: typing.Final = 2 + E: typing.Final = 2 # it's considered a constant because it's explicitely marked Final E = 4 LIST = [2.14] LIST.insert(0,0) @@ -1961,7 +1961,7 @@ def test_not_a_constant_module(systemcls: Type[model.System], capsys:CapSys) -> assert mod.contents['LANG'].kind is model.DocumentableKind.VARIABLE assert mod.contents['THING'].kind is model.DocumentableKind.VARIABLE assert mod.contents['OTHER'].kind is model.DocumentableKind.VARIABLE - assert mod.contents['E'].kind is model.DocumentableKind.VARIABLE + assert mod.contents['E'].kind is model.DocumentableKind.CONSTANT # all-caps mutables variables are flagged as constant: this is a trade-off # in between our weeknesses in terms static analysis (that is we don't recognized list modifications) @@ -3250,3 +3250,32 @@ def test_inline_docstring_at_wrong_place(systemcls: Type[model.System], capsys: assert not mod.contents['c'].docstring assert not mod.contents['d'].docstring assert not mod.contents['e'].docstring + +@systemcls_param +def test_Final_constant_under_control_flow_block_is_still_constant(systemcls: Type[model.System], capsys: CapSys) -> None: + """ + Test for issue https://github.com/twisted/pydoctor/issues/818 + """ + src = ''' + import sys, random, typing as t + if sys.version_info > (3,10): + v:t.Final = 1 + else: + v:t.Final = 2 + + if random.choice([True, False]): + w:t.Final = 1 + else: + w:t.Final = 2 + + x: t.Final + x = 34 + ''' + + mod = fromText(src, systemcls=systemcls) + assert not capsys.readouterr().out + + assert mod.contents['v'].kind == model.DocumentableKind.CONSTANT + assert mod.contents['w'].kind == model.DocumentableKind.CONSTANT + assert mod.contents['x'].kind == model.DocumentableKind.CONSTANT +