From 37c7c5b58d6621fdb5177e91963a8274440efbc2 Mon Sep 17 00:00:00 2001
From: Marek Fiala <marek.fiala@espressif.com>
Date: Tue, 5 Sep 2023 11:09:52 +0200
Subject: [PATCH] feat(tools): Add shellcheck for shell scripts to CI

- shellcheck fixes in shell scripts
---
 .pre-commit-config.yaml | 11 +++++++++++
 .shellcheckrc           |  5 +++++
 export.sh               |  7 ++++---
 install.sh              | 10 ++++++----
 4 files changed, 26 insertions(+), 7 deletions(-)
 create mode 100644 .shellcheckrc

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index a1964a9c97..773f183388 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -208,3 +208,14 @@ repos:
       - id: astyle_py
         # If you are modifying astyle version, update tools/format.sh as well
         args: ['--astyle-version=3.4.7', '--rules=tools/ci/astyle-rules.yml']
+  - repo: https://github.com/shellcheck-py/shellcheck-py
+    rev: v0.9.0.5
+    hooks:
+    - id: shellcheck
+      name: shellcheck bash
+      args: ['--shell', 'bash', '-x']
+      files: 'install.sh|export.sh'
+    - id: shellcheck
+      name: shellcheck dash (export.sh)
+      args: ['--shell', 'dash', '-x']
+      files: 'export.sh'
diff --git a/.shellcheckrc b/.shellcheckrc
new file mode 100644
index 0000000000..4b9f5f36b8
--- /dev/null
+++ b/.shellcheckrc
@@ -0,0 +1,5 @@
+# This is shellcheck config file
+# Files that are checked: install.sh, export.sh
+
+# Do not complain about variables determined at runtime (IDF_PATH)
+disable=SC1090
diff --git a/export.sh b/export.sh
index 4ef6ff84d9..d658b22eaa 100644
--- a/export.sh
+++ b/export.sh
@@ -65,6 +65,7 @@ __main() {
         self_path="${BASH_SOURCE}"
     elif [ -n "${ZSH_VERSION-}" ]
     then
+    # shellcheck disable=SC2296  # ignore parameter starts with '{' because it's zsh
         self_path="${(%):-%x}"
     fi
 
@@ -138,7 +139,7 @@ __main() {
 
     if [ -n "$BASH" ]
     then
-        path_prefix=${PATH%%${old_path}}
+        path_prefix="${PATH%%"${old_path}"}"
         # shellcheck disable=SC2169,SC2039  # unreachable with 'dash'
         if [ -n "${path_prefix}" ]; then
             __verbose "Added the following directories to PATH:"
@@ -201,7 +202,7 @@ __cleanup() {
     # Not unsetting IDF_PYTHON_ENV_PATH, it can be used by IDF build system
     # to check whether we are using a private Python environment
 
-    return $1
+    return "$1"
 }
 
 
@@ -222,7 +223,7 @@ __enable_autocomplete() {
     elif [ -n "${BASH_SOURCE-}" ]
     then
         WARNING_MSG="WARNING: Failed to load shell autocompletion for bash version: $BASH_VERSION!"
-        # shellcheck disable=SC3028,SC3054,SC2086  # code block for 'bash' only
+        # shellcheck disable=SC3028,SC3054,SC2086,SC2169  # code block for 'bash' only
         [ ${BASH_VERSINFO[0]} -lt 4 ] && { echo "$WARNING_MSG"; return; }
         eval "$(env LANG=en _IDF.PY_COMPLETE=$SOURCE_BASH idf.py)"  || echo "$WARNING_MSG"
     fi
diff --git a/install.sh b/install.sh
index 2206eeff34..868665c527 100755
--- a/install.sh
+++ b/install.sh
@@ -18,18 +18,20 @@ while getopts ":h" option; do
         h)
             "${ESP_PYTHON}" "${IDF_PATH}/tools/install_util.py" print_help sh
             exit;;
+        \?)
+            ;;
     esac
 done
 
-TARGETS=`"${ESP_PYTHON}" "${IDF_PATH}/tools/install_util.py" extract targets "$@"`
+TARGETS=$("${ESP_PYTHON}" "${IDF_PATH}/tools/install_util.py" extract targets "$@")
 
 echo "Installing ESP-IDF tools"
-"${ESP_PYTHON}" "${IDF_PATH}/tools/idf_tools.py" install --targets=${TARGETS}
+"${ESP_PYTHON}" "${IDF_PATH}/tools/idf_tools.py" install --targets="${TARGETS}"
 
-FEATURES=`"${ESP_PYTHON}" "${IDF_PATH}/tools/install_util.py" extract features "$@"`
+FEATURES=$("${ESP_PYTHON}" "${IDF_PATH}/tools/install_util.py" extract features "$@")
 
 echo "Installing Python environment and packages"
-"${ESP_PYTHON}" "${IDF_PATH}/tools/idf_tools.py" install-python-env --features=${FEATURES}
+"${ESP_PYTHON}" "${IDF_PATH}/tools/idf_tools.py" install-python-env --features="${FEATURES}"
 
 echo "All done! You can now run:"
 echo ""