schemagen: cache non-object $defs entries to avoid duplicate types
CI / build (-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++, clang-arm64, ubuntu-latest-arm64, true) (pull_request) Failing after 51s
CI / build (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc-arm64, ubuntu-latest-arm64, false) (pull_request) Failing after 51s
CI / pre-commit (pull_request) Successful in 52s
CI / build (-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++, clang-amd64, ubuntu-latest-amd64, true) (pull_request) Failing after 1m12s
CI / build (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc-amd64, ubuntu-latest-amd64, false) (pull_request) Failing after 1m14s
CI / build (-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++, clang-arm64, ubuntu-latest-arm64, true) (pull_request) Failing after 51s
CI / build (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc-arm64, ubuntu-latest-arm64, false) (pull_request) Failing after 51s
CI / pre-commit (pull_request) Successful in 52s
CI / build (-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++, clang-amd64, ubuntu-latest-amd64, true) (pull_request) Failing after 1m12s
CI / build (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc-amd64, ubuntu-latest-amd64, false) (pull_request) Failing after 1m14s
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.
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user