Merge f286589a32 into cc2983d375

common/json-schema-to-grammar.cpp

@@ -700,10 +700,9 @@ private:
         const std::string & name,
         const json & additional_properties)
     {
-        std::vector<std::string> required_props;
-        std::vector<std::string> optional_props;
-        std::unordered_map<std::string, std::string> prop_kv_rule_names;
         std::vector<std::string> prop_names;
+        prop_names.reserve(properties.size() + 1);
+        std::unordered_map<std::string, std::string> prop_kv_rule_names;
         for (const auto & kv : properties) {
             const auto &prop_name = kv.first;
             const auto &prop_schema = kv.second;
@@ -713,11 +712,6 @@ private:
                 name + (name.empty() ? "" : "-") + prop_name + "-kv",
                 format_literal(json(prop_name).dump()) + " space \":\" space " + prop_rule_name
             );
-            if (required.find(prop_name) != required.end()) {
-                required_props.push_back(prop_name);
-            } else {
-                optional_props.push_back(prop_name);
-            }
             prop_names.push_back(prop_name);
         }
         if ((additional_properties.is_boolean() && additional_properties.get<bool>()) || additional_properties.is_object()) {
@@ -731,23 +725,11 @@ private:
                 : _add_rule(sub_name + "-k", _not_strings(prop_names));
             std::string kv_rule = _add_rule(sub_name + "-kv", key_rule + " \":\" space " + value_rule);
             prop_kv_rule_names["*"] = kv_rule;
-            optional_props.push_back("*");
+            prop_names.push_back("*");
         }
 
         std::string rule = "\"{\" space ";
-        for (size_t i = 0; i < required_props.size(); i++) {
+        if (!prop_kv_rule_names.empty()) {
-            if (i > 0) {
-                rule += " \",\" space ";
-            }
-            rule += prop_kv_rule_names[required_props[i]];
-        }
-
-        if (!optional_props.empty()) {
-            rule += " (";
-            if (!required_props.empty()) {
-                rule += " \",\" space ( ";
-            }
-
             std::function<std::string(const std::vector<std::string> &, bool)> get_recursive_refs = [&](const std::vector<std::string> & ks, bool first_is_optional) {
                 std::string res;
                 if (ks.empty()) {
@@ -755,11 +737,15 @@ private:
                 }
                 std::string k = ks[0];
                 std::string kv_rule_name = prop_kv_rule_names[k];
-                std::string comma_ref = "( \",\" space " + kv_rule_name + " )";
+                std::string comma_ref = "\",\" space " + kv_rule_name;
                 if (first_is_optional) {
-                    res = comma_ref + (k == "*" ? "*" : "?");
+                    if (required.find(k) == required.end()) {
+                        res = "( " + comma_ref + (k == "*" ? " )*" : " )?");
+                    } else {
+                        res = comma_ref;
+                    }
                 } else {
-                    res = kv_rule_name + (k == "*" ? " " + comma_ref + "*" : "");
+                    res = kv_rule_name + (k == "*" ? " ( " + comma_ref + " )*" : "");
                 }
                 if (ks.size() > 1) {
                     res += " " + _add_rule(
@@ -770,16 +756,21 @@ private:
                 return res;
             };
 
-            for (size_t i = 0; i < optional_props.size(); i++) {
+            std::vector<std::string> alternatives;
-                if (i > 0) {
+            auto has_required = false;
-                    rule += " | ";
+            for (size_t i = 0; i < prop_names.size(); i++) {
+                alternatives.push_back(get_recursive_refs(std::vector<std::string>(prop_names.begin() + i, prop_names.end()), false));
+                if (required.find(prop_names[i]) != required.end()) {
+                    has_required = true;
+                    break;
                 }
-                rule += get_recursive_refs(std::vector<std::string>(optional_props.begin() + i, optional_props.end()), false);
             }
-            if (!required_props.empty()) {
+            auto alts = join(alternatives.begin(), alternatives.end(), " | ");
-                rule += " )";
+            if (alternatives.size() > 1 || !has_required) {
+                rule += "( " + alts + (has_required ? " )" : " )?");
+            } else {
+                rule += alts;
             }
-            rule += " )?";
         }
 
         rule += " \"}\" space";

examples/json_schema_to_grammar.py

@@ -8,6 +8,7 @@ import re
 import sys
 from typing import Any, List, Optional, Set, Tuple, Union
 
+
 def _build_repetition(item_rule, min_items, max_items, separator_rule=None):
 
     if min_items == 0 and max_items == 1:
@@ -679,49 +680,48 @@ class SchemaConverter:
         return n
 
     def _build_object_rule(self, properties: List[Tuple[str, Any]], required: Set[str], name: str, additional_properties: Optional[Union[bool, Any]]):
-        prop_order = self._prop_order
-        # sort by position in prop_order (if specified) then by original order
-        sorted_props = [kv[0] for _, kv in sorted(enumerate(properties), key=lambda ikv: (prop_order.get(ikv[1][0], len(prop_order)), ikv[0]))]
-
         prop_kv_rule_names = {}
+        prop_names = []
         for prop_name, prop_schema in properties:
             prop_rule_name = self.visit(prop_schema, f'{name}{"-" if name else ""}{prop_name}')
             prop_kv_rule_names[prop_name] = self._add_rule(
                 f'{name}{"-" if name else ""}{prop_name}-kv',
                 fr'{self._format_literal(json.dumps(prop_name))} space ":" space {prop_rule_name}'
             )
-        required_props = [k for k in sorted_props if k in required]
+            prop_names.append(prop_name)
-        optional_props = [k for k in sorted_props if k not in required]
+
+        prop_order = self._prop_order
+        if prop_order:
+            # sort by position in prop_order (if specified) then by original order
+            prop_names.sort(key=lambda k: (prop_order.get(k, float('inf')), prop_names.index(k)))
 
         if additional_properties is not None and additional_properties != False:
             sub_name = f'{name}{"-" if name else ""}additional'
             value_rule = self.visit(additional_properties, f'{sub_name}-value') if isinstance(additional_properties, dict) else \
                 self._add_primitive('value', PRIMITIVE_RULES['value'])
-            key_rule = self._add_primitive('string', PRIMITIVE_RULES['string']) if not sorted_props \
+            key_rule = self._add_primitive('string', PRIMITIVE_RULES['string']) if not prop_names \
-                else self._add_rule(f'{sub_name}-k', self._not_strings(sorted_props))
+                else self._add_rule(f'{sub_name}-k', self._not_strings(prop_names))
 
             prop_kv_rule_names["*"] = self._add_rule(
                 f'{sub_name}-kv',
                 f'{key_rule} ":" space {value_rule}'
             )
-            optional_props.append("*")
+            prop_names.append("*")
 
         rule = '"{" space '
-        rule += ' "," space '.join(prop_kv_rule_names[k] for k in required_props)
-
-        if optional_props:
-            rule += ' ('
-            if required_props:
-                rule += ' "," space ( '
 
+        if prop_kv_rule_names:
             def get_recursive_refs(ks, first_is_optional):
                 [k, *rest] = ks
                 kv_rule_name = prop_kv_rule_names[k]
-                comma_ref = f'( "," space {kv_rule_name} )'
+                comma_ref = f'"," space {kv_rule_name}'
                 if first_is_optional:
-                    res = comma_ref + ('*' if k == '*' else '?')
+                    if k not in required:
+                        res = '( ' + comma_ref + (' )*' if k == '*' else ' )?')
+                    else:
+                        res = comma_ref
                 else:
-                    res = kv_rule_name + (' ' + comma_ref + "*" if k == '*' else '')
+                    res = kv_rule_name + (' ( ' + comma_ref + " )*" if k == '*' else '')
                 if len(rest) > 0:
                     res += ' ' + self._add_rule(
                         f'{name}{"-" if name else ""}{k}-rest',
@@ -729,13 +729,19 @@ class SchemaConverter:
                     )
                 return res
 
-            rule += ' | '.join(
+            alternatives = []
-                get_recursive_refs(optional_props[i:], first_is_optional=False)
+            has_required = False
-                for i in range(len(optional_props))
+            for i, k in enumerate(prop_names):
-            )
+                alternatives.append(get_recursive_refs(prop_names[i:], first_is_optional=False))
-            if required_props:
+                if k in required:
-                rule += ' )'
+                    has_required = True
-            rule += ' )?'
+                    break
+
+            alts = ' | '.join(alternatives)
+            if len(alternatives) > 1 or not has_required:
+                rule += '( ' + alts + (' )' if has_required else ' )?')
+            else:
+                rule += alts
 
         rule += ' "}" space'
 

examples/server/public/json-schema-to-grammar.mjs

@@ -732,24 +732,26 @@ export class SchemaConverter {
   }
 
   _buildObjectRule(properties, required, name, additionalProperties) {
-    const propOrder = this._propOrder;
-    // sort by position in prop_order (if specified) then by original order
-    const sortedProps = properties.map(([k]) => k).sort((a, b) => {
-      const orderA = propOrder[a] || Infinity;
-      const orderB = propOrder[b] || Infinity;
-      return orderA - orderB || properties.findIndex(([k]) => k === a) - properties.findIndex(([k]) => k === b);
-    });
-
     const propKvRuleNames = {};
+    const propNames = []
     for (const [propName, propSchema] of properties) {
       const propRuleName = this.visit(propSchema, `${name ?? ''}${name ? '-' : ''}${propName}`);
       propKvRuleNames[propName] = this._addRule(
         `${name ?? ''}${name ? '-' : ''}${propName}-kv`,
         `${this._formatLiteral(JSON.stringify(propName))} space ":" space ${propRuleName}`
       );
+      propNames.push(propName);
+    }
+
+    const propOrder = this._propOrder;
+    if (Object.keys(propOrder).length > 0) {
+      // sort by position in prop_order (if specified) then by original order
+      propNames.sort((a, b) => {
+        const orderA = propOrder[a] || Infinity;
+        const orderB = propOrder[b] || Infinity;
+        return orderA - orderB || properties.findIndex(([k]) => k === a) - properties.findIndex(([k]) => k === b);
+      });
     }
-    const requiredProps = sortedProps.filter(k => required.has(k));
-    const optionalProps = sortedProps.filter(k => !required.has(k));
 
     if (additionalProperties) {
       const subName = `${name ?? ''}${name ? '-' : ''}additional`;
@@ -758,33 +760,32 @@ export class SchemaConverter {
         : this._addPrimitive('value', PRIMITIVE_RULES['value']);
 
       const key_rule =
-        sortedProps.length === 0 ? this._addPrimitive('string', PRIMITIVE_RULES['string'])
+        propNames.length === 0 ? this._addPrimitive('string', PRIMITIVE_RULES['string'])
-        : this._addRule(`${subName}-k`, this._notStrings(sortedProps));
+        : this._addRule(`${subName}-k`, this._notStrings(propNames));
 
       propKvRuleNames['*'] = this._addRule(
         `${subName}-kv`,
         `${key_rule} ":" space ${valueRule}`);
-      optionalProps.push('*');
+      propNames.push('*');
     }
 
     let rule = '"{" space ';
-    rule += requiredProps.map(k => propKvRuleNames[k]).join(' "," space ');
-
-    if (optionalProps.length > 0) {
-      rule += ' (';
-      if (requiredProps.length > 0) {
-        rule += ' "," space ( ';
-      }
 
+    if (propNames.length > 0) {
       const getRecursiveRefs = (ks, firstIsOptional) => {
         const [k, ...rest] = ks;
         const kvRuleName = propKvRuleNames[k];
         let res;
-        const commaRef = `( "," space ${kvRuleName} )`;
+        const commaRef = `"," space ${kvRuleName}`;
         if (firstIsOptional) {
-          res = commaRef + (k === '*' ? '*' : '?');
+          // res = commaRef + (k === '*' ? '*' : '?');
+          if (!required.has(k)) {
+            res = `( ${commaRef} )${k === '*' ? '*' : '?'}`;
+          } else {
+            res = commaRef;
+          }
         } else {
-          res = kvRuleName + (k === '*' ? ' ' + commaRef + '*' : '');
+          res = kvRuleName + (k === '*' ? ' ( ' + commaRef + ' )*' : '');
         }
         if (rest.length > 0) {
           res += ' ' + this._addRule(
@@ -795,11 +796,22 @@ export class SchemaConverter {
         return res;
       };
 
-      rule += optionalProps.map((_, i) => getRecursiveRefs(optionalProps.slice(i), false)).join(' | ');
+      let hasRequired = false;
-      if (requiredProps.length > 0) {
+      const alternatives = [];
-        rule += ' )';
+      for (let i = 0; i < propNames.length; i++) {
+        alternatives.push(getRecursiveRefs(propNames.slice(i), false));
+        if (required.has(propNames[i])) {
+          hasRequired = true;
+          break;
+        }
+      }
+
+      const alts = alternatives.join(' | ');
+      if (alternatives.length > 1 || !hasRequired) {
+        rule += `( ${alts} )${hasRequired ? '' : '?'}`;
+      } else {
+        rule += alts;
       }
-      rule += ' )?';
     }
 
     rule += ' "}" space';

tests/test-grammar-integration.cpp

@@ -1104,6 +1104,33 @@ static void test_json_schema() {
         }
     );
 
+    test_schema(
+        "object property order",
+        // Schema
+        R"""({
+            "type": "object",
+            "properties": {
+                "a": { "type": "integer" },
+                "b": { "type": "integer" },
+                "c": { "type": "integer" },
+                "d": { "type": "integer" }
+            },
+            "required": ["b", "d"]
+        })""",
+        // Passing strings
+        {
+            R"""({ "b": 0, "d": 0 })""",
+            R"""({ "b": 0, "d": 0, "E": -1 })""",
+            R"""({ "a": 0, "b": 0, "c": 0, "d": 0 })""",
+            R"""({ "a": 0, "b": 0, "c": 0, "d": 0, "E": -1 })""",
+        },
+        // Failing strings
+        {
+            R"""({ "E": -1, "b": 0, "d": 0 })""",
+            R"""({ "b": 0, "d": 0, "a": 0 })""",
+        }
+    );
+
     test_schema(
         "additional properties can't override other properties",
         R"""({

tests/test-json-schema-to-grammar.cpp

@@ -776,9 +776,11 @@ static void test_all(const std::string & lang, std::function<void(const TestCase
         R"""(
             a-kv ::= "\"a\"" space ":" space string
             b-kv ::= "\"b\"" space ":" space string
+            b-rest ::= "," space c-kv c-rest
             c-kv ::= "\"c\"" space ":" space string
+            c-rest ::= "," space a-kv
             char ::= [^"\\\x7F\x00-\x1F] | [\\] (["\\bfnrt] | "u" [0-9a-fA-F]{4})
-            root ::= "{" space b-kv "," space c-kv "," space a-kv "}" space
+            root ::= "{" space b-kv b-rest "}" space
             space ::= | " " | "\n" [ \t]{0,20}
             string ::= "\"" char* "\"" space
         )"""
@@ -798,7 +800,7 @@ static void test_all(const std::string & lang, std::function<void(const TestCase
         R"""(
             a-kv ::= "\"a\"" space ":" space string
             char ::= [^"\\\x7F\x00-\x1F] | [\\] (["\\bfnrt] | "u" [0-9a-fA-F]{4})
-            root ::= "{" space  (a-kv )? "}" space
+            root ::= "{" space ( a-kv )? "}" space
             space ::= | " " | "\n" [ \t]{0,20}
             string ::= "\"" char* "\"" space
         )"""
@@ -822,7 +824,7 @@ static void test_all(const std::string & lang, std::function<void(const TestCase
             b-rest ::= ( "," space c-kv )?
             c-kv ::= "\"c\"" space ":" space string
             char ::= [^"\\\x7F\x00-\x1F] | [\\] (["\\bfnrt] | "u" [0-9a-fA-F]{4})
-            root ::= "{" space  (a-kv a-rest | b-kv b-rest | c-kv )? "}" space
+            root ::= "{" space ( a-kv a-rest | b-kv b-rest | c-kv )? "}" space
             space ::= | " " | "\n" [ \t]{0,20}
             string ::= "\"" char* "\"" space
         )"""
@@ -830,13 +832,13 @@ static void test_all(const std::string & lang, std::function<void(const TestCase
 
     test({
         SUCCESS,
-        "required + optional props each in original order",
+        "required + optional props each in unified order",
         R"""({
             "properties": {
-                "b": {"type": "string"},
-                "a": {"type": "string"},
                 "d": {"type": "string"},
-                "c": {"type": "string"}
+                "c": {"type": "string"},
+                "b": {"type": "string"},
+                "a": {"type": "string"}
             },
             "required": ["a", "b"],
             "additionalProperties": false
@@ -844,11 +846,13 @@ static void test_all(const std::string & lang, std::function<void(const TestCase
         R"""(
             a-kv ::= "\"a\"" space ":" space string
             b-kv ::= "\"b\"" space ":" space string
+            b-rest ::= "," space a-kv
             c-kv ::= "\"c\"" space ":" space string
+            c-rest ::= "," space b-kv b-rest
             char ::= [^"\\\x7F\x00-\x1F] | [\\] (["\\bfnrt] | "u" [0-9a-fA-F]{4})
             d-kv ::= "\"d\"" space ":" space string
-            d-rest ::= ( "," space c-kv )?
+            d-rest ::= ( "," space c-kv )? c-rest
-            root ::= "{" space b-kv "," space a-kv ( "," space ( d-kv d-rest | c-kv ) )? "}" space
+            root ::= "{" space ( d-kv d-rest | c-kv c-rest | b-kv b-rest ) "}" space
             space ::= | " " | "\n" [ \t]{0,20}
             string ::= "\"" char* "\"" space
         )"""
@@ -868,7 +872,7 @@ static void test_all(const std::string & lang, std::function<void(const TestCase
             decimal-part ::= [0-9]{1,16}
             integral-part ::= [0] | [1-9] [0-9]{0,15}
             number ::= ("-"? integral-part) ("." decimal-part)? ([eE] [-+]? integral-part)? space
-            root ::= "{" space  (additional-kv ( "," space additional-kv )* )? "}" space
+            root ::= "{" space ( additional-kv ( "," space additional-kv )* )? "}" space
             space ::= | " " | "\n" [ \t]{0,20}
             string ::= "\"" char* "\"" space
         )"""
@@ -945,13 +949,14 @@ static void test_all(const std::string & lang, std::function<void(const TestCase
         })""",
         R"""(
             a-kv ::= "\"a\"" space ":" space number
+            a-rest ::= ( "," space additional-kv )*
             additional-k ::= ["] ( [a] char+ | [^"a] char* )? ["] space
             additional-kv ::= additional-k ":" space string
             char ::= [^"\\\x7F\x00-\x1F] | [\\] (["\\bfnrt] | "u" [0-9a-fA-F]{4})
             decimal-part ::= [0-9]{1,16}
             integral-part ::= [0] | [1-9] [0-9]{0,15}
             number ::= ("-"? integral-part) ("." decimal-part)? ([eE] [-+]? integral-part)? space
-            root ::= "{" space a-kv ( "," space ( additional-kv ( "," space additional-kv )* ) )? "}" space
+            root ::= "{" space a-kv a-rest "}" space
             space ::= | " " | "\n" [ \t]{0,20}
             string ::= "\"" char* "\"" space
         )"""
@@ -976,7 +981,7 @@ static void test_all(const std::string & lang, std::function<void(const TestCase
             decimal-part ::= [0-9]{1,16}
             integral-part ::= [0] | [1-9] [0-9]{0,15}
             number ::= ("-"? integral-part) ("." decimal-part)? ([eE] [-+]? integral-part)? space
-            root ::= "{" space  (a-kv a-rest | additional-kv ( "," space additional-kv )* )? "}" space
+            root ::= "{" space ( a-kv a-rest | additional-kv ( "," space additional-kv )* )? "}" space
             space ::= | " " | "\n" [ \t]{0,20}
         )"""
     });
@@ -999,11 +1004,12 @@ static void test_all(const std::string & lang, std::function<void(const TestCase
             also-kv ::= "\"also\"" space ":" space number
             also-rest ::= ( "," space additional-kv )*
             and-kv ::= "\"and\"" space ":" space number
+            and-rest ::= ( "," space also-kv )? also-rest
             char ::= [^"\\\x7F\x00-\x1F] | [\\] (["\\bfnrt] | "u" [0-9a-fA-F]{4})
             decimal-part ::= [0-9]{1,16}
             integral-part ::= [0] | [1-9] [0-9]{0,15}
             number ::= ("-"? integral-part) ("." decimal-part)? ([eE] [-+]? integral-part)? space
-            root ::= "{" space and-kv ( "," space ( also-kv also-rest | additional-kv ( "," space additional-kv )* ) )? "}" space
+            root ::= "{" space and-kv and-rest "}" space
             space ::= | " " | "\n" [ \t]{0,20}
         )"""
     });
@@ -1029,7 +1035,7 @@ static void test_all(const std::string & lang, std::function<void(const TestCase
             integer ::= ("-"? integral-part) space
             integral-part ::= [0] | [1-9] [0-9]{0,15}
             root ::= ("-"? integral-part) space
-            root0 ::= "{" space  (-kv -rest | a-kv a-rest | additional-kv ( "," space additional-kv )* )? "}" space
+            root0 ::= "{" space ( -kv -rest | a-kv a-rest | additional-kv ( "," space additional-kv )* )? "}" space
             space ::= | " " | "\n" [ \t]{0,20}
         )"""
     });
@@ -1054,7 +1060,7 @@ static void test_all(const std::string & lang, std::function<void(const TestCase
             char ::= [^"\\\x7F\x00-\x1F] | [\\] (["\\bfnrt] | "u" [0-9a-fA-F]{4})
             integer ::= ("-"? integral-part) space
             integral-part ::= [0] | [1-9] [0-9]{0,15}
-            root ::= "{" space  (a-kv a-rest | aa-kv aa-rest | additional-kv ( "," space additional-kv )* )? "}" space
+            root ::= "{" space ( a-kv a-rest | aa-kv aa-rest | additional-kv ( "," space additional-kv )* )? "}" space
             space ::= | " " | "\n" [ \t]{0,20}
         )"""
     });
@@ -1079,7 +1085,7 @@ static void test_all(const std::string & lang, std::function<void(const TestCase
             char ::= [^"\\\x7F\x00-\x1F] | [\\] (["\\bfnrt] | "u" [0-9a-fA-F]{4})
             integer ::= ("-"? integral-part) space
             integral-part ::= [0] | [1-9] [0-9]{0,15}
-            root ::= "{" space  (ab-kv ab-rest | ac-kv ac-rest | additional-kv ( "," space additional-kv )* )? "}" space
+            root ::= "{" space ( ab-kv ab-rest | ac-kv ac-rest | additional-kv ( "," space additional-kv )* )? "}" space
             space ::= | " " | "\n" [ \t]{0,20}
         )"""
     });
@@ -1135,10 +1141,10 @@ static void test_all(const std::string & lang, std::function<void(const TestCase
         R"""(
             alternative-0 ::= foo
             alternative-1 ::= bar
-            bar ::= "{" space  (bar-b-kv )? "}" space
+            bar ::= "{" space ( bar-b-kv )? "}" space
             bar-b-kv ::= "\"b\"" space ":" space number
             decimal-part ::= [0-9]{1,16}
-            foo ::= "{" space  (foo-a-kv )? "}" space
+            foo ::= "{" space ( foo-a-kv )? "}" space
             foo-a-kv ::= "\"a\"" space ":" space number
             integral-part ::= [0] | [1-9] [0-9]{0,15}
             number ::= ("-"? integral-part) ("." decimal-part)? ([eE] [-+]? integral-part)? space
@@ -1179,14 +1185,16 @@ static void test_all(const std::string & lang, std::function<void(const TestCase
         })""",
         R"""(
             a-kv ::= "\"a\"" space ":" space number
+            a-rest ::= "," space b-kv b-rest
             b-kv ::= "\"b\"" space ":" space number
+            b-rest ::= ( "," space d-kv )? d-rest
             c-kv ::= "\"c\"" space ":" space number
             d-kv ::= "\"d\"" space ":" space number
             d-rest ::= ( "," space c-kv )?
             decimal-part ::= [0-9]{1,16}
             integral-part ::= [0] | [1-9] [0-9]{0,15}
             number ::= ("-"? integral-part) ("." decimal-part)? ([eE] [-+]? integral-part)? space
-            root ::= "{" space a-kv "," space b-kv ( "," space ( d-kv d-rest | c-kv ) )? "}" space
+            root ::= "{" space a-kv a-rest "}" space
             space ::= | " " | "\n" [ \t]{0,20}
         )"""
     });