Skip to content

Commit

Permalink
Get rid of the AnnotationLinker - drop the verbose messages when an a…
Browse files Browse the repository at this point in the history
…nnotation is ambiguous (pydoctor is not a checker).

Get rid of ParsedDocstring.with_linker() this API did not make sens and was rather an anti-pattern.

Replace that with a new parameter of colorize_pyval(is_annotation: bool). obj_reference nodes can now be created with the attribute 'is_annotation' in which case they will be resolved a little bit differently.
  • Loading branch information
tristanlatr committed Nov 16, 2024
1 parent cd257eb commit 907792a
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 137 deletions.
54 changes: 22 additions & 32 deletions pydoctor/epydoc/markup/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,15 +211,6 @@ def to_text(self) -> str:
doc = self.to_node()
return ''.join(node2stan.gettext(doc))

def with_linker(self, linker: DocstringLinker) -> ParsedDocstring:
"""
Pre-set the linker object for this parsed docstring.
Whatever is passed to L{to_stan()} will be ignored.
"""
l = linker
return WrappedParsedDocstring(self,
to_stan=lambda this, _: this.to_stan(l))

def with_tag(self, tag: Tag) -> ParsedDocstring:
"""
Wraps the L{to_stan()} result inside the given tag.
Expand All @@ -228,18 +219,7 @@ def with_tag(self, tag: Tag) -> ParsedDocstring:
With this trick, the main tag is preserved. It can also be used to add
a custom CSS class on top of an existing parsed docstring.
"""
# We double wrap it with a transparent tag so the added tags survives ParsedDocstring.combine
# wich combines the content of the main div of the stan, not the div itself.
def to_stan(this: ParsedDocstring, linker: DocstringLinker) -> Tag:
# Since the stan is cached inside _stan attribute we can't simply use
# "lambda this, linker: tags.transparent(t(this.to_stan(linker)))" as the new to_stan method.
# this would not behave correctly because each time to_stan will be called, the content would be duplicated.
if this._stan is not None:
return this._stan
this._stan = Tag('')(tag(this.to_stan(linker)))
return this._stan

return WrappedParsedDocstring(self, to_stan)
return _ParsedDocstringWithTag(self, tag)

@classmethod
def combine(cls, elements: Sequence[ParsedDocstring]) -> ParsedDocstring:
Expand Down Expand Up @@ -299,28 +279,36 @@ def to_stan(self, linker: DocstringLinker) -> Tag:
stan(e.to_stan(linker).children)
return stan

class WrappedParsedDocstring(ParsedDocstring):
class _ParsedDocstringWithTag(ParsedDocstring):
"""
Wraps a parsed docstring to suppplement the to_stan() method.
Wraps a parsed docstring to wrap the result of the
the to_stan() method inside a custom Tag.
"""
def __init__(self,
other: ParsedDocstring,
to_stan: Callable[[ParsedDocstring, DocstringLinker], Tag]):
tag: Tag):
super().__init__(other.fields)
self.wrapped = other
"""
The wrapped parsed docstring.
"""
self._to_stan = to_stan

def to_stan(self, docstring_linker: DocstringLinker) -> Tag:
return self._to_stan(self.wrapped, docstring_linker)
self._tag = tag
self._stan: Tag | None = None

# We double wrap it with a transparent tag so the added tags survives ParsedDocstring.combine
# wich combines the content of the main div of the stan, not the div itself.
def to_stan(self, linker: DocstringLinker) -> Tag:
# Since the stan is cached inside _stan attribute we can't simply use
# "lambda this, linker: tags.transparent(self._tag(this.to_stan(linker)))" as the new to_stan method.
# this would not behave correctly because each time to_stan will be called, the content would be duplicated.
if (stan:=self._stan) is not None:
return stan
self._stan = stan = Tag('')(self._tag(self.wrapped.to_stan(linker)))
return stan

# Boring

def to_node(self) -> nodes.document:
return self.wrapped.to_node()

@property
def has_body(self) -> bool:
return self.wrapped.has_body

Check warning on line 314 in pydoctor/epydoc/markup/__init__.py

View check run for this annotation

Codecov / codecov/patch

pydoctor/epydoc/markup/__init__.py#L314

Added line #L314 was not covered by tests
Expand Down Expand Up @@ -388,14 +376,16 @@ class DocstringLinker(Protocol):
target URL for crossreference links.
"""

def link_to(self, target: str, label: "Flattenable") -> Tag:
def link_to(self, target: str, label: "Flattenable", *, is_annotation: bool = False) -> Tag:
"""
Format a link to a Python identifier.
This will resolve the identifier like Python itself would.
@param target: The name of the Python identifier that
should be linked to.
@param label: The label to show for the link.
@param is_annotation: Generated links will give precedence to the module
defined varaible rather the nested definitions when there are name colisions.
@return: The link, or just the label if the target was not found.
"""

Expand Down Expand Up @@ -430,7 +420,7 @@ def switch_context(self, ob:Optional['Documentable']) -> ContextManager[None]:
class NotFoundLinker(DocstringLinker):
"""A DocstringLinker implementation that cannot find any links."""

def link_to(self, target: str, label: "Flattenable") -> Tag:
def link_to(self, target: str, label: "Flattenable", *, is_annotation: bool = False) -> Tag:
return tags.transparent(label)

def link_xref(self, target: str, label: "Flattenable", lineno: int) -> Tag:
Expand Down
31 changes: 21 additions & 10 deletions pydoctor/epydoc/markup/_pyval_repr.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,9 @@ def __init__(self, document: nodes.document, is_complete: bool, warnings: List[s
def to_stan(self, docstring_linker: DocstringLinker) -> Tag:
return Tag('code')(super().to_stan(docstring_linker))

def colorize_pyval(pyval: Any, linelen:Optional[int], maxlines:int, linebreakok:bool=True, refmap:Optional[Dict[str, str]]=None) -> ColorizedPyvalRepr:
def colorize_pyval(pyval: Any, linelen:Optional[int], maxlines:int,
linebreakok:bool=True, refmap:Optional[Dict[str, str]]=None,
is_annotation: bool = False) -> ColorizedPyvalRepr:
"""
Get a L{ColorizedPyvalRepr} instance for this piece of ast.
Expand All @@ -209,14 +211,15 @@ def colorize_pyval(pyval: Any, linelen:Optional[int], maxlines:int, linebreakok:
This can be used for cases the where the linker might be wrong, obviously this is just a workaround.
@return: A L{ColorizedPyvalRepr} describing the given pyval.
"""
return PyvalColorizer(linelen=linelen, maxlines=maxlines, linebreakok=linebreakok, refmap=refmap).colorize(pyval)
return PyvalColorizer(linelen=linelen, maxlines=maxlines, linebreakok=linebreakok,
refmap=refmap, is_annotation=is_annotation).colorize(pyval)

def colorize_inline_pyval(pyval: Any, refmap:Optional[Dict[str, str]]=None) -> ColorizedPyvalRepr:
def colorize_inline_pyval(pyval: Any, refmap:Optional[Dict[str, str]]=None, is_annotation: bool = False) -> ColorizedPyvalRepr:
"""
Used to colorize type annotations and parameters default values.
@returns: C{L{colorize_pyval}(pyval, linelen=None, linebreakok=False)}
"""
return colorize_pyval(pyval, linelen=None, maxlines=1, linebreakok=False, refmap=refmap)
return colorize_pyval(pyval, linelen=None, maxlines=1, linebreakok=False, refmap=refmap, is_annotation=is_annotation)

def _get_str_func(pyval: AnyStr) -> Callable[[str], AnyStr]:
func = cast(Callable[[str], AnyStr], str if isinstance(pyval, str) else \
Expand Down Expand Up @@ -261,14 +264,17 @@ def _bytes_escape(b: bytes) -> str:

class PyvalColorizer:
"""
Syntax highlighter for Python values.
Syntax highlighter for Python AST (and some builtins types).
"""

def __init__(self, linelen:Optional[int], maxlines:int, linebreakok:bool=True, refmap:Optional[Dict[str, str]]=None):
def __init__(self, linelen:Optional[int], maxlines:int, linebreakok:bool=True,
refmap:Optional[Dict[str, str]]=None, is_annotation: bool = False):
self.linelen: Optional[int] = linelen if linelen!=0 else None
self.maxlines: Union[int, float] = maxlines if maxlines!=0 else float('inf')
self.linebreakok = linebreakok
self.refmap = refmap if refmap is not None else {}
self.is_annotation = is_annotation

# some edge cases require to compute the precedence ahead of time and can't be
# easily done with access only to the parent node of some operators.
self.explicit_precedence:Dict[ast.AST, int] = {}
Expand All @@ -284,7 +290,7 @@ def __init__(self, linelen:Optional[int], maxlines:int, linebreakok:bool=True, r
NUMBER_TAG = None # ints, floats, etc
QUOTE_TAG = 'variable-quote' # Quotes around strings.
STRING_TAG = 'variable-string' # Body of string literals
LINK_TAG = 'variable-link' # Links to other documentables, extracted from AST names and attributes.
LINK_TAG = None # Links, we don't use an explicit class here, but in node2stan.
ELLIPSIS_TAG = 'variable-ellipsis'
LINEWRAP_TAG = 'variable-linewrap'
UNKNOWN_TAG = 'variable-unknown'
Expand Down Expand Up @@ -1017,16 +1023,21 @@ def _output(self, s: AnyStr, css_class: Optional[str],
if (self.linelen is None or
state.charpos + segment_len <= self.linelen
or link is True
or css_class in ('variable-quote',)):
or css_class in (self.QUOTE_TAG,)):

state.charpos += segment_len

if link is True:
# Here, we bypass the linker if refmap contains the segment we're linking to.
# The linker can be problematic because it has some design blind spots when the same name is declared in the imports and in the module body.
# The linker can be problematic because it has some design blind spots when
# the same name is declared in the imports and in the module body.

# Note that the argument name is 'refuri', not 'refuid.
element = obj_reference('', segment, refuri=self.refmap.get(segment, segment))
element = obj_reference('', segment,
refuri=self.refmap.get(segment, segment))
if self.is_annotation:
# Don't set the attribute if it's not True.
element.attributes['is_annotation'] = True
elif css_class is not None:
element = nodes.inline('', segment, classes=[css_class])
else:
Expand Down
13 changes: 5 additions & 8 deletions pydoctor/epydoc2stan.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,10 +274,10 @@ def __init__(self, obj: model.Documentable):
def set_param_types_from_annotations(
self, annotations: Mapping[str, Optional[ast.expr]]
) -> None:
_linker = linker._AnnotationLinker(self.obj)
_linker = self.obj.docstring_linker
formatted_annotations = {
name: None if value is None
else ParamType(safe_to_stan(colorize_inline_pyval(value), _linker,
else ParamType(safe_to_stan(colorize_inline_pyval(value, is_annotation=True), _linker,
self.obj, fallback=colorized_pyval_fallback, section='annotation', report=False),
# don't spam the log, invalid annotation are going to be reported when the signature gets colorized
origin=FieldOrigin.FROM_AST)
Expand Down Expand Up @@ -853,8 +853,7 @@ def type2stan(obj: model.Documentable) -> Optional[Tag]:
if parsed_type is None:
return None
else:
_linker = linker._AnnotationLinker(obj)
return safe_to_stan(parsed_type, _linker, obj,
return safe_to_stan(parsed_type, obj.docstring_linker, obj,
fallback=colorized_pyval_fallback, section='annotation')

def get_parsed_type(obj: model.Documentable) -> Optional[ParsedDocstring]:
Expand All @@ -868,7 +867,7 @@ def get_parsed_type(obj: model.Documentable) -> Optional[ParsedDocstring]:
# Only Attribute instances have the 'annotation' attribute.
annotation: Optional[ast.expr] = getattr(obj, 'annotation', None)
if annotation is not None:
return colorize_inline_pyval(annotation)
return colorize_inline_pyval(annotation, is_annotation=True)

return None

Expand Down Expand Up @@ -1166,9 +1165,7 @@ def _colorize_signature_annotation(annotation: object,
Returns L{ParsedDocstring} with extra context to make
sure we resolve tha annotation correctly.
"""
return colorize_inline_pyval(annotation
# Make sure to use the annotation linker in the context of an annotation.
).with_linker(linker._AnnotationLinker(ctx)
return colorize_inline_pyval(annotation, is_annotation=True,
# Make sure the generated <code> tags are not stripped by ParsedDocstring.combine.
).with_tag(tags.transparent)

Expand Down
45 changes: 1 addition & 44 deletions pydoctor/linker.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def look_for_intersphinx(self, name: str) -> Optional[str]:

def link_to(self, identifier: str, label: "Flattenable", *, is_annotation: bool = False) -> Tag:
if is_annotation:
fullID = self.obj.expandAnnotationName(identifier)
fullID = (self.obj.parent or self.obj).expandAnnotationName(identifier)
else:
fullID = self.obj.expandName(identifier)

Expand Down Expand Up @@ -241,46 +241,3 @@ def _resolve_identifier_xref(self,
if self.reporting_obj:
self.reporting_obj.report(message, 'resolve_identifier_xref', lineno)
raise LookupError(identifier)

class _AnnotationLinker(DocstringLinker):
"""
Specialized linker to resolve annotations attached to the given L{Documentable}.
Links will be created in the context of C{obj} but
generated with the C{obj.module}'s linker when possible.
"""
def __init__(self, obj:'model.Documentable') -> None:
self._obj = obj
self._module = obj.module
self._scope = obj.parent or obj
self._scope_linker = _EpydocLinker(self._scope)

@property
def obj(self) -> 'model.Documentable':
return self._obj

def warn_ambiguous_annotation(self, target:str) -> None:
# report a low-level message about ambiguous annotation
mod_ann = self._module.expandName(target)
obj_ann = self._scope.expandName(target)
if mod_ann != obj_ann and '.' in obj_ann and '.' in mod_ann:
self.obj.report(
f'ambiguous annotation {target!r}, could be interpreted as '
f'{obj_ann!r} instead of {mod_ann!r}', section='annotation',
thresh=1
)

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._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._scope_linker.switch_context(ob):
yield
10 changes: 7 additions & 3 deletions pydoctor/node2stan.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"""
from __future__ import annotations

from functools import partial
from itertools import chain
import re
import optparse
Expand Down Expand Up @@ -99,17 +100,20 @@ def __init__(self,
super().__init__(document)

# don't allow <h1> tags, start at <h2>
# h1 is reserved for the page nodes.title.
# h1 is reserved for the page title.
self.section_level += 1

# Handle interpreted text (crossreferences)
def visit_title_reference(self, node: nodes.title_reference) -> None:
lineno = get_lineno(node)
self._handle_reference(node, link_func=lambda target, label: self._linker.link_xref(target, label, lineno))
self._handle_reference(node, link_func=partial(self._linker.link_xref, lineno=lineno))

# Handle internal references
def visit_obj_reference(self, node: obj_reference) -> None:
self._handle_reference(node, link_func=self._linker.link_to)
if node.attributes.get('is_annotation'):
self._handle_reference(node, link_func=partial(self._linker.link_to, is_annotation=True))
else:
self._handle_reference(node, link_func=self._linker.link_to)

def _handle_reference(self, node: nodes.title_reference, link_func: Callable[[str, "Flattenable"], "Flattenable"]) -> None:
label: "Flattenable"
Expand Down
7 changes: 4 additions & 3 deletions pydoctor/templatewriter/pages/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from twisted.web.template import Element, Tag, renderer, tags, CharRef
from pydoctor.extensions import zopeinterface

from pydoctor import epydoc2stan, model, linker, __version__
from pydoctor import epydoc2stan, model, __version__
from pydoctor.astbuilder import node2fullname
from pydoctor.templatewriter import util, TemplateLookup, TemplateElement
from pydoctor.templatewriter.pages.table import ChildTable
Expand Down Expand Up @@ -85,7 +85,7 @@ def format_class_signature(cls: model.Class) -> "Flattenable":
# the linker will only be used to resolve the generic arguments of the base classes,
# it won't actually resolve the base classes (see comment few lines below).
# this is why we're using the annotation linker.
_linker = linker._AnnotationLinker(cls)
_linker = cls.docstring_linker
if cls.rawbases:
r.append('(')

Expand All @@ -105,7 +105,8 @@ def format_class_signature(cls: model.Class) -> "Flattenable":

# link to external class, using the colorizer here
# to link to classes with generics (subscripts and other AST expr).
stan = epydoc2stan.safe_to_stan(colorize_inline_pyval(base_node, refmap=refmap), _linker, cls,
# we use is_annotation=True because bases are unstringed, they can contain annotations.
stan = epydoc2stan.safe_to_stan(colorize_inline_pyval(base_node, refmap=refmap, is_annotation=True), _linker, cls,
fallback=epydoc2stan.colorized_pyval_fallback,
section='rendering of class signature')
r.extend(stan.children)
Expand Down
25 changes: 23 additions & 2 deletions pydoctor/test/epydoc/test_pyval_repr.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
from pydoctor.test import NotFoundLinker
from pydoctor.stanutils import flatten, flatten_text, html2stan

def color(v: Any, linebreakok:bool=True, maxlines:int=5, linelen:int=40) -> str:
colorizer = PyvalColorizer(linelen=linelen, linebreakok=linebreakok, maxlines=maxlines)
def color(v: Any, linebreakok:bool=True, maxlines:int=5, linelen:int=40, is_annotation: bool = False) -> str:
colorizer = PyvalColorizer(linelen=linelen, linebreakok=linebreakok, maxlines=maxlines, is_annotation=is_annotation)
parsed_doc = colorizer.colorize(v)
return parsed_doc.to_node().pformat()

Expand Down Expand Up @@ -1576,3 +1576,24 @@ def test_expressions_parens(subtests:Any) -> None:
check_src("{**({} == {})}")
check_src("{**{'y': 2}, 'x': 1, None: True}")
check_src("{**{'y': 2}, **{'x': 1}}")


def test_is_annotation_flag() -> None:

# If a line goes beyond linelen, it is wrapped using the ``&crarr;`` element.
# Check that the last line gets a ``&crarr;`` when maxlines is exceeded:

assert color(extract_expr(ast.parse('list[dict] + set()')), is_annotation=True) == '''<document source="pyval_repr">
<obj_reference is_annotation="True" refuri="list">
list
[
<wbr>
<obj_reference is_annotation="True" refuri="dict">
dict
]
+
<obj_reference is_annotation="True" refuri="set">
set
(
)
'''
Loading

0 comments on commit 907792a

Please sign in to comment.