docs/qapi-domain: add warnings for malformed field lists
Normally, Sphinx will silently fall back to its standard field list processing if it doesn't match one of your defined fields. A lot of the time, that's not what we want - we want to be warned if we goof something up. For instance, the canonical argument field list form is: :arg type name: descr This form is captured by Sphinx and transformed so that the field label will become "Arguments:". It's possible to omit the type name and descr and still have it be processed correctly. However, if you omit the type name, Sphinx no longer recognizes it: :arg: this is not recognized. This will turn into an arbitrary field list entry whose label is "Arg:", and it otherwise silently fails. You may also see failures for doing things like using :values: instead of :value:, or :errors: instead of :error:, and so on. It's also case sensitive, and easy to trip up. Add a validator that guarantees all field list entries that are the direct child of an ObjectDescription use only recognized forms of field lists, and emit a warning (treated as error by default in most build configurations) whenever we detect one that is goofed up. However, there's still benefit to allowing arbitrary fields -- they are after all not a Sphinx invention, but perfectly normal docutils syntax. Create an allow list for known spellings we don't mind letting through, but warn against anything else. Signed-off-by: John Snow <jsnow@redhat.com> Message-ID: <20250311034303.75779-27-jsnow@redhat.com> Acked-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
This commit is contained in:
parent
6a41330206
commit
ef137a2241
@ -153,6 +153,15 @@ rst_epilog = ".. |CONFDIR| replace:: ``" + confdir + "``\n"
|
|||||||
with open(os.path.join(qemu_docdir, 'defs.rst.inc')) as f:
|
with open(os.path.join(qemu_docdir, 'defs.rst.inc')) as f:
|
||||||
rst_epilog += f.read()
|
rst_epilog += f.read()
|
||||||
|
|
||||||
|
|
||||||
|
# Normally, the QAPI domain is picky about what field lists you use to
|
||||||
|
# describe a QAPI entity. If you'd like to use arbitrary additional
|
||||||
|
# fields in source documentation, add them here.
|
||||||
|
qapi_allowed_fields = {
|
||||||
|
"see also",
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
# -- Options for HTML output ----------------------------------------------
|
# -- Options for HTML output ----------------------------------------------
|
||||||
|
|
||||||
# The theme to use for HTML and HTML Help pages. See the documentation for
|
# The theme to use for HTML and HTML Help pages. See the documentation for
|
||||||
|
@ -49,6 +49,19 @@ if TYPE_CHECKING:
|
|||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
|
def _unpack_field(
|
||||||
|
field: nodes.Node,
|
||||||
|
) -> Tuple[nodes.field_name, nodes.field_body]:
|
||||||
|
"""
|
||||||
|
docutils helper: unpack a field node in a type-safe manner.
|
||||||
|
"""
|
||||||
|
assert isinstance(field, nodes.field)
|
||||||
|
assert len(field.children) == 2
|
||||||
|
assert isinstance(field.children[0], nodes.field_name)
|
||||||
|
assert isinstance(field.children[1], nodes.field_body)
|
||||||
|
return (field.children[0], field.children[1])
|
||||||
|
|
||||||
|
|
||||||
class ObjectEntry(NamedTuple):
|
class ObjectEntry(NamedTuple):
|
||||||
docname: str
|
docname: str
|
||||||
node_id: str
|
node_id: str
|
||||||
@ -330,9 +343,64 @@ class QAPIObject(QAPIDescription):
|
|||||||
if infopips.children:
|
if infopips.children:
|
||||||
contentnode.insert(0, infopips)
|
contentnode.insert(0, infopips)
|
||||||
|
|
||||||
|
def _validate_field(self, field: nodes.field) -> None:
|
||||||
|
"""Validate field lists in this QAPI Object Description."""
|
||||||
|
name, _ = _unpack_field(field)
|
||||||
|
allowed_fields = set(self.env.app.config.qapi_allowed_fields)
|
||||||
|
|
||||||
|
field_label = name.astext()
|
||||||
|
if field_label in allowed_fields:
|
||||||
|
# Explicitly allowed field list name, OK.
|
||||||
|
return
|
||||||
|
|
||||||
|
try:
|
||||||
|
# split into field type and argument (if provided)
|
||||||
|
# e.g. `:arg type name: descr` is
|
||||||
|
# field_type = "arg", field_arg = "type name".
|
||||||
|
field_type, field_arg = field_label.split(None, 1)
|
||||||
|
except ValueError:
|
||||||
|
# No arguments provided
|
||||||
|
field_type = field_label
|
||||||
|
field_arg = ""
|
||||||
|
|
||||||
|
typemap = self.get_field_type_map()
|
||||||
|
if field_type in typemap:
|
||||||
|
# This is a special docfield, yet-to-be-processed. Catch
|
||||||
|
# correct names, but incorrect arguments. This mismatch WILL
|
||||||
|
# cause Sphinx to render this field incorrectly (without a
|
||||||
|
# warning), which is never what we want.
|
||||||
|
typedesc = typemap[field_type][0]
|
||||||
|
if typedesc.has_arg != bool(field_arg):
|
||||||
|
msg = f"docfield field list type {field_type!r} "
|
||||||
|
if typedesc.has_arg:
|
||||||
|
msg += "requires an argument."
|
||||||
|
else:
|
||||||
|
msg += "takes no arguments."
|
||||||
|
logger.warning(msg, location=field)
|
||||||
|
else:
|
||||||
|
# This is unrecognized entirely. It's valid rST to use
|
||||||
|
# arbitrary fields, but let's ensure the documentation
|
||||||
|
# writer has done this intentionally.
|
||||||
|
valid = ", ".join(sorted(set(typemap) | allowed_fields))
|
||||||
|
msg = (
|
||||||
|
f"Unrecognized field list name {field_label!r}.\n"
|
||||||
|
f"Valid fields for qapi:{self.objtype} are: {valid}\n"
|
||||||
|
"\n"
|
||||||
|
"If this usage is intentional, please add it to "
|
||||||
|
"'qapi_allowed_fields' in docs/conf.py."
|
||||||
|
)
|
||||||
|
logger.warning(msg, location=field)
|
||||||
|
|
||||||
def transform_content(self, content_node: addnodes.desc_content) -> None:
|
def transform_content(self, content_node: addnodes.desc_content) -> None:
|
||||||
self._add_infopips(content_node)
|
self._add_infopips(content_node)
|
||||||
|
|
||||||
|
# Validate field lists.
|
||||||
|
for child in content_node:
|
||||||
|
if isinstance(child, nodes.field_list):
|
||||||
|
for field in child.children:
|
||||||
|
assert isinstance(field, nodes.field)
|
||||||
|
self._validate_field(field)
|
||||||
|
|
||||||
|
|
||||||
class QAPICommand(QAPIObject):
|
class QAPICommand(QAPIObject):
|
||||||
"""Description of a QAPI Command."""
|
"""Description of a QAPI Command."""
|
||||||
@ -769,6 +837,12 @@ class QAPIDomain(Domain):
|
|||||||
|
|
||||||
def setup(app: Sphinx) -> Dict[str, Any]:
|
def setup(app: Sphinx) -> Dict[str, Any]:
|
||||||
app.setup_extension("sphinx.directives")
|
app.setup_extension("sphinx.directives")
|
||||||
|
app.add_config_value(
|
||||||
|
"qapi_allowed_fields",
|
||||||
|
set(),
|
||||||
|
"env", # Setting impacts parsing phase
|
||||||
|
types=set,
|
||||||
|
)
|
||||||
app.add_domain(QAPIDomain)
|
app.add_domain(QAPIDomain)
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
Loading…
x
Reference in New Issue
Block a user