Refactor/shellcheck bin (#3019)

* Shellcheck and update mycroft-config * Shellcheck and update mycroft-pip * Shellcheck and update mycroft-cli-client * Shellcheck and update mycroft-help * Shellcheck and update mycroft-listen * Shellcheck and update mycroft-mic-test * Shellcheck and update mycroft-msk * Shellcheck and update mycroft-msm * Shellcheck and update mycroft-say-to * Shellcheck and update mycroft-skill-testrunner * Shellcheck and update mycroft-speak * Shellcheck and update mycroft-start * Shellcheck and update mycroft-stop * Add shellcheck step to github actions This runs most of the shellcheck tests. The excludes are: - SC1091: Avoids errors when shellcheck can't find sourced file - SC2034: Unused variables, for example colors that aren't used yet - SC2012: use of ls, from what I can see in our case this is fine (wc -l) The version is locked to latest master as of November 2 2021
MycroftAI · Nov 4, 2021 · fd12c88 · fd12c88
1 parent 0ad093a
commit fd12c88
Show file tree

Hide file tree

Showing 14 changed files with 62 additions and 53 deletions.
diff --git a/.github/workflows/unit_test.yml b/.github/workflows/unit_test.yml
@@ -66,3 +66,14 @@ jobs:
       run: if [[ ${{ matrix.python-version }} == 3.9 ]]; then bash <(curl -s https://codecov.io/bash); fi
       env:
         CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
+  shellcheck:
+    name: Shellcheck
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - name: Shell check mycroft tools
+      uses: ludeeus/action-shellcheck@ceeca77f6538b97991ca2c2a2ebe1ab64e573b5e
+      env:
+        SHELLCHECK_OPTS: -x --exclude SC2034 --exclude SC2012 --exclude SC1091
+      with:
+        scandir: ./bin
diff --git a/bin/mycroft-cli-client b/bin/mycroft-cli-client
@@ -21,4 +21,4 @@ DIR="$( dirname "$SOURCE" )"
 source "$DIR/../venv-activate.sh" -q
 
 # Invoke the Command Line Interface
-python -m mycroft.client.text $@
+python -m mycroft.client.text "$@"
diff --git a/bin/mycroft-config b/bin/mycroft-config
@@ -15,7 +15,7 @@
 # limitations under the License.
 
 SOURCE="${BASH_SOURCE[0]}"
-cd -P "$( dirname "$SOURCE" )"
+cd -P "$( dirname "$SOURCE" )" || exit
 DIR="$( pwd )"
 script=${0}
 script=${script##*/}
@@ -49,7 +49,7 @@ function help() {
 
 VIEWER="nano --syntax=json --view"
 if [ -z "$EDITOR" ] ; then
-    if [ $( which sensible-editor ) ] ; then
+    if which sensible-editor > /dev/null ; then
         EDITOR="sensible-editor"
     else
         EDITOR="nano --syntax=json --tempfile"
@@ -82,14 +82,14 @@ function validate_config_file() {
         return 0
     fi
 
-    echo -n ${BLUE}
+    echo -n "${BLUE}"
 
     # Remove any comments (lines starting with # or //) found in the file and
     # Use jq to validate and output errors
     sed 's/^\s*[#\/].*$//g' "$1" | sed '/^$/d' | jq -e "." > /dev/null
     result=$?
 
-    echo -n ${RESET}
+    echo -n "${RESET}"
 
     #xxx echo "RESULT=$result for $1"
     return $result
@@ -99,7 +99,7 @@ _conf_file="${XDG_CONFIG_HOME:-$HOME/.config}/mycroft/mycroft.conf"
 function name_to_path() {
     case ${1} in
         "system")     _conf_file="/etc/mycroft/mycroft.conf" ;;
-        "user")       _conf_file=$(readlink -f ${XDG_CONFIG_HOME:-$HOME/.config}/mycroft/mycroft.conf) ;;
+        "user")       _conf_file=$(readlink -f "${XDG_CONFIG_HOME:-$HOME/.config}/mycroft/mycroft.conf") ;;
         "default")    _conf_file="$DIR/../mycroft/configuration/mycroft.conf" ;;
         "remote")     _conf_file="$HOME/.cache/mycroft/web_cache.json" ;;
 
@@ -113,12 +113,12 @@ function name_to_path() {
 ################################################################
 
 function edit_config() {
-    name_to_path $1
-    validate_config_file $_conf_file
+    name_to_path "$1"
+    validate_config_file "$_conf_file"
     rc=$?
     if [ $rc -ne 0 ] ; then
         echo "${YELLOW}WARNING: ${RESET}Configuration file did not pass validation before edits."
-        read -p "Review errors above and press ENTER to continue with editing."
+        read -r -p "Review errors above and press ENTER to continue with editing."
     fi
 
     if [ -f "${_conf_file}" ] ; then
@@ -128,7 +128,7 @@ function edit_config() {
         echo "}" >> "${TEMP}/mycroft.json"
     fi
 
-    while [ 1 ] ; do
+    while true ; do
         case $1 in
         system | user)
             # Allow user to edit
@@ -150,19 +150,18 @@ function edit_config() {
         fi
 
         # file was changed, validate changes
-        validate_config_file $TEMP/mycroft.json
-        if [ $? -ne 0 ] ; then
+        if validate_config_file $TEMP/mycroft.json > /dev/null ; then
+            key="S"
+        else
             echo "${YELLOW}WARNING: ${RESET}Configuration file does not pass validation, see errors above."
             echo "Press X to abandon changes, S to force save, any other key to edit again."
-            read -N1 -s key
-        else
-            key="S"
+            read -r -N1 -s key
         fi
 
         case $key in
         [Ss])
             echo "Saving..."
-            mv $TEMP/mycroft.json $_conf_file
+            mv $TEMP/mycroft.json "$_conf_file"
             signal_reload_config
             break
             ;;
@@ -180,11 +179,11 @@ function signal_reload_config() {
     source "$DIR/../venv-activate.sh" -q
 
     # Post a messagebus notification to reload the config file
-    output=$(python -m mycroft.messagebus.send "configuration.updated"  "{}")
+    python -m mycroft.messagebus.send "configuration.updated"  "{}" > /dev/null
 }
 
 function show_config() {
-    name_to_path $1
+    name_to_path "$1"
 
     # Use jq to display formatted nicely (after stripping out comments)
     sed 's/^\s*[#\/].*$//g' "${_conf_file}" | sed '/^$/d' | jq "."
@@ -201,7 +200,7 @@ function get_config() {
     json_config=$( source "$DIR/../venv-activate.sh" -q && python -c "import json; from mycroft.configuration import Configuration; print(json.dumps(Configuration.get()))" )
 
     # Read the given variable from the mix
-    echo ${json_config} | jq -r "${value}"
+    echo "${json_config}" | jq -r "${value}"
 }
 
 function set_config() {
@@ -212,27 +211,26 @@ function set_config() {
         value=".${value}"
     fi
 
-    jq "${value} = \"$2\"" $_conf_file > "${TEMP}/~mycroft.conf"
-    if [ $? -eq 0 ] ; then
+    if jq "${value} = \"$2\"" "$_conf_file" > "${TEMP}/~mycroft.conf" ; then
         # Successful update, replace the config file
-        mv "${TEMP}/~mycroft.conf" $_conf_file
+        mv "${TEMP}/~mycroft.conf" "$_conf_file"
         signal_reload_config
     fi
 }
 
 _opt=$1
 case ${_opt} in
     "edit")
-        edit_config $2
+        edit_config "$2"
         ;;
     "reload")
         signal_reload_config
         ;;
     "show")
-        show_config $2
+        show_config "$2"
         ;;
     "get")
-        get_config $2
+        get_config "$2"
         ;;
     "set")
         set_config "$2" "$3"

diff --git a/bin/mycroft-help b/bin/mycroft-help
@@ -15,7 +15,7 @@
 # limitations under the License.
 
 SOURCE="${BASH_SOURCE[0]}"
-cd -P "$( dirname "$SOURCE" )"/..
+cd -P "$( dirname "$SOURCE" )"/.. || exit
 DIR="$( pwd )"
 
 echo -e "\e[36mMycroft\e[0m is your open source voice assistant.  Full source"

diff --git a/bin/mycroft-listen b/bin/mycroft-listen
@@ -15,7 +15,7 @@
 # limitations under the License.
 
 SOURCE="${BASH_SOURCE[0]}"
-cd -P "$( dirname "$SOURCE" )"
+cd -P "$( dirname "$SOURCE" )" || exit
 DIR="$( pwd )"
 
 # Enter the Mycroft venv

diff --git a/bin/mycroft-mic-test b/bin/mycroft-mic-test
@@ -15,7 +15,7 @@
 # limitations under the License.
 
 SOURCE="${BASH_SOURCE[0]}"
-cd -P "$( dirname "$SOURCE" )"
+cd -P "$( dirname "$SOURCE" )" || exit
 DIR="$( pwd )"
 
 restart=0
@@ -32,7 +32,7 @@ fi
 
 
 # Launch the standard audiotest
-"$DIR/../start-mycroft.sh" audiotest $@
+"$DIR/../start-mycroft.sh" audiotest "$@"
 
 
 if [ $restart -eq 1 ]

diff --git a/bin/mycroft-msk b/bin/mycroft-msk
@@ -21,4 +21,4 @@ DIR="$( dirname "$SOURCE" )"
 source "$DIR/../venv-activate.sh" -q
 
 # Invoke the Mycroft Skills Kit from within the venv
-msk $@
+msk "$@"
diff --git a/bin/mycroft-msm b/bin/mycroft-msm
@@ -21,4 +21,4 @@ DIR="$( dirname "$SOURCE" )"
 source "$DIR/../venv-activate.sh" -q
 
 # Invoke the Mycroft Skills Manager (msm) within the venv
-msm $@
+msm "$@"
diff --git a/bin/mycroft-pip b/bin/mycroft-pip
@@ -21,4 +21,4 @@ DIR="$( dirname "$SOURCE" )"
 source "$DIR/../venv-activate.sh" -q
 
 # Install pip packages within the Mycroft venv
-pip $@
+pip "$@"
diff --git a/bin/mycroft-say-to b/bin/mycroft-say-to
@@ -15,7 +15,7 @@
 # limitations under the License.
 
 SOURCE="${BASH_SOURCE[0]}"
-cd -P "$( dirname "$SOURCE" )"
+cd -P "$( dirname "$SOURCE" )" || exit
 DIR="$( pwd )"
 
 # Enter the Mycroft venv
@@ -25,5 +25,5 @@ source "$DIR/../venv-activate.sh" -q
 set -- "${1:-$(</dev/stdin)}" "${@:2}"
 
 # Send a message to be spoken
-data="$@"
+data="$*"
 output=$(python -m mycroft.messagebus.send "recognizer_loop:utterance"  "{\"utterances\": [\"$data\"], \"lang\": \"en-us\"}")
diff --git a/bin/mycroft-skill-testrunner b/bin/mycroft-skill-testrunner
@@ -22,24 +22,24 @@ source "$DIR/../venv-activate.sh" -q
 
 function vktest-clear() {
     FEATURES_DIR="$DIR/../test/integrationtests/voight_kampff/features"
-    num_feature_files=$(ls $FEATURES_DIR | wc -l) 
+    num_feature_files=$(ls "$FEATURES_DIR" | wc -l) 
     # A clean directory will have `steps/` and `environment.py`
-    if [ $num_feature_files -gt "2" ] ; then
+    if [ "$num_feature_files" -gt "2" ] ; then
         echo "Removing Feature files..."
-        rm ${DIR}/../test/integrationtests/voight_kampff/features/*.feature
-        rm ${DIR}/../test/integrationtests/voight_kampff/features/*.config.json
+        rm "${DIR}"/../test/integrationtests/voight_kampff/features/*.feature
+        rm "${DIR}"/../test/integrationtests/voight_kampff/features/*.config.json
     fi
     STEPS_DIR="$FEATURES_DIR/steps"
-    num_steps_files=$(ls $STEPS_DIR | wc -l)
-    if [ $num_steps_files -gt "2" ] ; then
+    num_steps_files=$(ls "$STEPS_DIR" | wc -l)
+    if [ "$num_steps_files" -gt "2" ] ; then
         echo "Removing Custom Step files..."
         TMP_DIR="$STEPS_DIR/tmp"
-        mkdir $TMP_DIR
-        mv "$STEPS_DIR/configuration.py" $TMP_DIR
-        mv "$STEPS_DIR/utterance_responses.py" $TMP_DIR
-        rm ${STEPS_DIR}/*.py
-        mv ${TMP_DIR}/* $STEPS_DIR
-        rmdir $TMP_DIR
+        mkdir "$TMP_DIR"
+        mv "$STEPS_DIR/configuration.py" "$TMP_DIR"
+        mv "$STEPS_DIR/utterance_responses.py" "$TMP_DIR"
+        rm "${STEPS_DIR}"/*.py
+        mv "${TMP_DIR}"/* "$STEPS_DIR"
+        rmdir "$TMP_DIR"
     fi
     echo "Voight Kampff tests clear."
 }
@@ -55,5 +55,5 @@ elif [ "$1" = "vktest" ] ; then
         python -m test.integrationtests.voight_kampff "$@"
     fi
 else
-    python -m test.integrationtests.skills.runner $@
+    python -m test.integrationtests.skills.runner "$@"
 fi
diff --git a/bin/mycroft-speak b/bin/mycroft-speak
@@ -14,7 +14,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 SOURCE="${BASH_SOURCE[0]}"
-cd -P "$( dirname "$SOURCE" )"
+cd -P "$( dirname "$SOURCE" )" || exit
 DIR="$( pwd )"
 
 # Sets var 1 to stdin if no args were given
@@ -23,5 +23,5 @@ set -- "${1:-$(</dev/stdin)}" "${@:2}"
 source "$DIR/../venv-activate.sh" -q
 
 # Send a message to be spoken
-data="$@"
+data="$*"
 output=$(python -m mycroft.messagebus.send "speak"  "{\"utterance\": \"$data\"}")
diff --git a/bin/mycroft-start b/bin/mycroft-start
@@ -15,7 +15,7 @@
 # limitations under the License.
 
 SOURCE="${BASH_SOURCE[0]}"
-cd -P "$( dirname "$SOURCE" )"/..
+cd -P "$( dirname "$SOURCE" )"/.. || exit
 DIR="$( pwd )"
 
-. "$DIR/start-mycroft.sh" $@
+. "$DIR/start-mycroft.sh" "$@"
diff --git a/bin/mycroft-stop b/bin/mycroft-stop
@@ -15,7 +15,7 @@
 # limitations under the License.
 
 SOURCE="${BASH_SOURCE[0]}"
-cd -P "$( dirname "$SOURCE" )"/..
+cd -P "$( dirname "$SOURCE" )"/.. || exit
 DIR="$( pwd )"
 
-. "$DIR/stop-mycroft.sh" $@
+. "$DIR/stop-mycroft.sh" "$@"