Actions: Improved clang-tidy action (removed warnings and unnecessary…

… parameter) * Fix shellcheck warnings * Remove unnecessary parameter * Major refactoring with regexes Co-authored-by: Orhan Kupusoglu <[email protected]>
FlorianReimold · Jul 11, 2022 · cc6c2f2 · cc6c2f2
1 parent bc13f25
commit cc6c2f2
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 69 deletions.
diff --git a/build_linux/clang-tidy/build.sh b/build_linux/clang-tidy/build.sh
@@ -74,7 +74,7 @@ DCMAKE_CXX_COMPILER=              #'-DCMAKE_CXX_COMPILER=/usr/bin/clang++-14'
 
 # ------------------------------------------------------------------------------
 
-USAGE="$(basename $0) [-h|-help] [-b|--build <build>] [-c|compiler <C> <CXX>] [-m|--make] [-i|--filter <app> <cfg>] [-d|--database] [-a|--already] [-f|--files <files...>]
+USAGE="$(basename "$0") [-h|-help] [-b|--build <build>] [-c|compiler <C> <CXX>] [-m|--make] [-i|--filter <app> <cfg>] [-d|--database] [-a|--already] [-f|--files <files...>]
 run cmake, and then optionally make and/or clang-tidy - where:
     -h | --help                 show this help message and exit
     -b | --build <build>        build path relative to this script, default: '${PATH_BUILD}'
@@ -110,7 +110,7 @@ then
                                 if [[  $# -eq 0 ]];then break ; fi ;;
             -f | --files )      RUN_FILES='ON' ; shift ;
                                 if [[  $# -eq 0 ]];then echo "WARNING - missing file list" ; exit 0 ; fi ;
-                                FILE_LIST=($@) ; break ;;
+                                FILE_LIST=("$@") ; break ;;
             * )                 echo "ERROR - unknown option: '$1'" ; shift ; exit 1 ;;
         esac
     done
@@ -142,21 +142,24 @@ check_args() {
 
     # detect relative path by removing the final '/' and all trailing chars '*'
     local dir_rel=${PATH_BUILD%/*}
-    DIR_BUILD_NAME=${PATH_BUILD#$dir_rel/}
+    local root_dir_git
+    local path_clang_tidy
+
+    DIR_BUILD_NAME=${PATH_BUILD#"${dir_rel}"/}
     cd "${dir_rel}"
     DIR_BUILD_ROOT=$(pwd)
     DIR_BUILD="${DIR_BUILD_ROOT}/${DIR_BUILD_NAME}"
 
     # find the root directory where '.git/' resides
     cd "${DIR_SCRIPT}"
-    local root_dir_git=$(find_root_dir "${DIR_SCRIPT}")
+    root_dir_git=$(find_root_dir)
     cd "${root_dir_git}"
     DIR_ROOT=$(pwd)
 
     if [[ "${RUN_DATABASE}" == 'ON' || "${RUN_FILES}" == 'ON' ]]
     then
         set +e
-        local path_clang_tidy=$(which ${CLANG_TIDY})
+        path_clang_tidy=$(command -v ${CLANG_TIDY})
         set -e
         if [[ -z ${path_clang_tidy} ]]
         then
@@ -186,14 +189,14 @@ run_cmake() {
         echo "++ build type: ${CMAKE_BUILD_TYPE}"
         echo -e "\n++ running cmake ..."
 
-        rm -rf "${DIR_BUILD}/"
-        mkdir -p ${DIR_BUILD}
+        rm -rf "${DIR_BUILD:?}/"
+        mkdir -p "${DIR_BUILD}"
         cd "${DIR_BUILD}"
 
-        cmake "${DIR_ROOT}" ${DCMAKE_EXPORT_COMPILE_COMMANDS} \
-                            ${DCMAKE_C_COMPILER} \
-                            ${DCMAKE_CXX_COMPILER} \
-                            -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} \
+        cmake "${DIR_ROOT}" "${DCMAKE_EXPORT_COMPILE_COMMANDS}" \
+                            "${DCMAKE_C_COMPILER}" \
+                            "${DCMAKE_CXX_COMPILER}" \
+                            "-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}" \
                             -DECAL_THIRDPARTY_BUILD_PROTOBUF=OFF \
                             -DECAL_THIRDPARTY_BUILD_CURL=OFF  \
                             -DECAL_THIRDPARTY_BUILD_HDF5=OFF
@@ -207,8 +210,8 @@ run_make() {
         cd "${DIR_BUILD}"
         local cmd="cmake --build . -- -j${NUM_INST}"
         echo -e "\n++ ${cmd}\nsee: ${FILE_MAKE_OUTPUT}"
-        echo "${cmd}" >> ${FILE_MAKE_OUTPUT}
-        time ${cmd} |& tee -a ${FILE_MAKE_OUTPUT}
+        echo "${cmd}" >> "${FILE_MAKE_OUTPUT}"
+        time ${cmd} |& tee -a "${FILE_MAKE_OUTPUT}"
     fi
 }
 
@@ -242,7 +245,7 @@ filter_compile_commands() {
         cd "${DIR_SCRIPT}"
         echo "excluded directories:"
         cat "$PATH_EXC_CONFIG"
-        python3 ${PATH_FILTER} --build "${DIR_BUILD}"
+        python3 "${PATH_FILTER}" --build "${DIR_BUILD}"
 
         cp -a "${DIR_ROOT}/.clang-tidy" "${DIR_BUILD}"
         cd "${DIR_BUILD}"
@@ -267,8 +270,8 @@ run_clang_tidy_on_database() {
         # see: run-clang-tidy --help
         local cmd="run-${CLANG_TIDY} -j${NUM_INST}"
         echo -e "\n++ ${cmd}\ncfg: ${FILE_CLANG_TIDY_CONFIG}\nsee: ${FILE_CLANG_TIDY_OUTPUT}"
-        echo "${cmd}" >> ${FILE_CLANG_TIDY_OUTPUT}
-        time ${cmd} |& tee -a ${FILE_CLANG_TIDY_OUTPUT}
+        echo "${cmd}" >> "${FILE_CLANG_TIDY_OUTPUT}"
+        time ${cmd} |& tee -a "${FILE_CLANG_TIDY_OUTPUT}"
     fi
 }
 
@@ -281,10 +284,16 @@ run_clang_tidy_on_database() {
 #         ["c"]
 #     ]
 read_config_basic() {
+    local line_count
+    local line_trim
+    local line_no_quotes
+    local line_no_brackets
+    local subdir
+
     cd "${DIR_SCRIPT}"
     if [[ -f "$PATH_EXC_CONFIG" ]]
     then
-        local line_count=$(wc -l < "$PATH_EXC_CONFIG")
+        line_count=$(wc -l < "$PATH_EXC_CONFIG")
         if [[ "${line_count}" -gt "1" ]]
         then
             declare -a exc_dirs
@@ -293,16 +302,16 @@ read_config_basic() {
                 exc_dirs[${#exc_dirs[@]}]="$line"
             done < "$PATH_EXC_CONFIG"
             # eliminate the first and the last element, '[' and ']' respectively
-            unset exc_dirs[0]
-            unset exc_dirs[-1]
+            unset 'exc_dirs[0]'
+            unset 'exc_dirs[-1]'
             for exc_dir in "${exc_dirs[@]}"
             do
-                local line_trim=$(echo "${exc_dir}" | tr -d '[:space:]')
-                local line_no_quotes=$(echo "${line_trim}" | tr -d '"')
-                local line_no_brackets=$(echo "${line_no_quotes}" | tr -d '[\[\]]')
+                line_trim=$(echo "${exc_dir}" | tr -d '[:space:]')
+                line_no_quotes=$(echo "${line_trim}" | tr -d '"')
+                line_no_brackets=$(echo "${line_no_quotes}" | tr -d '[]')
                 declare -a dirs
-                IFS=',' read -a dirs <<< $line_no_brackets
-                local subdir=$(printf "/%s" "${dirs[@]}")
+                IFS=',' read -ar dirs <<< "${line_no_brackets}"
+                subdir=$(printf "/%s" "${dirs[@]}")
                 EXC_LIST[${#EXC_LIST[@]}]="${subdir}/"
             done
         else
@@ -316,9 +325,12 @@ read_config_basic() {
 
 # jq can parse valid JSONs, pretty-print is not required.
 read_config() {
+    local path_jq
+    local subdir
+
     cd "${DIR_SCRIPT}"
     set +e
-    local path_jq=$(which jq)
+    path_jq=$(command -v jq)
     set -e
     if [[ -z ${path_jq} ]]
     then
@@ -327,9 +339,9 @@ read_config() {
     else
         if [[ -f "$PATH_EXC_CONFIG" ]]
         then
-            for row in $(cat ${PATH_EXC_CONFIG} | jq -c '.[]')
+            for row in $(jq -c '.[]' < "${PATH_EXC_CONFIG}")
             do
-                local subdir=''
+                subdir=''
                 for dir in $(echo "${row}" | jq -r '.[]')
                 do
                     subdir="${subdir}/${dir}"
@@ -360,6 +372,12 @@ is_excluded() {
 run_clang_tidy_on_files() {
     if [[ "${RUN_FILES}" == 'ON' ]]
     then
+        local file_abs
+        local file_char
+        local file_base
+        local file_name
+        local file_ext
+
         read_config
 
         cd "${DIR_BUILD}"
@@ -370,17 +388,17 @@ run_clang_tidy_on_files() {
 
         for file in "${FILE_LIST[@]}"
         do
-            local file_abs="${file}"
-            local file_char="${file_abs:0:1}"
+            file_abs="${file}"
+            file_char="${file_abs:0:1}"
 
             if [[ ${file_char} != '/' ]]
             then
                 file_abs="${DIR_ROOT}/${file}"
             fi
 
-            local file_base=$(basename "${file_abs}")
-            local file_name="${file_base%.*}"
-            local file_ext="${file_base##*.}"
+            file_base=$(basename "${file_abs}")
+            file_name="${file_base%.*}"
+            file_ext="${file_base##*.}"
 
             echo "${SEP_1}"
             echo -e "-- ${file}\n   ${file_name} . ${file_ext}"
@@ -402,9 +420,9 @@ run_clang_tidy_on_files() {
                         local cmd="${CLANG_TIDY} -p . ${file_abs}"
                         echo "## analyzing"
                         echo "${SEP_2}"
-                        echo "${cmd}" |& tee -a ${FILE_CLANG_TIDY_OUTPUT}
-                        ${cmd} |& tee -a ${FILE_CLANG_TIDY_OUTPUT}
-                        echo "${SEP_2}" >> ${FILE_CLANG_TIDY_OUTPUT}
+                        echo "${cmd}" |& tee -a "${FILE_CLANG_TIDY_OUTPUT}"
+                        ${cmd} |& tee -a "${FILE_CLANG_TIDY_OUTPUT}"
+                        echo "${SEP_2}" >> "${FILE_CLANG_TIDY_OUTPUT}"
                     fi
                 else
                     echo "## skipping"

diff --git a/build_linux/clang-tidy/filter_clang_tidy.py b/build_linux/clang-tidy/filter_clang_tidy.py
@@ -5,16 +5,17 @@
 This script processes the 'compile_commands.json' file which CMake can generate since v2.8.5.
 `JSON Compilation Database Format Specification <https://clang.llvm.org/docs/JSONCompilationDatabase.html>`__
 
-The CPP files which are located in the directories given by 'excludes_clang_tidy.json' are filtered out.
+The CPP files whose paths contain the directories given by 'excludes_clang_tidy.json' are filtered out.
 The filtering results in two JSON files, one for included and one for excluded commands.
-H files located in this directories are included as '-isystem' instead of '-I',
+H files located in this directories may optionally be included as '-isystem' instead of '-I',
 so that they are excluded from static analysis as well.
 """
 
 import argparse
 import json
 import os
 import sys
+import re
 
 # ------------------------------------------------------------------------------
 
@@ -26,7 +27,6 @@
 
 path_config = ''
 path_build = ''
-path_root = ''
 isystem = DEF_ISYSTEM
 commands = []
 excludes = []
@@ -44,17 +44,16 @@ def read_args():
     """
     global path_build
     global path_config
-    global path_root
     global isystem
 
     # display default values
     parser = argparse.ArgumentParser(description='Filter compilation database.',
                                      formatter_class=argparse.ArgumentDefaultsHelpFormatter)
     parser.add_argument('--build',
-                        help='rel/abs path of the build directory',
+                        help='rel to this script/abs path of the build directory',
                         default=DEF_BUILD)
     parser.add_argument('--config',
-                        help='rel/abs path of the config JSON file',
+                        help='rel to this script/abs path of the config JSON file',
                         default=DEF_CONFIG)
     parser.add_argument('--isystem',
                         action='store_true',
@@ -71,7 +70,7 @@ def read_args():
         print(f'arg :: config  = "{config}"')
         print(f'arg :: isystem = {isystem}')
 
-    # change first working directory to script's directory
+    # first change working directory to script's directory
     os.chdir(os.path.dirname(os.path.abspath(sys.argv[0])))
     path_script = os.getcwd()
 
@@ -80,9 +79,6 @@ def read_args():
     else:
         path_build = os.path.realpath(os.path.join(path_script, build))
 
-    # assumption: CMake build directory is located in the root directory
-    path_root = os.path.realpath(os.path.join(path_build, '../.'))
-
     if os.path.isabs(config):
         path_config = config
     else:
@@ -106,14 +102,9 @@ def read_config():
 
     with open(path_config, 'r') as excludes_file:
         excludes_raw = json.load(excludes_file)
-
-    # The excludes_raw list contains each excluded directory name as a list,
-    # so that a proper platform-dependent string can be obtained easily.
-    # The 'directory' string starts artificially with path separator to prevent partial matches.
     for exclude_raw in excludes_raw:
-        exclude = os.path.sep + os.path.sep.join(exclude_raw)
+        exclude = os.path.sep.join(exclude_raw)
         excludes.append(exclude)
-
     if DEBUG:
         print('EXCLUDED DIRECTORIES:')
         print(json.dumps(excludes, indent = 4, ensure_ascii=False, sort_keys=False))
@@ -147,12 +138,11 @@ def filter_sources():
     for command in commands:
         if DEBUG:
             print(command['file'])
-        # use relative path to remove the common part of the path to increase speed & reliability
-        # path string starts artificially with path separator to prevent partial matches
-        path_str = os.path.sep + os.path.relpath(os.path.dirname(command['file']), path_root)
+
+        path_str = os.path.dirname(command['file'])
         include_folder = True
         for exclude in excludes:
-            if exclude in path_str:
+            if f'{os.path.sep}{exclude}{os.path.sep}' in path_str:
                 include_folder = False
                 break
 
@@ -173,33 +163,29 @@ def filter_headers():
     During the static analysis system headers are ignored unless stated otherwise.
     """
     if isystem:
-        # strip the leading path separator
-        for index, value in enumerate(excludes):
-            excludes[index] = value.lstrip(os.path.sep)
         for command in commands_inc:
+            path_str = command['command']
             for exclude in excludes:
-                exclude_path = os.path.join(path_root, exclude)
-                header_include = f'-I{exclude_path}'
-                if header_include in command['command']:
+                path_mod = re.subn(rf'\s?-I(\S*{os.path.sep}{exclude}{os.path.sep}\S*)\s?', r' -isystem \1 ', path_str)
+                if path_mod[1] > 0:
                     if DEBUG:
-                        print(f"-- {command['command']}")
-                    header_isystem = f'-isystem {exclude_path}'
-                    command['command'] = command['command'].replace(header_include, header_isystem)
+                        print(f"-- {path_str}")
+                    command['command'] = path_mod[0]
                     if DEBUG:
-                        print(f"++ {command['command']}")
+                        print(f"++ {path_mod[1]} replacements:\n++ {path_mod[0]}")
 
 def save_commands():
     """
     Save two JSON files, one for included and one for excluded commands.
     Line counts of these files must match the original commands, minus the extra '[' and ']' lines.
     """
-    PATH_COMPILE_COMMANDS_INC = os.path.join(path_build, 'compile_commands_inc.json')
-    PATH_COMPILE_COMMANDS_EXC = os.path.join(path_build, 'compile_commands_exc.json')
+    path_compile_commands_inc = os.path.join(path_build, 'compile_commands_inc.json')
+    path_compile_commands_exc = os.path.join(path_build, 'compile_commands_exc.json')
 
-    with open(PATH_COMPILE_COMMANDS_INC, 'w') as commands_inc_json:
+    with open(path_compile_commands_inc, 'w') as commands_inc_json:
         json.dump(commands_inc, commands_inc_json, indent=4, ensure_ascii=False, sort_keys=False)
 
-    with open(PATH_COMPILE_COMMANDS_EXC, 'w') as commands_exc_json:
+    with open(path_compile_commands_exc, 'w') as commands_exc_json:
         json.dump(commands_exc, commands_exc_json, indent=4, ensure_ascii=False, sort_keys=False)
 
 def main():