Skip to content

Commit

Permalink
Eliminate ParseError. (#1322)
Browse files Browse the repository at this point in the history
* Eliminate ParseError.

Another pseudo-extraction from #1210 ; this means that ExpectedL is the only ExpectedT that exists.

* MessageMap
  • Loading branch information
BillyONeal authored Jan 26, 2024
1 parent 8d673fc commit 910db8b
Show file tree
Hide file tree
Showing 17 changed files with 108 additions and 170 deletions.
6 changes: 2 additions & 4 deletions include/vcpkg/base/json.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
5 changes: 1 addition & 4 deletions include/vcpkg/base/message-data.inc.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <topic>`",
Expand Down
41 changes: 3 additions & 38 deletions include/vcpkg/base/parse.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,43 +9,8 @@

#include <vcpkg/textrowcol.h>

#include <memory>
#include <string>

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<ParseError>& up)
{
return LocalizedString::from_raw(up->to_string());
}
} parse_error_formatter;

} // namespace vcpkg

namespace vcpkg
{
struct SourceLoc
Expand All @@ -66,7 +31,7 @@ namespace vcpkg

struct ParseMessages
{
std::unique_ptr<ParseError> error;
Optional<LocalizedString> error;
std::vector<ParseMessage> warnings;

void exit_if_errors_or_warnings(StringView origin) const;
Expand Down Expand Up @@ -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<ParseError> 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); }
Expand Down
3 changes: 1 addition & 2 deletions locales/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -827,8 +827,7 @@
"ForMoreHelp": "For More Help",
"_ForMoreHelp.comment": "Printed before a suggestion for the user to run `vcpkg help <topic>`",
"ForceSystemBinariesOnWeirdPlatforms": "Environment variable VCPKG_FORCE_SYSTEM_BINARIES must be set on arm, s390x, ppc64le and riscv platforms.",
"FormattedParseMessageExpression": "on expression: {value}",
"_FormattedParseMessageExpression.comment": "Example of {value} is 'x64 & windows'",
"FormattedParseMessageExpressionPrefix": "on expression:",
"GHAParametersMissing": "The GHA binary source requires the ACTIONS_RUNTIME_TOKEN and ACTIONS_CACHE_URL environment variables to be set. See {url} for details.",
"_GHAParametersMissing.comment": "An example of {url} is https://github.com/microsoft/vcpkg.",
"GeneratedConfiguration": "Generated configuration {path}.",
Expand Down
2 changes: 1 addition & 1 deletion src/vcpkg-fuzz/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions src/vcpkg-test/ci-baseline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]")
Expand Down
39 changes: 18 additions & 21 deletions src/vcpkg-test/json.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include <vcpkg-test/util.h>

#include <vcpkg/base/json.h>
#include <vcpkg/base/messages.h>

#include <iostream>

Expand All @@ -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]")
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand All @@ -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("姐姐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: "é" ""
^)");
^)"));
}
36 changes: 18 additions & 18 deletions src/vcpkg-test/manifests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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{};
}
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"(<license string>:1:1: error: SPDX license expression was empty.
CHECK(messages.error.value_or_exit(VCPKG_LINE_INFO) ==
LocalizedString::from_raw(R"(<license string>: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"(<license string>: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"(<license string>: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"(<license string>: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"(<license string>: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"(<license string>: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"(<license string>: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);
Expand Down
37 changes: 13 additions & 24 deletions src/vcpkg/base/json.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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_; }
Expand Down Expand Up @@ -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);
Expand All @@ -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()

Expand Down
Loading

0 comments on commit 910db8b

Please sign in to comment.