From b5f1a48a7ce1683fa77607571c0b8568c02c0263 Mon Sep 17 00:00:00 2001 From: Dan Klishch Date: Fri, 12 Jan 2024 20:52:38 -0500 Subject: [PATCH] AK+Everywhere: Remove JsonValue APIs with implicit default values --- AK/JsonObject.cpp | 8 +-- AK/JsonValue.cpp | 32 +++++++--- AK/JsonValue.h | 61 +++++++++++-------- .../LibGL/GenerateGLAPIWrapper.cpp | 2 +- Tests/AK/TestJSON.cpp | 24 ++++---- Userland/Applications/Maps/UsersMapWidget.cpp | 5 +- .../Applications/PixelPaint/MainWidget.cpp | 2 +- .../Applications/Presenter/Presentation.cpp | 2 +- .../Applications/Presenter/SlideObject.cpp | 15 ++--- Userland/DevTools/Profiler/Profile.cpp | 2 +- .../Libraries/LibGUI/PropertyDeserializer.cpp | 32 ++++------ .../Libraries/LibGUI/PropertyDeserializer.h | 2 +- .../Libraries/LibJS/Runtime/JSONObject.cpp | 4 +- .../LibSymbolication/Symbolication.cpp | 2 +- .../WebDriver/TimeoutsConfiguration.cpp | 21 +++++-- .../WebContent/WebDriverConnection.cpp | 24 ++++---- 16 files changed, 132 insertions(+), 106 deletions(-) diff --git a/AK/JsonObject.cpp b/AK/JsonObject.cpp index 9df2543e9f..8ac0cce83a 100644 --- a/AK/JsonObject.cpp +++ b/AK/JsonObject.cpp @@ -138,16 +138,16 @@ Optional JsonObject::get_array(StringView key) const Optional JsonObject::get_double_with_precision_loss(StringView key) const { auto maybe_value = get(key); - if (maybe_value.has_value() && maybe_value->is_number()) - return maybe_value->to_number(); + if (maybe_value.has_value()) + return maybe_value->get_double_with_precision_loss(); return {}; } Optional JsonObject::get_float_with_precision_loss(StringView key) const { auto maybe_value = get(key); - if (maybe_value.has_value() && maybe_value->is_number()) - return maybe_value->to_number(); + if (maybe_value.has_value()) + return maybe_value->get_float_with_precision_loss(); return {}; } #endif diff --git a/AK/JsonValue.cpp b/AK/JsonValue.cpp index b6c20341f7..b1699fb716 100644 --- a/AK/JsonValue.cpp +++ b/AK/JsonValue.cpp @@ -77,15 +77,31 @@ bool JsonValue::equals(JsonValue const& other) const if (is_string() && other.is_string() && as_string() == other.as_string()) return true; -#if !defined(KERNEL) - if (is_number() && other.is_number() && to_number() == other.to_number()) { - return true; + if (is_number() && other.is_number()) { + auto normalize = [](Variant representation, bool& is_negative) { + return representation.visit( + [&](u64& value) -> Variant { + is_negative = false; + return value; + }, + [&](i64& value) -> Variant { + is_negative = value < 0; + return static_cast(abs(value)); + }, + [&](double& value) -> Variant { + is_negative = value < 0; + value = abs(value); + if (static_cast(static_cast(value)) == value) + return static_cast(value); + return value; + }); + }; + bool is_this_negative; + auto normalized_this = normalize(as_number(), is_this_negative); + bool is_that_negative; + auto normalized_that = normalize(other.as_number(), is_that_negative); + return is_this_negative == is_that_negative && normalized_this == normalized_that; } -#else - if (is_number() && other.is_number() && to_number() == other.to_number()) { - return true; - } -#endif if (is_array() && other.is_array() && as_array().size() == other.as_array().size()) { bool result = true; diff --git a/AK/JsonValue.h b/AK/JsonValue.h index fcddd67aef..ae05aceba8 100644 --- a/AK/JsonValue.h +++ b/AK/JsonValue.h @@ -92,29 +92,29 @@ public: return serialized(); } - int to_int(int default_value = 0) const { return to_i32(default_value); } - i32 to_i32(i32 default_value = 0) const { return to_number(default_value); } - i64 to_i64(i64 default_value = 0) const { return to_number(default_value); } + Optional get_int() const { return get_integer(); } + Optional get_i32() const { return get_integer(); } + Optional get_i64() const { return get_integer(); } - unsigned to_uint(unsigned default_value = 0) const { return to_u32(default_value); } - u32 to_u32(u32 default_value = 0) const { return to_number(default_value); } - u64 to_u64(u64 default_value = 0) const { return to_number(default_value); } - float to_float(float default_value = 0) const { return to_number(default_value); } - double to_double(double default_value = 0) const { return to_number(default_value); } + Optional get_uint() const { return get_integer(); } + Optional get_u32() const { return get_integer(); } + Optional get_u64() const { return get_integer(); } + Optional get_float_with_precision_loss() const { return get_number_with_precision_loss(); } + Optional get_double_with_precision_loss() const { return get_number_with_precision_loss(); } - FlatPtr to_addr(FlatPtr default_value = 0) const + Optional get_addr() const { #ifdef __LP64__ - return to_u64(default_value); + return get_u64(); #else - return to_u32(default_value); + return get_u32(); #endif } - bool to_bool(bool default_value = false) const + Optional get_bool() const { if (!is_bool()) - return default_value; + return {}; return as_bool(); } @@ -199,19 +199,22 @@ public: } template - T to_number(T default_value = 0) const + Optional get_number_with_precision_loss() const { - if (type() == Type::Double) - return (T)m_value.as_double; - if (type() == Type::Int32) - return (T)m_value.as_i32; - if (type() == Type::UnsignedInt32) - return (T)m_value.as_u32; - if (type() == Type::Int64) - return (T)m_value.as_i64; - if (type() == Type::UnsignedInt64) - return (T)m_value.as_u64; - return default_value; + switch (m_type) { + case Type::Double: + return static_cast(m_value.as_double); + case Type::Int32: + return static_cast(m_value.as_i32); + case Type::UnsignedInt32: + return static_cast(m_value.as_u32); + case Type::Int64: + return static_cast(m_value.as_i64); + case Type::UnsignedInt64: + return static_cast(m_value.as_u64); + default: + return {}; + } } template @@ -250,6 +253,14 @@ public: } } + template + Optional get_integer() const + { + if (!is_integer()) + return {}; + return as_integer(); + } + bool equals(JsonValue const& other) const; private: diff --git a/Meta/Lagom/Tools/CodeGenerators/LibGL/GenerateGLAPIWrapper.cpp b/Meta/Lagom/Tools/CodeGenerators/LibGL/GenerateGLAPIWrapper.cpp index 1357cb0f78..2ba7c9e07c 100644 --- a/Meta/Lagom/Tools/CodeGenerators/LibGL/GenerateGLAPIWrapper.cpp +++ b/Meta/Lagom/Tools/CodeGenerators/LibGL/GenerateGLAPIWrapper.cpp @@ -170,7 +170,7 @@ Variants read_variants_settings(JsonObject const& variants_obj) if (variants_obj.has_array("argument_counts"sv)) { variants.argument_counts.clear_with_capacity(); variants_obj.get_array("argument_counts"sv)->for_each([&](auto const& argument_count_value) { - variants.argument_counts.append(argument_count_value.to_u32()); + variants.argument_counts.append(argument_count_value.get_u32().value()); }); } if (variants_obj.has_array("argument_defaults"sv)) { diff --git a/Tests/AK/TestJSON.cpp b/Tests/AK/TestJSON.cpp index 9c059d2183..286535c613 100644 --- a/Tests/AK/TestJSON.cpp +++ b/Tests/AK/TestJSON.cpp @@ -133,28 +133,30 @@ TEST_CASE(json_parse_empty_string) TEST_CASE(json_parse_long_decimals) { auto value = JsonValue::from_string("1644452550.6489999294281"sv); - EXPECT_EQ(value.value().to_number(), 1644452550.6489999294281); + EXPECT_EQ(value.value().get_double_with_precision_loss(), 1644452550.6489999294281); } TEST_CASE(json_parse_number_with_exponent) { auto value_without_fraction = JsonValue::from_string("10e5"sv); - EXPECT_EQ(value_without_fraction.value().to_number(), 1000000.0); + EXPECT_EQ(value_without_fraction.value().get_double_with_precision_loss(), 1000000.0); auto value_with_fraction = JsonValue::from_string("10.5e5"sv); - EXPECT_EQ(value_with_fraction.value().to_number(), 1050000.0); + EXPECT_EQ(value_with_fraction.value().get_double_with_precision_loss(), 1050000.0); } TEST_CASE(json_parse_special_numbers) { -#define EXPECT_TO_MATCH_NUMBER_BIT_WISE(string_input, double_input) \ - do { \ - auto value_or_error = JsonValue::from_string(string_input##sv); \ - VERIFY(!value_or_error.is_error()); \ - if (value_or_error.is_error()) \ - dbgln("got {}", value_or_error.error()); \ - EXPECT(value_or_error.value().is_number()); \ - EXPECT_EQ(bit_cast(value_or_error.value().to_double(4321.0)), bit_cast(static_cast(double_input))); \ +#define EXPECT_TO_MATCH_NUMBER_BIT_WISE(string_input, double_input) \ + do { \ + auto value_or_error = JsonValue::from_string(string_input##sv); \ + VERIFY(!value_or_error.is_error()); \ + if (value_or_error.is_error()) \ + dbgln("got {}", value_or_error.error()); \ + auto value = value_or_error.release_value(); \ + EXPECT(value.is_number()); \ + auto value_as_double = value.get_double_with_precision_loss().value(); \ + EXPECT_EQ(bit_cast(value_as_double), bit_cast(static_cast(double_input))); \ } while (false) EXPECT_TO_MATCH_NUMBER_BIT_WISE("-0", -0.); diff --git a/Userland/Applications/Maps/UsersMapWidget.cpp b/Userland/Applications/Maps/UsersMapWidget.cpp index 6fcbe62bf9..aa66a446de 100644 --- a/Userland/Applications/Maps/UsersMapWidget.cpp +++ b/Userland/Applications/Maps/UsersMapWidget.cpp @@ -47,10 +47,11 @@ void UsersMapWidget::get_users() auto json_users = result.release_value().as_array(); for (size_t i = 0; i < json_users.size(); i++) { auto const& json_user = json_users.at(i).as_object(); + auto const& coordinates = json_user.get_array("coordinates"sv).release_value(); User user { MUST(String::from_byte_string(json_user.get_byte_string("nick"sv).release_value())), - { json_user.get_array("coordinates"sv).release_value().at(0).to_double(), - json_user.get_array("coordinates"sv).release_value().at(1).to_double() }, + { coordinates[0].get_double_with_precision_loss().value(), + coordinates[1].get_double_with_precision_loss().value() }, json_user.has_bool("contributor"sv), }; m_users.value().append(user); diff --git a/Userland/Applications/PixelPaint/MainWidget.cpp b/Userland/Applications/PixelPaint/MainWidget.cpp index bf62607dd2..565fa58542 100644 --- a/Userland/Applications/PixelPaint/MainWidget.cpp +++ b/Userland/Applications/PixelPaint/MainWidget.cpp @@ -1469,7 +1469,7 @@ ImageEditor& MainWidget::create_new_editor(NonnullRefPtr image) else return; - image_editor.add_guide(PixelPaint::Guide::construct(orientation, offset_value->to_number())); + image_editor.add_guide(PixelPaint::Guide::construct(orientation, offset_value->get_float_with_precision_loss().value_or(0))); }); } diff --git a/Userland/Applications/Presenter/Presentation.cpp b/Userland/Applications/Presenter/Presentation.cpp index 5e35bab9af..31f1f1f6c9 100644 --- a/Userland/Applications/Presenter/Presentation.cpp +++ b/Userland/Applications/Presenter/Presentation.cpp @@ -149,7 +149,7 @@ ErrorOr Presentation::parse_presentation_size(JsonObject const& me return Error::from_string_view("Width or aspect in incorrect format"sv); // We intentionally discard floating-point data here. If you need more resolution, just use a larger width. - auto const width = maybe_width->to_number(); + auto const width = maybe_width->get_number_with_precision_loss().value(); auto const aspect_parts = maybe_aspect->split_view(':'); if (aspect_parts.size() != 2) return Error::from_string_view("Aspect specification must have the exact format `width:height`"sv); diff --git a/Userland/Applications/Presenter/SlideObject.cpp b/Userland/Applications/Presenter/SlideObject.cpp index 0688bce5d7..44d31d4fb7 100644 --- a/Userland/Applications/Presenter/SlideObject.cpp +++ b/Userland/Applications/Presenter/SlideObject.cpp @@ -8,6 +8,7 @@ #include "Presentation.h" #include #include +#include #include #include @@ -42,16 +43,8 @@ ErrorOr> SlideObject::parse_slide_object(JsonObject c void SlideObject::set_property(StringView name, JsonValue value) { - if (name == "rect"sv) { - if (value.is_array() && value.as_array().size() == 4) { - Gfx::IntRect rect; - rect.set_x(value.as_array()[0].to_i32()); - rect.set_y(value.as_array()[1].to_i32()); - rect.set_width(value.as_array()[2].to_i32()); - rect.set_height(value.as_array()[3].to_i32()); - m_rect = rect; - } - } + if (name == "rect"sv) + m_rect = GUI::PropertyDeserializer {}(value).release_value_but_fixme_should_propagate_errors(); m_properties.set(name, move(value)); } @@ -74,7 +67,7 @@ void Text::set_property(StringView name, JsonValue value) } else if (name == "font-weight"sv) { m_font_weight = Gfx::name_to_weight(value.as_string()); } else if (name == "font-size"sv) { - m_font_size_in_pt = value.to_float(); + m_font_size_in_pt = value.get_float_with_precision_loss().value(); } else if (name == "text-alignment"sv) { m_text_align = value.as_string(); } diff --git a/Userland/DevTools/Profiler/Profile.cpp b/Userland/DevTools/Profiler/Profile.cpp index 7c282fe7ec..847392a38b 100644 --- a/Userland/DevTools/Profiler/Profile.cpp +++ b/Userland/DevTools/Profiler/Profile.cpp @@ -479,7 +479,7 @@ ErrorOr> Profile::load_from_perfcore_file(StringView path auto const& stack_array = stack.value(); for (ssize_t i = stack_array.values().size() - 1; i >= 0; --i) { auto const& frame = stack_array.at(i); - auto ptr = frame.to_number(); + auto ptr = frame.as_integer(); u32 offset = 0; DeprecatedFlyString object_name; ByteString symbol; diff --git a/Userland/Libraries/LibGUI/PropertyDeserializer.cpp b/Userland/Libraries/LibGUI/PropertyDeserializer.cpp index 514fb6a97a..be73930590 100644 --- a/Userland/Libraries/LibGUI/PropertyDeserializer.cpp +++ b/Userland/Libraries/LibGUI/PropertyDeserializer.cpp @@ -66,16 +66,10 @@ ErrorOr PropertyDeserializer::operator()(JsonValue c } else { auto const& array = value.as_array(); - auto get_i32 = [](JsonValue const& value) -> Optional { - if (value.is_integer()) - return value.to_i32(); - return {}; - }; - - x = get_i32(array[0]); - y = get_i32(array[1]); - width = get_i32(array[2]); - height = get_i32(array[3]); + x = array[0].get_i32(); + y = array[1].get_i32(); + width = array[2].get_i32(); + height = array[3].get_i32(); } if (!x.has_value()) @@ -103,16 +97,16 @@ ErrorOr PropertyDeserializer::operator()(JsonValue c auto const& array = value.as_array(); - auto const& width = array[0]; - if (!width.is_integer()) + auto const& width = array[0].get_i32(); + if (!width.has_value()) return Error::from_string_literal("Width must be an integer"); - auto const& height = array[1]; - if (!height.is_integer()) + auto const& height = array[1].get_i32(); + if (!height.has_value()) return Error::from_string_literal("Height must be an integer"); Gfx::IntSize size; - size.set_width(width.to_i32()); - size.set_height(height.to_i32()); + size.set_width(width.value()); + size.set_height(height.value()); return size; } @@ -129,10 +123,10 @@ ErrorOr PropertyDeserializer::operator()(JsonValue c int m[4]; for (size_t i = 0; i < size; ++i) { - auto const& margin = array[i]; - if (!margin.is_integer()) + auto const& margin = array[i].get_i32(); + if (!margin.has_value()) return Error::from_string_literal("Margin value should be an integer"); - m[i] = margin.to_i32(); + m[i] = margin.value(); } if (size == 1) diff --git a/Userland/Libraries/LibGUI/PropertyDeserializer.h b/Userland/Libraries/LibGUI/PropertyDeserializer.h index 00f6d56726..d2f13357c6 100644 --- a/Userland/Libraries/LibGUI/PropertyDeserializer.h +++ b/Userland/Libraries/LibGUI/PropertyDeserializer.h @@ -23,7 +23,7 @@ struct PropertyDeserializer { { if (!value.is_integer()) return Error::from_string_literal("Value is either not an integer or out of range for requested type"); - return value.to_number(); + return value.as_integer(); } }; diff --git a/Userland/Libraries/LibJS/Runtime/JSONObject.cpp b/Userland/Libraries/LibJS/Runtime/JSONObject.cpp index 782abf8af4..45a0722a75 100644 --- a/Userland/Libraries/LibJS/Runtime/JSONObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/JSONObject.cpp @@ -434,8 +434,8 @@ Value JSONObject::parse_json_value(VM& vm, JsonValue const& value) return Value(parse_json_array(vm, value.as_array())); if (value.is_null()) return js_null(); - if (value.is_number()) - return Value(value.to_double(0)); + if (auto double_value = value.get_double_with_precision_loss(); double_value.has_value()) + return Value(double_value.value()); if (value.is_string()) return PrimitiveString::create(vm, value.as_string()); if (value.is_bool()) diff --git a/Userland/Libraries/LibSymbolication/Symbolication.cpp b/Userland/Libraries/LibSymbolication/Symbolication.cpp index 5aa4e7a894..e00e68dba2 100644 --- a/Userland/Libraries/LibSymbolication/Symbolication.cpp +++ b/Userland/Libraries/LibSymbolication/Symbolication.cpp @@ -175,7 +175,7 @@ Vector symbolicate_thread(pid_t pid, pid_t tid, IncludeSourcePosition in stack.ensure_capacity(json.value().as_array().size()); for (auto& value : json.value().as_array().values()) { - stack.append(value.to_addr()); + stack.append(value.get_addr().value()); } } diff --git a/Userland/Libraries/LibWeb/WebDriver/TimeoutsConfiguration.cpp b/Userland/Libraries/LibWeb/WebDriver/TimeoutsConfiguration.cpp index 21866fe82a..ff0b3b3d28 100644 --- a/Userland/Libraries/LibWeb/WebDriver/TimeoutsConfiguration.cpp +++ b/Userland/Libraries/LibWeb/WebDriver/TimeoutsConfiguration.cpp @@ -51,13 +51,24 @@ ErrorOr json_deserialize_as_a_timeouts_configurati auto script_duration = value.as_object().get("script"sv); // 2. If script duration is a number and less than 0 or greater than maximum safe integer, or it is not null, return error with error code invalid argument. - if (script_duration.has_value() && script_duration->is_number() && (script_duration->to_i64() < 0 || script_duration->to_i64() > max_safe_integer)) - return Error::from_code(ErrorCode::InvalidArgument, "Invalid script duration"); - if (script_duration.has_value() && !script_duration->is_number() && !script_duration->is_null()) - return Error::from_code(ErrorCode::InvalidArgument, "Invalid script duration"); + Optional script_timeout; + if (script_duration.has_value()) { + bool is_valid; + if (auto duration = script_duration->get_double_with_precision_loss(); duration.has_value()) { + is_valid = *duration >= 0 && *duration <= max_safe_integer; + // FIXME: script_timeout should be double. + script_timeout = static_cast(*duration); + } else if (script_duration->is_null()) { + is_valid = true; + } else { + is_valid = false; + } + if (!is_valid) + return Error::from_code(ErrorCode::InvalidArgument, "Invalid script duration"); + } // 3. Set timeouts’s script timeout to script duration. - timeouts.script_timeout = (!script_duration.has_value() || script_duration->is_null()) ? Optional {} : script_duration->to_u64(); + timeouts.script_timeout = script_timeout; } // 4. If value has a property with the key "pageLoad": diff --git a/Userland/Services/WebContent/WebDriverConnection.cpp b/Userland/Services/WebContent/WebDriverConnection.cpp index 116112f47c..deee37d713 100644 --- a/Userland/Services/WebContent/WebDriverConnection.cpp +++ b/Userland/Services/WebContent/WebDriverConnection.cpp @@ -247,9 +247,9 @@ static ErrorOr get_property(JsonValue const return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidArgument, ByteString::formatted("Property '{}' is not a Boolean", key)); return property->as_bool(); } else if constexpr (IsSame) { - if (!property->is_integer()) - return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidArgument, ByteString::formatted("Property '{}' is not a Number", key)); - return property->to_u32(); + if (auto maybe_u32 = property->get_u32(); maybe_u32.has_value()) + return *maybe_u32; + return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidArgument, ByteString::formatted("Property '{}' is not a Number", key)); } else if constexpr (IsSame) { if (!property->is_array()) return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidArgument, ByteString::formatted("Property '{}' is not an Array", key)); @@ -617,20 +617,18 @@ Messages::WebDriverClient::SetWindowRectResponse WebDriverConnection::set_window auto const& properties = payload.as_object(); - auto resolve_property = [](auto name, auto const& property, auto min, auto max) -> ErrorOr, Web::WebDriver::Error> { + auto resolve_property = [](auto name, auto const& property, i32 min, i32 max) -> ErrorOr, Web::WebDriver::Error> { if (property.is_null()) return Optional {}; - if (!property.is_number()) + auto value = property.template get_integer(); + if (!value.has_value()) return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidArgument, ByteString::formatted("Property '{}' is not a Number", name)); + if (*value < min) + return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidArgument, ByteString::formatted("Property '{}' value {} exceeds the minimum allowed value {}", name, *value, min)); + if (*value > max) + return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidArgument, ByteString::formatted("Property '{}' value {} exceeds the maximum allowed value {}", name, *value, max)); - auto number = property.template to_number(); - - if (number < min) - return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidArgument, ByteString::formatted("Property '{}' value {} exceeds the minimum allowed value {}", name, number, min)); - if (number > max) - return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidArgument, ByteString::formatted("Property '{}' value {} exceeds the maximum allowed value {}", name, number, max)); - - return static_cast(number); + return value; }; // 1. Let width be the result of getting a property named width from the parameters argument, else let it be null.