Merge branch 'master' into feat/format-matcher

Makefile

@@ -33,21 +33,38 @@ EXCLUDED_TESTS := content *ecmascript_regex zeroTerminatedFloats non_bmp_regex
 EXCLUDED_TESTS := $(shell printf ":*optional_%s" $(EXCLUDED_TESTS) | cut -c2-):$(EXCLUDED_FORMAT_TESTS)
 EXCLUDED_TEST_CASES = "*leap second*"
 
-all: .build/bin/validate
+.PHONY: all
+all: validate
 all: run-test
 
+.PHONY: debug
 debug: CXX_FLAGS := $(CXX_FLAGS) -g -fsanitize=address
 debug: LD_FLAGS := $(LD_FLAGS) -fsanitize=address
 debug: test
 
+.PHONY: clean
 clean:
 	@ rm -rf .build
 
+.PHONY: test
 test: $(TEST_BINARIES)
 
+.PHONY: run-test
 run-test: test
 run-test: $(EXECUTE_TESTS)
 
+.PHONY: validate
+validate: .build/bin/validate
+
+.PHONY: annotation_test
+annotation_test: .build/bin/annotation_test.done
+
+.PHONY: extension_test
+extension_test: .build/bin/extension_test.done
+
+.PHONY: selfvalidate_test
+selfvalidate_test: .build/bin/selfvalidate_test.done
+
 # Actual Definitions (non-phony)
 .build/tests/%.o: tests/%.cxx $(HEADERS) $(TEST_HEADERS)
 	@ mkdir -p .build/tests

include/jvalidate/validation_result.h

@@ -301,6 +301,17 @@ private:
     results_[where][schema_path].annotations.emplace(name, std::move(message));
   }
 
+  /**
+   * @brief Remove annotations that we added without knowing if they were
+   * needed, now that we know that they are not required.
+   *
+   * @param where A path into the document being validated
+   * @param schema_path The schema path to remove
+   */
+  void unannotate(detail::Pointer const & where, detail::Pointer const & schema_path) {
+    results_[where].erase(schema_path);
+  }
+
   static void sanitize(std::string & message) {
     for (auto it = message.begin(); it != message.end(); ++it) {
       if (*it == '"' || *it == '\\') {

include/jvalidate/validation_visitor.h

@@ -190,10 +190,20 @@ public:
       }
     }
 
-    if (matches.size() == 1) {
-      return result(Status::Accept, "validates subschema ", *matches.begin());
+    if (matches.empty()) {
+      return result(Status::Reject, "validates no subschemas");
     }
-    return result(Status::Reject, "validates multiple subschemas ", matches);
+    if (matches.size() > 1) {
+      return result(Status::Reject, "validates multiple subschemas ", matches);
+    }
+
+    size_t const match = *matches.begin();
+    for (size_t i = 0; result_ and i < cons.children.size(); ++i) {
+      if (i != match) {
+        result_->unannotate(where_, schema_path_ / i);
+      }
+    }
+    return result(Status::Accept, "validates subschema ", match);
   }
 
   Status visit(constraint::NotConstraint const & cons, Adapter auto const & document) const {
@@ -837,7 +847,7 @@ private:
     }
     // Update the annotation/error content only if a failure is being reported,
     // or if we are in an "if" schema.
-    if ((rval == Status::Reject or tracking_ == StoreResults::ForAnything) and result_) {
+    if (should_annotate(rval)) {
       result_->merge(std::move(result));
     }
     return rval;

tests/annotation_test.cxx

@@ -1,5 +1,3 @@
-#include <string_view>
-
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
@@ -75,6 +73,24 @@ TEST(Annotation, NotSchemaFlipsAnnotationRule) {
   EXPECT_THAT(result, ErrorAt(""_jptr, "/not"_jptr, "minimum", "6 >= 5"));
 }
 
+TEST(Annotation, NotSchemaPropogatesDeeply) {
+  auto const schema = R"({
+    "not": {
+      "properties": {
+        "A": {
+          "enum": [ 1, 3, 4 ]
+        }
+      }
+    }
+  })"_json;
+
+  auto const instance = R"({"A": 3})"_json;
+
+  jvalidate::ValidationResult result = validate(schema, instance);
+
+  EXPECT_THAT(result, ErrorAt("/A"_jptr, "/not/properties/A"_jptr, "enum", "1"));
+}
+
 TEST(Annotation, PathFollowsSchemaNotConstraintModel) {
   auto const schema = R"({
     "$comment": "disallow is implemented in the form of NotConstraint[TypeConstraint]",
@@ -88,36 +104,142 @@ TEST(Annotation, PathFollowsSchemaNotConstraintModel) {
   EXPECT_THAT(result, ErrorAt("disallow", "string is in types [string]"));
 }
 
-TEST(Annotation, SomeConstraintsAnnotateBothValidAndInvalid) {
+TEST(Annotation, AttachesAlwaysFalseSensibly) {
   auto const schema = R"({
-    "$comment": "accepts any number <= 0 or >= 10",
-    "oneOf": [
-      { "minimum": 10 },
-      { "maximum": 0 }
-    ]
+    "properties": {
+      "A": false
+    }
   })"_json;
 
-  auto const instance = R"(-1)"_json;
+  auto const instance = R"({
+    "A": null
+  })"_json;
   jvalidate::ValidationResult result = validate(schema, instance);
 
-  EXPECT_THAT(result, Not(HasAnnotationAt(""_jptr, "/oneOf"_jptr)));
-  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/0"_jptr, "minimum", "-1 < 10"));
-  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/1"_jptr, "maximum", "-1 <= 0"));
+  EXPECT_THAT(result, ErrorAt("/A"_jptr, "/properties/A"_jptr, "", "always false"));
 }
 
-TEST(Annotation, AttachesAlwaysFalseSensibly) {
+TEST(Annotation, IfThenCanGiveABecauseReason) {
   auto const schema = R"({
-    "properties": {
-      "A": false
+    "if": {
+      "properties": {
+        "A": { "const": true }
+      },
+      "required": [ "A" ]
+    },
+    "then": {
+      "properties": {
+        "B": { "multipleOf": 3 }
+      }
+    },
+    "else": {
+      "properties": {
+        "B": { "multipleOf": 5 }
+      }
     }
   })"_json;
 
   auto const instance = R"({
-    "A": null
+    "A": true,
+    "B": 4
   })"_json;
+
   jvalidate::ValidationResult result = validate(schema, instance);
 
-  EXPECT_THAT(result, ErrorAt("/A"_jptr, "/properties/A"_jptr, "", "always false"));
+  EXPECT_THAT(result,
+              ErrorAt(""_jptr, "/if"_jptr, "required", "contains all required properties A"));
+  EXPECT_THAT(result, ErrorAt("/A"_jptr, "/if/properties/A"_jptr, "const", "matches value"));
+  EXPECT_THAT(result, ErrorAt("/B"_jptr, "/then/properties/B"_jptr, "multipleOf",
+                              "4 is not a multiple of 3"));
+}
+
+TEST(Annotation, IfElseCanGiveABecauseReason) {
+  auto const schema = R"({
+    "if": {
+      "properties": {
+        "A": { "const": true }
+      },
+      "required": [ "A" ]
+    },
+    "then": {
+      "properties": {
+        "B": { "multipleOf": 3 }
+      }
+    },
+    "else": {
+      "properties": {
+        "B": { "multipleOf": 5 }
+      }
+    }
+  })"_json;
+
+  auto const instance = R"({
+    "A": false,
+    "B": 4
+  })"_json;
+
+  jvalidate::ValidationResult result = validate(schema, instance);
+
+  EXPECT_THAT(result,
+              ErrorAt(""_jptr, "/if"_jptr, "required", "contains all required properties A"));
+  EXPECT_THAT(result, ErrorAt("/A"_jptr, "/if/properties/A"_jptr, "const", "true was expected"));
+  EXPECT_THAT(result, ErrorAt("/B"_jptr, "/else/properties/B"_jptr, "multipleOf",
+                              "4 is not a multiple of 5"));
+}
+
+TEST(Annotation, OneOfGivesAllReasonsOnTooMany) {
+  auto const schema = R"({
+    "oneOf": [
+      { "minimum": 5 },
+      { "multipleOf": 3 },
+      { "maximum": 2 }
+    ]
+  })"_json;
+
+  auto const instance = R"(6)"_json;
+
+  jvalidate::ValidationResult result = validate(schema, instance);
+
+  EXPECT_THAT(result, ErrorAt(""_jptr, ""_jptr, "oneOf", "validates multiple subschemas [ 0, 1 ]"));
+  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/0"_jptr, "minimum", "6 >= 5"));
+  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/1"_jptr, "multipleOf", "6 is a multiple of 3"));
+  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/2"_jptr, "maximum", "6 > 2"));
+}
+
+TEST(Annotation, OneOfGivesAllReasonsOnNoMatches) {
+  auto const schema = R"({
+    "oneOf": [
+      { "minimum": 5 },
+      { "multipleOf": 3 },
+      { "maximum": 2 }
+    ]
+  })"_json;
+
+  auto const instance = R"(4)"_json;
+
+  jvalidate::ValidationResult result = validate(schema, instance);
+
+  EXPECT_THAT(result, ErrorAt(""_jptr, ""_jptr, "oneOf", "validates no subschemas"));
+  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/0"_jptr, "minimum", "4 < 5"));
+  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/1"_jptr, "multipleOf", "4 is not a multiple of 3"));
+  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/2"_jptr, "maximum", "4 > 2"));
+}
+
+TEST(Annotation, OneOfGivesAllReasonsOnMatch) {
+  auto const schema = R"({
+    "oneOf": [
+      { "minimum": 5 },
+      { "multipleOf": 3 },
+      { "maximum": 2 }
+    ]
+  })"_json;
+
+  auto const instance = R"(3)"_json;
+
+  jvalidate::ValidationResult result = validate(schema, instance);
+
+  EXPECT_THAT(result, Not(HasAnnotationAt(""_jptr, ""_jptr)));
+  EXPECT_THAT(result, ErrorAt(""_jptr, "/oneOf/1"_jptr, "multipleOf", "3 is a multiple of 3"));
 }
 
 int main(int argc, char ** argv) {

tests/matchers.h

@@ -1,6 +1,8 @@
+#include "gtest/internal/gtest-internal.h"
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include <ios>
 #include <jvalidate/detail/pointer.h>
 #include <jvalidate/validation_result.h>
 
@@ -31,34 +33,40 @@ MATCHER_P(HasAnnotationsFor, doc_path, "") { return arg.has(doc_path); }
 
 MATCHER_P2(HasAnnotationAt, doc_path, schema_path, "") { return arg.has(doc_path, schema_path); }
 
-MATCHER_P2(AnnotationAt, key, matcher, "") {
-  auto const * anno = arg.annotation({}, {}, key);
-  if (not anno) {
-    return false;
-  }
-  return testing::ExplainMatchResult(matcher, *anno, result_listener);
-}
-
 MATCHER_P4(AnnotationAt, doc_path, schema_path, key, matcher, "") {
   auto const * anno = arg.annotation(doc_path, schema_path, key);
+  *result_listener << "\ninstanceLocation: " << doc_path << " is "
+                   << (arg.has(doc_path) ? "present" : "missing");
+  *result_listener << "\nevaluationLocation: " << schema_path << " is "
+                   << (arg.has(doc_path, schema_path) ? "present" : "missing");
   if (not anno) {
+    *result_listener << "\nannotation: (nil)";
     return false;
   }
+  *result_listener << "\nannotation: " << testing::PrintToString(*anno);
   return testing::ExplainMatchResult(matcher, *anno, result_listener);
 }
 
-MATCHER_P2(ErrorAt, key, matcher, "") {
-  auto const * anno = arg.error({}, {}, key);
-  if (not anno) {
-    return false;
-  }
-  return testing::ExplainMatchResult(matcher, *anno, result_listener);
+template <typename M> auto AnnotationAt(std::string const & key, M && matcher) {
+  return AnnotationAt(jvalidate::detail::Pointer(), jvalidate::detail::Pointer(), key,
+                      std::forward<M>(matcher));
 }
 
 MATCHER_P4(ErrorAt, doc_path, schema_path, key, matcher, "") {
   auto const * anno = arg.error(doc_path, schema_path, key);
+  *result_listener << "\ninstanceLocation: " << doc_path << " is "
+                   << (arg.has(doc_path) ? "present" : "missing");
+  *result_listener << "\nevaluationLocation: " << schema_path << " is "
+                   << (arg.has(doc_path, schema_path) ? "present" : "missing");
   if (not anno) {
+    *result_listener << "\nannotation: (nil)";
     return false;
   }
+  *result_listener << "\nannotation: " << testing::PrintToString(*anno);
   return testing::ExplainMatchResult(matcher, *anno, result_listener);
 }
+
+template <typename M> auto ErrorAt(std::string const & key, M && matcher) {
+  return ErrorAt(jvalidate::detail::Pointer(), jvalidate::detail::Pointer(), key,
+                 std::forward<M>(matcher));
+}