enable shell check and fix linting (#1177)

.github/workflows/pull_request.yml

@@ -9,7 +9,6 @@ jobs:
     name: Soundness
     uses: swiftlang/github-workflows/.github/workflows/soundness.yml@main
     with:
-      shell_check_enabled: false
       format_check_enabled: false
       license_header_check_project_name: "Swift Distributed Actors"
       # We need to move to 6.0 here but have to fix the new warnings first

IntegrationTests/tests_01_cluster/shared.sh

@@ -17,14 +17,14 @@ RED='\033[0;31m'
 RST='\033[0m'
 
 function echoerr() {
-    echo "${RED}$@${RST}" 1>&2;
+    echo "${RED}$*${RST}" 1>&2;
 }
 
 function _killall() {
     set +e
     local killall_app_name="$1"
     echo "> KILLALL: $killall_app_name"
-    ps aux | grep ${killall_app_name} | awk '{ print $2 }' | xargs kill -9  # ignore-unacceptable-language
+    pkill -f "${killall_app_name}"
     set -e
 }
 
@@ -33,22 +33,22 @@ function wait_log_exists() {
     _expected_line="$2"
     if [[ "$#" -eq 3 ]]; then
         _max_spins="$3"
-        max_spins=$(expr ${_max_spins} + 0)
+        max_spins=$(("${_max_spins}" + 0))
     else
         max_spins=20
     fi
     spin=1 # spin counter
-    while [[ $(cat ${_log_file} | grep "${_expected_line}" | wc -l) -ne 1 ]]; do
+    while [[ $(grep -c "${_expected_line}" < "${_log_file}") -ne 1 ]]; do
         echo "---------------------------------------------------------------------------------------------------------"
-        cat ${_log_file}
+        cat "${_log_file}"
         echo "========================================================================================================="
 
         sleep 1
         spin=$((spin+1))
         if [[ ${spin} -eq ${max_spins} ]]; then
             echoerr "Never saw enough '${_expected_line}' in logs."
-            cat ${_log_file}
-            exit -1
+            cat "${_log_file}"
+            exit 255
         fi
     done
 

IntegrationTests/tests_01_cluster/test_01_suspend_causes_unreachable_unsuspend_causes_reachable.sh

@@ -16,49 +16,52 @@
 set -e
 #set -x # verbose
 
+# shellcheck disable=SC2155
 declare -r my_path="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
-declare -r root_path="$my_path/.."
 
 declare -r app_name='it_Clustered_swim_suspension_reachability'
 
-source ${my_path}/shared.sh
+
+# shellcheck source=IntegrationTests/tests_01_cluster/shared.sh
+source "${my_path}"/shared.sh
 
 declare -r first_logs=/tmp/sact_first.out
 declare -r second_logs=/tmp/sact_second.out
 rm -f ${first_logs}
 rm -f ${second_logs}
 
 stdbuf -i0 -o0 -e0 swift run it_Clustered_swim_suspension_reachability 7337 > ${first_logs} 2>&1 &
-declare -r first_pid=$(echo $!)
+# shellcheck disable=SC2155
+declare -r first_pid=$($!)
 wait_log_exists ${first_logs} 'Binding to: ' 200 # since it might be compiling again...
 
 stdbuf -i0 -o0 -e0 swift run it_Clustered_swim_suspension_reachability 8228 127.0.0.1 7337 > ${second_logs} 2>&1 &
-declare -r second_pid=$(echo $!)
+# shellcheck disable=SC2155
+declare -r second_pid=$($!)
 wait_log_exists ${second_logs} 'Binding to: ' 200 # since it might be compiling again...
 
 echo "Waiting nodes to become .up..."
 wait_log_exists ${first_logs} 'Event: membershipChange(sact://System@127.0.0.1:8228 :: \[joining\] -> \[     up\])' 50
 echo 'Second member seen .up, good...'
 
 # suspend the second process, causing unreachability
-kill -SIGSTOP ${second_pid}  # ignore-unacceptable-language
+kill -SIGSTOP "${second_pid}"  # ignore-unacceptable-language
 jobs
 
 wait_log_exists ${first_logs} 'Event: reachabilityChange(DistributedCluster.Cluster.ReachabilityChange.*127.0.0.1:8228, status: up, reachability: unreachable' 50
 echo 'Second member seen .unreachable, good...'
 
 # resume it in the background
-kill -SIGCONT ${second_pid}  # ignore-unacceptable-language
+kill -SIGCONT "${second_pid}"  # ignore-unacceptable-language
 
 # it should become reachable again
-declare -r expected_second_member_unreachable=
 wait_log_exists ${first_logs} 'Event: reachabilityChange(DistributedCluster.Cluster.ReachabilityChange.*127.0.0.1:8228, status: up, reachability: reachable' 50
 echo 'Second member seen .unreachable, good...'
 
 
 # === cleanup ----------------------------------------------------------------------------------------------------------
 
-kill -9 ${first_pid}  # ignore-unacceptable-language
-kill -9 ${second_pid}  # ignore-unacceptable-language
+kill -9 "${first_pid}"  # ignore-unacceptable-language
+kill -9 "${second_pid}"  # ignore-unacceptable-language
 
 _killall ${app_name}  # ignore-unacceptable-language

IntegrationTests/tests_01_cluster/test_02_ungraceful_shutdown_recovery.sh

@@ -16,12 +16,14 @@
 set -e
 #set -x # verbose
 
+# shellcheck disable=SC2155
 declare -r my_path="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
-declare -r root_path="$my_path/.."
 
 declare -r app_name='it_Clustered_swim_ungraceful_shutdown'
 
-source ${my_path}/shared.sh
+
+# shellcheck source=IntegrationTests/tests_01_cluster/shared.sh
+source "${my_path}"/shared.sh
 
 declare -r first_logs=/tmp/sact_first.out
 declare -r second_logs=/tmp/sact_second.out
@@ -33,15 +35,18 @@ rm -f ${killed_logs}
 rm -f ${replacement_logs}
 
 stdbuf -i0 -o0 -e0 swift run it_Clustered_swim_ungraceful_shutdown Frist 7337 > ${first_logs} 2>&1 &
-declare -r first_pid=$(echo $!)
+# shellcheck disable=SC2155
+declare -r first_pid=$($!)
 wait_log_exists ${first_logs} 'Binding to: ' 200 # since it might be compiling again...
 
 stdbuf -i0 -o0 -e0 swift run it_Clustered_swim_ungraceful_shutdown Second 8228 127.0.0.1 7337 > ${second_logs} 2>&1 &
-declare -r second_pid=$(echo $!)
+# shellcheck disable=SC2155
+declare -r second_pid=$($!)
 wait_log_exists ${second_logs} 'Binding to: ' 200 # since it might be compiling again...
 
 stdbuf -i0 -o0 -e0 swift run it_Clustered_swim_ungraceful_shutdown Killed 9119 127.0.0.1 7337 > ${killed_logs} 2>&1 &  # ignore-unacceptable-language
-declare -r killed_pid=$(echo $!)  # ignore-unacceptable-language
+# shellcheck disable=SC2155
+declare -r killed_pid=$($!)  # ignore-unacceptable-language
 wait_log_exists ${killed_logs} 'Binding to: ' 200 # since it might be compiling again...
 
 echo "Waiting nodes to become .up..."
@@ -53,11 +58,12 @@ sleep 1
 
 # SIGKILL the third member, causing ungraceful shutdown
 echo "Killing PID ${killed_pid}"  # ignore-unacceptable-language
-kill -9 ${killed_pid}  # ignore-unacceptable-language
+kill -9 "${killed_pid}"  # ignore-unacceptable-language
 
 # Immediately restart the third process
 stdbuf -i0 -o0 -e0 swift run it_Clustered_swim_ungraceful_shutdown Replacement 9119 127.0.0.1 7337 >> ${replacement_logs} 2>&1 &
-declare -r replacement_pid=$(echo $!)
+# shellcheck disable=SC2155
+declare -r replacement_pid=$($!)
 wait_log_exists ${replacement_logs} 'Binding to: ' 200 # just to be safe...
 
 # The original third node should go .down while the replacement becomes .up
@@ -68,8 +74,8 @@ echo 'Replacement member .up, good...'
 
 # === cleanup ----------------------------------------------------------------------------------------------------------
 
-kill -9 ${first_pid}  # ignore-unacceptable-language
-kill -9 ${second_pid}  # ignore-unacceptable-language
-kill -9 ${replacement_pid}  # ignore-unacceptable-language
+kill -9 "${first_pid}"  # ignore-unacceptable-language
+kill -9 "${second_pid}"  # ignore-unacceptable-language
+kill -9 "${replacement_pid}"  # ignore-unacceptable-language
 
 _killall ${app_name}

scripts/dev/diff_update_license_header_year.sh

@@ -18,8 +18,8 @@ set -e
 # Diffs with main and updates any files that differ from it.
 # Useful to keep copyright years up to date.
 
-diff main --name-only | while IFS= read file; do
+diff main --name-only | while IFS= read -r file; do
   echo "Update copyright year in: ${file}"
-  sed -i .bak 's/2018 Apple Inc./2019 Apple Inc./g' ${file};
+  sed -i .bak 's/2018 Apple Inc./2019 Apple Inc./g' "${file}";
   rm "${file}.bak"
 done

scripts/dev/find_test_failures.sh

@@ -22,17 +22,17 @@ failures_count=0
 failures=()
 
 IFS=$'\n'
-fails=$(cat $logs | grep -n "' failed (")
+fails=$(grep -n "' failed (" < "$logs")
 for fail in $fails; do
     printf "\033[0;31m!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!\033[0m\n"
-    printf "\033[0;31m$fail\033[0m\n"
+    printf "\033[0;31m%s\033[0m\n" "$fail"
 
     failures+=( "$fail" )
-    test_name=$(echo $fail | awk '{ print $3 }')
-    logs_stop_line=$(echo $fail | awk '{ print $1 }' | awk 'BEGIN { FS = ":" } ; { print $1 }')
-    logs_start_line=$(cat $logs | grep -n "$test_name started at" | awk 'BEGIN { FS = ":" } { print $1 }')
+    test_name=$(echo "$fail" | awk '{ print $3 }')
+    logs_stop_line=$(echo "$fail" | awk '{ print $1 }' | awk 'BEGIN { FS = ":" } ; { print $1 }')
+    logs_start_line=$(grep -n "$test_name started at" < "$logs" | awk 'BEGIN { FS = ":" } { print $1 }')
 
-    tail -n +${logs_start_line} $logs | head -n $(expr $logs_stop_line - $logs_start_line)
+    tail -n +"${logs_start_line}" "$logs" | head -n "$(("$logs_stop_line" - "$logs_start_line"))"
     failures_count+=1
 done
 
@@ -41,11 +41,11 @@ if [[ "$failures_count" -ne 0 ]]; then
     printf "\033[0;31mFAILED TESTS: \033[0m\n"
 
     for failure in "${failures[@]}" ; do
-    printf "\033[0;31m  - $failure \033[0m\n"
+    printf "\033[0;31m  - %s \033[0m\n" "$failure"
 
     done
 
     printf "\033[0;31m!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!\033[0m\n"
 
-    exit -1
+    exit 255
 fi

scripts/dev/sact_backtrace_atos.sh

@@ -55,10 +55,10 @@ BIN=$1
 PID=$2
 # PID=$(ps aux | grep "SampleLetItCrash" | grep -v "grep" | awk '{print $2}')
 
-while read line
+while read -r line
 do
   address="$(echo "$line" | awk '{ print $3 }')"
-  source_pos=$(atos -p $PID -o $BIN -fullPath $address 2> /dev/null)
+  source_pos=$(atos -p "$PID" -o "$BIN" -fullPath "$address" 2> /dev/null)
 
   echo -n "$line" | awk 'BEGIN{} { printf "%s\t%s\t%s\t atos:", $1, $2, $3 }'
   echo -e "$CYAN$source_pos$NC"

scripts/dev/sact_backtrace_demangle.sh

@@ -41,7 +41,7 @@
 CYAN='\033[0;36m'
 NC='\033[0m' # No Color
 
-while read line
+while read -r line
 do
   mangled=$(echo "$line" | awk '{ print $4 }')
   de_mangled=$(echo "$mangled" | swift demangle)

scripts/dev/until_failure.sh

@@ -13,4 +13,4 @@
 ##
 ##===----------------------------------------------------------------------===##
 
-while $@; do :; done
+while "$@"; do :; done

scripts/dump_actors.sh

@@ -14,4 +14,5 @@
 ##===----------------------------------------------------------------------===##
 
 # The output of this is important for InspectKit so change it very carefully and adjust InspectKit whenever this script changes
+# shellcheck disable=SC2009
 ps aux | grep distributed-actors | grep xctest | head -n1 | awk '{print $2}' | xargs swift-inspect dump-concurrency | grep DistributedCluster | sort

scripts/generate_protos.sh

@@ -20,7 +20,7 @@ root_path="$my_path/.."
 
 proto_path="$root_path/Protos"
 
-pushd $proto_path >> /dev/null
+pushd "$proto_path" >> /dev/null
 
 declare -a public_protos
 public_protos=(
@@ -57,10 +57,10 @@ for visibility in public default; do
       out_name="${base_name%.*}.pb.swift"
       dest_dir="../Sources/DistributedCluster/${out_dir}/Protobuf"
       dest_file="${dest_dir}/${out_name}"
-      mkdir -p ${dest_dir}
+      mkdir -p "${dest_dir}"
       command="protoc --swift_out=. ${p} ${swift_opt}"
-      echo $command
-     `$command`
+      echo "$command"
+      eval "$command"
       mv "${out_dir}/${out_name}" "${dest_file}"
   done
 done
@@ -74,16 +74,16 @@ for internal_proto_path in "${internal_proto_paths[@]}"; do
   (
     pushd "$internal_proto_path" >> /dev/null
 
-    find . -name "*.proto" | while read p; do
+    find . -name "*.proto" | while read -r p; do
       out_dir=$( dirname "$p" )
       base_name=$( echo basename "$p" | sed "s/.*\///" )
       out_name="${base_name%.*}.pb.swift"
       dest_dir="../${out_dir}/Protobuf"
       dest_file="${dest_dir}/${out_name}"
-      mkdir -p ${dest_dir}
+      mkdir -p "${dest_dir}"
       command="protoc --swift_out=. ${p}"
-      echo $command
-      `$command`
+      echo "$command"
+      eval "$command"
       mv "${out_dir}/${out_name}" "${dest_file}"
     done
 

scripts/validate_format.sh

@@ -21,13 +21,13 @@ command -v swiftformat >/dev/null 2>&1 || { echo >&2 "'swiftformat' could not be
 printf "=> Checking format... \n"
 # format the code and exit with error if it was not formatted correctly
 MODIFIED_FILES=$(git diff --name-only main | grep swift)
-printf "Modiified files to check:\n${MODIFIED_FILES}"
+printf "Modiified files to check:\n%s" "${MODIFIED_FILES}"
 if [[ -z "$MODIFIED_FILES" ]]
 then
   echo '   * Skipping, no Swift files changes in the branch...'
   exit 0
 fi
 
 printf "\n==> "
-swiftformat  $MODIFIED_FILES > /dev/null || { echo >&2 "'swiftformat' invocation failed"; exit 1; }
+swiftformat  "$MODIFIED_FILES" > /dev/null || { echo >&2 "'swiftformat' invocation failed"; exit 1; }
 printf "\033[0;32mokay.\033[0m\n"