Skip to content

Commit

Permalink
Add lint to ensure substitutions use the full form
Browse files Browse the repository at this point in the history
Check that any variable substitutions use the full ${VAR} form,
not just $VAR (but don't check for quoting yet).
  • Loading branch information
aneeshusa committed Aug 5, 2016
1 parent 79ef9b4 commit 9231ca1
Show file tree
Hide file tree
Showing 10 changed files with 23 additions and 14 deletions.
2 changes: 1 addition & 1 deletion components/servo/fake-ld.sh
Expand Up @@ -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"
4 changes: 2 additions & 2 deletions etc/ci/lockfile_changed.sh
Expand Up @@ -9,5 +9,5 @@ set -o nounset
set -o pipefail

diff="$(git diff -- */*/Cargo.lock)"
echo "$diff"
[[ ! $diff ]]
echo "${diff}"
[[ -z "${diff}" ]]
4 changes: 2 additions & 2 deletions etc/ci/manifest_changed.sh
Expand Up @@ -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}" ]]
2 changes: 1 addition & 1 deletion etc/ci/upload_docs.sh
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions etc/ci/upload_nightly.sh
Expand Up @@ -27,7 +27,7 @@ upload() {


main() {
if [[ "$#" != 1 ]]; then
if [[ "${#}" != 1 ]]; then
usage >&2
return 1
fi
Expand Down Expand Up @@ -58,4 +58,4 @@ main() {
upload "${platform}" ${package} "${extension}"
}

main "$@"
main "${@}"
6 changes: 3 additions & 3 deletions ports/geckolib/gecko_bindings/tools/regen.sh
Expand Up @@ -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

Expand All @@ -32,4 +32,4 @@ else
LIBCLANG_PATH="$(brew --prefix llvm38)/lib/llvm-3.8/lib"
fi

./regen.py --target all "$@"
./regen.py --target all "${@}"
6 changes: 3 additions & 3 deletions ports/geckolib/gecko_bindings/tools/setup_bindgen.sh
Expand Up @@ -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
Expand All @@ -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
Expand Down
7 changes: 7 additions & 0 deletions python/tidy/servo_tidy/tidy.py
Expand Up @@ -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 \
Expand Down
1 change: 1 addition & 0 deletions python/tidy/servo_tidy_tests/shell_tidy.sh
Expand Up @@ -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"
1 change: 1 addition & 0 deletions python/tidy/servo_tidy_tests/test_tidy.py
Expand Up @@ -53,6 +53,7 @@ def test_shell(self):
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):
Expand Down

0 comments on commit 9231ca1

Please sign in to comment.