Преглед на файлове

refactor: propogate expected handling to load_*, and test cases

Sam Jaffe преди 1 седмица
родител
ревизия
a09eb4867d

+ 5 - 4
include/jvalidate/adapter.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "jvalidate/compat/expected.h"
 #include <cstdint>
 #include <filesystem>
 #include <fstream>
@@ -386,13 +387,13 @@ public:
 };
 
 template <typename JSON>
-bool load_file(std::filesystem::path const & path, JSON & out, std::string & error) noexcept {
+auto load_file(std::filesystem::path const & path, JSON & out) noexcept
+    -> detail::expected<void, std::string> {
   std::ifstream in(path);
   if (in.bad()) {
-    error = "file error";
-    return false;
+    return detail::unexpected("file error");
   }
-  return load_stream(in, out, error);
+  return load_stream(in, out);
 }
 }
 

+ 7 - 2
include/jvalidate/adapters/jsoncpp.h

@@ -5,6 +5,7 @@
 #include <json/value.h>
 
 #include <jvalidate/adapter.h>
+#include <jvalidate/compat/expected.h>
 #include <jvalidate/detail/expect.h>
 #include <jvalidate/detail/number.h>
 #include <jvalidate/detail/simple_adapter.h>
@@ -12,9 +13,13 @@
 
 namespace jvalidate::adapter {
 template <>
-inline bool load_stream(std::istream & in, Json::Value & out, std::string & error) noexcept {
+inline auto load_stream(std::istream & in, Json::Value & out) noexcept
+    -> detail::expected<void, std::string> {
   Json::CharReaderBuilder builder;
-  return Json::parseFromStream(builder, in, &out, &error);
+  if (std::string error; not Json::parseFromStream(builder, in, &out, &error)) {
+    return detail::unexpected(error);
+  }
+  return {};
 }
 
 template <typename JSON> class JsonCppAdapter;

+ 7 - 6
include/jvalidate/compat/curl.h

@@ -5,6 +5,7 @@
 #include <curl/curl.h>
 
 #include <jvalidate/adapter.h>
+#include <jvalidate/compat/expected.h>
 #include <jvalidate/forward.h>
 #include <jvalidate/uri.h>
 
@@ -17,7 +18,8 @@ inline size_t transfer_to_buffer(char * data, size_t size, size_t nmemb, void *
 }
 
 template <typename JSON>
-bool curl_get(jvalidate::URI const & uri, JSON & out, std::string & error) noexcept {
+auto curl_get(jvalidate::URI const & uri, JSON & out) noexcept
+    -> detail::expected<void, std::string> {
   using jvalidate::adapter::load_file;
   using jvalidate::adapter::load_stream;
   if (uri.scheme().starts_with("http")) {
@@ -32,15 +34,14 @@ bool curl_get(jvalidate::URI const & uri, JSON & out, std::string & error) noexc
       curl_easy_cleanup(curl);
 
       if (res == CURLE_OK) {
-        return load_stream(ss, out, error);
+        return load_stream(ss, out);
       }
     }
-    return false;
+    return detail::unexpected("curl error");
   } else if (uri.scheme() == "file") {
-    return load_file(uri.resource(), out, error);
+    return load_file(uri.resource(), out);
   } else {
-    error = "unknown scheme";
-    return false;
+    return detail::unexpected("unknown scheme");
   }
 }
 }

+ 2 - 4
include/jvalidate/compat/expected.h

@@ -68,8 +68,6 @@ template <typename E> class bad_expected_access : public bad_expected_access<voi
 public:
   bad_expected_access(E error) : error_(std::move(error)) {}
 
-  char const * what() const noexcept override { return error_.c_str(); }
-
   constexpr const E & error() const & noexcept { return error_; }
   constexpr E & error() & noexcept { return error_; }
   constexpr E && error() && noexcept { return std::move(error_); }
@@ -250,11 +248,11 @@ public:
 
   template <typename G>
   constexpr explicit(!std::is_convertible_v<const G &, E>) expected(unexpected<G> const & e)
-      : value_(std::in_place_index<1>, e.error()) {}
+      : value_(e.error()) {}
 
   template <typename G>
   constexpr explicit(!std::is_convertible_v<G, E>) expected(unexpected<G> && e)
-      : value_(std::in_place_index<1>, std::move(e).error()) {}
+      : value_(std::move(e).error()) {}
 
   constexpr explicit expected(std::in_place_t) {}
 

+ 1 - 1
include/jvalidate/document_cache.h

@@ -65,7 +65,7 @@ public:
     }
 
     auto [it, created] = cache_.try_emplace(uri);
-    if (created && not resolve_(uri, it->second, error)) {
+    if (created && not resolve_(uri, it->second)) {
       // Doing it this way skips out on a move operation for the JSON object,
       // which could be useful if someone is using a legacy JSON object type.
       // Since std::map promises stability we don't need to concern ourselves

+ 8 - 2
include/jvalidate/forward.h

@@ -8,6 +8,8 @@
 #include <type_traits>
 #include <variant>
 
+#include <jvalidate/compat/expected.h>
+
 #define DISCARD1_IMPL(_, ...) __VA_ARGS__
 #define DISCARD1(...) DISCARD1_IMPL(__VA_ARGS__)
 
@@ -17,6 +19,9 @@
 
 namespace jvalidate::detail {
 }
+namespace jvalidate::adapter::detail {
+using namespace jvalidate::detail;
+}
 namespace jvalidate::format::detail {
 using namespace jvalidate::detail;
 }
@@ -42,7 +47,7 @@ template <typename V> struct AdapterTraits<V const> : AdapterTraits<V> {};
 
 template <typename JSON> using AdapterFor = typename AdapterTraits<JSON>::template Adapter<JSON>;
 template <typename JSON>
-bool load_stream(std::istream & in, JSON & out, std::string & error) noexcept;
+auto load_stream(std::istream & in, JSON & out) noexcept -> detail::expected<void, std::string>;
 }
 
 namespace jvalidate::schema {
@@ -202,7 +207,8 @@ template <Adapter Root, RegexEngine RE, typename ExtensionVisitor> class Validat
 template <RegexEngine RE, typename ExtensionVisitor> class Validator;
 
 template <Adapter A>
-using URIResolver = bool (*)(URI const &, typename A::value_type &, std::string &) noexcept;
+using URIResolver = auto (*)(URI const &, typename A::value_type &) noexcept
+    -> detail::expected<void, std::string>;
 }
 
 namespace jvalidate::extension {

+ 5 - 6
src/validate.cxx

@@ -61,12 +61,12 @@ int main(int argc, char const * const * argv) {
 
   Json::Value jschema;
   Json::Value jobject;
-  if (std::string error; !load_file(args.schema, jschema, error)) {
-    std::cerr << "Error loading schema: " << error << std::endl;
+  if (auto r = load_file(args.schema, jschema); not r) {
+    std::cerr << "Error loading schema: " << r.error() << std::endl;
     return EXIT_FAILURE;
   }
-  if (std::string error; !load_file(args.object, jobject, error)) {
-    std::cerr << "Error loading schema: " << error << std::endl;
+  if (auto r = load_file(args.object, jobject); not r) {
+    std::cerr << "Error loading schema: " << r.error() << std::endl;
     return EXIT_FAILURE;
   }
 
@@ -90,8 +90,7 @@ int main(int argc, char const * const * argv) {
   {
     std::stringstream ss;
     ss << result;
-    std::string _;
-    load_stream(ss, json, _);
+    load_stream(ss, json);
   }
 
   std::map<std::string, Json::Value> because;

+ 26 - 34
tests/detail_test.cxx

@@ -21,8 +21,8 @@ using JsonCppAdapter = jvalidate::adapter::JsonCppAdapter<Json::Value const>;
 using jvalidate::adapter::Type;
 
 using testing::Eq;
+using testing::HasSubstr;
 using testing::Test;
-using testing::ThrowsMessage;
 using testing::Types;
 using testing::VariantWith;
 
@@ -50,65 +50,58 @@ TEST(PointerTest, BackCoercesIntToString) {
 TEST(PointerTest, BackIsEmptySafe) { EXPECT_THAT(""_jptr.back(), ""); }
 
 TEST(PointerTest, ForbidsBadTilde) {
-  EXPECT_NO_THROW("/~1"_jptr);
-  EXPECT_THROW("/~2"_jptr, std::runtime_error);
+  EXPECT_THAT(Pointer::parse("/~1"), Expected(testing::_));
+  EXPECT_THAT(Pointer::parse("/~2"), Unexpected(HasSubstr("illegal tilde")));
 }
 
 TEST(PointerTest, CanConcatenate) { EXPECT_THAT("/A"_jptr / "/B"_jptr, "/A/B"_jptr); }
 
 TEST(PointerTest, CanGoToParent) { EXPECT_THAT("/A/B"_jptr / parent, "/A"_jptr); }
 
-TEST(PointerTest, Print) { EXPECT_THAT(testing::PrintToString(Pointer("/B/0/A")), "/B/0/A"); }
+TEST(PointerTest, Print) { EXPECT_THAT(testing::PrintToString("/B/0/A"_jptr), "/B/0/A"); }
 
 TEST(RelatvivePointerTest, CannotPrefixWithZero) {
-  EXPECT_THROW(RelativePointer("01"), std::runtime_error);
+  EXPECT_THAT(RelativePointer::parse("01"), Unexpected("Cannot zero-prefix a relative pointer"));
 }
 
 TEST(RelatvivePointerTest, Print) {
-  EXPECT_THAT(testing::PrintToString(RelativePointer("0")), "0");
-  EXPECT_THAT(testing::PrintToString(RelativePointer("1#")), "1#");
-  EXPECT_THAT(testing::PrintToString(RelativePointer("1/B/0/A")), "1/B/0/A");
+  EXPECT_THAT(testing::PrintToString("0"_relptr), "0");
+  EXPECT_THAT(testing::PrintToString("1#"_relptr), "1#");
+  EXPECT_THAT(testing::PrintToString("1/B/0/A"_relptr), "1/B/0/A");
 }
 
 TEST(RelatvivePointerTest, ZeroIsHere) {
   Json::Value json;
   json["A"] = 1;
-  RelativePointer const rel("0");
-  EXPECT_THAT(rel.inspect("/A"_jptr, JsonCppAdapter(json)),
-              VariantWith<JsonCppAdapter>(JsonCppAdapter(json["A"])))
-      << rel;
+  EXPECT_THAT("0"_relptr.inspect("/A"_jptr, JsonCppAdapter(json)),
+              VariantWith<JsonCppAdapter>(JsonCppAdapter(json["A"])));
 }
 
 TEST(RelatvivePointerTest, CanWalkBackwards) {
   Json::Value json;
   json["A"] = 1;
-  RelativePointer const rel("1");
-  EXPECT_THAT(rel.inspect("/A"_jptr, JsonCppAdapter(json)),
-              VariantWith<JsonCppAdapter>(JsonCppAdapter(json)))
-      << rel;
+  EXPECT_THAT("1"_relptr.inspect("/A"_jptr, JsonCppAdapter(json)),
+              VariantWith<JsonCppAdapter>(JsonCppAdapter(json)));
 }
 
 TEST(RelatvivePointerTest, CanFetchKey) {
   Json::Value json;
   json["A"] = 1;
-  RelativePointer const rel("0#");
-  EXPECT_THAT(rel.inspect("/A"_jptr, JsonCppAdapter(json)), VariantWith<std::string>(Eq("A")))
-      << rel;
+  EXPECT_THAT("0#"_relptr.inspect("/A"_jptr, JsonCppAdapter(json)),
+              VariantWith<std::string>(Eq("A")));
 }
 
 TEST(RelatvivePointerTest, CanGoUpAndDown) {
   Json::Value json;
   json["A"] = 1;
   json["B"] = 2;
-  RelativePointer const rel("1/B");
-  EXPECT_THAT(rel.inspect("/A"_jptr, JsonCppAdapter(json)),
-              VariantWith<JsonCppAdapter>(JsonCppAdapter(json["B"])))
-      << rel;
+  EXPECT_THAT("1/B"_relptr.inspect("/A"_jptr, JsonCppAdapter(json)),
+              VariantWith<JsonCppAdapter>(JsonCppAdapter(json["B"])));
 }
 
 TEST(ReferenceTest, Print) {
   EXPECT_THAT(testing::PrintToString(Reference(jvalidate::URI("file://path/to/document.json"),
-                                               Anchor("Anchor"), Pointer("/key/1/id"))),
+                                               Anchor("Anchor"), "/key/1/id"_jptr)),
               "file://path/to/document.json#Anchor/key/1/id");
 }
 
@@ -149,30 +142,29 @@ TEST(NumberTest, DoubleOutOfIntegerRange) {
   EXPECT_FALSE(fits_in_integer(-10000000000000000000.0));
 }
 
-TYPED_TEST(NumberFromStrTest, NumberParsesIntegers) { EXPECT_THAT(from_str<TypeParam>("10"), 10); }
+TYPED_TEST(NumberFromStrTest, NumberParsesIntegers) {
+  EXPECT_THAT(parse_integer<TypeParam>("10"), Expected(10));
+}
 
 TYPED_TEST(NumberFromStrTest, NumberParsesNegativeIntegers) {
   if constexpr (std::is_signed_v<TypeParam>) {
-    EXPECT_THAT(from_str<TypeParam>("-10"), -10);
+    EXPECT_THAT(parse_integer<TypeParam>("-10"), Expected(-10));
   } else {
-    EXPECT_THAT([] { from_str<TypeParam>("-10"); },
-                ThrowsMessage<std::runtime_error>("Invalid argument"));
+    EXPECT_THAT(parse_integer<TypeParam>("-10"), Unexpected(std::errc::invalid_argument));
   }
 }
 
 TYPED_TEST(NumberFromStrTest, ThrowsOnPlusSign) {
-  EXPECT_THAT([] { from_str<TypeParam>("+10"); },
-              ThrowsMessage<std::runtime_error>("Invalid argument"));
+  EXPECT_THAT(parse_integer<TypeParam>("+10"), Unexpected(std::errc::invalid_argument));
 }
 
 TYPED_TEST(NumberFromStrTest, ThrowsOnTooManyChars) {
-  EXPECT_THAT([] { from_str<TypeParam>("10 coconuts"); },
-              ThrowsMessage<std::runtime_error>("NaN: 10 coconuts"));
+  EXPECT_THAT(parse_integer<TypeParam>("10 coconuts"), Unexpected(std::errc::result_out_of_range));
 }
 
 TYPED_TEST(NumberFromStrTest, ThrowsOnOutOfRange) {
-  EXPECT_THAT([] { from_str<TypeParam>("99999999999999999999999999999"); },
-              ThrowsMessage<std::runtime_error>("Result too large"));
+  EXPECT_THAT(parse_integer<TypeParam>("99999999999999999999999999999"),
+              Unexpected(std::errc::result_out_of_range));
 }
 
 TEST(StringAdapterTest, IsStringy) {

+ 19 - 0
tests/matchers.h

@@ -3,6 +3,7 @@
 #include <gtest/gtest.h>
 
 #include <jvalidate/detail/pointer.h>
+#include <jvalidate/detail/relative_pointer.h>
 #include <jvalidate/validation_result.h>
 
 #include <json/reader.h>
@@ -12,6 +13,10 @@ inline auto operator""_jptr(char const * data, size_t len) {
   return jvalidate::detail::Pointer::parse(std::string_view{data, len}).value();
 }
 
+inline auto operator""_relptr(char const * data, size_t len) {
+  return jvalidate::detail::RelativePointer::parse(std::string_view{data, len}).value();
+}
+
 inline Json::Value const operator""_json(char const * data, size_t len) {
   Json::Value value;
 
@@ -26,6 +31,20 @@ inline Json::Value const operator""_json(char const * data, size_t len) {
   return value;
 }
 
+MATCHER_P(Expected, m, "") {
+  if (arg.has_value()) {
+    return testing::ExplainMatchResult(m, arg.value(), result_listener);
+  }
+  return false;
+}
+
+MATCHER_P(Unexpected, m, "") {
+  if (not arg.has_value()) {
+    return testing::ExplainMatchResult(m, arg.error(), result_listener);
+  }
+  return false;
+}
+
 MATCHER(Valid, "") { return arg.valid(); }
 
 MATCHER_P(HasAnnotationsFor, doc_path, "") { return arg.has(doc_path); }

+ 4 - 6
tests/selfvalidate_test.cxx

@@ -28,14 +28,13 @@ using testing::TestWithParam;
 
 using jvalidate::adapter::load_file;
 
-bool load_external_for_test(jvalidate::URI const & uri, Json::Value & out,
-                            std::string & error) noexcept {
+auto load_external_for_test(jvalidate::URI const & uri, Json::Value & out) noexcept {
   constexpr std::string_view g_fake_url = "localhost:1234/";
   if (uri.scheme().starts_with("http") && uri.resource().starts_with(g_fake_url)) {
     std::string_view path = uri.resource().substr(g_fake_url.size());
-    return load_file(JSONSchemaTestSuiteDir() / "remotes" / path, out, error);
+    return load_file(JSONSchemaTestSuiteDir() / "remotes" / path, out);
   }
-  return jvalidate::curl_get(uri, out, error);
+  return jvalidate::curl_get(uri, out);
 }
 
 class JsonSchemaTest : public TestWithParam<SchemaParams> {
@@ -98,8 +97,7 @@ TEST_P(JsonSchemaTest, TestSuite) {
   auto const & [version, file] = GetParam();
   Json::Value spec;
 
-  std::string error;
-  EXPECT_TRUE(load_file(file, spec, error)) << error;
+  EXPECT_TRUE(load_file(file, spec));
 
   bool is_format = file.string().find("optional/format") != std::string::npos;
   for (auto const & suite : spec) {