refactor: some more cleanup of includes and method signatures

Makefile

@@ -13,7 +13,7 @@ CXX_FLAGS := -Wall -Wextra -Werror -std=c++20 \
 	     -isystem include/ -I/opt/homebrew/opt/icu4c/include \
 	     -DJVALIDATE_USE_EXCEPTIONS -DJVALIDATE_LOAD_FAILURE_AS_FALSE_SCHEMA
 
-LD_FLAGS := -L/opt/local/lib -L/opt/homebrew/opt/icu4c/lib -licuuc
+LD_FLAGS := -L/opt/homebrew/lib -L/opt/homebrew/opt/icu4c/lib -licuuc
 
 TEST_DIR := tests/
 INCLUDE_DIR := include/

include/jvalidate/adapters/jsoncpp.h

@@ -1,6 +1,4 @@
 #pragma once
-#include <compare>
-#include <memory>
 #include <type_traits>
 
 #include <json/value.h>
@@ -34,7 +32,8 @@ public:
   }
 
   void assign(std::string const & key, Const const & frozen) const
-      requires(not std::is_const_v<JSON>) {
+    requires(not std::is_const_v<JSON>)
+  {
     (*this)[key].assign(frozen);
   }
 };
@@ -76,7 +75,9 @@ public:
   static std::string key(auto it) { return it.key().asString(); }
 
   using detail::SimpleAdapter<JSON>::assign;
-  void assign(Adapter const & adapter) const requires(not std::is_const_v<JSON>) {
+  void assign(Adapter const & adapter) const
+    requires(not std::is_const_v<JSON>)
+  {
     switch (adapter.type()) {
     case Type::Null:
       *value() = Json::nullValue;
@@ -109,7 +110,8 @@ public:
     }
   }
 
-public : using JsonCppAdapter::SimpleAdapter::value;
+public:
   using JsonCppAdapter::SimpleAdapter::const_value;
+  using JsonCppAdapter::SimpleAdapter::value;
 };
 }

include/jvalidate/constraint/constraint.h

@@ -1,7 +1,5 @@
 #pragma once
 
-#include <variant>
-
 #include <jvalidate/constraint/visitor.h>
 #include <jvalidate/detail/pointer.h>
 #include <jvalidate/enum.h>
@@ -10,9 +8,6 @@
 
 namespace jvalidate::constraint {
 class Constraint {
-public:
-  using SubConstraint = std::variant<schema::Node const *, std::unique_ptr<Constraint>>;
-
 public:
   virtual ~Constraint() = default;
   virtual Status accept(ConstraintVisitor const & visitor) const = 0;

include/jvalidate/detail/simple_adapter.h

@@ -1,6 +1,6 @@
 #pragma once
 
-#include <unordered_map>
+#include <map>
 #include <vector>
 
 #include <jvalidate/adapter.h>
@@ -11,6 +11,14 @@
 #include <jvalidate/status.h>
 
 namespace jvalidate::adapter::detail {
+/**
+ * @brief A basic implementation of an Adapter for object JSON, which acts like
+ * a map<string, Adapter>. Implements the ObjectAdapter constraint.
+ *
+ * @tparam JSON The actual underlying json type
+ * @tparam Adapter The concrete implementation class for the Adapter. Used to
+ * proxy the mapped_type of the underlying JSON Object.
+ */
 template <typename JSON, typename Adapter = AdapterFor<JSON>> class SimpleObjectAdapter {
 public:
   using underlying_iterator_t = decltype(std::declval<JSON>().begin());
@@ -21,7 +29,7 @@ public:
   size_t size() const { return value_ ? value_->size() : 0; }
 
   const_iterator find(std::string const & key) const {
-    return std::find_if(begin(), end(), [key](auto const & kv) { return kv.first == key; });
+    return std::find_if(begin(), end(), [&key](auto const & kv) { return kv.first == key; });
   }
 
   bool contains(std::string const & key) const { return find(key) != end(); }
@@ -50,6 +58,14 @@ private:
   JSON * value_;
 };
 
+/**
+ * @brief A basic implementation of an Adapter for array JSON, which acts like
+ * a vector<Adapter>. Implements the ArrayAdapter constraint.
+ *
+ * @tparam JSON The actual underlying json type
+ * @tparam Adapter The concrete implementation class for the Adapter. Used to
+ * proxy the mapped_type of the underlying JSON Array.
+ */
 template <typename JSON, typename Adapter = AdapterFor<JSON>> class SimpleArrayAdapter {
 public:
   using underlying_iterator_t = decltype(std::declval<JSON>().begin());
@@ -87,6 +103,32 @@ private:
   JSON * value_;
 };
 
+/**
+ * @brief A basic implementation of an Adapter, for any JSON type that
+ * implements standard components.
+ *
+ * Provides final implementations of the virtual methods of
+ * jvalidate::detail::Adapter:
+ * - apply_array(AdapterCallback)        -> Status
+ * - array_size()                        -> size_t
+ * - apply_object(ObjectAdapterCallback) -> Status
+ * - object_size()                       -> size_t
+ * - equals(Adapter, bool)               -> bool
+ * - freeze()                            -> unique_ptr<Const const>
+ *
+ * Fulfills the constraint jvalidate::Adapter by providing default
+ * implementations of:
+ * - as_array()                          -> SimpleArrayAdapter<JSON>
+ * - as_object()                         -> SimpleObjectAdapter<JSON>
+ *
+ * Fulfills one third of the constraint jvalidate::MutableAdapter, iff CRTP
+ * implements the other two constraints.
+ * - assign(Const)                       -> void
+ *
+ * @tparam JSON The actual underlying json type
+ * @tparam CRTP The concrete implementation class below this, using the
+ * "Curiously Recursive Template Pattern".
+ */
 template <typename JSON, typename CRTP = AdapterFor<JSON>> class SimpleAdapter : public Adapter {
 public:
   static constexpr bool is_mutable =
@@ -131,7 +173,9 @@ public:
     return std::make_unique<GenericConst<value_type>>(const_value());
   }
 
-  void assign(Const const & frozen) const requires(is_mutable) {
+  void assign(Const const & frozen) const
+    requires(is_mutable)
+  {
     EXPECT_M(value_ != nullptr, "Failed to create a new element in parent object");
     if (auto * this_frozen = dynamic_cast<GenericConst<value_type> const *>(&frozen)) {
       *value_ = this_frozen->value();
@@ -144,11 +188,15 @@ public:
     });
   }
 
-  auto operator<=>(SimpleAdapter const & rhs) const requires std::totally_ordered<JSON> {
+  auto operator<=>(SimpleAdapter const & rhs) const
+    requires std::totally_ordered<JSON>
+  {
     using ord = std::strong_ordering;
+    // Optimization - first we compare pointers
     if (value_ == rhs.value_) {
       return ord::equal;
     }
+    // TODO: Can I implement this as `return *value_ <=> *rhs.value_`?
     if (value_ && rhs.value_) {
       if (*value_ < *rhs.value_) {
         return ord::less;
@@ -158,6 +206,7 @@ public:
       }
       return ord::equal;
     }
+    // Treat JSON(null) and nullptr as equivalent
     if (value_) {
       return type() == Type::Null ? ord::equivalent : ord::greater;
     }

include/jvalidate/forward.h

@@ -3,6 +3,7 @@
 #include <functional>
 #include <string>
 #include <type_traits>
+#include <variant>
 
 namespace jvalidate {
 class Schema;
@@ -26,51 +27,60 @@ template <typename V> struct AdapterTraits<V const> : AdapterTraits<V> {};
 template <typename JSON> using AdapterFor = typename AdapterTraits<JSON>::template Adapter<JSON>;
 }
 
+namespace jvalidate::schema {
+enum class Version : int;
+class Node;
+}
+
 namespace jvalidate::constraint {
 class ConstraintVisitor;
 class Constraint;
+using SubConstraint = std::variant<schema::Node const *, std::unique_ptr<Constraint>>;
 template <typename> class SimpleConstraint;
-class ExtensionConstraint;
-
-class TypeConstraint;
-class EnumConstraint;
-class AllOfConstraint;
-class AnyOfConstraint;
-class OneOfConstraint;
-class NotConstraint;
-class ConditionalConstraint;
-
-class MaximumConstraint;
-class MinimumConstraint;
-class MultipleOfConstraint;
-
-class MaxLengthConstraint;
-class MinLengthConstraint;
-class PatternConstraint;
-class FormatConstraint;
-
-class AdditionalItemsConstraint;
-class ContainsConstraint;
-class MaxItemsConstraint;
-class MinItemsConstraint;
-class TupleConstraint;
-class UnevaluatedItemsConstraint;
-class UniqueItemsConstraint;
-
-class AdditionalPropertiesConstraint;
-class DependenciesConstraint;
-class MaxPropertiesConstraint;
-class MinPropertiesConstraint;
-class PatternPropertiesConstraint;
-class PropertiesConstraint;
-class PropertyNamesConstraint;
-class RequiredConstraint;
-class UnevaluatedPropertiesConstraint;
-}
 
-namespace jvalidate::schema {
-enum class Version : int;
-class Node;
+#define CONSTRAINT_IMPLEMENTATION_LIST(X)                                                          \
+  /* General Constraints - See jvalidate/constraint/general_constraint.h */                        \
+  X(TypeConstraint)                                                                                \
+  X(EnumConstraint)                                                                                \
+  X(AllOfConstraint)                                                                               \
+  X(AnyOfConstraint)                                                                               \
+  X(OneOfConstraint)                                                                               \
+  X(NotConstraint)                                                                                 \
+  X(ConditionalConstraint)                                                                         \
+  /* Number Constraints - See jvalidate/constraint/number_constraint.h */                          \
+  X(MaximumConstraint)                                                                             \
+  X(MinimumConstraint)                                                                             \
+  X(MultipleOfConstraint)                                                                          \
+  /* String Constraints - See jvalidate/constraint/string_constraint.h */                          \
+  X(MaxLengthConstraint)                                                                           \
+  X(MinLengthConstraint)                                                                           \
+  X(PatternConstraint)                                                                             \
+  X(FormatConstraint)                                                                              \
+  /* Array Constraints - See jvalidate/constraint/array_constraint.h */                            \
+  X(AdditionalItemsConstraint)                                                                     \
+  X(ContainsConstraint)                                                                            \
+  X(MaxItemsConstraint)                                                                            \
+  X(MinItemsConstraint)                                                                            \
+  X(TupleConstraint)                                                                               \
+  X(UnevaluatedItemsConstraint)                                                                    \
+  X(UniqueItemsConstraint)                                                                         \
+  /* Object Constraints - See jvalidate/constraint/object_constraint.h */                          \
+  X(AdditionalPropertiesConstraint)                                                                \
+  X(DependenciesConstraint)                                                                        \
+  X(MaxPropertiesConstraint)                                                                       \
+  X(MinPropertiesConstraint)                                                                       \
+  X(PatternPropertiesConstraint)                                                                   \
+  X(PropertiesConstraint)                                                                          \
+  X(PropertyNamesConstraint)                                                                       \
+  X(RequiredConstraint)                                                                            \
+  X(UnevaluatedPropertiesConstraint)                                                               \
+  /* ExtensionConstraint - A special constraint for all user-defined constraints */                \
+  X(ExtensionConstraint)
+
+#define FORWARD_DECLARE_CLASS(TYPE) class TYPE;
+CONSTRAINT_IMPLEMENTATION_LIST(FORWARD_DECLARE_CLASS);
+#undef FORWARD_DECLARE_CLASS
+
 }
 
 namespace jvalidate {
@@ -104,6 +114,16 @@ concept ObjectAdapter = requires(A const a) {
   { a.end() } -> ObjectIterator;
 };
 
+/**
+ * @brief A concept representing the actual adapter form. Used in most code
+ * that actually cares to interact with JSON, instead of having to define a
+ * template like AdapterInterface<ArrayAdapter, ObjectAdapter> for the basic
+ * adapter, or return pointers-to-interfaces in order to access array/object
+ * data.
+ *
+ * This concept functionally defines an Adapter being an AdapterInterface with
+ * an as_array and as_object method added on.
+ */
 template <typename A>
 concept Adapter = std::is_base_of_v<adapter::Adapter, A> && requires(A const a) {
   typename A::value_type;
@@ -124,6 +144,15 @@ concept MutableObject = ObjectAdapter<A> && requires(A const a) {
   { a.assign("", std::declval<adapter::Const>()) };
 };
 
+/**
+ * @brief An extension of Adapter that is capable of altering the underlying
+ * JSON node. In general, that means that a non-const ref is captured when
+ * building an Adapter.
+ *
+ * Requires that we can assign both to "this", and to values at a key (in the
+ * case of object type).
+ * Implies that as_array is also a wrapper for mutable JSON.
+ */
 template <typename A>
 concept MutableAdapter = Adapter<A> && requires(A const a) {
   { a.assign(std::declval<adapter::Const>()) };

include/jvalidate/validation_result.h

@@ -2,7 +2,6 @@
 
 #include <map>
 #include <ostream>
-#include <unordered_set>
 #include <utility>
 #include <variant>
 #include <vector>
@@ -13,10 +12,29 @@
 namespace jvalidate {
 class ValidationResult {
 public:
+  // Only allow ValidationVisitor to construct the elements of a validation result
   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>>;
+
+  /**
+   * @brief The result info at any given (DocPointer, SchemaPointer) path.
+   * The key for errors/annotations represents the leaf element of SchemaPointer
+   * instead of including it in the map.
+   *
+   * This allows better locality of error info. For example:
+   * {
+   *   "valid": false,
+   *   "evaluationPath": "/definitions/EvenPercent",
+   *   "instanceLocation": "/foo/bar/percentages/2",
+   *   "errors": {
+   *     "max": "105 > 100",
+   *     "multipleOf": "105 is not a multiple of 2"
+   *   }
+   * }
+   */
   struct LocalResult {
     bool valid;
     std::map<std::string, Annotation> errors;
@@ -40,6 +58,40 @@ private:
   std::map<DocPointer, std::map<SchemaPointer, LocalResult>> results_;
 
 public:
+  /**
+   * @brief Writes this object to an osteam in the list format as described in
+   * https://json-schema.org/blog/posts/interpreting-output
+   * This means that the json-schema for a ValidationResult looks like this:
+   * {
+   *   "$defs": {
+   *     "Pointer": {
+   *       "format": "json-pointer",
+   *       "type": "string"
+   *     },
+   *     "Annotation": {
+   *       "items": { "type": "string" },
+   *       "type": [ "string", "array" ]
+   *     }
+   *   },
+   *   "properties": {
+   *     "valid": { "type": "boolean" },
+   *     "details": {
+   *       "items": {
+   *         "properties": {
+   *           "valid": { "type": "boolean" },
+   *           "evaluationPath": { "$ref": "#/$defs/Pointer" },
+   *           "instanceLocation": { "$ref": "#/$defs/Pointer" },
+   *           "annotations": { "$ref": "#/$defs/Annotation" },
+   *           "errors": { "$ref": "#/$defs/Annotation" }
+   *         }
+   *         "type": "object"
+   *       },
+   *       "type": "array"
+   *     }
+   *   }
+   *   "type": "object"
+   * }
+   */
   friend std::ostream & operator<<(std::ostream & os, ValidationResult const & result) {
     char const * div = "\n";
     os << "{\n" << indent(1) << R"("valid": )" << (result.valid_ ? "true" : "false") << ',' << '\n';
@@ -82,12 +134,42 @@ public:
     os << indent(i) << '}';
   }
 
+  /**
+   * @brief Are there any validation details associated with the given document
+   * location and schema section.
+   *
+   * @param where A path into the document being validated
+   * @param schema_path The schema path (not counting the leaf element that
+   * actually evaluates the document)
+   *
+   * @return true if the schema path has produced an annotation or error for the
+   * document path
+   */
   bool has(detail::Pointer const & where, detail::Pointer const & schema_path) const {
     return has(where) && results_.at(where).contains(schema_path);
   }
 
+  /**
+   * @brief Are there any validation details associated with the given document
+   * location
+   *
+   * @param where A path into the document being validated
+   *
+   * @return true if any rule has produced an annotation or error for the
+   * document path
+   */
   bool has(detail::Pointer const & where) const { return results_.contains(where); }
 
+  /**
+   * @brief Extracts the annotation for requested document and schema location, if it exists
+   *
+   * @param where A path into the document being validated
+   * @param schema_path The schema path, without its leaf element
+   * @param name The leaf schema path (i.e. the rule being evaluated).
+   * @pre name.empty() == schema_path.empty()
+   *
+   * @returns An Annotation for the given path info provided, or nullptr if no annotation exists
+   */
   Annotation const * annotation(detail::Pointer const & where, detail::Pointer const & schema_path,
                                 std::string const & name) const {
     if (not results_.contains(where)) {
@@ -104,6 +186,16 @@ public:
     return &local.annotations.at(name);
   }
 
+  /**
+   * @brief Extracts the error for requested document and schema location, if it exists
+   *
+   * @param where A path into the document being validated
+   * @param schema_path The schema path, without its leaf element
+   * @param name The leaf schema path (i.e. the rule being evaluated).
+   * @pre name.empty() == schema_path.empty()
+   *
+   * @returns An Annotation for the given path info provided, or nullptr if no annotation exists
+   */
   Annotation const * error(detail::Pointer const & where, detail::Pointer const & schema_path,
                            std::string const & name) const {
     if (not results_.contains(where)) {
@@ -121,7 +213,13 @@ public:
   }
 
 private:
-  void merge(ValidationResult && result) {
+  /**
+   * @brief Transfer the contents of another ValidationResult into this one using
+   * {@see std::map::merge} to transfer the data minimizing the need for copy/move.
+   *
+   * @param result The ValidationResult being consumed
+   */
+  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);
@@ -130,6 +228,13 @@ private:
     }
   }
 
+  /**
+   * @brief Declare that the document is accepted/rejected by the given schema
+   *
+   * @param where A path into the document being validated
+   * @param schema_path The schema path
+   * @param valid Is this location valid according to the schema
+   */
   void valid(detail::Pointer const & where, detail::Pointer const & schema_path, bool valid) {
     if (has(where, schema_path)) {
       results_[where][schema_path].valid = valid;
@@ -139,6 +244,18 @@ private:
     }
   }
 
+  /**
+   * @brief Attach an error message for part of the document.
+   * Because of the existance of things like "not" schemas, error() can also be
+   * called to add an Annotation for a gate that is passed, but was within a
+   * "not" schema.
+   *
+   * @param where A path into the document being validated
+   * @param schema_path The schema path, without its leaf element
+   * @param name The leaf schema path (i.e. the rule being evaluated).
+   * @pre name.empty() == schema_path.empty()
+   * @param message The annotation(s) being placed as an error
+   */
   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)) {
@@ -147,6 +264,15 @@ private:
     results_[where][schema_path].errors.emplace(name, std::move(message));
   }
 
+  /**
+   * @brief Attach some contextual annotations for part of the document
+   *
+   * @param where A path into the document being validated
+   * @param schema_path The schema path, without its leaf element
+   * @param name The leaf schema path (i.e. the rule being evaluated).
+   * @pre name.empty() == schema_path.empty()
+   * @param message The annotation(s) being placed for context
+   */
   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)) {

include/jvalidate/validation_visitor.h

@@ -652,8 +652,7 @@ private:
   }
 
   template <typename... K>
-  Status validate_subschema(constraint::Constraint::SubConstraint const & subschema,
-                            K const &... keys) const {
+  Status validate_subschema(constraint::SubConstraint const & subschema, K const &... keys) const {
     if (schema::Node const * const * ppschema = std::get_if<0>(&subschema)) {
       return validate_subschema(*ppschema, keys...);
     } else {