feat: add cppcheck into ci (#291)

* get cppcheck suppressions file Signed-off-by: ISP akm <[email protected]> * change NOLINT->cppcheck-suppress Signed-off-by: ISP akm <[email protected]> * NOLINT->cppcheck-suppress Signed-off-by: ISP akm <[email protected]> * add cppcheck-all-files and remove clang-tidy-differential Signed-off-by: ISP akm <[email protected]> * add NOLINT Signed-off-by: ISP akm <[email protected]> * move suppression comments Signed-off-by: ISP akm <[email protected]> * move suppression comments Signed-off-by: ISP akm <[email protected]> * move suppression comments Signed-off-by: ISP akm <[email protected]> * move suppression comments Signed-off-by: ISP akm <[email protected]> * move suppression comments Signed-off-by: ISP akm <[email protected]> * move suppression comments Signed-off-by: ISP akm <[email protected]> * Update cppcheck-differential.yaml * Update trace_node.cpp * temporary change Signed-off-by: ISP akm <[email protected]> * change .cppcheck-suppressions download timing Signed-off-by: ISP akm <[email protected]> * add LF to build-and-test-differential.yaml Signed-off-by: ISP akm <[email protected]> * ci(pre-commit): autofix * modify cppcheck-differential Signed-off-by: ISP akm <[email protected]> * formatting cppcheck-differential Signed-off-by: ISP akm <[email protected]> * modify cppcheck-differential Signed-off-by: ISP akm <[email protected]> * add install snapd Signed-off-by: ISP akm <[email protected]> * fix workflow version Signed-off-by: ISP akm <[email protected]> * cppcheck is installed via apt-get Signed-off-by: ISP akm <[email protected]> * remove snapd and ros2 container Signed-off-by: ISP akm <[email protected]> * fix files variable setting Signed-off-by: ISP akm <[email protected]> * fix files variable setting Signed-off-by: ISP akm <[email protected]> * restore clang-tidy-differential Signed-off-by: ISP akm <[email protected]> --------- Signed-off-by: ISP akm <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
tier4 · Jun 27, 2024 · 8358563 · 8358563
1 parent 0bf777d
commit 8358563
Show file tree

Hide file tree

Showing 7 changed files with 161 additions and 7 deletions.
diff --git a/.github/workflows/cppcheck-all-files.yaml b/.github/workflows/cppcheck-all-files.yaml
@@ -0,0 +1,63 @@
+name: cppcheck-all-files
+
+on:
+  pull_request:
+  workflow_dispatch:
+
+jobs:
+  cppcheck-all-files:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out repository
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+
+      - name: Install dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y wget
+
+      # cppcheck from apt does not yet support --check-level args, and thus install from snap
+      - name: Install Cppcheck from snap
+        run: |
+          sudo snap install cppcheck
+
+      # Download the cppcheck suppression file
+      - name: Download cppcheck suppression file
+        run: |
+          wget https://raw.githubusercontent.com/autowarefoundation/autoware.universe/main/.cppcheck_suppressions -O .cppcheck_suppressions
+
+      - name: Get all C++ files
+        id: all-files
+        run: |
+          find . -type f \( -name "*.cpp" -o -name "*.hpp" \) > all_files.txt
+          cat all_files.txt
+
+      - name: Run Cppcheck on all files
+        continue-on-error: true
+        id: cppcheck
+        run: |
+          files=$(cat all_files.txt)
+          if [ -n "$files" ]; then
+            echo "Running Cppcheck on all files: $files"
+            cppcheck --inline-suppr --enable=all --inconclusive --check-level=exhaustive --error-exitcode=1 --suppressions-list=.cppcheck_suppressions $files 2> cppcheck-report.txt
+          else
+            echo "No C++ files found."
+            touch cppcheck-report.txt
+          fi
+        shell: bash
+
+      - name: Show cppcheck-report result
+        run: |
+          cat cppcheck-report.txt
+
+      - name: Upload Cppcheck report
+        uses: actions/upload-artifact@v4
+        with:
+          name: cppcheck-report
+          path: cppcheck-report.txt
+
+      - name: Fail the job if Cppcheck failed
+        if: ${{ steps.cppcheck.outcome == 'failure' }}
+        run: exit 1
diff --git a/.github/workflows/cppcheck-differential.yaml b/.github/workflows/cppcheck-differential.yaml
@@ -0,0 +1,83 @@
+name: cppcheck-differential
+
+on:
+  pull_request:
+
+jobs:
+  cppcheck-differential:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out repository
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+
+      - name: Install dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y wget
+
+      # cppcheck from apt does not yet support --check-level args, and thus install from snap
+      - name: Install Cppcheck from snap
+        run: |
+          sudo snap install cppcheck
+
+      # Download the cppcheck suppression file
+      - name: Download cppcheck suppression file
+        run: |
+          wget https://raw.githubusercontent.com/autowarefoundation/autoware.universe/main/.cppcheck_suppressions -O .cppcheck_suppressions
+
+      - name: Remove exec_depend
+        uses: autowarefoundation/autoware-github-actions/remove-exec-depend@v1
+
+      - name: Get modified packages
+        id: get-modified-packages
+        uses: autowarefoundation/autoware-github-actions/get-modified-packages@v1
+
+      # Display the modified packages
+      - name: Show modified packages
+        run: |
+          echo "Modified packages:"
+          echo "${{ steps.get-modified-packages.outputs.modified_packages }}"
+
+      - name: Get modified files
+        id: get-modified-files
+        uses: tj-actions/changed-files@v44
+        with:
+          files: |
+            **/*.cpp
+            **/*.hpp
+
+      # Display the modified files
+      - name: Show modified files
+        run: |
+          echo "Modified files:"
+          echo "${{ steps.get-modified-files.outputs.all_changed_files }}"
+
+      - name: Run Cppcheck on changed files
+        continue-on-error: true
+        id: cppcheck
+        run: |
+          files=$(echo ${{ steps.get-modified-files.outputs.all_changed_files }})
+          if [ -n "$files" ]; then
+            echo "Running Cppcheck on changed files: $files"
+            cppcheck --inline-suppr --enable=all --inconclusive --check-level=exhaustive --error-exitcode=1 --suppressions-list=.cppcheck_suppressions $files 2> cppcheck-report.txt
+          else
+            echo "No C++ files changed."
+            touch cppcheck-report.txt
+          fi
+        shell: bash
+
+      - name: Show cppcheck-report result
+        run: |
+          cat cppcheck-report.txt
+
+      - name: Upload Cppcheck report
+        uses: actions/upload-artifact@v4
+        with:
+          name: cppcheck-report
+          path: cppcheck-report.txt
+
+      - name: Fail the job if Cppcheck failed
+        if: steps.cppcheck.outcome == 'failure'
+        run: exit 1
diff --git a/CARET_trace/include/caret_trace/data_container.hpp b/CARET_trace/include/caret_trace/data_container.hpp
@@ -186,11 +186,11 @@ class DataContainer : public DataContainerInterface
     std::shared_ptr<RclcppIpbToSubscription::KeysT> rclcpp_ipb_to_subscription,
     std::shared_ptr<RmwImplementation::KeysT> rmw_implementation);
 
-  bool record(uint64_t loop_count = 1);
+  bool record(uint64_t loop_count = 1) override;
 
-  void start_recording();
+  void start_recording() override;
 
-  void reset();
+  void reset() override;
 
   /// @brief Store data for add_callback_group trace points.
   /// @tparam ...Args Data types to be stored.

diff --git a/CARET_trace/include/caret_trace/lttng_session.hpp b/CARET_trace/include/caret_trace/lttng_session.hpp
@@ -46,10 +46,10 @@ class LttngSessionImpl : public LttngSession
 public:
   LttngSessionImpl() : started_session_running_(is_session_running()) {}
 
-  ~LttngSessionImpl() {}
+  ~LttngSessionImpl() override {}
 
-  bool is_session_running() const;
-  bool started_session_running() const;
+  bool is_session_running() const override;
+  bool started_session_running() const override;
 
 private:
   mutable std::mutex mtx_;

diff --git a/CARET_trace/include/caret_trace/trace_node.hpp b/CARET_trace/include/caret_trace/trace_node.hpp
@@ -78,7 +78,7 @@ class TraceNode : public rclcpp::Node, public TraceNodeInterface
     rclcpp::Logger::Level level = rclcpp::Logger::Level::Info, bool use_log = false,
     bool execute_timer_on_run = true);
 
-  ~TraceNode();
+  ~TraceNode() override;
 
   /// @brief Check whther current status allows recording.
   /// @return True if recording is allowed, false otherwise.

diff --git a/CARET_trace/src/trace_node.cpp b/CARET_trace/src/trace_node.cpp
@@ -166,12 +166,14 @@ bool TraceNode::is_recording_allowed() const
 bool TraceNode::is_recording_allowed_init() const
 {
   return true;
+#if 0  // ignore following
   std::shared_lock<std::shared_mutex> lock(mutex_);
 
   // NOTE: Since PREPARE to RECORD is a continuous state transition
   // with a return value of TRUE, no mutex is required.
   // On the other hand, the transition to the PREPARE state is a boundary, so a mutex is required.
   return status_ == TRACE_STATUS::RECORD || status_ == TRACE_STATUS::PREPARE;
+#endif
 }
 
 const TRACE_STATUS & TraceNode::get_status() const

diff --git a/CARET_trace/test/test_scenario.cpp b/CARET_trace/test/test_scenario.cpp
@@ -38,11 +38,13 @@ using rclcpp::Logger;
 
 void add_data(DataContainer & container, int loop)
 {
+  // cppcheck-suppress-begin nullPointerArithmetic
   char * ptr = nullptr;
   for (auto i = 0; i < loop; i++) {
     ptr++;
     container.store_rcl_init(ptr, 0);
   }
+  // cppcheck-suppress-end nullPointerArithmetic
 }
 
 void record_data(DataContainer & container, int loop)
@@ -66,11 +68,13 @@ TEST(ScenarioTest, TestSingleThread)
 
   auto loop = 1000;
 
+  // cppcheck-suppress-begin nullPointerArithmetic
   char * ptr = nullptr;
   for (auto i = 0; i < loop; i++) {
     ptr++;
     EXPECT_CALL(rcl_init_mock, Call(ptr, 0)).Times(1);
   }
+  // cppcheck-suppress-end nullPointerArithmetic
 
   add_data(container, loop);
   container.record(loop);
@@ -92,11 +96,13 @@ TEST(ScenarioTest, TestMultiThread)
 
   auto loop = 1000;
 
+  // cppcheck-suppress-begin nullPointerArithmetic
   char * ptr = nullptr;
   for (auto i = 0; i < loop; i++) {
     ptr++;
     EXPECT_CALL(rcl_init_mock, Call(ptr, 0)).WillRepeatedly(Return());
   }
+  // cppcheck-suppress-end nullPointerArithmetic
 
   // NOTE: Ensure recording data. Avoid container.record() before add_data() is called.
   add_data(container, 1);