Clean up new schema code.

- Declare the full child lists of the body-alias elements.
- Verify read-table coverage: every generated row array must be consumed.
- Fix stale attribute facts on hand-read elements.

PiperOrigin-RevId: 958685667
Change-Id: I4a914f3136a5078eb8ca24aa4e162d55afafd923

doc/generate/generate_mjcf_table.py

@@ -130,7 +130,12 @@ def generate() -> str:
         constraints.append(f"  {{{row_index}, '{KIND_CHAR[con.kind]}', "
                            f'"{spec}"}},')
 
-    children = [c for c in element.children() if c.name != element.name]
+    # self-recursion is implied by card R; alias elements (worldbody, frame,
+    # replicate) have no rows -- mjXSchema::NameMatch admits their tags
+    # against the body row
+    children = [c for c in element.children()
+                if c.name != element.name
+                and 'alias' not in schema.elements[c.name].facets]
     if project:
       # plugin configuration is not settable per-class
       children = [c for c in children if c.name != 'plugin']

doc/generate/generate_read_table.py

@@ -21,8 +21,9 @@ shared group arrays.
 Which elements get rows is determined automatically: every schema element with
 a bound spec struct and at least one table-drivable attribute is included,
 unless it appears in NOT_TABLE_DRIVEN (elements whose OneX() readers have
-custom logic). A coverage check in doc_test verifies that every eligible
+custom logic). A coverage check in doc_test verifies that every emitted row
-element is accounted for.
+array is consumed by the reader or writer, and that NOT_TABLE_DRIVEN names
+only elements that exist in the schema.
 
 Field offsets are emitted as offsetof() expressions, so binding mistakes are
 compile errors, and the field's C type (parsed from mjspec.h) selects the row
@@ -154,19 +155,15 @@ _HEADER = '''\
 '''
 
 
+def array_name(element_name):
+  """Return the row-array name emitted for an element."""
+  return f'k{element_name.capitalize()}Attrs'
+
+
 def _has_table_attrs(schema, element):
   """Check if the element has at least one non-custom table-drivable attr."""
-  for member in element.members:
+  return any('reading' not in attr.facets
-    if isinstance(member, mjcf_schema.Attr) and 'reading' not in member.facets:
+             for attr in schema.expanded_attrs(element))
-      return True
-    if isinstance(member, mjcf_schema.Use):
-      group = schema.groups.get(member.group)
-      if group:
-        for gm in group.members:
-          if (isinstance(gm, mjcf_schema.Attr)
-              and 'reading' not in gm.facets):
-            return True
-  return False
 
 
 def table_driven_elements(schema):
@@ -190,23 +187,6 @@ def table_driven_elements(schema):
   return result
 
 
-def uncovered_elements(schema):
-  """Return element names eligible for table-driving but not accounted for.
-
-  An element is "eligible" if it has a bound spec struct and at least one
-  table-drivable attribute. Every eligible element should be either
-  auto-included by table_driven_elements() or listed in NOT_TABLE_DRIVEN.
-  This function returns any that fall through the cracks — used by doc_test.
-
-  Args:
-    schema: Parsed schema object.
-  """
-  eligible = {name for name, el in schema.elements.items()
-              if el.spec and _has_table_attrs(schema, el)}
-  auto = set(table_driven_elements(schema))
-  return eligible - auto - NOT_TABLE_DRIVEN
-
-
 def parse_spec_structs(*paths):
   """Parse struct layouts from C headers.
 
@@ -399,7 +379,7 @@ def generate():
   out = [_HEADER]
   for name in migrated:
     struct, rows = rows_for(schema, structs, name)
-    array = f'k{name.capitalize()}Attrs'
+    array = array_name(name)
     out.append(f'// {name} ({struct})')
     out.append(f'inline constexpr mjXAttr {array}[] = {{')
     for row in rows:
@@ -413,7 +393,7 @@ def generate():
              ' int n; };')
   out.append('inline constexpr mjXSensorEntry kSensorDispatch[] = {')
   for name in SENSOR_DISPATCH:
-    array = f'k{name.capitalize()}Attrs'
+    array = array_name(name)
     tag = schema.elements[name].xml_name()
     out.append(f'  {{"{tag}", {array}, {array}N}},')
   out.append('};')

doc/generate/mjcf_schema.py

@@ -212,8 +212,6 @@ class Schema:
   """Top-level parsed representation of an MJCF schema."""
   enums: dict[str, Enum]
   groups: dict[str, Group]
-  enums: dict[str, Enum]
-  groups: dict[str, Group]
   elements: dict[str, Element]
   path: str
 

src/xml/mjcf.schema

@@ -964,6 +964,8 @@ element body : mjsBody {
   simple             : enum<FalseAuto> = auto
   user               : double[] (field=userdata)
   child body R    # recursive
+  child frame R
+  child replicate *
   child inertial ?
   child joint *
   child freejoint *
@@ -1150,7 +1152,7 @@ element composite_joint (xml=joint) {
   armature           : double
   solreffix          : double[1..mjNREF]
   solimpfix          : double[1..mjNIMP]
-  type               : enum<comp> (required)
+  type               : enum<jointtype>
   axis               : double[3]
   limited            : enum<FalseTrueAuto>
   range              : double[2]
@@ -1172,7 +1174,7 @@ element composite_skin (xml=skin) {
 }
 
 element composite_geom (xml=geom) {
-  type               : enum<comp> (required)
+  type               : enum<geomtype>
   contype            : int
   conaffinity        : int
   condim             : int
@@ -2133,10 +2135,10 @@ element sensor_plugin : mjsSensor (xml=plugin) {
   plugin             : string
   instance           : ref<instance>
   cutoff             : double
-  objtype            : string (required)
+  objtype            : string    # objtype/objname, reftype/refname: pairwise
-  objname            : string (required)
+  objname            : string    # co-occurrence enforced by the reader
   reftype            : string
-  refname            : string (required)
+  refname            : string
   user               : double[]
   child config *
 }
@@ -2153,7 +2155,7 @@ element custom {
 element numeric : mjsNumeric {
   name               : id<numeric> (required)
   size               : int
-  data               : string (required)
+  data               : string
 }
 
 element text : mjsText {
@@ -2193,11 +2195,23 @@ element key : mjsKey {
 
 #---------------------- worldbody, frame, replicate (body aliases) ---------------------------------
 
+# The runtime validator admits the full body surface for all three aliases
+# (mjXSchema::NameMatch); the reader enforces the restrictions. The child
+# lists below record the compilable surface, for artifact emitters.
+
+# a restricted body: no attributes, no inertial or joints
 element worldbody (alias=body) {
   child body *
   child frame *
   child replicate *
-  # body children are admitted through the alias (all except inertial)
+  child geom *
+  child attach *
+  child site *
+  child camera *
+  child light *
+  child plugin *
+  child composite *
+  child flexcomp *
 }
 
 element frame : mjsFrame (alias=body) {
@@ -2205,17 +2219,42 @@ element frame : mjsFrame (alias=body) {
   childclass  : ref<default>
   pos         : double[3]
   use orientation
-  # body children are admitted through the alias
+  child body R
+  child frame R
+  child replicate *
+  child inertial ?
+  child joint *
+  child freejoint *
+  child geom *
+  child attach *
+  child site *
+  child camera *
+  child light *
+  child plugin *
+  child composite *
+  child flexcomp *
 }
 
-# read into the replicate expansion: hand-written
+# read into the replicate expansion: hand-written.
+# no freejoint or flexcomp children: the replicate expansion rejects them
 element replicate (alias=body) {
   count       : int (required)
   offset      : double[3]
   euler       : double[3]
   sep         : string
   childclass  : ref<default>
-  # body children are admitted through the alias
+  child body *
+  child frame *
+  child replicate R
+  child inertial ?
+  child joint *
+  child geom *
+  child attach *
+  child site *
+  child camera *
+  child light *
+  child plugin *
+  child composite *
 }
 
 

test/doc/doc_test.py

@@ -14,9 +14,9 @@
 # ==============================================================================
 """Tests that the API reference documentation is complete and up to date."""
 
+import os
 import re
 
-import os
 import sys
 import unittest as googletest
 _SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
@@ -74,53 +74,101 @@ _EXTRA_DOCUMENTED_TYPES = {
 }
 
 
+def _get_path(*path_parts: str) -> str:
+  """Returns absolute path for a repository-relative path."""
+  return os.path.join(_REPO_ROOT, *path_parts)
+
+
+def _check_up_to_date(test_case, rel_path, generated_content):
+  """Checks that a generated file matches the checked-in version."""
+  path = _get_path(*rel_path.split('/'))
+  with open(path, 'r', encoding='utf-8') as file:
+    current = file.read()
+  if generated_content != current:
+    filename = os.path.basename(rel_path)
+    test_case.fail(f"The file '{filename}' needs to be updated.")
+
+
 class DocTest(googletest.TestCase):
 
   def test_api_header(self):
     """Checks that references.h matches the generated output."""
-    header_file = os.path.join(_REPO_ROOT, 'doc', 'includes', 'references.h')
     source = generate_api_header.generate_reference_header(
         generate_api_header.read_headers()
     )
-    with open(header_file, 'r', encoding='utf-8') as file:
+    _check_up_to_date(self, 'doc/includes/references.h', source)
-      if source != file.read():
-        self.fail("The file 'references.h' needs to be updated.")
 
   def test_mjcf_table(self):
     """Checks that mjcf_table.inc matches the schema-generated output."""
-    table_file = os.path.join(_REPO_ROOT, 'src', 'xml', 'generated',
+    _check_up_to_date(
-                              'mjcf_table.inc')
+        self,
-    source = generate_mjcf_table.generate()
+        'src/xml/generated/mjcf_table.inc',
-    with open(table_file, 'r', encoding='utf-8') as file:
+        generate_mjcf_table.generate(),
-      if source != file.read():
+    )
-        self.fail("The file 'mjcf_table.inc' needs to be updated.")
 
   def test_default_table(self):
     """Checks that mjcf_default_table.inc matches the schema-generated output."""
-    table_file = os.path.join(_REPO_ROOT, 'src', 'xml', 'generated',
+    _check_up_to_date(
-                              'mjcf_default_table.inc')
+        self,
-    source = generate_default_table.generate()
+        'src/xml/generated/mjcf_default_table.inc',
-    with open(table_file, 'r', encoding='utf-8') as file:
+        generate_default_table.generate(),
-      if source != file.read():
+    )
-        self.fail("The file 'mjcf_default_table.inc' needs to be updated.")
 
   def test_mjcf_map(self):
     """Checks that mjcf_map.h matches the schema-generated output."""
-    map_file = os.path.join(_REPO_ROOT, 'src', 'xml', 'generated',
+    _check_up_to_date(
-                            'mjcf_map.h')
+        self, 'src/xml/generated/mjcf_map.h', generate_mjcf_map.generate()
-    source = generate_mjcf_map.generate()
+    )
-    with open(map_file, 'r', encoding='utf-8') as file:
-      if source != file.read():
-        self.fail("The file 'mjcf_map.h' needs to be updated.")
 
   def test_read_table(self):
     """Checks that mjcf_read_table.inc matches the schema-generated output."""
-    table_file = os.path.join(_REPO_ROOT, 'src', 'xml', 'generated',
+    _check_up_to_date(
-                              'mjcf_read_table.inc')
+        self,
-    source = generate_read_table.generate()
+        'src/xml/generated/mjcf_read_table.inc',
-    with open(table_file, 'r', encoding='utf-8') as file:
+        generate_read_table.generate(),
-      if source != file.read():
+    )
-        self.fail("The file 'mjcf_read_table.inc' needs to be updated.")
+
+  def test_read_table_consumed(self):
+    """Checks that every generated row array is consumed, and none is stale.
+
+    Elements are auto-included in the read table unless excluded by
+    NOT_TABLE_DRIVEN, so a new bound element whose OneX() reader was never
+    migrated would get rows that nothing reads; unused inline constexpr
+    arrays do not even warn. Conversely, a stale NOT_TABLE_DRIVEN entry
+    (e.g. after an element rename) silently stops excluding.
+    """
+    schema = mjcf_schema.parse_file(_get_path('src/xml/mjcf.schema'))
+    sources = ''
+    for filename in ('xml_native_reader.cc', 'xml_native_writer.cc'):
+      path = _get_path('src/xml', filename)
+      with open(path, 'r', encoding='utf-8') as file:
+        sources += file.read()
+    errors = []
+    dispatched = set(generate_read_table.SENSOR_DISPATCH)
+    for name in generate_read_table.table_driven_elements(schema):
+      if name in dispatched:
+        continue  # consumed through kSensorDispatch
+      array = generate_read_table.array_name(name)
+      if array not in sources:
+        errors.append(
+            f"array '{array}' for element '{name}' is generated but never "
+            'consumed: migrate its reader to ReadAttrTable or add the '
+            'element to NOT_TABLE_DRIVEN'
+        )
+    if 'kSensorDispatch' not in sources:
+      errors.append("'kSensorDispatch' is never consumed")
+    for _, array in generate_read_table.EMIT_GROUPS.values():
+      if array not in sources:
+        errors.append(
+            f"group array '{array}' is generated but never consumed"
+        )
+    stale = generate_read_table.NOT_TABLE_DRIVEN - set(schema.elements)
+    for name in sorted(stale):
+      errors.append(
+          f"NOT_TABLE_DRIVEN entry '{name}' does not name a schema element"
+      )
+    if errors:
+      self.fail('read-table coverage:\n' + '\n'.join(errors))
 
   def test_schema_enum_coverage(self):
     """Checks schema enums against the C enums they bind.
@@ -133,28 +181,41 @@ class DocTest(googletest.TestCase):
     # C members deliberately not exposed as XML keywords
     exempt = {
         'bodysleep': {  # resolved states of 'auto', not settable
-            'mjSLEEP_AUTO_ALLOWED', 'mjSLEEP_AUTO_NEVER'},
+            'mjSLEEP_AUTO_ALLOWED',
-        'geomtype': {   # rendering-only types and the missing-geom sentinel
+            'mjSLEEP_AUTO_NEVER',
-            'mjGEOM_ARROW', 'mjGEOM_ARROW1', 'mjGEOM_ARROW2', 'mjGEOM_LINE',
+        },
-            'mjGEOM_LINEBOX', 'mjGEOM_FLEX', 'mjGEOM_SKIN', 'mjGEOM_LABEL',
+        'geomtype': {  # rendering-only types and the missing-geom sentinel
-            'mjGEOM_TRIANGLE', 'mjGEOM_NONE'},
+            'mjGEOM_ARROW',
-        'texrole': {    # not settable from XML
+            'mjGEOM_ARROW1',
-            'mjTEXROLE_USER'},
+            'mjGEOM_ARROW2',
+            'mjGEOM_LINE',
+            'mjGEOM_LINEBOX',
+            'mjGEOM_FLEX',
+            'mjGEOM_SKIN',
+            'mjGEOM_LABEL',
+            'mjGEOM_TRIANGLE',
+            'mjGEOM_NONE',
+        },
+        'texrole': {  # not settable from XML
+            'mjTEXROLE_USER'
+        },
     }
     # deliberately partial: keywords are a documented subset of the C enum
     partial = {'frameobj'}
 
     enums_c = {}
     for name in ('mjtype.h', 'mjspec.h'):
-      path = os.path.join(_REPO_ROOT, 'include', 'mujoco', name)
+      path = _get_path('include', 'mujoco', name)
       with open(path, 'r', encoding='utf-8') as file:
         content = file.read()
-      for m in re.finditer(r'typedef enum (mjt\w+)\s*\{(.*?)\}\s*\1;',
+      for m in re.finditer(
-                           content, re.S):
+          r'typedef enum (mjt\w+)\s*\{(.*?)\}\s*\1;', content, re.S
-        enums_c[m.group(1)] = re.findall(r'^\s*(mj[A-Z]\w+)', m.group(2),
+      ):
-                                         re.M)
+        enums_c[m.group(1)] = re.findall(
+            r'^\s*(mj[A-Z]\w+)', m.group(2), re.M
+        )
 
-    schema_path = os.path.join(_REPO_ROOT, 'src', 'xml', 'mjcf.schema')
+    schema_path = _get_path('src/xml/mjcf.schema')
     schema = mjcf_schema.parse_file(schema_path)
     errors = []
     for name, enum in schema.enums.items():
@@ -169,38 +230,34 @@ class DocTest(googletest.TestCase):
         errors.append(f'  {name}: {bad} is not a member of {enum.ctype}')
       if name in partial:
         continue
-      uncovered = {m for m in members - constants
+      uncovered = {
-                   if not re.match(r'mjN[A-Z]', m)} - exempt.get(name, set())
+          m
+          for m in members - constants
+          if not re.search(r'^mjN[A-Z]', m)
+      } - exempt.get(name, set())
       for miss in sorted(uncovered):
-        errors.append(f'  {name}: {enum.ctype} member {miss} has no keyword '
+        errors.append(
-                      '(add it to the schema or to the exemptions here)')
+            f'  {name}: {enum.ctype} member {miss} has no keyword (add it to'
+            ' the schema or to the exemptions here)'
+        )
     if errors:
       self.fail('schema enum coverage:\n' + '\n'.join(errors))
 
   def test_schema(self):
     """Checks that XMLschema.rst matches the generated output."""
-    schema_file = os.path.join(_REPO_ROOT, 'doc', 'XMLschema.rst')
+    _check_up_to_date(self, 'doc/XMLschema.rst', generate_schema.generate())
-    source = generate_schema.generate()
-    with open(schema_file, 'r', encoding='utf-8') as file:
-      if source != file.read():
-        self.fail("The file 'XMLschema.rst' needs to be updated.")
 
   def test_functions(self):
     """Checks that functions.rst matches the generated output."""
-    functions_file = os.path.join(
+    _check_up_to_date(
-        _REPO_ROOT, 'doc', 'APIreference', 'functions.rst'
+        self,
+        'doc/APIreference/functions.rst',
+        generate_functions.generate(),
     )
-    source = generate_functions.generate()
-    with open(functions_file, 'r', encoding='utf-8') as file:
-      if source != file.read():
-        self.fail("The file 'functions.rst' needs to be updated.")
 
   def test_all_functions_included(self):
     """Checks that every public C function has an entry in functions.rst."""
-
+    functions_file = _get_path('doc/APIreference/functions.rst')
-    functions_file = os.path.join(
-        _REPO_ROOT, 'doc', 'APIreference', 'functions.rst'
-    )
     with open(functions_file, 'r', encoding='utf-8') as file:
       content = file.read()
 
@@ -209,7 +266,9 @@ class DocTest(googletest.TestCase):
     )
 
     api = generate_api_header.read_headers()
-    header_funcs = {token for token, d in api.items() if d.c_type == 'FUNCTION'}
+    header_funcs = {
+        token for token, d in api.items() if d.c_type == 'FUNCTION'
+    }
 
     errors = []
     for token in sorted(header_funcs - documented - _FUNCTIONS_TO_SKIP):
@@ -224,10 +283,7 @@ class DocTest(googletest.TestCase):
 
   def test_all_types_included(self):
     """Checks that every public struct and enum has an entry in APItypes.rst."""
-
+    types_file = _get_path('doc/APIreference/APItypes.rst')
-    types_file = os.path.join(
-        _REPO_ROOT, 'doc', 'APIreference', 'APItypes.rst'
-    )
     with open(types_file, 'r', encoding='utf-8') as file:
       content = file.read()
 
@@ -255,25 +311,42 @@ class DocTest(googletest.TestCase):
 
   def test_element_constraints_diamond_inheritance(self):
     con = mjcf_schema.Constraint(
-        kind='exclusive', bundles=(('a',), ('b',)), doc=None, line=1)
+        kind='exclusive', bundles=(('a',), ('b',)), doc=None, line=1
+    )
     common_group = mjcf_schema.Group(
-        name='common', variant=False, members=[con], doc=None, line=1)
+        name='common', variant=False, members=[con], doc=None, line=1
+    )
     group1 = mjcf_schema.Group(
-        name='group1', variant=False,
+        name='group1',
-        members=[mjcf_schema.Use(group='common', line=1)], doc=None, line=1)
+        variant=False,
+        members=[mjcf_schema.Use(group='common', line=1)],
+        doc=None,
+        line=1,
+    )
     group2 = mjcf_schema.Group(
-        name='group2', variant=False,
+        name='group2',
-        members=[mjcf_schema.Use(group='common', line=1)], doc=None, line=1)
+        variant=False,
+        members=[mjcf_schema.Use(group='common', line=1)],
+        doc=None,
+        line=1,
+    )
     elem = mjcf_schema.Element(
-        name='elem', spec=None, facets={},
+        name='elem',
-        members=[mjcf_schema.Use(group='group1', line=1),
+        spec=None,
-                 mjcf_schema.Use(group='group2', line=1)],
+        facets={},
-        doc=None, line=1)
+        members=[
+            mjcf_schema.Use(group='group1', line=1),
+            mjcf_schema.Use(group='group2', line=1),
+        ],
+        doc=None,
+        line=1,
+    )
     schema = mjcf_schema.Schema(
         enums={},
         groups={'common': common_group, 'group1': group1, 'group2': group2},
         elements={'elem': elem},
-        path='<test>')
+        path='<test>',
+    )
     cons = generate_mjcf_table._element_constraints(schema, elem)
     self.assertEqual(len(cons), 1)  # pylint: disable=g-generic-assert