Skip to content

Commit

Permalink
Add lint for backticks in shell scripts
Browse files Browse the repository at this point in the history
The "$(some_command arg1 arg2)" form is preferred to the
`some_command arg1 arg2` form because it nests unambiguously.
Add a lint for this to tidy.
  • Loading branch information
aneeshusa committed Aug 5, 2016
1 parent 4bc629b commit 79ef9b4
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 14 deletions.
4 changes: 2 additions & 2 deletions ports/geckolib/gecko_bindings/tools/regen.sh
Expand Up @@ -27,9 +27,9 @@ if [ ! -d /usr/include ]; then
fi

if [ "$(uname)" == "Linux" ]; then
LIBCLANG_PATH=/usr/lib/llvm-3.8/lib;
LIBCLANG_PATH=/usr/lib/llvm-3.8/lib
else
LIBCLANG_PATH=`brew --prefix llvm38`/lib/llvm-3.8/lib;
LIBCLANG_PATH="$(brew --prefix llvm38)/lib/llvm-3.8/lib"
fi

./regen.py --target all "$@"
4 changes: 2 additions & 2 deletions ports/geckolib/gecko_bindings/tools/setup_bindgen.sh
Expand Up @@ -13,9 +13,9 @@ cd "$(dirname $0)"

# Setup and build bindgen.
if [ "$(uname)" == "Linux" ]; then
export LIBCLANG_PATH=/usr/lib/llvm-3.8/lib;
export LIBCLANG_PATH=/usr/lib/llvm-3.8/lib
else
export LIBCLANG_PATH=`brew --prefix llvm38`/lib/llvm-3.8/lib;
export LIBCLANG_PATH="$(brew --prefix llvm38)/lib/llvm-3.8/lib"
fi

# Make sure we have llvm38.
Expand Down
27 changes: 17 additions & 10 deletions python/tidy/servo_tidy/tidy.py
Expand Up @@ -318,27 +318,34 @@ def check_shell(file_name, lines):
shebang = "#!/usr/bin/env bash"
required_options = {"set -o errexit", "set -o nounset", "set -o pipefail"}

did_shebang_check = False

if len(lines) == 0:
yield (0, 'script is an empty file')
else:
if lines[0].rstrip() != shebang:
yield (1, 'script does not have shebang "{}"'.format(shebang))
return

if lines[0].rstrip() != shebang:
yield (1, 'script does not have shebang "{}"'.format(shebang))

for idx in range(1, len(lines)):
stripped = lines[idx].rstrip()
for idx in range(1, len(lines)):
stripped = lines[idx].rstrip()
# Comments or blank lines are ignored. (Trailing whitespace is caught with a separate linter.)
if lines[idx].startswith("#") or stripped == "":
continue

# Comments or blank lines are ignored. (Trailing whitespace is caught with a separate linter.)
if lines[idx].startswith("#") or stripped == "":
continue
elif stripped in required_options:
if not did_shebang_check:
if stripped in required_options:
required_options.remove(stripped)
else:
# The first non-comment, non-whitespace, non-option line is the first "real" line of the script.
# The shebang, options, etc. must come before this.
if len(required_options) != 0:
formatted = ['"{}"'.format(opt) for opt in required_options]
yield (idx + 1, "script is missing options {}".format(", ".join(formatted)))
break
did_shebang_check = True

if "`" in stripped:
yield (idx + 1, "script should not use backticks for command substitution")


def check_rust(file_name, lines):
Expand Down
2 changes: 2 additions & 0 deletions python/tidy/servo_tidy_tests/shell_tidy.sh
Expand Up @@ -4,4 +4,6 @@

set -o nounset

# Talking about some `concept in backticks` # shouldn't trigger
echo "hello world"
some_var=`echo "command substitution"`
1 change: 1 addition & 0 deletions python/tidy/servo_tidy_tests/test_tidy.py
Expand Up @@ -52,6 +52,7 @@ def test_shell(self):
errors = tidy.collect_errors_for_files(iterFile('shell_tidy.sh'), [], [tidy.check_shell], print_text=False)
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.assertNoMoreErrors(errors)

def test_rust(self):
Expand Down

0 comments on commit 79ef9b4

Please sign in to comment.