Merge pull request #419 from DrylandEcology/feature_clangtidy

Update SOILWAT2 code base to pass clang-tidy checks - excluded checks, see ".clang-tidy" and ".clang-tidy_swtest" - requires clang-tidy v17 or later - make targets: "tidy-bin" and "tidy-test" - script: "tools/run_tidy.sh"
DrylandEcology · Aug 6, 2024 · 2c840d7 · 2c840d7
2 parents 2319861 + b9cefe3
commit 2c840d7
Show file tree

Hide file tree

Showing 74 changed files with 6,186 additions and 3,697 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -0,0 +1,34 @@
+---
+# Checks + accepted suppressions for SOILWAT2
+Checks: >
+    clang-diagnostic-*,
+    clang-analyzer-*,
+    -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,
+    bugprone-*,
+    -bugprone-easily-swappable-parameters,
+    cert-*,
+    concurrency-*,
+    -concurrency-mt-unsafe,
+    cppcoreguidelines-*,
+    -cppcoreguidelines-avoid-magic-numbers,
+    -cppcoreguidelines-init-variables,
+    -cppcoreguidelines-macro-to-enum,
+    misc-*,
+    -misc-include-cleaner,
+    mpi-*,
+    performance-*,
+    portability-*,
+    readability-*,
+    -readability-function-cognitive-complexity,
+    -readability-identifier-length,
+    -readability-magic-numbers
+WarningsAsErrors: '*'
+HeaderFilterRegex: ''
+FormatStyle:     none
+SystemHeaders:   false
+CheckOptions:
+    - key: readability-braces-around-statements.ShortStatementLines
+      value: '2'
+    - key: readability-uppercase-literal-suffix.NewSuffixes
+      value: 'L;LL;LU;LLU'
+...
diff --git a/.clang-tidy_swtest b/.clang-tidy_swtest
@@ -0,0 +1,43 @@
+---
+# Checks + accepted suppressions for SOILWAT2 tests
+Checks: >
+    clang-diagnostic-*,
+    clang-analyzer-*,
+    -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,
+    bugprone-*,
+    -bugprone-easily-swappable-parameters,
+    cert-*,
+    concurrency-*,
+    -concurrency-mt-unsafe,
+    cppcoreguidelines-*,
+    -cppcoreguidelines-avoid-c-arrays,
+    -cppcoreguidelines-avoid-goto,
+    -cppcoreguidelines-avoid-magic-numbers,
+    -cppcoreguidelines-init-variables,
+    -cppcoreguidelines-macro-to-enum,
+    -cppcoreguidelines-no-malloc,
+    -cppcoreguidelines-owning-memory,
+    -cppcoreguidelines-pro-bounds-array-to-pointer-decay,
+    -cppcoreguidelines-pro-bounds-constant-array-index,
+    -cppcoreguidelines-pro-bounds-pointer-arithmetic,
+    -cppcoreguidelines-pro-type-cstyle-cast,
+    -cppcoreguidelines-pro-type-vararg,
+    misc-*,
+    -misc-include-cleaner,
+    mpi-*,
+    performance-*,
+    portability-*,
+    readability-*,
+    -readability-function-cognitive-complexity,
+    -readability-identifier-length,
+    -readability-magic-numbers
+WarningsAsErrors: '*'
+HeaderFilterRegex: ''
+FormatStyle:     none
+SystemHeaders:   false
+CheckOptions:
+    - key: readability-braces-around-statements.ShortStatementLines
+      value: '2'
+    - key: readability-uppercase-literal-suffix.NewSuffixes
+      value: 'L;LL;LU;LLU'
+...
diff --git a/.github/workflows/clang-tidy-check.yml b/.github/workflows/clang-tidy-check.yml
@@ -0,0 +1,29 @@
+name: clang-tidy check
+
+# ubuntu-24.04 comes with clang v18 as default
+# ubuntu-latest as of 2024-Aug-01 points to ubuntu-22.04 with clang v14
+
+on:
+  push:
+    branches: [master, main, release/**]
+  pull_request:
+    branches: [master, main, release/**]
+
+jobs:
+  formatting-check:
+    name: Tidy Check
+    runs-on: ubuntu-24.04
+
+    steps:
+    - name: Checkout repository and submodules
+      uses: actions/checkout@v4
+      with:
+        submodules: recursive
+
+    - name: Install netCDF-C and udunits2
+      run: |
+        sudo apt-get update
+        sudo apt-get install libnetcdf-dev libudunits2-dev
+
+    - name: Run
+      run: ./tools/run_tidy.sh
diff --git a/doc/additional_pages/code_contribution_guidelines.md b/doc/additional_pages/code_contribution_guidelines.md
@@ -12,6 +12,7 @@
 [netCDF]: https://downloads.unidata.ucar.edu/netcdf/
 [udunits2]: https://downloads.unidata.ucar.edu/udunits/
 [ClangFormat]: https://clang.llvm.org/docs/ClangFormat.html
+[clang-tidy]: https://clang.llvm.org/extra/clang-tidy
 [iwyu]:https://include-what-you-use.org/
 
 
@@ -21,8 +22,9 @@ Go back to the [main page](README.md).
 1. [SOILWAT2 code](#SOILWAT2_code)
 2. [Code guidelines](#guidelines)
     1. [Code format](#code_format)
-    2. [Include directives](#includes)
-    3. [Other guidelines](#other_guidelines)
+    2. [Code checks](#code_checks)
+    3. [Include directives](#includes)
+    4. [Other guidelines](#other_guidelines)
 3. [Code documentation](#code_documentation)
 4. [Code tests](#code_tests)
     1. [Unit tests](#unit_tests)
@@ -135,6 +137,27 @@ checks code style for pull requests into the main branch and release branches.
 <br>
 
 
+
+<a name="code_checks"></a>
+### Code checks
+
+We use [clang-tidy][] to check code in the `SOILWAT2` repository.
+
+The files `".clang-tidy"` and `".clang-tidy_swtests"` document all details
+related to these code checks.
+
+We have `make` targets `"tidy-bin"` and `"tidy-test"` to run these checks on
+the code and on the test code respectively.
+
+We also have a script `"tools/run_tidy.sh"` that runs all appropriate checks.
+The script exits with a failure code if any check reports code issues.
+
+A github action workflow `".github/workflows/clang-tidy-check.yml"`
+checks code for pull requests into the main branch and release branches.
+
+<br>
+
+
 <a name="includes"></a>
 ### Include directives
 

diff --git a/include/SW_Control.h b/include/SW_Control.h
@@ -33,7 +33,7 @@ void SW_CTL_init_ptrs(SW_RUN *sw);
 void SW_CTL_alloc_outptrs(SW_RUN *sw, LOG_INFO *LogInfo);
 
 void SW_RUN_deepCopy(
-    SW_RUN *source, SW_RUN *dest, SW_OUT_DOM *DomRun, LOG_INFO *LogInfo
+    SW_RUN *source, SW_RUN *dest, SW_OUT_DOM *OutDom, LOG_INFO *LogInfo
 );
 
 void SW_CTL_setup_domain(

diff --git a/include/SW_Defines.h b/include/SW_Defines.h
@@ -22,8 +22,7 @@
 #ifndef SOILW_DEF_H
 #define SOILW_DEF_H
 
-#include "include/generic.h" // for IntUS, RealF
-#include <time.h>            // for time_t, timespec
+#include <time.h> // for time_t, timespec
 
 #if !defined(RSOILWAT)              /* rSOILWAT2 uses R's RNGs */
 #include "external/pcg/pcg_basic.h" // for pcg32_random_t
@@ -119,7 +118,7 @@ extern "C" {
 #define OUT_DIGITS 6
 
 /** Separator used when generating text-based output files (csv-format) */
-#define _OUTSEP ','
+#define OUTSEP ','
 
 // was 256 & 1024...
 #define MAX_FILENAMESIZE 512
@@ -171,9 +170,8 @@ extern "C" {
 #define MAX_WEEKS 53
 #define MAX_DAYS 366
 
-#define SWRC_PARAM_NMAX                                  \
-    6 /**< Maximal number of SWRC parameters implemented \
-       */
+/** Maximal number of SWRC parameters implemented */
+#define SWRC_PARAM_NMAX 6
 
 /*
    Indices to daily input flags/indices (dailyInputFlags & dailyInputIndices in
@@ -217,8 +215,8 @@ extern "C" {
 #define eSW_Year 3
 #define eSW_NoTime 999 // no time period
 // c++ doesn't support (pd)++ for pd as a typedef enum OutPeriod in
-// macro `ForEachOutPeriod` --> instead, define as type `IntUS`
-typedef IntUS OutPeriod;
+// macro `ForEachOutPeriod` --> instead, define as type unsigned int
+typedef unsigned short OutPeriod;
 
 /*
   * Number of output keys
@@ -281,7 +279,7 @@ typedef IntUS OutPeriod;
  * before I got the documentation.
  */
 typedef struct {
-    RealF xinflec, yinflec, range, slope;
+    double xinflec, yinflec, range, slope;
 } tanfunc_t;
 
 /* standardize the test for missing */
@@ -317,6 +315,14 @@ typedef struct timespec WallTimeSpec;
 typedef time_t WallTimeSpec;
 #endif
 
+/* Memory copying via `sw_memccpy()` and SOILWAT2's custom function */
+#if (defined(_POSIX_C_SOURCE) && _POSIX_C_SOURCE >= 200809L) || \
+    (defined(__STDC__) && defined(__STDC_VERSION__) &&          \
+     __STDC_VERSION__ >= 202311L)
+#define sw_memccpy memccpy
+#else
+#define sw_memccpy sw_memccpy_custom
+#endif
 
 /* =================================================== */
 /*                   RNG structs                    */

diff --git a/include/SW_Files.h b/include/SW_Files.h
@@ -78,7 +78,9 @@ void SW_F_deepCopy(PATH_INFO *source, PATH_INFO *dest, LOG_INFO *LogInfo);
 
 void SW_F_init_ptrs(char *InFiles[]);
 
-void SW_F_construct(const char *firstfile, char _ProjDir[], LOG_INFO *LogInfo);
+void SW_F_construct(
+    const char *firstfile, char SW_ProjDir[], LOG_INFO *LogInfo
+);
 
 void SW_F_deconstruct(char *InFiles[]);
 

diff --git a/include/SW_Flow_lib.h b/include/SW_Flow_lib.h
@@ -92,10 +92,10 @@ void infiltrate_water_high(
     double drain[],
     double *drainout,
     double pptleft,
-    int nlyrs,
-    double swcfc[],
+    unsigned int nlyrs,
+    const double swcfc[],
     double swcsat[],
-    double impermeability[],
+    const double impermeability[],
     double *standingWater,
     double lyrFrozen[]
 );
@@ -105,7 +105,7 @@ void transp_weighted_avg(
     SW_SITE *SW_Site,
     unsigned int n_tr_rgns,
     LyrIndex n_layers,
-    unsigned int tr_regions[],
+    const unsigned int tr_regions[],
     double swc[],
     int VegType,
     LOG_INFO *LogInfo
@@ -190,7 +190,7 @@ void remove_from_soil(
     SW_SITE *SW_Site,
     double *aet,
     unsigned int nlyrs,
-    double coeff[],
+    const double coeff[],
     double rate,
     double swcmin[],
     double lyrFrozen[],
@@ -215,7 +215,7 @@ void hydraulic_redistribution(
     SW_SITE *SW_Site,
     unsigned int vegk,
     unsigned int nlyrs,
-    double lyrFrozen[],
+    const double lyrFrozen[],
     double maxCondroot,
     double swp50,
     double shapeCond,
@@ -281,8 +281,8 @@ void lyrTemp_to_lyrSoil_temperature(
 
 void lyrSoil_to_lyrTemp_temperature(
     unsigned int nlyrSoil,
-    double depth_Soil[],
-    double avgLyrTemp[],
+    const double depth_Soil[],
+    const double avgLyrTemp[],
     double endTemp,
     unsigned int nlyrTemp,
     double depth_Temp[],
@@ -293,8 +293,8 @@ void lyrSoil_to_lyrTemp_temperature(
 void lyrSoil_to_lyrTemp(
     double cor[MAX_ST_RGR][MAX_LAYERS + 1],
     unsigned int nlyrSoil,
-    double width_Soil[],
-    double var[],
+    const double width_Soil[],
+    const double var[],
     unsigned int nlyrTemp,
     double width_Temp,
     double res[]
@@ -324,8 +324,8 @@ void soil_temperature_setup(
     double avgLyrTempInit[],
     double sTconst,
     unsigned int nlyrs,
-    double fc[],
-    double wp[],
+    const double fc[],
+    const double wp[],
     double deltaX,
     double theMaxDepth,
     unsigned int nRgr,
@@ -340,18 +340,18 @@ void set_frozen_unfrozen(
     double avgLyrTemp[],
     double swc[],
     double swc_sat[],
-    double width[],
+    const double width[],
     double lyrFrozen[]
 );
 
 unsigned int adjust_Tsoil_by_freezing_and_thawing(
-    double oldavgLyrTemp[],
-    double avgLyrTemp[],
+    const double oldavgLyrTemp[],
+    const double avgLyrTemp[],
     double shParam,
     unsigned int nlyrs,
-    double vwc[],
-    double bDensity[],
-    Bool fusion_pool_init,
+    const double vwc[],
+    const double bDensity[],
+    Bool *fusion_pool_init,
     double oldsFusionPool_actual[]
 );
 
@@ -360,20 +360,20 @@ void soil_temperature_today(
     double deltaX,
     double sT1,
     double sTconst,
-    int nRgr,
+    unsigned int nRgr,
     double avgLyrTempR[],
-    double oldavgLyrTempR[],
-    double vwcR[],
-    double wpR[],
-    double fcR[],
-    double bDensityR[],
+    const double oldavgLyrTempR[],
+    const double vwcR[],
+    const double wpR[],
+    const double fcR[],
+    const double bDensityR[],
     double csParam1,
     double csParam2,
     double shParam,
     Bool *ptr_stError,
     double surface_range,
     double temperatureRangeR[],
-    double depthsR[],
+    const double depthsR[],
     TimeInt year,
     TimeInt doy
 );

diff --git a/include/SW_Main_lib.h b/include/SW_Main_lib.h
@@ -22,7 +22,7 @@ void sw_init_args(
     int argc,
     char **argv,
     Bool *EchoInits,
-    char **_firstfile,
+    char **firstfile,
     unsigned long *userSUID,
     double *wallTimeLimit,
     Bool *renameDomainTemplateNC,