2 Commits
Author SHA1 Message Date
weaselbot 414abca9c0 schemagen: drop support for additionalProperties: true
CI / build (-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++, clang-arm64, ubuntu-latest-arm64, true) (pull_request) Successful in 51s
CI / pre-commit (pull_request) Successful in 52s
CI / build (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc-arm64, ubuntu-latest-arm64, false) (pull_request) Successful in 49s
CI / build (-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++, clang-amd64, ubuntu-latest-amd64, true) (pull_request) Successful in 1m30s
CI / build (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc-amd64, ubuntu-latest-amd64, false) (pull_request) Successful in 1m23s
Following review feedback, the generator no longer supports permissive
objects. Changes:

- Reject `additionalProperties: true` at generation time.
- Treat an absent `additionalProperties` as `false`, so every object is
  strict by default and unknown keys are rejected during parsing.
- Remove the now-dead permissive-object infrastructure: `Kind::Skip`,
  `Cat::Skip`, `kSkip`, the per-frame `unknown` key set, and `isStrict()`.
- Update the README feature/rejection tables accordingly.
- Remove the permissive "loose" object from example.schema.json and the
  associated tests from test_gen.cpp.
- Add Python unit tests verifying the new `additionalProperties` behavior.

All tests pass (`ctest --output-on-failure`).
2026-06-23 14:07:41 -04:00
weaselbot a4ed5a9171 schemagen: reject duplicate unknown keys in non-strict objects
CI / pre-commit (pull_request) Successful in 55s
CI / build (-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++, clang-arm64, ubuntu-latest-arm64, true) (pull_request) Successful in 59s
CI / build (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc-arm64, ubuntu-latest-arm64, false) (pull_request) Successful in 1m0s
CI / build (-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++, clang-amd64, ubuntu-latest-amd64, true) (pull_request) Successful in 1m34s
CI / build (-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++, gcc-amd64, ubuntu-latest-amd64, false) (pull_request) Successful in 1m27s
Track unknown keys in a per-object unordered_set so that permissive
objects (additionalProperties absent/true) still reject duplicate keys,
matching the README guarantee.

- Add std::unordered_set<std::string> to Frame.
- Insert unknown keys in cbKeyData and reject duplicates before skipping.
- Add a permissive "loose" subobject to example.schema.json.
- Test single unknown key accepted and duplicate unknown/known keys rejected.
2026-06-23 12:27:05 -04:00
3 changed files with 60 additions and 55 deletions
+5 -5
View File
@@ -45,7 +45,7 @@ into the result, so it is non-movable.
| `$ref` to `$defs`/`definitions` | the referenced named struct | | `$ref` to `$defs`/`definitions` | the referenced named struct |
| recursive `$ref` | `std::unique_ptr<T>` (cycle broken) | | recursive `$ref` | `std::unique_ptr<T>` (cycle broken) |
| `additionalProperties: false` | unknown keys rejected | | `additionalProperties: false` | unknown keys rejected |
| `additionalProperties` absent / `true` | unknown keys' values skipped | | `additionalProperties` absent / `true` | not supported (rejected at generation) |
## Schema violations (rejected at parse time) ## Schema violations (rejected at parse time)
@@ -55,15 +55,15 @@ into the result, so it is non-movable.
- value not in a string `enum` - value not in a string `enum`
- a number not representable in the target type (e.g. `1.5` for an `integer`) - a number not representable in the target type (e.g. `1.5` for an `integer`)
- duplicate object keys - duplicate object keys
- unknown key under `additionalProperties: false` - unknown key in any object
## Not supported (rejected at generation time, no fallback) ## Not supported (rejected at generation time, no fallback)
`oneOf` / `anyOf` / `allOf` / `not` / `if`-`then`-`else`, `oneOf` / `anyOf` / `allOf` / `not` / `if`-`then`-`else`,
`patternProperties`, `additionalProperties` with a schema (typed map), `patternProperties`, `additionalProperties` with a schema (typed map),
`prefixItems` (tuples), `const`, `dependentSchemas`/`dependentRequired`, `additionalProperties: true`, `prefixItems` (tuples), `const`,
union `type` lists other than `["T", "null"]`, non-string enums, and remote `dependentSchemas`/`dependentRequired`, union `type` lists other than
(`$ref` to other documents). `["T", "null"]`, non-string enums, and remote (`$ref` to other documents).
## Notes ## Notes
+47
View File
@@ -87,5 +87,52 @@ class SchemagenKeywordTest(unittest.TestCase):
self.assertNotIn(f"std::optional<std::string> {kw};", stdout) self.assertNotIn(f"std::optional<std::string> {kw};", stdout)
class SchemagenAdditionalPropertiesTest(unittest.TestCase):
def run_schemagen(self, schema, args=None):
"""Run schemagen on a schema dict. Returns (returncode, stdout, stderr)."""
with tempfile.NamedTemporaryFile("w", suffix=".json", delete=False) as fp:
json.dump(schema, fp)
schema_path = fp.name
try:
cmd = [sys.executable, SCRIPT, schema_path]
if args:
cmd.extend(args)
result = subprocess.run(cmd, capture_output=True, text=True, check=False)
return result.returncode, result.stdout, result.stderr
finally:
os.unlink(schema_path)
def test_additional_properties_true_rejected(self):
schema = {
"type": "object",
"additionalProperties": True,
"properties": {"name": {"type": "string"}},
}
rc, stdout, stderr = self.run_schemagen(schema)
self.assertNotEqual(rc, 0)
self.assertIn("additionalProperties: true is not supported", stderr)
def test_additional_properties_absent_defaults_to_strict(self):
schema = {
"type": "object",
"properties": {"name": {"type": "string"}},
}
rc, stdout, stderr = self.run_schemagen(schema)
self.assertEqual(rc, 0, msg=stderr)
# The generated parser should reject unknown keys. Verify the key-matching
# helper returns -1 for an unknown key and cbKeyData rejects it.
self.assertIn("int matchKey(Kind k, std::string_view key) const {", stdout)
self.assertNotIn("bool isStrict(Kind k) const", stdout)
def test_additional_properties_false_accepted(self):
schema = {
"type": "object",
"additionalProperties": False,
"properties": {"name": {"type": "string"}},
}
rc, stdout, stderr = self.run_schemagen(schema)
self.assertEqual(rc, 0, msg=stderr)
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()
+8 -50
View File
@@ -60,7 +60,6 @@ class ObjectType:
def __init__(self, name): def __init__(self, name):
self.name = name self.name = name
self.fields = [] # list[Field] self.fields = [] # list[Field]
self.strict = False # additionalProperties: false
class EnumType: class EnumType:
@@ -325,12 +324,13 @@ class Builder:
# register for $ref cycles before building fields # register for $ref cycles before building fields
if defname is not None: if defname is not None:
self._building[defname] = TObj(name) self._building[defname] = TObj(name)
ap = node.get("additionalProperties", True) ap = node.get("additionalProperties", False)
if ap is True:
raise GenError("additionalProperties: true is not supported")
if isinstance(ap, dict): if isinstance(ap, dict):
raise GenError( raise GenError(
"additionalProperties with a schema (typed map) is not " "supported yet" "additionalProperties with a schema (typed map) is not " "supported yet"
) )
obj.strict = ap is False
required = set(node.get("required", [])) required = set(node.get("required", []))
props = node.get("properties", {}) props = node.get("properties", {})
seen_cpp = set() seen_cpp = set()
@@ -587,7 +587,6 @@ namespace {ns} {{"""
) )
if not root_is_container: if not root_is_container:
kinds.append("RootScalar") kinds.append("RootScalar")
kinds.append("Skip")
self.kind_order = kinds self.kind_order = kinds
return " enum class Kind : uint8_t { " + ", ".join(kinds) + " };" return " enum class Kind : uint8_t { " + ", ".join(kinds) + " };"
@@ -771,17 +770,6 @@ namespace {ns} {{"""
lines.append(" }") lines.append(" }")
return "\n".join(lines) return "\n".join(lines)
def _is_strict(self):
strict = [n for n, o in self.b.objects.items() if o.strict]
if not strict:
return " bool isStrict(Kind) const { return false; }"
cases = " ".join(f"case Kind::{n}:" for n in strict)
return (
" bool isStrict(Kind k) const {\n"
f" switch (k) {{ {cases} return true; default: return false; }}\n"
" }"
)
def _enum_name_arrays(self): def _enum_name_arrays(self):
out = [] out = []
for e in self.b.enums.values(): for e in self.b.enums.values():
@@ -818,13 +806,7 @@ namespace {ns} {{"""
else: else:
lines.append(" if (stack_.empty()) { reject(); return; }") lines.append(" if (stack_.empty()) { reject(); return; }")
lines.append(" Frame &f = stack_.back();") lines.append(" Frame &f = stack_.back();")
lines.append(
" if (f.kind == Kind::Skip) { stack_.push_back(Frame{Kind::Skip, nullptr}); return; }"
)
lines.append(" SlotInfo si = slotInfoG(f);") lines.append(" SlotInfo si = slotInfoG(f);")
lines.append(
" if (si.cat == Cat::Skip) { stack_.push_back(Frame{Kind::Skip, nullptr}); return; }"
)
lines.append(f" if (si.cat != Cat::{event_cat}) {{ reject(); return; }}") lines.append(f" if (si.cat != Cat::{event_cat}) {{ reject(); return; }}")
lines.append(" void *p = engage(f, true);") lines.append(" void *p = engage(f, true);")
lines.append(" stack_.push_back(Frame{si.child, p});") lines.append(" stack_.push_back(Frame{si.child, p});")
@@ -872,7 +854,7 @@ public:
private: private:
{kind_enum} {kind_enum}
enum class Cat {{ Reject, Str, Int, Dbl, Bool, Enum, Obj, Arr, Skip }}; enum class Cat {{ Reject, Str, Int, Dbl, Bool, Enum, Obj, Arr }};
struct SlotInfo {{ struct SlotInfo {{
Cat cat; Cat cat;
Kind child{{}}; Kind child{{}};
@@ -883,11 +865,10 @@ private:
struct Frame {{ struct Frame {{
Kind kind; Kind kind;
void *dest; void *dest;
int field = -1; // object: selected field (-1 want key, -2 skip) int field = -1; // object: selected field (-1 means waiting for key)
std::vector<uint64_t> seen; // populated field bitset (object frames) std::vector<uint64_t> seen; // populated field bitset (object frames)
}}; }};
static constexpr int kWantKey = -1; static constexpr int kWantKey = -1;
static constexpr int kSkip = -2;
{self._enum_name_arrays()} {self._enum_name_arrays()}
@@ -902,10 +883,9 @@ private:
void reject() {{ error_ = true; }} void reject() {{ error_ = true; }}
// Wrap the generated slotInfo() with the generic key/skip states. // Wrap the generated slotInfo() with the generic key state.
SlotInfo slotInfoG(const Frame &f) {{ SlotInfo slotInfoG(const Frame &f) {{
if (isObjectKind(f.kind)) {{ if (isObjectKind(f.kind)) {{
if (f.field == kSkip) return SlotInfo{{Cat::Skip}};
if (f.field < 0) return SlotInfo{{Cat::Reject}}; if (f.field < 0) return SlotInfo{{Cat::Reject}};
}} }}
return slotInfo(f); return slotInfo(f);
@@ -930,11 +910,6 @@ private:
void cbEndObject() {{ void cbEndObject() {{
if (error_) return; if (error_) return;
Frame &f = stack_.back(); Frame &f = stack_.back();
if (f.kind == Kind::Skip) {{
stack_.pop_back();
if (stack_.empty() || stack_.back().kind != Kind::Skip) valueComplete();
return;
}}
if (isObjectKind(f.kind)) {{ if (isObjectKind(f.kind)) {{
const auto &req = requiredMask(f.kind); const auto &req = requiredMask(f.kind);
bool missing = false; bool missing = false;
@@ -952,11 +927,6 @@ private:
void cbEndArray() {{ void cbEndArray() {{
if (error_) return; if (error_) return;
Frame f = stack_.back(); Frame f = stack_.back();
if (f.kind == Kind::Skip) {{
stack_.pop_back();
if (stack_.empty() || stack_.back().kind != Kind::Skip) valueComplete();
return;
}}
stack_.pop_back(); stack_.pop_back();
valueComplete(); valueComplete();
}} }}
@@ -968,12 +938,10 @@ private:
scratch_.append(buf, len); scratch_.append(buf, len);
if (!done) return; if (!done) return;
int idx = matchKey(f.kind, scratch_); int idx = matchKey(f.kind, scratch_);
scratch_.clear();
if (idx < 0) {{ if (idx < 0) {{
if (isStrict(f.kind)) {{ reject(); return; }} reject(); return; // unknown key
f.field = kSkip;
return;
}} }}
scratch_.clear();
if (f.seen[idx >> 6] & (1ull << (idx & 63))) {{ if (f.seen[idx >> 6] & (1ull << (idx & 63))) {{
reject(); return; // duplicate key reject(); return; // duplicate key
}} }}
@@ -983,9 +951,7 @@ private:
void cbStringData(const char *buf, int len, int done) {{ void cbStringData(const char *buf, int len, int done) {{
if (error_) return; if (error_) return;
Frame &f = stack_.back(); Frame &f = stack_.back();
if (f.kind == Kind::Skip) return;
SlotInfo si = slotInfoG(f); SlotInfo si = slotInfoG(f);
if (si.cat == Cat::Skip) {{ if (done) valueComplete(); return; }}
if (si.cat == Cat::Str) {{ if (si.cat == Cat::Str) {{
auto *s = (std::string *)engage(f, !started_); auto *s = (std::string *)engage(f, !started_);
s->append(buf, len); s->append(buf, len);
@@ -1011,9 +977,7 @@ private:
void cbNumberData(const char *buf, int len, int done) {{ void cbNumberData(const char *buf, int len, int done) {{
if (error_) return; if (error_) return;
Frame &f = stack_.back(); Frame &f = stack_.back();
if (f.kind == Kind::Skip) return;
SlotInfo si = slotInfoG(f); SlotInfo si = slotInfoG(f);
if (si.cat == Cat::Skip) {{ if (done) valueComplete(); return; }}
if (si.cat != Cat::Int && si.cat != Cat::Dbl) {{ reject(); return; }} if (si.cat != Cat::Int && si.cat != Cat::Dbl) {{ reject(); return; }}
scratch_.append(buf, len); scratch_.append(buf, len);
started_ = true; started_ = true;
@@ -1051,9 +1015,7 @@ private:
void cbBool(bool value) {{ void cbBool(bool value) {{
if (error_) return; if (error_) return;
Frame &f = stack_.back(); Frame &f = stack_.back();
if (f.kind == Kind::Skip) return;
SlotInfo si = slotInfoG(f); SlotInfo si = slotInfoG(f);
if (si.cat == Cat::Skip) {{ valueComplete(); return; }}
if (si.cat != Cat::Bool) {{ reject(); return; }} if (si.cat != Cat::Bool) {{ reject(); return; }}
*(bool *)engage(f, true) = value; *(bool *)engage(f, true) = value;
valueComplete(); valueComplete();
@@ -1066,9 +1028,7 @@ private:
void cbNull() {{ void cbNull() {{
if (error_) return; if (error_) return;
Frame &f = stack_.back(); Frame &f = stack_.back();
if (f.kind == Kind::Skip) return;
SlotInfo si = slotInfoG(f); SlotInfo si = slotInfoG(f);
if (si.cat == Cat::Skip) {{ valueComplete(); return; }}
if (!si.nullable) {{ reject(); return; }} if (!si.nullable) {{ reject(); return; }}
if (isArrayKind(f.kind)) appendNull(f); // keep null array elements if (isArrayKind(f.kind)) appendNull(f); // keep null array elements
valueComplete(); // leave optional empty / pointer null valueComplete(); // leave optional empty / pointer null
@@ -1106,8 +1066,6 @@ private:
{self._reqmask()} {self._reqmask()}
{self._is_object_kind()} {self._is_object_kind()}
{self._is_strict()}
}}; }};
""" """