tidy: enable clang-analyzer

This enables a myriad of complex checks.

.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: "*"

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;
+}

src/c_api/prepared_statement.cpp

@@ -1,4 +1,5 @@
 #include "binder/bound_statement.h"
+#include "c_api/helpers.h"
 #include "c_api/kuzu.h"
 #include "common/types/value/value.h"
 #include "main/kuzu.h"
@@ -43,9 +44,7 @@ char* kuzu_prepared_statement_get_error_message(kuzu_prepared_statement* prepare
     if (error_message.empty()) {
         return nullptr;
     }
-    char* error_message_c = (char*)malloc(error_message.size() + 1);
-    strcpy(error_message_c, error_message.c_str());
-    return error_message_c;
+    return convertToOwnedCString(error_message);
 }
 
 void kuzu_prepared_statement_bind_bool(

src/c_api/query_result.cpp

@@ -1,3 +1,4 @@
+#include "c_api/helpers.h"
 #include "c_api/kuzu.h"
 #include "main/kuzu.h"
 
@@ -24,9 +25,7 @@ char* kuzu_query_result_get_error_message(kuzu_query_result* query_result) {
     if (error_message.empty()) {
         return nullptr;
     }
-    char* error_message_c = (char*)malloc(error_message.size() + 1);
-    strcpy(error_message_c, error_message.c_str());
-    return error_message_c;
+    return convertToOwnedCString(error_message);
 }
 
 uint64_t kuzu_query_result_get_num_columns(kuzu_query_result* query_result) {
@@ -38,10 +37,7 @@ char* kuzu_query_result_get_column_name(kuzu_query_result* query_result, uint64_
     if (index >= column_names.size()) {
         return nullptr;
     }
-    auto column_name = column_names[index];
-    char* column_name_c = (char*)malloc(column_name.size() + 1);
-    strcpy(column_name_c, column_name.c_str());
-    return column_name_c;
+    return convertToOwnedCString(column_names[index]);
 }
 
 kuzu_logical_type* kuzu_query_result_get_column_data_type(
@@ -80,10 +76,8 @@ kuzu_flat_tuple* kuzu_query_result_get_next(kuzu_query_result* query_result) {
 }
 
 char* kuzu_query_result_to_string(kuzu_query_result* query_result) {
-    auto string = static_cast<QueryResult*>(query_result->_query_result)->toString();
-    char* string_c = (char*)malloc(string.size() + 1);
-    strcpy(string_c, string.c_str());
-    return string_c;
+    return convertToOwnedCString(
+        static_cast<QueryResult*>(query_result->_query_result)->toString());
 }
 
 void kuzu_query_result_write_to_csv(kuzu_query_result* query_result, const char* file_path,

src/c_api/value.cpp

@@ -1,5 +1,6 @@
 #include "common/types/value/value.h"
 
+#include "c_api/helpers.h"
 #include "c_api/kuzu.h"
 #include "common/types/internal_id_t.h"
 #include "common/types/types.h"
@@ -185,10 +186,7 @@ uint64_t kuzu_value_get_struct_num_fields(kuzu_value* value) {
 char* kuzu_value_get_struct_field_name(kuzu_value* value, uint64_t index) {
     auto val = static_cast<Value*>(value->_value);
     auto data_type = val->getDataType();
-    auto struct_field_name = StructType::getFields(data_type)[index]->getName();
-    auto* c_struct_field_name = (char*)malloc(sizeof(char) * (struct_field_name.size() + 1));
-    strcpy(c_struct_field_name, struct_field_name.c_str());
-    return c_struct_field_name;
+    return convertToOwnedCString(StructType::getFields(data_type)[index]->getName());
 }
 
 kuzu_value* kuzu_value_get_struct_field_value(kuzu_value* value, uint64_t index) {
@@ -291,24 +289,16 @@ kuzu_interval_t kuzu_value_get_interval(kuzu_value* value) {
 }
 
 char* kuzu_value_get_string(kuzu_value* value) {
-    auto string_val = static_cast<Value*>(value->_value)->getValue<std::string>();
-    auto* c_string = (char*)malloc(string_val.size() + 1);
-    strcpy(c_string, string_val.c_str());
-    return c_string;
+    return convertToOwnedCString(static_cast<Value*>(value->_value)->getValue<std::string>());
 }
 
 uint8_t* kuzu_value_get_blob(kuzu_value* value) {
-    auto string_val = static_cast<Value*>(value->_value)->getValue<std::string>();
-    auto* c_blob = (uint8_t*)malloc(string_val.size() + 1);
-    strcpy((char*)c_blob, string_val.c_str());
-    return c_blob;
+    return (uint8_t*)convertToOwnedCString(
+        static_cast<Value*>(value->_value)->getValue<std::string>());
 }
 
 char* kuzu_value_to_string(kuzu_value* value) {
-    auto string_val = static_cast<Value*>(value->_value)->toString();
-    auto* c_string = (char*)malloc(string_val.size() + 1);
-    strcpy(c_string, string_val.c_str());
-    return c_string;
+    return convertToOwnedCString(static_cast<Value*>(value->_value)->toString());
 }
 
 kuzu_value* kuzu_node_val_get_id_val(kuzu_value* node_val) {
@@ -336,21 +326,16 @@ kuzu_internal_id_t kuzu_node_val_get_id(kuzu_value* node_val) {
 }
 
 char* kuzu_node_val_get_label_name(kuzu_value* node_val) {
-    auto label_name = NodeVal::getLabelName(static_cast<Value*>(node_val->_value));
-    auto* c_string = (char*)malloc(label_name.size() + 1);
-    strcpy(c_string, label_name.c_str());
-    return c_string;
+    return convertToOwnedCString(NodeVal::getLabelName(static_cast<Value*>(node_val->_value)));
 }
 
 uint64_t kuzu_node_val_get_property_size(kuzu_value* node_val) {
     return NodeVal::getNumProperties(static_cast<Value*>(node_val->_value));
 }
 
 char* kuzu_node_val_get_property_name_at(kuzu_value* node_val, uint64_t index) {
-    auto name = NodeVal::getPropertyName(static_cast<Value*>(node_val->_value), index);
-    auto* c_string = (char*)malloc(name.size() + 1);
-    strcpy(c_string, name.c_str());
-    return c_string;
+    return convertToOwnedCString(
+        NodeVal::getPropertyName(static_cast<Value*>(node_val->_value), index));
 }
 
 kuzu_value* kuzu_node_val_get_property_value_at(kuzu_value* node_val, uint64_t index) {
@@ -362,10 +347,7 @@ kuzu_value* kuzu_node_val_get_property_value_at(kuzu_value* node_val, uint64_t i
 }
 
 char* kuzu_node_val_to_string(kuzu_value* node_val) {
-    auto string_val = NodeVal::toString(static_cast<Value*>(node_val->_value));
-    auto* c_string = (char*)malloc(string_val.size() + 1);
-    strcpy(c_string, string_val.c_str());
-    return c_string;
+    return convertToOwnedCString(NodeVal::toString(static_cast<Value*>(node_val->_value)));
 }
 
 kuzu_value* kuzu_rel_val_get_src_id_val(kuzu_value* rel_val) {
@@ -401,20 +383,15 @@ kuzu_internal_id_t kuzu_rel_val_get_dst_id(kuzu_value* rel_val) {
 }
 
 char* kuzu_rel_val_get_label_name(kuzu_value* rel_val) {
-    auto label = RelVal::getLabelName(static_cast<Value*>(rel_val->_value));
-    auto* c_string = (char*)malloc(label.size() + 1);
-    strcpy(c_string, label.c_str());
-    return c_string;
+    return convertToOwnedCString(RelVal::getLabelName(static_cast<Value*>(rel_val->_value)));
 }
 
 uint64_t kuzu_rel_val_get_property_size(kuzu_value* rel_val) {
     return RelVal::getNumProperties(static_cast<Value*>(rel_val->_value));
 }
 char* kuzu_rel_val_get_property_name_at(kuzu_value* rel_val, uint64_t index) {
-    auto name = RelVal::getPropertyName(static_cast<Value*>(rel_val->_value), index);
-    auto* c_string = (char*)malloc(name.size() + 1);
-    strcpy(c_string, name.c_str());
-    return c_string;
+    return convertToOwnedCString(
+        RelVal::getPropertyName(static_cast<Value*>(rel_val->_value), index));
 }
 
 kuzu_value* kuzu_rel_val_get_property_value_at(kuzu_value* rel_val, uint64_t index) {
@@ -426,8 +403,5 @@ kuzu_value* kuzu_rel_val_get_property_value_at(kuzu_value* rel_val, uint64_t ind
 }
 
 char* kuzu_rel_val_to_string(kuzu_value* rel_val) {
-    auto string_val = RelVal::toString(static_cast<Value*>(rel_val->_value));
-    auto* c_string = (char*)malloc(string_val.size() + 1);
-    strcpy(c_string, string_val.c_str());
-    return c_string;
+    return convertToOwnedCString(RelVal::toString(static_cast<Value*>(rel_val->_value)));
 }

src/include/c_api/helpers.h

@@ -1,3 +1,5 @@
+#pragma once
+
 #include <string>
 
 char* convertToOwnedCString(const std::string& str);

src/include/common/types/cast_helpers.h

@@ -122,7 +122,8 @@ struct DateToStringCast {
         }
         // optionally add BC to the end of the date
         if (add_bc) {
-            strcpy(ptr, " (BC)");
+            strcpy(ptr, " (BC)"); // NOLINT(clang-analyzer-security.insecureAPI.strcpy): safety
+                                  // guaranteed by Length().
         }
     }
 };
@@ -282,7 +283,8 @@ struct IntervalToStringCast {
             }
         } else if (length == 0) {
             // empty interval: default to 00:00:00
-            strcpy(buffer, "00:00:00");
+            strcpy(buffer, "00:00:00"); // NOLINT(clang-analyzer-security.insecureAPI.strcpy):
+                                        // safety guaranteed by Length().
             return 8;
         }
         return length;

test/test_files/exceptions/binder/binder_error.test

@@ -38,12 +38,12 @@ Binder exception: Expression in WITH must be aliased (use AS).
 -LOG BindToDifferentVariableType1
 -STATEMENT MATCH (a:person)-[e1:knows]->(b:person) WITH e1 AS a MATCH (a) RETURN *
 ---- error
-Binder exception: a has data type REL. (NODE) was expected.
+Binder exception: a has data type REL but (NODE) was expected.
 
 -LOG BindToDifferentVariableType2
 -STATEMENT MATCH (a:person)-[e1:knows]->(b:person) WITH a.age + 1 AS a MATCH (a) RETURN *
 ---- error
-Binder exception: a has data type INT64. (NODE) was expected.
+Binder exception: a has data type INT64 but (NODE) was expected.
 
 -LOG BindEmptyStar
 -STATEMENT RETURN *
@@ -73,7 +73,7 @@ Binder exception: Variable b is not in scope.
 -LOG BindPropertyLookUpOnExpression
 -STATEMENT MATCH (a:person)-[e1:knows]->(b:person) RETURN (a.age + 2).age
 ---- error
-Binder exception: +(a.age,2) has data type INT64. (NODE,REL,STRUCT) was expected.
+Binder exception: +(a.age,2) has data type INT64 but (NODE,REL,STRUCT) was expected.
 
 -LOG BindPropertyNotExist
 -STATEMENT MATCH (a:person)-[e1:knows]->(b:person) RETURN a.foo
@@ -222,7 +222,7 @@ Binder exception: The number of columns to union/union all must be the same.
 -LOG UnionAllUnmatchedDataTypesOfExpressions
 -STATEMENT MATCH (p:person) RETURN p.fName UNION ALL MATCH (p1:person) RETURN p1.age
 ---- error
-Binder exception: p1.age has data type INT64. (STRING) was expected.
+Binder exception: p1.age has data type INT64 but (STRING) was expected.
 
 -LOG UnionAndUnionAllInSingleQuery
 -STATEMENT MATCH (p:person) RETURN p.age UNION ALL MATCH (p1:person) RETURN p1.age UNION MATCH (p1:person) RETURN p1.age
@@ -240,17 +240,17 @@ Binder exception: Read after update is not supported. Try query with multiple st
 -LOG SetDataTypeMisMatch
 -STATEMENT MATCH (a:person) SET a.age = 'hh'
 ---- error
-Binder exception: Expression hh has data type STRING but expect INT64. Implicit cast is not supported.
+Binder exception: Expression hh has data type STRING but expected INT64. Implicit cast is not supported.
 
 -LOG CreateNodeDataTypeMisMatch
 -STATEMENT CREATE (a:person {age:'hh'})
 ---- error
-Binder exception: Expression hh has data type STRING but expect INT64. Implicit cast is not supported.
+Binder exception: Expression hh has data type STRING but expected INT64. Implicit cast is not supported.
 
 -LOG CreateRelDataTypeMisMatch
 -STATEMENT CREATE (a:person)-[:knows {date:12}]->(b:person)
 ---- error
-Binder exception: Expression 12 has data type INT64 but expect DATE. Implicit cast is not supported.
+Binder exception: Expression 12 has data type INT64 but expected DATE. Implicit cast is not supported.
 
 -LOG DeleteNodeProperty
 -STATEMENT MATCH (a:person) DELETE a.age
@@ -420,7 +420,7 @@ Binder exception: person table already has property fName.
 -LOG AddPropertyUnmatchedDefaultValueType
 -STATEMENT alter table person add intCol INT64 DEFAULT '3.2'
 ---- error
-Binder exception: Expression 3.2 has data type STRING but expect INT64. Implicit cast is not supported.
+Binder exception: Expression 3.2 has data type STRING but expected INT64. Implicit cast is not supported.
 
 -LOG RenameNonExistedTable
 -STATEMENT alter table person1 rename to person2
@@ -503,7 +503,7 @@ Binder exception: Invalid option name: thread.
 -LOG InvalidCallOptionValue
 -STATEMENT CALL threads='abc'
 ---- error
-Binder exception: abc has data type STRING. (INT64) was expected.
+Binder exception: abc has data type STRING but (INT64) was expected.
 
 -LOG AllShortestPathInvalidLowerBound
 -STATEMENT MATCH p = (a)-[* ALL SHORTEST 2..3]-(b) RETURN p

test/test_files/exceptions/copy/wrong_header.test

@@ -52,7 +52,7 @@ Copy exception: Invalid: CSV parse error: Expected 4 columns, got 3: 10,24,1
 ---- ok
 -STATEMENT COPY person FROM "${KUZU_ROOT_DIRECTORY}/dataset/copy-test/node/parquet/types_50k_1.parquet" (HEADER=true)
 ---- error
-Copy exception: Unmatched number of columns in parquet file. Expect: 3, got: 13.
+Copy exception: Unmatched number of columns in parquet file. Expected: 3, got: 13.
 
 -CASE RelUnmatchedNumColumns
 -STATEMENT create node table person (ID1 SERIAL, ID INT64, fName INT64, age INT64, PRIMARY KEY (ID1))
@@ -61,4 +61,4 @@ Copy exception: Unmatched number of columns in parquet file. Expect: 3, got: 13.
 ---- ok
 -STATEMENT COPY knows FROM "${KUZU_ROOT_DIRECTORY}/dataset/demo-db/parquet/follows.parquet" (HEADER=true)
 ---- error
-Copy exception: Unmatched number of columns in parquet file. Expect: 4, got: 3.
+Copy exception: Unmatched number of columns in parquet file. Expected: 4, got: 3.