Enforce idiomatic error handling in bash (exercism#1736)

* Enforce idiomatic error handling in bash

Single commands that may error can stil be achieved with `|| true`.
With this change, we'll only see a limited number of clippy warnings
in CI. I think that's fine, the one fixing it can still remove these
flags locally while working on the fix to see all warnings.

* Fix CI

and indent bash scripts with 4 spaces

.github/workflows/tests.yml

@@ -60,6 +60,9 @@ jobs:
       - name: Checkout code
         uses: actions/checkout@3df4ab11eba7bda6032a0b82a6bb43b11571feac # v4.0.0
 
+      - name: Fetch origin/main
+        run: git fetch --depth=1 origin main
+
       - name: Setup toolchain
         uses: dtolnay/rust-toolchain@0e66bd3e6b38ec0ad5312288c83e47c143e6b09e
         with:
@@ -130,6 +133,7 @@ jobs:
         uses: dtolnay/rust-toolchain@0e66bd3e6b38ec0ad5312288c83e47c143e6b09e
         with:
           toolchain: ${{ matrix.rust }}
+          components: clippy
 
       - name: Clippy tests
         env:

bin/check_exercises.sh

@@ -1,11 +1,5 @@
 #!/usr/bin/env bash
-
-# In DENYWARNINGS or CLIPPY mode, do not set -e so that we run all tests.
-# This allows us to see all warnings.
-# If we are in neither DENYWARNINGS nor CLIPPY mode, do set -e.
-if [ -z "$DENYWARNINGS" ] && [ -z "$CLIPPY" ]; then
-    set -e
-fi
+set -eo pipefail
 
 # can't benchmark with a stable compiler; to bench, use
 # $ BENCHMARK=1 rustup run nightly bin/check_exercises.sh
@@ -18,13 +12,14 @@ fi
 repo=$(git rev-parse --show-toplevel)
 
 if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then
-   files="$(
-      git diff --diff-filter=d --name-only remotes/origin/main |
-      grep "exercises/" |
-      cut -d '/' -f -3 |
-      sort -u |
-      awk -v repo="$repo" '{print repo"/"$1}'
-   )"
+    git fetch --depth=1 origin main
+    files="$(
+        git diff --diff-filter=d --name-only remotes/origin/main |
+        grep "exercises/" || true |
+        cut -d '/' -f -3 |
+        sort -u |
+        awk -v repo="$repo" '{print repo"/"$1}'
+    )"
 else
     files="$repo/exercises/*/*"
 fi
@@ -33,39 +28,39 @@ return_code=0
 # An exercise worth testing is defined here as any top level directory with
 # a 'tests' directory and a .meta/config.json.
 for exercise in $files; do
-   exercise="$exercise/$target_dir"
+    exercise="$exercise/$target_dir"
 
-   # This assumes that exercises are only one directory deep
-   # and that the primary module is named the same as the directory
-   directory=$(dirname "${exercise}")
+    # This assumes that exercises are only one directory deep
+    # and that the primary module is named the same as the directory
+    directory=$(dirname "${exercise}")
 
-   # An exercise must have a .meta/config.json
-   metaconf="$directory/.meta/config.json"
-   if [ ! -f "$metaconf" ]; then
-      continue
-   fi
+    # An exercise must have a .meta/config.json
+    metaconf="$directory/.meta/config.json"
+    if [ ! -f "$metaconf" ]; then
+        continue
+    fi
 
-   release=""
-   if [ -z "$BENCHMARK" ] && jq --exit-status '.custom?."test-in-release-mode"?' "$metaconf"; then
-      # release mode is enabled if not benchmarking and the appropriate key is neither `false` nor `null`.
-      release="--release"
-   fi
+    release=""
+    if [ -z "$BENCHMARK" ] && jq --exit-status '.custom?."test-in-release-mode"?' "$metaconf"; then
+        # release mode is enabled if not benchmarking and the appropriate key is neither `false` nor `null`.
+        release="--release"
+    fi
 
-   if [ -n "$DENYWARNINGS" ]; then
-      # Output a progress dot, because we may not otherwise produce output in > 10 mins.
-      echo -n '.'
-      # No-run mode so we see no test output.
-      # Quiet mode so we see no compile output
-      # (such as "Compiling"/"Downloading").
-      # Compiler errors will still be shown though.
-      # Both flags are necessary to keep things quiet.
-      ./bin/test_exercise.sh "$directory" --quiet --no-run
-      return_code=$((return_code | $?))
-   else
-      # Run the test and get the status
-      ./bin/test_exercise.sh "$directory" $release
-      return_code=$((return_code | $?))
-   fi
+    if [ -n "$DENYWARNINGS" ]; then
+        # Output a progress dot, because we may not otherwise produce output in > 10 mins.
+        echo -n '.'
+        # No-run mode so we see no test output.
+        # Quiet mode so we see no compile output
+        # (such as "Compiling"/"Downloading").
+        # Compiler errors will still be shown though.
+        # Both flags are necessary to keep things quiet.
+        ./bin/test_exercise.sh "$directory" --quiet --no-run
+        return_code=$((return_code | $?))
+    else
+        # Run the test and get the status
+        ./bin/test_exercise.sh "$directory" $release
+        return_code=$((return_code | $?))
+    fi
 done
 
 exit $return_code

bin/ensure_stubs_compile.sh

@@ -1,15 +1,17 @@
 #!/usr/bin/env bash
+set -eo pipefail
 
 repo="$(git rev-parse --show-toplevel)"
 
 if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then
-  changed_exercises="$(
-    git diff --diff-filter=d --name-only remotes/origin/main |
-    grep "exercises/" |
-    cut -d '/' -f -3 |
-    sort -u |
-    awk -v repo="$repo" '{print repo"/"$1}'
-  )"
+    git fetch --depth=1 origin main
+    changed_exercises="$(
+        git diff --diff-filter=d --name-only remotes/origin/main |
+        grep "exercises/" || true |
+        cut -d '/' -f -3 |
+        sort -u |
+        awk -v repo="$repo" '{print repo"/"$1}'
+    )"
 else
     # we want globbing and it does not actually assign locally
     # shellcheck disable=SC2125
@@ -31,47 +33,47 @@ for dir in $changed_exercises; do
 
     # An exercise must have a .meta/config.json
     if [ ! -f "$config_file" ]; then
-       continue
+        continue
     fi
 
     if jq --exit-status '.custom?."allowed-to-not-compile"?' "$config_file"; then
-      echo "$exercise's stub is allowed to not compile"
-      continue
+        echo "$exercise's stub is allowed to not compile"
+        continue
     fi
 
     # Backup tests and stub; this script will modify them.
     cp -r "$dir/tests" "$dir/tests.orig"
     cp "$dir/src/lib.rs" "$dir/lib.rs.orig"
 
-  # In CI, we may have already compiled using the example solution.
-  # So we want to touch the src/lib.rs in some way,
-  # so that we surely recompile using the stub.
-  # Since we also want to deny warnings, that will also serve as the touch.
-  sed -i -e '1i #![deny(warnings)]' "$dir/src/lib.rs"
+    # In CI, we may have already compiled using the example solution.
+    # So we want to touch the src/lib.rs in some way,
+    # so that we surely recompile using the stub.
+    # Since we also want to deny warnings, that will also serve as the touch.
+    sed -i -e '1i #![deny(warnings)]' "$dir/src/lib.rs"
 
     # Deny warnings in the tests that may result from compiling the stubs.
     # This helps avoid, for example, an overflowing literal warning
     # that could be caused by a stub with a type that is too small.
     sed -i -e '1i #![deny(warnings)]' "$dir"/tests/*.rs
 
-  if [ -n "$CLIPPY" ]; then
-    echo "clippy $exercise..."
-    # We don't find it useful in general to have students implement Default,
-    # since we're generally not going to test it.
-    if ! (
-      cd "$dir" &&
-      cargo clippy --lib --tests --color always -- --allow clippy::new_without_default 2>clippy.log
-    ); then
-      cat "$dir/clippy.log"
-      broken="$broken\n$exercise"
-    else
-      # Just to show progress
-      echo "... OK"
+    if [ -n "$CLIPPY" ]; then
+        echo "clippy $exercise..."
+        # We don't find it useful in general to have students implement Default,
+        # since we're generally not going to test it.
+        if ! (
+            cd "$dir" &&
+            cargo clippy --lib --tests --color always -- --allow clippy::new_without_default 2>clippy.log
+        ); then
+            cat "$dir/clippy.log"
+            broken="$broken\n$exercise"
+        else
+            # Just to show progress
+            echo "... OK"
+        fi
+    elif ! (cd "$dir" && cargo test --quiet --no-run); then
+        echo "$exercise's stub does not compile; please make it compile"
+        broken="$broken\n$exercise"
     fi
-  elif ! (cd "$dir" && cargo test --quiet --no-run); then
-    echo "$exercise's stub does not compile; please make it compile"
-    broken="$broken\n$exercise"
-  fi
 
     # Restore tests and stub.
     mv "$dir/lib.rs.orig" "$dir/src/lib.rs"
@@ -80,6 +82,6 @@ for dir in $changed_exercises; do
 done
 
 if [ -n "$broken" ]; then
-  echo "Exercises that don't compile:$broken"
-  exit 1
+    echo "Exercises that don't compile:$broken"
+    exit 1
 fi

bin/format_exercises.sh

@@ -1,6 +1,7 @@
 #!/usr/bin/env bash
+set -eo pipefail
+
 # Format existing exercises using rustfmt
-set -e
 
 repo=$(git rev-parse --show-toplevel)