Merge pull request 'schemagen: drop support for additionalProperties: true' (#28) from weaselbot/weaseljson:weaselbot/issue-20 into main

Reviewed-on: weaselab/weaseljson#28

contrib/schemagen/README.md

@@ -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`,
+`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
 

contrib/schemagen/test_schemagen.py

@@ -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()

contrib/schemagen/weaseljson_schemagen.py

@@ -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()
@@ -620,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) + " };"
 
@@ -809,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():
@@ -861,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});")
@@ -918,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{{}};
@@ -929,11 +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)
   }};
   static constexpr int kWantKey = -1;
-  static constexpr int kSkip = -2;
 
 {self._enum_name_arrays()}
 
@@ -948,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);
@@ -976,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;
@@ -998,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();
   }}
@@ -1014,12 +984,10 @@ private:
     scratch_.append(buf, len);
     if (!done) return;
     int idx = matchKey(f.kind, scratch_);
-    scratch_.clear();
     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))) {{
       reject(); return;  // duplicate key
     }}
@@ -1029,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);
@@ -1057,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;
@@ -1097,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();
@@ -1115,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
@@ -1155,8 +1115,6 @@ private:
 {self._reqmask()}
 
 {self._is_object_kind()}
-
-{self._is_strict()}
 }};
 """