#Fix 30 Add shellcheck to CI and resolve shellcheck warnings and most…

… suggest… (#38) * Add shellcheck to CI and resolve shellcheck warnings and most suggestions * Lock down standards a bit more and tidy up * move config into shellcheckrc
tgenv · May 21, 2024 · 7a1a8ab · 7a1a8ab
1 parent 47fed40
commit 7a1a8ab
Show file tree

Hide file tree

Showing 16 changed files with 117 additions and 61 deletions.
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -10,6 +10,17 @@ on:
   workflow_dispatch:
 
 jobs:
+  shellcheck:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: 'actions/checkout@v2'
+        with:
+          fetch-depth: 1
+      - name: run shellcheck
+        uses: sudo-bot/action-shellcheck@latest
+        with:
+            cli-args: "test/* bin/* libexec/*"
+
   test-pull-requests-docker:
     if: github.event_name == 'pull_request' || (github.event_name == 'push' && github.ref == 'refs/heads/main')
     runs-on: ubuntu-latest

diff --git a/.shellcheckrc b/.shellcheckrc
@@ -0,0 +1,21 @@
+enable=require-variable-braces
+enables=quote-safe-variables
+
+# Disable SC1091: Not following (<REASON>)
+#
+# https://www.shellcheck.net/wiki/SC1091
+disable=SC1091
+
+# Disable SC2012: Use find instead of ls to better handle non-alphanumeric filenames.
+# https://www.shellcheck.net/wiki/SC2012
+# Notes: We should revisit this one maybe
+disable=SC2012
+
+# Disable SC2015: Note that A && B || C is not if-then-else. C may run when A is true.
+# https://www.shellcheck.net/wiki/SC2015
+disable=SC2015
+
+# https://www.shellcheck.net/wiki/SC2143
+# Use grep -q instead of comparing output with [ -n .. ].
+# Notes: We should revisit this one maybe
+disable=SC2143
diff --git a/README.md b/README.md
@@ -39,7 +39,6 @@ Currently tgenv supports the following OSes
  * [Uninstalling](#uninstalling-no_entry_sign) :no_entry_sign:
  * [License](#license-thumbsup) :thumbsup:
 
-
 ---
 
 ## Installation :wrench:
@@ -236,3 +235,22 @@ And delete the previous export `$PATH` .
 ## LICENSE :thumbsup:
 - [tgenv itself](https://github.com/tgenv/tgenv/blob/master/LICENSE)
 - [tfenv ](https://github.com/kamatama41/tgenv/blob/master/LICENSE) : tfenv mainly uses tfenv's source code
+
+## Contributing
+
+This project requires contributions adhere to [shellcheck](https://www.shellcheck.net/) standards which are configured in [.shellcheckrc](.shellcheckrc).
+
+Shell check can be run locally in your CLI. Refer to the shell check README for [installation instructions](https://github.com/koalaman/shellcheck?tab=readme-ov-file#installing).
+
+```bash
+shellcheck test/* bin/* libexec/*
+```
+
+### Auto Fix
+
+Shellcheck can output diffs which can then be automatically applied to the code. See for details.
+
+```
+shellcheck -f diff my-script.sh | patch
+```
+
diff --git a/bin/terragrunt b/bin/terragrunt
@@ -2,11 +2,11 @@
 set -e
 [ -n "${TGENV_DEBUG}" ] && set -x
 
-program="${0##*/}"
+# program="${0##*/}"
 
 export TGENV_DISABLE_COLOR=${TGENV_DISABLE_COLOR:-0}
 for arg in "$@"; do
-  if [ "$arg" = "-no-color" ]; then
+  if [ "${arg}" = "-no-color" ]; then
     export TGENV_DISABLE_COLOR=1
     break
   fi

diff --git a/bin/tgenv b/bin/tgenv
@@ -11,7 +11,7 @@ readlink_f() {
   local file_name
 
   while [ "${target_file}" != "" ]; do
-    cd "$(dirname ${target_file})"
+    cd "$(dirname "${target_file}")"
     file_name="$(basename "${target_file}")"
     target_file="$(readlink "${file_name}")"
   done
@@ -29,9 +29,12 @@ PATH="${TGENV_ROOT}/libexec:${PATH}"
 export PATH
 export TGENV_DIR="${PWD}"
 
+# shellcheck disable=SC2120
 abort() {
-  { if [ "${#}" -eq 0 ]; then cat -
-    else echo "tgenv: ${*}"
+  { if [ "${#}" -eq 0 ]; then
+        cat -
+    else
+        echo "tgenv: ${*}"
     fi
   } >&2
   exit 1

diff --git a/libexec/helpers b/libexec/helpers
@@ -14,19 +14,19 @@ RESET="\033[0;39m"
 
 
 function error_and_die() {
-  if [[ "$TGENV_DISABLE_COLOR" -eq 1 ]]; then
-    echo -e "tgenv: $(basename ${0}): [ERROR] ${1}" >&2
+  if [[ "${TGENV_DISABLE_COLOR}" -eq 1 ]]; then
+    echo -e "tgenv: $(basename "${0}"): [ERROR] ${1}" >&2
   else
-    echo -e "tgenv: $(basename ${0}): ${RED}[ERROR] ${1}${RESET}" >&2
+    echo -e "tgenv: $(basename "${0}"): ${RED}[ERROR] ${1}${RESET}" >&2
   fi
   exit 1
 }
 
 function warn_and_continue() {
-  if [[ "${TGENV_DISABLE_COLOR}" -eq 1 ]]; then
-    echo -e "tgenv: $(basename ${0}): [WARN] ${1}" >&2
+  if [ "${TGENV_DISABLE_COLOR}" -eq 1 ]; then
+    echo -e "tgenv: $(basename "${0}"): [WARN] ${1}" >&2
   else
-    echo -e "tgenv: $(basename ${0}): ${YELLOW}[WARN] ${1}${RESET}" >&2
+    echo -e "tgenv: $(basename "${0}"): ${YELLOW}[WARN] ${1}${RESET}" >&2
   fi
 }
 
@@ -57,5 +57,5 @@ function curlw () {
     TLS_OPT=""
   fi
 
-  curl ${TLS_OPT} "$@"
+  curl "${TLS_OPT}" "$@"
 }
diff --git a/libexec/tgenv---version b/libexec/tgenv---version
@@ -20,4 +20,4 @@ if cd "${BASH_SOURCE%/*}" 2>/dev/null && git remote -v 2>/dev/null | grep -q tge
   git_revision="${git_revision#v}"
 fi
 
-echo "tgenv ${git_revision:-$version}"
+echo "tgenv ${git_revision:-${version}}"
diff --git a/libexec/tgenv-list-remote b/libexec/tgenv-list-remote
@@ -19,10 +19,10 @@ git ls-remote --tags https://github.com/gruntwork-io/terragrunt \
   | tee "${tmp_list_all_versions_offline}"
 return_code=$?
 
-if [ $return_code != 0 ];then
+if [ "${return_code}" != 0 ];then
   [ -f "${tmp_list_all_versions_offline}" ] && rm -f "${tmp_list_all_versions_offline}"
   warn_and_continue "Failed to list tags for https://github.com/gruntwork-io/terragrunt"
-  print=`cat ${TGENV_ROOT}/list_all_versions_offline`
+#   print=`cat ${TGENV_ROOT}/list_all_versions_offline`
 else
   mv -f "${tmp_list_all_versions_offline}" "${TGENV_ROOT}/list_all_versions_offline"
 fi
diff --git a/libexec/tgenv-upgrade b/libexec/tgenv-upgrade
@@ -4,29 +4,29 @@
 #
 # This script just works like a software
 # version updater. It's a shortcut to
-# $ git checkout master && git pull
+# $ git checkout main && git pull
 #
 ##########################################
 
 set -e
 [ -n "${TGENV_DEBUG}" ] && set -x
 source "${TGENV_ROOT}/libexec/helpers"
 
+# Consider using $(git rev-parse --abbrev-ref HEAD) instead
 TGENV_GITBRANCH=$(git branch)
-TGENV_GITCHECKOUTMAIN=$(git -C $TGENV_GITBRANCH checkout main)
+TGENV_GITCHECKOUTMAIN=$(git -C "${TGENV_GITBRANCH}" checkout main)
 TGENV_GITPULL=$(git pull)
-TGENV_GITLOG=$(git -C $TGENV_GITBRANCH log -1)
+TGENV_GITLOG=$(git -C "${TGENV_GITBRANCH}" log -1)
 TGENV_GITSTASH=$(git stash)
 
 # If branch it was different of main executes the checkout them
-if [$TGENV_GITBRANCH != 'main']; 
-then
-   $TGENV_GITSTASH
-   $TGENV_GITCHECKOUTMAIN
+if [ "${TGENV_GITBRANCH}" != 'main' ]; then
+   "${TGENV_GITSTASH}"
+   "${TGENV_GITCHECKOUTMAIN}"
 else
-   $TGENV_GITPULL
+   "${TGENV_GITPULL}"
 fi;
 
-echo $TGENV_GITLOG
+echo "${TGENV_GITLOG}"
 
 exit 0
diff --git a/libexec/tgenv-version-name b/libexec/tgenv-version-name
@@ -5,7 +5,7 @@ set -e
 [ -n "${TGENV_DEBUG}" ] && set -x
 source "${TGENV_ROOT}/libexec/helpers"
 
-mkdir -p ${TGENV_ROOT}/versions
+mkdir -p "${TGENV_ROOT}"/versions
 
 TGENV_VERSION_FILE="$(tgenv-version-file)"
 TGENV_VERSION="$(cat "${TGENV_VERSION_FILE}" || true)"

diff --git a/test/install_deps.sh b/test/install_deps.sh
@@ -1,7 +1,7 @@
 #!/usr/bin/env bash
 set -uo pipefail;
 
-if [[ $(uname) == 'Darwin' ]] && [ $(which brew) ]; then
+if [[ $(uname) == 'Darwin' ]] && [ "$(command -v brew)" ]; then
   brew install grep;
   exit
 fi;
diff --git a/test/run.sh b/test/run.sh
@@ -5,18 +5,21 @@ if [ -n "${TGENV_DEBUG}" ]; then
   set -x
 fi
 
-export TGENV_ROOT=$(cd $(dirname ${0})/.. && pwd)
+TGENV_ROOT="$(cd "$(dirname "${0}")"/.. && pwd)"
+export TGENV_ROOT
+
 export PATH="${TGENV_ROOT}/bin:${PATH}"
 
 errors=()
 if [ ${#} -ne 0 ];then
-  targets="${@}"
+  targets="$*"
 else
-  targets=$(\ls $(dirname ${0}) | grep 'test_')
+  # shellcheck disable=SC2010
+  targets=$(\ls "$(dirname "${0}")" | grep 'test_')
 fi
 
 for t in ${targets}; do
-  bash $(dirname ${0})/${t} || errors+=( ${t} )
+  bash "$(dirname "${0}")/${t}" || errors+=( "${t}" )
 done
 
 if [ ${#errors[@]} -ne 0 ];then

diff --git a/test/test_install_and_use.sh b/test/test_install_and_use.sh
@@ -4,9 +4,9 @@ declare -a errors
 
 source "${TGENV_ROOT}/libexec/helpers"
 
-[ -n "$TGENV_DEBUG" ] && set -x
-source $(dirname $0)/helpers.sh \
-  || error_and_die "Failed to load test helpers: $(dirname $0)/helpers.sh"
+[ -n "${TGENV_DEBUG}" ] && set -x
+source "$(dirname "${0}")/helpers.sh" \
+  || error_and_die "Failed to load test helpers: $(dirname "${0}")/helpers.sh"
 
 ##################################################
 # Test install latest version
@@ -17,8 +17,8 @@ cleanup || error_and_die "Cleanup failed?!"
 v=$(tgenv list-remote | head -n 1)
 (
   tgenv install latest || exit 1
-  tgenv use ${v} || exit 1
-  check_version ${v} || exit 1
+  tgenv use "${v}" || exit 1
+  check_version "${v}" || exit 1
 ) || error_and_proceed "Installing latest version ${v}"
 
 ##################################################
@@ -31,7 +31,7 @@ v=$(tgenv list-remote | grep 0.37.4 | head -n 1)
 (
   tgenv install latest:^0.37 || exit 1
   tgenv use latest:^0.37 || exit 1
-  check_version ${v} || exit 1
+  check_version "${v}" || exit 1
 ) || error_and_proceed "Installing latest version ${v} with Regex"
 
 ##################################################
@@ -42,9 +42,9 @@ cleanup || error_and_die "Cleanup failed?!"
 
 v=0.36.3
 (
-  tgenv install ${v} || exit 1
-  tgenv use ${v} || exit 1
-  check_version ${v} || exit 1
+  tgenv install "${v}" || exit 1
+  tgenv use "${v}" || exit 1
+  check_version "${v}" || exit 1
 ) || error_and_proceed "Installing specific version ${v}"
 
 ##################################################
@@ -54,10 +54,10 @@ echo "### Install specific .terragrunt-version"
 cleanup || error_and_die "Cleanup failed?!"
 
 v=0.36.1
-echo ${v} > ./.terragrunt-version
+echo "${v}" > ./.terragrunt-version
 (
   tgenv install || exit 1
-  check_version ${v} || exit 1
+  check_version "${v}" || exit 1
 ) || error_and_proceed "Installing .terragrunt-version ${v}"
 
 ##################################################
@@ -83,7 +83,7 @@ v=$(tgenv list-remote | grep -e '^0.38' | head -n 1)
 echo "latest:^0.38" > ./.terragrunt-version
 (
   tgenv install || exit 1
-  check_version ${v} || exit 1
+  check_version "${v}" || exit 1
 ) || error_and_proceed "Installing .terragrunt-version ${v}"
 
 ##################################################
@@ -94,7 +94,7 @@ cleanup || error_and_die "Cleanup failed?!"
 
 v=9.9.9
 expected_error_message="No versions matching '${v}' found in remote"
-[ -z "$(tgenv install ${v} 2>&1 | grep "${expected_error_message}")" ] \
+[ -z "$(tgenv install "${v}" 2>&1 | grep "${expected_error_message}")" ] \
   && error_and_proceed "Installing invalid version ${v}"
 
 ##################################################
@@ -105,7 +105,7 @@ cleanup || error_and_die "Cleanup failed?!"
 
 v="latest:word"
 expected_error_message="No versions matching '${v}' found in remote"
-[ -z "$(tgenv install ${v} 2>&1 | grep "${expected_error_message}")" ] \
+[ -z "$(tgenv install "${v}" 2>&1 | grep "${expected_error_message}")" ] \
   && error_and_proceed "Installing invalid version ${v}"
 
 if [ ${#errors[@]} -gt 0 ]; then

diff --git a/test/test_list.sh b/test/test_list.sh
@@ -6,8 +6,8 @@ declare -a errors
 source "${TGENV_ROOT}/libexec/helpers"
 
 [ "${TGENV_DEBUG:-0}" -gt 0 ] && set -x
-source $(dirname $0)/helpers.sh \
-  || error_and_die "Failed to load test helpers: $(dirname $0)/helpers.sh"
+source "$(dirname "$0")/helpers.sh" \
+  || error_and_die "Failed to load test helpers: $(dirname "${0}")/helpers.sh"
 
 echo "### List local versions"
 cleanup || error_and_die "Cleanup failed?!"
@@ -20,7 +20,7 @@ versions=(
   0.29.7
 )
 for v in "${versions[@]}"; do
-  tgenv install ${v} || error_and_proceed "Install of version ${v} failed"
+  tgenv install "${v}" || error_and_proceed "Install of version ${v} failed"
 done
 tgenv use 0.38.12
 

diff --git a/test/test_symlink.sh b/test/test_symlink.sh
@@ -5,21 +5,21 @@ declare -a errors
 source "${TGENV_ROOT}/libexec/helpers"
 
 [ -n "${TGENV_DEBUG}" ] && set -x
-source $(dirname $0)/helpers.sh \
-  || error_and_die "Failed to load test helpers: $(dirname $0)/helpers.sh"
+source "$(dirname "${0}")/helpers.sh" \
+  || error_and_die "Failed to load test helpers: $(dirname "${0}")/helpers.sh"
 
 TGENV_BIN_DIR=/tmp/tgenv-test
-rm -rf ${TGENV_BIN_DIR} && mkdir ${TGENV_BIN_DIR}
-ln -s ${PWD}/bin/* ${TGENV_BIN_DIR}
+rm -rf "${TGENV_BIN_DIR}" && mkdir "${TGENV_BIN_DIR}"
+ln -s "${PWD}"/bin/* "${TGENV_BIN_DIR}"
 export PATH="${TGENV_BIN_DIR}:${PATH}"
 
 echo "### Test supporting symlink"
 cleanup || error_and_die "Cleanup failed?!"
 
 v=0.35.17
-tgenv install ${v} || error_and_proceed "Install failed"
-tgenv use ${v} || error_and_proceed "Use failed"
-check_version ${v} || error_and_proceed "Version check failed"
+tgenv install "${v}" || error_and_proceed "Install failed"
+tgenv use "${v}" || error_and_proceed "Use failed"
+check_version "${v}" || error_and_proceed "Version check failed"
 
 if [ ${#errors[@]} -gt 0 ]; then
   echo -e "\033[0;31m===== The following symlink tests failed =====\033[0;39m" >&2