Shellcheck Tweaks (#6147)

.github/workflows/test.yml

@@ -33,7 +33,7 @@ jobs:
         with:
           check_together: 'yes'
           format: tty
-          severity: error
+          severity: warning
 
       - name: Spell-Checking
         uses: codespell-project/actions-codespell@master

.shellcheckrc

@@ -0,0 +1 @@
+disable=SC1090,SC1091 # Ignore warnings about being unable to follow sourced files

advanced/Scripts/database_migration/gravity-db.sh

@@ -1,5 +1,5 @@
 #!/usr/bin/env bash
-# shellcheck disable=SC1090
+
 
 # Pi-hole: A black hole for Internet advertisements
 # (c) 2019 Pi-hole, LLC (https://pi-hole.net)
@@ -13,9 +13,8 @@
 readonly scriptPath="/etc/.pihole/advanced/Scripts/database_migration/gravity"
 
 upgrade_gravityDB(){
-    local database piholeDir version
+    local database version
     database="${1}"
-    piholeDir="${2}"
 
     # Exit early if the database does not exist (e.g. in CI tests)
     if [[ ! -f "${database}" ]]; then

advanced/Scripts/list.sh

@@ -1,5 +1,4 @@
 #!/usr/bin/env bash
-# shellcheck disable=SC1090
 
 # Pi-hole: A black hole for Internet advertisements
 # (c) 2017 Pi-hole, LLC (https://pi-hole.net)

advanced/Scripts/piholeARPTable.sh

@@ -1,5 +1,4 @@
 #!/usr/bin/env bash
-# shellcheck disable=SC1090
 
 # Pi-hole: A black hole for Internet advertisements
 # (c) 2019 Pi-hole, LLC (https://pi-hole.net)

advanced/Scripts/piholeCheckout.sh

@@ -9,6 +9,7 @@
 # Please see LICENSE file for your rights under this license.
 
 readonly PI_HOLE_FILES_DIR="/etc/.pihole"
+# shellcheck disable=SC2034
 SKIP_INSTALL="true"
 source "${PI_HOLE_FILES_DIR}/automated install/basic-install.sh"
 
@@ -59,6 +60,7 @@ checkout() {
         exit 1;
     fi
 
+    # shellcheck disable=SC2154
     if ! is_repo "${webInterfaceDir}" ; then
         echo -e "  ${COL_LIGHT_RED}Error: Web Admin repo is missing from system!"
         echo -e "  Please re-run install script from https://github.com/pi-hole/pi-hole${COL_NC}"
@@ -103,13 +105,14 @@ checkout() {
         echo "master" > /etc/pihole/ftlbranch
         chmod 644 /etc/pihole/ftlbranch
     elif [[ "${1}" == "core" ]] ; then
+        # shellcheck disable=SC2154
         str="Fetching branches from ${piholeGitUrl}"
         echo -ne "  ${INFO} $str"
         if ! fully_fetch_repo "${PI_HOLE_FILES_DIR}" ; then
             echo -e "${OVER}  ${CROSS} $str"
             exit 1
         fi
-        corebranches=($(get_available_branches "${PI_HOLE_FILES_DIR}"))
+        mapfile -t corebranches < <(get_available_branches "${PI_HOLE_FILES_DIR}")
 
         if [[ "${corebranches[*]}" == *"master"* ]]; then
             echo -e "${OVER}  ${TICK} $str"
@@ -130,13 +133,14 @@ checkout() {
         fi
         checkout_pull_branch "${PI_HOLE_FILES_DIR}" "${2}"
     elif [[ "${1}" == "web" ]] ; then
+        # shellcheck disable=SC2154
         str="Fetching branches from ${webInterfaceGitUrl}"
         echo -ne "  ${INFO} $str"
         if ! fully_fetch_repo "${webInterfaceDir}" ; then
             echo -e "${OVER}  ${CROSS} $str"
             exit 1
         fi
-        webbranches=($(get_available_branches "${webInterfaceDir}"))
+        mapfile -t webbranches < <(get_available_branches "${webInterfaceDir}")
 
         if [[ "${webbranches[*]}" == *"master"* ]]; then
             echo -e "${OVER}  ${TICK} $str"
@@ -167,7 +171,7 @@ checkout() {
 
         # Check if requested branch is available
         echo -e "  ${INFO} Checking for availability of branch ${COL_CYAN}${2}${COL_NC} on GitHub"
-        ftlbranches=( $(git ls-remote https://github.com/pi-hole/ftl | grep "refs/heads" | cut -d'/' -f3- -) )
+        mapfile -t ftlbranches < <(git ls-remote https://github.com/pi-hole/ftl | grep "refs/heads" | cut -d'/' -f3- -)
         # If returned array is empty -> connectivity issue
         if [[ ${#ftlbranches[@]} -eq 0 ]]; then
             echo -e "  ${CROSS} Unable to fetch branches from GitHub. Please check your Internet connection and try again later."
@@ -209,13 +213,15 @@ checkout() {
             # Update local and remote versions via updatechecker
             /opt/pihole/updatecheck.sh
         else
-            if [ $? -eq 1 ]; then
+            local status
+            status=$?
+            if [ $status -eq 1 ]; then
                 # Binary for requested branch is not available, may still be
                 # int he process of being built or CI build job failed
                 printf "  %b Binary for requested branch is not available, please try again later.\\n" ${CROSS}
                 printf "      If the issue persists, please contact Pi-hole Support and ask them to re-generate the binary.\\n"
                 exit 1
-            elif [ $? -eq 2 ]; then
+            elif [ $status -eq 2 ]; then
                 printf "  %b Unable to download from ftl.pi-hole.net. Please check your Internet connection and try again later.\\n" "${CROSS}"
                 exit 1
             else

advanced/Scripts/query.sh

@@ -1,5 +1,5 @@
 #!/usr/bin/env sh
-# shellcheck disable=SC1090
+
 
 # Ignore warning about `local` being undefinded in POSIX
 # shellcheck disable=SC3043

advanced/Scripts/update.sh

@@ -21,7 +21,6 @@ SKIP_INSTALL=true
 # when --check-only is passed to this script, it will not perform the actual update
 CHECK_ONLY=false
 
-# shellcheck disable=SC1090
 source "${PI_HOLE_FILES_DIR}/automated install/basic-install.sh"
 # shellcheck disable=SC1091
 source "/opt/pihole/COL_TABLE"
@@ -36,7 +35,8 @@ source "${PI_HOLE_INSTALL_DIR}/utils.sh"
 # getFTLConfigValue() sourced from utils.sh
 
 # Honour configured paths for the web application.
-readonly ADMIN_INTERFACE_DIR=$(getFTLConfigValue "webserver.paths.webroot")$(getFTLConfigValue "webserver.paths.webhome")
+ADMIN_INTERFACE_DIR=$(getFTLConfigValue "webserver.paths.webroot")$(getFTLConfigValue "webserver.paths.webhome")
+readonly ADMIN_INTERFACE_DIR
 
 GitCheckUpdateAvail() {
     local directory

advanced/Scripts/updatecheck.sh

@@ -42,7 +42,8 @@ function get_remote_hash() {
 # shellcheck disable=SC1091
 . /opt/pihole/utils.sh
 
-readonly ADMIN_INTERFACE_DIR=$(getFTLConfigValue "webserver.paths.webroot")$(getFTLConfigValue "webserver.paths.webhome")
+ADMIN_INTERFACE_DIR=$(getFTLConfigValue "webserver.paths.webroot")$(getFTLConfigValue "webserver.paths.webhome")
+readonly ADMIN_INTERFACE_DIR
 
 # Remove the below three legacy files if they exist
 rm -f "/etc/pihole/GitHubVersions"

advanced/Scripts/utils.sh

@@ -88,8 +88,8 @@ getFTLConfigValue(){
 #######################
 setFTLConfigValue(){
   pihole-FTL --config "${1}" "${2}" >/dev/null
-  if [[ $? -eq 5 ]]; then
+  if [ $? -eq 5 ]; then
-    echo -e "  ${CROSS} ${1} set by environment variable. Please unset it to use this function"
+    printf "  %s %s set by environment variable. Please unset it to use this function\n" "${CROSS}" "${1}"
     exit 5
   fi
 }

advanced/Scripts/version.sh

@@ -16,12 +16,10 @@
 cachedVersions="/etc/pihole/versions"
 
 if [ -f ${cachedVersions} ]; then
-    # shellcheck disable=SC1090
     . "$cachedVersions"
 else
     echo "Could not find /etc/pihole/versions. Running update now."
     pihole updatechecker
-    # shellcheck disable=SC1090
     . "$cachedVersions"
 fi
 

advanced/Templates/pihole-FTL-poststop.sh

@@ -3,7 +3,6 @@
 # Source utils.sh for getFTLConfigValue()
 PI_HOLE_SCRIPT_DIR='/opt/pihole'
 utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh"
-# shellcheck disable=SC1090
 . "${utilsfile}"
 
 # Get file paths

advanced/Templates/pihole-FTL-prestart.sh

@@ -3,7 +3,6 @@
 # Source utils.sh for getFTLConfigValue()
 PI_HOLE_SCRIPT_DIR='/opt/pihole'
 utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh"
-# shellcheck disable=SC1090
 . "${utilsfile}"
 
 # Get file paths

advanced/Templates/pihole-FTL.service

@@ -12,7 +12,6 @@
 # Source utils.sh for getFTLConfigValue(), getFTLPID()
 PI_HOLE_SCRIPT_DIR="/opt/pihole"
 utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh"
-# shellcheck disable=SC1090
 . "${utilsfile}"
 
 

automated install/basic-install.sh

@@ -1,5 +1,4 @@
 #!/usr/bin/env bash
-# shellcheck disable=SC1090
 
 # Pi-hole: A black hole for Internet advertisements
 # (c) Pi-hole (https://pi-hole.net)
@@ -1752,7 +1751,8 @@ checkSelinux() {
 
 check_download_exists() {
     # Check if the download exists and we can reach the server
-    local status=$(curl --head --silent "https://ftl.pi-hole.net/${1}" | head -n 1)
+    local status
+    status=$(curl --head --silent "https://ftl.pi-hole.net/${1}" | head -n 1)
 
     # Check the status code
     if grep -q "200" <<<"$status"; then
@@ -2069,13 +2069,13 @@ FTLcheckUpdate() {
         path="${ftlBranch}/${binary}"
 
         # Check whether or not the binary for this FTL branch actually exists. If not, then there is no update!
-        # shellcheck disable=SC1090
         if ! check_download_exists "$path"; then
-            if [ $? -eq 1 ]; then
+            local status
+            status=$?
+            if [ "${status}" -eq 1 ]; then
                 printf "  %b Branch \"%s\" is not available.\\n" "${INFO}" "${ftlBranch}"
                 printf "  %b Use %bpihole checkout ftl [branchname]%b to switch to a valid branch.\\n" "${INFO}" "${COL_LIGHT_GREEN}" "${COL_NC}"
-                return 2
+            elif [ "${status}" -eq 2 ]; then
-            elif [ $? -eq 2 ]; then
                 printf "  %b Unable to download from ftl.pi-hole.net. Please check your Internet connection and try again later.\\n" "${CROSS}"
                 return 3
             else

automated install/uninstall.sh

@@ -11,7 +11,8 @@
 source "/opt/pihole/COL_TABLE"
 source "/opt/pihole/utils.sh"
 
-readonly ADMIN_INTERFACE_DIR=$(getFTLConfigValue "webserver.paths.webroot")$(getFTLConfigValue "webserver.paths.webhome")
+ADMIN_INTERFACE_DIR=$(getFTLConfigValue "webserver.paths.webroot")$(getFTLConfigValue "webserver.paths.webhome")
+readonly ADMIN_INTERFACE_DIR
 
 while true; do
     read -rp "  ${QST} Are you sure you would like to remove ${COL_WHITE}Pi-hole${COL_NC}? [y/N] " answer
@@ -39,6 +40,7 @@ else
 fi
 
 readonly PI_HOLE_FILES_DIR="/etc/.pihole"
+# shellcheck disable=SC2034
 SKIP_INSTALL="true"
 source "${PI_HOLE_FILES_DIR}/automated install/basic-install.sh"
 

gravity.sh

@@ -1,5 +1,4 @@
 #!/usr/bin/env bash
-# shellcheck disable=SC1090
 
 # Pi-hole: A black hole for Internet advertisements
 # (c) 2017 Pi-hole, LLC (https://pi-hole.net)
@@ -16,11 +15,9 @@ export LC_ALL=C
 PI_HOLE_SCRIPT_DIR="/opt/pihole"
 # Source utils.sh for GetFTLConfigValue
 utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh"
-# shellcheck disable=SC1090
 . "${utilsfile}"
 
 coltable="${PI_HOLE_SCRIPT_DIR}/COL_TABLE"
-# shellcheck disable=SC1090
 . "${coltable}"
 # shellcheck disable=SC1091
 . "/etc/.pihole/advanced/Scripts/database_migration/gravity-db.sh"
@@ -306,7 +303,7 @@ migrate_to_database() {
     fi
 
     # Check if gravity database needs to be updated
-    upgrade_gravityDB "${gravityDBfile}" "${piholeDir}"
+    upgrade_gravityDB "${gravityDBfile}"
 
     # Migrate list files to new database
     if [ -e "${adListFile}" ]; then
@@ -334,7 +331,7 @@ migrate_to_database() {
   fi
 
   # Check if gravity database needs to be updated
-  upgrade_gravityDB "${gravityDBfile}" "${piholeDir}"
+  upgrade_gravityDB "${gravityDBfile}"
 }
 
 # Determine if DNS resolution is available before proceeding
@@ -1100,7 +1097,7 @@ for var in "$@"; do
   "-t" | "--timeit") timed=true ;;
   "-r" | "--repair") repairSelector "$3" ;;
   "-u" | "--upgrade")
-    upgrade_gravityDB "${gravityDBfile}" "${piholeDir}"
+    upgrade_gravityDB "${gravityDBfile}"
     exit 0
     ;;
   "-h" | "--help") helpFunc ;;

pihole

@@ -247,12 +247,14 @@ Time:
   data=$(PostFTLData "dns/blocking" "{ \"blocking\": ${1}, \"timer\": ${tt} }")
 
   # Check the response
-  local extra=" forever"
+  local extra timer
-  local timer="$(echo "${data}"| jq --raw-output '.timer' )"
+  extra=" forever"
+  timer="$(echo "${data}"| jq --raw-output '.timer' )"
   if [[ "${timer}" != "null" ]]; then
     extra=" for ${timer}s"
   fi
-  local str="Pi-hole $(echo "${data}" | jq --raw-output '.blocking')${extra}"
+  local str
+  str="Pi-hole $(echo "${data}" | jq --raw-output '.blocking')${extra}"
 
   # Logout from the API
   LogoutAPI
@@ -375,14 +377,16 @@ statusFunc() {
 
 tailFunc() {
   # Warn user if Pi-hole's logging is disabled
-  local logging_enabled=$(getFTLConfigValue dns.queryLogging)
+  local logging_enabled
+  logging_enabled=$(getFTLConfigValue dns.queryLogging)
   if [[ "${logging_enabled}" != "true" ]]; then
     echo "  ${CROSS} Warning: Query logging is disabled"
   fi
   echo -e "  ${INFO} Press Ctrl-C to exit"
 
   # Get logfile path
-  readonly LOGFILE=$(getFTLConfigValue files.log.dnsmasq)
+  readonly LOGFILE
+  LOGFILE=$(getFTLConfigValue files.log.dnsmasq)
 
   # Strip date from each line
   # Color blocklist/denylist/wildcard entries as red