fable: Change validate to a non-throwing interface

BREAKING CHANGE: Previous use of `Schema::validate(const Conf&)` should now use
`Schema::validate_or_throw(const Conf&)`.

BREAKING CHANGE: Custom schemas should change their implementation
of `validate(const Conf&)` to `bool validate(const Conf&, std::optional<SchemaError>`.

See the changes in this commit to see how the built-in schemas are
modified for examples of how to change your implementation.

Author: cassava

engine/src/main_stack.cpp

@@ -69,7 +69,7 @@ void merge_stack(const StackOptions& opt, Stack& s, const std::string& filepath)
     }
 
     s.from_conf(c);
-    s.validate();
+    s.validate_self();
   };
 
   if (!opt.error) {

engine/src/stack.cpp

@@ -604,25 +604,40 @@ void Stack::from_conf(const Conf& _conf, size_t depth) {
   }
 
   // Apply everything else.
-  this->schema().validate(c);
+  this->schema().validate_or_throw(c);
   this->schema().from_conf(c);
   this->reset_schema();
 }
 
-void Stack::validate(const Conf& c) const {
+void Stack::validate_self() const {
+  check_consistency();
+  check_defaults();
+}
+
+void Stack::validate_or_throw(const Conf& c) const {
   Stack copy(*this);
   copy.from_conf(c);
-  copy.validate();
+  copy.validate_self();
 }
 
-void Stack::validate() const {
-  check_consistency();
-  check_defaults();
+bool Stack::validate(const Conf &c, std::optional<SchemaError> &err) const {
+  Stack copy(*this);
+  try {
+    copy.from_conf(c);
+    copy.validate_self();
+  } catch (fable::SchemaError& e) {
+    err.emplace(e);
+    return false;
+  } catch (Error& e) {
+    err.emplace(SchemaError(c, schema().json_schema(), "{}", e.what()));
+    return false;
+  }
+  return true;
 }
 
 bool Stack::is_valid() const {
   try {
-    validate();
+    validate_self();
     return true;
   } catch (...) {
     // TODO(ben): Replace ... with specific error classes

engine/src/stack.hpp

@@ -178,13 +178,17 @@ struct LoggingConf : public Confable {
     };
   }
 
-  void validate(const Conf& c) const override {
+  bool validate(const Conf& c, std::optional<SchemaError>& err) const override {
     const auto& s = this->schema();
-    s.validate(c);
+    if (!s.validate(c, err)) {
+      return false;
+    }
     if (!c.has("pattern") && !c.has("level")) {
-      throw SchemaError(c, s.json_schema(),
-                        "require at least one of 'pattern' or 'level' properties");
+      err.emplace(SchemaError(c, s.json_schema(),
+                              "require at least one of 'pattern' or 'level' properties"));
+      return false;
     }
+    return true;
   }
 };
 
@@ -756,9 +760,9 @@ class VehicleSchema : public fable::schema::Base<VehicleSchema> {
     return v.schema().json_schema();
   }
 
-  void validate(const Conf& c) const override {
+  bool validate(const Conf& c, std::optional<SchemaError>& err) const override {
     VehicleConf v{components_};
-    v.schema().validate(c);
+    return v.schema().validate(c, err);
   }
 
   Json serialize(const Type& x) const { return x.to_json(); }
@@ -1018,7 +1022,7 @@ class Stack : public Confable {
    * consistency and correctness (but not necessarily completeness, except for
    * any references made) of the entire configuration.
    */
-  void validate() const;
+  void validate_self() const;
 
   /**
    * Return true if this configuration would be valid.
@@ -1081,13 +1085,14 @@ class Stack : public Confable {
   /**
    * Validate a configuration.
    *
-   * This cannot normally be done with `schema().validate(c)`, because plugins
+   * This cannot normally be done with `schema().validate()`, because plugins
    * needed to be loaded in order to validate sections of the schema. This
    * requires partial application of the schema.
    *
-   * This should be equivalent to from_conf() followed by validate().
+   * This should be equivalent to from_conf() followed by validate_self().
    */
-  void validate(const Conf& c) const override;
+  bool validate(const Conf& c, std::optional<SchemaError>& err) const override;
+  void validate_or_throw(const Conf& c) const override;
 
   void to_json(Json& j) const override;
   void from_conf(const Conf& c) override { from_conf(c, 0); }

fable/include/fable/confable.hpp

@@ -79,14 +79,32 @@ class Confable {
   /**
    * Validate a Conf without applying it.
    *
+   * By default, this uses the validate_or_throw() method on schema(). If you want to
+   * guarantee anything extending beyond what's possible with schema, you can
+   * do that here.
+   *
+   * This method should NOT call from_conf without also overriding from_conf
+   * to prevent infinite recursion.
+   */
+  virtual void validate_or_throw(const Conf& c) const;
+
+  /**
+   * Validate a Conf without applying it and without throwing.
+   *
    * By default, this uses the validate() method on schema(). If you want to
    * guarantee anything extending beyond what's possible with schema, you can
    * do that here.
    *
-   * This methodshould NOT call from_conf without also overriding from_conf
+   * This method should NOT call from_conf without also overriding from_conf
    * to prevent infinite recursion.
+   *
+   * This method should not reset err unless the method returns false.
+   *
+   * \param c JSON to validate
+   * \param err reference to store error in
+   * \return true if validate successful
    */
-  virtual void validate(const Conf& c) const;
+  virtual bool validate(const Conf& c, std::optional<SchemaError>& err) const;
 
   /**
    * Deserialize a Confable from a Conf.

fable/include/fable/schema.hpp

@@ -254,7 +254,7 @@ class Schema : public schema::Interface {
   void set_description(std::string s) override { return impl_->set_description(std::move(s)); }
   Json usage() const override { return impl_->usage(); }
   Json json_schema() const override { return impl_->json_schema(); }
-  void validate(const Conf& c) const override { impl_->validate(c); }
+  bool validate(const Conf& c, std::optional<SchemaError>& err) const override { return impl_->validate(c, err); }
   void to_json(Json& j) const override { impl_->to_json(j); }
   void from_conf(const Conf& c) override { impl_->from_conf(c); }
   void reset_ptr() override { impl_->reset_ptr(); }

fable/include/fable/schema/array.hpp

@@ -169,19 +169,23 @@ class Array : public Base<Array<T, N, P>> {
     return j;
   }
 
-  void validate(const Conf& c) const override {
+  bool validate(const Conf& c, std::optional<SchemaError>& err) const override {
     if (option_require_all_) {
-      this->validate_type(c);
-      this->validate_array(c);
-      return;
+      if (!this->validate_type(c, err)) {
+        return false;
+      }
+      if (!this->validate_array(c, err)) {
+        return false;
+      }
+      return true;
     }
 
     if (c->type() == JsonType::array) {
-      this->validate_array(c);
+      return this->validate_array(c, err);
     } else if (c->type() == JsonType::object) {
-      this->validate_object(c);
+      return this->validate_object(c, err);
     } else {
-      this->throw_wrong_type(c);
+      return this->set_wrong_type(err, c);
     }
   }
 
@@ -233,7 +237,7 @@ class Array : public Base<Array<T, N, P>> {
     } else if (c->type() == JsonType::object) {
       this->deserialize_from_object(v, c);
     } else {
-      this->throw_wrong_type(c);
+      throw this->wrong_type(c);
     }
   }
 
@@ -273,14 +277,17 @@ class Array : public Base<Array<T, N, P>> {
    * That is, all elements in the array must be set, no less and
    * no more.
    */
-  void validate_array(const Conf& c) const {
+  bool validate_array(const Conf& c, std::optional<SchemaError>& err) const {
     assert(c->type() == JsonType::array);
     if (c->size() != N) {
-      this->throw_error(c, "require exactly {} items in array, got {}", N, c->size());
+      return this->set_error(err, c, "require exactly {} items in array, got {}", N, c->size());
     }
     for (const auto& x : c.to_array()) {
-      prototype_.validate(x);
+      if (!prototype_.validate(x, err)) {
+        return false;
+      }
     }
+    return true;
   }
 
   /**
@@ -304,13 +311,16 @@ class Array : public Base<Array<T, N, P>> {
    * less than the size N of the array. Otherwise an error is thrown.
    * More than one index can be set at once. Unset indexes are left as is.
    */
-  void validate_object(const Conf& c) const {
+  bool validate_object(const Conf& c, std::optional<SchemaError>& err) const {
     assert(c->type() == JsonType::object);
     for (const auto& kv : c->items()) {
       const auto& key = kv.key();
       this->parse_index(c, key);
-      prototype_.validate(c.at(key));
+      if (prototype_.validate(c.at(key), err)) {
+        return false;
+      }
     }
+    return true;
   }
 
   /**
@@ -329,25 +339,25 @@ class Array : public Base<Array<T, N, P>> {
     // We'd like to just be able to use std::stoul, but unfortunately
     // the standard library seems to think strings like "234x" are ok.
     if (s.empty()) {
-      this->throw_error(c, "invalid index key in object, require integer, got ''");
+      throw this->error(c, "invalid index key in object, require integer, got ''");
     } else if (s.size() > 1 && s[0] == '0') {
-      this->throw_error(c, "invalid index key in object, require base-10 value, got '{}'", s);
+      throw this->error(c, "invalid index key in object, require base-10 value, got '{}'", s);
     }
     for (char ch : s) {
       if (ch < '0' || ch > '9') {
-        this->throw_error(c, "invalid index key in object, require integer, got '{}'", s);
+        throw this->error(c, "invalid index key in object, require integer, got '{}'", s);
       }
     }
     size_t idx = std::stoul(s);
     if (idx >= N) {
-      this->throw_error(c, "out-of-range index key in object, require < {}, got '{}'", N, s);
+      throw this->error(c, "out-of-range index key in object, require < {}, got '{}'", N, s);
     }
     return idx;
   }
 
-  [[noreturn]] void throw_wrong_type(const Conf& c) const {
+  [[nodiscard]] SchemaError wrong_type(const Conf& c) const {
     std::string got = to_string(c->type());
-    this->throw_error(c, "property must have type array or object, got {}", got);
+    return this->error(c, "property must have type array or object, got {}", got);
   }
 
   void deserialize_from_object(Type& array, const Conf& c) const {

fable/include/fable/schema/boolean.hpp

@@ -39,7 +39,7 @@ class Boolean : public Base<Boolean> {
 
  public:  // Overrides
   Json json_schema() const override;
-  void validate(const Conf& c) const override;
+  bool validate(const Conf& c, std::optional<SchemaError>& err) const override;
   using Interface::to_json;
   void to_json(Json& j) const override;
   void from_conf(const Conf& c) override;

fable/include/fable/schema/confable.hpp

@@ -63,11 +63,11 @@ class FromConfable : public Base<FromConfable<T>> {
     return j;
   }
 
-  void validate(const Conf& c) const override {
+  bool validate(const Conf& c, std::optional<SchemaError>& err) const override {
     if (ptr_ == nullptr) {
-      schema_.validate(c);
+      return schema_.validate(c, err);
     } else {
-      ptr_->validate(c);
+      return ptr_->validate(c, err);
     }
   }
 

fable/include/fable/schema/const.hpp

@@ -57,29 +57,30 @@ class Const : public Base<Const<T, P>> {
     return j;
   }
 
-  void validate(const Conf& c) const override {
+  bool validate(const Conf& c, std::optional<SchemaError>& err) const override {
     Type tmp = prototype_.deserialize(c);
     if (tmp != constant_) {
-      this->throw_error(c, "expected const value {}, got {}", constant_, tmp);
+      return this->set_error(err, c, "expected const value {}, got {}", constant_, tmp);
     }
+    return true;
   }
 
   using Interface::to_json;
   void to_json(Json& j) const override { j = serialize(constant_); }
 
-  void from_conf(const Conf& c) override { validate(c); }
+  void from_conf(const Conf& c) override { this->validate_or_throw(c); }
 
   Json serialize(const Type& x) const { return prototype_.serialize(x); }
 
   Type deserialize(const Conf& c) const {
-    validate(c);
+    this->validate_or_throw(c);
     return constant_;
   }
 
   void serialize_into(Json& j, const Type& x) const { prototype_.serialize_into(j, x); }
 
   void deserialize_into(const Conf& c, Type& x) const {
-    validate(c);
+    this->validate_or_throw(c);
     x = constant_;
   }
 

fable/include/fable/schema/custom.hpp

@@ -65,7 +65,7 @@ class CustomDeserializer : public schema::Interface {
   void set_description(std::string s) override { return impl_->set_description(std::move(s)); }
   Json usage() const override { return impl_->usage(); }
   Json json_schema() const override { return impl_->json_schema(); };
-  void validate(const Conf& c) const override { impl_->validate(c); }
+  bool validate(const Conf& c, std::optional<SchemaError>& err) const override { return impl_->validate(c, err); }
   void to_json(Json& j) const override { impl_->to_json(j); }
 
   void from_conf(const Conf& c) override {