fix: clean up Result object

include/jvalidate/validation_result.h

@@ -5,6 +5,7 @@
 #include <unordered_set>
 #include <vector>
 
+#include <jvalidate/detail/pointer.h>
 #include <jvalidate/forward.h>
 
 namespace jvalidate {
@@ -12,103 +13,37 @@ class ValidationResult {
 public:
   template <Adapter A, RegexEngine RE> friend class ValidationVisitor;
 
-  struct Errors {
-    bool empty() const { return message.empty() && properties.empty() && items.empty(); }
-
-    std::string constraint;
-    std::string message;
-    std::map<std::string, ValidationResult> properties;
-    std::map<size_t, ValidationResult> items;
-  };
-
 private:
-  std::unordered_set<std::string> visited_properties_;
-  std::unordered_set<size_t> visited_items_;
-
-  std::string message_;
-  std::vector<Errors> errors_;
+  std::map<detail::Pointer, std::map<detail::Pointer, std::string>> errors_;
 
 public:
-  void message(std::string const & message) {
-    (errors_.empty() ? message_ : errors_.back().message) = message;
-  }
-
-  bool empty() const {
-    return message_.empty() &&
-           std::all_of(errors_.begin(), errors_.end(), [](auto & e) { return e.empty(); });
-  }
-  bool has_visited(size_t item) const { return visited_items_.contains(item); }
-  bool has_visited(std::string const & property) const {
-    return visited_properties_.contains(property);
-  }
-
   friend std::ostream & operator<<(std::ostream & os, ValidationResult const & result) {
-    result.print(os, 0);
-    return os;
-  }
-
-private:
-  void constraint(std::string const & name) { errors_.push_back(Errors{.constraint = name}); }
-
-  void visit(size_t item) { visited_items_.emplace(item); }
-  void visit(std::string const & property) { visited_properties_.emplace(property); }
-
-  void error(size_t item, ValidationResult && result) {
-    if (errors_.empty()) {
-      return;
-    }
-    if (!result.empty()) {
-      errors_.back().items.emplace(item, std::move(result));
-    }
-    visited_items_.emplace(item);
-  }
-
-  void error(std::string const & property, ValidationResult && result) {
-    if (errors_.empty()) {
-      return;
-    }
-    if (!result.empty()) {
-      errors_.back().properties.emplace(property, std::move(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";
+        }
+      }
     }
-    visited_properties_.emplace(property);
+    return os;
   }
 
 private:
-  static void indent(std::ostream & os, int depth) {
-    for (int i = 0; i < depth; ++i) {
-      os << ' ' << ' ';
+  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 print(std::ostream & os, int depth) const {
-    if (not message_.empty()) {
-      indent(os, depth);
-      os << message_ << "\n";
-    }
-
-    for (auto const & error : errors_) {
-      if (error.empty()) {
-        continue;
-      }
-
-      indent(os, depth);
-      os << "for constraint '" << error.constraint << "'";
-      if (not error.message.empty()) {
-        os << ": " << error.message;
-      }
-      os << "\n";
-
-      for (auto const & [i, r] : error.items) {
-        indent(os, depth + 1);
-        os << "at " << i << ":\n";
-        r.print(os, depth + 2);
-      }
-      for (auto const & [i, r] : error.properties) {
-        indent(os, depth + 1);
-        os << "at '" << i << "':\n";
-        r.print(os, depth + 2);
-      }
-    }
+  void add_error(detail::Pointer const & where, detail::Pointer const & schema_path,
+                 std::string const & message) {
+    errors_[where][schema_path] += message;
   }
 };
 }

include/jvalidate/validation_visitor.h

@@ -10,12 +10,15 @@
 #include <jvalidate/constraint/visitor.h>
 #include <jvalidate/detail/expect.h>
 #include <jvalidate/detail/iostream.h>
+#include <jvalidate/detail/pointer.h>
 #include <jvalidate/forward.h>
 #include <jvalidate/schema.h>
 #include <jvalidate/status.h>
 #include <jvalidate/validation_config.h>
 #include <jvalidate/validation_result.h>
 
+#define VISITED(type) std::get<std::unordered_set<type>>(visited_)
+
 #define NOOP_UNLESS_TYPE(etype) RETURN_UNLESS(adapter::Type::etype & document_.type(), Status::Noop)
 
 #define BREAK_EARLY_IF_NO_RESULT_TREE()                                                            \
@@ -31,19 +34,21 @@ class ValidationVisitor : public constraint::ConstraintVisitor {
 private:
   A document_;
   detail::Pointer where_;
+  detail::Pointer schema_path_;
 
   schema::Node const & schema_;
 
   ValidationResult * result_;
-  ValidationResult * local_result_;
 
   ValidationConfig const & cfg_;
   std::unordered_map<std::string, RE> & regex_cache_;
 
+  mutable std::tuple<std::unordered_set<size_t>, std::unordered_set<std::string>> visited_;
+
 public:
   ValidationVisitor(A const & json, schema::Node const & schema, ValidationConfig const & cfg,
                     std::unordered_map<std::string, RE> & regex_cache, ValidationResult * result)
-      : ValidationVisitor(json, schema, cfg, regex_cache, {}, result) {}
+      : ValidationVisitor(json, schema, cfg, regex_cache, {}, {}, result) {}
 
   Status visit(constraint::TypeConstraint const & cons) const {
     adapter::Type const type = document_.type();
@@ -76,8 +81,10 @@ public:
   Status visit(constraint::AllOfConstraint const & cons) const {
     Status rval = Status::Accept;
 
+    size_t i = 0;
     for (schema::Node const * subschema : cons.children) {
-      rval &= validate_subschema(subschema);
+      rval &= validate_subschema(subschema, i);
+      ++i;
       BREAK_EARLY_IF_NO_RESULT_TREE();
     }
 
@@ -85,10 +92,12 @@ public:
   }
 
   Status visit(constraint::AnyOfConstraint const & cons) const {
+    size_t i = 0;
     for (schema::Node const * subschema : cons.children) {
-      if (validate_subschema(subschema)) {
+      if (validate_subschema(subschema, i)) {
         return Status::Accept;
       }
+      ++i;
     }
 
     return Status::Reject;
@@ -96,24 +105,26 @@ public:
 
   Status visit(constraint::OneOfConstraint const & cons) const {
     size_t matches = 0;
+    size_t i = 0;
     for (schema::Node const * subschema : cons.children) {
-      if (validate_subschema(subschema)) {
+      if (validate_subschema(subschema, i)) {
         ++matches;
       }
+      ++i;
     }
 
     return matches == 1 ? Status::Accept : Status::Reject;
   }
 
   Status visit(constraint::NotConstraint const & cons) const {
-    return validate_subschema(cons.child) == Status::Reject;
+    return validate_subschema(cons.child, "not") == Status::Reject;
   }
 
   Status visit(constraint::ConditionalConstraint const & cons) const {
-    if (validate_subschema(cons.if_constraint)) {
-      return validate_subschema(cons.then_constraint);
+    if (validate_subschema(cons.if_constraint, "if")) {
+      return validate_subschema(cons.then_constraint, "then");
     }
-    return validate_subschema(cons.else_constraint);
+    return validate_subschema(cons.else_constraint, "else");
   }
 
   Status visit(constraint::MaximumConstraint const & cons) const {
@@ -279,7 +290,7 @@ public:
         continue;
       }
 
-      rval &= validate_subschema(subschema);
+      rval &= validate_subschema(subschema, key);
       BREAK_EARLY_IF_NO_RESULT_TREE();
     }
 
@@ -358,7 +369,7 @@ public:
     for (auto const & [key, _] : document_.as_object()) {
       // TODO(samjaffe): Should we prefer a std::string adapter like valijson?
       typename A::value_type key_json{key};
-      rval &= validate_subschema_on(cons.key_schema, A(key_json), "$$key");
+      rval &= validate_subschema_on(cons.key_schema, A(key_json), std::string("$$key"));
     }
     return rval;
   }
@@ -380,30 +391,32 @@ public:
   }
 
   Status visit(constraint::UnevaluatedItemsConstraint const & cons) const {
-    EXPECT_M(local_result_, "Invalid State - no result object for post-constraint");
     NOOP_UNLESS_TYPE(Array);
 
     Status rval = Status::Accept;
     auto array = document_.as_array();
     for (size_t i = 0; i < array.size(); ++i) {
-      if (not local_result_->has_visited(i)) {
+      if (not VISITED(size_t).contains(i)) {
         rval &= validate_subschema_on(cons.subschema, array[i], i);
       }
       BREAK_EARLY_IF_NO_RESULT_TREE();
     }
+
+    return rval;
   }
 
   Status visit(constraint::UnevaluatedPropertiesConstraint const & cons) const {
-    EXPECT_M(local_result_, "Invalid State - no result object for post-constraint");
     NOOP_UNLESS_TYPE(Object);
 
     Status rval = Status::Accept;
     for (auto const & [key, elem] : document_.as_object()) {
-      if (not local_result_->has_visited(key)) {
+      if (not VISITED(std::string).contains(key)) {
         rval &= validate_subschema_on(cons.subschema, elem, key);
       }
       BREAK_EARLY_IF_NO_RESULT_TREE();
     }
+
+    return rval;
   }
 
   Status validate() {
@@ -412,30 +425,21 @@ public:
       return Status::Reject;
     }
 
-    ValidationResult local_result;
-    if (schema_.requires_result_context() && not local_result_) {
-      // Ensure that we store results even if there aren't any...
-      local_result_ = &local_result;
-    }
-
     Status rval = Status::Noop;
     if (auto ref = schema_.reference_schema()) {
-      rval = validate_subschema(*ref);
+      rval = validate_subschema(*ref, "$ref");
     }
 
+    detail::Pointer const current_schema = schema_path_;
     for (auto const & [key, p_constraint] : schema_.constraints()) {
       BREAK_EARLY_IF_NO_RESULT_TREE();
-      if (result_) {
-        result_->constraint(key);
-      }
+      schema_path_ = current_schema / key;
       rval &= p_constraint->accept(*this);
     }
 
     for (auto const & [key, p_constraint] : schema_.post_constraints()) {
       BREAK_EARLY_IF_NO_RESULT_TREE();
-      if (result_) {
-        result_->constraint(key);
-      }
+      schema_path_ = current_schema / key;
       rval &= p_constraint->accept(*this);
     }
 
@@ -450,20 +454,21 @@ private:
     std::stringstream ss;
     using ::jvalidate::operator<<;
     [[maybe_unused]] int _[] = {(ss << std::forward<Args>(args), 0)...};
-    result_->message(ss.str());
+    result_->add_error(where_, schema_path_, ss.str());
   }
 
   ValidationVisitor(A const & json, schema::Node const & schema, ValidationConfig const & cfg,
                     std::unordered_map<std::string, RE> & regex_cache,
-                    detail::Pointer const & where, ValidationResult * result,
-                    ValidationResult * local_result = nullptr)
-      : document_(json), where_(where), schema_(schema), cfg_(cfg), regex_cache_(regex_cache),
-        result_(result), local_result_(local_result ?: result_) {}
+                    detail::Pointer const & where, detail::Pointer const & schema_path,
+                    ValidationResult * result)
+      : document_(json), where_(where), schema_path_(schema_path), schema_(schema), cfg_(cfg),
+        regex_cache_(regex_cache), result_(result) {}
 
-  Status validate_subschema(schema::Node const * subschema) const {
+  template <typename K>
+  Status validate_subschema(schema::Node const * subschema, K const & key) const {
     EXPECT(subschema != &schema_); // TODO(samjaffe) - Figure out what's causing this infinite loop
-    return ValidationVisitor(document_, *subschema, cfg_, regex_cache_, where_, result_,
-                             local_result_)
+    return ValidationVisitor(document_, *subschema, cfg_, regex_cache_, where_, schema_path_ / key,
+                             result_)
         .validate();
   }
 
@@ -473,13 +478,14 @@ private:
     ValidationResult next;
     ValidationResult * pnext = result_ ? &next : nullptr;
 
-    auto status =
-        ValidationVisitor(document, *subschema, cfg_, regex_cache_, where_ / key, pnext).validate();
-    if (status != Status::Noop and local_result_) {
-      local_result_->visit(key);
+    auto status = ValidationVisitor(document, *subschema, cfg_, regex_cache_, where_ / key,
+                                    schema_path_, pnext)
+                      .validate();
+    if (status != Status::Noop) {
+      VISITED(K).insert(key);
     }
     if (status == Status::Reject and result_) {
-      result_->error(key, std::move(next));
+      result_->add_error(std::move(next));
     }
     return status;
   }