From 7952bd00b687c3c1f5458f1750267e2bc1469ddd Mon Sep 17 00:00:00 2001 From: Jim Berlage Date: Tue, 5 Jul 2016 12:15:40 -0500 Subject: [PATCH] Add linting for shell scripts This changes tidy to check shell scripts for the proper shebang and options. It does not check that variables are formatted correctly. It also adds a check for the MPL 2.0 license in shell scripts. --- components/servo/fake-ld.sh | 18 ++++++++-- etc/ci/check_no_unwrap.sh | 9 +++-- etc/ci/lockfile_changed.sh | 4 +++ etc/ci/manifest_changed.sh | 8 +++-- etc/ci/upload_docs.sh | 9 +++-- etc/ci/upload_nightly.sh | 6 +++- python/tidy/servo_tidy/licenseck.py | 8 +++++ python/tidy/servo_tidy/tidy.py | 42 ++++++++++++++++++++-- python/tidy/servo_tidy_tests/shell_tidy.sh | 7 ++++ python/tidy/servo_tidy_tests/test_tidy.py | 6 ++++ 10 files changed, 104 insertions(+), 13 deletions(-) create mode 100644 python/tidy/servo_tidy_tests/shell_tidy.sh diff --git a/components/servo/fake-ld.sh b/components/servo/fake-ld.sh index 2babdbd80c18..f14e94ea8c9f 100755 --- a/components/servo/fake-ld.sh +++ b/components/servo/fake-ld.sh @@ -1,3 +1,15 @@ -#!/bin/bash -TARGET_DIR=$OUT_DIR/../../.. -arm-linux-androideabi-gcc $@ $LDFLAGS -lc -o $TARGET_DIR/libservo.so -shared && touch $TARGET_DIR/servo +#!/usr/bin/env bash + +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + +set -o errexit +set -o nounset +set -o pipefail + +TARGET_DIR="${OUT_DIR}/../../.." +arm-linux-androideabi-gcc "$@" \ + "${LDFLAGS-}" -lc -shared \ + -o "${TARGET_DIR}/libservo.so" +touch "${TARGET_DIR}/servo" diff --git a/etc/ci/check_no_unwrap.sh b/etc/ci/check_no_unwrap.sh index f19f18d7a10c..b6cf5dc837fa 100755 --- a/etc/ci/check_no_unwrap.sh +++ b/etc/ci/check_no_unwrap.sh @@ -1,12 +1,17 @@ #!/usr/bin/env bash -# + +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + # Make sure listed files do not contain "unwrap" set -o errexit set -o nounset set -o pipefail -cd "$(git rev-parse --show-toplevel)" # cd into repo root so make sure paths works in any case +# cd into repo root to make sure paths work in any case +cd "$(git rev-parse --show-toplevel)" # files that should not contain "unwrap" FILES=("components/compositing/compositor.rs" diff --git a/etc/ci/lockfile_changed.sh b/etc/ci/lockfile_changed.sh index e89e044f3753..1d11acee4730 100755 --- a/etc/ci/lockfile_changed.sh +++ b/etc/ci/lockfile_changed.sh @@ -1,5 +1,9 @@ #!/usr/bin/env bash +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + set -o errexit set -o nounset set -o pipefail diff --git a/etc/ci/manifest_changed.sh b/etc/ci/manifest_changed.sh index 31db44740999..195e4b9ef7ec 100755 --- a/etc/ci/manifest_changed.sh +++ b/etc/ci/manifest_changed.sh @@ -1,5 +1,9 @@ #!/usr/bin/env bash +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + set -o errexit set -o nounset set -o pipefail @@ -7,8 +11,8 @@ set -o pipefail # We shouldn't need any binary at all to update the manifests. # Adding "SKIP_TESTS" to skip tests, it doesn't really skip the tests. # It will run "run_wpt" with "'test_list': ['SKIP_TESTS']", -# and then pass it into wptrunner, which won't be able to find any tests named "SKIP_TESTS", -# and thus won't run any. +# and then pass it into wptrunner, which won't be able to find any tests named +# "SKIP_TESTS", and thus won't run any. # Adding "--binary=" to skip looking for a compiled servo binary. ./mach test-wpt --manifest-update --binary= SKIP_TESTS > /dev/null diff --git a/etc/ci/upload_docs.sh b/etc/ci/upload_docs.sh index c5aed30699d7..fc8d7416f656 100755 --- a/etc/ci/upload_docs.sh +++ b/etc/ci/upload_docs.sh @@ -1,5 +1,9 @@ #!/usr/bin/env bash -# + +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + # Helper script to upload docs to doc.servo.org. # Requires ghp-import (from pip) # GitHub API token must be passed in environment var TOKEN @@ -16,7 +20,8 @@ cp etc/doc.servo.org/* target/doc/ python components/style/properties/build.py servo html -OUT_DIR="`pwd`/target/doc/servo" make -f makefile.cargo -C components/script dom_docs +OUT_DIR="$(pwd)/target/doc/servo" \ + make -f makefile.cargo -C components/script dom_docs rm -rf target/doc/servo/.cache ghp-import -n target/doc diff --git a/etc/ci/upload_nightly.sh b/etc/ci/upload_nightly.sh index e3033cc0902e..ca3d275d96dd 100755 --- a/etc/ci/upload_nightly.sh +++ b/etc/ci/upload_nightly.sh @@ -1,5 +1,9 @@ #!/usr/bin/env bash +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + set -o errexit set -o nounset set -o pipefail @@ -12,7 +16,7 @@ usage() { upload() { - local package_filename + local package_filename package_filename="$(basename "${2}")" local -r nightly_upload_dir="s3://servo-builds/nightly/${1}" local -r package_upload_path="${nightly_upload_dir}/${package_filename}" diff --git a/python/tidy/servo_tidy/licenseck.py b/python/tidy/servo_tidy/licenseck.py index 03113da507dc..dce01460d246 100644 --- a/python/tidy/servo_tidy/licenseck.py +++ b/python/tidy/servo_tidy/licenseck.py @@ -23,6 +23,14 @@ # file, You can obtain one at http://mozilla.org/MPL/2.0/. """, +"""\ +#!/usr/bin/env bash + +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +""", + """\ #!/usr/bin/env python diff --git a/python/tidy/servo_tidy/tidy.py b/python/tidy/servo_tidy/tidy.py index 873af6ca22b3..34eea7f882ae 100644 --- a/python/tidy/servo_tidy/tidy.py +++ b/python/tidy/servo_tidy/tidy.py @@ -26,7 +26,7 @@ # File patterns to include in the non-WPT tidy check. FILE_PATTERNS_TO_CHECK = ["*.rs", "*.rc", "*.cpp", "*.c", - "*.h", "Cargo.lock", "*.py", + "*.h", "Cargo.lock", "*.py", "*.sh", "*.toml", "*.webidl", "*.json"] # File patterns that are ignored for all tidy and lint checks. @@ -43,6 +43,7 @@ os.path.join(".", "tests", "wpt", "metadata", "MANIFEST.json"), os.path.join(".", "tests", "wpt", "metadata-css", "MANIFEST.json"), os.path.join(".", "components", "script", "dom", "webidls", "ForceTouchEvent.webidl"), + os.path.join(".", "support", "android", "openssl.sh"), # FIXME(pcwalton, #11679): This is a workaround for a tidy error on the quoted string # `"__TEXT,_info_plist"` inside an attribute. os.path.join(".", "components", "servo", "platform", "macos", "mod.rs"), @@ -169,7 +170,11 @@ def check_modeline(file_name, lines): def check_length(file_name, idx, line): if file_name.endswith(".lock") or file_name.endswith(".json"): raise StopIteration - max_length = 120 + # Prefer shorter lines when shell scripting. + if file_name.endswith(".sh"): + max_length = 80 + else: + max_length = 120 if len(line.rstrip('\n')) > max_length: yield (idx + 1, "Line is longer than %d characters" % max_length) @@ -306,6 +311,36 @@ def check_toml(file_name, lines): yield (0, ".toml file should contain a valid license.") +def check_shell(file_name, lines): + if not file_name.endswith(".sh"): + raise StopIteration + + shebang = "#!/usr/bin/env bash" + required_options = {"set -o errexit", "set -o nounset", "set -o pipefail"} + + 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)) + + 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 + elif 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 + + def check_rust(file_name, lines): if not file_name.endswith(".rs") or \ file_name.endswith(".mako.rs") or \ @@ -694,7 +729,8 @@ def scan(only_changed_files=False, progress=True): # standard checks files_to_check = filter_files('.', only_changed_files, progress) checking_functions = (check_flake8, check_lock, check_webidl_spec, check_json) - line_checking_functions = (check_license, check_by_line, check_toml, check_rust, check_spec, check_modeline) + line_checking_functions = (check_license, check_by_line, check_toml, check_shell, + check_rust, check_spec, check_modeline) errors = collect_errors_for_files(files_to_check, checking_functions, line_checking_functions) # check dependecy licenses dep_license_errors = check_dep_license_errors(get_dep_toml_files(only_changed_files), progress) diff --git a/python/tidy/servo_tidy_tests/shell_tidy.sh b/python/tidy/servo_tidy_tests/shell_tidy.sh new file mode 100644 index 000000000000..fbf0e784755b --- /dev/null +++ b/python/tidy/servo_tidy_tests/shell_tidy.sh @@ -0,0 +1,7 @@ +#!/bin/bash +# +# Tests tidy for shell scripts. + +set -o nounset + +echo "hello world" diff --git a/python/tidy/servo_tidy_tests/test_tidy.py b/python/tidy/servo_tidy_tests/test_tidy.py index 8db19a5efd72..3ad7e03d6677 100644 --- a/python/tidy/servo_tidy_tests/test_tidy.py +++ b/python/tidy/servo_tidy_tests/test_tidy.py @@ -48,6 +48,12 @@ def test_licence(self): self.assertEqual('incorrect license', errors.next()[2]) self.assertNoMoreErrors(errors) + 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.assertNoMoreErrors(errors) + def test_rust(self): errors = tidy.collect_errors_for_files(iterFile('rust_tidy.rs'), [], [tidy.check_rust], print_text=False) self.assertEqual('use statement spans multiple lines', errors.next()[2])