refactor: improve annotation feature to allow both accepted/rejected notation

include/jvalidate/detail/scoped_state.h

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

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_);                        \
+    }                                                                                              \
+  }

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);
 }

include/jvalidate/validation_result.h

@@ -14,17 +14,17 @@ public:
   template <Adapter A, RegexEngine RE> friend class ValidationVisitor;
 
 private:
-  std::map<detail::Pointer, std::map<detail::Pointer, std::string>> errors_;
+  std::map<detail::Pointer, std::map<detail::Pointer, std::string>> annotations_;
 
 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();
+    for (auto const & [where, annotations] : result.annotations_) {
+      if (annotations.size() == 1) {
+        auto const & [schema_path, message] = *annotations.begin();
         std::cout << where << ": " << schema_path << ": " << message << "\n";
       } else {
         std::cout << where << "\n";
-        for (auto const & [schema_path, message] : errors) {
+        for (auto const & [schema_path, message] : annotations) {
           std::cout << "  " << schema_path << ": " << message << "\n";
         }
       }
@@ -33,17 +33,17 @@ public:
   }
 
 private:
-  void add_error(ValidationResult && result) {
-    for (auto const & [where, errors] : result.errors_) {
+  void annotate(ValidationResult && result) {
+    for (auto const & [where, errors] : result.annotations_) {
       for (auto const & [schema_path, message] : errors) {
-        errors_[where][schema_path] += message;
+        annotations_[where][schema_path] += message;
       }
     }
   }
 
-  void add_error(detail::Pointer const & where, detail::Pointer const & schema_path,
-                 std::string const & message) {
-    errors_[where][schema_path] += message;
+  void annotate(detail::Pointer const & where, detail::Pointer const & schema_path,
+                std::string const & message) {
+    annotations_[where][schema_path] += message;
   }
 };
 }

include/jvalidate/validation_visitor.h

@@ -13,6 +13,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>
@@ -36,6 +37,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_;
@@ -50,6 +52,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,
@@ -60,18 +63,17 @@ public:
     adapter::Type const type = document_.type();
     for (adapter::Type const accept : cons.types) {
       if (type == accept) {
-        return Status::Accept;
+        return note(Status::Accept, "type ", type, " is one of [", cons.types, "]");
       }
       if (accept == adapter::Type::Number && type == adapter::Type::Integer) {
-        return Status::Accept;
+        return note(Status::Accept, "type ", type, " is one of [", cons.types, "]");
       }
       if (accept == adapter::Type::Integer && type == adapter::Type::Number &&
           detail::is_json_integer(document_.as_number())) {
-        return Status::Accept;
+        return note(Status::Accept, "type ", type, " is one of [", cons.types, "]");
       }
     }
-    add_error("type ", type, " is not one of {", cons.types, "}");
-    return Status::Reject;
+    return note(Status::Reject, "type ", type, " is not one of [", cons.types, "]");
   }
 
   Status visit(constraint::ExtensionConstraint const & cons) const {
@@ -82,13 +84,14 @@ public:
     auto is_equal = [this](auto const & frozen) {
       return document_.equals(frozen, cfg_.strict_equality);
     };
+    size_t index = 0;
     for (auto const & option : cons.enumeration) {
       if (option->apply(is_equal)) {
-        return Status::Accept;
+        return note(Status::Accept, "value is enum ", index);
       }
+      ++index;
     }
-    add_error("equals none of the values");
-    return Status::Reject;
+    return note(Status::Reject, "value is none of the enums");
   }
 
   Status visit(constraint::AllOfConstraint const & cons) const {
@@ -106,9 +109,9 @@ public:
     }
 
     if (rval == Status::Reject) {
-      add_error("does not validate subschemas ", unmatched);
+      return note(rval, "does not validate subschemas ", unmatched);
     }
-    return rval;
+    return note(rval, "validates all subschemas");
   }
 
   Status visit(constraint::AnyOfConstraint const & cons) const {
@@ -126,13 +129,15 @@ public:
     }
 
     if (rval == Status::Reject) {
-      add_error("validates none of the subschemas");
+      return note(rval, "validates none of the subschemas");
     }
-    return rval;
+    return note(rval, "validates subschema ", i);
   }
 
   Status visit(constraint::OneOfConstraint const & cons) const {
+    scoped_state(tracking_, StoreResults::ForAnything);
     std::set<size_t> matches;
+
     size_t i = 0;
     for (schema::Node const * subschema : cons.children) {
       if (validate_subschema(subschema, i)) {
@@ -142,31 +147,28 @@ public:
     }
 
     if (matches.size() == 1) {
-      return Status::Accept;
+      return note(Status::Accept, "validates subschema ", *matches.begin());
     }
-    add_error("validates subschemas ", matches);
-    return Status::Reject;
+    return note(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");
+    if (not rejected) {
+      annotate("actually validates subschema");
     }
-    return rval;
+    return rejected;
   }
 
   Status visit(constraint::ConditionalConstraint const & cons) const {
-    VisitedAnnotation * suppress = nullptr;
-    std::swap(suppress, visited_);
-    bool const if_result = validate_subschema(cons.if_constraint);
-    std::swap(suppress, visited_);
-
-    if (if_result) {
+    bool const if_true = [this, &cons]() {
+      scoped_state(tracking_, StoreResults::ForAnything);
+      return validate_subschema(cons.if_constraint);
+    };
+    if (if_true) {
       return validate_subschema(cons.then_constraint, detail::parent, "then");
     }
     return validate_subschema(cons.else_constraint, detail::parent, "else");
@@ -176,18 +178,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 note(Status::Reject, value, cons.exclusive ? " >= " : " > ", cons.value);
+      } else {
+        return note(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 note(Status::Reject, value, cons.exclusive ? " >= " : " > ", cons.value);
+      } else {
+        return note(Status::Accept, value, cons.exclusive ? " < " : " <= ", cons.value);
       }
-      return true;
     default:
       return Status::Noop;
     }
@@ -197,18 +197,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 note(Status::Reject, value, cons.exclusive ? " <= " : " < ", cons.value);
+      } else {
+        return note(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 note(Status::Reject, value, cons.exclusive ? " <= " : " < ", cons.value);
+      } else {
+        return note(Status::Accept, value, cons.exclusive ? " > " : " >= ", cons.value);
       }
-      return true;
     default:
       return Status::Noop;
     }
@@ -219,51 +217,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 note(Status::Reject, value, " is not a multiple of ", cons.value);
+    } else {
+      return note(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 note(Status::Reject, "'", str, "' of length ", len, " is >", cons.value);
+    } else {
+      return note(Status::Accept, "'", str, "' 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 note(Status::Reject, "'", str, "' of length ", len, " is <", cons.value);
+    } else {
+      return note(Status::Accept, "'", str, "' 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 note(Status::Accept, "'", str, "' matches pattern /", cons.regex, "/");
     }
-    return true;
+    return note(Status::Reject, "'", str, "' 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("format '", cons.format, "'");
     if (not cfg_.validate_format && not cons.is_assertion) {
       return true;
     }
 
-    add_error("unimplemented format assertion: '", cons.format, "'");
-    return false;
+    return note(Status::Reject, " is unimplemented");
   }
 
   Status visit(constraint::AdditionalItemsConstraint const & cons) const {
@@ -294,32 +295,30 @@ public:
     }
 
     if (matches < minimum) {
-      add_error("array does not contain at least ", minimum, " matching elements");
-      return Status::Reject;
+      return note(Status::Reject, "array contains <", minimum, " matching elements");
     }
     if (matches > maximum) {
-      add_error("array contains more than ", maximum, " matching elements");
-      return Status::Reject;
+      return note(Status::Reject, "array contains >", maximum, " matching elements");
     }
     return Status::Accept;
   }
 
   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 note(Status::Reject, "array of size ", size, " is >", cons.value);
+    } else {
+      return note(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 note(Status::Reject, "array of size ", size, " is <", cons.value);
+    } else {
+      return note(Status::Accept, "array of size ", size, " is >=", cons.value);
     }
-    return true;
   }
 
   Status visit(constraint::TupleConstraint const & cons) const {
@@ -341,26 +340,26 @@ public:
     NOOP_UNLESS_TYPE(Array);
 
     if constexpr (std::totally_ordered<A>) {
-      std::set<A> cache;
+      std::map<A, size_t> cache;
+      size_t index = 0;
       for (A const & elem : document_.as_array()) {
-        if (not cache.insert(elem).second) {
-          add_error("array contains duplicate elements");
-          return Status::Reject;
+        if (auto [it, created] = cache.emplace(elem, index); not created) {
+          return note(Status::Reject, "items ", it->second, " and ", index, " are equal");
         }
+        ++index;
       }
     } else {
       auto array = document_.as_array();
       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 note(Status::Reject, "items ", i, " and ", j, " are equal");
           }
         }
       }
     }
 
-    return Status::Accept;
+    return note(Status::Accept, "all array items are unique");
   }
 
   Status visit(constraint::AdditionalPropertiesConstraint const & cons) const {
@@ -419,20 +418,20 @@ 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 note(Status::Reject, "object of size ", size, " is >", cons.value);
+    } else {
+      return note(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 note(Status::Reject, "object of size ", size, " is <", cons.value);
+    } else {
+      return note(Status::Accept, "object of size ", size, " is >=", cons.value);
     }
-    return true;
   }
 
   Status visit(constraint::PatternPropertiesConstraint const & cons) const {
@@ -498,11 +497,10 @@ public:
     }
 
     if (required.empty()) {
-      return Status::Accept;
+      return note(Status::Accept, "contains all required properties ", cons.properties);
     }
 
-    add_error("missing required properties ", required);
-    return Status::Reject;
+    return note(Status::Reject, "missing required properties ", required);
   }
 
   Status visit(constraint::UnevaluatedItemsConstraint const & cons) const {
@@ -541,8 +539,8 @@ public:
   }
 
   Status validate() {
-    if (auto const & reject = schema_->rejects_all()) {
-      add_error(*reject);
+    if (std::optional<std::string> const & reject = schema_->rejects_all()) {
+      annotate(*reject);
       return Status::Reject;
     }
 
@@ -557,7 +555,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");
     }
 
@@ -578,14 +576,36 @@ public:
   }
 
 private:
-  template <typename... Args> void add_error(Args &&... args) const {
+  template <typename... Args> void annotate(Args &&... args) const {
     if (not result_) {
       return;
     }
     std::stringstream ss;
     using ::jvalidate::operator<<;
     [[maybe_unused]] int _[] = {(ss << std::forward<Args>(args), 0)...};
-    result_->add_error(where_, schema_path_, ss.str());
+    result_->annotate(where_, schema_path_, ss.str());
+  }
+
+  template <typename... Args> Status note(Status stat, Args &&... args) const {
+    switch (tracking_) {
+    case StoreResults::ForAnything:
+      if (stat != Status::Noop) {
+        annotate(std::forward<Args>(args)...);
+      }
+      break;
+    case StoreResults::ForValid:
+      if (stat == Status::Accept) {
+        annotate(std::forward<Args>(args)...);
+      }
+      break;
+    case StoreResults::ForInvalid:
+      if (stat == Status::Reject) {
+        annotate(std::forward<Args>(args)...);
+      }
+      break;
+    }
+
+    return stat;
   }
 
   template <typename C> static void merge_visited(C & to, C const & from) {
@@ -625,7 +645,7 @@ private:
       VISITED(K).insert(key);
     }
     if (status == Status::Reject and result_) {
-      result_->add_error(std::move(result));
+      result_->annotate(std::move(result));
     }
     return status;
   }