Eliminate ParseError.

Another extraction from microsoft#1210 ; this means that ExpectedL is the only ExpectedT that exists.
BillyONeal · Jan 2, 2024 · 6ae7db8 · 6ae7db8
1 parent bfda089
commit 6ae7db8
Show file tree

Hide file tree

Showing 12 changed files with 41 additions and 70 deletions.
diff --git a/include/vcpkg/base/json.h b/include/vcpkg/base/json.h
@@ -327,10 +327,8 @@ namespace vcpkg::Json
         JsonStyle style;
     };
 
-    ExpectedT<ParsedJson, std::unique_ptr<ParseError>> parse_file(const ReadOnlyFilesystem&,
-                                                                  const Path&,
-                                                                  std::error_code& ec);
-    ExpectedT<ParsedJson, std::unique_ptr<ParseError>> parse(StringView text, StringView origin);
+    ExpectedL<ParsedJson> parse_file(const ReadOnlyFilesystem&, const Path&, std::error_code& ec);
+    ExpectedL<ParsedJson> parse(StringView text, StringView origin);
     ParsedJson parse_file(LineInfo li, const ReadOnlyFilesystem&, const Path&);
     ExpectedL<Json::Object> parse_object(StringView text, StringView origin);
 

diff --git a/include/vcpkg/base/parse.h b/include/vcpkg/base/parse.h
@@ -35,15 +35,6 @@ namespace vcpkg
 
         std::string to_string() const;
     };
-
-    constexpr struct ParseErrorFormatter
-    {
-        LocalizedString operator()(const std::unique_ptr<ParseError>& up)
-        {
-            return LocalizedString::from_raw(up->to_string());
-        }
-    } parse_error_formatter;
-
 } // namespace vcpkg
 
 namespace vcpkg
@@ -142,7 +133,6 @@ namespace vcpkg
         void add_warning(LocalizedString&& message, const SourceLoc& loc);
 
         const ParseError* get_error() const { return m_messages.error.get(); }
-        std::unique_ptr<ParseError> extract_error() { return std::move(m_messages.error); }
 
         const ParseMessages& messages() const { return m_messages; }
         ParseMessages&& extract_messages() { return std::move(m_messages); }

diff --git a/src/vcpkg-fuzz/main.cpp b/src/vcpkg-fuzz/main.cpp
@@ -137,7 +137,7 @@ namespace
         auto res = Json::parse(text, origin);
         if (!res)
         {
-            Checks::exit_with_message(VCPKG_LINE_INFO, res.error()->to_string());
+            Checks::msg_exit_with_message(VCPKG_LINE_INFO, res.error());
         }
 
         Checks::exit_success(VCPKG_LINE_INFO);

diff --git a/src/vcpkg-test/json.cpp b/src/vcpkg-test/json.cpp
@@ -1,6 +1,7 @@
 #include <vcpkg-test/util.h>
 
 #include <vcpkg/base/json.h>
+#include <vcpkg/base/messages.h>
 
 #include <iostream>
 
@@ -18,7 +19,7 @@ static auto u8_string_to_char_string(const char8_t (&literal)[Sz]) -> const char
 #define U8_STR(s) (u8"" s)
 #endif
 
-namespace Json = vcpkg::Json;
+using namespace vcpkg;
 using Json::Value;
 
 TEST_CASE ("JSON stringify weird strings", "[json]")
@@ -55,9 +56,9 @@ TEST_CASE ("JSON parse strings", "[json]")
     REQUIRE(res.get()->value.is_string());
     REQUIRE(res.get()->value.string(VCPKG_LINE_INFO) == "\xED\xA0\x80");
 
-    const auto make_json_string = [](vcpkg::StringView sv) { return '"' + sv.to_string() + '"'; };
-    const vcpkg::StringView radical = U8_STR("⎷");
-    const vcpkg::StringView grin = U8_STR("😁");
+    const auto make_json_string = [](StringView sv) { return '"' + sv.to_string() + '"'; };
+    const StringView radical = U8_STR("⎷");
+    const StringView grin = U8_STR("😁");
 
     res = Json::parse(R"("\uD83D\uDE01")", "test"); // paired surrogates for grin
     REQUIRE(res);
@@ -212,14 +213,14 @@ TEST_CASE ("JSON parse objects", "[json]")
 
 TEST_CASE ("JSON parse full file", "[json]")
 {
-    vcpkg::StringView json =
+    StringView json =
 #include "large-json-document.json.inc"
         ;
 
     auto res = Json::parse(json, "test");
     if (!res)
     {
-        std::cerr << res.error()->to_string() << '\n';
+        std::cerr << res.error() << '\n';
     }
     REQUIRE(res);
 }
@@ -228,47 +229,43 @@ TEST_CASE ("JSON track newlines", "[json]")
 {
     auto res = Json::parse("{\n,", "filename");
     REQUIRE(!res);
-    REQUIRE(res.error()->to_string() ==
-            R"(filename:2:1: error: Unexpected character; expected property name
+    REQUIRE(res.error() ==
+            LocalizedString::from_raw(R"(filename:2:1: error: Unexpected character; expected property name
   on expression: ,
-                 ^)");
+                 ^)"));
 }
 
 TEST_CASE ("JSON duplicated object keys", "[json]")
 {
     auto res = Json::parse("{\"name\": 1, \"name\": 2}", "filename");
     REQUIRE(!res);
-    REQUIRE(res.error()->to_string() ==
-            R"(filename:1:13: error: Duplicated key "name" in an object
+    REQUIRE(res.error() == LocalizedString::from_raw(R"(filename:1:13: error: Duplicated key "name" in an object
   on expression: {"name": 1, "name": 2}
-                             ^)");
+                             ^)"));
 }
 
 TEST_CASE ("JSON support unicode characters in errors", "[json]")
 {
     // unicode characters w/ bytes >1
     auto res = Json::parse(R"json("Δx/Δt" "")json", "filename");
     REQUIRE(!res);
-    CHECK(res.error()->to_string() ==
-          R"(filename:1:9: error: Unexpected character; expected EOF
+    CHECK(res.error() == LocalizedString::from_raw(R"(filename:1:9: error: Unexpected character; expected EOF
   on expression: "Δx/Δt" ""
-                         ^)");
+                         ^)"));
 
     // full width unicode characters
     // note that the A is full width
     res = Json::parse(R"json("姐姐aＡ" "")json", "filename");
     REQUIRE(!res);
-    CHECK(res.error()->to_string() ==
-          R"(filename:1:8: error: Unexpected character; expected EOF
+    CHECK(res.error() == LocalizedString::from_raw(R"(filename:1:8: error: Unexpected character; expected EOF
   on expression: "姐姐aＡ" ""
-                           ^)");
+                           ^)"));
 
     // incorrect errors in the face of combining characters
     // (this test should be fixed once the underlying bug is fixed)
     res = Json::parse(R"json("é" "")json", "filename");
     REQUIRE(!res);
-    CHECK(res.error()->to_string() ==
-          R"(filename:1:6: error: Unexpected character; expected EOF
+    CHECK(res.error() == LocalizedString::from_raw(R"(filename:1:6: error: Unexpected character; expected EOF
   on expression: "é" ""
-                      ^)");
+                      ^)"));
 }
diff --git a/src/vcpkg-test/manifests.cpp b/src/vcpkg-test/manifests.cpp
@@ -22,7 +22,7 @@ static Json::Object parse_json_object(StringView sv)
     {
         INFO("Error found while parsing JSON document:");
         INFO(sv.to_string());
-        FAIL(json.error()->to_string());
+        FAIL(json.error());
         return Json::Object{};
     }
 }

diff --git a/src/vcpkg/base/json.cpp b/src/vcpkg/base/json.cpp
@@ -992,7 +992,7 @@ namespace vcpkg::Json
                 }
             }
 
-            static ExpectedT<ParsedJson, std::unique_ptr<ParseError>> parse(StringView json, StringView origin)
+            static ExpectedL<ParsedJson> parse(StringView json, StringView origin)
             {
                 StatsTimer t(g_json_parsing_stats);
 
@@ -1004,16 +1004,14 @@ namespace vcpkg::Json
                 if (!parser.at_eof())
                 {
                     parser.add_error(msg::format(msgUnexpectedEOFExpectedChar));
-                    return parser.extract_error();
                 }
-                else if (parser.get_error())
-                {
-                    return parser.extract_error();
-                }
-                else
+
+                if (const auto maybe_error = parser.get_error())
                 {
-                    return ParsedJson{std::move(val), parser.style()};
+                    return LocalizedString::from_raw(maybe_error->to_string());
                 }
+
+                return ParsedJson{std::move(val), parser.style()};
             }
 
             JsonStyle style() const noexcept { return style_; }
@@ -1100,14 +1098,12 @@ namespace vcpkg::Json
         return true;
     }
 
-    ExpectedT<ParsedJson, std::unique_ptr<ParseError>> parse_file(const ReadOnlyFilesystem& fs,
-                                                                  const Path& json_file,
-                                                                  std::error_code& ec)
+    ExpectedL<ParsedJson> parse_file(const ReadOnlyFilesystem& fs, const Path& json_file, std::error_code& ec)
     {
         auto res = fs.read_contents(json_file, ec);
         if (ec)
         {
-            return std::unique_ptr<ParseError>();
+            return format_filesystem_call_error(ec, "read_contents", {json_file});
         }
 
         return parse(res, json_file);
@@ -1116,34 +1112,27 @@ namespace vcpkg::Json
     ParsedJson parse_file(vcpkg::LineInfo li, const ReadOnlyFilesystem& fs, const Path& json_file)
     {
         std::error_code ec;
-        auto ret = parse_file(fs, json_file, ec).map_error(parse_error_formatter);
+        auto ret = parse_file(fs, json_file, ec);
         if (ec)
         {
             Checks::msg_exit_with_error(li, format_filesystem_call_error(ec, "read_contents", {json_file}));
         }
         return std::move(ret).value_or_exit(VCPKG_LINE_INFO);
     }
 
-    ExpectedT<ParsedJson, std::unique_ptr<ParseError>> parse(StringView json, StringView origin)
-    {
-        return Parser::parse(json, origin);
-    }
+    ExpectedL<ParsedJson> parse(StringView json, StringView origin) { return Parser::parse(json, origin); }
 
     ExpectedL<Json::Object> parse_object(StringView text, StringView origin)
     {
-        auto maybeValueIsh = parse(text, origin);
-        if (auto asValueIsh = maybeValueIsh.get())
-        {
-            auto& asValue = asValueIsh->value;
+        return parse(text, origin).then([&](ParsedJson&& mabeValueIsh) -> ExpectedL<Json::Object> {
+            auto& asValue = mabeValueIsh.value;
             if (asValue.is_object())
             {
                 return std::move(asValue).object(VCPKG_LINE_INFO);
             }
 
             return msg::format(msgJsonErrorMustBeAnObject, msg::path = origin);
-        }
-
-        return LocalizedString::from_raw(maybeValueIsh.error()->to_string());
+        });
     }
     // } auto parse()
 

diff --git a/src/vcpkg/commands.format-manifest.cpp b/src/vcpkg/commands.format-manifest.cpp
@@ -32,7 +32,7 @@ namespace
         auto parsed_json_opt = Json::parse(contents, manifest_path);
         if (!parsed_json_opt)
         {
-            msg::println(Color::error, LocalizedString::from_raw(parsed_json_opt.error()->to_string()));
+            msg::println(Color::error, parsed_json_opt.error());
             return nullopt;
         }
 

diff --git a/src/vcpkg/configuration.cpp b/src/vcpkg/configuration.cpp
@@ -883,7 +883,7 @@ namespace vcpkg
         auto conf = Json::parse(contents, origin);
         if (!conf)
         {
-            messageSink.println(Color::error, LocalizedString::from_raw(conf.error()->to_string()));
+            messageSink.println(Color::error, conf.error());
             return nullopt;
         }
 

diff --git a/src/vcpkg/platform-expression.cpp b/src/vcpkg/platform-expression.cpp
@@ -645,7 +645,7 @@ namespace vcpkg::PlatformExpression
         ExpressionParser parser(expression, multiple_binary_operators);
         auto res = parser.parse();
 
-        if (auto p = parser.extract_error())
+        if (auto p = parser.get_error())
         {
             return LocalizedString::from_raw(p->to_string());
         }

diff --git a/src/vcpkg/registries.cpp b/src/vcpkg/registries.cpp
@@ -1371,7 +1371,7 @@ namespace
         auto maybe_value = Json::parse(contents, origin);
         if (!maybe_value)
         {
-            return LocalizedString::from_raw(maybe_value.error()->to_string());
+            return std::move(maybe_value).error();
         }
 
         auto& value = *maybe_value.get();

diff --git a/src/vcpkg/vcpkgcmdarguments.cpp b/src/vcpkg/vcpkgcmdarguments.cpp
@@ -635,10 +635,7 @@ namespace vcpkg
         auto maybe_vcpkg_recursive_data = get_environment_variable(RECURSIVE_DATA_ENV);
         if (auto vcpkg_recursive_data = maybe_vcpkg_recursive_data.get())
         {
-            auto rec_doc = Json::parse(*vcpkg_recursive_data, RECURSIVE_DATA_ENV)
-                               .map_error(parse_error_formatter)
-                               .value_or_exit(VCPKG_LINE_INFO)
-                               .value;
+            auto rec_doc = Json::parse(*vcpkg_recursive_data, RECURSIVE_DATA_ENV).value_or_exit(VCPKG_LINE_INFO).value;
             const auto& obj = rec_doc.object(VCPKG_LINE_INFO);
 
             if (auto entry = obj.get(VCPKG_ROOT_ARG_NAME))

diff --git a/src/vcpkg/vcpkgpaths.cpp b/src/vcpkg/vcpkgpaths.cpp
@@ -509,7 +509,7 @@ namespace
         }
         else
         {
-            Debug::print("Failed to load lockfile:\n", maybe_lock_contents.error()->to_string());
+            Debug::print("Failed to load lockfile:\n", maybe_lock_contents.error());
             return ret;
         }
     }