Follow-up on #780 (#784)

* Fix the function to upgrade annotations. Respect annotations rules like in our linker.

pydoctor/astutils.py

@@ -115,11 +115,21 @@ def node2dottedname(node: Optional[ast.AST]) -> Optional[List[str]]:
     parts.reverse()
     return parts
 
-def node2fullname(expr: Optional[ast.AST], ctx: 'model.Documentable') -> Optional[str]:
+def node2fullname(expr: Optional[ast.AST], 
+                  ctx: model.Documentable | None = None, 
+                  *,
+                  expandName:Callable[[str], str] | None = None) -> Optional[str]:
+    if expandName is None:
+        if ctx is None:
+            raise TypeError('this function takes exactly two arguments')
+        expandName = ctx.expandName
+    elif ctx is not None:
+        raise TypeError('this function takes exactly two arguments')
+
     dottedname = node2dottedname(expr)
     if dottedname is None:
         return None
-    return ctx.expandName('.'.join(dottedname))
+    return expandName('.'.join(dottedname))
 
 def bind_args(sig: Signature, call: ast.Call) -> BoundArguments:
     """
@@ -294,7 +304,7 @@ def visit(self, node:ast.AST) -> ast.expr:...
 
     def __init__(self, ctx: model.Documentable) -> None:
         def _node2fullname(node:ast.expr) -> str | None:
-            return node2fullname(node, ctx)
+            return node2fullname(node, expandName=ctx.expandAnnotationName)
         self.node2fullname = _node2fullname
 
     def _union_args_to_bitor(self, args: list[ast.expr], ctxnode:ast.AST) -> ast.BinOp:
@@ -359,6 +369,10 @@ def visit_Subscript(self, node: ast.Subscript) -> ast.expr:
         "typing.FrozenSet": 'frozenset',
 }
 
+# These do not belong in the deprecated builtins aliases, so we make sure it doesn't happen.
+assert 'typing.Union' not in DEPRECATED_TYPING_ALIAS_BUILTINS
+assert 'typing.Optional' not in DEPRECATED_TYPING_ALIAS_BUILTINS
+
 TYPING_ALIAS = (
         "typing.Hashable",
         "typing.Awaitable",

pydoctor/linker.py

@@ -131,8 +131,11 @@ def look_for_intersphinx(self, name: str) -> Optional[str]:
         """
         return self.obj.system.intersphinx.getLink(name)
 
-    def link_to(self, identifier: str, label: "Flattenable") -> Tag:
-        fullID = self.obj.expandName(identifier)
+    def link_to(self, identifier: str, label: "Flattenable", *, is_annotation: bool = False) -> Tag:
+        if is_annotation:
+            fullID = self.obj.expandAnnotationName(identifier)
+        else:
+            fullID = self.obj.expandName(identifier)
 
         target = self.obj.system.objForFullName(fullID)
         if target is not None:
@@ -250,8 +253,7 @@ def __init__(self, obj:'model.Documentable') -> None:
         self._obj = obj
         self._module = obj.module
         self._scope = obj.parent or obj
-        self._module_linker = self._module.docstring_linker
-        self._scope_linker = self._scope.docstring_linker
+        self._scope_linker = _EpydocLinker(self._scope)
 
     @property
     def obj(self) -> 'model.Documentable':
@@ -272,18 +274,13 @@ def link_to(self, target: str, label: "Flattenable") -> Tag:
         with self.switch_context(self._obj):
             if self._module.isNameDefined(target):
                 self.warn_ambiguous_annotation(target)
-                return self._module_linker.link_to(target, label)
-            elif self._scope.isNameDefined(target):
-                return self._scope_linker.link_to(target, label)
-            else:
-                return self._module_linker.link_to(target, label)
+            return self._scope_linker.link_to(target, label, is_annotation=True)
 
     def link_xref(self, target: str, label: "Flattenable", lineno: int) -> Tag:
         with self.switch_context(self._obj):
             return self.obj.docstring_linker.link_xref(target, label, lineno)
 
     @contextlib.contextmanager
     def switch_context(self, ob:Optional['model.Documentable']) -> Iterator[None]:
-        with self._module_linker.switch_context(ob):
-            with self._scope_linker.switch_context(ob):
-                yield
+        with self._scope_linker.switch_context(ob):
+            yield

pydoctor/model.py

@@ -346,6 +346,17 @@ class E:
             obj = nxt
         return '.'.join([full_name] + parts[i + 1:])
 
+    def expandAnnotationName(self, name: str) -> str:
+        """
+        Like L{expandName} but gives precedence to the module scope when a 
+        name is defined both in the current scope and the module scope.
+        """
+        if self.module.isNameDefined(name):
+            return self.module.expandName(name)
+        elif self.isNameDefined(name):
+            return self.expandName(name)
+        return self.module.expandName(name)
+
     def resolveName(self, name: str) -> Optional['Documentable']:
         """Return the object named by "name" (using Python's lookup rules) in
         this context, if any is known to pydoctor."""

pydoctor/test/test_astbuilder.py

@@ -1408,6 +1408,9 @@ def test_upgrade_annotation(systemcls: Type[model.System]) -> None:
     f: Union[(str,)]
     g: Optional[1, 2] # wrong, so we don't process it
     h: Union[list[str]]
+  
+    class List:
+        Union: Union[a, b]
     ''', systemcls=systemcls)
     assert ann_str_and_line(mod.contents['a']) == ('str | int', 2)
     assert ann_str_and_line(mod.contents['b']) == ('str | None', 3)
@@ -1417,6 +1420,7 @@ def test_upgrade_annotation(systemcls: Type[model.System]) -> None:
     assert ann_str_and_line(mod.contents['f']) == ('str', 7)
     assert ann_str_and_line(mod.contents['g']) == ('Optional[1, 2]', 8)
     assert ann_str_and_line(mod.contents['h']) == ('list[str]', 9)
+    assert ann_str_and_line(mod.contents['List'].contents['Union']) == ('a | b', 12)
 
 @pytest.mark.parametrize('annotation', ("[", "pass", "1 ; 2"))
 @systemcls_param