Add clang-tidy to CMake and CI (Exawind#294)

Co-authored-by: Shreyas Ananthan <[email protected]>

.clang-tidy

@@ -0,0 +1,55 @@
+---
+#Checks:          '*,-cppcoreguidelines-avoid-magic-numbers,-readability-magic-numbers,-readability-braces-around-statements,-fuchsia-default-arguments,-fuchsia-default-arguments-calls,-fuchsia-overloaded-operator,-fuchsia-statically-constructed-objects,-google-runtime-references,-hicpp-no-assembler,-llvm-include-order,-llvmlibc-callee-namespace,-llvmlibc-implementation-in-namespace,-llvmlibc-restrict-system-libc-headers'
+Checks:          'clang-diagnostic-*,clang-analyzer-*,corecppguidelines-*,modernize-*,readability-*'
+WarningsAsErrors: ''
+#HeaderFilterRegex: '^.*/(AMReX)/.*\.H$'
+HeaderFilterRegex: '^((?!/amrex/Src/|/googletest/).)*$'
+AnalyzeTemporaryDtors: false
+FormatStyle:     none
+User:            user
+CheckOptions:
+  - key:             cert-dcl16-c.NewSuffixes
+    value:           'L;LL;LU;LLU'
+  - key:             cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField
+    value:           '0'
+  - key:             cert-str34-c.DiagnoseSignedUnsignedCharComparisons
+    value:           '0'
+  - key:             cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors
+    value:           '1'
+  - key:             cppcoreguidelines-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic
+    value:           '1'
+  - key:             google-readability-braces-around-statements.ShortStatementLines
+    value:           '1'
+  - key:             google-readability-function-size.StatementThreshold
+    value:           '800'
+  - key:             google-readability-namespace-comments.ShortNamespaceLines
+    value:           '10'
+  - key:             google-readability-namespace-comments.SpacesBeforeComments
+    value:           '2'
+  - key:             llvm-else-after-return.WarnOnConditionVariables
+    value:           '0'
+  - key:             llvm-else-after-return.WarnOnUnfixable
+    value:           '0'
+  - key:             llvm-qualified-auto.AddConstToQualified
+    value:           '0'
+  - key:             modernize-loop-convert.MaxCopySize
+    value:           '16'
+  - key:             modernize-loop-convert.MinConfidence
+    value:           reasonable
+  - key:             modernize-loop-convert.NamingStyle
+    value:           CamelCase
+  - key:             modernize-pass-by-value.IncludeStyle
+    value:           llvm
+  - key:             modernize-replace-auto-ptr.IncludeStyle
+    value:           llvm
+  - key:             modernize-use-nullptr.NullMacros
+    value:           'NULL'
+  - { key: readability-identifier-naming.NamespaceCase,       value: lower_case }
+  - { key: readability-identifier-naming.ClassCase,           value: CamelCase  }
+  - { key: readability-identifier-naming.PrivateMemberPrefix, value: m_         }
+  - { key: readability-identifier-naming.ProtectedMemberPrefix, value: m_         }
+  - { key: readability-identifier-naming.StructCase,          value: CamelCase  }
+  - { key: readability-identifier-naming.FunctionCase,        value: lower_case }
+  - { key: readability-identifier-naming.VariableCase,        value: lower_case }
+  - { key: readability-identifier-naming.GlobalConstantCase,  value: UPPER_CASE }
+...

.github/workflows/ci.yml

@@ -23,7 +23,7 @@ on:
       - '.github/workflows/ci.yml'
 
 jobs:
-  build:
+  Build:
     runs-on: ${{matrix.os}}
     strategy:
       matrix:

.github/workflows/lint.yml

@@ -23,7 +23,7 @@ on:
       - '.github/workflows/lint.yml'
 
 jobs:
-  Lint:
+  cppcheck:
     runs-on: macos-latest
     steps:
       - name: Cancel previous runs
@@ -39,15 +39,15 @@ jobs:
         run: |
           echo "NPROCS=$(sysctl -n hw.ncpu)" >> $GITHUB_ENV
           cmake \
-          -Bbuild-lint \
+          -Bbuild-cppcheck \
           -DAMR_WIND_ENABLE_MPI:BOOL=OFF \
           -DAMR_WIND_ENABLE_TESTS:BOOL=ON \
           -DAMR_WIND_TEST_WITH_FCOMPARE:BOOL=OFF \
           -DAMR_WIND_ENABLE_MASA:BOOL=OFF \
           -DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=ON \
           ${{github.workspace}}
       - name: Check
-        working-directory: build-lint
+        working-directory: build-cppcheck
         run: |
           # Using a working directory for cppcheck makes analysis faster
           mkdir cppcheck-wd
@@ -61,15 +61,56 @@ jobs:
           # Warnings in header files are unavoidable, so we filter out submodule headers after analysis
           # Might be wrong to assume cppcheck always reports issues using 3 lines
            awk -v nlines=2 '/submods\/amrex/ || /submods\/googletest/ {for (i=0; i<nlines; i++) {getline}; next} 1' \
-            < cppcheck.txt > amr-wind-static-analysis.txt
-      - name: Report
-        working-directory: build-lint
+            < cppcheck.txt > cppcheck-warnings.txt
+      - name: Short report
+        working-directory: build-cppcheck
+        run: |
+          cat cppcheck-warnings.txt | egrep 'error:|performance:|portability:|style:|warning:' | sort | \
+            awk 'BEGIN{i=0}{print $0}{i++}END{print "Warnings: "i}'
+      - name: Full report
+        working-directory: build-cppcheck
         run: |
           echo "::add-matcher::.github/problem-matchers/gcc.json"
-          # Might be wrong to assume cppcheck always reports issues using 3 lines
-          WARNINGS_TMP=$(wc -l < amr-wind-static-analysis.txt | xargs echo -n)
-          WARNINGS=$(bc <<< "$WARNINGS_TMP/3")
-          printf "%s warnings\n" "${WARNINGS}" >> amr-wind-static-analysis.txt
-          cat amr-wind-static-analysis.txt
-          # Just report for now and don't fail
-          #exit ${WARNINGS}
+          cat cppcheck-warnings.txt
+  clang-tidy:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Cancel previous runs
+        uses: styfle/[email protected]
+        with:
+          access_token: ${{github.token}}
+      - uses: actions/checkout@v2
+        with:
+          submodules: true
+      - name: Dependencies
+        run: |
+          sudo apt-get install -y clang-tidy-9
+          sudo update-alternatives --install /usr/bin/clang-tidy clang-tidy /usr/bin/clang-tidy-9 100
+      - name: Configure
+        run: |
+          echo "NPROCS=$(nproc)" >> $GITHUB_ENV
+          cmake \
+          -Bbuild-clang-tidy \
+          -DCMAKE_CXX_COMPILER:STRING=clang++ \
+          -DCMAKE_C_COMPILER:STRING=clang \
+          -DAMR_WIND_ENABLE_MPI:BOOL=OFF \
+          -DAMR_WIND_ENABLE_TESTS:BOOL=ON \
+          -DAMR_WIND_TEST_WITH_FCOMPARE:BOOL=OFF \
+          -DAMR_WIND_ENABLE_MASA:BOOL=OFF \
+          -DAMR_WIND_ENABLE_ALL_WARNINGS:BOOL=OFF \
+          -DAMR_WIND_ENABLE_CLANG_TIDY:BOOL=ON \
+          -DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=ON \
+          ${{github.workspace}}
+      - name: Check
+        working-directory: build-clang-tidy
+        run: cmake --build . -- -j ${{env.NPROCS}} 2> clang-tidy-warnings.txt
+      - name: Short report
+        working-directory: build-clang-tidy
+        run: |
+          cat clang-tidy-warnings.txt | grep "warning:" | sort | \
+            awk 'BEGIN{i=0}{print $0}{i++}END{print "Warnings: "i}'
+      - name: Full report
+        working-directory: build-clang-tidy
+        run: |
+          #echo "::add-matcher::.github/problem-matchers/gcc.json"
+          cat clang-tidy-warnings.txt

CMakeLists.txt

@@ -5,14 +5,14 @@ project(AMR-Wind CXX C)
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake")
 include(CMakePackageConfigHelpers)
 include(GNUInstallDirs)
-include(${CMAKE_SOURCE_DIR}/cmake/amr-wind-utils.cmake)
+include(amr-wind-utils)
 
 ########################## OPTIONS #####################################
 
 #General options for the project
-option(AMR_WIND_ENABLE_FORTRAN "Enable Fortran interfaces" OFF)
 option(AMR_WIND_ENABLE_ALL_WARNINGS "Show most warnings for most compilers" ON)
 option(AMR_WIND_ENABLE_WERROR "Treat compiler warnings as errors" OFF)
+option(AMR_WIND_ENABLE_CLANG_TIDY "Compile with clang-tidy static analysis" OFF)
 option(AMR_WIND_ENABLE_FCOMPARE "Enable building fcompare when not testing" OFF)
 
 #Enabling tests overrides the executable options
@@ -90,15 +90,14 @@ add_library(${amr_wind_lib_name} OBJECT)
 add_library(${aw_api_lib} SHARED)
 add_executable(${amr_wind_exe_name})
 
-include(${CMAKE_SOURCE_DIR}/cmake/set_compile_flags.cmake)
-
-if (AMR_WIND_ENABLE_FORTRAN)
-  #Keep our Fortran module files confined to a directory
-  set_target_properties(
-    ${amr_wind_lib_name} PROPERTIES
-    Fortran_MODULE_DIRECTORY "${CMAKE_BINARY_DIR}/fortran_modules/")
+init_clang_tidy()
+if(CLANG_TIDY_EXE)
+  set_target_properties(${amr_wind_lib_name} ${aw_api_lib} ${amr_wind_exe_name}
+                        PROPERTIES CXX_CLANG_TIDY ${CLANG_TIDY_EXE})
 endif()
 
+include(${CMAKE_SOURCE_DIR}/cmake/set_compile_flags.cmake)
+
 if (AMR_WIND_ENABLE_NETCDF)
   set(CMAKE_PREFIX_PATH ${NETCDF_DIR} ${CMAKE_PREFIX_PATH})
   find_package(NetCDF REQUIRED)
@@ -128,6 +127,10 @@ endif()
 
 if (AMR_WIND_ENABLE_UNIT_TESTS OR AMR_WIND_ENABLE_TESTS)
   add_executable(${amr_wind_unit_test_exe_name})
+  if(CLANG_TIDY_EXE)
+    set_target_properties(${amr_wind_unit_test_exe_name}
+                          PROPERTIES CXX_CLANG_TIDY ${CLANG_TIDY_EXE})
+  endif()
   add_subdirectory("submods/googletest")
   add_subdirectory("unit_tests")
   if (AMR_WIND_ENABLE_CUDA)

amr-wind/CMakeLists.txt

@@ -4,7 +4,6 @@ target_sources(${amr_wind_lib_name}
   incflo.cpp
   incflo_compute_dt.cpp
   incflo_regrid.cpp
-
   CFDSim.cpp
   )
 
@@ -36,7 +35,6 @@ if(AMR_WIND_ENABLE_MASA)
   add_subdirectory(mms)
 endif()
 
-# Generate AMReX build information file
 include(AMReXBuildInfo)
 generate_buildinfo(${amr_wind_lib_name} ${CMAKE_SOURCE_DIR})
 

cmake/amr-wind-utils.cmake

@@ -60,3 +60,14 @@ macro(init_amrex)
       CACHE INTERNAL "Path to fcompare executable for regression tests")
   endif()
 endmacro(init_amrex)
+
+macro(init_clang_tidy)
+  if(AMR_WIND_ENABLE_CLANG_TIDY)
+    find_program(CLANG_TIDY_EXE NAMES "clang-tidy")
+    if(CLANG_TIDY_EXE)
+      message(STATUS "clang-tidy found: ${CLANG_TIDY_EXE}")
+    else()
+      message(WARNING "clang-tidy not found.")
+    endif()
+  endif()
+endmacro(init_clang_tidy)

cmake/set_amrex_options.cmake

@@ -10,8 +10,8 @@ set(AMReX_COMM_PROFILE OFF)
 set(AMReX_CONDUIT OFF)
 set(AMReX_PRECISION DOUBLE)
 set(AMReX_EB OFF)
-set(AMReX_FORTRAN ${AMR_WIND_ENABLE_FORTRAN})
-set(AMReX_FORTRAN_INTERFACES ${AMR_WIND_ENABLE_FORTRAN})
+set(AMReX_FORTRAN OFF)
+set(AMReX_FORTRAN_INTERFACES OFF)
 set(AMReX_FPE OFF)
 set(AMReX_HYPRE ${AMR_WIND_ENABLE_HYPRE})
 set(AMReX_LINEAR_SOLVERS ON)

cmake/set_compile_flags.cmake

@@ -26,12 +26,6 @@ separate_arguments(AMR_WIND_CXX_FLAGS)
 target_compile_options(
   ${amr_wind_lib_name} PRIVATE
   $<$<COMPILE_LANGUAGE:CXX>:${AMR_WIND_CXX_FLAGS}>)
-if (AMR_WIND_ENABLE_FORTRAN)
-  separate_arguments(AMR_WIND_Fortran_FLAGS)
-  target_compile_options(
-    ${amr_wind_lib_name} PRIVATE
-    $<$<COMPILE_LANGUAGE:Fortran>:${AMR_WIND_Fortran_FLAGS}>)
-endif()
 
 # Building on CUDA requires additional considerations
 if (AMR_WIND_ENABLE_CUDA)