From 9231ca1c6991c2eaf0d7e08674b2fc9c84a31695 Mon Sep 17 00:00:00 2001 From: Aneesh Agrawal Date: Thu, 4 Aug 2016 18:12:54 -0400 Subject: [PATCH] Add lint to ensure substitutions use the full form Check that any variable substitutions use the full ${VAR} form, not just $VAR (but don't check for quoting yet). --- components/servo/fake-ld.sh | 2 +- etc/ci/lockfile_changed.sh | 4 ++-- etc/ci/manifest_changed.sh | 4 ++-- etc/ci/upload_docs.sh | 2 +- etc/ci/upload_nightly.sh | 4 ++-- ports/geckolib/gecko_bindings/tools/regen.sh | 6 +++--- ports/geckolib/gecko_bindings/tools/setup_bindgen.sh | 6 +++--- python/tidy/servo_tidy/tidy.py | 7 +++++++ python/tidy/servo_tidy_tests/shell_tidy.sh | 1 + python/tidy/servo_tidy_tests/test_tidy.py | 1 + 10 files changed, 23 insertions(+), 14 deletions(-) diff --git a/components/servo/fake-ld.sh b/components/servo/fake-ld.sh index f14e94ea8c9..d0dd042dbf4 100755 --- a/components/servo/fake-ld.sh +++ b/components/servo/fake-ld.sh @@ -9,7 +9,7 @@ set -o nounset set -o pipefail TARGET_DIR="${OUT_DIR}/../../.." -arm-linux-androideabi-gcc "$@" \ +arm-linux-androideabi-gcc "${@}" \ "${LDFLAGS-}" -lc -shared \ -o "${TARGET_DIR}/libservo.so" touch "${TARGET_DIR}/servo" diff --git a/etc/ci/lockfile_changed.sh b/etc/ci/lockfile_changed.sh index 1d11acee473..6b3191291cc 100755 --- a/etc/ci/lockfile_changed.sh +++ b/etc/ci/lockfile_changed.sh @@ -9,5 +9,5 @@ set -o nounset set -o pipefail diff="$(git diff -- */*/Cargo.lock)" -echo "$diff" -[[ ! $diff ]] +echo "${diff}" +[[ -z "${diff}" ]] diff --git a/etc/ci/manifest_changed.sh b/etc/ci/manifest_changed.sh index 195e4b9ef7e..24fc9baec6c 100755 --- a/etc/ci/manifest_changed.sh +++ b/etc/ci/manifest_changed.sh @@ -17,5 +17,5 @@ set -o pipefail ./mach test-wpt --manifest-update --binary= SKIP_TESTS > /dev/null diff="$(git diff -- tests/*/MANIFEST.json)" -echo "$diff" -[[ ! $diff ]] +echo "${diff}" +[[ -z "${diff}" ]] diff --git a/etc/ci/upload_docs.sh b/etc/ci/upload_docs.sh index fc8d7416f65..f55e5cb73db 100755 --- a/etc/ci/upload_docs.sh +++ b/etc/ci/upload_docs.sh @@ -12,7 +12,7 @@ set -o errexit set -o nounset set -o pipefail -cd "$(dirname $0)/../.." +cd "$(dirname ${0})/../.." ./mach doc # etc/doc.servo.org/index.html overwrites $(mach rust-root)/doc/index.html diff --git a/etc/ci/upload_nightly.sh b/etc/ci/upload_nightly.sh index 4f20ab658d4..bd6da3e6625 100755 --- a/etc/ci/upload_nightly.sh +++ b/etc/ci/upload_nightly.sh @@ -27,7 +27,7 @@ upload() { main() { - if [[ "$#" != 1 ]]; then + if [[ "${#}" != 1 ]]; then usage >&2 return 1 fi @@ -58,4 +58,4 @@ main() { upload "${platform}" ${package} "${extension}" } -main "$@" +main "${@}" diff --git a/ports/geckolib/gecko_bindings/tools/regen.sh b/ports/geckolib/gecko_bindings/tools/regen.sh index 21b998141c9..7bbd908c1f3 100755 --- a/ports/geckolib/gecko_bindings/tools/regen.sh +++ b/ports/geckolib/gecko_bindings/tools/regen.sh @@ -8,8 +8,8 @@ set -o errexit set -o nounset set -o pipefail -if [ $# -eq 0 ]; then - echo "Usage: $0 /path/to/gecko/objdir [other-regen.py-flags]" +if [ ${#} -eq 0 ]; then + echo "Usage: ${0} /path/to/gecko/objdir [other-regen.py-flags]" exit 1 fi @@ -32,4 +32,4 @@ else LIBCLANG_PATH="$(brew --prefix llvm38)/lib/llvm-3.8/lib" fi -./regen.py --target all "$@" +./regen.py --target all "${@}" diff --git a/ports/geckolib/gecko_bindings/tools/setup_bindgen.sh b/ports/geckolib/gecko_bindings/tools/setup_bindgen.sh index 31a81111b6d..2c6c8831697 100755 --- a/ports/geckolib/gecko_bindings/tools/setup_bindgen.sh +++ b/ports/geckolib/gecko_bindings/tools/setup_bindgen.sh @@ -9,7 +9,7 @@ set -o nounset set -o pipefail # Run in the tools directory. -cd "$(dirname $0)" +cd "$(dirname ${0})" # Setup and build bindgen. if [ "$(uname)" == "Linux" ]; then @@ -25,8 +25,8 @@ if [ ! -x "$(command -v clang-3.8)" ]; then exit 1 fi -export LD_LIBRARY_PATH=$LIBCLANG_PATH -export DYLD_LIBRARY_PATH=$LIBCLANG_PATH +export LD_LIBRARY_PATH="${LIBCLANG_PATH}" +export DYLD_LIBRARY_PATH="${LIBCLANG_PATH}" # Check for multirust if [ ! -x "$(command -v multirust)" ]; then diff --git a/python/tidy/servo_tidy/tidy.py b/python/tidy/servo_tidy/tidy.py index d47aa0137a0..50fd7ca8a5c 100644 --- a/python/tidy/servo_tidy/tidy.py +++ b/python/tidy/servo_tidy/tidy.py @@ -347,6 +347,13 @@ def check_shell(file_name, lines): if "`" in stripped: yield (idx + 1, "script should not use backticks for command substitution") + for dollar in re.finditer('\$', stripped): + next_idx = dollar.end() + if next_idx < len(stripped): + next_char = stripped[next_idx] + if not (next_char == '{' or next_char == '('): + yield(idx + 1, "variable substitutions should use the full \"${VAR}\" form") + def check_rust(file_name, lines): if not file_name.endswith(".rs") or \ diff --git a/python/tidy/servo_tidy_tests/shell_tidy.sh b/python/tidy/servo_tidy_tests/shell_tidy.sh index ff8e5fc7240..e24b361fe16 100644 --- a/python/tidy/servo_tidy_tests/shell_tidy.sh +++ b/python/tidy/servo_tidy_tests/shell_tidy.sh @@ -7,3 +7,4 @@ set -o nounset # Talking about some `concept in backticks` # shouldn't trigger echo "hello world" some_var=`echo "command substitution"` +another_var="$some_var" diff --git a/python/tidy/servo_tidy_tests/test_tidy.py b/python/tidy/servo_tidy_tests/test_tidy.py index 58e8f6e5fd3..2eee04a19a3 100644 --- a/python/tidy/servo_tidy_tests/test_tidy.py +++ b/python/tidy/servo_tidy_tests/test_tidy.py @@ -53,6 +53,7 @@ class CheckTidiness(unittest.TestCase): self.assertEqual('script does not have shebang "#!/usr/bin/env bash"', errors.next()[2]) self.assertEqual('script is missing options "set -o errexit", "set -o pipefail"', errors.next()[2]) self.assertEqual('script should not use backticks for command substitution', errors.next()[2]) + self.assertEqual('variable substitutions should use the full \"${VAR}\" form', errors.next()[2]) self.assertNoMoreErrors(errors) def test_rust(self):