clang-tidy safety/security reports

CLANG_TIDY_CONFIGURATION.md

@@ -82,20 +82,20 @@ The project uses clang-tidy to enforce modern C++23 best practices and C++ Core
 The configuration enforces consistent naming:
 
 - **Namespaces:** `lower_case`
-- **Classes/Structs/Enums:** `CamelCase`
+- **Classes/Structs/Enums:** `lower_case`
 - **Functions:** `lower_case`
 - **Variables/Parameters:** `lower_case`
-- **Private/Protected Members:** `m_` prefix + `lower_case`
-- **Constants/Enum Values:** `UPPER_CASE`
-- **Type Aliases/Typedefs:** `CamelCase`
+- **Private/Protected/Constant Members:** `lower_case` with `_` suffix
+- **Macros:** `UPPER_CASE`
+- **Constants/Enum Values:** `lower_case`
+- **Type Aliases/Typedefs:** `lower_case`
 - **Template Parameters:** `CamelCase`
 
 ### Function Complexity Limits
 
 - **Line Threshold:** 100 lines per function
 - **Statement Threshold:** 50 statements per function
 - **Branch Threshold:** 10 branches per function
-- **Parameter Threshold:** 6 parameters per function
 
 ## GitHub Actions Workflows
 
@@ -106,23 +106,23 @@ The configuration enforces consistent naming:
 **Features:**
 
 - Runs on PR to main branch and manual trigger
-- Uses clang-tidy version 20 via CMake target
-- Configures project and builds it first (required for accurate analysis)
-- Uses CMake's `clang-tidy-check` target for consistency
+- Uses clang-tidy via `CMAKE_CXX_CLANG_TIDY` during build
+- Configures project with compile commands export
 - Reports warnings and errors with detailed output
-- Uploads clang-tidy log as artifact on failure
+- Uploads clang-tidy fixes YAML and log as artifacts
+- Posts inline PR comments for detected issues
 
 **How it works:**
 
-1. Configure the project with `CMAKE_EXPORT_COMPILE_COMMANDS=ON`
-2. Build the project to ensure all generated files exist
-3. Run `cmake --build . --target clang-tidy-check`
-4. CMake automatically runs clang-tidy on all project source files
+1. Configure the project with `CMAKE_EXPORT_COMPILE_COMMANDS=ON` and `CMAKE_CXX_CLANG_TIDY='clang-tidy;--export-fixes=clang-tidy-fixes.yaml'`
+2. Build the project - clang-tidy runs automatically on each source file during compilation
+3. Parse output and upload artifacts
+4. Post inline comments on PR with issue details
 
 **How to use:**
 
 - Automatically runs on every pull request
-- Review the workflow output for details on any issues
+- Review the workflow output and inline PR comments for details
 - Comment `@phlexbot tidy-fix` to attempt automatic fixes
 
 ### Clang-Tidy Fix (`clang-tidy-fix.yaml`)
@@ -131,17 +131,19 @@ The configuration enforces consistent naming:
 
 **Features:**
 
-- Triggered by `@phlexbot tidy-fix` comment on PRs
-- Uses clang-tidy version 20 via CMake target
-- Builds project first, then applies fixes using CMake's `clang-tidy-fix` target
+- Triggered by `@phlexbot tidy-fix [<check>...]` comment on PRs
+- Optionally specify specific checks to fix
+- Attempts to reuse existing fixes from check workflow artifacts
+- Falls back to running clang-tidy with `CMAKE_CXX_CLANG_TIDY` if needed
+- Applies fixes using `clang-apply-replacements`
 - Commits and pushes changes automatically
-- Comments on PR with status
+- Posts inline PR comments with remaining issues
 
 **How it works:**
 
-1. Configure and build the project
-2. Run `cmake --build . --target clang-tidy-fix`
-3. CMake runs clang-tidy with `--fix --fix-errors` on all sources
+1. Try to download existing `clang-tidy-fixes.yaml` from check workflow
+2. If not available, configure and build with clang-tidy to generate fixes
+3. Apply fixes using `clang-apply-replacements`
 4. Commit and push any changes
 
 **Important notes:**
@@ -160,39 +162,26 @@ The configuration enforces consistent naming:
 
 ## CMake Integration
 
-The project provides CMake targets for clang-tidy analysis:
-
 ### Build-Time Integration
 
-Enable clang-tidy during compilation:
+Enable clang-tidy during compilation using `CMAKE_CXX_CLANG_TIDY`:
 
 ```bash
-cmake -DENABLE_CLANG_TIDY=ON /path/to/source
-cmake --build .
+cmake -DCMAKE_CXX_CLANG_TIDY='clang-tidy' -B build -S .
+cmake --build build
 ```
 
 When enabled, clang-tidy runs automatically on every C++ file during compilation, providing immediate feedback.
 
-### CMake Targets
-
-**`clang-tidy-check`** - Run clang-tidy on all project sources (read-only):
-
-```bash
-cmake --build . --target clang-tidy-check
-```
-
-**`clang-tidy-fix`** - Apply clang-tidy fixes to all project sources:
+To export fixes for later application:
 
 ```bash
-cmake --build . --target clang-tidy-fix
+cmake -DCMAKE_CXX_CLANG_TIDY='clang-tidy;--export-fixes=clang-tidy-fixes.yaml' -B build -S .
+cmake --build build
+clang-apply-replacements build
 ```
 
-These targets:
-
-- Use the `.clang-tidy` configuration file automatically
-- Only analyze project source files (not dependencies)
-- Work with the project's compile_commands.json
-- Are generator-independent (work with Ninja, Make, etc.)
+Note: The project sets `CMAKE_EXPORT_COMPILE_COMMANDS=ON` by default, which generates `compile_commands.json` for clang-tidy to use.
 
 ## Integration with Existing Workflows
 
@@ -205,49 +194,45 @@ Both can be run independently or together as needed.
 
 ## Local Usage
 
-### Using CMake Targets (Recommended)
+### Build-Time Checks (Recommended)
 
-The project provides CMake targets for clang-tidy:
+Enable clang-tidy to run on every file during compilation:
 
 ```bash
-# Configure project (compile_commands.json is created automatically)
-cmake --preset=default /path/to/source
+cmake -DCMAKE_CXX_CLANG_TIDY='clang-tidy' -B build -S .
+cmake --build build -j $(nproc)
+```
 
-# Build the project first
-cmake --build . -j $(nproc)
+This provides immediate feedback as you build, catching issues early.
 
-# Run clang-tidy checks
-cmake --build . --target clang-tidy-check
+### Generate and Apply Fixes
 
-# Apply automatic fixes
-cmake --build . --target clang-tidy-fix
-```
+To generate fixes and apply them:
 
-### Build-Time Checks
+```bash
+# Configure with fix export
+cmake -DCMAKE_CXX_CLANG_TIDY='clang-tidy;--export-fixes=clang-tidy-fixes.yaml' -B build -S .
 
-Enable clang-tidy to run on every file during compilation:
+# Build (generates fixes)
+cmake --build build -j $(nproc)
 
-```bash
-cmake -DENABLE_CLANG_TIDY=ON /path/to/source
-cmake --build .
+# Apply fixes
+clang-apply-replacements build
 ```
 
-This provides immediate feedback as you build, catching issues early.
-
 ### Manual Invocation
 
 For manual control, you can run clang-tidy directly:
 
 ```bash
 # Check a specific file
-clang-tidy-20 -p /path/to/build file.cpp
+clang-tidy -p build phlex/core/framework_graph.cpp
 
 # Apply fixes to a specific file
-clang-tidy-20 -p /path/to/build --fix file.cpp
+clang-tidy -p build --fix phlex/core/framework_graph.cpp
 
 # Check all project files
-find srcs/phlex -name "*.cpp" | \
-  xargs clang-tidy-20 -p /path/to/build
+find phlex -name "*.cpp" | xargs clang-tidy -p build
 ```
 
 ## VS Code Integration

CMakeLists.txt

@@ -63,7 +63,6 @@ option(ENABLE_TSAN "Enable Thread Sanitizer" OFF)
 option(ENABLE_ASAN "Enable Address Sanitizer" OFF)
 option(PHLEX_USE_FORM "Enable experimental integration with FORM" OFF)
 option(ENABLE_COVERAGE "Enable code coverage instrumentation" OFF)
-option(ENABLE_CLANG_TIDY "Enable clang-tidy checks during build" OFF)
 
 add_compile_options(
   -Wall
@@ -163,22 +162,6 @@ if(ENABLE_COVERAGE)
   endif()
 endif()
 
-# Configure clang-tidy integration
-find_program(CLANG_TIDY_EXECUTABLE NAMES clang-tidy-20 clang-tidy)
-
-if(ENABLE_CLANG_TIDY)
-  if(CLANG_TIDY_EXECUTABLE)
-    message(STATUS "Found clang-tidy: ${CLANG_TIDY_EXECUTABLE}")
-    set(
-      CMAKE_CXX_CLANG_TIDY
-      ${CLANG_TIDY_EXECUTABLE}
-      --config-file=${CMAKE_SOURCE_DIR}/.clang-tidy
-    )
-  else()
-    message(WARNING "clang-tidy not found, disabling clang-tidy checks")
-  endif()
-endif()
-
 if(NOT CMAKE_BUILD_TYPE)
   set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "Build type" FORCE)
 endif()
@@ -202,10 +185,4 @@ if(BUILD_TESTING)
   endif()
 endif()
 
-# Report clang-tidy availability
-if(CLANG_TIDY_EXECUTABLE)
-  message(STATUS "Clang-tidy available: ${CLANG_TIDY_EXECUTABLE}")
-  message(STATUS "Use -DCMAKE_CXX_CLANG_TIDY=clang-tidy to enable automatic checks during build")
-endif()
-
 cet_cmake_config()

phlex/app/load_module.cpp

@@ -9,6 +9,7 @@
 #include "boost/dll/import.hpp"
 #include "boost/json.hpp"
 
+#include <cstdlib>
 #include <functional>
 #include <string>
 #include <string_view>
@@ -29,7 +30,9 @@ namespace phlex::experimental {
     template <typename creator_t>
     std::function<creator_t> plugin_loader(std::string const& spec, std::string const& symbol_name)
     {
-      char const* plugin_path_ptr = std::getenv("PHLEX_PLUGIN_PATH");
+      // Called during single-threaded graph construction
+      char const* plugin_path_ptr =
+        std::getenv("PHLEX_PLUGIN_PATH"); // NOLINT(concurrency-mt-unsafe)
       if (!plugin_path_ptr)
         throw std::runtime_error("PHLEX_PLUGIN_PATH has not been set.");
 

test/form/toy_tracker.cpp

@@ -1,6 +1,8 @@
 #include "toy_tracker.hpp"
 #include "data_products/track_start.hpp"
 
+#include <cstdlib>
+
 ToyTracker::ToyTracker(int maxTracks) : m_maxTracks(maxTracks) {}
 
 std::vector<TrackStart> ToyTracker::operator()()
@@ -19,7 +21,7 @@ std::vector<TrackStart> ToyTracker::operator()()
 int32_t ToyTracker::generateRandom()
 {
   //Get a 32-bit random integer with even the lowest allowed precision of rand()
-  int rand1 = rand() % 32768;
-  int rand2 = rand() % 32768;
+  int rand1 = rand() % 32768; // NOLINT(concurrency-mt-unsafe) - Test code, single-threaded
+  int rand2 = rand() % 32768; // NOLINT(concurrency-mt-unsafe) - Test code, single-threaded
   return (rand1 * 32768 + rand2);
 }

test/form/writer.cpp

@@ -20,12 +20,12 @@ static char const* const seg_id = "[EVENT=%08X;SEG=%08X]";
 
 void generate(std::vector<float>& vrand, int size)
 {
-  int rand1 = rand() % 32768;
-  int rand2 = rand() % 32768;
+  int rand1 = rand() % 32768; // NOLINT(concurrency-mt-unsafe) - Single-threaded test
+  int rand2 = rand() % 32768; // NOLINT(concurrency-mt-unsafe) - Single-threaded test
   int npx = (rand1 * 32768 + rand2) % size;
   for (int nelement = 0; nelement < npx; ++nelement) {
-    int rand1 = rand() % 32768;
-    int rand2 = rand() % 32768;
+    int rand1 = rand() % 32768; // NOLINT(concurrency-mt-unsafe) - Single-threaded test
+    int rand2 = rand() % 32768; // NOLINT(concurrency-mt-unsafe) - Single-threaded test
     float random = float(rand1 * 32768 + rand2) / (32768 * 32768);
     vrand.push_back(random);
   }

test/hierarchical_nodes.cpp

@@ -22,12 +22,13 @@
 #include "test/products_for_output.hpp"
 
 #include "catch2/catch_test_macros.hpp"
+#include "fmt/chrono.h"
 #include "fmt/std.h"
 #include "spdlog/spdlog.h"
 
 #include <atomic>
+#include <chrono>
 #include <cmath>
-#include <ctime>
 #include <string>
 
 using namespace phlex;
@@ -64,19 +65,14 @@ namespace {
     return std::sqrt(static_cast<double>(data.total) / data.number);
   }
 
-  std::string strtime(std::time_t tm)
+  std::string strtime(std::chrono::system_clock::time_point tp)
   {
-    char buffer[32];
-    std::strncpy(buffer, std::ctime(&tm), 26);
-    return buffer;
+    return fmt::format("{:%a %b %d %H:%M:%S %Y}", tp);
   }
 
   void print_result(handle<double> result, std::string const& stringized_time)
   {
-    spdlog::debug("{}: {} @ {}",
-                  result.data_cell_index().to_string(),
-                  *result,
-                  stringized_time.substr(0, stringized_time.find('\n')));
+    spdlog::debug("{}: {} @ {}", result.data_cell_index().to_string(), *result, stringized_time);
   }
 }
 
@@ -89,9 +85,9 @@ TEST_CASE("Hierarchical nodes", "[graph]")
   experimental::framework_graph g{driver_for_test(gen)};
 
   g.provide("provide_time",
-            [](data_cell_index const& index) -> std::time_t {
+            [](data_cell_index const& index) {
               spdlog::info("Providing time for {}", index.to_string());
-              return std::time(nullptr);
+              return std::chrono::system_clock::now();
             })
     .output_product(product_query{.creator = "input"_id, .layer = "run"_id, .suffix = "time"_id});