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.

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

LibSemVer: Propogate errors so we get an error instead of crashing #25525

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
86 changes: 43 additions & 43 deletions Tests/LibSemVer/TestSemVer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,73 +34,73 @@

TEST_CASE(to_string) // NOLINT(readability-function-cognitive-complexity, readability-function-size)
{
EXPECT_EQ(GET_SEMVER("1.2.3"sv).to_string(), GET_STRING("1.2.3"sv));
EXPECT_EQ(GET_SEMVER("1.2.3"sv).to_string(), GET_STRING("1.2.3"sv));
EXPECT_EQ(GET_SEMVER("10.20.30"sv).to_string(), GET_STRING("10.20.30"sv));
EXPECT_EQ(GET_SEMVER("1.1.2-prerelease+meta"sv).to_string(), GET_STRING("1.1.2-prerelease+meta"sv));
EXPECT_EQ(GET_SEMVER("1.1.2+meta"sv).to_string(), GET_STRING("1.1.2+meta"sv));
EXPECT_EQ(GET_SEMVER("1.1.2+meta-valid"sv).to_string(), GET_STRING("1.1.2+meta-valid"sv));
EXPECT_EQ(GET_SEMVER("1.0.0-alpha"sv).to_string(), GET_STRING("1.0.0-alpha"sv));
EXPECT_EQ(GET_SEMVER("1.0.0-beta"sv).to_string(), GET_STRING("1.0.0-beta"sv));
EXPECT_EQ(GET_SEMVER("1.0.0-alpha.beta"sv).to_string(), GET_STRING("1.0.0-alpha.beta"sv));
EXPECT_EQ(GET_SEMVER("1.0.0-alpha.beta.1"sv).to_string(), GET_STRING("1.0.0-alpha.beta.1"sv));
EXPECT_EQ(GET_SEMVER("1.0.0-alpha.1"sv).to_string(), GET_STRING("1.0.0-alpha.1"sv));
EXPECT_EQ(GET_SEMVER("1.0.0-alpha0.valid"sv).to_string(), GET_STRING("1.0.0-alpha0.valid"sv));
EXPECT_EQ(GET_SEMVER("1.0.0-alpha.0valid"sv).to_string(), GET_STRING("1.0.0-alpha.0valid"sv));
EXPECT_EQ(GET_SEMVER("1.0.0-rc.1+build.1"sv).to_string(), GET_STRING("1.0.0-rc.1+build.1"sv));
EXPECT_EQ(GET_SEMVER("2.0.0-rc.1+build.123"sv).to_string(), GET_STRING("2.0.0-rc.1+build.123"sv));
EXPECT_EQ(GET_SEMVER("1.2.3-beta"sv).to_string(), GET_STRING("1.2.3-beta"sv));
EXPECT_EQ(GET_SEMVER("10.2.3-DEV-SNAPSHOT"sv).to_string(), GET_STRING("10.2.3-DEV-SNAPSHOT"sv));
EXPECT_EQ(GET_SEMVER("1.2.3-SNAPSHOT-123"sv).to_string(), GET_STRING("1.2.3-SNAPSHOT-123"sv));
EXPECT_EQ(GET_SEMVER("1.0.0"sv).to_string(), GET_STRING("1.0.0"sv));
EXPECT_EQ(GET_SEMVER("2.0.0"sv).to_string(), GET_STRING("2.0.0"sv));
EXPECT_EQ(GET_SEMVER("1.1.7"sv).to_string(), GET_STRING("1.1.7"sv));
EXPECT_EQ(GET_SEMVER("2.0.0+build.1848"sv).to_string(), GET_STRING("2.0.0+build.1848"sv));
EXPECT_EQ(GET_SEMVER("2.0.1-alpha.1227"sv).to_string(), GET_STRING("2.0.1-alpha.1227"sv));
EXPECT_EQ(GET_SEMVER("1.0.0-alpha+beta"sv).to_string(), GET_STRING("1.0.0-alpha+beta"sv));
EXPECT_EQ(GET_SEMVER("1.0.0-alpha-a.b-c-somethinglong+build.1-aef.1-its-okay"sv).to_string(), GET_STRING("1.0.0-alpha-a.b-c-somethinglong+build.1-aef.1-its-okay"sv));
EXPECT_EQ(GET_SEMVER("1.2.3----RC-SNAPSHOT.12.9.1--.12+788"sv).to_string(), GET_STRING("1.2.3----RC-SNAPSHOT.12.9.1--.12+788"sv));
EXPECT_EQ(GET_SEMVER("1.2.3----RC-SNAPSHOT.12.9.1--"sv).to_string(), GET_STRING("1.2.3----RC-SNAPSHOT.12.9.1--"sv));
EXPECT_EQ(GET_SEMVER("1.2.3----R-S.12.9.1--.12+meta"sv).to_string(), GET_STRING("1.2.3----R-S.12.9.1--.12+meta"sv));
EXPECT_EQ(GET_SEMVER("1.2.3----RC-SNAPSHOT.12.9.1--.12"sv).to_string(), GET_STRING("1.2.3----RC-SNAPSHOT.12.9.1--.12"sv));
EXPECT_EQ(GET_SEMVER("1.0.0+0.build.1-rc.10000aaa-kk-0.1"sv).to_string(), GET_STRING("1.0.0+0.build.1-rc.10000aaa-kk-0.1"sv));
EXPECT_EQ(GET_SEMVER("1.0.0-0A.is.legal"sv).to_string(), GET_STRING("1.0.0-0A.is.legal"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.2.3"sv).to_string()), GET_STRING("1.2.3"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.2.3"sv).to_string()), GET_STRING("1.2.3"sv));
EXPECT_EQ(MUST(GET_SEMVER("10.20.30"sv).to_string()), GET_STRING("10.20.30"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.1.2-prerelease+meta"sv).to_string()), GET_STRING("1.1.2-prerelease+meta"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.1.2+meta"sv).to_string()), GET_STRING("1.1.2+meta"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.1.2+meta-valid"sv).to_string()), GET_STRING("1.1.2+meta-valid"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.0.0-alpha"sv).to_string()), GET_STRING("1.0.0-alpha"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.0.0-beta"sv).to_string()), GET_STRING("1.0.0-beta"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.0.0-alpha.beta"sv).to_string()), GET_STRING("1.0.0-alpha.beta"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.0.0-alpha.beta.1"sv).to_string()), GET_STRING("1.0.0-alpha.beta.1"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.0.0-alpha.1"sv).to_string()), GET_STRING("1.0.0-alpha.1"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.0.0-alpha0.valid"sv).to_string()), GET_STRING("1.0.0-alpha0.valid"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.0.0-alpha.0valid"sv).to_string()), GET_STRING("1.0.0-alpha.0valid"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.0.0-rc.1+build.1"sv).to_string()), GET_STRING("1.0.0-rc.1+build.1"sv));
EXPECT_EQ(MUST(GET_SEMVER("2.0.0-rc.1+build.123"sv).to_string()), GET_STRING("2.0.0-rc.1+build.123"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.2.3-beta"sv).to_string()), GET_STRING("1.2.3-beta"sv));
EXPECT_EQ(MUST(GET_SEMVER("10.2.3-DEV-SNAPSHOT"sv).to_string()), GET_STRING("10.2.3-DEV-SNAPSHOT"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.2.3-SNAPSHOT-123"sv).to_string()), GET_STRING("1.2.3-SNAPSHOT-123"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.0.0"sv).to_string()), GET_STRING("1.0.0"sv));
EXPECT_EQ(MUST(GET_SEMVER("2.0.0"sv).to_string()), GET_STRING("2.0.0"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.1.7"sv).to_string()), GET_STRING("1.1.7"sv));
EXPECT_EQ(MUST(GET_SEMVER("2.0.0+build.1848"sv).to_string()), GET_STRING("2.0.0+build.1848"sv));
EXPECT_EQ(MUST(GET_SEMVER("2.0.1-alpha.1227"sv).to_string()), GET_STRING("2.0.1-alpha.1227"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.0.0-alpha+beta"sv).to_string()), GET_STRING("1.0.0-alpha+beta"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.0.0-alpha-a.b-c-somethinglong+build.1-aef.1-its-okay"sv).to_string()), GET_STRING("1.0.0-alpha-a.b-c-somethinglong+build.1-aef.1-its-okay"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.2.3----RC-SNAPSHOT.12.9.1--.12+788"sv).to_string()), GET_STRING("1.2.3----RC-SNAPSHOT.12.9.1--.12+788"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.2.3----RC-SNAPSHOT.12.9.1--"sv).to_string()), GET_STRING("1.2.3----RC-SNAPSHOT.12.9.1--"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.2.3----R-S.12.9.1--.12+meta"sv).to_string()), GET_STRING("1.2.3----R-S.12.9.1--.12+meta"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.2.3----RC-SNAPSHOT.12.9.1--.12"sv).to_string()), GET_STRING("1.2.3----RC-SNAPSHOT.12.9.1--.12"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.0.0+0.build.1-rc.10000aaa-kk-0.1"sv).to_string()), GET_STRING("1.0.0+0.build.1-rc.10000aaa-kk-0.1"sv));
EXPECT_EQ(MUST(GET_SEMVER("1.0.0-0A.is.legal"sv).to_string()), GET_STRING("1.0.0-0A.is.legal"sv));
}

TEST_CASE(normal_bump) // NOLINT(readability-function-cognitive-complexity)
{
auto version = GET_SEMVER("1.1.2-prerelease+meta"sv);

// normal bumps
auto major_bump = version.bump(SemVer::BumpType::Major);
auto major_bump = MUST(version.bump(SemVer::BumpType::Major));
EXPECT_EQ(major_bump.major(), version.major() + 1);
EXPECT_EQ(major_bump.minor(), 0ul);
EXPECT_EQ(major_bump.patch(), 0ul);
EXPECT(major_bump.suffix().is_empty());
EXPECT(MUST(major_bump.suffix()).is_empty());

auto minor_bump = version.bump(SemVer::BumpType::Minor);
auto minor_bump = MUST(version.bump(SemVer::BumpType::Minor));
EXPECT_EQ(minor_bump.major(), version.major());
EXPECT_EQ(minor_bump.minor(), version.minor() + 1);
EXPECT_EQ(minor_bump.patch(), 0ul);
EXPECT(minor_bump.suffix().is_empty());
EXPECT(MUST(minor_bump.suffix()).is_empty());

auto patch_bump = version.bump(SemVer::BumpType::Patch);
auto patch_bump = MUST(version.bump(SemVer::BumpType::Patch));
EXPECT_EQ(patch_bump.major(), version.major());
EXPECT_EQ(patch_bump.minor(), version.minor());
EXPECT_EQ(patch_bump.patch(), version.patch() + 1);
EXPECT(minor_bump.suffix().is_empty());
EXPECT(MUST(minor_bump.suffix()).is_empty());
}

TEST_CASE(prerelease_bump_increment_numeric)
{
auto version = GET_SEMVER("1.1.2-0"sv);

auto prerelease_bump = version.bump(SemVer::BumpType::Prerelease);
auto prerelease_bump = MUST(version.bump(SemVer::BumpType::Prerelease));
EXPECT_EQ(prerelease_bump.major(), version.major());
EXPECT_EQ(prerelease_bump.minor(), version.minor());
EXPECT_EQ(prerelease_bump.patch(), version.patch());
EXPECT_NE(prerelease_bump.prerelease(), version.prerelease());
EXPECT(prerelease_bump.build_metadata().is_empty());
EXPECT(MUST(prerelease_bump.build_metadata()).is_empty());

auto version_prerelease_parts = version.prerelease_identifiers();
auto bumped_prerelease_parts = prerelease_bump.prerelease_identifiers();
Expand All @@ -112,12 +112,12 @@ TEST_CASE(prerelease_bump_rightmost_numeric_part)
{
auto version = GET_SEMVER("1.1.2-a.1.0.c"sv);

auto prerelease_bump = version.bump(SemVer::BumpType::Prerelease);
auto prerelease_bump = MUST(version.bump(SemVer::BumpType::Prerelease));
EXPECT_EQ(prerelease_bump.major(), version.major());
EXPECT_EQ(prerelease_bump.minor(), version.minor());
EXPECT_EQ(prerelease_bump.patch(), version.patch());
EXPECT_NE(prerelease_bump.prerelease(), version.prerelease());
EXPECT(prerelease_bump.build_metadata().is_empty());
EXPECT(MUST(prerelease_bump.build_metadata()).is_empty());

auto version_prerelease_parts = version.prerelease_identifiers();
auto bumped_prerelease_parts = prerelease_bump.prerelease_identifiers();
Expand All @@ -129,12 +129,12 @@ TEST_CASE(prerelease_bump_add_zero_if_no_numeric)
{
auto version = GET_SEMVER("1.1.2-only.strings"sv);

auto prerelease_bump = version.bump(SemVer::BumpType::Prerelease);
auto prerelease_bump = MUST(version.bump(SemVer::BumpType::Prerelease));
EXPECT_EQ(prerelease_bump.major(), version.major());
EXPECT_EQ(prerelease_bump.minor(), version.minor());
EXPECT_EQ(prerelease_bump.patch(), version.patch());
EXPECT_NE(prerelease_bump.prerelease(), version.prerelease());
EXPECT(prerelease_bump.build_metadata().is_empty());
EXPECT(MUST(prerelease_bump.build_metadata()).is_empty());

auto version_prerelease_parts = version.prerelease_identifiers();
auto bumped_prerelease_parts = prerelease_bump.prerelease_identifiers();
Expand Down
16 changes: 8 additions & 8 deletions Userland/Libraries/LibSemVer/SemVer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,22 @@
#include <LibSemVer/SemVer.h>

namespace SemVer {
String SemVer::suffix() const
ErrorOr<String> SemVer::suffix() const
{
StringBuilder sb;
if (!m_prerelease_identifiers.is_empty())
sb.appendff("-{}", prerelease());
if (!m_build_metadata_identifiers.is_empty())
sb.appendff("+{}", build_metadata());
return sb.to_string().release_value_but_fixme_should_propagate_errors();
sb.appendff("+{}", TRY(build_metadata()));
return TRY(sb.to_string());
}

String SemVer::to_string() const
ErrorOr<String> SemVer::to_string() const
{
return String::formatted("{}{}{}{}{}{}", m_major, m_number_separator, m_minor, m_number_separator, m_patch, suffix()).release_value_but_fixme_should_propagate_errors();
return TRY(String::formatted("{}{}{}{}{}{}", m_major, m_number_separator, m_minor, m_number_separator, m_patch, TRY(suffix())));
}

SemVer SemVer::bump(BumpType type) const
ErrorOr<SemVer> SemVer::bump(BumpType type) const
{
switch (type) {
case BumpType::Major:
Expand All @@ -45,7 +45,7 @@ SemVer SemVer::bump(BumpType type) const
auto numeric_identifier = identifier.to_number<u32>();
if (numeric_identifier.has_value()) {
is_found = true;
identifier = String::formatted("{}", numeric_identifier.value() + 1).release_value_but_fixme_should_propagate_errors();
identifier = TRY(String::formatted("{}", numeric_identifier.value() + 1));
break;
}
}
Expand Down Expand Up @@ -298,6 +298,6 @@ ErrorOr<SemVer> from_string_view(StringView const& version, char normal_version_
bool is_valid(StringView const& version, char normal_version_separator)
{
auto result = from_string_view(version, normal_version_separator);
return !result.is_error() && result.release_value().to_string() == version;
return !result.is_error() && MUST(result.release_value().to_string()) == version;
}
}
10 changes: 5 additions & 5 deletions Userland/Libraries/LibSemVer/SemVer.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ class SemVer {
return String::join('.', m_prerelease_identifiers).release_value_but_fixme_should_propagate_errors();
}
[[nodiscard]] ReadonlySpan<String> build_metadata_identifiers() const { return m_build_metadata_identifiers.span(); }
[[nodiscard]] String build_metadata() const { return String::join('.', m_build_metadata_identifiers).release_value_but_fixme_should_propagate_errors(); }
[[nodiscard]] ErrorOr<String> build_metadata() const { return TRY(String::join('.', m_build_metadata_identifiers)); }

[[nodiscard]] SemVer bump(BumpType) const;
[[nodiscard]] ErrorOr<SemVer> bump(BumpType) const;

[[nodiscard]] bool is_same(SemVer const&, CompareType = CompareType::Exact) const;
[[nodiscard]] bool is_greater_than(SemVer const&) const;
Expand All @@ -73,8 +73,8 @@ class SemVer {

[[nodiscard]] bool satisfies(StringView const& semver_spec) const;

[[nodiscard]] String suffix() const;
[[nodiscard]] String to_string() const;
[[nodiscard]] ErrorOr<String> suffix() const;
[[nodiscard]] ErrorOr<String> to_string() const;

private:
char m_number_separator;
Expand All @@ -95,6 +95,6 @@ template<>
struct AK::Formatter<SemVer::SemVer> : Formatter<String> {
ErrorOr<void> format(FormatBuilder& builder, SemVer::SemVer const& value)
{
return Formatter<String>::format(builder, value.to_string());
return Formatter<String>::format(builder, TRY(value.to_string()));
}
};
13 changes: 7 additions & 6 deletions Userland/Utilities/pkg/AvailablePortDatabase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,17 @@
#include <LibShell/PosixParser.h>
#include <LibShell/Shell.h>

void AvailablePortDatabase::query_details_for_package(HashMap<String, InstalledPort> const& installed_ports_map, StringView package_name, bool verbose)
ErrorOr<void> AvailablePortDatabase::query_details_for_package(HashMap<String, InstalledPort> const& installed_ports_map, StringView package_name, bool verbose)
{
auto possible_available_port = m_available_ports.find(package_name);
if (possible_available_port == m_available_ports.end()) {
outln("pkg: No match for queried name \"{}\"", package_name);
return;
return {};
}

auto& available_port = possible_available_port->value;

outln("{}: {}, {}", available_port.name(), available_port.version_string(), available_port.website());
outln("{}: {}, {}", available_port.name(), TRY(available_port.version_string()), available_port.website());
if (verbose) {
out("Installed: ");
auto installed_port = installed_ports_map.find(package_name);
Expand All @@ -51,15 +51,15 @@ void AvailablePortDatabase::query_details_for_package(HashMap<String, InstalledP
auto error_or_installed_version = installed_port->value.version_semver();

if (error_or_available_version.is_error() || error_or_installed_version.is_error()) {
auto ip_version = installed_port->value.version_string();
auto ap_version = available_port.version_string();
auto ip_version = TRY(installed_port->value.version_string());
auto ap_version = TRY(available_port.version_string());

if (ip_version == ap_version) {
outln("Already on latest version");
} else {
outln("Update to {} available", ap_version);
}
return;
return {};
}

auto available_version = error_or_available_version.value();
Expand All @@ -73,6 +73,7 @@ void AvailablePortDatabase::query_details_for_package(HashMap<String, InstalledP
outln("No");
}
}
return {};
}

static Optional<Markdown::Table::Column const&> get_column_in_table(Markdown::Table const& ports_table, StringView column_name)
Expand Down
2 changes: 1 addition & 1 deletion Userland/Utilities/pkg/AvailablePortDatabase.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class AvailablePortDatabase {
static ErrorOr<NonnullOwnPtr<AvailablePortDatabase>> instantiate_ports_database(StringView path);
static ErrorOr<int> download_available_ports_list_file(StringView path);

void query_details_for_package(HashMap<String, InstalledPort> const& installed_ports_map, StringView package_name, bool verbose);
ErrorOr<void> query_details_for_package(HashMap<String, InstalledPort> const& installed_ports_map, StringView package_name, bool verbose);

HashMap<String, AvailablePort> const& map() const { return m_available_ports; }
String const& path() const { return m_path; }
Expand Down
2 changes: 1 addition & 1 deletion Userland/Utilities/pkg/InstalledPortDatabase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ ErrorOr<void> InstalledPortDatabase::for_each_by_type(InstalledPort::Type type,

ErrorOr<void> InstalledPortDatabase::insert_new_port_to_ports_database(InstalledPort::Type type, String name, InstalledPort port, Vector<Port> const& dependencies)
{
TRY(m_database_file->write_until_depleted(ByteString::formatted("{} {} {}\n", type == InstalledPort::Type::Auto ? "auto"sv : "manual"sv, name, port.version_string())));
TRY(m_database_file->write_until_depleted(ByteString::formatted("{} {} {}\n", type == InstalledPort::Type::Auto ? "auto"sv : "manual"sv, name, TRY(port.version_string()))));
if (!dependencies.is_empty()) {
TRY(m_database_file->write_until_depleted(ByteString::formatted("dependency {}", name)));
for (auto& dependency : dependencies) {
Expand Down
2 changes: 1 addition & 1 deletion Userland/Utilities/pkg/Port.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class Port {

public:
[[nodiscard]] String name() const { return m_name; }
[[nodiscard]] String version_string() const { return m_version.has<String>() ? m_version.get<String>() : m_version.get<SemVer::SemVer>().to_string(); }
[[nodiscard]] ErrorOr<String> version_string() const { return m_version.has<String>() ? m_version.get<String>() : TRY(m_version.get<SemVer::SemVer>().to_string()); }
[[nodiscard]] ErrorOr<SemVer::SemVer> version_semver() const
{
if (m_version.has<String>())
Expand Down
Loading