Skip to content

Commit

Permalink
Push ExpectedL into the SourceControlFile::parse family. (#1240)
Browse files Browse the repository at this point in the history
  • Loading branch information
@BillyONeal
BillyONeal authored Oct 26, 2023
1 parent 468bbe3 commit 8f4f3fa
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 65 deletions.
12 changes: 6 additions & 6 deletions include/vcpkg/sourceparagraph.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,13 @@ namespace vcpkg
struct SourceControlFile
{
SourceControlFile clone() const;
static ParseExpected<SourceControlFile> parse_project_manifest_object(StringView origin,
const Json::Object& object,
MessageSink& warnings_sink);
static ExpectedL<std::unique_ptr<SourceControlFile>> parse_project_manifest_object(StringView origin,
const Json::Object& object,
MessageSink& warnings_sink);

static ParseExpected<SourceControlFile> parse_port_manifest_object(StringView origin,
const Json::Object& object,
MessageSink& warnings_sink);
static ExpectedL<std::unique_ptr<SourceControlFile>> parse_port_manifest_object(StringView origin,
const Json::Object& object,
MessageSink& warnings_sink);

static ParseExpected<SourceControlFile> parse_control_file(StringView origin,
std::vector<Paragraph>&& control_paragraphs);
Expand Down
87 changes: 43 additions & 44 deletions src/vcpkg-test/manifests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,34 +39,35 @@ enum class PrintErrors : bool
Yes,
};

static ParseExpected<SourceControlFile> test_parse_project_manifest(const Json::Object& obj,
PrintErrors print = PrintErrors::Yes)
static ExpectedL<std::unique_ptr<SourceControlFile>> test_parse_project_manifest(const Json::Object& obj,
PrintErrors print = PrintErrors::Yes)
{
auto res = SourceControlFile::parse_project_manifest_object("<test manifest>", obj, null_sink);
if (!res.has_value() && print == PrintErrors::Yes)
{
print_error_message(res.error());
msg::println(Color::error, res.error());
}
return res;
}

static ParseExpected<SourceControlFile> test_parse_port_manifest(const Json::Object& obj,
PrintErrors print = PrintErrors::Yes)
static ExpectedL<std::unique_ptr<SourceControlFile>> test_parse_port_manifest(const Json::Object& obj,
PrintErrors print = PrintErrors::Yes)
{
auto res = SourceControlFile::parse_port_manifest_object("<test manifest>", obj, null_sink);
if (!res.has_value() && print == PrintErrors::Yes)
{
print_error_message(res.error());
msg::println(Color::error, res.error());
}
return res;
}

static ParseExpected<SourceControlFile> test_parse_project_manifest(StringView obj,
PrintErrors print = PrintErrors::Yes)
static ExpectedL<std::unique_ptr<SourceControlFile>> test_parse_project_manifest(StringView obj,
PrintErrors print = PrintErrors::Yes)
{
return test_parse_project_manifest(parse_json_object(obj), print);
}
static ParseExpected<SourceControlFile> test_parse_port_manifest(StringView obj, PrintErrors print = PrintErrors::Yes)
static ExpectedL<std::unique_ptr<SourceControlFile>> test_parse_port_manifest(StringView obj,
PrintErrors print = PrintErrors::Yes)
{
return test_parse_port_manifest(parse_json_object(obj), print);
}
Expand Down Expand Up @@ -909,7 +910,7 @@ TEST_CASE ("manifest construct maximum", "[manifests]")
auto res = SourceControlFile::parse_port_manifest_object("<test manifest>", object, null_sink);
if (!res.has_value())
{
print_error_message(res.error());
msg::println(Color::error, res.error());
}
REQUIRE(res.has_value());
REQUIRE(*res.get() != nullptr);
Expand Down Expand Up @@ -1294,9 +1295,9 @@ TEST_CASE ("default-feature-core errors", "[manifests]")
})json",
PrintErrors::No);
REQUIRE(!m_pgh.has_value());
REQUIRE(m_pgh.error()->to_string() == "error: while loading <test manifest>:\n"
"$.default-features[0] (a default feature): the feature \"core\" turns off "
"default features and thus can't be in the default features list");
REQUIRE(m_pgh.error().data() == "error: while loading <test manifest>:\n"
"$.default-features[0] (a default feature): the feature \"core\" turns off "
"default features and thus can't be in the default features list");
}

TEST_CASE ("default-feature-core-object errors", "[manifests]")
Expand All @@ -1306,9 +1307,9 @@ TEST_CASE ("default-feature-core-object errors", "[manifests]")
})json",
PrintErrors::No);
REQUIRE(!m_pgh.has_value());
REQUIRE(m_pgh.error()->to_string() == "error: while loading <test manifest>:\n"
"$.default-features[0].name (a default feature): the feature \"core\" turns "
"off default features and thus can't be in the default features list");
REQUIRE(m_pgh.error().data() == "error: while loading <test manifest>:\n"
"$.default-features[0].name (a default feature): the feature \"core\" turns "
"off default features and thus can't be in the default features list");
}

TEST_CASE ("default-feature-default errors", "[manifests]")
Expand All @@ -1318,10 +1319,9 @@ TEST_CASE ("default-feature-default errors", "[manifests]")
})json",
PrintErrors::No);
REQUIRE(!m_pgh.has_value());
REQUIRE(m_pgh.error()->to_string() ==
"error: while loading <test manifest>:\n"
"$.default-features[0] (a default feature): the feature \"default\" refers to "
"the set of default features and thus can't be in the default features list");
REQUIRE(m_pgh.error().data() == "error: while loading <test manifest>:\n"
"$.default-features[0] (a default feature): the feature \"default\" refers to "
"the set of default features and thus can't be in the default features list");
}

TEST_CASE ("default-feature-default-object errors", "[manifests]")
Expand All @@ -1331,7 +1331,7 @@ TEST_CASE ("default-feature-default-object errors", "[manifests]")
})json",
PrintErrors::No);
REQUIRE(!m_pgh.has_value());
REQUIRE(m_pgh.error()->to_string() ==
REQUIRE(m_pgh.error().data() ==
"error: while loading <test manifest>:\n"
"$.default-features[0].name (a default feature): the feature \"default\" refers to the set of default "
"features and thus can't be in the default features list");
Expand All @@ -1344,10 +1344,10 @@ TEST_CASE ("default-feature-empty errors", "[manifests]")
})json",
PrintErrors::No);
REQUIRE(!m_pgh.has_value());
REQUIRE(m_pgh.error()->to_string() == "error: while loading <test manifest>:\n"
"$.default-features[0] (a feature name): \"\" is not a valid feature name. "
"Feature names must be lowercase alphanumeric+hypens and not reserved (see "
"https://learn.microsoft.com/vcpkg/users/manifests for more information).");
REQUIRE(m_pgh.error().data() == "error: while loading <test manifest>:\n"
"$.default-features[0] (a feature name): \"\" is not a valid feature name. "
"Feature names must be lowercase alphanumeric+hypens and not reserved (see "
"https://learn.microsoft.com/vcpkg/users/manifests for more information).");
}

TEST_CASE ("default-feature-empty-object errors", "[manifests]")
Expand All @@ -1357,11 +1357,10 @@ TEST_CASE ("default-feature-empty-object errors", "[manifests]")
})json",
PrintErrors::No);
REQUIRE(!m_pgh.has_value());
REQUIRE(m_pgh.error()->to_string() ==
"error: while loading <test manifest>:\n"
"$.default-features[0].name (a feature name): \"\" is not a valid feature name. "
"Feature names must be lowercase alphanumeric+hypens and not reserved (see "
"https://learn.microsoft.com/vcpkg/users/manifests for more information).");
REQUIRE(m_pgh.error().data() == "error: while loading <test manifest>:\n"
"$.default-features[0].name (a feature name): \"\" is not a valid feature name. "
"Feature names must be lowercase alphanumeric+hypens and not reserved (see "
"https://learn.microsoft.com/vcpkg/users/manifests for more information).");
}

TEST_CASE ("dependency-name-empty errors", "[manifests]")
Expand All @@ -1371,10 +1370,10 @@ TEST_CASE ("dependency-name-empty errors", "[manifests]")
})json",
PrintErrors::No);
REQUIRE(!m_pgh.has_value());
REQUIRE(m_pgh.error()->to_string() == "error: while loading <test manifest>:\n"
"$.dependencies[0] (a package name): \"\" is not a valid package name. "
"Package names must be lowercase alphanumeric+hypens and not reserved (see "
"https://learn.microsoft.com/vcpkg/users/manifests for more information).");
REQUIRE(m_pgh.error().data() == "error: while loading <test manifest>:\n"
"$.dependencies[0] (a package name): \"\" is not a valid package name. "
"Package names must be lowercase alphanumeric+hypens and not reserved (see "
"https://learn.microsoft.com/vcpkg/users/manifests for more information).");
}

TEST_CASE ("dependency-name-empty-object errors", "[manifests]")
Expand All @@ -1384,10 +1383,10 @@ TEST_CASE ("dependency-name-empty-object errors", "[manifests]")
})json",
PrintErrors::No);
REQUIRE(!m_pgh.has_value());
REQUIRE(m_pgh.error()->to_string() == "error: while loading <test manifest>:\n"
"$.dependencies[0].name (a package name): \"\" is not a valid package name. "
"Package names must be lowercase alphanumeric+hypens and not reserved (see "
"https://learn.microsoft.com/vcpkg/users/manifests for more information).");
REQUIRE(m_pgh.error().data() == "error: while loading <test manifest>:\n"
"$.dependencies[0].name (a package name): \"\" is not a valid package name. "
"Package names must be lowercase alphanumeric+hypens and not reserved (see "
"https://learn.microsoft.com/vcpkg/users/manifests for more information).");
}

TEST_CASE ("dependency-feature-name-core errors", "[manifests]")
Expand All @@ -1402,7 +1401,7 @@ TEST_CASE ("dependency-feature-name-core errors", "[manifests]")
})json",
PrintErrors::No);
REQUIRE(!m_pgh.has_value());
REQUIRE(m_pgh.error()->to_string() ==
REQUIRE(m_pgh.error().data() ==
"error: while loading <test manifest>:\n"
"$.dependencies[0].features[0] (a feature name): the feature \"core\" cannot be in a dependency's feature "
"list. To turn off default features, add \"default-features\": false instead.");
Expand All @@ -1421,7 +1420,7 @@ TEST_CASE ("dependency-feature-name-core-object errors", "[manifests]")
PrintErrors::No);
REQUIRE(!m_pgh.has_value());
REQUIRE(
m_pgh.error()->to_string() ==
m_pgh.error().data() ==
"error: while loading <test manifest>:\n"
"$.dependencies[0].features[0].name (a feature name): the feature \"core\" cannot be in a dependency's feature "
"list. To turn off default features, add \"default-features\": false instead.");
Expand All @@ -1439,7 +1438,7 @@ TEST_CASE ("dependency-feature-name-default errors", "[manifests]")
})json",
PrintErrors::No);
REQUIRE(!m_pgh.has_value());
REQUIRE(m_pgh.error()->to_string() ==
REQUIRE(m_pgh.error().data() ==
"error: while loading <test manifest>:\n"
"$.dependencies[0].features[0] (a feature name): the feature \"default\" cannot be in a dependency's "
"feature list. To turn on default features, add \"default-features\": true instead.");
Expand All @@ -1457,7 +1456,7 @@ TEST_CASE ("dependency-feature-name-default-object errors", "[manifests]")
})json",
PrintErrors::No);
REQUIRE(!m_pgh.has_value());
REQUIRE(m_pgh.error()->to_string() ==
REQUIRE(m_pgh.error().data() ==
"error: while loading <test manifest>:\n"
"$.dependencies[0].features[0].name (a feature name): the feature \"default\" cannot be in a dependency's "
"feature list. To turn on default features, add \"default-features\": true instead.");
Expand All @@ -1474,7 +1473,7 @@ TEST_CASE ("dependency-feature-name-empty errors", "[manifests]")
})json",
PrintErrors::No);
REQUIRE(!m_pgh.has_value());
REQUIRE(m_pgh.error()->to_string() ==
REQUIRE(m_pgh.error().data() ==
"error: while loading <test manifest>:\n"
"$.dependencies[0].features[0] (a feature name): \"\" is not a valid feature name. Feature names must be "
"lowercase alphanumeric+hypens and not reserved (see https://learn.microsoft.com/vcpkg/users/manifests for "
Expand All @@ -1493,7 +1492,7 @@ TEST_CASE ("dependency-feature-name-empty-object errors", "[manifests]")
})json",
PrintErrors::No);
REQUIRE(!m_pgh.has_value());
REQUIRE(m_pgh.error()->to_string() ==
REQUIRE(m_pgh.error().data() ==
"error: while loading <test manifest>:\n"
"$.dependencies[0].features[0].name (a feature name): \"\" is not a valid feature name. Feature names must "
"be lowercase alphanumeric+hypens and not reserved (see https://learn.microsoft.com/vcpkg/users/manifests "
Expand Down
4 changes: 2 additions & 2 deletions src/vcpkg/paragraphs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,11 +389,11 @@ namespace
StringView text,
StringView origin,
MessageSink& warning_sink,
ParseExpected<SourceControlFile> (*do_parse)(StringView, const Json::Object&, MessageSink&))
ExpectedL<std::unique_ptr<SourceControlFile>> (*do_parse)(StringView, const Json::Object&, MessageSink&))
{
StatsTimer timer(g_load_ports_stats);
return Json::parse_object(text, origin).then([&](Json::Object&& object) {
return do_parse(origin, std::move(object), warning_sink).map_error(ToLocalizedString);
return do_parse(origin, std::move(object), warning_sink);
});
}
}
Expand Down
24 changes: 11 additions & 13 deletions src/vcpkg/sourceparagraph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1439,9 +1439,9 @@ namespace vcpkg
}

template<class ManifestDeserializerType>
static ParseExpected<SourceControlFile> parse_manifest_object_impl(StringView origin,
const Json::Object& manifest,
MessageSink& warnings_sink)
static ExpectedL<std::unique_ptr<SourceControlFile>> parse_manifest_object_impl(StringView origin,
const Json::Object& manifest,
MessageSink& warnings_sink)
{
Json::Reader reader;

Expand All @@ -1454,10 +1454,10 @@ namespace vcpkg

if (!reader.errors().empty())
{
auto err = std::make_unique<ParseControlErrorInfo>();
err->name = origin.to_string();
err->other_errors = std::move(reader.errors());
return err;
ParseControlErrorInfo err;
err.name = origin.to_string();
err.other_errors = std::move(reader.errors());
return LocalizedString::from_raw(err.to_string());
}
else if (auto p = res.get())
{
Expand All @@ -1469,16 +1469,14 @@ namespace vcpkg
}
}

ParseExpected<SourceControlFile> SourceControlFile::parse_project_manifest_object(StringView origin,
const Json::Object& manifest,
MessageSink& warnings_sink)
ExpectedL<std::unique_ptr<SourceControlFile>> SourceControlFile::parse_project_manifest_object(
StringView origin, const Json::Object& manifest, MessageSink& warnings_sink)
{
return parse_manifest_object_impl<ProjectManifestDeserializer>(origin, manifest, warnings_sink);
}

ParseExpected<SourceControlFile> SourceControlFile::parse_port_manifest_object(StringView origin,
const Json::Object& manifest,
MessageSink& warnings_sink)
ExpectedL<std::unique_ptr<SourceControlFile>> SourceControlFile::parse_port_manifest_object(
StringView origin, const Json::Object& manifest, MessageSink& warnings_sink)
{
return parse_manifest_object_impl<PortManifestDeserializer>(origin, manifest, warnings_sink);
}
Expand Down

0 comments on commit 8f4f3fa

Please sign in to comment.