diff options
Diffstat (limited to 'python')
| -rw-r--r-- | python/tidy/servo_tidy/tidy.py | 37 | ||||
| -rw-r--r-- | python/tidy/servo_tidy_tests/shell_tidy.sh | 7 | ||||
| -rw-r--r-- | python/tidy/servo_tidy_tests/test_tidy.py | 4 |
3 files changed, 38 insertions, 10 deletions
diff --git a/python/tidy/servo_tidy/tidy.py b/python/tidy/servo_tidy/tidy.py index 34eea7f882a..3433e8265ed 100644 --- a/python/tidy/servo_tidy/tidy.py +++ b/python/tidy/servo_tidy/tidy.py @@ -318,19 +318,23 @@ 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 - for idx in range(1, len(lines)): - stripped = lines[idx].rstrip() + 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() + # 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. @@ -338,7 +342,20 @@ def check_shell(file_name, lines): 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") + + if " [ " in stripped or stripped.startswith("[ "): + yield (idx + 1, "script should use `[[` instead of `[` for conditional testing") + + 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): diff --git a/python/tidy/servo_tidy_tests/shell_tidy.sh b/python/tidy/servo_tidy_tests/shell_tidy.sh index fbf0e784755..e38358fc3b6 100644 --- a/python/tidy/servo_tidy_tests/shell_tidy.sh +++ b/python/tidy/servo_tidy_tests/shell_tidy.sh @@ -4,4 +4,11 @@ set -o nounset +# Talking about some `concept in backticks` # shouldn't trigger echo "hello world" +some_var=`echo "command substitution"` +another_var="$some_var" +if [ -z "${some_var}" ]; then + echo "should have used [[" +fi +[ -z "${another_var}" ] diff --git a/python/tidy/servo_tidy_tests/test_tidy.py b/python/tidy/servo_tidy_tests/test_tidy.py index b08e6057541..92b304a5c1a 100644 --- a/python/tidy/servo_tidy_tests/test_tidy.py +++ b/python/tidy/servo_tidy_tests/test_tidy.py @@ -52,6 +52,10 @@ class CheckTidiness(unittest.TestCase): 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.assertEqual('variable substitutions should use the full \"${VAR}\" form', errors.next()[2]) + self.assertEqual('script should use `[[` instead of `[` for conditional testing', errors.next()[2]) + self.assertEqual('script should use `[[` instead of `[` for conditional testing', errors.next()[2]) self.assertNoMoreErrors(errors) def test_rust(self): |