forked from weaselab/weaseljson
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
This commit is contained in:
@@ -3,6 +3,8 @@
|
|||||||
|
|
||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
|
import re
|
||||||
|
import shutil
|
||||||
import subprocess
|
import subprocess
|
||||||
import sys
|
import sys
|
||||||
import tempfile
|
import tempfile
|
||||||
@@ -87,5 +89,110 @@ class SchemagenKeywordTest(unittest.TestCase):
|
|||||||
self.assertNotIn(f"std::optional<std::string> {kw};", stdout)
|
self.assertNotIn(f"std::optional<std::string> {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<Root1>;", out)
|
||||||
|
self.assertNotIn("using Root = std::vector<Root>;", 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__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
@@ -216,12 +216,26 @@ class Builder:
|
|||||||
base = camel(hint)
|
base = camel(hint)
|
||||||
name = base
|
name = base
|
||||||
i = 1
|
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
|
i += 1
|
||||||
name = f"{base}{i}"
|
|
||||||
self._used_names.add(name)
|
self._used_names.add(name)
|
||||||
return 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):
|
def ref_name(self, ref):
|
||||||
if not ref.startswith("#/"):
|
if not ref.startswith("#/"):
|
||||||
raise GenError(f"only local $ref supported, got: {ref}")
|
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.kind_order = [] # all Kind enumerators in declaration order
|
||||||
self.root_ty = None
|
self.root_ty = None
|
||||||
self.root_nullable = False
|
self.root_nullable = False
|
||||||
|
self._arr_counter = 0
|
||||||
|
|
||||||
# -- type strings -------------------------------------------------------
|
# -- type strings -------------------------------------------------------
|
||||||
def base_cpp(self, ty):
|
def base_cpp(self, ty):
|
||||||
@@ -465,11 +480,19 @@ class Emitter:
|
|||||||
def arr_kind(self, tarr):
|
def arr_kind(self, tarr):
|
||||||
sig = self.base_cpp(tarr)
|
sig = self.base_cpp(tarr)
|
||||||
if sig not in self.arr_kinds:
|
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_kinds[sig] = name
|
||||||
self.arr_types.append((name, tarr))
|
self.arr_types.append((name, tarr))
|
||||||
|
self.b._used_names.add(name)
|
||||||
return self.arr_kinds[sig]
|
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):
|
def cat(self, ty):
|
||||||
if isinstance(ty, TScalar):
|
if isinstance(ty, TScalar):
|
||||||
return {"str": "Str", "int": "Int", "dbl": "Dbl", "bool": "Bool"}[ty.kind]
|
return {"str": "Str", "int": "Int", "dbl": "Dbl", "bool": "Bool"}[ty.kind]
|
||||||
|
|||||||
Reference in New Issue
Block a user