Improve and simplify presubmit.sh

presubmit.sh was using a glob to find shell files, but it is no
longer good enough to find all the files. Use find instead and put
the list into an array.

Both shellcheck and grep (in readonlyvars_tests) can be invoked with
a list of files. Remove the loops and pass the expanded file array
directly. Additionally, both shellcheck and grep say nothing if they
don't find any problem, so there is no need to redirect the output
to a file. These two points make both tests much simpler.

readonlyvars_tests now outputs errors in a different way, which is
simply the output of grep. I think it is actually clearer, as you
get both the line number and colouring.

Change-Id: I79b2694a6eee91b02b52d2426726415dc2cd65a8

1 files changed, 12 insertions, 35 deletions

diff --git a/presubmit.sh b/presubmit.sh
index e1a4b84..0d356f1 100755
--- a/presubmit.sh
+++ b/presubmit.sh
@@ -11,11 +11,17 @@ readonly shellcheck_version=" 0.3.3"
 # SC2102 - https://github.com/koalaman/shellcheck/issues/682
 readonly shellcheck_exclude_list="SC2102"
 
+declare -a shell_files
+
+find_shell_files() {
+  # In case some filenames contain spaces
+  local IFS=$'\n'
+  shell_files=($(find "${local_path}" -name '*.sh'))
+  readonly shell_files
+}
+
 # Run shellcheck on all the .sh files.
 shellcheck_tests() {
-  local shellcheck_failure=false
-  local -r shellcheck_log=$(mktemp)
-
   if ! exists shellcheck; then
     log E "No \"shellcheck\" available in your path. apt-get install shellcheck ?"
     log E "You can also find shellcheck here: https://github.com/koalaman/shellcheck ."
@@ -29,50 +35,21 @@ shellcheck_tests() {
     log W "Things might fail!"
   fi
 
-  for file in ${local_path}/**/*.sh; do
-    shellcheck -e "${shellcheck_exclude_list}" "${file}" >> "${shellcheck_log}"
-    if [[ $? -ne 0 ]]; then
-      shellcheck_failure=true
-    fi
-  done
 
-  if [[ ${shellcheck_failure} == true ]]; then
-    cat "${shellcheck_log}"
-    rm "${shellcheck_log}"
-    return 1
-  else
-    rm "${shellcheck_log}"
-    return 0
-  fi
+  shellcheck -e "${shellcheck_exclude_list}" "${shell_files[@]}"
 }
 
 # Check for invalid constructions regarding read-only variables and the local
 # scope.
 readonlyvars_tests() {
-  local readonlyvars_failure=false
-  local -r readonlyvars_log=$(mktemp)
   local -r str_local="local"
   local -r str_ronly="readonly"
 
-  for file in ${local_path}/**/*.sh; do
-    grep -s -e "${str_local} ${str_ronly} " -e "${str_ronly} ${str_local} " "${file}" >> "${readonlyvars_log}"
-    if [[ $? -eq 0 ]]; then
-      readonlyvars_failure=true
-      echo "^--- ${file}" >> "${readonlyvars_log}"
-    fi
-  done
-
-  if [[ ${readonlyvars_failure} == true ]]; then
-    cat "${readonlyvars_log}"
-    rm "${readonlyvars_log}"
-    return 1
-  else
-    rm "${readonlyvars_log}"
-    return 0
-  fi
+  ! grep -Hn --color -E "\<(${str_local} ${str_ronly}|${str_ronly} ${str_local})\>" "${shell_files[@]}"
 }
 
 main() {
+  find_shell_files
   shellcheck_tests
   if_error "shellcheck presubmit test FAILED."
   readonlyvars_tests