Fix shellcheck issues (#1980)

#1971 introduced a `shellcheck`
lint. However, it also introduced some bugs due to adding quotes in a
few places where word splitting was used.

Thanks to gleasonk@ for spotting this:
#1976

As described in https://www.shellcheck.net/wiki/SC2086, quoting is
generally preferable when expanding a variable. However, when building a
command line it is better to create an array and then expand the array.
When using a regular variable, quoting can actually lead to incorrect
behavior: for example, given `x="file1 file2"`, `clang-format $x` will
correctly call `clang-format` on `file1` and `file2`, however
`clang-format "$x"` will call `clang-format` on a non-existent file
called `file1 file2`.

The lint itself was actually broken because of using `-printf '%P\0'`
with `find`. This produces null-delimited output, which `mapfile` does
not expect and does not parse correctly (only the first file name will
be extracted). (While implementing the check I thought I needed to use
`printf` to remove the the `./` prefix introduced by `find`, but turns
out that's not the case).

While investigating this change I discovered that the whitespace lint
check is also subtly broken. Indeed in `get_source_files` `xargs` is
used at the end of the pipeline, which puts everything on one line,
which causes subsequent uses of `xargs -L1` to behave incorrectly (the
underlying command is executed only once with all files as arguments).

build_tools/github_actions/lint_clang_format.sh

@@ -36,16 +36,16 @@ if [[ $# -ne 0 ]] ; then
 fi
 
 echo "Gathering changed files..."
-CHANGED_FILES=$(git diff "$BASE_BRANCH" HEAD --name-only --diff-filter=d | grep '.*\.h\|.*\.cpp' | xargs)
-if [[ -z "$CHANGED_FILES" ]]; then
+mapfile -t CHANGED_FILES < <(git diff "$BASE_BRANCH" HEAD --name-only --diff-filter=d | grep '.*\.h\|.*\.cpp')
+if (( ${#CHANGED_FILES[@]} == 0 )); then
   echo "No files to format."
   exit 0
 fi
 
 echo "Running clang-format [mode=$FORMAT_MODE]..."
-echo "  Files: $CHANGED_FILES"
+echo "  Files: " "${CHANGED_FILES[@]}"
 if [[ $FORMAT_MODE == 'fix' ]]; then
-  clang-format --style=google -i $CHANGED_FILES
+  clang-format --style=google -i "${CHANGED_FILES[@]}"
 else
-  clang-format --style=google --dry-run --Werror $CHANGED_FILES
+  clang-format --style=google --dry-run --Werror "${CHANGED_FILES[@]}"
 fi

build_tools/github_actions/lint_shellcheck.sh

@@ -32,7 +32,7 @@ readonly STABLEHLO_ROOT_DIR="${SCRIPT_DIR}/../.."
 cd "$STABLEHLO_ROOT_DIR"
 
 IFS=$'\n'
-mapfile -t targets < <(find . -type f -name '*.sh' -not -path './llvm*' -printf '%P\0')
+mapfile -t targets < <(find . -type f -name '*.sh' -not -path './llvm*')
 
 echo "Running shellcheck:"
 shellcheck --version

build_tools/github_actions/lint_whitespace_checks.sh

@@ -35,43 +35,43 @@ if [[ $# -ne 0 ]] ; then
 fi
 
 get_source_files() {
-  git diff "$BASE_BRANCH" HEAD --name-only --diff-filter=d | grep '.*\.cpp$\|.*\.h$\|.*\.md$\|.*\.mlir$\|.*\.sh$\|.*\.td$\|.*\.txt$\|.*\.yml$\|.*\.yaml$' | xargs
+  git diff "$BASE_BRANCH" HEAD --name-only --diff-filter=d | grep '.*\.cpp$\|.*\.h$\|.*\.md$\|.*\.mlir$\|.*\.sh$\|.*\.td$\|.*\.txt$\|.*\.yml$\|.*\.yaml$'
 }
 echo "Checking whitespace:"
-echo "  $(get_source_files)"
+echo "  $(get_source_files | xargs)"
 
 files_without_eof_newline() {
   # shellcheck disable=SC2016
-  [[ -z $(get_source_files) ]] || get_source_files | xargs -L1 bash -c 'test "$(tail -c 1 "$0")" && echo "$0"'
+  get_source_files | xargs -L1 bash -c 'test "$(tail -c 1 "$0")" && echo "$0"'
 }
 
 files_with_trailing_whitespace() {
-  get_source_files | xargs grep -lP '[ \t]+$'
+  get_source_files | xargs -L1 grep -lP '[ \t]+$'
 }
 
 fix_files_without_eof_newline() {
   # shellcheck disable=SC2016,SC1003
-  echo "$1" | xargs --no-run-if-empty sed -i -e '$a\'
+  echo "$@" | xargs --no-run-if-empty sed -i -e '$a\'
 }
 
 fix_files_with_trailing_whitespace() {
-  echo "$1" | xargs --no-run-if-empty sed -i 's/[ \t]*$//'
+  echo "$@" | xargs --no-run-if-empty sed -i 's/[ \t]*$//'
 }
 
-EOF_NL=$(files_without_eof_newline)
-TRAIL_WS=$(files_with_trailing_whitespace)
+mapfile -t EOF_NL < <(files_without_eof_newline)
+mapfile -t TRAIL_WS < <(files_with_trailing_whitespace)
 
 if [[ $FORMAT_MODE == 'fix' ]]; then
   echo "Fixing EOF newlines..."
-  fix_files_without_eof_newline "$EOF_NL"
+  fix_files_without_eof_newline "${EOF_NL[@]}"
   echo "Fixing trailing whitespaces..."
-  fix_files_with_trailing_whitespace "$TRAIL_WS"
+  fix_files_with_trailing_whitespace "${TRAIL_WS[@]}"
 else
-  if [ -n "$EOF_NL$TRAIL_WS" ]; then
+  if (( ${#EOF_NL[@]} + ${#TRAIL_WS[@]} )); then
     echo "Missing newline at EOF:"
-    echo "$EOF_NL"
+    echo "${EOF_NL[@]}"
     echo "Has trailing whitespace:"
-    echo "$TRAIL_WS"
+    echo "${TRAIL_WS[@]}"
     echo
     echo "Auto-fix using:"
     echo "  $ ./build_tools/github_actions/lint_whitespace_checks.sh -f"