Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Push ExpectedL into the SourceControlFile::parse family. #1240

Merged
merged 1 commit into from
Oct 26, 2023
Merged

Push ExpectedL into the SourceControlFile::parse family. #1240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions include/vcpkg/sourceparagraph.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,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
Open in desktop
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 @@ -886,7 +887,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 @@ -1271,9 +1272,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 @@ -1283,9 +1284,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 @@ -1295,10 +1296,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 @@ -1308,7 +1308,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 @@ -1321,10 +1321,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 @@ -1334,11 +1334,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 @@ -1348,10 +1347,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 @@ -1361,10 +1360,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 @@ -1379,7 +1378,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 @@ -1398,7 +1397,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 @@ -1416,7 +1415,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 @@ -1434,7 +1433,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 @@ -1451,7 +1450,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 @@ -1470,7 +1469,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
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1452,9 +1452,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 @@ -1467,10 +1467,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 @@ -1482,16 +1482,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