From 717f30099f71b5921fa4361ebf0a8c3688017955 Mon Sep 17 00:00:00 2001 From: Weaselbot Date: Tue, 23 Jun 2026 22:16:34 -0400 Subject: [PATCH 1/2] schemagen: cache non-object $defs entries to avoid duplicate types Extend the existing per-definition cache (`self._building`) to enum, array, and scalar $defs, not just object definitions. This ensures that multiple $refs to the same non-object definition reuse the same C++ type instead of generating Role, Role2, Role3, etc. - Cache the built (type, nullable) tuple under defname for enum, scalar, and array definitions. - Pre-register array definitions before recursing into items so $ref cycles resolve to the same TArr instance. - Store object definitions as (TObj, nullable) tuples so nullable object $defs also preserve their nullability when referenced. Add a regression test for issue #17 covering reused enum and array-of-enum $defs. --- contrib/schemagen/test_schemagen.py | 31 +++++++++++++++++++++ contrib/schemagen/weaseljson_schemagen.py | 34 +++++++++++++++++------ 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/contrib/schemagen/test_schemagen.py b/contrib/schemagen/test_schemagen.py index 36b956c..fdf195b 100644 --- a/contrib/schemagen/test_schemagen.py +++ b/contrib/schemagen/test_schemagen.py @@ -195,6 +195,37 @@ class SchemagenCollisionTest(unittest.TestCase): self.assertIn("Arr0", enumerators) self.assertEqual(len(enumerators), len(set(enumerators))) + def test_non_object_defs_reused_across_refs(self): + """Regression test for issue #17: enum and array $defs must be reused.""" + schema = { + "type": "object", + "properties": { + "role1": {"$ref": "#/$defs/Role"}, + "role2": {"$ref": "#/$defs/Role"}, + "roles1": {"$ref": "#/$defs/Roles"}, + "roles2": {"$ref": "#/$defs/Roles"}, + }, + "$defs": { + "Role": {"enum": ["admin", "user"]}, + "Roles": {"type": "array", "items": {"$ref": "#/$defs/Role"}}, + }, + } + out = self.generate_and_compile(schema) + # Exactly one Role enum is generated. + self.assertIn("enum class Role : int { admin, user };", out) + self.assertNotIn("enum class Role1 : int", out) + self.assertNotIn("enum class Role2 : int", out) + # The array of enum is represented by a single kind. + m = re.search(r"enum class Kind : uint8_t \{([^}]+)\}", out) + self.assertIsNotNone(m) + enumerators = [e.strip() for e in m.group(1).split(",")] + self.assertEqual(enumerators.count("Arr0"), 1) + # All four fields use the same C++ types. + self.assertIn("std::optional role1;", out) + self.assertIn("std::optional role2;", out) + self.assertIn("std::optional> roles1;", out) + self.assertIn("std::optional> roles2;", out) + class SchemagenAdditionalPropertiesTest(unittest.TestCase): def run_schemagen(self, schema, args=None): diff --git a/contrib/schemagen/weaseljson_schemagen.py b/contrib/schemagen/weaseljson_schemagen.py index 7e1c0c5..be75e72 100644 --- a/contrib/schemagen/weaseljson_schemagen.py +++ b/contrib/schemagen/weaseljson_schemagen.py @@ -250,7 +250,6 @@ class Builder: if defname not in self.defs: raise GenError(f"$ref to unknown def: {defname}") node = self.defs[defname] - # For objects we must register the name before recursing into fields. return self.build_type(node, defname, defname=defname) def build_type(self, node, hint, defname=None): @@ -279,6 +278,10 @@ class Builder: if "$ref" in node: return self.build_def(self.ref_name(node["$ref"])) + # Non-object $defs entries must reuse the same type for every $ref. + if defname is not None and defname in self._building: + return self._building[defname] + # nullability via type lists: ["string", "null"] nullable = False typ = node.get("type") @@ -297,18 +300,29 @@ class Builder: raise GenError("only non-empty string enums are supported") name = defname and self.unique_name(defname) or self.unique_name(hint) self.enums[name] = EnumType(name, list(vals)) - return (TEnum(name), nullable) + result = (TEnum(name), nullable) + if defname is not None: + self._building[defname] = result + return result if typ == "object" or (typ is None and "properties" in node): - return (self._build_object(node, hint, defname), nullable) + return self._build_object(node, hint, defname, nullable) if typ == "array": if "items" not in node or not isinstance(node["items"], dict): raise GenError("arrays require a single 'items' schema") + # Register the array before building its items so $ref cycles back + # to this definition resolve to the same TArr instance. + t = TArr(None, False) + result = (t, nullable) + if defname is not None: + self._building[defname] = result elem, elem_nullable = self._unpack( self.build_type(node["items"], hint + "Item") ) - return (TArr(elem, elem_nullable), nullable) + t.elem = elem + t.elem_nullable = elem_nullable + return result scalar = { "string": "str", @@ -317,7 +331,10 @@ class Builder: "boolean": "bool", }.get(typ) if scalar: - return (TScalar(scalar), nullable) + result = (TScalar(scalar), nullable) + if defname is not None: + self._building[defname] = result + return result if typ == "null": raise GenError("'null'-only types are not supported") @@ -331,13 +348,14 @@ class Builder: return result return (result, False) - def _build_object(self, node, hint, defname): + def _build_object(self, node, hint, defname, nullable=False): name = self.unique_name(defname or hint) obj = ObjectType(name) self.objects[name] = obj # register for $ref cycles before building fields + tobj = TObj(name) if defname is not None: - self._building[defname] = TObj(name) + self._building[defname] = (tobj, nullable) ap = node.get("additionalProperties", False) if ap is True: raise GenError("additionalProperties: true is not supported") @@ -358,7 +376,7 @@ class Builder: cpp = f"{base}{i}" seen_cpp.add(cpp) obj.fields.append(Field(key, cpp, ty, key in required, nullable)) - return TObj(name) + return tobj # --------------------------------------------------------------------------- From e155e0bbf48d367ce63fd8ceaf62c275eb73bb5f Mon Sep 17 00:00:00 2001 From: Weaselbot Date: Wed, 24 Jun 2026 09:51:19 -0400 Subject: [PATCH 2/2] schemagen: restore nullable tuple return for object schemas The previous change to cache object definitions started returning the bare TObj from build_type for object schemas, discarding the nullable flag. This caused nullable root objects to be emitted as plain structs instead of std::optional, breaking the nullable root tests. Return the (TObj, nullable) tuple so callers (including the root emitter) see the correct nullability again. --- contrib/schemagen/weaseljson_schemagen.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/schemagen/weaseljson_schemagen.py b/contrib/schemagen/weaseljson_schemagen.py index be75e72..8c8b72b 100644 --- a/contrib/schemagen/weaseljson_schemagen.py +++ b/contrib/schemagen/weaseljson_schemagen.py @@ -306,7 +306,7 @@ class Builder: return result if typ == "object" or (typ is None and "properties" in node): - return self._build_object(node, hint, defname, nullable) + return (self._build_object(node, hint, defname, nullable), nullable) if typ == "array": if "items" not in node or not isinstance(node["items"], dict):