From 46ff8e21640dd163897a7f8a0124ab322385d6c0 Mon Sep 17 00:00:00 2001 From: Weaselbot Date: Tue, 23 Jun 2026 12:23:09 -0400 Subject: [PATCH] schemagen: reserve generated Root/Skip/RootScalar and avoid Kind/ArrN collisions Make the type-name allocator aware of the identifiers the generator emits itself (`Root` alias, `Skip`/`RootScalar` Kind enumerators) so user `$defs` names can no longer collide with them. Array-kind names (`Arr0`, `Arr1`, ...) are now allocated only after checking for object/enum names, preventing duplicate `Kind` enumerators when a schema defines e.g. `Arr0`. Add Python regression tests that also syntax-check the generated headers with a C++ compiler. Closes #21 --- contrib/schemagen/test_schemagen.py | 107 ++++++++++++++++++++++ contrib/schemagen/weaseljson_schemagen.py | 29 +++++- 2 files changed, 133 insertions(+), 3 deletions(-) diff --git a/contrib/schemagen/test_schemagen.py b/contrib/schemagen/test_schemagen.py index 15d3a67..2ddebac 100644 --- a/contrib/schemagen/test_schemagen.py +++ b/contrib/schemagen/test_schemagen.py @@ -3,6 +3,8 @@ import json import os +import re +import shutil import subprocess import sys import tempfile @@ -87,5 +89,110 @@ class SchemagenKeywordTest(unittest.TestCase): self.assertNotIn(f"std::optional {kw};", stdout) +class SchemagenCollisionTest(unittest.TestCase): + """Regression tests for issue #21: generated Root alias / Kind enum collisions.""" + + def setUp(self): + self.repo_root = os.path.dirname( + os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + ) + self.include_dir = os.path.join(self.repo_root, "include") + self.compiler = shutil.which("c++") + + def generate_and_compile(self, schema): + """Run schemagen on schema and syntax-check the resulting header.""" + with tempfile.TemporaryDirectory() as tmpdir: + schema_path = os.path.join(tmpdir, "schema.json") + with open(schema_path, "w") as fp: + json.dump(schema, fp) + header_path = os.path.join(tmpdir, "gen.h") + cmd = [ + sys.executable, + SCRIPT, + schema_path, + "-o", + header_path, + "--namespace", + "test_schema", + ] + result = subprocess.run(cmd, capture_output=True, text=True, check=False) + self.assertEqual(result.returncode, 0, msg=result.stderr) + + if self.compiler: + cpp_path = os.path.join(tmpdir, "test.cpp") + with open(cpp_path, "w") as fp: + fp.write( + '#include "gen.h"\n' + "int main() {\n" + " test_schema::RootBuilder b;\n" + " test_schema::Root r = b.take();\n" + " (void)r;\n" + "}\n" + ) + comp = subprocess.run( + [ + self.compiler, + "-std=c++20", + "-fsyntax-only", + "-I", + self.include_dir, + "-I", + tmpdir, + cpp_path, + ], + capture_output=True, + text=True, + check=False, + ) + self.assertEqual(comp.returncode, 0, msg=comp.stderr) + + with open(header_path) as fp: + return fp.read() + + def test_root_alias_does_not_collide_with_user_type(self): + schema = { + "type": "array", + "items": {"$ref": "#/$defs/Root"}, + "$defs": {"Root": {"enum": ["a", "b"]}}, + } + out = self.generate_and_compile(schema) + self.assertIn("enum class Root1 : int { a, b };", out) + self.assertIn("using Root = std::vector;", out) + self.assertNotIn("using Root = std::vector;", out) + + def test_kind_enum_does_not_duplicate_arr0(self): + schema = { + "type": "object", + "properties": { + "arr": {"type": "array", "items": {"type": "string"}}, + "obj": {"$ref": "#/$defs/Arr0"}, + }, + "$defs": {"Arr0": {"type": "object", "properties": {}}}, + } + out = self.generate_and_compile(schema) + self.assertIn("struct Arr0", out) + m = re.search(r"enum class Kind : uint8_t \{([^}]+)\}", out) + self.assertIsNotNone(m) + enumerators = [e.strip() for e in m.group(1).split(",")] + self.assertIn("Arr0", enumerators) + self.assertIn("Arr1", enumerators) + self.assertEqual(len(enumerators), len(set(enumerators))) + + def test_skip_is_reserved(self): + schema = { + "type": "array", + "items": {"$ref": "#/$defs/Skip"}, + "$defs": {"Skip": {"type": "object", "properties": {}}}, + } + out = self.generate_and_compile(schema) + self.assertIn("struct Skip1", out) + m = re.search(r"enum class Kind : uint8_t \{([^}]+)\}", out) + self.assertIsNotNone(m) + enumerators = [e.strip() for e in m.group(1).split(",")] + self.assertIn("Skip", enumerators) + self.assertIn("Skip1", enumerators) + self.assertEqual(len(enumerators), len(set(enumerators))) + + if __name__ == "__main__": unittest.main() diff --git a/contrib/schemagen/weaseljson_schemagen.py b/contrib/schemagen/weaseljson_schemagen.py index 11b268b..ce61f51 100644 --- a/contrib/schemagen/weaseljson_schemagen.py +++ b/contrib/schemagen/weaseljson_schemagen.py @@ -216,12 +216,26 @@ class Builder: base = camel(hint) name = base i = 1 - while name in self._used_names: + while self._name_taken(name): + candidate = f"{base}{i}" + # A numeric suffix can itself land on a reserved generated name + # (e.g. hint "Arr0" -> "Arr01"). Use an underscore separator so + # we never loop through the reserved block. + if self._is_reserved(candidate): + candidate = f"{base}_{i}" + name = candidate i += 1 - name = f"{base}{i}" self._used_names.add(name) return name + def _name_taken(self, name): + return name in self._used_names or self._is_reserved(name) + + @staticmethod + def _is_reserved(name): + """Names generated internally that must not collide with user types.""" + return name in ("Root", "Skip", "RootScalar") + def ref_name(self, ref): if not ref.startswith("#/"): raise GenError(f"only local $ref supported, got: {ref}") @@ -435,6 +449,7 @@ class Emitter: self.kind_order = [] # all Kind enumerators in declaration order self.root_ty = None self.root_nullable = False + self._arr_counter = 0 # -- type strings ------------------------------------------------------- def base_cpp(self, ty): @@ -465,11 +480,19 @@ class Emitter: def arr_kind(self, tarr): sig = self.base_cpp(tarr) if sig not in self.arr_kinds: - name = f"Arr{len(self.arr_kinds)}" + name = self._fresh_arr_kind_name() self.arr_kinds[sig] = name self.arr_types.append((name, tarr)) + self.b._used_names.add(name) return self.arr_kinds[sig] + def _fresh_arr_kind_name(self): + while True: + name = f"Arr{self._arr_counter}" + self._arr_counter += 1 + if not self.b._name_taken(name): + return name + def cat(self, ty): if isinstance(ty, TScalar): return {"str": "Str", "int": "Int", "dbl": "Dbl", "bool": "Bool"}[ty.kind]