fix: resolve shellcheck warnings in test and packaging scripts (#518)

* fix: resolve shellcheck warnings in test and packaging scripts

- Add missing shebang to doinst.sh (SC2148)
- Quote variables throughout to prevent word splitting/globbing (SC2086)
- Replace echo "\n" with printf for escape sequences (SC2028)
- Replace backticks with $() (SC2006)
- Replace expr with $(()) arithmetic (SC2003)
- Replace [ ! $(grep ...) ] with ! grep -q (SC2143)
- Replace ls | wc -l with find | wc -l (SC2012)
- Replace printf "---\n" with echo for plain separator lines (SC3045)

All scripts remain POSIX/dash compatible.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: resolve shellcheck warnings in test_btrfs_clone.sh

- Quote $(id -u) to prevent word splitting (SC2046)
- Replace [ -n "$(mount | grep ...)" ] with mount | grep -q (SC2143)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: resolve shellcheck warnings in COMMON and fix CI path

- Add shell directive to COMMON (SC2148)
- Quote variables, fix backticks/expr, fix -o in [ ] (SC2086/SC2006/SC2003/SC2166)
- Fix pattern expansion quoting in cmp_substr (SC2295)
- Quote basename argument (SC2086)
- Fix broken echo strings in error message
- Remove unused TESTS_DIR variable
- Add SC2034 disable comments for variables used by sourcing scripts
- Update SHELLCHECK_OPTS path to test/ so shellcheck can follow COMMON source

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* ci: trigger shellcheck CI re-run

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: use SCRIPTDIR so shellcheck can find COMMON on CI

SHELLCHECK_OPTS used $GITHUB_WORKSPACE which GitHub Actions does not
expand in env: blocks. SCRIPTDIR is shellcheck's built-in token that
resolves to the directory of the script being checked, so shellcheck
can find COMMON alongside each test script without a hardcoded path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: revert find to ls in test_basic.sh, disable SC2012

The OpenSUSE Tumbleweed CI container does not have 'find' in PATH.
Revert to the original ls -A approach and suppress SC2012 since
filenames in the waste folder are controlled and the count is reliable.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: andy5995

.github/workflows/shellcheck.yml

@@ -21,4 +21,4 @@ jobs:
       - name: Run ShellCheck
         uses: ludeeus/action-shellcheck@master
         env:
-          SHELLCHECK_OPTS: -x -P $GITHUB_WORKSPACE
+          SHELLCHECK_OPTS: -x -P SCRIPTDIR

packaging/Slackbuild/rmw/doinst.sh

@@ -1,3 +1,4 @@
+#!/bin/sh
 DEPRECATED="etc/rmwrc"
 if [ -f "$DEPRECATED" ]; then
   rm "$DEPRECATED"

packaging/appimage/pre-appimage.sh

@@ -7,7 +7,7 @@ set -ev
 
 # Set default workspace if not provided
 WORKSPACE=${WORKSPACE:-$(pwd)}
-echo $WORKSPACE
+echo "$WORKSPACE"
 # Check if the workspace path is absolute
 if [[ "$WORKSPACE" != /* ]]; then
   echo "The workspace path must be absolute"
@@ -23,7 +23,7 @@ if [[ "$SOURCE_ROOT" != /* ]]; then
   exit 1
 fi
 # Verify that you're in the source root
-echo $SOURCE_ROOT
+echo "$SOURCE_ROOT"
 test -f "$SOURCE_ROOT/src/main.c"
 
 # Define and create application directory if it doesn't exist
@@ -62,7 +62,7 @@ fi
 cd "$BUILD_DIR"
 ninja
 meson test -v
-meson install --destdir=$APPDIR --skip-subprojects
+meson install --destdir="$APPDIR" --skip-subprojects
 
 # Set up output directory
 OUT_DIR="$WORKSPACE/out"
@@ -76,9 +76,9 @@ export LINUXDEPLOY_OUTPUT_VERSION="$VERSION"
 # Generate AppImage using linuxdeploy
 linuxdeploy \
   --appdir="$APPDIR" \
-  --custom-apprun=$SOURCE_ROOT/packaging/appimage/AppRun \
-  -d $SOURCE_ROOT/packaging/rmw.desktop \
-  --icon-file=$SOURCE_ROOT/packaging/rmw_icon_32x32.png \
+  --custom-apprun="$SOURCE_ROOT"/packaging/appimage/AppRun \
+  -d "$SOURCE_ROOT"/packaging/rmw.desktop \
+  --icon-file="$SOURCE_ROOT"/packaging/rmw_icon_32x32.png \
   --icon-filename=rmw \
-  --executable=$APPDIR/usr/bin/rmw \
+  --executable="$APPDIR"/usr/bin/rmw \
   -o appimage

test/COMMON

@@ -1,50 +1,58 @@
-# included by tests scripts
+# shellcheck shell=sh
+# included by test scripts
 
 create_some_files() {
 mkdir -p somefiles/topdir/dir1/dir2/dir3
 
 c=1
-while [ $c -lt 25 ]
+while [ "$c" -lt 25 ]
 do
-   touch somefiles/topdir/dir1/dir2/dir3/$c
-   c=`expr $c + 1`
+   touch "somefiles/topdir/dir1/dir2/dir3/$c"
+   c=$((c + 1))
 done
 
 touch somefiles/read_only_file
 chmod ugo-w somefiles/read_only_file
 }
 
 cmp_substr () {
-  if [ -z "$1" -o -z "$2" ]; then
+  if [ -z "$1" ] || [ -z "$2" ]; then
     return 1
   fi
-  [ -z "${1##*$2*}" ]
+  [ -z "${1##*"$2"*}" ]
   return $?
 }
 
 if test -z "${MESON_BUILD_ROOT}" || test -z "${RMW_FAKE_HOME}"; then
-  echo "This script is used by the build system. Use 'meson test'
-  echo 'from the builddir."
+  echo "This script is used by the build system. Use 'meson test'"
+  echo "from the builddir."
   exit 1
 fi
 
-RMW_FAKE_HOME="${RMW_FAKE_HOME}"/$(basename $0)_dir
-TESTS_DIR="${MESON_SOURCE_ROOT}/test"
+RMW_FAKE_HOME="${RMW_FAKE_HOME}/$(basename "$0")_dir"
+# shellcheck disable=SC2034
 CONFIG="${MESON_SOURCE_ROOT}/test/conf/rmw.testrc"
+# shellcheck disable=SC2034
 ALT_CONFIG="${MESON_SOURCE_ROOT}/test/conf/rmw.alt.testrc"
+# shellcheck disable=SC2034
 PURGE_DISABLED_CONFIG="${MESON_SOURCE_ROOT}/test/conf/rmw.purge_disabled.testrc"
 export BIN_DIR="${MESON_BUILD_ROOT}"
 
 export RMW_FAKE_HOME
 
-if [ -e ${RMW_FAKE_HOME} ]; then
-  rm -rf ${RMW_FAKE_HOME}
+if [ -e "${RMW_FAKE_HOME}" ]; then
+  rm -rf "${RMW_FAKE_HOME}"
 fi
 
+# shellcheck disable=SC2034
 RMW_TEST_CMD_STRING="${BIN_DIR}/rmw -c $CONFIG"
 
-PRIMARY_WASTE_DIR="${RMW_FAKE_HOME}"/.Waste
+# shellcheck disable=SC2034
+PRIMARY_WASTE_DIR="${RMW_FAKE_HOME}/.Waste"
 
+# shellcheck disable=SC2034
 RMW_ALT_TEST_CMD_STRING="${BIN_DIR}/rmw -c ${ALT_CONFIG}"
+# shellcheck disable=SC2034
 RMW_PURGE_DISABLED_CMD="${BIN_DIR}/rmw -c ${PURGE_DISABLED_CONFIG}"
+# shellcheck disable=SC2034
 SEPARATOR="\n\n--- "

test/test_basic.sh

@@ -16,11 +16,12 @@ echo "rmw should display folders on removable devices that are not mounted"
 echo "$SEPARATOR"
 output="$($RMW_TEST_CMD_STRING -l)"
 echo "${output}"
-test "${output}" = "/mnt/fs/Trash-$(id -u)
+expected="/mnt/fs/Trash-$(id -u)
 /mnt/sda10000/example_waste
-"${MESON_BUILD_ROOT}"/test/rmw-tests-home/test_basic.sh_dir/.Waste
-"${MESON_BUILD_ROOT}"/test/rmw-tests-home/test_basic.sh_dir/.local/share/Waste-2
-"${MESON_BUILD_ROOT}"/test/rmw-tests-home/test_basic.sh_dir/.local/share/Waste-3"
+${MESON_BUILD_ROOT}/test/rmw-tests-home/test_basic.sh_dir/.Waste
+${MESON_BUILD_ROOT}/test/rmw-tests-home/test_basic.sh_dir/.local/share/Waste-2
+${MESON_BUILD_ROOT}/test/rmw-tests-home/test_basic.sh_dir/.local/share/Waste-3"
+test "${output}" = "${expected}"
 
 echo "$SEPARATOR"
 
@@ -29,13 +30,13 @@ cd "${RMW_FAKE_HOME}"
 mkdir tmp-files
 cd "${RMW_FAKE_HOME}"/tmp-files
 
-echo "\n\n == creating temporary files to be deleted"
+printf "\n\n == creating temporary files to be deleted\n"
 for file in 1 2 3; do
   touch $file
 done
 cd "${RMW_FAKE_HOME}"/..
 
-echo "\n\n == rmw should be able to operate on multiple files\n"
+printf "\n\n == rmw should be able to operate on multiple files\n"
 $RMW_TEST_CMD_STRING --verbose "${RMW_FAKE_HOME}"/tmp-files/*
 
 test -f "${PRIMARY_WASTE_DIR}/files/1"
@@ -53,24 +54,26 @@ for file in 1 2 3; do
 done
 $RMW_TEST_CMD_STRING 1 2 3
 
-echo "\n\n == Show contents of the files and info directories"
+printf "\n\n == Show contents of the files and info directories\n"
 
 test -n "$(ls -A "$PRIMARY_WASTE_DIR"/files)"
 test -n "$(ls -A "$PRIMARY_WASTE_DIR"/info)"
 
+# shellcheck disable=SC2012
 output="$(ls -A "$PRIMARY_WASTE_DIR"/files | wc -l | sed 's/ //g')"
 test "$output" = "6"
 
 echo "$SEPARATOR"
 echo "  == rmw should refuse to move a waste folder or a file that resides within a waste folder"
 output="$($RMW_TEST_CMD_STRING "$PRIMARY_WASTE_DIR"/info || true)"
-test "${output}" = " :warning: "$PRIMARY_WASTE_DIR"/info resides within a waste folder and has been ignored
+expected=" :warning: ${PRIMARY_WASTE_DIR}/info resides within a waste folder and has been ignored
 0 items were removed to the waste folder"
+test "${output}" = "${expected}"
 
 # If the file gets removed (which it shouldn't), then the test that follows it will fail
 $RMW_TEST_CMD_STRING "$PRIMARY_WASTE_DIR"/info/1.trashinfo || true
 
-echo "\n\n == Display contents of 1.trashinfo "
+printf "\n\n == Display contents of 1.trashinfo \n"
 cat "$PRIMARY_WASTE_DIR"/info/1.trashinfo
 
 echo "$SEPARATOR"
@@ -88,13 +91,13 @@ $RMW_TEST_CMD_STRING --verbose -z "$PRIMARY_WASTE_DIR"/files/1
 $RMW_TEST_CMD_STRING --verbose -z "$PRIMARY_WASTE_DIR"/files/2
 $RMW_TEST_CMD_STRING --verbose -z "$PRIMARY_WASTE_DIR"/files/3
 
-echo "\n\n == test that the files are restored to their previous locations"
+printf "\n\n == test that the files are restored to their previous locations\n"
 
 test -f "${RMW_FAKE_HOME}"/tmp-files/1
 test -f "${RMW_FAKE_HOME}"/tmp-files/2
 test -f "${RMW_FAKE_HOME}"/tmp-files/3
 
-echo "\n\n == test that the .trashinfo files have been removed"
+printf "\n\n == test that the .trashinfo files have been removed\n"
 
 test ! -f "$PRIMARY_WASTE_DIR"/info/1.trashinfo
 test ! -f "$PRIMARY_WASTE_DIR"/info/2.trashinfo
@@ -118,7 +121,7 @@ cmp_substr "$($RMW_ALT_TEST_CMD_STRING -l)" \
 cmp_substr "$($RMW_TEST_CMD_STRING '')" \
   "skipping"
 
-output=$($RMW_TEST_CMD_STRING ${RMW_FAKE_HOME})
+output=$($RMW_TEST_CMD_STRING "${RMW_FAKE_HOME}")
 cmp_substr "$output" "Skipping"
 
 echo "Basic tests passed"

test/test_btrfs_clone.sh

@@ -22,7 +22,7 @@ IS_BTRFS_MOUNTED="$(mount | grep rmw-btrfs)" || true
 if [ -z "$IS_BTRFS_MOUNTED" ]; then
   sudo mount -o loop "$IMAGE_PATH" \
     "$BTRFS_IMAGE_MOUNTPOINT"
-  sudo chown $(id -u) -R "$BTRFS_IMAGE_MOUNTPOINT"
+  sudo chown "$(id -u)" -R "$BTRFS_IMAGE_MOUNTPOINT"
 fi
 
 cd "$BTRFS_IMAGE_MOUNTPOINT"
@@ -70,7 +70,7 @@ test -f foo
 
 cd
 
-if [ -n "$(mount | grep rmw-btrfs)" ]; then
+if mount | grep -q rmw-btrfs; then
   sudo umount "$BTRFS_IMAGE_MOUNTPOINT"
 fi
 

test/test_media_root.sh

@@ -37,26 +37,26 @@ PREV_RMW_FAKE_HOME=${RMW_FAKE_HOME}
 unset RMW_FAKE_HOME
 mkdir -p "$PREV_RMW_FAKE_HOME"
 test_file_path=${PREV_RMW_FAKE_HOME}/$test_file
-if test -f $test_file_path; then
-  rm $test_file_path
+if test -f "$test_file_path"; then
+  rm "$test_file_path"
 fi
 if test -f /home/andy/src/rmw-project/.Trash-1000/files/$test_file; then
   rm /home/andy/src/rmw-project/.Trash-1000/files/$test_file
 fi
 if test -f /home/andy/src/rmw-project/.Trash-1000/info/$test_file.trashinfo; then
   rm /home/andy/src/rmw-project/.Trash-1000/info/$test_file.trashinfo
 fi
-touch $test_file_path
-$BIN_DIR/rmw -c /home/andy/.config/rmwrc $test_file_path
+touch "$test_file_path"
+"$BIN_DIR"/rmw -c /home/andy/.config/rmwrc "$test_file_path"
 
 output=$(grep Path /home/andy/src/rmw-project/.Trash-1000/info/$test_file.trashinfo)
 
 # There should be no leading '/' in the filename.
-path_expected=$(echo ${MESON_BUILD_ROOT} | sed -e "s/\/home\/andy\/src\/rmw-project\///g")
-echo $path_expected
+path_expected=$(echo "${MESON_BUILD_ROOT}" | sed -e "s/\/home\/andy\/src\/rmw-project\///g")
+echo "$path_expected"
 test "$output" = "Path=${path_expected}/test/rmw-tests-home/test_media_root.sh_dir/media_root_test"
 
-output=$($BIN_DIR/rmw -uvv -c /home/andy/.config/rmwrc | grep media_root_test)
+output=$("$BIN_DIR"/rmw -uvv -c /home/andy/.config/rmwrc | grep media_root_test)
 
 test "$output" = "+'/home/andy/src/rmw-project/.Trash-1000/files/media_root_test' -> '${MESON_BUILD_ROOT}/test/rmw-tests-home/test_media_root.sh_dir/media_root_test'
 -/home/andy/src/rmw-project/.Trash-1000/info/media_root_test.trashinfo"

test/test_purging.sh

@@ -26,14 +26,14 @@ cd "${RMW_FAKE_HOME}"/tmp-files
 create_temp_files() {
   cd "${RMW_FAKE_HOME}"/tmp-files
 
-  echo "\n\n == creating temporary files to be rmw'ed"
+  printf "\n\n == creating temporary files to be rmw'ed\n"
 
   for file in abc 123 xyz; do
     i=0
     while [ "$i" -lt 8 ]
     do
-      echo "0000" >> $file
-      i=`expr $i + 1`
+      echo "0000" >> "$file"
+      i=$((i + 1))
     done
   done
 }
@@ -53,13 +53,12 @@ $RMW_TEST_CMD_STRING --verbose "${RMW_FAKE_HOME}"/somefiles/*
 
 echo "$SEPARATOR"
 echo '  == purging disabled should output a message that purging is disabled'
-output=`$RMW_PURGE_DISABLED_CMD --empty -f`
-expected=`echo "purging is disabled ('expire_age' is set to '0')" | cut -b1-20`
-output=`echo $output | cut -b1-20`
+output=$($RMW_PURGE_DISABLED_CMD --empty -f)
+expected=$(echo "purging is disabled ('expire_age' is set to '0')" | cut -b1-20)
+output=$(echo "$output" | cut -b1-20)
 test "${output}" = "${expected}"
 
 # Should not work if '-f' isn't used"
-substring=
 cmp_substr "$(echo y | $RMW_ALT_TEST_CMD_STRING --purge --empty)" \
   "purge has been skipped"
 
@@ -79,7 +78,7 @@ test -e "$PRIMARY_WASTE_DIR"/files/topdir
 echo "$SEPARATOR"
 echo " == Make sure the correct string (filename) is displayed when using -vvg"
 output="$($RMW_TEST_CMD_STRING -vvg)"
-echo $output
+echo "$output"
 cmp_substr "$output"  "'read_only_file' will be purged in 90$(locale decimal_point)"
 cmp_substr "$output" "'topdir' will be purged in 90$(locale decimal_point)"
 
@@ -116,9 +115,9 @@ test ! -e "$PRIMARY_WASTE_DIR"/info/xyz.trashinfo
 create_temp_files
 
 cd "${RMW_FAKE_HOME}"/..
-echo "\n\n == use a built-in environmental variable to write a"
+printf "\n\n == use a built-in environmental variable to write a\n"
 echo " == fake year to the .trashinfo files when running rmw"
-echo "-----------------------------------------------------\n"
+echo "-----------------------------------------------------"
 
 RMW_FAKE_YEAR=true $RMW_TEST_CMD_STRING --verbose "${RMW_FAKE_HOME}"/tmp-files/*
 cmp_substr "$(cat "$PRIMARY_WASTE_DIR"/info/abc.trashinfo)" \
@@ -167,7 +166,7 @@ echo " == Test 'show_purge_stats' == "
 create_temp_files
 RMW_FAKE_YEAR=True $RMW_TEST_CMD_STRING "${RMW_FAKE_HOME}"/tmp-files/*
 output=$($RMW_TEST_CMD_STRING -g)
-echo $output
+echo "$output"
 cmp_substr "$output" "Purging files based"
 cmp_substr "$output" "3 items purged"
 

test/test_restore.sh

@@ -41,10 +41,11 @@ echo "OUTPUT:"
 echo "---"
 echo "$output"
 echo "---"
-test "$output" = "+'"${RMW_FAKE_HOME}"/.Waste/files/read_only_file' -> '"${RMW_FAKE_HOME}"/somefiles/read_only_file'
--"${RMW_FAKE_HOME}"/.Waste/info/read_only_file.trashinfo
-+'"${RMW_FAKE_HOME}"/.Waste/files/topdir' -> '"${RMW_FAKE_HOME}"/somefiles/topdir'
--"${RMW_FAKE_HOME}"/.Waste/info/topdir.trashinfo"
+expected="+'${RMW_FAKE_HOME}/.Waste/files/read_only_file' -> '${RMW_FAKE_HOME}/somefiles/read_only_file'
+-${RMW_FAKE_HOME}/.Waste/info/read_only_file.trashinfo
++'${RMW_FAKE_HOME}/.Waste/files/topdir' -> '${RMW_FAKE_HOME}/somefiles/topdir'
+-${RMW_FAKE_HOME}/.Waste/info/topdir.trashinfo"
+test "$output" = "$expected"
 
 echo "$SEPARATOR"
 echo "Show result when no undo file exists..."
@@ -74,7 +75,7 @@ $RMW_TEST_CMD_STRING -z nonexistent_fil* && exit 1
 
 echo "$SEPARATOR"
 echo "Symlinks"
-ln -s ${RMW_FAKE_HOME} "${RMW_FAKE_HOME}"/link_test
+ln -s "${RMW_FAKE_HOME}" "${RMW_FAKE_HOME}"/link_test
 # broken link
 ln -s broken_symlink_test "${RMW_FAKE_HOME}"/link_test2
 $RMW_TEST_CMD_STRING "${RMW_FAKE_HOME}"/link_test "${RMW_FAKE_HOME}"/link_test2
@@ -89,8 +90,8 @@ mkdir tmpfoo
 for t in "foo" "bar" ".boo" ".far"; do
   touch tmpfoo/$t
   ${RMW_TEST_CMD_STRING} tmpfoo/$t
-  cat ${PRIMARY_WASTE_DIR}/info/$t.trashinfo
-  cd ${PRIMARY_WASTE_DIR}
+  cat "${PRIMARY_WASTE_DIR}/info/$t.trashinfo"
+  cd "${PRIMARY_WASTE_DIR}"
   ${RMW_TEST_CMD_STRING} -z files/$t
   cd "${RMW_FAKE_HOME}"
   test -f tmpfoo/$t
@@ -100,17 +101,17 @@ done
 for t in ".boo" ".far"; do
   touch tmpfoo/$t
   ${RMW_TEST_CMD_STRING} tmpfoo/.*
-  ${RMW_TEST_CMD_STRING} -z ${PRIMARY_WASTE_DIR}/files/$t
+  ${RMW_TEST_CMD_STRING} -z "${PRIMARY_WASTE_DIR}/files/$t"
   test -f tmpfoo/$t
 done
 
 # a dot dir
-cmp_substr "$(${RMW_TEST_CMD_STRING} -z ${PRIMARY_WASTE_DIR}/files/. && exit 1)" \
+cmp_substr "$(${RMW_TEST_CMD_STRING} -z "${PRIMARY_WASTE_DIR}/files/." && exit 1)" \
   "refusing to process"
 
 # I don't want to force anyone to install Xvfb for this single test
 # so I'll only run it if it's already installed
-if [ -n "$(command -v Xvfb)" ] && [ ! $(grep "DISABLE_CURSES" "$MESON_BUILD_ROOT/src/config.h") ]; then
+if [ -n "$(command -v Xvfb)" ] && ! grep -q "DISABLE_CURSES" "$MESON_BUILD_ROOT/src/config.h"; then
   # Start Xvfb on display :99
   Xvfb :99 &
   XVFB_PID=$!