Fix case where additionalProperties: false was not taking effect (#229

)

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

src/compiler/compile_describe.cc

@@ -906,6 +906,22 @@ struct DescribeVisitor {
     return message.str();
   }
 
+  auto operator()(const AssertionDefinesExactly &step) const -> std::string {
+    const auto &value{step_value(step)};
+    assert(value.size() > 1);
+    std::ostringstream message;
+    message << "The object value was expected to only define 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

@@ -223,6 +223,7 @@ struct StepVisitor {
   HANDLE_STEP("assertion", "fail", AssertionFail)
   HANDLE_STEP("assertion", "defines", AssertionDefines)
   HANDLE_STEP("assertion", "defines-all", AssertionDefinesAll)
+  HANDLE_STEP("assertion", "defines-exactly", AssertionDefinesExactly)
   HANDLE_STEP("assertion", "property-dependencies",
               AssertionPropertyDependencies)
   HANDLE_STEP("assertion", "type", AssertionType)

src/compiler/default_compiler_draft4.h

@@ -430,6 +430,20 @@ auto compiler_draft4_validation_required(const Context &context,
     if (properties.size() == 1) {
       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() &&
+               schema_context.schema.defines("properties") &&
+               schema_context.schema.at("properties").is_object() &&
+               schema_context.schema.at("properties").size() ==
+                   properties.size() &&
+               std::all_of(properties.cbegin(), properties.cend(),
+                           [&schema_context](const auto &property) {
+                             return schema_context.schema.at("properties")
+                                 .defines(property);
+                           })) {
+      return {make<AssertionDefinesExactly>(
+          context, schema_context, dynamic_context, std::move(properties))};
     } else {
       return {make<AssertionDefinesAll>(
           context, schema_context, dynamic_context, std::move(properties))};

src/evaluator/dispatch.inc.h

@@ -47,6 +47,26 @@ switch (static_cast<InstructionIndex>(instruction.index())) {
     EVALUATE_END(assertion, AssertionDefinesAll);
   }
 
+  case IS_INSTRUCTION(AssertionDefinesExactly): {
+    EVALUATE_BEGIN_NON_STRING(assertion, AssertionDefinesExactly,
+                              target.is_object());
+
+    // Otherwise we are we even emitting this instruction?
+    assert(assertion.value.size() > 1);
+
+    if (assertion.value.size() == target.object_size()) {
+      result = true;
+      for (const auto &property : assertion.value) {
+        if (!target.defines(property)) {
+          result = false;
+          break;
+        }
+      }
+    }
+
+    EVALUATE_END(assertion, AssertionDefinesExactly);
+  }
+
   case IS_INSTRUCTION(AssertionPropertyDependencies): {
     EVALUATE_BEGIN_NON_STRING(assertion, AssertionPropertyDependencies,
                               target.is_object());

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

@@ -17,6 +17,7 @@ namespace sourcemeta::blaze {
 struct AssertionFail;
 struct AssertionDefines;
 struct AssertionDefinesAll;
+struct AssertionDefinesExactly;
 struct AssertionPropertyDependencies;
 struct AssertionType;
 struct AssertionTypeAny;
@@ -103,12 +104,12 @@ struct ControlDynamicAnchorJump;
 /// Represents a schema compilation step that can be evaluated
 using Instruction = std::variant<
     AssertionFail, AssertionDefines, AssertionDefinesAll,
-    AssertionPropertyDependencies, AssertionType, AssertionTypeAny,
-    AssertionTypeStrict, AssertionTypeStrictAny, AssertionTypeStringBounded,
-    AssertionTypeStringUpper, AssertionTypeArrayBounded,
-    AssertionTypeArrayUpper, AssertionTypeObjectBounded,
-    AssertionTypeObjectUpper, AssertionRegex, AssertionStringSizeLess,
-    AssertionStringSizeGreater, AssertionArraySizeLess,
+    AssertionDefinesExactly, AssertionPropertyDependencies, AssertionType,
+    AssertionTypeAny, AssertionTypeStrict, AssertionTypeStrictAny,
+    AssertionTypeStringBounded, AssertionTypeStringUpper,
+    AssertionTypeArrayBounded, AssertionTypeArrayUpper,
+    AssertionTypeObjectBounded, AssertionTypeObjectUpper, AssertionRegex,
+    AssertionStringSizeLess, AssertionStringSizeGreater, AssertionArraySizeLess,
     AssertionArraySizeGreater, AssertionObjectSizeLess,
     AssertionObjectSizeGreater, AssertionEqual, AssertionEqualsAny,
     AssertionGreaterEqual, AssertionLessEqual, AssertionGreater, AssertionLess,
@@ -139,6 +140,7 @@ enum class InstructionIndex : std::uint8_t {
   AssertionFail = 0,
   AssertionDefines,
   AssertionDefinesAll,
+  AssertionDefinesExactly,
   AssertionPropertyDependencies,
   AssertionType,
   AssertionTypeAny,
@@ -267,6 +269,11 @@ DEFINE_STEP_WITH_VALUE(Assertion, Defines, ValueString)
 /// a set of properties
 DEFINE_STEP_WITH_VALUE(Assertion, DefinesAll, 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 an object defines
 /// a set of properties if it defines other set of properties

test/evaluator/evaluator_draft4_test.cc

@@ -2783,14 +2783,14 @@ TEST(Evaluator_draft4, additionalProperties_12) {
 
   EVALUATE_WITH_TRACE_FAST_SUCCESS(schema, instance, 3);
 
-  EVALUATE_TRACE_PRE(0, AssertionDefinesAll, "/required", "#/required", "");
+  EVALUATE_TRACE_PRE(0, AssertionDefinesExactly, "/required", "#/required", "");
   EVALUATE_TRACE_PRE(1, AssertionPropertyTypeStrict, "/properties/bar/type",
                      "#/properties/bar/type", "/bar");
   EVALUATE_TRACE_PRE(2, AssertionPropertyTypeStrict, "/properties/foo/type",
                      "#/properties/foo/type", "/foo");
 
-  EVALUATE_TRACE_POST_SUCCESS(0, AssertionDefinesAll, "/required", "#/required",
-                              "");
+  EVALUATE_TRACE_POST_SUCCESS(0, AssertionDefinesExactly, "/required",
+                              "#/required", "");
   EVALUATE_TRACE_POST_SUCCESS(1, AssertionPropertyTypeStrict,
                               "/properties/bar/type", "#/properties/bar/type",
                               "/bar");
@@ -2799,14 +2799,39 @@ TEST(Evaluator_draft4, additionalProperties_12) {
                               "/foo");
 
   EVALUATE_TRACE_POST_DESCRIBE(instance, 0,
-                               "The object value was expected to define "
+                               "The object value was expected to only define "
                                "properties \"foo\", and \"bar\"");
   EVALUATE_TRACE_POST_DESCRIBE(instance, 1,
                                "The value was expected to be of type boolean");
   EVALUATE_TRACE_POST_DESCRIBE(instance, 2,
                                "The value was expected to be of type boolean");
 }
 
+TEST(Evaluator_draft4, additionalProperties_13) {
+  const sourcemeta::jsontoolkit::JSON schema{
+      sourcemeta::jsontoolkit::parse(R"JSON({
+    "$schema": "http://json-schema.org/draft-04/schema#",
+    "required": [ "foo", "bar" ],
+    "additionalProperties": false,
+    "properties": {
+      "foo": { "type": "boolean" },
+      "bar": { "type": "boolean" }
+    }
+  })JSON")};
+
+  const sourcemeta::jsontoolkit::JSON instance{sourcemeta::jsontoolkit::parse(
+      "{ \"foo\": true, \"bar\": false, \"baz\": 1 }")};
+
+  EVALUATE_WITH_TRACE_FAST_FAILURE(schema, instance, 1);
+
+  EVALUATE_TRACE_PRE(0, AssertionDefinesExactly, "/required", "#/required", "");
+  EVALUATE_TRACE_POST_FAILURE(0, AssertionDefinesExactly, "/required",
+                              "#/required", "");
+  EVALUATE_TRACE_POST_DESCRIBE(instance, 0,
+                               "The object value was expected to only define "
+                               "properties \"foo\", and \"bar\"");
+}
+
 TEST(Evaluator_draft4, not_1) {
   const sourcemeta::jsontoolkit::JSON schema{
       sourcemeta::jsontoolkit::parse(R"JSON({