LibSQL: Remove infallible type conversions from SQL::Value

Force the callers to either know that the type is convertible, or to handle the conversion failure.
SerenityOS · Sep 23, 2022 · a426665 · a426665
1 parent 95fd413
commit a426665
Show file tree

Hide file tree

Showing 8 changed files with 22 additions and 64 deletions.
diff --git a/Tests/LibSQL/TestSqlBtreeIndex.cpp b/Tests/LibSQL/TestSqlBtreeIndex.cpp
@@ -211,7 +211,7 @@ void insert_into_and_scan_btree(int num_keys)
             if (prev.size()) {
                 EXPECT(prev < key);
             }
-            auto key_value = (int)key[0];
+            auto key_value = key[0].to_int();
             for (auto ix = 0; ix < num_keys; ix++) {
                 if (keys[ix] == key_value) {
                     EXPECT_EQ(key.pointer(), pointers[ix]);

diff --git a/Tests/LibSQL/TestSqlHashIndex.cpp b/Tests/LibSQL/TestSqlHashIndex.cpp
@@ -268,12 +268,14 @@ void insert_into_and_scan_hash_index(int num_keys)
         int count = 0;
         for (auto iter = hash_index->begin(); !iter.is_end(); iter++, count++) {
             auto key = (*iter);
-            auto key_value = (int)key[0];
+            auto key_value = key[0].to_int();
+            VERIFY(key_value.has_value());
+
             for (auto ix = 0; ix < num_keys; ix++) {
                 if (keys[ix] == key_value) {
                     EXPECT_EQ(key.pointer(), pointers[ix]);
                     if (found[ix])
-                        FAIL(String::formatted("Key {}, index {} already found previously", key_value, ix));
+                        FAIL(String::formatted("Key {}, index {} already found previously", *key_value, ix));
                     found[ix] = true;
                     break;
                 }

diff --git a/Tests/LibSQL/TestSqlValueAndTuple.cpp b/Tests/LibSQL/TestSqlValueAndTuple.cpp
@@ -84,12 +84,6 @@ TEST_CASE(text_value_to_other_types)
     }
 }
 
-TEST_CASE(text_value_to_int_crash)
-{
-    SQL::Value v(SQL::SQLType::Text, "Not a valid integer");
-    EXPECT_CRASH("Can't convert 'Not a valid integer' to integer", [&]() { (void) (int) v; return Test::Crash::Failure::DidNotCrash; });
-}
-
 TEST_CASE(serialize_text_value)
 {
     SQL::Value v("Test");
@@ -100,7 +94,7 @@ TEST_CASE(serialize_text_value)
 
     serializer.rewind();
     auto v2 = serializer.deserialize<SQL::Value>();
-    EXPECT((String)v2 == "Test");
+    EXPECT(v2.to_string() == "Test");
 }
 
 TEST_CASE(integer_value)
@@ -226,14 +220,14 @@ TEST_CASE(assign_int_to_text_value)
 {
     SQL::Value text(SQL::SQLType::Text);
     text = 42;
-    EXPECT_EQ((String)text, "42");
+    EXPECT_EQ(text.to_string(), "42");
 }
 
 TEST_CASE(copy_value)
 {
     SQL::Value text(SQL::SQLType::Text, 42);
     SQL::Value copy(text);
-    EXPECT_EQ((String)copy, "42");
+    EXPECT_EQ(copy.to_string(), "42");
 }
 
 TEST_CASE(compare_text_to_int)
@@ -281,7 +275,7 @@ TEST_CASE(serialize_boolean_value)
 {
     SQL::Value v(true);
     EXPECT_EQ(v.type(), SQL::SQLType::Boolean);
-    EXPECT(bool(v));
+    EXPECT_EQ(v.to_bool(), true);
 
     SQL::Serializer serializer;
     serializer.serialize<SQL::Value>(v);
@@ -290,7 +284,7 @@ TEST_CASE(serialize_boolean_value)
     auto v2 = serializer.deserialize<SQL::Value>();
     EXPECT(!v2.is_null());
     EXPECT_EQ(v2.type(), SQL::SQLType::Boolean);
-    EXPECT(bool(v2));
+    EXPECT_EQ(v2.to_bool(), true);
     EXPECT_EQ(v, v2);
 }
 
@@ -508,8 +502,8 @@ TEST_CASE(serialize_tuple)
     tuple["col1"] = "Test";
     tuple["col2"] = 42;
 
-    EXPECT_EQ((String)tuple[0], "Test");
-    EXPECT_EQ((int)tuple[1], 42);
+    EXPECT_EQ(tuple[0], "Test");
+    EXPECT_EQ(tuple[1], 42);
 
     SQL::Serializer serializer;
     serializer.serialize<SQL::Tuple>(tuple);

diff --git a/Userland/Libraries/LibSQL/AST/Expression.cpp b/Userland/Libraries/LibSQL/AST/Expression.cpp
@@ -126,23 +126,23 @@ ResultOr<Value> UnaryOperatorExpression::evaluate(ExecutionContext& context) con
         return Result { SQLCommand::Unknown, SQLErrorCode::NumericOperatorTypeMismatch, UnaryOperator_name(type()) };
     case UnaryOperator::Minus:
         if (expression_value.type() == SQLType::Integer) {
-            expression_value = -int(expression_value);
+            expression_value = -expression_value.to_int().value();
             return expression_value;
         }
         if (expression_value.type() == SQLType::Float) {
-            expression_value = -double(expression_value);
+            expression_value = -expression_value.to_double().value();
             return expression_value;
         }
         return Result { SQLCommand::Unknown, SQLErrorCode::NumericOperatorTypeMismatch, UnaryOperator_name(type()) };
     case UnaryOperator::Not:
         if (expression_value.type() == SQLType::Boolean) {
-            expression_value = !bool(expression_value);
+            expression_value = !expression_value.to_bool().value();
             return expression_value;
         }
         return Result { SQLCommand::Unknown, SQLErrorCode::BooleanOperatorTypeMismatch, UnaryOperator_name(type()) };
     case UnaryOperator::BitwiseNot:
         if (expression_value.type() == SQLType::Integer) {
-            expression_value = ~u32(expression_value);
+            expression_value = ~expression_value.to_u32().value();
             return expression_value;
         }
         return Result { SQLCommand::Unknown, SQLErrorCode::IntegerOperatorTypeMismatch, UnaryOperator_name(type()) };

diff --git a/Userland/Libraries/LibSQL/AST/Select.cpp b/Userland/Libraries/LibSQL/AST/Select.cpp
@@ -92,8 +92,8 @@ ResultOr<ResultSet> Select::execute(ExecutionContext& context) const
         context.current_row = &row;
 
         if (where_clause()) {
-            auto where_result = TRY(where_clause()->evaluate(context));
-            if (!where_result)
+            auto where_result = TRY(where_clause()->evaluate(context)).to_bool();
+            if (!where_result.has_value() || !where_result.value())
                 continue;
         }
 

diff --git a/Userland/Libraries/LibSQL/Meta.cpp b/Userland/Libraries/LibSQL/Meta.cpp
@@ -183,9 +183,10 @@ void TableDef::append_column(String name, SQLType sql_type)
 
 void TableDef::append_column(Key const& column)
 {
-    append_column(
-        (String)column["column_name"],
-        (SQLType)((int)column["column_type"]));
+    auto column_type = column["column_type"].to_int();
+    VERIFY(column_type.has_value());
+
+    append_column(column["column_name"].to_string(), static_cast<SQLType>(*column_type));
 }
 
 Key TableDef::make_key(SchemaDef const& schema_def)

diff --git a/Userland/Libraries/LibSQL/Value.cpp b/Userland/Libraries/LibSQL/Value.cpp
@@ -189,39 +189,6 @@ Optional<Vector<Value>> Value::to_vector() const
         return {};
 }
 
-Value::operator String() const
-{
-    return to_string();
-}
-
-Value::operator int() const
-{
-    auto i = to_int();
-    VERIFY(i.has_value());
-    return i.value();
-}
-
-Value::operator u32() const
-{
-    auto i = to_u32();
-    VERIFY(i.has_value());
-    return i.value();
-}
-
-Value::operator double() const
-{
-    auto d = to_double();
-    VERIFY(d.has_value());
-    return d.value();
-}
-
-Value::operator bool() const
-{
-    auto b = to_bool();
-    VERIFY(b.has_value());
-    return b.value();
-}
-
 void Value::assign(Value const& other_value)
 {
     m_impl.visit([&](auto& impl) { impl.assign(other_value); });

diff --git a/Userland/Libraries/LibSQL/Value.h b/Userland/Libraries/LibSQL/Value.h
@@ -78,12 +78,6 @@ class Value {
     [[nodiscard]] Optional<bool> to_bool() const;
     [[nodiscard]] Optional<Vector<Value>> to_vector() const;
 
-    explicit operator String() const;
-    explicit operator int() const;
-    explicit operator u32() const;
-    explicit operator double() const;
-    explicit operator bool() const;
-
     void assign(Value const& other_value);
     void assign(String const& string_value);
     void assign(int int_value);