[CI] Add clang-tidy check with clang-analyzer and performance checks (#490)

Author: BewareMyPower

.clang-tidy

@@ -0,0 +1,32 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+---
+# TODO: add more checks
+# - bugprone
+# - cppcoreguidelines
+# - portability
+# - readability
+Checks: >
+  -*,
+  clang-analyzer-*,
+  -clang-analyzer-security.insecureAPI.rand,
+  performance-*,
+  -performance-inefficient-string-concatenation
+WarningsAsErrors: '*'
+HeaderFilterRegex: '^(?!.*PulsarApi\.pb\.h$).*$'
+FormatStyle:   none

.github/workflows/ci-pr-validation.yaml

@@ -77,10 +77,37 @@ jobs:
           cmake -S wireshark -B build-wireshark
           cmake --build build-wireshark
 
+  lint:
+    name: Lint
+    needs: formatting-check
+    runs-on: ubuntu-24.04
+    timeout-minutes: 120
+
+    steps:
+      - name: checkout
+        uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
+          submodules: recursive
+
+      - name: Build the project
+        run: |
+          cmake -B build -DINTEGRATE_VCPKG=ON -DBUILD_TESTS=ON -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
+          cmake --build build -j8
+
+      - name: Tidy check
+        run: |
+          sudo apt-get install -y clang-tidy
+          ./build-support/run_clang_tidy.sh
+          if [[ $? -ne 0 ]]; then
+              echo "clang-tidy failed"
+              exit 1
+          fi
+
   unit-tests:
     name: Run unit tests
     needs: formatting-check
-    runs-on: ubuntu-22.04
+    runs-on: ubuntu-24.04
     timeout-minutes: 120
 
     steps:
@@ -92,8 +119,8 @@ jobs:
 
       - name: Build core libraries
         run: |
-          cmake . -DINTEGRATE_VCPKG=ON -DBUILD_TESTS=OFF
-          cmake --build . -j8
+          cmake -B build -DINTEGRATE_VCPKG=ON -DBUILD_TESTS=OFF
+          cmake --build build -j8
 
       - name: Check formatting
         run: |
@@ -105,29 +132,29 @@ jobs:
 
       - name: Build tests
         run: |
-          cmake . -DINTEGRATE_VCPKG=ON -DBUILD_TESTS=ON
-          cmake --build . -j8
+          cmake -B build -DINTEGRATE_VCPKG=ON -DBUILD_TESTS=ON -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
+          cmake --build build -j8
 
       - name: Install gtest-parallel
         run: |
           sudo curl -o /gtest-parallel https://raw.githubusercontent.com/google/gtest-parallel/master/gtest_parallel.py 
 
       - name: Run unit tests
-        run: RETRY_FAILED=3 ./run-unit-tests.sh
+        run: RETRY_FAILED=3 CMAKE_BUILD_DIRECTORY=./build ./run-unit-tests.sh
 
       - name: Build perf tools
         run: |
-          cmake . -DINTEGRATE_VCPKG=ON -DBUILD_TESTS=ON -DBUILD_PERF_TOOLS=ON
-          cmake --build . -j8
+          cmake -B build -DINTEGRATE_VCPKG=ON -DBUILD_TESTS=ON -DBUILD_PERF_TOOLS=ON
+          cmake --build build -j8
 
       - name: Verify custom vcpkg installation
         run: |
           mv vcpkg /tmp/
-          cmake -B build -DINTEGRATE_VCPKG=ON -DCMAKE_TOOLCHAIN_FILE="/tmp/vcpkg/scripts/buildsystems/vcpkg.cmake"
+          cmake -B build-2 -DINTEGRATE_VCPKG=ON -DCMAKE_TOOLCHAIN_FILE="/tmp/vcpkg/scripts/buildsystems/vcpkg.cmake"
 
   cpp20-build:
     name: Build with the C++20 standard
-    needs: formatting-check
+    needs: lint
     runs-on: ubuntu-22.04
     timeout-minutes: 60
 
@@ -270,7 +297,7 @@ jobs:
   package:
     name: Build ${{matrix.pkg.name}} ${{matrix.cpu.platform}}
     runs-on: ubuntu-22.04
-    needs: unit-tests
+    needs: [lint, unit-tests]
     timeout-minutes: 500
 
     strategy:
@@ -314,7 +341,7 @@ jobs:
     timeout-minutes: 120
     name: Build CPP Client on macOS with static dependencies
     runs-on: macos-14
-    needs: formatting-check
+    needs: lint
     steps:
       - name: checkout
         uses: actions/checkout@v3
@@ -329,7 +356,7 @@ jobs:
   check-completion:
     name: Check Completion
     runs-on: ubuntu-latest
-    needs: [formatting-check, wireshark-dissector-build, unit-tests, cpp20-build, cpp-build-windows, package, cpp-build-macos]
+    needs: [formatting-check, wireshark-dissector-build, lint, unit-tests, cpp20-build, cpp-build-windows, package, cpp-build-macos]
 
     steps:
       - run: true

.gitignore

@@ -106,3 +106,6 @@ Testing
 .test-token.txt
 pkg/mac/.build
 pkg/mac/.install
+
+# Used in ./build-support/run_clang_tidy.sh
+files.txt

CMakeLists.txt

@@ -97,8 +97,6 @@ else() # GCC or Clang are mostly compatible:
     # Options unique to Clang or GCC:
     if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
         add_compile_options(-Qunused-arguments) 
-    elseif (CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND NOT (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 8.1))
-        add_compile_options(-Wno-stringop-truncation)
     endif()
 endif()
 

build-support/run_clang_tidy.sh

@@ -0,0 +1,41 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+set -e
+cd `dirname $0`/..
+
+FILES=$(find $PWD/include $PWD/lib $PWD/tests $PWD/examples -name "*.h" -o -name "*.cc" \
+    | grep -v "lib\/c\/" | grep -v "lib\/checksum\/" | grep -v "lib\/lz4\/" \
+    | grep -v "include\/pulsar\/c\/" | grep -v "tests\/c\/")
+
+rm -f files.txt
+for FILE in $FILES; do
+    echo $FILE >> files.txt
+done
+# run-clang-tidy from older version of LLVM requires python but not python3 as the env, so we cannot run it directly
+SCRIPT=$(which run-clang-tidy)
+set +e
+nproc
+if [[ $? == 0 ]]; then
+    python3 $SCRIPT -p build -j$(nproc) $(cat files.txt)
+else
+    python3 $SCRIPT -p build -j8 $(cat files.txt)
+fi
+rm -f files.txt

include/pulsar/Authentication.h

@@ -394,10 +394,7 @@ class PULSAR_PUBLIC AuthAthenz : public Authentication {
 // currently mainly works for access token
 class Oauth2TokenResult {
    public:
-    enum
-    {
-        undefined_expiration = -1
-    };
+    static constexpr int undefined_expiration = -1;
 
     Oauth2TokenResult();
     ~Oauth2TokenResult();

include/pulsar/Client.h

@@ -101,7 +101,7 @@ class PULSAR_PUBLIC Client {
      * @param callback the callback that is triggered when the producer is created successfully or not
      * @param callback Callback function that is invoked when the operation is completed
      */
-    void createProducerAsync(const std::string& topic, CreateProducerCallback callback);
+    void createProducerAsync(const std::string& topic, const CreateProducerCallback& callback);
 
     /**
      * Asynchronously create a producer with the customized ProducerConfiguration for publishing on a specific
@@ -110,8 +110,8 @@ class PULSAR_PUBLIC Client {
      * @param topic the name of the topic where to produce
      * @param conf the customized ProducerConfiguration
      */
-    void createProducerAsync(const std::string& topic, ProducerConfiguration conf,
-                             CreateProducerCallback callback);
+    void createProducerAsync(const std::string& topic, const ProducerConfiguration& conf,
+                             const CreateProducerCallback& callback);
 
     /**
      * Subscribe to a given topic and subscription combination with the default ConsumerConfiguration
@@ -144,7 +144,7 @@ class PULSAR_PUBLIC Client {
      * default ConsumerConfiguration are asynchronously subscribed successfully or not
      */
     void subscribeAsync(const std::string& topic, const std::string& subscriptionName,
-                        SubscribeCallback callback);
+                        const SubscribeCallback& callback);
 
     /**
      * Asynchronously subscribe to a given topic and subscription combination with the customized
@@ -157,7 +157,7 @@ class PULSAR_PUBLIC Client {
      * customized ConsumerConfiguration are asynchronously subscribed successfully or not
      */
     void subscribeAsync(const std::string& topic, const std::string& subscriptionName,
-                        const ConsumerConfiguration& conf, SubscribeCallback callback);
+                        const ConsumerConfiguration& conf, const SubscribeCallback& callback);
 
     /**
      * Subscribe to multiple topics under the same namespace.
@@ -191,7 +191,7 @@ class PULSAR_PUBLIC Client {
 
      */
     void subscribeAsync(const std::vector<std::string>& topics, const std::string& subscriptionName,
-                        SubscribeCallback callback);
+                        const SubscribeCallback& callback);
 
     /**
      * Asynchronously subscribe to a list of topics and subscription combination using the customized
@@ -204,7 +204,7 @@ class PULSAR_PUBLIC Client {
      * the customized ConsumerConfiguration are asynchronously subscribed successfully or not
      */
     void subscribeAsync(const std::vector<std::string>& topics, const std::string& subscriptionName,
-                        const ConsumerConfiguration& conf, SubscribeCallback callback);
+                        const ConsumerConfiguration& conf, const SubscribeCallback& callback);
 
     /**
      * Subscribe to multiple topics, which match given regexPattern, under the same namespace.
@@ -227,7 +227,7 @@ class PULSAR_PUBLIC Client {
      * SubscribeCallback)
      */
     void subscribeWithRegexAsync(const std::string& regexPattern, const std::string& subscriptionName,
-                                 SubscribeCallback callback);
+                                 const SubscribeCallback& callback);
 
     /**
      * Asynchronously subscribe to multiple topics (which match given regexPatterns) with the customized
@@ -240,7 +240,7 @@ class PULSAR_PUBLIC Client {
      * ConsumerConfiguration under the same namespace are asynchronously subscribed successfully or not
      */
     void subscribeWithRegexAsync(const std::string& regexPattern, const std::string& subscriptionName,
-                                 const ConsumerConfiguration& conf, SubscribeCallback callback);
+                                 const ConsumerConfiguration& conf, const SubscribeCallback& callback);
 
     /**
      * Create a topic reader with given {@code ReaderConfiguration} for reading messages from the specified
@@ -301,7 +301,7 @@ class PULSAR_PUBLIC Client {
      * @return the Reader object
      */
     void createReaderAsync(const std::string& topic, const MessageId& startMessageId,
-                           const ReaderConfiguration& conf, ReaderCallback callback);
+                           const ReaderConfiguration& conf, const ReaderCallback& callback);
 
     /**
      * Create a table view with given {@code TableViewConfiguration} for specified topic.
@@ -331,7 +331,7 @@ class PULSAR_PUBLIC Client {
      * built from a message that already exists
      */
     void createTableViewAsync(const std::string& topic, const TableViewConfiguration& conf,
-                              TableViewCallback callBack);
+                              const TableViewCallback& callBack);
 
     /**
      * Get the list of partitions for a given topic.
@@ -363,7 +363,7 @@ class PULSAR_PUBLIC Client {
      *            the callback that will be invoked when the list of partitions is available
      * @since 2.3.0
      */
-    void getPartitionsForTopicAsync(const std::string& topic, GetPartitionsCallback callback);
+    void getPartitionsForTopicAsync(const std::string& topic, const GetPartitionsCallback& callback);
 
     /**
      *
@@ -380,7 +380,7 @@ class PULSAR_PUBLIC Client {
      * @param callback the callback that is triggered when the Pulsar client is asynchronously closed
      * successfully or not
      */
-    void closeAsync(CloseCallback callback);
+    void closeAsync(const CloseCallback& callback);
 
     /**
      * Perform immediate shutdown of Pulsar client.
@@ -415,7 +415,7 @@ class PULSAR_PUBLIC Client {
                             std::function<void(Result, const SchemaInfo&)> callback);
 
    private:
-    Client(const std::shared_ptr<ClientImpl>);
+    Client(const std::shared_ptr<ClientImpl>&);
 
     friend class PulsarFriend;
     friend class PulsarWrapper;

include/pulsar/ClientConfiguration.h

@@ -23,6 +23,8 @@
 #include <pulsar/Logger.h>
 #include <pulsar/defines.h>
 
+#include <cstdint>
+
 namespace pulsar {
 class PulsarWrapper;
 struct ClientConfigurationImpl;
@@ -32,7 +34,7 @@ class PULSAR_PUBLIC ClientConfiguration {
     ~ClientConfiguration();
     ClientConfiguration(const ClientConfiguration&);
     ClientConfiguration& operator=(const ClientConfiguration&);
-    enum ProxyProtocol
+    enum ProxyProtocol : uint8_t
     {
         SNI = 0
     };

include/pulsar/CompressionType.h

@@ -19,8 +19,10 @@
 #ifndef PULSAR_COMPRESSIONTYPE_H_
 #define PULSAR_COMPRESSIONTYPE_H_
 
+#include <cstdint>
+
 namespace pulsar {
-enum CompressionType
+enum CompressionType : std::uint8_t
 {
     CompressionNone = 0,
     CompressionLZ4 = 1,

include/pulsar/ConsoleLoggerFactory.h

@@ -21,6 +21,8 @@
 
 #include <pulsar/Logger.h>
 
+#include <memory>
+
 namespace pulsar {
 
 class ConsoleLoggerFactoryImpl;