Introduce a new LoopPropertiesType(Strict) compiler step

Signed-off-by: Juan Cruz Viotti <[email protected]>
sourcemeta · Sep 23, 2024 · 536d5b0 · 536d5b0
1 parent a4c71c9
commit 536d5b0
Show file tree

Hide file tree

Showing 7 changed files with 109 additions and 89 deletions.
diff --git a/src/jsonschema/compile_describe.cc b/src/jsonschema/compile_describe.cc
@@ -637,6 +637,22 @@ struct DescribeVisitor {
     return message.str();
   }
 
+  auto operator()(const SchemaCompilerLoopPropertiesType &step) const
+      -> std::string {
+    std::ostringstream message;
+    message << "The object properties were expected to be of type "
+            << to_string(step.value);
+    return message.str();
+  }
+
+  auto operator()(const SchemaCompilerLoopPropertiesTypeStrict &step) const
+      -> std::string {
+    std::ostringstream message;
+    message << "The object properties were expected to be of type "
+            << to_string(step.value);
+    return message.str();
+  }
+
   auto operator()(const SchemaCompilerLoopKeys &) const -> std::string {
     assert(this->keyword == "propertyNames");
     assert(this->target.is_object());

diff --git a/src/jsonschema/compile_evaluate.cc b/src/jsonschema/compile_evaluate.cc
@@ -1005,6 +1005,42 @@ auto evaluate_step(
 
   evaluate_loop_properties_except_end:
     EVALUATE_END(loop, SchemaCompilerLoopPropertiesExcept);
+  } else if (IS_STEP(SchemaCompilerLoopPropertiesType)) {
+    EVALUATE_BEGIN(loop, SchemaCompilerLoopPropertiesType, target.is_object());
+    result = true;
+    for (const auto &entry : target.as_object()) {
+      context.enter(entry.first);
+      const auto &entry_target{context.resolve_target()};
+      // In non-strict mode, we consider a real number that represents an
+      // integer to be an integer
+      if (entry_target.type() != loop.value &&
+          (loop.value != JSON::Type::Integer ||
+           entry_target.is_integer_real())) {
+        result = false;
+        context.leave();
+        break;
+      }
+
+      context.leave();
+    }
+
+    EVALUATE_END(loop, SchemaCompilerLoopPropertiesType);
+  } else if (IS_STEP(SchemaCompilerLoopPropertiesTypeStrict)) {
+    EVALUATE_BEGIN(loop, SchemaCompilerLoopPropertiesTypeStrict,
+                   target.is_object());
+    result = true;
+    for (const auto &entry : target.as_object()) {
+      context.enter(entry.first);
+      if (context.resolve_target().type() != loop.value) {
+        result = false;
+        context.leave();
+        break;
+      }
+
+      context.leave();
+    }
+
+    EVALUATE_END(loop, SchemaCompilerLoopPropertiesTypeStrict);
   } else if (IS_STEP(SchemaCompilerLoopKeys)) {
     EVALUATE_BEGIN(loop, SchemaCompilerLoopKeys, target.is_object());
     result = true;

diff --git a/src/jsonschema/compile_helpers.h b/src/jsonschema/compile_helpers.h
@@ -17,7 +17,7 @@ auto make(const bool report, const SchemaCompilerContext &context,
           const SchemaCompilerSchemaContext &schema_context,
           const SchemaCompilerDynamicContext &dynamic_context,
           // Take the value type from the "type" property of the step struct
-          decltype(std::declval<Step>().value) &&value) -> Step {
+          const decltype(std::declval<Step>().value) &value) -> Step {
   return {
       dynamic_context.keyword.empty()
           ? dynamic_context.base_schema_location
@@ -28,7 +28,7 @@ auto make(const bool report, const SchemaCompilerContext &context,
       schema_context.base.recompose(),
       context.uses_dynamic_scopes,
       report,
-      std::move(value)};
+      value};
 }
 
 // Instantiate an applicator step

diff --git a/src/jsonschema/compile_json.cc b/src/jsonschema/compile_json.cc
@@ -271,6 +271,9 @@ struct StepVisitor {
   HANDLE_STEP("loop", "properties-no-annotation",
               SchemaCompilerLoopPropertiesNoAnnotation)
   HANDLE_STEP("loop", "properties-except", SchemaCompilerLoopPropertiesExcept)
+  HANDLE_STEP("loop", "properties-type", SchemaCompilerLoopPropertiesType)
+  HANDLE_STEP("loop", "properties-type-strict",
+              SchemaCompilerLoopPropertiesTypeStrict)
   HANDLE_STEP("loop", "keys", SchemaCompilerLoopKeys)
   HANDLE_STEP("loop", "items", SchemaCompilerLoopItems)
   HANDLE_STEP("loop", "items-unmarked", SchemaCompilerLoopItemsUnmarked)

diff --git a/src/jsonschema/default_compiler_draft4.h b/src/jsonschema/default_compiler_draft4.h
@@ -613,6 +613,24 @@ auto compiler_draft4_applicator_additionalproperties_conditional_annotation(
         true, context, schema_context, dynamic_context, std::move(filter),
         std::move(children))};
   } else {
+    if (children.size() == 1) {
+      // Optimize `additionalProperties` set to just `type`, which is a
+      // pretty common pattern
+      if (std::holds_alternative<SchemaCompilerAssertionTypeStrict>(
+              children.front())) {
+        const auto &type_step{
+            std::get<SchemaCompilerAssertionTypeStrict>(children.front())};
+        return {make<SchemaCompilerLoopPropertiesTypeStrict>(
+            true, context, schema_context, dynamic_context, type_step.value)};
+      } else if (std::holds_alternative<SchemaCompilerAssertionType>(
+                     children.front())) {
+        const auto &type_step{
+            std::get<SchemaCompilerAssertionType>(children.front())};
+        return {make<SchemaCompilerLoopPropertiesType>(
+            true, context, schema_context, dynamic_context, type_step.value)};
+      }
+    }
+
     return {make<SchemaCompilerLoopProperties>(
         true, context, schema_context, dynamic_context,
         SchemaCompilerValueNone{}, std::move(children))};

diff --git a/src/jsonschema/include/sourcemeta/jsontoolkit/jsonschema_compile.h b/src/jsonschema/include/sourcemeta/jsontoolkit/jsonschema_compile.h
@@ -171,6 +171,8 @@ struct SchemaCompilerLoopProperties;
 struct SchemaCompilerLoopPropertiesRegex;
 struct SchemaCompilerLoopPropertiesNoAnnotation;
 struct SchemaCompilerLoopPropertiesExcept;
+struct SchemaCompilerLoopPropertiesType;
+struct SchemaCompilerLoopPropertiesTypeStrict;
 struct SchemaCompilerLoopKeys;
 struct SchemaCompilerLoopItems;
 struct SchemaCompilerLoopItemsUnmarked;
@@ -214,7 +216,8 @@ using SchemaCompilerTemplate = std::vector<std::variant<
     SchemaCompilerLogicalWhenArraySizeEqual, SchemaCompilerLoopPropertiesMatch,
     SchemaCompilerLoopProperties, SchemaCompilerLoopPropertiesRegex,
     SchemaCompilerLoopPropertiesNoAnnotation,
-    SchemaCompilerLoopPropertiesExcept, SchemaCompilerLoopKeys,
+    SchemaCompilerLoopPropertiesExcept, SchemaCompilerLoopPropertiesType,
+    SchemaCompilerLoopPropertiesTypeStrict, SchemaCompilerLoopKeys,
     SchemaCompilerLoopItems, SchemaCompilerLoopItemsUnmarked,
     SchemaCompilerLoopItemsUnevaluated, SchemaCompilerLoopContains,
     SchemaCompilerControlLabel, SchemaCompilerControlMark,
@@ -490,6 +493,16 @@ DEFINE_STEP_APPLICATOR(Loop, PropertiesNoAnnotation, SchemaCompilerValueStrings)
 DEFINE_STEP_APPLICATOR(Loop, PropertiesExcept,
                        SchemaCompilerValuePropertyFilter)
 
+/// @ingroup jsonschema_compiler_instructions
+/// @brief Represents a compiler step that checks every object property is of a
+/// given type
+DEFINE_STEP_WITH_VALUE(Loop, PropertiesType, SchemaCompilerValueType)
+
+/// @ingroup jsonschema_compiler_instructions
+/// @brief Represents a compiler step that checks every object property is of a
+/// given type (strict mode)
+DEFINE_STEP_WITH_VALUE(Loop, PropertiesTypeStrict, SchemaCompilerValueType)
+
 /// @ingroup jsonschema_compiler_instructions
 /// @brief Represents a compiler step that loops over object property keys
 DEFINE_STEP_APPLICATOR(Loop, Keys, SchemaCompilerValueNone)

diff --git a/test/jsonschema/jsonschema_compile_draft4_test.cc b/test/jsonschema/jsonschema_compile_draft4_test.cc
@@ -834,26 +834,15 @@ TEST(JSONSchema_compile_draft4, ref_9) {
   const sourcemeta::jsontoolkit::JSON instance{
       sourcemeta::jsontoolkit::parse("{ \"foo\": true }")};
 
-  EVALUATE_WITH_TRACE_FAST_SUCCESS(compiled_schema, instance, 2);
+  EVALUATE_WITH_TRACE_FAST_SUCCESS(compiled_schema, instance, 1);
 
-  EVALUATE_TRACE_PRE(0, LoopProperties, "/additionalProperties",
+  EVALUATE_TRACE_PRE(0, LoopPropertiesTypeStrict, "/additionalProperties",
                      "#/additionalProperties", "");
-  EVALUATE_TRACE_PRE(1, AssertionTypeStrict,
-                     "/additionalProperties/$ref/$ref/type",
-                     "#/definitions/two/type", "/foo");
-
-  EVALUATE_TRACE_POST_SUCCESS(0, AssertionTypeStrict,
-                              "/additionalProperties/$ref/$ref/type",
-                              "#/definitions/two/type", "/foo");
-  EVALUATE_TRACE_POST_SUCCESS(1, LoopProperties, "/additionalProperties",
-                              "#/additionalProperties", "");
-
-  EVALUATE_TRACE_POST_DESCRIBE(instance, 0,
-                               "The value was expected to be of type boolean");
+  EVALUATE_TRACE_POST_SUCCESS(0, LoopPropertiesTypeStrict,
+                              "/additionalProperties", "#/additionalProperties",
+                              "");
   EVALUATE_TRACE_POST_DESCRIBE(
-      instance, 1,
-      "The object properties not covered by other adjacent object keywords "
-      "were expected to validate against this subschema");
+      instance, 0, "The object properties were expected to be of type boolean");
 }
 
 TEST(JSONSchema_compile_draft4, ref_10) {
@@ -876,26 +865,15 @@ TEST(JSONSchema_compile_draft4, ref_10) {
   const sourcemeta::jsontoolkit::JSON instance{
       sourcemeta::jsontoolkit::parse("{ \"foo\": true }")};
 
-  EVALUATE_WITH_TRACE_FAST_SUCCESS(compiled_schema, instance, 2);
+  EVALUATE_WITH_TRACE_FAST_SUCCESS(compiled_schema, instance, 1);
 
-  EVALUATE_TRACE_PRE(0, LoopProperties, "/additionalProperties",
+  EVALUATE_TRACE_PRE(0, LoopPropertiesTypeStrict, "/additionalProperties",
                      "#/additionalProperties", "");
-  EVALUATE_TRACE_PRE(1, AssertionTypeStrict,
-                     "/additionalProperties/$ref/$ref/type",
-                     "#/definitions/two/type", "/foo");
-
-  EVALUATE_TRACE_POST_SUCCESS(0, AssertionTypeStrict,
-                              "/additionalProperties/$ref/$ref/type",
-                              "#/definitions/two/type", "/foo");
-  EVALUATE_TRACE_POST_SUCCESS(1, LoopProperties, "/additionalProperties",
-                              "#/additionalProperties", "");
-
-  EVALUATE_TRACE_POST_DESCRIBE(instance, 0,
-                               "The value was expected to be of type boolean");
+  EVALUATE_TRACE_POST_SUCCESS(0, LoopPropertiesTypeStrict,
+                              "/additionalProperties", "#/additionalProperties",
+                              "");
   EVALUATE_TRACE_POST_DESCRIBE(
-      instance, 1,
-      "The object properties not covered by other adjacent object keywords "
-      "were expected to validate against this subschema");
+      instance, 0, "The object properties were expected to be of type boolean");
 }
 
 TEST(JSONSchema_compile_draft4, ref_11) {
@@ -1539,59 +1517,15 @@ TEST(JSONSchema_compile_draft4, additionalProperties_1) {
   const sourcemeta::jsontoolkit::JSON instance{
       sourcemeta::jsontoolkit::parse("{ \"bar\": 2, \"foo\": 1 }")};
 
-  EVALUATE_WITH_TRACE_FAST_SUCCESS(compiled_schema, instance, 3);
-
-  if (FIRST_PROPERTY_IS(instance, "foo")) {
-    EVALUATE_TRACE_PRE(0, LoopProperties, "/additionalProperties",
-                       "#/additionalProperties", "");
-    EVALUATE_TRACE_PRE(1, AssertionTypeStrict, "/additionalProperties/type",
-                       "#/additionalProperties/type", "/foo");
-    EVALUATE_TRACE_PRE(2, AssertionTypeStrict, "/additionalProperties/type",
-                       "#/additionalProperties/type", "/bar");
-
-    EVALUATE_TRACE_POST_SUCCESS(0, AssertionTypeStrict,
-                                "/additionalProperties/type",
-                                "#/additionalProperties/type", "/foo");
-    EVALUATE_TRACE_POST_SUCCESS(1, AssertionTypeStrict,
-                                "/additionalProperties/type",
-                                "#/additionalProperties/type", "/bar");
-    EVALUATE_TRACE_POST_SUCCESS(2, LoopProperties, "/additionalProperties",
-                                "#/additionalProperties", "");
-
-    EVALUATE_TRACE_POST_DESCRIBE(
-        instance, 0, "The value was expected to be of type integer");
-    EVALUATE_TRACE_POST_DESCRIBE(
-        instance, 1, "The value was expected to be of type integer");
-    EVALUATE_TRACE_POST_DESCRIBE(instance, 2,
-                                 "The object properties not covered by other "
-                                 "adjacent object keywords were "
-                                 "expected to validate against this subschema");
-  } else {
-    EVALUATE_TRACE_PRE(0, LoopProperties, "/additionalProperties",
-                       "#/additionalProperties", "");
-    EVALUATE_TRACE_PRE(1, AssertionTypeStrict, "/additionalProperties/type",
-                       "#/additionalProperties/type", "/bar");
-    EVALUATE_TRACE_PRE(2, AssertionTypeStrict, "/additionalProperties/type",
-                       "#/additionalProperties/type", "/foo");
-
-    EVALUATE_TRACE_POST_SUCCESS(0, AssertionTypeStrict,
-                                "/additionalProperties/type",
-                                "#/additionalProperties/type", "/bar");
-    EVALUATE_TRACE_POST_SUCCESS(1, AssertionTypeStrict,
-                                "/additionalProperties/type",
-                                "#/additionalProperties/type", "/foo");
-    EVALUATE_TRACE_POST_SUCCESS(2, LoopProperties, "/additionalProperties",
-                                "#/additionalProperties", "");
+  EVALUATE_WITH_TRACE_FAST_SUCCESS(compiled_schema, instance, 1);
 
-    EVALUATE_TRACE_POST_DESCRIBE(
-        instance, 0, "The value was expected to be of type integer");
-    EVALUATE_TRACE_POST_DESCRIBE(
-        instance, 1, "The value was expected to be of type integer");
-    EVALUATE_TRACE_POST_DESCRIBE(instance, 2,
-                                 "The object properties not covered by other "
-                                 "adjacent object keywords were "
-                                 "expected to validate against this subschema");
-  }
+  EVALUATE_TRACE_PRE(0, LoopPropertiesTypeStrict, "/additionalProperties",
+                     "#/additionalProperties", "");
+  EVALUATE_TRACE_POST_SUCCESS(0, LoopPropertiesTypeStrict,
+                              "/additionalProperties", "#/additionalProperties",
+                              "");
+  EVALUATE_TRACE_POST_DESCRIBE(
+      instance, 0, "The object properties were expected to be of type integer");
 }
 
 TEST(JSONSchema_compile_draft4, additionalProperties_2) {