diff --git a/include/vcpkg/base/json.h b/include/vcpkg/base/json.h index 7897667b59..ff5ac07351 100644 --- a/include/vcpkg/base/json.h +++ b/include/vcpkg/base/json.h @@ -327,10 +327,8 @@ namespace vcpkg::Json JsonStyle style; }; - ExpectedT> parse_file(const ReadOnlyFilesystem&, - const Path&, - std::error_code& ec); - ExpectedT> parse(StringView text, StringView origin); + ExpectedL parse_file(const ReadOnlyFilesystem&, const Path&, std::error_code& ec); + ExpectedL parse(StringView text, StringView origin); ParsedJson parse_file(LineInfo li, const ReadOnlyFilesystem&, const Path&); ExpectedL parse_object(StringView text, StringView origin); diff --git a/include/vcpkg/base/message-data.inc.h b/include/vcpkg/base/message-data.inc.h index bd08ee1e25..c9eb7b5910 100644 --- a/include/vcpkg/base/message-data.inc.h +++ b/include/vcpkg/base/message-data.inc.h @@ -1340,10 +1340,7 @@ DECLARE_MESSAGE( (), "", "Environment variable VCPKG_FORCE_SYSTEM_BINARIES must be set on arm, s390x, ppc64le and riscv platforms.") -DECLARE_MESSAGE(FormattedParseMessageExpression, - (msg::value), - "Example of {value} is 'x64 & windows'", - "on expression: {value}") +DECLARE_MESSAGE(FormattedParseMessageExpressionPrefix, (), "", "on expression:") DECLARE_MESSAGE(ForMoreHelp, (), "Printed before a suggestion for the user to run `vcpkg help `", diff --git a/include/vcpkg/base/parse.h b/include/vcpkg/base/parse.h index bbdd7097f3..0f46a14010 100644 --- a/include/vcpkg/base/parse.h +++ b/include/vcpkg/base/parse.h @@ -9,43 +9,8 @@ #include -#include #include -namespace vcpkg -{ - struct ParseError - { - ParseError(std::string origin, int row, int column, int caret_col, std::string line, LocalizedString message) - : origin(std::move(origin)) - , row(row) - , column(column) - , caret_col(caret_col) - , line(std::move(line)) - , message(std::move(message)) - { - } - - const std::string origin; - const int row; - const int column; - const int caret_col; - const std::string line; - const LocalizedString message; - - std::string to_string() const; - }; - - constexpr struct ParseErrorFormatter - { - LocalizedString operator()(const std::unique_ptr& up) - { - return LocalizedString::from_raw(up->to_string()); - } - } parse_error_formatter; - -} // namespace vcpkg - namespace vcpkg { struct SourceLoc @@ -66,7 +31,7 @@ namespace vcpkg struct ParseMessages { - std::unique_ptr error; + Optional error; std::vector warnings; void exit_if_errors_or_warnings(StringView origin) const; @@ -141,8 +106,8 @@ namespace vcpkg void add_warning(LocalizedString&& message); void add_warning(LocalizedString&& message, const SourceLoc& loc); - const ParseError* get_error() const { return m_messages.error.get(); } - std::unique_ptr extract_error() { return std::move(m_messages.error); } + const LocalizedString* get_error() const& { return m_messages.error.get(); } + LocalizedString* get_error() && { return m_messages.error.get(); } 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 index be6373d53b..6e37224d28 100644 --- 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/ci-baseline.cpp b/src/vcpkg-test/ci-baseline.cpp index 337e7f95cb..85aca625a3 100644 --- a/src/vcpkg-test/ci-baseline.cpp +++ b/src/vcpkg-test/ci-baseline.cpp @@ -250,8 +250,7 @@ static void check_error(const std::string& input, const std::string& expected_er auto actual = parse_ci_baseline(input, "test", m); CHECK(actual.empty()); CHECK(m.warnings.empty()); - REQUIRE(m.error); - CHECK(m.error->to_string() == expected_error); + CHECK(m.error.value_or_exit(VCPKG_LINE_INFO) == LocalizedString::from_raw(expected_error)); } TEST_CASE ("Parse Errors", "[ci-baseline]") diff --git a/src/vcpkg-test/json.cpp b/src/vcpkg-test/json.cpp index 044a14908e..5a0af93595 100644 --- a/src/vcpkg-test/json.cpp +++ b/src/vcpkg-test/json.cpp @@ -1,6 +1,7 @@ #include #include +#include #include @@ -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,20 +229,19 @@ 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]") @@ -249,26 +249,23 @@ 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("姐姐aA" "")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: "姐姐aA" "" - ^)"); + ^)")); // incorrect errors in the face of combining characters // (this test should be fixed once the underlying bug is fixed) res = Json::parse(R"json("é" "")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: "é" "" - ^)"); + ^)")); } diff --git a/src/vcpkg-test/manifests.cpp b/src/vcpkg-test/manifests.cpp index ce3a8a58d6..2bebac9bad 100644 --- 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{}; } } @@ -1179,13 +1179,13 @@ static bool license_is_parsable(StringView license) { ParseMessages messages; parse_spdx_license_expression(license, messages); - return messages.error == nullptr; + return !messages.error.has_value(); } static bool license_is_strict(StringView license) { ParseMessages messages; parse_spdx_license_expression(license, messages); - return messages.error == nullptr && messages.warnings.empty(); + return !messages.error.has_value() && messages.warnings.empty(); } static std::string test_format_parse_warning(const ParseMessage& msg) @@ -1242,32 +1242,32 @@ TEST_CASE ("license error messages", "[manifests][license]") { ParseMessages messages; parse_spdx_license_expression("", messages); - REQUIRE(messages.error); - CHECK(messages.error->to_string() == R"(:1:1: error: SPDX license expression was empty. + CHECK(messages.error.value_or_exit(VCPKG_LINE_INFO) == + LocalizedString::from_raw(R"(:1:1: error: SPDX license expression was empty. on expression: - ^)"); + ^)")); parse_spdx_license_expression("MIT ()", messages); - REQUIRE(messages.error); - CHECK(messages.error->to_string() == - R"(:1:5: error: Expected a compound or the end of the string, found a parenthesis. + CHECK(messages.error.value_or_exit(VCPKG_LINE_INFO) == + LocalizedString::from_raw( + R"(:1:5: error: Expected a compound or the end of the string, found a parenthesis. on expression: MIT () - ^)"); + ^)")); parse_spdx_license_expression("MIT +", messages); - REQUIRE(messages.error); CHECK( - messages.error->to_string() == - R"(:1:5: error: SPDX license expression contains an extra '+'. These are only allowed directly after a license identifier. + messages.error.value_or_exit(VCPKG_LINE_INFO) == + LocalizedString::from_raw( + R"(:1:5: error: SPDX license expression contains an extra '+'. These are only allowed directly after a license identifier. on expression: MIT + - ^)"); + ^)")); parse_spdx_license_expression("MIT AND", messages); - REQUIRE(messages.error); - CHECK(messages.error->to_string() == - R"(:1:8: error: Expected a license name, found the end of the string. + CHECK( + messages.error.value_or_exit(VCPKG_LINE_INFO) == + LocalizedString::from_raw(R"(:1:8: error: Expected a license name, found the end of the string. on expression: MIT AND - ^)"); + ^)")); parse_spdx_license_expression("MIT AND unknownlicense", messages); CHECK(!messages.error); diff --git a/src/vcpkg/base/json.cpp b/src/vcpkg/base/json.cpp index 0d9518085e..2bcd4318b9 100644 --- a/src/vcpkg/base/json.cpp +++ b/src/vcpkg/base/json.cpp @@ -992,7 +992,7 @@ namespace vcpkg::Json } } - static ExpectedT> parse(StringView json, StringView origin) + static ExpectedL 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 = std::move(parser).get_error()) { - return ParsedJson{std::move(val), parser.style()}; + return std::move(*maybe_error); } + + return ParsedJson{std::move(val), parser.style()}; } JsonStyle style() const noexcept { return style_; } @@ -1100,14 +1098,12 @@ namespace vcpkg::Json return true; } - ExpectedT> parse_file(const ReadOnlyFilesystem& fs, - const Path& json_file, - std::error_code& ec) + ExpectedL 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(); + return format_filesystem_call_error(ec, "read_contents", {json_file}); } return parse(res, json_file); @@ -1116,7 +1112,7 @@ 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})); @@ -1124,26 +1120,19 @@ namespace vcpkg::Json return std::move(ret).value_or_exit(VCPKG_LINE_INFO); } - ExpectedT> parse(StringView json, StringView origin) - { - return Parser::parse(json, origin); - } + ExpectedL parse(StringView json, StringView origin) { return Parser::parse(json, origin); } ExpectedL 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 { + 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/base/parse.cpp b/src/vcpkg/base/parse.cpp index bdadabf937..1c069789fe 100644 --- a/src/vcpkg/base/parse.cpp +++ b/src/vcpkg/base/parse.cpp @@ -22,53 +22,53 @@ namespace vcpkg } } - std::string ParseError::to_string() const + static void append_caret_line(LocalizedString& res, const SourceLoc& loc) { - auto decoder = Unicode::Utf8Decoder(line.data(), line.data() + line.size()); - ParseMessage as_message; - as_message.location = SourceLoc{std::next(decoder, caret_col), decoder, row, column}; - as_message.message = message; - return as_message.format(origin, MessageKind::Error).extract_data(); - } - - LocalizedString ParseMessage::format(StringView origin, MessageKind kind) const - { - LocalizedString res; - if (!origin.empty()) - { - res.append_raw(fmt::format("{}:{}:{}: ", origin, location.row, location.column)); - } - - res.append_raw(kind == MessageKind::Warning ? WarningPrefix : ErrorPrefix); - res.append(message); - res.append_raw('\n'); - - auto line_end = Util::find_if(location.it, ParserBase::is_lineend); + auto line_end = Util::find_if(loc.it, ParserBase::is_lineend); StringView line = StringView{ - location.start_of_line.pointer_to_current(), + loc.start_of_line.pointer_to_current(), line_end.pointer_to_current(), }; - res.append_indent().append(msgFormattedParseMessageExpression, msg::value = line); - res.append_raw('\n'); - auto caret_point = StringView{location.start_of_line.pointer_to_current(), location.it.pointer_to_current()}; - auto formatted_caret_point = msg::format(msgFormattedParseMessageExpression, msg::value = caret_point); + LocalizedString line_prefix = msg::format(msgFormattedParseMessageExpressionPrefix); + size_t line_prefix_space = 1; // for the space after the prefix + for (char32_t ch : Unicode::Utf8Decoder(line_prefix)) + { + line_prefix_space += 1 + Unicode::is_double_width_code_point(ch); + } + + res.append_indent().append(line_prefix).append_raw(' ').append_raw(line).append_raw('\n'); std::string caret_string; - caret_string.reserve(formatted_caret_point.data().size()); - for (char32_t ch : Unicode::Utf8Decoder(formatted_caret_point)) + caret_string.append(line_prefix_space, ' '); + // note *it is excluded because it is where the ^ goes + for (auto it = loc.start_of_line; it != loc.it; ++it) { - if (ch == '\t') + if (*it == '\t') caret_string.push_back('\t'); - else if (Unicode::is_double_width_code_point(ch)) - caret_string.append(" "); + else if (Unicode::is_double_width_code_point(*it)) + caret_string.append(2, ' '); else caret_string.push_back(' '); } + caret_string.push_back('^'); res.append_indent().append_raw(caret_string); + } + + LocalizedString ParseMessage::format(StringView origin, MessageKind kind) const + { + LocalizedString res; + if (!origin.empty()) + { + res.append_raw(fmt::format("{}:{}:{}: ", origin, location.row, location.column)); + } + res.append_raw(kind == MessageKind::Warning ? WarningPrefix : ErrorPrefix); + res.append(message); + res.append_raw('\n'); + append_caret_line(res, location); return res; } @@ -79,9 +79,9 @@ namespace vcpkg msg::println(warning.format(origin, MessageKind::Warning)); } - if (error) + if (auto e = error.get()) { - Checks::msg_exit_with_message(VCPKG_LINE_INFO, LocalizedString::from_raw(error->to_string())); + Checks::msg_exit_with_message(VCPKG_LINE_INFO, *e); } if (!warnings.empty()) @@ -186,19 +186,16 @@ namespace vcpkg // avoid cascading errors by only saving the first if (!m_messages.error) { - // find end of line - auto line_end = loc.it; - while (line_end != line_end.end() && *line_end != '\n' && *line_end != '\r') + auto& res = m_messages.error.emplace(); + if (!m_origin.empty()) { - ++line_end; + res.append_raw(fmt::format("{}:{}:{}: ", m_origin, loc.row, loc.column)); } - m_messages.error = std::make_unique( - m_origin.to_string(), - loc.row, - loc.column, - static_cast(std::distance(loc.start_of_line, loc.it)), - std::string(loc.start_of_line.pointer_to_current(), line_end.pointer_to_current()), - std::move(message)); + + res.append_raw(ErrorPrefix); + res.append(message); + res.append_raw('\n'); + append_caret_line(res, loc); } // Avoid error loops by skipping to the end diff --git a/src/vcpkg/binarycaching.cpp b/src/vcpkg/binarycaching.cpp index b94f537381..4db24f84fe 100644 --- a/src/vcpkg/binarycaching.cpp +++ b/src/vcpkg/binarycaching.cpp @@ -2354,14 +2354,14 @@ ExpectedL vcpkg::parse_binary_provider_configs(const st default_parser.parse(); if (auto err = default_parser.get_error()) { - return LocalizedString::from_raw(err->message); + return *err; } BinaryConfigParser env_parser(env_string, "VCPKG_BINARY_SOURCES", &s); env_parser.parse(); if (auto err = env_parser.get_error()) { - return LocalizedString::from_raw(err->to_string()); + return *err; } for (auto&& arg : args) @@ -2370,7 +2370,7 @@ ExpectedL vcpkg::parse_binary_provider_configs(const st arg_parser.parse(); if (auto err = arg_parser.get_error()) { - return LocalizedString::from_raw(err->to_string()); + return *err; } } diff --git a/src/vcpkg/commands.format-manifest.cpp b/src/vcpkg/commands.format-manifest.cpp index dff60d3535..cc0a65cae5 100644 --- 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 index 342372888f..16e192eb03 100644 --- 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 index 374b9f9f0a..8bb46ef614 100644 --- 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 index 36e34520b1..9a3e901035 100644 --- 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 index 9418046c1c..b7c1cf8558 100644 --- 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 index 042ce572b6..fac3a21217 100644 --- 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; } }