Enable clang-analyzer

.clang-tidy

@@ -1,4 +1,5 @@
 # Bugprone exception escape is super unhelpful.
+# NewDeleteLeaks has many false positives w.r.t smart pointers.
 Checks:
   -*,
 
@@ -10,7 +11,10 @@ Checks:
   -bugprone-narrowing-conversions,
   -bugprone-signed-char-misuse,
 
-  clang-analyzer-deadcode.DeadStores,
+  clang-analyzer-*,
+  -clang-analyzer-cplusplus.NewDeleteLeaks,
+
   concurrency-*,
+
 HeaderFilterRegex: src/include|tools/benchmark/include|tools/nodejs_api/src_cpp/include|tools/python_api/src_cpp/include|tools/rust_api/include|tools/shell/include
 WarningsAsErrors: "*"

dataset/csv-error-tests/union-no-conversion.csv

@@ -0,0 +1,2 @@
+"col:UNION(u UINT8, s INT8)"
+a

src/binder/bind/bind_copy.cpp

@@ -5,7 +5,7 @@
 #include "catalog/rel_table_schema.h"
 #include "common/exception/binder.h"
 #include "common/exception/message.h"
-#include "common/string_utils.h"
+#include "common/string_format.h"
 #include "common/table_type.h"
 #include "parser/copy.h"
 
@@ -78,8 +78,8 @@ std::unique_ptr<BoundStatement> Binder::bindCopyFromClause(const Statement& stat
     switch (tableSchema->tableType) {
     case TableType::REL_GROUP:
     case TableType::RDF: {
-        throw BinderException(StringUtils::string_format("Cannot copy into {} table with type {}.",
-            tableName, TableTypeUtils::toString(tableSchema->tableType)));
+        throw BinderException(stringFormat("Cannot copy into {} table with type {}.", tableName,
+            TableTypeUtils::toString(tableSchema->tableType)));
     }
     default:
         break;

src/binder/bind/bind_create_macro.cpp

@@ -1,6 +1,7 @@
 #include "binder/binder.h"
 #include "binder/bound_create_macro.h"
 #include "common/exception/binder.h"
+#include "common/string_format.h"
 #include "common/string_utils.h"
 #include "parser/create_macro.h"
 
@@ -13,8 +14,9 @@ namespace binder {
 std::unique_ptr<BoundStatement> Binder::bindCreateMacro(const Statement& statement) {
     auto& createMacro = reinterpret_cast<const CreateMacro&>(statement);
     auto macroName = createMacro.getMacroName();
+    StringUtils::toUpper(macroName);
     if (catalog.getReadOnlyVersion()->containMacro(macroName)) {
-        throw BinderException{StringUtils::string_format("Macro {} already exists.", macroName)};
+        throw BinderException{stringFormat("Macro {} already exists.", macroName)};
     }
     parser::default_macro_args defaultArgs;
     for (auto& defaultArg : createMacro.getDefaultArgs()) {
@@ -23,7 +25,7 @@ std::unique_ptr<BoundStatement> Binder::bindCreateMacro(const Statement& stateme
     auto scalarMacro =
         std::make_unique<function::ScalarMacroFunction>(createMacro.getMacroExpression()->copy(),
             createMacro.getPositionalArgs(), std::move(defaultArgs));
-    return std::make_unique<BoundCreateMacro>(macroName, std::move(scalarMacro));
+    return std::make_unique<BoundCreateMacro>(std::move(macroName), std::move(scalarMacro));
 }
 
 } // namespace binder

src/binder/bind/bind_ddl.cpp

@@ -6,6 +6,7 @@
 #include "catalog/rel_table_group_schema.h"
 #include "catalog/rel_table_schema.h"
 #include "common/exception/binder.h"
+#include "common/string_format.h"
 #include "common/string_utils.h"
 #include "parser/ddl/alter.h"
 #include "parser/ddl/create_table.h"
@@ -26,12 +27,12 @@ std::vector<std::unique_ptr<Property>> Binder::bindProperties(
     boundPropertyNameDataTypes.reserve(propertyNameDataTypes.size());
     for (auto& propertyNameDataType : propertyNameDataTypes) {
         if (boundPropertyNames.contains(propertyNameDataType.first)) {
-            throw BinderException(StringUtils::string_format(
-                "Duplicated column name: {}, column name must be unique.",
-                propertyNameDataType.first));
+            throw BinderException(
+                stringFormat("Duplicated column name: {}, column name must be unique.",
+                    propertyNameDataType.first));
         } else if (TableSchema::isReservedPropertyName(propertyNameDataType.first)) {
             throw BinderException(
-                StringUtils::string_format("PropertyName: {} is an internal reserved propertyName.",
+                stringFormat("PropertyName: {} is an internal reserved propertyName.",
                     propertyNameDataType.first));
         }
         boundPropertyNameDataTypes.push_back(std::make_unique<Property>(
@@ -111,10 +112,9 @@ std::unique_ptr<BoundCreateTableInfo> Binder::bindCreateRelTableInfo(const Creat
             boundProperty->getDataType()->getLogicalTypeID() == LogicalTypeID::UNION ||
             boundProperty->getDataType()->getLogicalTypeID() == LogicalTypeID::STRUCT ||
             boundProperty->getDataType()->getLogicalTypeID() == LogicalTypeID::MAP) {
-            throw BinderException(
-                StringUtils::string_format("{} property is not supported in rel table.",
-                    LogicalTypeUtils::dataTypeToString(
-                        boundProperty->getDataType()->getLogicalTypeID())));
+            throw BinderException(stringFormat("{} property is not supported in rel table.",
+                LogicalTypeUtils::dataTypeToString(
+                    boundProperty->getDataType()->getLogicalTypeID())));
         }
     }
     auto extraInfo = (ExtraCreateRelTableInfo*)info->extraInfo.get();
@@ -206,9 +206,9 @@ std::unique_ptr<BoundStatement> Binder::bindDropTable(const Statement& statement
         for (auto& schema : catalogContent->getRelTableSchemas()) {
             auto relTableSchema = reinterpret_cast<RelTableSchema*>(schema);
             if (relTableSchema->isSrcOrDstTable(tableID)) {
-                throw BinderException(StringUtils::string_format(
-                    "Cannot delete node table {} referenced by rel table {}.",
-                    tableSchema->tableName, relTableSchema->tableName));
+                throw BinderException(
+                    stringFormat("Cannot delete node table {} referenced by rel table {}.",
+                        tableSchema->tableName, relTableSchema->tableName));
             }
         }
     } break;
@@ -217,9 +217,9 @@ std::unique_ptr<BoundStatement> Binder::bindDropTable(const Statement& statement
             auto relTableGroupSchema = reinterpret_cast<RelTableGroupSchema*>(schema);
             for (auto& relTableID : relTableGroupSchema->getRelTableIDs()) {
                 if (relTableID == tableSchema->getTableID()) {
-                    throw BinderException(StringUtils::string_format(
-                        "Cannot delete rel table {} referenced by rel group {}.",
-                        tableSchema->tableName, relTableGroupSchema->tableName));
+                    throw BinderException(
+                        stringFormat("Cannot delete rel table {} referenced by rel group {}.",
+                            tableSchema->tableName, relTableGroupSchema->tableName));
                 }
             }
         }
@@ -287,7 +287,7 @@ static void validatePropertyDDLOnTable(TableSchema* tableSchema, const std::stri
     case TableType::REL_GROUP:
     case TableType::RDF: {
         throw BinderException(
-            StringUtils::string_format("Cannot {} property on table {} with type {}.", ddlOperation,
+            stringFormat("Cannot {} property on table {} with type {}.", ddlOperation,
                 tableSchema->tableName, TableTypeUtils::toString(tableSchema->tableType)));
     }
     default:

src/binder/bind/bind_file_scan.cpp

@@ -3,6 +3,7 @@
 #include "binder/expression/literal_expression.h"
 #include "common/exception/binder.h"
 #include "common/exception/copy.h"
+#include "common/string_format.h"
 #include "common/string_utils.h"
 
 using namespace kuzu::parser;
@@ -39,15 +40,12 @@ std::vector<std::string> Binder::bindFilePaths(const std::vector<std::string>& f
     for (auto& filePath : filePaths) {
         auto globbedFilePaths = FileUtils::globFilePath(filePath);
         if (globbedFilePaths.empty()) {
-            throw BinderException{StringUtils::string_format(
-                "No file found that matches the pattern: {}.", filePath)};
+            throw BinderException{
+                stringFormat("No file found that matches the pattern: {}.", filePath)};
         }
         boundFilePaths.insert(
             boundFilePaths.end(), globbedFilePaths.begin(), globbedFilePaths.end());
     }
-    if (boundFilePaths.empty()) {
-        throw BinderException{StringUtils::string_format("Invalid file path: {}.", filePaths[0])};
-    }
     return boundFilePaths;
 }
 

src/binder/bind/bind_graph_pattern.cpp

@@ -10,7 +10,7 @@
 #include "catalog/rel_table_group_schema.h"
 #include "catalog/rel_table_schema.h"
 #include "common/exception/binder.h"
-#include "common/string_utils.h"
+#include "common/string_format.h"
 #include "function/cast/cast_utils.h"
 #include "main/client_context.h"
 
@@ -186,9 +186,10 @@ static std::unique_ptr<Expression> createPropertyExpression(const std::string& p
     }
     for (auto type : propertyDataTypes) {
         if (*propertyDataTypes[0] != *type) {
-            StringUtils::string_format("Expect same data type for property {} but find {} and {}",
-                propertyName, LogicalTypeUtils::dataTypeToString(*type),
-                LogicalTypeUtils::dataTypeToString(*propertyDataTypes[0]));
+            throw BinderException(
+                stringFormat("Expected the same data type for property {} but found {} and {}.",
+                    propertyName, LogicalTypeUtils::dataTypeToString(*type),
+                    LogicalTypeUtils::dataTypeToString(*propertyDataTypes[0])));
         }
     }
     return make_unique<PropertyExpression>(*propertyDataTypes[0], propertyName, uniqueVariableName,
@@ -315,7 +316,7 @@ static void bindRecursiveRelProjectionList(const expression_vector& projectionLi
     std::vector<std::string>& fieldNames, std::vector<std::unique_ptr<LogicalType>>& fieldTypes) {
     for (auto& expression : projectionList) {
         if (expression->expressionType != common::PROPERTY) {
-            throw BinderException(StringUtils::string_format(
+            throw BinderException(stringFormat(
                 "Unsupported projection item {} on recursive rel.", expression->toString()));
         }
         auto property = reinterpret_cast<PropertyExpression*>(expression.get());

src/binder/bind/bind_reading_clause.cpp

@@ -5,6 +5,7 @@
 #include "binder/query/reading_clause/bound_match_clause.h"
 #include "binder/query/reading_clause/bound_unwind_clause.h"
 #include "common/exception/binder.h"
+#include "common/string_format.h"
 #include "function/table_functions/bind_input.h"
 #include "parser/query/reading_clause/in_query_call_clause.h"
 #include "parser/query/reading_clause/load_from.h"
@@ -189,10 +190,9 @@ void Binder::validateColumnTypes(const std::vector<std::string>& columnNames,
     assert(expectedColumnTypes.size() == detectedColumnTypes.size());
     for (auto i = 0; i < expectedColumnTypes.size(); ++i) {
         if (*expectedColumnTypes[i] != *detectedColumnTypes[i]) {
-            throw BinderException(
-                StringUtils::string_format("Column `{}` type mismatch. Expected {} but got {}.",
-                    columnNames[i], LogicalTypeUtils::dataTypeToString(*expectedColumnTypes[i]),
-                    LogicalTypeUtils::dataTypeToString(*detectedColumnTypes[i])));
+            throw BinderException(stringFormat("Column `{}` type mismatch. Expected {} but got {}.",
+                columnNames[i], LogicalTypeUtils::dataTypeToString(*expectedColumnTypes[i]),
+                LogicalTypeUtils::dataTypeToString(*detectedColumnTypes[i])));
         }
     }
 }
@@ -202,7 +202,7 @@ void Binder::validateNumColumns(uint32_t expectedNumber, uint32_t detectedNumber
         return; // Empty CSV. Continue processing.
     }
     if (expectedNumber != detectedNumber) {
-        throw BinderException(StringUtils::string_format(
+        throw BinderException(stringFormat(
             "Number of columns mismatch. Expected {} but got {}.", expectedNumber, detectedNumber));
     }
 }
@@ -267,7 +267,7 @@ void Binder::sniffFile(const common::ReaderConfig& readerConfig, uint32_t fileId
         columnTypes.push_back(columnType->copy());
     } break;
     default:
-        throw BinderException(StringUtils::string_format(
+        throw BinderException(stringFormat(
             "Cannot sniff header of file type {}", FileTypeUtils::toString(readerConfig.fileType)));
     }
 }

src/binder/bind/bind_updating_clause.cpp

@@ -6,7 +6,7 @@
 #include "binder/query/updating_clause/bound_set_clause.h"
 #include "catalog/node_table_schema.h"
 #include "common/exception/binder.h"
-#include "common/string_utils.h"
+#include "common/string_format.h"
 #include "parser/query/updating_clause/delete_clause.h"
 #include "parser/query/updating_clause/insert_clause.h"
 #include "parser/query/updating_clause/merge_clause.h"
@@ -163,7 +163,7 @@ std::unique_ptr<BoundInsertInfo> Binder::bindInsertRelInfo(
     }
     if (ExpressionUtil::isRecursiveRelVariable(*rel)) {
         throw BinderException(
-            common::StringUtils::string_format("Cannot create recursive rel {}.", rel->toString()));
+            common::stringFormat("Cannot create recursive rel {}.", rel->toString()));
     }
     auto relTableID = rel->getSingleTableID();
     auto tableSchema = catalog.getReadOnlyVersion()->getTableSchema(relTableID);

src/binder/bind_expression/bind_property_expression.cpp

@@ -2,7 +2,7 @@
 #include "binder/expression/rel_expression.h"
 #include "binder/expression_binder.h"
 #include "common/exception/binder.h"
-#include "common/string_utils.h"
+#include "common/string_format.h"
 #include "parser/expression/parsed_property_expression.h"
 
 using namespace kuzu::common;
@@ -63,7 +63,7 @@ std::shared_ptr<Expression> ExpressionBinder::bindPropertyExpression(
     const ParsedExpression& parsedExpression) {
     auto& propertyExpression = (ParsedPropertyExpression&)parsedExpression;
     if (propertyExpression.isStar()) {
-        throw BinderException(StringUtils::string_format(
+        throw BinderException(stringFormat(
             "Cannot bind {} as a single property expression.", parsedExpression.toString()));
     }
     auto propertyName = propertyExpression.getPropertyName();

src/binder/binder.cpp

@@ -5,7 +5,7 @@
 #include "catalog/rel_table_schema.h"
 #include "common/exception/binder.h"
 #include "common/exception/not_implemented.h"
-#include "common/string_utils.h"
+#include "common/string_format.h"
 
 using namespace kuzu::common;
 using namespace kuzu::parser;
@@ -66,8 +66,7 @@ std::shared_ptr<Expression> Binder::bindWhereExpression(const ParsedExpression&
 common::table_id_t Binder::bindTableID(const std::string& tableName) const {
     auto catalogContent = catalog.getReadOnlyVersion();
     if (!catalogContent->containsTable(tableName)) {
-        throw BinderException(
-            common::StringUtils::string_format("Table {} does not exist.", tableName));
+        throw BinderException(common::stringFormat("Table {} does not exist.", tableName));
     }
     return catalogContent->getTableID(tableName);
 }
@@ -110,9 +109,8 @@ std::unique_ptr<LogicalType> Binder::bindDataType(const std::string& dataType) {
         auto numElementsPerPage = storage::PageUtils::getNumElementsInAPage(
             storage::StorageUtils::getDataTypeSize(boundType), true /* hasNull */);
         if (numElementsPerPage == 0) {
-            throw BinderException(
-                StringUtils::string_format("Cannot store a fixed list of size {} in a page.",
-                    storage::StorageUtils::getDataTypeSize(boundType)));
+            throw BinderException(stringFormat("Cannot store a fixed list of size {} in a page.",
+                storage::StorageUtils::getDataTypeSize(boundType)));
         }
     }
     return std::make_unique<LogicalType>(boundType);

src/binder/expression_binder.cpp

@@ -94,8 +94,8 @@ std::shared_ptr<Expression> ExpressionBinder::implicitCastIfNecessary(
     if (expression->dataType.getLogicalTypeID() == LogicalTypeID::ANY) {
         if (targetTypeID == LogicalTypeID::VAR_LIST) {
             // e.g. len($1) we cannot infer the child type for $1.
-            throw BinderException("Cannot resolve recursive data type for expression " +
-                                  expression->toString() + ".");
+            throw BinderException(stringFormat(
+                "Cannot resolve recursive data type for expression {}.", expression->toString()));
         }
         resolveAnyDataType(*expression, LogicalType(targetTypeID));
         return expression;
@@ -118,10 +118,10 @@ std::shared_ptr<Expression> ExpressionBinder::implicitCast(
             std::move(bindData), std::move(children), execFunc, nullptr /* selectFunc */,
             std::move(uniqueName));
     } else {
-        throw BinderException("Expression " + expression->toString() + " has data type " +
-                              LogicalTypeUtils::dataTypeToString(expression->dataType) +
-                              " but expect " + LogicalTypeUtils::dataTypeToString(targetType) +
-                              ". Implicit cast is not supported.");
+        throw BinderException(stringFormat(
+            "Expression {} has data type {} but expected {}. Implicit cast is not supported.",
+            expression->toString(), LogicalTypeUtils::dataTypeToString(expression->dataType),
+            LogicalTypeUtils::dataTypeToString(targetType)));
     }
 }
 
@@ -139,10 +139,9 @@ void ExpressionBinder::validateExpectedDataType(
     auto dataType = expression.dataType;
     auto targetsSet = std::unordered_set<LogicalTypeID>{targets.begin(), targets.end()};
     if (!targetsSet.contains(dataType.getLogicalTypeID())) {
-        throw BinderException(expression.toString() + " has data type " +
-                              LogicalTypeUtils::dataTypeToString(dataType.getLogicalTypeID()) +
-                              ". " + LogicalTypeUtils::dataTypesToString(targets) +
-                              " was expected.");
+        throw BinderException(stringFormat("{} has data type {} but {} was expected.",
+            expression.toString(), LogicalTypeUtils::dataTypeToString(dataType.getLogicalTypeID()),
+            LogicalTypeUtils::dataTypesToString(targets)));
     }
 }
 
@@ -152,7 +151,7 @@ void ExpressionBinder::validateAggregationExpressionIsNotNested(const Expression
     }
     if (ExpressionVisitor::hasAggregate(*expression.getChild(0))) {
         throw BinderException(
-            "Expression " + expression.toString() + " contains nested aggregation.");
+            stringFormat("Expression {} contains nested aggregation.", expression.toString()));
     }
 }
 

src/c_api/CMakeLists.txt

@@ -3,6 +3,7 @@ add_library(kuzu_c_api
         connection.cpp
         database.cpp
         data_type.cpp
+        helpers.cpp
         flat_tuple.cpp
         prepared_statement.cpp
         query_result.cpp

src/c_api/connection.cpp

@@ -1,4 +1,5 @@
 #include "binder/bound_statement_result.h"
+#include "c_api/helpers.h"
 #include "c_api/kuzu.h"
 #include "common/exception/exception.h"
 #include "main/kuzu.h"

src/c_api/flat_tuple.cpp

@@ -1,5 +1,6 @@
 #include "processor/result/flat_tuple.h"
 
+#include "c_api/helpers.h"
 #include "c_api/kuzu.h"
 #include "common/exception/exception.h"
 
@@ -35,8 +36,5 @@ kuzu_value* kuzu_flat_tuple_get_value(kuzu_flat_tuple* flat_tuple, uint64_t inde
 
 char* kuzu_flat_tuple_to_string(kuzu_flat_tuple* flat_tuple) {
     auto flat_tuple_shared_ptr = static_cast<std::shared_ptr<FlatTuple>*>(flat_tuple->_flat_tuple);
-    auto string = (*flat_tuple_shared_ptr)->toString();
-    char* string_c = (char*)malloc(string.size() + 1);
-    strcpy(string_c, string.c_str());
-    return string_c;
+    return convertToOwnedCString((*flat_tuple_shared_ptr)->toString());
 }

src/c_api/helpers.cpp

@@ -0,0 +1,11 @@
+#include "c_api/helpers.h"
+
+#include <cstring>
+
+char* convertToOwnedCString(const std::string& str) {
+    size_t src_len = str.size();
+    auto* c_str = (char*)malloc(sizeof(char) * (src_len + 1));
+    memcpy(c_str, str.c_str(), src_len);
+    c_str[src_len] = '\0';
+    return c_str;
+}