Fixes #818

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
 ^^^^^^^^^^^^^^^^
 

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, 

pydoctor/test/test_astbuilder.py

@@ -1953,15 +1953,15 @@ 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)
     ''', systemcls=systemcls)
     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
+