diff --git a/doc/generate/generate_mjcf_table.py b/doc/generate/generate_mjcf_table.py index 15b42b76..13a00840 100644 --- a/doc/generate/generate_mjcf_table.py +++ b/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'] diff --git a/doc/generate/generate_read_table.py b/doc/generate/generate_read_table.py index ed183f01..d14eaeda 100644 --- a/doc/generate/generate_read_table.py +++ b/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 -element is accounted for. +custom logic). A coverage check in doc_test verifies that every emitted row +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: - if isinstance(member, mjcf_schema.Attr) and 'reading' not in member.facets: - 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 + return any('reading' not in attr.facets + for attr in schema.expanded_attrs(element)) 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('};') diff --git a/doc/generate/mjcf_schema.py b/doc/generate/mjcf_schema.py index c054bd48..7c18bec6 100644 --- a/doc/generate/mjcf_schema.py +++ b/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 diff --git a/src/xml/mjcf.schema b/src/xml/mjcf.schema index 7fa4b5ff..4e0000d7 100644 --- a/src/xml/mjcf.schema +++ b/src/xml/mjcf.schema @@ -964,6 +964,8 @@ element body : mjsBody { simple : enum = 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 (required) + type : enum axis : double[3] limited : enum range : double[2] @@ -1172,7 +1174,7 @@ element composite_skin (xml=skin) { } element composite_geom (xml=geom) { - type : enum (required) + type : enum contype : int conaffinity : int condim : int @@ -2133,10 +2135,10 @@ element sensor_plugin : mjsSensor (xml=plugin) { plugin : string instance : ref cutoff : double - objtype : string (required) - objname : string (required) + objtype : string # objtype/objname, reftype/refname: pairwise + 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 (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 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 - # 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 * } diff --git a/test/doc/doc_test.py b/test/doc/doc_test.py index af9a532b..a666bb70 100644 --- a/test/doc/doc_test.py +++ b/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: - if source != file.read(): - self.fail("The file 'references.h' needs to be updated.") + _check_up_to_date(self, 'doc/includes/references.h', source) 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', - 'mjcf_table.inc') - source = generate_mjcf_table.generate() - with open(table_file, 'r', encoding='utf-8') as file: - if source != file.read(): - self.fail("The file 'mjcf_table.inc' needs to be updated.") + _check_up_to_date( + self, + 'src/xml/generated/mjcf_table.inc', + generate_mjcf_table.generate(), + ) 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', - 'mjcf_default_table.inc') - source = generate_default_table.generate() - with open(table_file, 'r', encoding='utf-8') as file: - if source != file.read(): - self.fail("The file 'mjcf_default_table.inc' needs to be updated.") + _check_up_to_date( + self, + 'src/xml/generated/mjcf_default_table.inc', + generate_default_table.generate(), + ) 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', - 'mjcf_map.h') - 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.") + _check_up_to_date( + self, 'src/xml/generated/mjcf_map.h', generate_mjcf_map.generate() + ) 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', - 'mjcf_read_table.inc') - source = generate_read_table.generate() - with open(table_file, 'r', encoding='utf-8') as file: - if source != file.read(): - self.fail("The file 'mjcf_read_table.inc' needs to be updated.") + _check_up_to_date( + self, + 'src/xml/generated/mjcf_read_table.inc', + generate_read_table.generate(), + ) + + 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'}, - 'geomtype': { # rendering-only types and the missing-geom sentinel - 'mjGEOM_ARROW', 'mjGEOM_ARROW1', 'mjGEOM_ARROW2', 'mjGEOM_LINE', - 'mjGEOM_LINEBOX', 'mjGEOM_FLEX', 'mjGEOM_SKIN', 'mjGEOM_LABEL', - 'mjGEOM_TRIANGLE', 'mjGEOM_NONE'}, - 'texrole': { # not settable from XML - 'mjTEXROLE_USER'}, + 'mjSLEEP_AUTO_ALLOWED', + 'mjSLEEP_AUTO_NEVER', + }, + 'geomtype': { # rendering-only types and the missing-geom sentinel + 'mjGEOM_ARROW', + 'mjGEOM_ARROW1', + '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;', - content, re.S): - enums_c[m.group(1)] = re.findall(r'^\s*(mj[A-Z]\w+)', m.group(2), - re.M) + for m in re.finditer( + 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 + ) - 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 - if not re.match(r'mjN[A-Z]', m)} - exempt.get(name, set()) + uncovered = { + 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 ' - '(add it to the schema or to the exemptions here)') + errors.append( + 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') - 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.") + _check_up_to_date(self, 'doc/XMLschema.rst', generate_schema.generate()) def test_functions(self): """Checks that functions.rst matches the generated output.""" - functions_file = os.path.join( - _REPO_ROOT, 'doc', 'APIreference', 'functions.rst' + _check_up_to_date( + 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 = os.path.join( - _REPO_ROOT, 'doc', 'APIreference', 'functions.rst' - ) + functions_file = _get_path('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 = os.path.join( - _REPO_ROOT, 'doc', 'APIreference', 'APItypes.rst' - ) + types_file = _get_path('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, - members=[mjcf_schema.Use(group='common', line=1)], doc=None, line=1) + name='group1', + variant=False, + members=[mjcf_schema.Use(group='common', line=1)], + doc=None, + line=1, + ) group2 = mjcf_schema.Group( - name='group2', variant=False, - members=[mjcf_schema.Use(group='common', line=1)], doc=None, line=1) + name='group2', + variant=False, + members=[mjcf_schema.Use(group='common', line=1)], + doc=None, + line=1, + ) elem = mjcf_schema.Element( - name='elem', spec=None, facets={}, - members=[mjcf_schema.Use(group='group1', line=1), - mjcf_schema.Use(group='group2', line=1)], - doc=None, line=1) + name='elem', + spec=None, + facets={}, + 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='') + path='', + ) cons = generate_mjcf_table._element_constraints(schema, elem) self.assertEqual(len(cons), 1) # pylint: disable=g-generic-assert