Don't emit type: object if the schema also defines required (#230)

Signed-off-by: Juan Cruz Viotti <[email protected]>

src/compiler/compile_describe.cc

@@ -862,6 +862,14 @@ struct DescribeVisitor {
     return message.str();
   }
 
+  auto operator()(const AssertionDefinesStrict &step) const -> std::string {
+    std::ostringstream message;
+    message
+        << "The value was expected to be an object that defines the property "
+        << escape_string(step_value(step));
+    return message.str();
+  }
+
   auto operator()(const AssertionDefinesAll &step) const -> std::string {
     const auto &value{step_value(step)};
     assert(value.size() > 1);
@@ -906,6 +914,23 @@ struct DescribeVisitor {
     return message.str();
   }
 
+  auto operator()(const AssertionDefinesAllStrict &step) const -> std::string {
+    const auto &value{step_value(step)};
+    assert(value.size() > 1);
+    std::ostringstream message;
+    message
+        << "The value was expected to be an object that defines properties ";
+    for (auto iterator = value.cbegin(); iterator != value.cend(); ++iterator) {
+      if (std::next(iterator) == value.cend()) {
+        message << "and " << escape_string(*iterator);
+      } else {
+        message << escape_string(*iterator) << ", ";
+      }
+    }
+
+    return message.str();
+  }
+
   auto operator()(const AssertionDefinesExactly &step) const -> std::string {
     const auto &value{step_value(step)};
     assert(value.size() > 1);
@@ -922,6 +947,24 @@ struct DescribeVisitor {
     return message.str();
   }
 
+  auto operator()(const AssertionDefinesExactlyStrict &step) const
+      -> std::string {
+    const auto &value{step_value(step)};
+    assert(value.size() > 1);
+    std::ostringstream message;
+    message << "The value was expected to be an object that only defines "
+               "properties ";
+    for (auto iterator = value.cbegin(); iterator != value.cend(); ++iterator) {
+      if (std::next(iterator) == value.cend()) {
+        message << "and " << escape_string(*iterator);
+      } else {
+        message << escape_string(*iterator) << ", ";
+      }
+    }
+
+    return message.str();
+  }
+
   auto operator()(const AssertionType &step) const -> std::string {
     std::ostringstream message;
     describe_type_check(this->valid, this->target.type(), step_value(step),

src/compiler/compile_json.cc

@@ -222,8 +222,12 @@ struct StepVisitor {
 
   HANDLE_STEP("assertion", "fail", AssertionFail)
   HANDLE_STEP("assertion", "defines", AssertionDefines)
+  HANDLE_STEP("assertion", "defines-strict", AssertionDefinesStrict)
   HANDLE_STEP("assertion", "defines-all", AssertionDefinesAll)
+  HANDLE_STEP("assertion", "defines-all-strict", AssertionDefinesAllStrict)
   HANDLE_STEP("assertion", "defines-exactly", AssertionDefinesExactly)
+  HANDLE_STEP("assertion", "defines-exactly-strict",
+              AssertionDefinesExactlyStrict)
   HANDLE_STEP("assertion", "property-dependencies",
               AssertionPropertyDependencies)
   HANDLE_STEP("assertion", "type", AssertionType)

src/compiler/default_compiler_draft4.h

@@ -238,6 +238,11 @@ auto compiler_draft4_validation_type(const Context &context,
         return {};
       }
 
+      if (context.mode == Mode::FastValidation &&
+          schema_context.schema.defines("required")) {
+        return {};
+      }
+
       return {make<AssertionTypeStrict>(
           context, schema_context, dynamic_context,
           sourcemeta::jsontoolkit::JSON::Type::Object)};
@@ -417,6 +422,11 @@ auto compiler_draft4_validation_required(const Context &context,
     return {};
   }
 
+  const auto assume_object{schema_context.schema.defines("type") &&
+                           schema_context.schema.at("type").is_string() &&
+                           schema_context.schema.at("type").to_string() ==
+                               "object"};
+
   if (schema_context.schema.at(dynamic_context.keyword).empty()) {
     return {};
   } else if (schema_context.schema.at(dynamic_context.keyword).size() > 1) {
@@ -428,8 +438,14 @@ auto compiler_draft4_validation_required(const Context &context,
     }
 
     if (properties.size() == 1) {
-      return {make<AssertionDefines>(context, schema_context, dynamic_context,
-                                     ValueString{*(properties.cbegin())})};
+      if (context.mode == Mode::FastValidation && assume_object) {
+        return {make<AssertionDefinesStrict>(
+            context, schema_context, dynamic_context,
+            ValueString{*(properties.cbegin())})};
+      } else {
+        return {make<AssertionDefines>(context, schema_context, dynamic_context,
+                                       ValueString{*(properties.cbegin())})};
+      }
     } else if (schema_context.schema.defines("additionalProperties") &&
                schema_context.schema.at("additionalProperties").is_boolean() &&
                !schema_context.schema.at("additionalProperties").to_boolean() &&
@@ -442,12 +458,28 @@ auto compiler_draft4_validation_required(const Context &context,
                              return schema_context.schema.at("properties")
                                  .defines(property);
                            })) {
-      return {make<AssertionDefinesExactly>(
+      if (context.mode == Mode::FastValidation && assume_object) {
+        return {make<AssertionDefinesExactlyStrict>(
+            context, schema_context, dynamic_context, std::move(properties))};
+      } else {
+        return {make<AssertionDefinesExactly>(
+            context, schema_context, dynamic_context, std::move(properties))};
+      }
+    } else if (context.mode == Mode::FastValidation && assume_object) {
+      return {make<AssertionDefinesAllStrict>(
           context, schema_context, dynamic_context, std::move(properties))};
     } else {
       return {make<AssertionDefinesAll>(
           context, schema_context, dynamic_context, std::move(properties))};
     }
+  } else if (context.mode == Mode::FastValidation && assume_object) {
+    assert(
+        schema_context.schema.at(dynamic_context.keyword).front().is_string());
+    return {make<AssertionDefinesStrict>(
+        context, schema_context, dynamic_context,
+        ValueString{schema_context.schema.at(dynamic_context.keyword)
+                        .front()
+                        .to_string()})};
   } else {
     assert(
         schema_context.schema.at(dynamic_context.keyword).front().is_string());

src/compiler/default_compiler_draft6.h

@@ -88,6 +88,11 @@ auto compiler_draft6_validation_type(const Context &context,
         return {};
       }
 
+      if (context.mode == Mode::FastValidation &&
+          schema_context.schema.defines("required")) {
+        return {};
+      }
+
       return {make<AssertionTypeStrict>(
           context, schema_context, dynamic_context,
           sourcemeta::jsontoolkit::JSON::Type::Object)};

src/evaluator/dispatch.inc.h

@@ -26,6 +26,13 @@ switch (static_cast<InstructionIndex>(instruction.index())) {
     EVALUATE_END(assertion, AssertionDefines);
   }
 
+  case IS_INSTRUCTION(AssertionDefinesStrict): {
+    EVALUATE_BEGIN_NO_PRECONDITION(assertion, AssertionDefinesStrict);
+    const auto &target{get(instance, assertion.relative_instance_location)};
+    result = target.is_object() && target.defines(assertion.value);
+    EVALUATE_END(assertion, AssertionDefinesStrict);
+  }
+
   case IS_INSTRUCTION(AssertionDefinesAll): {
     EVALUATE_BEGIN_NON_STRING(assertion, AssertionDefinesAll,
                               target.is_object());
@@ -47,6 +54,27 @@ switch (static_cast<InstructionIndex>(instruction.index())) {
     EVALUATE_END(assertion, AssertionDefinesAll);
   }
 
+  case IS_INSTRUCTION(AssertionDefinesAllStrict): {
+    EVALUATE_BEGIN_NO_PRECONDITION(assertion, AssertionDefinesAllStrict);
+    const auto &target{get(instance, assertion.relative_instance_location)};
+
+    // Otherwise we are we even emitting this instruction?
+    assert(assertion.value.size() > 1);
+
+    // Otherwise there is no way the instance can satisfy it anyway
+    if (target.is_object() && assertion.value.size() <= target.object_size()) {
+      result = true;
+      for (const auto &property : assertion.value) {
+        if (!target.defines(property)) {
+          result = false;
+          break;
+        }
+      }
+    }
+
+    EVALUATE_END(assertion, AssertionDefinesAllStrict);
+  }
+
   case IS_INSTRUCTION(AssertionDefinesExactly): {
     EVALUATE_BEGIN_NON_STRING(assertion, AssertionDefinesExactly,
                               target.is_object());
@@ -67,6 +95,26 @@ switch (static_cast<InstructionIndex>(instruction.index())) {
     EVALUATE_END(assertion, AssertionDefinesExactly);
   }
 
+  case IS_INSTRUCTION(AssertionDefinesExactlyStrict): {
+    EVALUATE_BEGIN_NO_PRECONDITION(assertion, AssertionDefinesExactlyStrict);
+    const auto &target{get(instance, assertion.relative_instance_location)};
+
+    // Otherwise we are we even emitting this instruction?
+    assert(assertion.value.size() > 1);
+
+    if (target.is_object() && assertion.value.size() == target.object_size()) {
+      result = true;
+      for (const auto &property : assertion.value) {
+        if (!target.defines(property)) {
+          result = false;
+          break;
+        }
+      }
+    }
+
+    EVALUATE_END(assertion, AssertionDefinesExactlyStrict);
+  }
+
   case IS_INSTRUCTION(AssertionPropertyDependencies): {
     EVALUATE_BEGIN_NON_STRING(assertion, AssertionPropertyDependencies,
                               target.is_object());

src/evaluator/include/sourcemeta/blaze/evaluator_instruction.h

@@ -16,8 +16,11 @@ namespace sourcemeta::blaze {
 #ifndef DOXYGEN
 struct AssertionFail;
 struct AssertionDefines;
+struct AssertionDefinesStrict;
 struct AssertionDefinesAll;
+struct AssertionDefinesAllStrict;
 struct AssertionDefinesExactly;
+struct AssertionDefinesExactlyStrict;
 struct AssertionPropertyDependencies;
 struct AssertionType;
 struct AssertionTypeAny;
@@ -103,8 +106,9 @@ struct ControlDynamicAnchorJump;
 /// @ingroup evaluator
 /// Represents a schema compilation step that can be evaluated
 using Instruction = std::variant<
-    AssertionFail, AssertionDefines, AssertionDefinesAll,
-    AssertionDefinesExactly, AssertionPropertyDependencies, AssertionType,
+    AssertionFail, AssertionDefines, AssertionDefinesStrict,
+    AssertionDefinesAll, AssertionDefinesAllStrict, AssertionDefinesExactly,
+    AssertionDefinesExactlyStrict, AssertionPropertyDependencies, AssertionType,
     AssertionTypeAny, AssertionTypeStrict, AssertionTypeStrictAny,
     AssertionTypeStringBounded, AssertionTypeStringUpper,
     AssertionTypeArrayBounded, AssertionTypeArrayUpper,
@@ -139,8 +143,11 @@ using Instruction = std::variant<
 enum class InstructionIndex : std::uint8_t {
   AssertionFail = 0,
   AssertionDefines,
+  AssertionDefinesStrict,
   AssertionDefinesAll,
+  AssertionDefinesAllStrict,
   AssertionDefinesExactly,
+  AssertionDefinesExactlyStrict,
   AssertionPropertyDependencies,
   AssertionType,
   AssertionTypeAny,
@@ -264,16 +271,31 @@ DEFINE_STEP_WITH_VALUE(Assertion, Fail, ValueNone)
 /// a given property
 DEFINE_STEP_WITH_VALUE(Assertion, Defines, ValueString)
 
+/// @ingroup evaluator_instructions
+/// @brief Represents a compiler assertion step that checks that the instance is
+/// an object and that it defines a given property
+DEFINE_STEP_WITH_VALUE(Assertion, DefinesStrict, ValueString)
+
 /// @ingroup evaluator_instructions
 /// @brief Represents a compiler assertion step that checks if an object defines
 /// a set of properties
 DEFINE_STEP_WITH_VALUE(Assertion, DefinesAll, ValueStrings)
 
+/// @ingroup evaluator_instructions
+/// @brief Represents a compiler assertion step that checks that the instance is
+/// an object and that it defines a set of properties
+DEFINE_STEP_WITH_VALUE(Assertion, DefinesAllStrict, ValueStrings)
+
 /// @ingroup evaluator_instructions
 /// @brief Represents a compiler assertion step that checks if an object defines
 /// a set of properties and no other ones
 DEFINE_STEP_WITH_VALUE(Assertion, DefinesExactly, ValueStrings)
 
+/// @ingroup evaluator_instructions
+/// @brief Represents a compiler assertion step that checks if the instance is
+/// an object and defines a set of properties and no other ones
+DEFINE_STEP_WITH_VALUE(Assertion, DefinesExactlyStrict, ValueStrings)
+
 /// @ingroup evaluator_instructions
 /// @brief Represents a compiler assertion step that checks if an object defines
 /// a set of properties if it defines other set of properties