fix: fix clang tidy error (#21)

Author: zjw1111

.github/workflows/clang_test.yaml

@@ -46,4 +46,4 @@ jobs:
         env:
           CC: clang
           CXX: clang++
-        run: ci/scripts/build_paimon.sh $(pwd)
+        run: ci/scripts/build_paimon.sh $(pwd) false true

.pre-commit-config.yaml

@@ -69,15 +69,15 @@ repos:
         ]
 
   - repo: https://github.com/cpplint/cpplint
-    rev: 1.6.1
+    rev: 2.0.2
     hooks:
       - id: cpplint
         alias: cpp
         name: C++ Lint
         args:
           - "--quiet"
           - "--verbose=2"
-          - "--filter=-whitespace/line_length,-whitespace/parens,-build/c++11,-readability/nolint,-runtime/references"
+          - "--filter=-whitespace/line_length,-whitespace/parens,-whitespace/indent_namespace,-build/include_what_you_use,-build/c++11,-build/c++17,-readability/nolint,-runtime/references"
 
         types_or:
           - c++

CMakeLists.txt

@@ -118,7 +118,7 @@ if(PAIMON_LINT_GIT_DIFF_MODE)
     set(GIT_DIFF_FILE_PATH ${CMAKE_BINARY_DIR}/git_diff_files.txt)
     add_custom_target(update_diff_files
                       # get git-diff file list
-                      COMMAND git diff-index --name-only --diff-filter=a --cached
+                      COMMAND git diff-index --name-only --diff-filter=d --cached
                               ${PAIMON_LINT_GIT_TARGET_COMMIT} > ${GIT_DIFF_FILE_PATH}
                       # convert to absolute path
                       COMMAND sed -i \"s|^|${CMAKE_SOURCE_DIR}/|\" ${GIT_DIFF_FILE_PATH}

ci/scripts/build_paimon.sh

@@ -18,6 +18,7 @@ set -eux
 
 source_dir=${1}
 enable_sanitizer=${2:-false}
+check_clang_tidy=${3:-false}
 build_dir=${1}/build
 
 mkdir ${build_dir}
@@ -42,6 +43,10 @@ cmake "${CMAKE_ARGS[@]}" ${source_dir}
 cmake --build . -- -j$(nproc)
 ctest --output-on-failure -j $(nproc)
 
+if [[ "${check_clang_tidy}" == "true" ]]; then
+    cmake --build . --target check-clang-tidy
+fi
+
 popd
 
 rm -rf ${build_dir}

include/paimon/result.h

@@ -37,13 +37,14 @@ class PAIMON_MUST_USE_TYPE PAIMON_EXPORT Result {
         status_.~Status();
     }
 
+    // NOLINTBEGIN(google-explicit-constructor, runtime/explicit)
     /// Construct a successful result with a copy of the given value.
     /// @param data The value to store in result.
-    Result(const T& data) : status_(), data_(data) {}  // NOLINT(runtime/explicit)
+    Result(const T& data) : status_(), data_(data) {}
 
     /// Construct a successful result by moving the given value.
     /// @param data The value to move into result.
-    Result(T&& data) : status_(), data_(std::move(data)) {}  // NOLINT(runtime/explicit)
+    Result(T&& data) : status_(), data_(std::move(data)) {}
 
     /// Template constructor for converting compatible pointer types.
     /// Support T = std::unique_ptr<B> and U = std::unique_ptr<D> convert, where D is derived class
@@ -53,11 +54,12 @@ class PAIMON_MUST_USE_TYPE PAIMON_EXPORT Result {
               std::enable_if_t<
                   is_pointer<U>::value && is_pointer<T>::value &&
                   std::is_convertible_v<value_type_traits_t<U>, value_type_traits_t<T>>>* = nullptr>
-    Result(U&& data) : status_(), data_(std::move(data)) {}  // NOLINT(runtime/explicit)
+    Result(U&& data) : status_(), data_(std::move(data)) {}
 
     /// Construct a failed result with the given status.
     /// @param status The status object describing the error.
-    Result(const Status& status) : status_(status) {}  // NOLINT(runtime/explicit)
+    Result(const Status& status) : status_(status) {}
+    // NOLINTEND(google-explicit-constructor, runtime/explicit)
 
     /// Copy constructor.
     /// @param other The result to copy from.
@@ -79,20 +81,22 @@ class PAIMON_MUST_USE_TYPE PAIMON_EXPORT Result {
         MakeStatus(other.status_);
     }
 
+    // NOLINTBEGIN(google-explicit-constructor, runtime/explicit)
     /// Template move constructor for converting compatible `Result` types.
     /// @param other The result to move from.
     template <typename U,
               std::enable_if_t<
                   is_pointer<U>::value && is_pointer<T>::value &&
                   std::is_convertible_v<value_type_traits_t<U>, value_type_traits_t<T>>>* = nullptr>
-    Result(Result<U>&& other) noexcept {  // NOLINT(runtime/explicit)
+    Result(Result<U>&& other) noexcept {
         if (other.ok()) {
             MakeValue(std::move(other).value());
         }
         // If we moved the status, the other status may become ok but the other
         // value hasn't been constructed => crash on other destructor.
         MakeStatus(other.status());
     }
+    // NOLINTEND(google-explicit-constructor, runtime/explicit)
 
     /// Copy assignment operator.
     /// @param other The result to copy from.

src/paimon/common/data/binary_row_test.cpp

@@ -74,7 +74,7 @@ TEST_F(BinaryRowTest, TestBasic) {
     row.SetInt(0, 5);
     row.SetDouble(1, 5.8);
     ASSERT_EQ(5, row.GetInt(0));
-    ASSERT_EQ((double)5.8, row.GetDouble(1));
+    ASSERT_EQ(static_cast<double>(5.8), row.GetDouble(1));
 
     row.Clear();
     std::shared_ptr<Bytes> bytes1 = Bytes::AllocateBytes(100, pool.get());
@@ -405,10 +405,10 @@ TEST_F(BinaryRowTest, TestCompatibleWithJava) {
         int32_t arity = 1;
         BinaryRow row(arity);
         BinaryRowWriter writer(&row, 0, pool.get());
-        writer.WriteInt(0, static_cast<int32_t>(18));
+        writer.WriteInt(0, 18);
         writer.Complete();
 
-        ASSERT_EQ(row.GetInt(0), (int32_t)18);
+        ASSERT_EQ(row.GetInt(0), 18);
 
         auto bytes = SerializationUtils::SerializeBinaryRow(row, pool.get());
         std::string bytes_view(bytes->data(), bytes->size());
@@ -418,7 +418,7 @@ TEST_F(BinaryRowTest, TestCompatibleWithJava) {
         ASSERT_EQ(bytes_view, expect_view);
         ASSERT_OK_AND_ASSIGN(auto de_row, SerializationUtils::DeserializeBinaryRow(bytes));
         ASSERT_EQ(1, de_row.GetFieldCount());
-        ASSERT_EQ(de_row.GetInt(0), (int32_t)18);
+        ASSERT_EQ(de_row.GetInt(0), 18);
     }
 }
 
@@ -564,8 +564,8 @@ TEST_F(BinaryRowTest, TestBinaryRowSerializer) {
     test_string1.reserve(str_size);
     test_string2.reserve(str_size);
     for (int32_t j = 0; j < str_size; j++) {
-        test_string1 += paimon::test::RandomNumber(0, 25) + 'a';
-        test_string2 += paimon::test::RandomNumber(0, 25) + 'a';
+        test_string1 += static_cast<char>(paimon::test::RandomNumber(0, 25) + 'a');
+        test_string2 += static_cast<char>(paimon::test::RandomNumber(0, 25) + 'a');
     }
     std::shared_ptr<Bytes> bytes1 = Bytes::AllocateBytes(test_string1, pool.get());
     std::shared_ptr<Bytes> bytes2 = Bytes::AllocateBytes(test_string2, pool.get());

src/paimon/common/data/generic_row_test.cpp

@@ -78,13 +78,13 @@ TEST(GenericRowTest, TestSimple) {
     // test get
     ASSERT_FALSE(row.IsNullAt(0));
     ASSERT_EQ(row.GetBoolean(0), true);
-    ASSERT_EQ(row.GetByte(1), (char)1);
-    ASSERT_EQ(row.GetShort(2), (int16_t)2);
-    ASSERT_EQ(row.GetInt(3), (int32_t)3);
-    ASSERT_EQ(row.GetDate(3), (int32_t)3);
-    ASSERT_EQ(row.GetLong(4), (int64_t)4);
-    ASSERT_EQ(row.GetFloat(5), (float)5.1);
-    ASSERT_EQ(row.GetDouble(6), (double)6.12);
+    ASSERT_EQ(row.GetByte(1), static_cast<char>(1));
+    ASSERT_EQ(row.GetShort(2), static_cast<int16_t>(2));
+    ASSERT_EQ(row.GetInt(3), static_cast<int32_t>(3));
+    ASSERT_EQ(row.GetDate(3), static_cast<int32_t>(3));
+    ASSERT_EQ(row.GetLong(4), static_cast<int64_t>(4));
+    ASSERT_EQ(row.GetFloat(5), static_cast<float>(5.1));
+    ASSERT_EQ(row.GetDouble(6), static_cast<double>(6.12));
     ASSERT_EQ(row.GetString(7), str);
     ASSERT_EQ(*row.GetBinary(8), *bytes);
     ASSERT_EQ(std::string(row.GetStringView(9)), str9);

src/paimon/common/data/record_batch_test.cpp

@@ -58,7 +58,7 @@ TEST(RecordBatchTest, TestSimple) {
         ASSERT_TRUE(struct_builder.Append().ok());
         ASSERT_TRUE(string_builder->Append("20240813").ok());
         ASSERT_TRUE(int_builder->Append(23).ok());
-        ASSERT_TRUE(long_builder->Append((int64_t)1722848484308ll + i).ok());
+        ASSERT_TRUE(long_builder->Append(static_cast<int64_t>(1722848484308ll + i)).ok());
         ASSERT_TRUE(bool_builder->Append(static_cast<bool>(i % 2)).ok());
     }
     std::shared_ptr<arrow::Array> array;

src/paimon/common/file_index/file_index_format_test.cpp

@@ -149,6 +149,7 @@ TEST_F(FileIndexFormatTest, TestSimple) {
     }
 }
 
+// NOLINTNEXTLINE(google-readability-function-size)
 TEST_F(FileIndexFormatTest, TestBitmapIndexWithTimestamp) {
     auto schema = arrow::schema({
         arrow::field("ts_sec", arrow::timestamp(arrow::TimeUnit::SECOND)),

src/paimon/common/memory/memory_segment.h

@@ -73,16 +73,16 @@ class PAIMON_EXPORT MemorySegment {
     template <typename T>
     inline void Get(int32_t index, T* dst, int32_t offset, int32_t length) const {
         // check the byte array offset and length and the status
-        assert((int32_t)dst->size() >= (offset + length));
-        assert((int32_t)heap_memory_->size() >= (index + length));
+        assert(static_cast<int32_t>(dst->size()) >= (offset + length));
+        assert(static_cast<int32_t>(heap_memory_->size()) >= (index + length));
         std::memcpy(const_cast<char*>(dst->data()) + offset, heap_memory_->data() + index, length);
     }
 
     template <typename T>
     inline void Put(int32_t index, const T& src, int32_t offset, int32_t length) {
         // check the byte array offset and length
-        assert((int32_t)src.size() >= (offset + length));
-        assert((int32_t)heap_memory_->size() >= (index + length));
+        assert(static_cast<int32_t>(src.size()) >= (offset + length));
+        assert(static_cast<int32_t>(heap_memory_->size()) >= (index + length));
         std::memcpy(heap_memory_->data() + index, src.data() + offset, length);
     }