Re: [PATCH 24/27] docs/qapi-domain: add type cross-refs to field lists

[John Snow]

On Fri, Apr 19, 2024 at 12:38 AM John Snow <jsnow@redhat.com> wrote:
>
> This commit, finally, adds cross-referencing support to various field
> lists; modeled tightly after Sphinx's own Python domain code.
>
> Cross-referencing support is added to type names provided to :arg:,
> :memb:, :returns: and :choice:.
>
> :feat:, :error: and :value:, which do not take type names, do not
> support this syntax.
>
> The general syntax is simple:
>
> :arg TypeName ArgName: Lorem Ipsum ...
>
> The domain will transform TypeName into :qapi:type:`TypeName` in this
> basic case, and also apply the ``literal`` decoration to indicate that
> this is a type cross-reference.
>
> For Optional arguments, the special "?" suffix is used. Because "*" has
> special meaning in ReST that would cause parsing errors, we elect to use
> "?" instead. The special syntax processing in QAPIXrefMixin strips this
> character from the end of any type name argument and will append ",
> Optional" to the rendered output, applying the cross-reference only to
> the actual type name.
>
> The intent here is that the actual syntax in doc-blocks need not change;
> but e.g. qapidoc.py will need to process and transform "@arg foo lorem
> ipsum" into ":arg type? foo: lorem ipsum" based on the schema
> information. Therefore, nobody should ever actually witness this
> intermediate syntax unless they are writing manual documentation or the
> doc transmogrifier breaks.
>
> For array arguments, type names can similarly be surrounded by "[]",
> which are stripped off and then re-appended outside of the
> cross-reference.
>
> Note: The mixin pattern here (borrowed from Sphinx) confuses mypy
> because it cannot tell that it will be mixed into a descendent of
> Field. Doing that instead causes more errors, because many versions of
> Sphinx erroneously did not mark various arguments as Optional, so we're
> a bit hosed either way. Do the simpler thing.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  docs/qapi/index.rst        |  34 ++++++++++++
>  docs/sphinx/qapi-domain.py | 110 +++++++++++++++++++++++++++++++++++--
>  2 files changed, 138 insertions(+), 6 deletions(-)
>
> diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
> index 8352a27d4a5..6e85ea5280d 100644
> --- a/docs/qapi/index.rst
> +++ b/docs/qapi/index.rst
> @@ -105,6 +105,11 @@ Explicit cross-referencing syntax for QAPI modules is 
> available with
>     :arg str bar: Another normal parameter description.
>     :arg baz: Missing a type.
>     :arg no-descr:
> +   :arg int? oof: Testing optional argument parsing.
> +   :arg [XDbgBlockGraphNode] rab: Testing array argument parsing.
> +   :arg [BitmapSyncMode]? zab: Testing optional array argument parsing,
> +      even though Markus said this should never happen. I believe him,
> +      but I didn't *forbid* the syntax either.
>     :arg BitmapSyncMode discrim: How about branches in commands?
>
>     .. qapi:branch:: discrim on-success
> @@ -261,3 +266,32 @@ Explicit cross-referencing syntax for QAPI modules is 
> available with
>
>        :memb str key-secret: ID of a QCryptoSecret object providing a
>           passphrase for unlocking the encryption
> +
> +.. qapi:command:: x-debug-query-block-graph
> +   :since: 4.0
> +   :unstable:
> +
> +   Get the block graph.
> +
> +   :feat unstable: This command is meant for debugging.
> +   :return XDbgBlockGraph: lorem ipsum ...
> +
> +.. qapi:struct:: XDbgBlockGraph
> +   :since: 4.0
> +
> +   Block Graph - list of nodes and list of edges.
> +
> +   :memb [XDbgBlockGraphNode] nodes:
> +   :memb [XDbgBlockGraphEdge] edges:
> +
> +.. qapi:struct:: XDbgBlockGraphNode
> +   :since: 4.0
> +
> +   :memb uint64 id: Block graph node identifier.  This @id is generated only 
> for
> +      x-debug-query-block-graph and does not relate to any other
> +      identifiers in Qemu.
> +   :memb XDbgBlockGraphNodeType type: Type of graph node.  Can be one of
> +      block-backend, block-job or block-driver-state.
> +   :memb str name: Human readable name of the node.  Corresponds to
> +      node-name for block-driver-state nodes; is not guaranteed to be
> +      unique in the whole graph (with block-jobs and block-backends).
> diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
> index bf8bb933345..074453193ce 100644
> --- a/docs/sphinx/qapi-domain.py
> +++ b/docs/sphinx/qapi-domain.py
> @@ -50,11 +50,12 @@
>
>  if TYPE_CHECKING:
>      from docutils.nodes import Element, Node
> +    from docutils.parsers.rst.states import Inliner
>
>      from sphinx.application import Sphinx
>      from sphinx.builders import Builder
>      from sphinx.environment import BuildEnvironment
> -    from sphinx.util.typing import OptionSpec
> +    from sphinx.util.typing import OptionSpec, TextlikeNode
>
>  logger = logging.getLogger(__name__)
>
> @@ -68,6 +69,90 @@ class ObjectEntry(NamedTuple):
>      aliased: bool
>
>
> +class QAPIXrefMixin:
> +    def make_xref(
> +        self,
> +        rolename: str,
> +        domain: str,
> +        target: str,
> +        innernode: type[TextlikeNode] = nodes.literal,
> +        contnode: Optional[Node] = None,
> +        env: Optional[BuildEnvironment] = None,
> +        inliner: Optional[Inliner] = None,
> +        location: Optional[Node] = None,
> +    ) -> Node:
> +        result = super().make_xref(  # type: ignore[misc]
> +            rolename,
> +            domain,
> +            target,
> +            innernode=innernode,
> +            contnode=contnode,
> +            env=env,
> +            inliner=None,
> +            location=None,
> +        )
> +        if isinstance(result, pending_xref):
> +            assert env is not None
> +            result["refspecific"] = True
> +            result["qapi:module"] = env.ref_context.get("qapi:module")
> +
> +        assert isinstance(result, Node)

A bug snuck in because I made edits after I published to GitLab; this
line should be:

assert isinstance(result, nodes.Node)

> +        return result
> +
> +    def make_xrefs(
> +        self,
> +        rolename: str,
> +        domain: str,
> +        target: str,
> +        innernode: type[TextlikeNode] = nodes.literal,
> +        contnode: Optional[Node] = None,
> +        env: Optional[BuildEnvironment] = None,
> +        inliner: Optional[Inliner] = None,
> +        location: Optional[Node] = None,
> +    ) -> list[Node]:
> +        # Note: this function is called on up to three fields of text:
> +        # (1) The field name argument (e.g. member/arg name)
> +        # (2) The field name type (e.g. member/arg type)
> +        # (3) The field *body* text, for Fields that do not take arguments.
> +
> +        list_type = False
> +        optional = False
> +
> +        # If the rolename is qapi:type, we know we are processing a type
> +        # and not an arg/memb name or field body text.
> +        if rolename == "type":
> +            # force the innernode class to be a literal.
> +            innernode = nodes.literal
> +
> +            # Type names that end with "?" are considered Optional
> +            # arguments and should be documented as such, but it's not
> +            # part of the xref itself.
> +            if target.endswith("?"):
> +                optional = True
> +                target = target[:-1]
> +
> +            # Type names wrapped in brackets denote lists. strip the
> +            # brackets and remember to add them back later.
> +            if target.startswith("[") and target.endswith("]"):
> +                list_type = True
> +                target = target[1:-1]
> +
> +        results = []
> +        result = self.make_xref(
> +            rolename, domain, target, innernode, contnode, env, inliner, 
> location
> +        )
> +        results.append(result)
> +
> +        if list_type:
> +            results.insert(0, nodes.literal("[", "["))
> +            results.append(nodes.literal("]", "]"))
> +        if optional:
> +            results.append(nodes.Text(", "))
> +            results.append(nodes.emphasis("?", "Optional"))
> +
> +        return results
> +
> +
>  class QAPIXRefRole(XRefRole):
>      def process_link(
>          self,
> @@ -96,6 +181,14 @@ def process_link(
>          return title, target
>
>
> +class QAPIGroupedField(QAPIXrefMixin, GroupedField):
> +    pass
> +
> +
> +class QAPITypedField(QAPIXrefMixin, TypedField):
> +    pass
> +
> +
>  def since_validator(param: str) -> str:
>      """
>      Validate the `:since: X.Y` option field.
> @@ -416,10 +509,11 @@ class QAPICommand(QAPIObject):
>      doc_field_types = QAPIObject.doc_field_types.copy()
>      doc_field_types.extend(
>          [
> -            TypedField(
> +            QAPITypedField(
>                  "argument",
>                  label=_("Arguments"),
>                  names=("arg",),
> +                typerolename="type",
>                  can_collapse=True,
>              ),
>              GroupedField(
> @@ -428,9 +522,10 @@ class QAPICommand(QAPIObject):
>                  names=("error",),
>                  can_collapse=True,
>              ),
> -            GroupedField(
> +            QAPIGroupedField(
>                  "returnvalue",
>                  label=_("Returns"),
> +                rolename="type",
>                  names=("return", "returns"),
>                  can_collapse=True,
>              ),
> @@ -460,10 +555,11 @@ class QAPIAlternate(QAPIObject):
>      doc_field_types = QAPIObject.doc_field_types.copy()
>      doc_field_types.extend(
>          [
> -            TypedField(
> +            QAPITypedField(
>                  "choice",
>                  label=_("Choices"),
>                  names=("choice",),
> +                typerolename="type",
>                  can_collapse=True,
>              ),
>          ]
> @@ -476,10 +572,11 @@ class QAPIObjectWithMembers(QAPIObject):
>      doc_field_types = QAPIObject.doc_field_types.copy()
>      doc_field_types.extend(
>          [
> -            TypedField(
> +            QAPITypedField(
>                  "member",
>                  label=_("Members"),
>                  names=("memb",),
> +                typerolename="type",
>                  can_collapse=True,
>              ),
>          ]
> @@ -629,12 +726,13 @@ def run(self) -> list[Node]:
>          # of per-class to incorporate the branch conditions as a label
>          # name.
>          self.doc_field_types = [
> -            TypedField(
> +            QAPITypedField(
>                  "branch-arg-or-memb",
>                  label=f"[{discrim} = {value}]",
>                  # In a branch, we don't actually use the name of the
>                  # field name to generate the label; so allow either-or.
>                  names=("arg", "memb"),
> +                typerolename="type",
>              ),
>          ]
>
> --
> 2.44.0
>