Skip to content

Commit

Permalink
Add linting for shell scripts
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Jim Berlage committed Jul 22, 2016
1 parent 1e0321f commit 7952bd0
Show file tree
Hide file tree
Showing 10 changed files with 104 additions and 13 deletions.
18 changes: 15 additions & 3 deletions 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"
9 changes: 7 additions & 2 deletions 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"
Expand Down
4 changes: 4 additions & 0 deletions 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
Expand Down
8 changes: 6 additions & 2 deletions etc/ci/manifest_changed.sh
@@ -1,14 +1,18 @@
#!/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

# 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

Expand Down
9 changes: 7 additions & 2 deletions 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
Expand All @@ -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
Expand Down
6 changes: 5 additions & 1 deletion 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
Expand All @@ -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}"
Expand Down
8 changes: 8 additions & 0 deletions python/tidy/servo_tidy/licenseck.py
Expand Up @@ -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
Expand Down
42 changes: 39 additions & 3 deletions python/tidy/servo_tidy/tidy.py
Expand Up @@ -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.
Expand All @@ -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"),
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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 \
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions 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"
6 changes: 6 additions & 0 deletions python/tidy/servo_tidy_tests/test_tidy.py
Expand Up @@ -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])
Expand Down

0 comments on commit 7952bd0

Please sign in to comment.