Browse Source

Merge branch 'refactor/results/track-error-and-success'

Sam Jaffe 1 year ago
parent
commit
b8cfca798e

+ 20 - 3
Makefile

@@ -1,3 +1,4 @@
+SHELL=/bin/bash -o pipefail
 INTERACTIVE:=$(shell [ -t 0 ] && echo 1)
 
 ifdef INTERACTIVE
@@ -22,7 +23,8 @@ HEADERS := $(shell find $(INCLUDE_DIR) -name *.h)
 TEST_HEADERS := $(wildcard $(TEST_DIR)*.h)
 TEST_SOURCES := $(wildcard $(TEST_DIR)*.cxx)
 TEST_OBJECTS := $(patsubst %.cxx, .build/%.o, $(TEST_SOURCES))
-TEST_BINARIES := .build/bin/selfvalidate
+TEST_BINARIES := .build/bin/selfvalidate .build/bin/annotation_test
+EXECUTE_TESTS := $(patsubst %, %.done, $(TEST_BINARIES))
 
 EXCLUDED_TESTS := format* content ecmascript_regex zeroTerminatedFloats
 EXCLUDED_TESTS := $(shell printf ":*optional_%s" $(EXCLUDED_TESTS) | cut -c2-)
@@ -39,9 +41,9 @@ clean:
 test: $(TEST_BINARIES)
 
 run-test: test
-run-test:
-	.build/bin/selfvalidate --gtest_filter=-$(EXCLUDED_TESTS) $(CLEAN_ANSI)
+run-test: $(EXECUTE_TESTS)
 
+# Actual Definitions (non-phony)
 .build/tests/%.o: tests/%.cxx $(HEADERS) $(TEST_HEADERS)
 	@ mkdir -p .build/tests
 	$(CXX) $(CXX_FLAGS) -c $< -o $@
@@ -49,4 +51,19 @@ run-test:
 
 .build/bin/selfvalidate: .build/tests/selfvalidate_test.o
 	@ mkdir -p .build/bin
+	@ rm -f $@.done
 	$(CXX) $< -o $@ $(LD_FLAGS) -ljsoncpp -lgmock -lcurl -lgtest
+
+.build/bin/selfvalidate.done: .build/bin/selfvalidate
+	.build/bin/selfvalidate --gtest_filter=-$(EXCLUDED_TESTS) $(CLEAN_ANSI)
+	@ touch $@
+
+
+.build/bin/annotation_test: .build/tests/annotation_test.o
+	@ mkdir -p .build/bin
+	@ rm -f .build/test/annotation_test.done
+	$(CXX) $< -o $@ $(LD_FLAGS) -ljsoncpp -lgmock -lgtest
+
+.build/bin/annotation_test.done: .build/bin/annotation_test
+	.build/bin/annotation_test $(CLEAN_ANSI)
+	@ touch $@

+ 8 - 0
include/jvalidate/detail/pointer.h

@@ -62,6 +62,14 @@ public:
     return document;
   }
 
+  std::string back() const {
+    struct {
+      std::string operator()(std::string const & in) const { return in; }
+      std::string operator()(size_t in) const { return std::to_string(in); }
+    } g_as_str;
+    return tokens_.empty() ? "" : std::visit(g_as_str, tokens_.back());
+  }
+
   bool empty() const { return tokens_.empty(); }
 
   bool starts_with(Pointer const & other) const {

+ 27 - 0
include/jvalidate/detail/scoped_state.h

@@ -0,0 +1,27 @@
+#pragma once
+
+#include <functional>
+#include <type_traits>
+
+#define CONCAT2(A, B) A##B
+#define CONCAT(A, B) CONCAT2(A, B)
+#define scoped_state(prop, value)                                                                  \
+  auto CONCAT(scoped_state_, __LINE__) = detail::ScopedState(prop, value)
+
+namespace jvalidate::detail {
+class ScopedState {
+private:
+  std::function<void()> reset_;
+
+public:
+  template <typename T, typename S>
+  requires(std::is_constructible_v<T, S>) ScopedState(T & prop, S value)
+      : reset_([reset = prop, &prop]() { prop = reset; }) {
+    prop = std::move(value);
+  }
+
+  ~ScopedState() { reset_(); }
+
+  explicit operator bool() const { return true; }
+};
+}

+ 59 - 0
include/jvalidate/detail/tribool.h

@@ -0,0 +1,59 @@
+#pragma once
+
+#include <compare>
+
+#define JVALIDATE_TRIBOOL_TYPE(TypeName, True, False, Maybe)                                       \
+  class TypeName {                                                                                 \
+  public:                                                                                          \
+    enum Enum { True, False, Maybe };                                                              \
+                                                                                                   \
+  private:                                                                                         \
+    Enum state_;                                                                                   \
+                                                                                                   \
+  public:                                                                                          \
+    TypeName(bool state) : state_(state ? True : False) {}                                         \
+    TypeName(Enum state) : state_(state) {}                                                        \
+                                                                                                   \
+    operator Enum() const { return state_; }                                                       \
+    explicit operator bool() const { return state_ != False; }                                     \
+                                                                                                   \
+    friend TypeName operator!(TypeName val) {                                                      \
+      if (val.state_ == Maybe) {                                                                   \
+        return Maybe;                                                                              \
+      }                                                                                            \
+      return val.state_ == False ? True : False;                                                   \
+    }                                                                                              \
+                                                                                                   \
+    friend TypeName operator|(TypeName lhs, TypeName rhs) {                                        \
+      if (lhs.state_ == Maybe && rhs.state_ == Maybe) {                                            \
+        return Maybe;                                                                              \
+      }                                                                                            \
+      if (lhs.state_ == True || rhs.state_ == True) {                                              \
+        return True;                                                                               \
+      }                                                                                            \
+      return False;                                                                                \
+    }                                                                                              \
+                                                                                                   \
+    friend TypeName operator&(TypeName lhs, TypeName rhs) {                                        \
+      if (lhs.state_ == Maybe && rhs.state_ == Maybe) {                                            \
+        return Maybe;                                                                              \
+      }                                                                                            \
+      if (lhs.state_ == False || rhs.state_ == False) {                                            \
+        return False;                                                                              \
+      }                                                                                            \
+      return True;                                                                                 \
+    }                                                                                              \
+                                                                                                   \
+    TypeName & operator&=(TypeName rhs) { return *this = *this & rhs; }                            \
+    TypeName & operator|=(TypeName rhs) { return *this = *this | rhs; }                            \
+                                                                                                   \
+    friend auto operator==(TypeName lhs, TypeName::Enum rhs) {                                     \
+      return static_cast<int>(lhs.state_) == static_cast<int>(rhs);                                \
+    }                                                                                              \
+    friend auto operator!=(TypeName lhs, TypeName::Enum rhs) {                                     \
+      return static_cast<int>(lhs.state_) != static_cast<int>(rhs);                                \
+    }                                                                                              \
+    friend auto operator<=>(TypeName lhs, TypeName rhs) {                                          \
+      return static_cast<int>(lhs.state_) <=> static_cast<int>(rhs.state_);                        \
+    }                                                                                              \
+  }

+ 2 - 54
include/jvalidate/status.h

@@ -1,59 +1,7 @@
 #pragma once
 
-#include <compare>
+#include <jvalidate/detail/tribool.h>
 
 namespace jvalidate {
-class Status {
-public:
-  enum Enum { Accept, Reject, Noop };
-
-private:
-  Enum state_;
-
-public:
-  Status(bool state) : state_(state ? Accept : Reject) {}
-  Status(Enum state) : state_(state) {}
-
-  explicit operator bool() const { return state_ != Reject; }
-
-  friend Status operator!(Status val) {
-    if (val.state_ == Noop) {
-      return Status::Noop;
-    }
-    return val.state_ == Reject ? Accept : Reject;
-  }
-
-  friend Status operator|(Status lhs, Status rhs) {
-    if (lhs.state_ == Noop && rhs.state_ == Noop) {
-      return Noop;
-    }
-    if (lhs.state_ == Accept || rhs.state_ == Accept) {
-      return Accept;
-    }
-    return Reject;
-  }
-
-  friend Status operator&(Status lhs, Status rhs) {
-    if (lhs.state_ == Noop && rhs.state_ == Noop) {
-      return Noop;
-    }
-    if (lhs.state_ == Reject || rhs.state_ == Reject) {
-      return Reject;
-    }
-    return Accept;
-  }
-
-  Status & operator&=(Status rhs) { return *this = *this & rhs; }
-  Status & operator|=(Status rhs) { return *this = *this | rhs; }
-
-  friend auto operator==(Status lhs, Status::Enum rhs) {
-    return static_cast<int>(lhs.state_) == static_cast<int>(rhs);
-  }
-  friend auto operator!=(Status lhs, Status::Enum rhs) {
-    return static_cast<int>(lhs.state_) != static_cast<int>(rhs);
-  }
-  friend auto operator<=>(Status lhs, Status rhs) {
-    return static_cast<int>(lhs.state_) <=> static_cast<int>(rhs.state_);
-  }
-};
+JVALIDATE_TRIBOOL_TYPE(Status, Accept, Reject, Noop);
 }

+ 126 - 17
include/jvalidate/validation_result.h

@@ -3,6 +3,8 @@
 #include <map>
 #include <ostream>
 #include <unordered_set>
+#include <utility>
+#include <variant>
 #include <vector>
 
 #include <jvalidate/detail/pointer.h>
@@ -12,38 +14,145 @@ namespace jvalidate {
 class ValidationResult {
 public:
   template <Adapter A, RegexEngine RE> friend class ValidationVisitor;
+  using DocPointer = detail::Pointer;
+  using SchemaPointer = detail::Pointer;
+  using Annotation = std::variant<std::string, std::vector<std::string>>;
+  struct LocalResult {
+    bool valid;
+    std::map<std::string, Annotation> errors;
+    std::map<std::string, Annotation> annotations;
+  };
+
+  struct indent {
+    indent(int i) : i(i) {}
+
+    friend std::ostream & operator<<(std::ostream & os, indent id) {
+      while (id.i-- > 0)
+        os << "  ";
+      return os;
+    }
+
+    int i;
+  };
 
 private:
-  std::map<detail::Pointer, std::map<detail::Pointer, std::string>> errors_;
+  bool valid_;
+  std::map<DocPointer, std::map<SchemaPointer, LocalResult>> results_;
 
 public:
   friend std::ostream & operator<<(std::ostream & os, ValidationResult const & result) {
-    for (auto const & [where, errors] : result.errors_) {
-      if (errors.size() == 1) {
-        auto const & [schema_path, message] = *errors.begin();
-        std::cout << where << ": " << schema_path << ": " << message << "\n";
-      } else {
-        std::cout << where << "\n";
-        for (auto const & [schema_path, message] : errors) {
-          std::cout << "  " << schema_path << ": " << message << "\n";
+    char const * div = "\n";
+    os << "{\n" << indent(1) << R"("valid": )" << (result.valid_ ? "true" : "false") << ',' << '\n';
+    os << indent(1) << R"("details": [)";
+    for (auto const & [doc_path, by_schema] : result.results_) {
+      for (auto const & [schema_path, local] : by_schema) {
+        os << std::exchange(div, ",\n") << indent(2) << '{' << '\n';
+        os << indent(3) << R"("valid": )" << (local.valid ? "true" : "false") << ',' << '\n';
+        os << indent(3) << R"("evaluationPath": ")" << schema_path << '"' << ',' << '\n';
+        os << indent(3) << R"("instanceLocation": ")" << doc_path << '"';
+        print(os, local.annotations, "annotations", 3);
+        print(os, local.errors, "errors", 3);
+        os << '\n' << indent(2) << '}';
+      }
+    }
+    return os << '\n' << indent(1) << ']' << '\n' << '}';
+  }
+
+  static void print(std::ostream & os, std::map<std::string, Annotation> const & named,
+                    std::string_view name, int const i) {
+    if (named.empty()) {
+      return;
+    }
+    os << ',' << '\n';
+    os << indent(i) << '"' << name << '"' << ':' << ' ' << '{' << '\n';
+    for (auto const & [key, anno] : named) {
+      os << indent(i + 1) << '"' << key << '"' << ':' << ' ';
+      if (auto const * str = std::get_if<0>(&anno)) {
+        os << '"' << *str << '"';
+      } else if (auto const * vec = std::get_if<1>(&anno)) {
+        os << '[';
+        char const * div = "\n";
+        for (size_t n = 0; n < vec->size(); ++n) {
+          os << std::exchange(div, ",\n") << indent(i + 2) << '"' << vec->at(n) << '"';
         }
+        os << '\n' << indent(i + 1) << ']';
       }
+      os << '\n';
+    }
+    os << indent(i) << '}';
+  }
+
+  bool has(detail::Pointer const & where, detail::Pointer const & schema_path) const {
+    return has(where) && results_.at(where).contains(schema_path);
+  }
+
+  bool has(detail::Pointer const & where) const { return results_.contains(where); }
+
+  Annotation const * annotation(detail::Pointer const & where, detail::Pointer const & schema_path,
+                                std::string const & name) const {
+    if (not results_.contains(where)) {
+      return nullptr;
     }
-    return os;
+    auto const & by_schema = results_.at(where);
+    if (not by_schema.contains(schema_path)) {
+      return nullptr;
+    }
+    auto const & local = by_schema.at(schema_path);
+    if (not local.annotations.contains(name)) {
+      return nullptr;
+    }
+    return &local.annotations.at(name);
+  }
+
+  Annotation const * error(detail::Pointer const & where, detail::Pointer const & schema_path,
+                           std::string const & name) const {
+    if (not results_.contains(where)) {
+      return nullptr;
+    }
+    auto const & by_schema = results_.at(where);
+    if (not by_schema.contains(schema_path)) {
+      return nullptr;
+    }
+    auto const & local = by_schema.at(schema_path);
+    if (not local.errors.contains(name)) {
+      return nullptr;
+    }
+    return &local.errors.at(name);
   }
 
 private:
-  void add_error(ValidationResult && result) {
-    for (auto const & [where, errors] : result.errors_) {
-      for (auto const & [schema_path, message] : errors) {
-        errors_[where][schema_path] += message;
+  void merge(ValidationResult && result) {
+    for (auto && [where, by_schema] : result.results_) {
+      for (auto && [schema_path, local] : by_schema) {
+        results_[where][schema_path].annotations.merge(local.annotations);
+        results_[where][schema_path].errors.merge(local.errors);
       }
     }
   }
 
-  void add_error(detail::Pointer const & where, detail::Pointer const & schema_path,
-                 std::string const & message) {
-    errors_[where][schema_path] += message;
+  void valid(detail::Pointer const & where, detail::Pointer const & schema_path, bool valid) {
+    if (has(where, schema_path)) {
+      results_[where][schema_path].valid = valid;
+    }
+    if (where.empty() && schema_path.empty()) {
+      valid_ = valid;
+    }
+  }
+
+  void error(detail::Pointer const & where, detail::Pointer const & schema_path,
+             std::string const & name, Annotation message) {
+    if (std::visit([](auto const & v) { return v.empty(); }, message)) {
+      return;
+    }
+    results_[where][schema_path].errors.emplace(name, std::move(message));
+  }
+
+  void annotate(detail::Pointer const & where, detail::Pointer const & schema_path,
+                std::string const & name, Annotation message) {
+    if (std::visit([](auto const & v) { return v.empty(); }, message)) {
+      return;
+    }
+    results_[where][schema_path].annotations.emplace(name, std::move(message));
   }
 };
 }

+ 188 - 123
include/jvalidate/validation_visitor.h

@@ -1,7 +1,9 @@
 #pragma once
 
 #include <tuple>
+#include <type_traits>
 #include <unordered_map>
+#include <vector>
 
 #include <jvalidate/constraint/array_constraint.h>
 #include <jvalidate/constraint/general_constraint.h>
@@ -14,6 +16,7 @@
 #include <jvalidate/detail/iostream.h>
 #include <jvalidate/detail/number.h>
 #include <jvalidate/detail/pointer.h>
+#include <jvalidate/detail/scoped_state.h>
 #include <jvalidate/forward.h>
 #include <jvalidate/schema.h>
 #include <jvalidate/status.h>
@@ -22,6 +25,15 @@
 
 #define VISITED(type) std::get<std::unordered_set<type>>(*visited_)
 
+#define VALIDATE_SUBSCHEMA_AND_MARK_LOCAL_VISIT(subschema, subinstance, path, local_visited)       \
+  do {                                                                                             \
+    Status const partial = validate_subschema_on(subschema, subinstance, path);                    \
+    rval &= partial;                                                                               \
+    if (result_ and partial != Status::Noop) {                                                     \
+      local_visited.insert(local_visited.end(), path);                                             \
+    }                                                                                              \
+  } while (false)
+
 #define NOOP_UNLESS_TYPE(etype)                                                                    \
   RETURN_UNLESS(adapter::Type::etype == document_.type(), Status::Noop)
 
@@ -37,6 +49,7 @@ template <Adapter A, RegexEngine RE>
 class ValidationVisitor : public constraint::ConstraintVisitor {
 private:
   using VisitedAnnotation = std::tuple<std::unordered_set<size_t>, std::unordered_set<std::string>>;
+  JVALIDATE_TRIBOOL_TYPE(StoreResults, ForValid, ForInvalid, ForAnything);
 
 private:
   A document_;
@@ -51,6 +64,7 @@ private:
   std::unordered_map<std::string, RE> & regex_cache_;
 
   mutable VisitedAnnotation * visited_ = nullptr;
+  mutable StoreResults tracking_ = StoreResults::ForInvalid;
 
 public:
   ValidationVisitor(A const & json, schema::Node const & schema, ValidationConfig const & cfg,
@@ -59,20 +73,20 @@ public:
 
   Status visit(constraint::TypeConstraint const & cons) const {
     adapter::Type const type = document_.type();
+
     for (adapter::Type const accept : cons.types) {
       if (type == accept) {
-        return Status::Accept;
+        return result(Status::Accept, type, " is in types [", cons.types, "]");
       }
       if (accept == adapter::Type::Number && type == adapter::Type::Integer) {
-        return Status::Accept;
+        return result(Status::Accept, type, " is in types [", cons.types, "]");
       }
       if (accept == adapter::Type::Integer && type == adapter::Type::Number &&
           detail::is_json_integer(document_.as_number())) {
-        return Status::Accept;
+        return result(Status::Accept, type, " is in types [", cons.types, "]");
       }
     }
-    add_error("type ", type, " is not one of {", cons.types, "}");
-    return Status::Reject;
+    return result(Status::Reject, type, " is not in types [", cons.types, "]");
   }
 
   Status visit(constraint::ExtensionConstraint const & cons) const {
@@ -83,12 +97,11 @@ public:
     auto is_equal = [this](auto const & frozen) {
       return document_.equals(frozen, cfg_.strict_equality);
     };
-    for (auto const & option : cons.enumeration) {
+    for (auto const & [index, option] : detail::enumerate(cons.enumeration)) {
       if (option->apply(is_equal)) {
-        return Status::Accept;
+        return result(Status::Accept, index);
       }
     }
-    add_error("equals none of the values");
     return Status::Reject;
   }
 
@@ -105,61 +118,60 @@ public:
     }
 
     if (rval == Status::Reject) {
-      add_error("does not validate subschemas ", unmatched);
+      return result(rval, "does not validate subschemas ", unmatched);
     }
-    return rval;
+    return result(rval, "validates all subschemas");
   }
 
   Status visit(constraint::AnyOfConstraint const & cons) const {
-    Status rval = Status::Reject;
-
+    std::optional<size_t> first_validated;
     for (auto const & [index, subschema] : detail::enumerate(cons.children)) {
       if (validate_subschema(subschema, index)) {
-        rval = Status::Accept;
+        first_validated = index;
       }
-      if (not visited_ && rval == Status::Accept) {
+      if (not visited_ && first_validated.has_value()) {
         break;
       }
     }
 
-    if (rval == Status::Reject) {
-      add_error("validates none of the subschemas");
+    if (first_validated.has_value()) {
+      return result(Status::Accept, "validates subschema ", *first_validated);
     }
-    return rval;
+    return result(Status::Reject, "validates none of the subschemas");
   }
 
   Status visit(constraint::OneOfConstraint const & cons) const {
     std::set<size_t> matches;
 
     for (auto const & [index, subschema] : detail::enumerate(cons.children)) {
+      scoped_state(tracking_, StoreResults::ForAnything);
       if (validate_subschema(subschema, index)) {
         matches.insert(index);
       }
     }
 
     if (matches.size() == 1) {
-      return Status::Accept;
+      return result(Status::Accept, "validates subschema ", *matches.begin());
     }
-    add_error("validates subschemas ", matches);
-    return Status::Reject;
+    return result(Status::Reject, "validates multiple subschemas ", matches);
   }
 
   Status visit(constraint::NotConstraint const & cons) const {
-    VisitedAnnotation * suppress = nullptr;
-    std::swap(suppress, visited_);
-    bool const rval = validate_subschema(cons.child) == Status::Reject;
-    std::swap(suppress, visited_);
+    scoped_state(visited_, nullptr);
+    scoped_state(tracking_, !tracking_);
+    bool const rejected = validate_subschema(cons.child) == Status::Reject;
 
-    if (not rval) {
-      add_error("actually validates subschema");
-    }
-    return rval;
+    return rejected;
   }
 
   Status visit(constraint::ConditionalConstraint const & cons) const {
-    bool const if_result(validate_subschema(cons.if_constraint));
+    Status const if_true = [this, &cons]() {
+      scoped_state(tracking_, StoreResults::ForAnything);
+      return validate_subschema(cons.if_constraint);
+    }();
 
-    if (if_result) {
+    annotate(if_true ? "valid" : "invalid");
+    if (if_true) {
       return validate_subschema(cons.then_constraint, detail::parent, "then");
     }
     return validate_subschema(cons.else_constraint, detail::parent, "else");
@@ -169,18 +181,16 @@ public:
     switch (document_.type()) {
     case adapter::Type::Integer:
       if (int64_t value = document_.as_integer(); not cons(value)) {
-        add_error("integer ", value, " exceeds ", cons.exclusive ? "exclusive " : "", "maximum of ",
-                  cons.value);
-        return false;
+        return result(Status::Reject, value, cons.exclusive ? " >= " : " > ", cons.value);
+      } else {
+        return result(Status::Accept, value, cons.exclusive ? " < " : " <= ", cons.value);
       }
-      return true;
     case adapter::Type::Number:
       if (double value = document_.as_number(); not cons(value)) {
-        add_error("number ", value, " exceeds ", cons.exclusive ? "exclusive " : "", "maximum of ",
-                  cons.value);
-        return false;
+        return result(Status::Reject, value, cons.exclusive ? " >= " : " > ", cons.value);
+      } else {
+        return result(Status::Accept, value, cons.exclusive ? " < " : " <= ", cons.value);
       }
-      return true;
     default:
       return Status::Noop;
     }
@@ -190,18 +200,16 @@ public:
     switch (document_.type()) {
     case adapter::Type::Integer:
       if (int64_t value = document_.as_integer(); not cons(value)) {
-        add_error("integer ", value, " fails ", cons.exclusive ? "exclusive " : "", "minimum of ",
-                  cons.value);
-        return false;
+        return result(Status::Reject, value, cons.exclusive ? " <= " : " < ", cons.value);
+      } else {
+        return result(Status::Accept, value, cons.exclusive ? " > " : " >= ", cons.value);
       }
-      return true;
     case adapter::Type::Number:
       if (double value = document_.as_number(); not cons(value)) {
-        add_error("number ", value, " fails ", cons.exclusive ? "exclusive " : "", "minimum of ",
-                  cons.value);
-        return false;
+        return result(Status::Reject, value, cons.exclusive ? " <= " : " < ", cons.value);
+      } else {
+        return result(Status::Accept, value, cons.exclusive ? " > " : " >= ", cons.value);
       }
-      return true;
     default:
       return Status::Noop;
     }
@@ -212,51 +220,54 @@ public:
     RETURN_UNLESS(type == adapter::Type::Number || type == adapter::Type::Integer, Status::Noop);
 
     if (double value = document_.as_number(); not cons(value)) {
-      add_error("number ", value, " is not a multiple of ", cons.value);
-      return false;
+      return result(Status::Reject, value, " is not a multiple of ", cons.value);
+    } else {
+      return result(Status::Accept, value, " is a multiple of ", cons.value);
     }
-    return true;
   }
 
   Status visit(constraint::MaxLengthConstraint const & cons) const {
     NOOP_UNLESS_TYPE(String);
-    if (auto str = document_.as_string(); detail::length(str) > cons.value) {
-      add_error("string '", str, "' is greater than the maximum length of ", cons.value);
-      return false;
+    std::string const str = document_.as_string();
+    if (int64_t len = detail::length(str); len > cons.value) {
+      return result(Status::Reject, "string of length ", len, " is >", cons.value);
+    } else {
+      return result(Status::Accept, "string of length ", len, " is <=", cons.value);
     }
-    return true;
   }
 
   Status visit(constraint::MinLengthConstraint const & cons) const {
     NOOP_UNLESS_TYPE(String);
-    if (auto str = document_.as_string(); detail::length(str) < cons.value) {
-      add_error("string '", str, "' is less than the minimum length of ", cons.value);
-      return false;
+    std::string const str = document_.as_string();
+    if (int64_t len = detail::length(str); len < cons.value) {
+      return result(Status::Reject, "string of length ", len, " is <", cons.value);
+    } else {
+      return result(Status::Accept, "string of length ", len, " is >=", cons.value);
     }
-    return true;
   }
 
   Status visit(constraint::PatternConstraint const & cons) const {
     NOOP_UNLESS_TYPE(String);
 
     RE const & regex = regex_cache_.try_emplace(cons.regex, cons.regex).first->second;
-    if (auto str = document_.as_string(); not regex.search(str)) {
-      add_error("string '", str, "' does not match pattern /", cons.regex, "/");
-      return false;
+    std::string const str = document_.as_string();
+    if (regex.search(str)) {
+      return result(Status::Accept, "string matches pattern /", cons.regex, "/");
     }
-    return true;
+    return result(Status::Reject, "string does not match pattern /", cons.regex, "/");
   }
 
   Status visit(constraint::FormatConstraint const & cons) const {
     // https://json-schema.org/draft/2020-12/draft-bhutton-json-schema-validation-01#name-defined-formats
     NOOP_UNLESS_TYPE(String);
 
+    // TODO(samjaffe): annotate(cons.format)
+    annotate(cons.format);
     if (not cfg_.validate_format && not cons.is_assertion) {
       return true;
     }
 
-    add_error("unimplemented format assertion: '", cons.format, "'");
-    return false;
+    return result(Status::Reject, " is unimplemented");
   }
 
   Status visit(constraint::AdditionalItemsConstraint const & cons) const {
@@ -265,11 +276,13 @@ public:
     auto array = document_.as_array();
 
     Status rval = Status::Accept;
+    std::vector<size_t> items;
     for (size_t i = cons.applies_after_nth; i < array.size(); ++i) {
-      rval &= validate_subschema_on(cons.subschema, array[i], i);
+      VALIDATE_SUBSCHEMA_AND_MARK_LOCAL_VISIT(cons.subschema, array[i], i, items);
       BREAK_EARLY_IF_NO_RESULT_TREE();
     }
 
+    annotate_list(items);
     return rval;
   }
 
@@ -287,32 +300,30 @@ public:
     }
 
     if (matches < minimum) {
-      add_error("array does not contain at least ", minimum, " matching elements");
-      return Status::Reject;
+      return result(Status::Reject, "array contains <", minimum, " matching items");
     }
     if (matches > maximum) {
-      add_error("array contains more than ", maximum, " matching elements");
-      return Status::Reject;
+      return result(Status::Reject, "array contains >", maximum, " matching items");
     }
-    return Status::Accept;
+    return result(Status::Accept, "array contains ", matches, " matching items");
   }
 
   Status visit(constraint::MaxItemsConstraint const & cons) const {
     NOOP_UNLESS_TYPE(Array);
-    if (auto size = document_.array_size(); size > cons.value) {
-      add_error("array with ", size, " items is greater than the maximum of ", cons.value);
-      return false;
+    if (size_t size = document_.array_size(); size > cons.value) {
+      return result(Status::Reject, "array of size ", size, " is >", cons.value);
+    } else {
+      return result(Status::Accept, "array of size ", size, " is <=", cons.value);
     }
-    return true;
   }
 
   Status visit(constraint::MinItemsConstraint const & cons) const {
     NOOP_UNLESS_TYPE(Array);
-    if (auto size = document_.array_size(); size < cons.value) {
-      add_error("array with ", size, " items is less than the minimum of ", cons.value);
-      return false;
+    if (size_t size = document_.array_size(); size < cons.value) {
+      return result(Status::Reject, "array of size ", size, " is <", cons.value);
+    } else {
+      return result(Status::Accept, "array of size ", size, " is >=", cons.value);
     }
-    return true;
   }
 
   Status visit(constraint::TupleConstraint const & cons) const {
@@ -320,13 +331,16 @@ public:
 
     Status rval = Status::Accept;
 
-    auto array = document_.as_array();
-    size_t const n = std::min(cons.items.size(), array.size());
-    for (size_t i = 0; i < n; ++i) {
-      rval &= validate_subschema_on(cons.items[i], array[i], i);
+    std::vector<size_t> items;
+    for (auto const & [index, item] : detail::enumerate(document_.as_array())) {
+      if (index >= cons.items.size()) {
+        break;
+      }
+      VALIDATE_SUBSCHEMA_AND_MARK_LOCAL_VISIT(cons.items[index], item, index, items);
       BREAK_EARLY_IF_NO_RESULT_TREE();
     }
 
+    annotate_list(items);
     return rval;
   }
 
@@ -334,11 +348,10 @@ public:
     NOOP_UNLESS_TYPE(Array);
 
     if constexpr (std::totally_ordered<A>) {
-      std::set<A> cache;
-      for (A const & elem : document_.as_array()) {
-        if (not cache.insert(elem).second) {
-          add_error("array contains duplicate elements");
-          return Status::Reject;
+      std::map<A, size_t> cache;
+      for (auto const & [index, elem] : detail::enumerate(document_.as_array())) {
+        if (auto [it, created] = cache.emplace(elem, index); not created) {
+          return result(Status::Reject, "items ", it->second, " and ", index, " are equal");
         }
       }
     } else {
@@ -346,14 +359,13 @@ public:
       for (size_t i = 0; i < array.size(); ++i) {
         for (size_t j = i + 1; j < array.size(); ++j) {
           if (array[i].equals(array[j], true)) {
-            add_error("array elements ", i, " and ", j, " are equal");
-            return Status::Reject;
+            return result(Status::Reject, "items ", i, " and ", j, " are equal");
           }
         }
       }
     }
 
-    return Status::Accept;
+    return result(Status::Accept, "all array items are unique");
   }
 
   Status visit(constraint::AdditionalPropertiesConstraint const & cons) const {
@@ -370,13 +382,15 @@ public:
     };
 
     Status rval = Status::Accept;
+    std::vector<std::string> properties;
     for (auto const & [key, elem] : document_.as_object()) {
       if (not cons.properties.contains(key) && not matches_any_pattern(key)) {
-        rval &= validate_subschema_on(cons.subschema, elem, key);
+        VALIDATE_SUBSCHEMA_AND_MARK_LOCAL_VISIT(cons.subschema, elem, key, properties);
       }
       BREAK_EARLY_IF_NO_RESULT_TREE();
     }
 
+    annotate_list(properties);
     return rval;
   }
 
@@ -412,36 +426,39 @@ public:
 
   Status visit(constraint::MaxPropertiesConstraint const & cons) const {
     NOOP_UNLESS_TYPE(Object);
-    if (auto size = document_.object_size(); size > cons.value) {
-      add_error("object with ", size, " properties is greater than the maximum of ", cons.value);
-      return false;
+    if (size_t size = document_.object_size(); size > cons.value) {
+      return result(Status::Reject, "object of size ", size, " is >", cons.value);
+    } else {
+      return result(Status::Accept, "object of size ", size, " is <=", cons.value);
     }
-    return true;
   }
 
   Status visit(constraint::MinPropertiesConstraint const & cons) const {
     NOOP_UNLESS_TYPE(Object);
-    if (auto size = document_.object_size(); size < cons.value) {
-      add_error("object with ", size, " properties is less than the minimum of ", cons.value);
-      return false;
+    if (size_t size = document_.object_size(); size < cons.value) {
+      return result(Status::Reject, "object of size ", size, " is <", cons.value);
+    } else {
+      return result(Status::Accept, "object of size ", size, " is >=", cons.value);
     }
-    return true;
   }
 
   Status visit(constraint::PatternPropertiesConstraint const & cons) const {
     NOOP_UNLESS_TYPE(Object);
 
+    std::vector<std::string> properties;
     Status rval = Status::Accept;
     for (auto const & [pattern, subschema] : cons.properties) {
       RE const & regex = regex_cache_.try_emplace(pattern, pattern).first->second;
       for (auto const & [key, elem] : document_.as_object()) {
-        if (regex.search(key)) {
-          rval &= validate_subschema_on(subschema, elem, key);
+        if (not regex.search(key)) {
+          continue;
         }
+        VALIDATE_SUBSCHEMA_AND_MARK_LOCAL_VISIT(subschema, elem, key, properties);
         BREAK_EARLY_IF_NO_RESULT_TREE();
       }
     }
 
+    annotate_list(properties);
     return rval;
   }
 
@@ -460,13 +477,15 @@ public:
       }
     }
 
+    std::vector<std::string> properties;
     for (auto const & [key, elem] : object) {
       if (auto it = cons.properties.find(key); it != cons.properties.end()) {
-        rval &= validate_subschema_on(it->second, elem, key);
+        VALIDATE_SUBSCHEMA_AND_MARK_LOCAL_VISIT(it->second, elem, key, properties);
       }
       BREAK_EARLY_IF_NO_RESULT_TREE();
     }
 
+    annotate_list(properties);
     return rval;
   }
 
@@ -491,11 +510,10 @@ public:
     }
 
     if (required.empty()) {
-      return Status::Accept;
+      return result(Status::Accept, "contains all required properties ", cons.properties);
     }
 
-    add_error("missing required properties ", required);
-    return Status::Reject;
+    return result(Status::Reject, "missing required properties ", required);
   }
 
   Status visit(constraint::UnevaluatedItemsConstraint const & cons) const {
@@ -505,14 +523,15 @@ public:
     }
 
     Status rval = Status::Accept;
-    auto array = document_.as_array();
-    for (size_t i = 0; i < array.size(); ++i) {
-      if (not VISITED(size_t).contains(i)) {
-        rval &= validate_subschema_on(cons.subschema, array[i], i);
+    std::vector<size_t> items;
+    for (auto const & [index, item] : detail::enumerate(document_.as_array())) {
+      if (not VISITED(size_t).contains(index)) {
+        VALIDATE_SUBSCHEMA_AND_MARK_LOCAL_VISIT(cons.subschema, item, index, items);
       }
       BREAK_EARLY_IF_NO_RESULT_TREE();
     }
 
+    annotate_list(items);
     return rval;
   }
 
@@ -523,24 +542,30 @@ public:
     }
 
     Status rval = Status::Accept;
+    std::vector<std::string> properties;
     for (auto const & [key, elem] : document_.as_object()) {
       if (not VISITED(std::string).contains(key)) {
-        rval &= validate_subschema_on(cons.subschema, elem, key);
+        VALIDATE_SUBSCHEMA_AND_MARK_LOCAL_VISIT(cons.subschema, elem, key, properties);
       }
       BREAK_EARLY_IF_NO_RESULT_TREE();
     }
 
+    annotate_list(properties);
     return rval;
   }
 
   Status validate() {
-    if (auto const & reject = schema_->rejects_all()) {
-      add_error(*reject);
+    if (std::optional<std::string> const & reject = schema_->rejects_all()) {
+      if (should_annotate(Status::Reject)) {
+        result_->error(where_, schema_path_, "", *reject);
+      }
+      (result_ ? result_->valid(where_, schema_path_, false) : void());
       return Status::Reject;
     }
 
     if (schema_->accepts_all()) {
       // An accept-all schema is not No-Op for the purpose of unevaluated*
+      (result_ ? result_->valid(where_, schema_path_, true) : void());
       return Status::Accept;
     }
 
@@ -550,7 +575,7 @@ public:
     }
 
     Status rval = Status::Noop;
-    if (auto ref = schema_->reference_schema()) {
+    if (std::optional<schema::Node const *> ref = schema_->reference_schema()) {
       rval = validate_subschema(*ref, "$ref");
     }
 
@@ -567,22 +592,62 @@ public:
       rval &= p_constraint->accept(*this);
     }
 
+    (result_ ? result_->valid(where_, schema_path_, rval) : void());
     return rval;
   }
 
 private:
-  template <typename... Args> void add_error(Args &&... args) const {
-    if (not result_) {
-      return;
-    }
+  template <typename S>
+  requires(std::is_constructible_v<std::string, S>) static std::string fmt(S const & str) {
+    return str;
+  }
+
+  static std::string fmt(auto const &... args) {
     std::stringstream ss;
     using ::jvalidate::operator<<;
-    [[maybe_unused]] int _[] = {(ss << std::forward<Args>(args), 0)...};
-    result_->add_error(where_, schema_path_, ss.str());
+    [[maybe_unused]] int _[] = {(ss << args, 0)...};
+    return ss.str();
+  }
+
+  static std::vector<std::string> fmtlist(auto const & arg) {
+    std::vector<std::string> strs;
+    for (auto const & elem : arg) {
+      strs.push_back(fmt(elem));
+    }
+    return strs;
   }
 
-  template <typename C> static void merge_visited(C & to, C const & from) {
-    to.insert(from.begin(), from.end());
+  bool should_annotate(Status stat) const {
+    if (not result_) {
+      return false;
+    }
+    switch (tracking_) {
+    case StoreResults::ForAnything:
+      return stat != Status::Noop;
+    case StoreResults::ForValid:
+      return stat == Status::Accept;
+    case StoreResults::ForInvalid:
+      return stat == Status::Reject;
+    }
+  }
+
+#define ANNOTATION_HELPER(name, ADD, FMT)                                                          \
+  void name(auto const &... args) const {                                                          \
+    if (not result_) {                                                                             \
+      /* do nothing if there's no result object to append to */                                    \
+    } else if (schema_path_.empty()) {                                                             \
+      result_->ADD(where_, schema_path_, "", FMT(args...));                                        \
+    } else {                                                                                       \
+      result_->ADD(where_, schema_path_.parent(), schema_path_.back(), FMT(args...));              \
+    }                                                                                              \
+  }
+
+  ANNOTATION_HELPER(error, error, fmt)
+  ANNOTATION_HELPER(annotate, annotate, fmt)
+  ANNOTATION_HELPER(annotate_list, annotate, fmtlist)
+
+  Status result(Status stat, auto const &... args) const {
+    return (should_annotate(stat) ? error(args...) : void(), stat);
   }
 
   template <typename... K>
@@ -607,8 +672,8 @@ private:
     Status rval = next.validate();
 
     if (rval == Status::Accept and visited_) {
-      merge_visited(std::get<0>(*visited_), std::get<0>(annotate));
-      merge_visited(std::get<1>(*visited_), std::get<1>(annotate));
+      std::get<0>(*visited_).merge(std::get<0>(annotate));
+      std::get<1>(*visited_).merge(std::get<1>(annotate));
     }
     return rval;
   }
@@ -628,7 +693,7 @@ private:
       VISITED(K).insert(key);
     }
     if (status == Status::Reject and result_) {
-      result_->add_error(std::move(result));
+      result_->merge(std::move(result));
     }
     return status;
   }

+ 182 - 0
tests/annotation_test.cxx

@@ -0,0 +1,182 @@
+#include <string_view>
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include <jvalidate/adapter.h>
+#include <jvalidate/adapters/jsoncpp.h>
+#include <jvalidate/detail/pointer.h>
+#include <jvalidate/enum.h>
+#include <jvalidate/schema.h>
+#include <jvalidate/status.h>
+#include <jvalidate/uri.h>
+#include <jvalidate/validation_result.h>
+#include <jvalidate/validator.h>
+
+#include <json/reader.h>
+#include <json/value.h>
+
+using enum jvalidate::schema::Version;
+using testing::Not;
+
+auto operator""_jptr(char const * data, size_t len) {
+  return jvalidate::detail::Pointer(std::string_view{data, len});
+}
+
+Json::Value operator""_json(char const * data, size_t len) {
+  Json::Value value;
+
+  Json::CharReaderBuilder builder;
+  std::unique_ptr<Json::CharReader> reader(builder.newCharReader());
+
+  std::string error;
+  if (not reader->parse(data, data + len, &value, &error)) {
+    throw std::runtime_error(error);
+  }
+
+  return value;
+}
+
+auto validate(Json::Value const & schema_doc, Json::Value const & instance_doc,
+              jvalidate::schema::Version version = Draft2020_12) {
+  jvalidate::Schema const schema(schema_doc, version);
+
+  jvalidate::ValidationResult result;
+  (void)jvalidate::Validator(schema).validate(instance_doc, &result);
+
+  return result;
+}
+
+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);
+  if (not anno) {
+    return false;
+  }
+  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);
+}
+
+MATCHER_P4(ErrorAt, doc_path, schema_path, key, matcher, "") {
+  auto const * anno = arg.error(doc_path, schema_path, key);
+  if (not anno) {
+    return false;
+  }
+  return testing::ExplainMatchResult(matcher, *anno, result_listener);
+}
+
+TEST(Annotation, AttachesFormattingAnnotation) {
+  auto const schema = R"({
+    "format": "uri"
+  })"_json;
+
+  auto const instance = R"("http://json-schema.org")"_json;
+
+  jvalidate::ValidationResult result = validate(schema, instance);
+
+  EXPECT_THAT(result, AnnotationAt("format", "uri"));
+}
+
+TEST(Annotation, AnnotatesErrors) {
+  auto const schema = R"({
+    "minimum": 5
+  })"_json;
+
+  auto const instance = R"(4)"_json;
+
+  jvalidate::ValidationResult result = validate(schema, instance);
+
+  EXPECT_THAT(result, ErrorAt("minimum", "4 < 5"));
+}
+
+TEST(Annotation, DoesNotAnnotatesValid) {
+  auto const schema = R"({
+    "minimum": 5
+  })"_json;
+
+  auto const instance = R"(6)"_json;
+
+  jvalidate::ValidationResult result = validate(schema, instance);
+
+  EXPECT_THAT(result, Not(HasAnnotationsFor(""_jptr)));
+}
+
+TEST(Annotation, NotSchemaFlipsAnnotationRule) {
+  auto const schema = R"({
+    "not": { "minimum": 5 }
+  })"_json;
+
+  auto const instance = R"(6)"_json;
+
+  jvalidate::ValidationResult result = validate(schema, instance);
+
+  EXPECT_THAT(result, ErrorAt(""_jptr, "/not"_jptr, "minimum", "6 >= 5"));
+}
+
+TEST(Annotation, PathFollowsSchemaNotConstraintModel) {
+  auto const schema = R"({
+    "$comment": "disallow is implemented in the form of NotConstraint[TypeConstraint]",
+    "disallow": "string"
+  })"_json;
+
+  auto const instance = R"("hello")"_json;
+
+  jvalidate::ValidationResult result = validate(schema, instance, Draft03);
+
+  EXPECT_THAT(result, ErrorAt("disallow", "string is in types [string]"));
+}
+
+TEST(Annotation, SomeConstraintsAnnotateBothValidAndInvalid) {
+  auto const schema = R"({
+    "$comment": "accepts any number <= 0 or >= 10",
+    "oneOf": [
+      { "minimum": 10 },
+      { "maximum": 0 }
+    ]
+  })"_json;
+
+  auto const instance = R"(-1)"_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"));
+}
+
+TEST(Annotation, AttachesAlwaysFalseSensibly) {
+  auto const schema = R"({
+    "properties": {
+      "A": false
+    }
+  })"_json;
+
+  auto const instance = R"({
+    "A": null
+  })"_json;
+  jvalidate::ValidationResult result = validate(schema, instance);
+
+  EXPECT_THAT(result, ErrorAt("/A"_jptr, "/properties"_jptr, "", "always false"));
+}
+
+int main(int argc, char ** argv) {
+  testing::InitGoogleMock(&argc, argv);
+  return RUN_ALL_TESTS();
+}

+ 34 - 0
tests/custom_filter.h

@@ -0,0 +1,34 @@
+#pragma once
+
+#include <algorithm>
+#include <regex>
+#include <string>
+#include <vector>
+
+class Glob {
+private:
+  std::regex re_;
+
+public:
+  explicit Glob(std::string glob) {
+    for (size_t i = glob.find('.'); i != std::string::npos; i = glob.find('.', i + 2)) {
+      glob.insert(glob.begin() + i, '\\');
+    }
+    for (size_t i = glob.find('*'); i != std::string::npos; i = glob.find('*', i + 2)) {
+      glob.insert(glob.begin() + i, '.');
+    }
+    re_ = std::regex(glob);
+  }
+
+  bool operator==(std::string const & str) const { return std::regex_search(str, re_); }
+};
+
+struct RecursiveTestFilter {
+  std::vector<Glob> whitelist;
+  std::vector<Glob> blacklist;
+
+  bool accepts(std::string const & str) const {
+    return std::count(blacklist.begin(), blacklist.end(), str) == 0 and
+           (whitelist.empty() or std::count(whitelist.begin(), whitelist.end(), str) > 0);
+  }
+};

+ 2 - 29
tests/selfvalidate_test.cxx

@@ -22,40 +22,13 @@
 #include <json/value.h>
 #include <json/writer.h>
 
+#include "./custom_filter.h"
 #include "./json_schema_test_suite.h"
 
 using jvalidate::schema::Version;
 
 using testing::TestWithParam;
 
-class Glob {
-private:
-  std::regex re_;
-
-public:
-  explicit Glob(std::string glob) {
-    for (size_t i = glob.find('.'); i != std::string::npos; i = glob.find('.', i + 2)) {
-      glob.insert(glob.begin() + i, '\\');
-    }
-    for (size_t i = glob.find('*'); i != std::string::npos; i = glob.find('*', i + 2)) {
-      glob.insert(glob.begin() + i, '.');
-    }
-    re_ = std::regex(glob);
-  }
-
-  bool operator==(std::string const & str) const { return std::regex_search(str, re_); }
-};
-
-struct RecursiveTestFilter {
-  std::vector<Glob> whitelist;
-  std::vector<Glob> blacklist;
-
-  bool accepts(std::string const & str) const {
-    return std::count(blacklist.begin(), blacklist.end(), str) == 0 and
-      (whitelist.empty() or std::count(whitelist.begin(), whitelist.end(), str) > 0);
-  }
-};
-
 bool load_stream(std::istream & in, Json::Value & out) {
   Json::CharReaderBuilder builder;
   std::string error;
@@ -124,7 +97,7 @@ protected:
         tokens[i - 1] += tokens[i];
         tokens.erase(tokens.begin() + i);
       }
-      for (auto &tok : tokens) {
+      for (auto & tok : tokens) {
         into.emplace_back(tok);
       }
     };