Add clang tidy

.clang-tidy

@@ -1,155 +1,18 @@
 ---
-# Enable ALL the things! Except not really
-# misc-non-private-member-variables-in-classes: the options don't do anything
-# modernize-use-nodiscard: too aggressive, attribute is situationally useful
-Checks: "*,\
-  -google-readability-todo,\
-  -altera-*,\
-  -fuchsia-*,\
-  fuchsia-multiple-inheritance,\
-  -llvm-header-guard,\
-  -llvm-include-order,\
-  -llvmlibc-*,\
-  -modernize-use-nodiscard,\
-  -misc-non-private-member-variables-in-classes"
+# Keep the initial CI policy focused on diagnostics that are actionable and
+# complementary to cppcheck, sanitizers, and clang-format.
+Checks: >-
+  clang-analyzer-*,
+  bugprone-*,
+  performance-*,
+  -bugprone-derived-method-shadowing-base-method,
+  -bugprone-easily-swappable-parameters,
+  -bugprone-macro-parentheses,
+  -bugprone-throwing-static-initialization,
+  -performance-avoid-endl,
+  -performance-enum-size,
+  -performance-unnecessary-value-param
 WarningsAsErrors: ''
-CheckOptions:
-  - key: 'bugprone-argument-comment.StrictMode'
-    value: 'true'
-# Prefer using enum classes with 2 values for parameters instead of bools
-  - key: 'bugprone-argument-comment.CommentBoolLiterals'
-    value: 'true'
-  - key: 'bugprone-misplaced-widening-cast.CheckImplicitCasts'
-    value: 'true'
-  - key: 'bugprone-sizeof-expression.WarnOnSizeOfIntegerExpression'
-    value: 'true'
-  - key: 'bugprone-suspicious-string-compare.WarnOnLogicalNotComparison'
-    value: 'true'
-  - key: 'readability-simplify-boolean-expr.ChainedConditionalReturn'
-    value: 'true'
-  - key: 'readability-simplify-boolean-expr.ChainedConditionalAssignment'
-    value: 'true'
-  - key: 'readability-uniqueptr-delete-release.PreferResetCall'
-    value: 'true'
-  - key: 'cppcoreguidelines-init-variables.MathHeader'
-    value: '<cmath>'
-  - key: 'cppcoreguidelines-narrowing-conversions.PedanticMode'
-    value: 'true'
-  - key: 'readability-else-after-return.WarnOnUnfixable'
-    value: 'true'
-  - key: 'readability-else-after-return.WarnOnConditionVariables'
-    value: 'true'
-  - key: 'readability-inconsistent-declaration-parameter-name.Strict'
-    value: 'true'
-  - key: 'readability-qualified-auto.AddConstToQualified'
-    value: 'true'
-  - key: 'readability-redundant-access-specifiers.CheckFirstDeclaration'
-    value: 'true'
-# These seem to be the most common identifier styles
-  - key: 'readability-identifier-naming.AbstractClassCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.ClassCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.ClassConstantCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.ClassMemberCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.ClassMethodCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.ConstantCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.ConstantMemberCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.ConstantParameterCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.ConstantPointerParameterCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.ConstexprFunctionCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.ConstexprMethodCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.ConstexprVariableCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.EnumCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.EnumConstantCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.FunctionCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.GlobalConstantCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.GlobalConstantPointerCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.GlobalFunctionCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.GlobalPointerCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.GlobalVariableCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.InlineNamespaceCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.LocalConstantCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.LocalConstantPointerCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.LocalPointerCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.LocalVariableCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.MacroDefinitionCase'
-    value: 'UPPER_CASE'
-  - key: 'readability-identifier-naming.MemberCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.MethodCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.NamespaceCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.ParameterCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.ParameterPackCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.PointerParameterCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.PrivateMemberCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.PrivateMemberPrefix'
-    value: 'm_'
-  - key: 'readability-identifier-naming.PrivateMethodCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.ProtectedMemberCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.ProtectedMemberPrefix'
-    value: 'm_'
-  - key: 'readability-identifier-naming.ProtectedMethodCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.PublicMemberCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.PublicMethodCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.ScopedEnumConstantCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.StaticConstantCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.StaticVariableCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.StructCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.TemplateParameterCase'
-    value: 'CamelCase'
-  - key: 'readability-identifier-naming.TemplateTemplateParameterCase'
-    value: 'CamelCase'
-  - key: 'readability-identifier-naming.TypeAliasCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.TypedefCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.TypeTemplateParameterCase'
-    value: 'CamelCase'
-  - key: 'readability-identifier-naming.UnionCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.ValueTemplateParameterCase'
-    value: 'CamelCase'
-  - key: 'readability-identifier-naming.VariableCase'
-    value: 'lower_case'
-  - key: 'readability-identifier-naming.VirtualMethodCase'
-    value: 'lower_case'
+HeaderFilterRegex: '^(.*[/\\])?src/'
+FormatStyle: file
 ...

.github/workflows/clang-tidy.yml

@@ -0,0 +1,49 @@
+name: Clang-Tidy
+
+permissions:
+  contents: read
+
+on:
+  push:
+    branches:
+    - main
+    paths:
+    - 'src/**'
+    - '.clang-tidy'
+    - 'CMakeLists.txt'
+    - 'CMakePresets.json'
+    - 'cmake/**'
+  pull_request:
+    paths:
+    - 'src/**'
+    - '.clang-tidy'
+    - 'CMakeLists.txt'
+    - 'CMakePresets.json'
+    - 'cmake/**'
+  workflow_dispatch:
+
+jobs:
+  clang-tidy:
+    name: Run clang-tidy static analysis
+    runs-on: ubuntu-latest
+
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v4
+
+    - name: Install dependencies
+      run: |
+        sudo apt-get update
+        sudo apt-get install -y clang-tidy libhdf5-dev
+        git clone https://github.com/catchorg/Catch2.git
+        cd Catch2
+        git checkout "v3.5.3"
+        cmake -Bbuild -H. -DBUILD_TESTING=OFF
+        sudo cmake --build build/ --target install
+
+    - name: Configure compilation database
+      run: cmake --preset=ci-ubuntu -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
+
+    - name: Run clang-tidy
+      continue-on-error: true
+      run: cmake --build build --target tidy-check

CHANGELOG.md

@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
     * Added tutorial on using the ROS3 and remfile drivers to read NWB files in S3 (`docs/pages/userdocs/reads3.dox`) (@oruebel, [#308](https://github.com/NeurodataWithoutBorders/aqnwb/pull/308); @bendichter, [#309](https://github.com/NeurodataWithoutBorders/aqnwb/pull/309))
 * Added `ElectricalSeries::writeAllChannels` method and `IO::writeElectricalSeriesData` overload to simplify zero-copy interleaved multichannel writes. (@copilot, @oruebel, [#293](https://github.com/NeurodataWithoutBorders/aqnwb/pull/293))
 * Added `ElectricalSeries::channelsAtSameSampleOffset` method to check if all channels are at the same sample offset, which is a requirement for using `writeAllChannels`. (@copilot, @oruebel, [#293](https://github.com/NeurodataWithoutBorders/aqnwb/pull/293))
+* Added an advisory clang-tidy workflow and `tidy-check` CMake target using compilation databases to analyze focused correctness and performance checks on production source changes. ([#322](https://github.com/NeurodataWithoutBorders/aqnwb/pull/322))
 * Added new `BaseIO::findObject` and `RegisteredType::findOwnedObject` methods to simplify searching for objects by name. Added `HDF5IO::findObject` override method to optimize the search for HDF5 objects. (@oruebel, [#308](https://github.com/NeurodataWithoutBorders/aqnwb/pull/308))   
 
 ### Changed

CMakePresets.json

@@ -36,13 +36,6 @@
         "CMAKE_CXX_CPPCHECK": "cppcheck;--inline-suppr"
       }
     },
-    {
-      "name": "clang-tidy",
-      "hidden": true,
-      "cacheVariables": {
-        "CMAKE_CXX_CLANG_TIDY": "clang-tidy;--header-filter=^${sourceDir}/"
-      }
-    },
     {
       "name": "ci-std",
       "description": "This preset makes sure the project actually builds with at least the specified standard",

cmake/clang-tidy-targets.cmake

@@ -0,0 +1,25 @@
+find_program(
+    default_clang_tidy_command
+    NAMES clang-tidy
+    HINTS
+        /opt/homebrew/opt/llvm/bin
+        /usr/local/opt/llvm/bin
+)
+
+set(
+    CLANG_TIDY_COMMAND
+    "${default_clang_tidy_command}"
+    CACHE FILEPATH
+    "Static analyzer to use"
+)
+
+add_custom_target(
+    tidy-check
+    COMMAND "${CMAKE_COMMAND}"
+    -D "CLANG_TIDY_COMMAND=${CLANG_TIDY_COMMAND}"
+    -D "COMPILE_COMMANDS_DIR=${PROJECT_BINARY_DIR}"
+    -P "${PROJECT_SOURCE_DIR}/cmake/clang-tidy.cmake"
+    WORKING_DIRECTORY "${PROJECT_SOURCE_DIR}"
+    COMMENT "Running clang-tidy static analysis"
+    VERBATIM
+)

cmake/clang-tidy.cmake

@@ -0,0 +1,65 @@
+cmake_minimum_required(VERSION 3.15)
+
+macro(default name)
+  if(NOT DEFINED "${name}")
+    set("${name}" "${ARGN}")
+  endif()
+endmacro()
+
+default(CLANG_TIDY_COMMAND clang-tidy)
+default(COMPILE_COMMANDS_DIR "${CMAKE_BINARY_DIR}")
+default(SOURCE_DIRS "src")
+
+if(NOT EXISTS "${COMPILE_COMMANDS_DIR}/compile_commands.json")
+  message(FATAL_ERROR
+    "clang-tidy requires a compilation database. Reconfigure with "
+    "-DCMAKE_EXPORT_COMPILE_COMMANDS=ON."
+  )
+endif()
+
+get_filename_component(PROJECT_SOURCE_DIR "${CMAKE_CURRENT_LIST_DIR}/.." ABSOLUTE)
+
+set(source_files "")
+foreach(dir IN LISTS SOURCE_DIRS)
+  file(GLOB_RECURSE dir_source_files "${PROJECT_SOURCE_DIR}/${dir}/*.cpp")
+  list(APPEND source_files ${dir_source_files})
+endforeach()
+
+if(NOT source_files)
+  message(FATAL_ERROR "No C++ source files found for clang-tidy analysis.")
+endif()
+
+set(clang_tidy_args
+    -p "${COMPILE_COMMANDS_DIR}"
+    --warnings-as-errors=*
+)
+
+if(APPLE)
+  execute_process(
+      COMMAND xcrun --show-sdk-path
+      OUTPUT_VARIABLE sdk_path
+      OUTPUT_STRIP_TRAILING_WHITESPACE
+      RESULT_VARIABLE sdk_result
+  )
+  if(NOT sdk_result EQUAL "0")
+    message(FATAL_ERROR "Unable to determine the macOS SDK path for clang-tidy.")
+  endif()
+  list(APPEND clang_tidy_args
+      "--extra-arg=-isysroot${sdk_path}"
+      "--extra-arg=-isystem${sdk_path}/usr/include/c++/v1"
+  )
+endif()
+
+execute_process(
+    COMMAND "${CLANG_TIDY_COMMAND}" ${clang_tidy_args} ${source_files}
+    WORKING_DIRECTORY "${PROJECT_SOURCE_DIR}"
+    RESULT_VARIABLE result
+    OUTPUT_VARIABLE output
+    ERROR_VARIABLE error_output
+)
+
+if(NOT result EQUAL "0")
+  message(FATAL_ERROR
+    "clang-tidy failed with result '${result}':\n${output}${error_output}"
+  )
+endif()

cmake/dev-mode.cmake

@@ -18,5 +18,6 @@ endif()
 include(cmake/lint-targets.cmake)
 include(cmake/spell-targets.cmake)
 include(cmake/cppcheck-targets.cmake)
+include(cmake/clang-tidy-targets.cmake)
 
 add_folders(Project)

docs/pages/devdocs/testing.dox

@@ -109,4 +109,26 @@
  * [GitHub Security](https://github.com/NeurodataWithoutBorders/aqnwb/security/code-scanning) 
  * tab, where cppcheck results can be browsed and tracked over time.
  *
+ * \section testing_clang_tidy Static Analysis (clang-tidy)
+ *
+ * AqNWB uses ``clang-tidy`` as an advisory, compiler-aware static analysis check.
+ * Its focused initial policy covers Clang static-analyzer, ``bugprone``, and
+ * ``performance`` checks for production code. Unlike cppcheck, clang-tidy uses
+ * the CMake compilation database, so it analyzes the compiler flags, include
+ * paths, and definitions used by the project.
+ *
+ * To run it locally, first configure a build with an exported compilation database,
+ * then invoke the ``tidy-check`` target:
+ *
+ * \code{.sh}
+ * cmake --preset=dev -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
+ * cmake --build --preset=dev --target=tidy-check
+ * \endcode
+ *
+ * The ``clang-tidy`` GitHub Actions workflow defined in
+ * [.github/workflows/clang-tidy.yml](https://github.com/NeurodataWithoutBorders/aqnwb/blob/main/.github/workflows/clang-tidy.yml)
+ * runs on changes to production source or analysis configuration. It invokes the
+ * same ``tidy-check`` target and analyzes all production translation units.
+ * Findings are advisory while initial CI coverage is established.
+ *
  */