forked from weaselab/weaseljson
schemagen: drop support for additionalProperties: true
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`).
This commit is contained in:
@@ -45,7 +45,7 @@ into the result, so it is non-movable.
|
||||
| `$ref` to `$defs`/`definitions` | the referenced named struct |
|
||||
| recursive `$ref` | `std::unique_ptr<T>` (cycle broken) |
|
||||
| `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)
|
||||
|
||||
@@ -55,15 +55,15 @@ into the result, so it is non-movable.
|
||||
- value not in a string `enum`
|
||||
- a number not representable in the target type (e.g. `1.5` for an `integer`)
|
||||
- duplicate object keys
|
||||
- unknown key under `additionalProperties: false`
|
||||
- unknown key in any object
|
||||
|
||||
## Not supported (rejected at generation time, no fallback)
|
||||
|
||||
`oneOf` / `anyOf` / `allOf` / `not` / `if`-`then`-`else`,
|
||||
`patternProperties`, `additionalProperties` with a schema (typed map),
|
||||
`prefixItems` (tuples), `const`, `dependentSchemas`/`dependentRequired`,
|
||||
union `type` lists other than `["T", "null"]`, non-string enums, and remote
|
||||
(`$ref` to other documents).
|
||||
`additionalProperties: true`, `prefixItems` (tuples), `const`,
|
||||
`dependentSchemas`/`dependentRequired`, union `type` lists other than
|
||||
`["T", "null"]`, non-string enums, and remote (`$ref` to other documents).
|
||||
|
||||
## Notes
|
||||
|
||||
|
||||
@@ -52,14 +52,6 @@
|
||||
}
|
||||
}
|
||||
},
|
||||
"loose": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"known": {
|
||||
"type": "string"
|
||||
}
|
||||
}
|
||||
},
|
||||
"friends": {
|
||||
"type": "array",
|
||||
"items": {
|
||||
|
||||
@@ -186,20 +186,6 @@ int main() {
|
||||
expectReject(R"([1,2,3])", "array where object expected (root)");
|
||||
expectReject(R"({"name":"x","age":1,)", "truncated / invalid json");
|
||||
|
||||
// ---- duplicate keys in permissive (additionalProperties allowed) object
|
||||
// ----
|
||||
{
|
||||
RootBuilder b;
|
||||
WeaselJsonStatus s =
|
||||
parseStrided(b, R"({"name":"x","age":1,"loose":{"unknown":1}})");
|
||||
CHECK(s == WeaselJson_OK);
|
||||
printf("ok single unknown key in non-strict object\n");
|
||||
}
|
||||
expectReject(R"({"name":"x","age":1,"loose":{"unknown":1,"unknown":2}})",
|
||||
"duplicate unknown key in non-strict object");
|
||||
expectReject(R"({"name":"x","age":1,"loose":{"known":"a","known":"b"}})",
|
||||
"duplicate known key in non-strict object");
|
||||
|
||||
if (failures == 0) {
|
||||
printf("\nALL TESTS PASSED\n");
|
||||
return 0;
|
||||
|
||||
@@ -178,21 +178,69 @@ class SchemagenCollisionTest(unittest.TestCase):
|
||||
self.assertIn("Arr1", enumerators)
|
||||
self.assertEqual(len(enumerators), len(set(enumerators)))
|
||||
|
||||
def test_skip_is_reserved(self):
|
||||
def test_skip_user_type_is_allowed(self):
|
||||
schema = {
|
||||
"type": "array",
|
||||
"items": {"$ref": "#/$defs/Skip"},
|
||||
"$defs": {"Skip": {"type": "object", "properties": {}}},
|
||||
}
|
||||
out = self.generate_and_compile(schema)
|
||||
self.assertIn("struct Skip1", out)
|
||||
self.assertIn("struct Skip", out)
|
||||
self.assertNotIn("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.assertIn("Arr0", enumerators)
|
||||
self.assertEqual(len(enumerators), len(set(enumerators)))
|
||||
|
||||
|
||||
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__":
|
||||
unittest.main()
|
||||
|
||||
@@ -60,7 +60,6 @@ class ObjectType:
|
||||
def __init__(self, name):
|
||||
self.name = name
|
||||
self.fields = [] # list[Field]
|
||||
self.strict = False # additionalProperties: false
|
||||
|
||||
|
||||
class EnumType:
|
||||
@@ -234,7 +233,7 @@ class Builder:
|
||||
@staticmethod
|
||||
def _is_reserved(name):
|
||||
"""Names generated internally that must not collide with user types."""
|
||||
return name in ("Root", "Skip", "RootScalar")
|
||||
return name in ("Root", "RootScalar")
|
||||
|
||||
def ref_name(self, ref):
|
||||
if not ref.startswith("#/"):
|
||||
@@ -339,12 +338,13 @@ class Builder:
|
||||
# register for $ref cycles before building fields
|
||||
if defname is not None:
|
||||
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):
|
||||
raise GenError(
|
||||
"additionalProperties with a schema (typed map) is not " "supported yet"
|
||||
)
|
||||
obj.strict = ap is False
|
||||
required = set(node.get("required", []))
|
||||
props = node.get("properties", {})
|
||||
seen_cpp = set()
|
||||
@@ -566,7 +566,6 @@ class Emitter:
|
||||
#include <optional>
|
||||
#include <string>
|
||||
#include <string_view>
|
||||
#include <unordered_set>
|
||||
#include <vector>
|
||||
|
||||
#include "weaseljson.h"
|
||||
@@ -621,7 +620,6 @@ namespace {ns} {{"""
|
||||
root_is_container = isinstance(self.root_ty, (TObj, TArr))
|
||||
if not root_is_container:
|
||||
kinds.append("RootScalar")
|
||||
kinds.append("Skip")
|
||||
self.kind_order = kinds
|
||||
return " enum class Kind : uint8_t { " + ", ".join(kinds) + " };"
|
||||
|
||||
@@ -810,17 +808,6 @@ namespace {ns} {{"""
|
||||
lines.append(" }")
|
||||
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):
|
||||
out = []
|
||||
for e in self.b.enums.values():
|
||||
@@ -862,13 +849,7 @@ namespace {ns} {{"""
|
||||
else:
|
||||
lines.append(" if (stack_.empty()) { reject(); return; }")
|
||||
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(
|
||||
" 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(" void *p = engage(f, true);")
|
||||
lines.append(" stack_.push_back(Frame{si.child, p});")
|
||||
@@ -919,7 +900,7 @@ public:
|
||||
|
||||
private:
|
||||
{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 {{
|
||||
Cat cat;
|
||||
Kind child{{}};
|
||||
@@ -930,12 +911,10 @@ private:
|
||||
struct Frame {{
|
||||
Kind kind;
|
||||
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::unordered_set<std::string> unknown; // unknown keys seen in non-strict objects
|
||||
}};
|
||||
static constexpr int kWantKey = -1;
|
||||
static constexpr int kSkip = -2;
|
||||
|
||||
{self._enum_name_arrays()}
|
||||
|
||||
@@ -950,10 +929,9 @@ private:
|
||||
|
||||
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) {{
|
||||
if (isObjectKind(f.kind)) {{
|
||||
if (f.field == kSkip) return SlotInfo{{Cat::Skip}};
|
||||
if (f.field < 0) return SlotInfo{{Cat::Reject}};
|
||||
}}
|
||||
return slotInfo(f);
|
||||
@@ -978,11 +956,6 @@ private:
|
||||
void cbEndObject() {{
|
||||
if (error_) return;
|
||||
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)) {{
|
||||
const auto &req = requiredMask(f.kind);
|
||||
bool missing = false;
|
||||
@@ -1000,11 +973,6 @@ private:
|
||||
void cbEndArray() {{
|
||||
if (error_) return;
|
||||
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();
|
||||
valueComplete();
|
||||
}}
|
||||
@@ -1017,13 +985,7 @@ private:
|
||||
if (!done) return;
|
||||
int idx = matchKey(f.kind, scratch_);
|
||||
if (idx < 0) {{
|
||||
if (isStrict(f.kind)) {{ reject(); return; }}
|
||||
if (!f.unknown.insert(scratch_).second) {{
|
||||
reject(); return; // duplicate unknown key
|
||||
}}
|
||||
scratch_.clear();
|
||||
f.field = kSkip;
|
||||
return;
|
||||
reject(); return; // unknown key
|
||||
}}
|
||||
scratch_.clear();
|
||||
if (f.seen[idx >> 6] & (1ull << (idx & 63))) {{
|
||||
@@ -1035,9 +997,7 @@ private:
|
||||
void cbStringData(const char *buf, int len, int done) {{
|
||||
if (error_) return;
|
||||
Frame &f = stack_.back();
|
||||
if (f.kind == Kind::Skip) return;
|
||||
SlotInfo si = slotInfoG(f);
|
||||
if (si.cat == Cat::Skip) {{ if (done) valueComplete(); return; }}
|
||||
if (si.cat == Cat::Str) {{
|
||||
auto *s = (std::string *)engage(f, !started_);
|
||||
s->append(buf, len);
|
||||
@@ -1063,9 +1023,7 @@ private:
|
||||
void cbNumberData(const char *buf, int len, int done) {{
|
||||
if (error_) return;
|
||||
Frame &f = stack_.back();
|
||||
if (f.kind == Kind::Skip) return;
|
||||
SlotInfo si = slotInfoG(f);
|
||||
if (si.cat == Cat::Skip) {{ if (done) valueComplete(); return; }}
|
||||
if (si.cat != Cat::Int && si.cat != Cat::Dbl) {{ reject(); return; }}
|
||||
scratch_.append(buf, len);
|
||||
started_ = true;
|
||||
@@ -1103,9 +1061,7 @@ private:
|
||||
void cbBool(bool value) {{
|
||||
if (error_) return;
|
||||
Frame &f = stack_.back();
|
||||
if (f.kind == Kind::Skip) return;
|
||||
SlotInfo si = slotInfoG(f);
|
||||
if (si.cat == Cat::Skip) {{ valueComplete(); return; }}
|
||||
if (si.cat != Cat::Bool) {{ reject(); return; }}
|
||||
*(bool *)engage(f, true) = value;
|
||||
valueComplete();
|
||||
@@ -1121,9 +1077,7 @@ private:
|
||||
{null_at_root}
|
||||
}}
|
||||
Frame &f = stack_.back();
|
||||
if (f.kind == Kind::Skip) return;
|
||||
SlotInfo si = slotInfoG(f);
|
||||
if (si.cat == Cat::Skip) {{ valueComplete(); return; }}
|
||||
if (!si.nullable) {{ reject(); return; }}
|
||||
if (isArrayKind(f.kind)) appendNull(f); // keep null array elements
|
||||
valueComplete(); // leave optional empty / pointer null
|
||||
@@ -1161,8 +1115,6 @@ private:
|
||||
{self._reqmask()}
|
||||
|
||||
{self._is_object_kind()}
|
||||
|
||||
{self._is_strict()}
|
||||
}};
|
||||
"""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user