aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMartin Robinson <mrobinson@igalia.com>2023-03-21 18:52:46 +0100
committerMartin Robinson <mrobinson@igalia.com>2023-03-22 15:37:56 +0100
commitec9cbeefd88176e4fb85d1066beafa9eec19d51e (patch)
tree2dda9ca8c59bce80bec9b0c5cb9d93c8ecb21cf1
parent259ccff4919d5a764fc38cd8ebb0f7eaa0ad12c0 (diff)
downloadservo-ec9cbeefd88176e4fb85d1066beafa9eec19d51e.tar.gz
servo-ec9cbeefd88176e4fb85d1066beafa9eec19d51e.zip
Merge forbidden panic check into test-tidy
This cleans up the GitHub actions yaml a bit and ensures that developers are running this check locally before submitting changes. In addition, it allows adding tests for this check. Finally, this change fixes the tidy tests by upgrading voluptuous for Python 3.10 as well as by reverting an inadvertent change for NixOS compatibility on one of the dummy testing files.
-rw-r--r--.github/workflows/main.yml2
-rw-r--r--.github/workflows/wpt-nightly.yml2
-rwxr-xr-xetc/ci/check_no_panic.sh34
-rw-r--r--python/tidy/servo_tidy/tidy.py18
-rw-r--r--python/tidy/servo_tidy_tests/rust_tidy.rs4
-rw-r--r--python/tidy/servo_tidy_tests/shell_tidy.sh2
-rw-r--r--python/tidy/servo_tidy_tests/test_tidy.py2
-rw-r--r--python/tidy/setup.py2
8 files changed, 26 insertions, 40 deletions
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 9e15e465edc..b2774990459 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -175,8 +175,6 @@ jobs:
run: python3 ./mach build --release
- name: Lockfile check
run: ./etc/ci/lockfile_changed.sh
- - name: Forbidden panic check
- run: ./etc/ci/check_no_panic.sh
- name: Package binary
run: tar -czf target.tar.gz target/release/servo resources
- name: Archive binary
diff --git a/.github/workflows/wpt-nightly.yml b/.github/workflows/wpt-nightly.yml
index d1b4da572f7..1d733b11ab3 100644
--- a/.github/workflows/wpt-nightly.yml
+++ b/.github/workflows/wpt-nightly.yml
@@ -27,8 +27,6 @@ jobs:
run: python3 ./mach build --release
- name: Lockfile check
run: ./etc/ci/lockfile_changed.sh
- - name: Forbidden panic check
- run: ./etc/ci/check_no_panic.sh
- name: Package binary
run: tar -czf target.tar.gz target/release/servo resources
- name: Archive binary
diff --git a/etc/ci/check_no_panic.sh b/etc/ci/check_no_panic.sh
deleted file mode 100755
index a4bead60cec..00000000000
--- a/etc/ci/check_no_panic.sh
+++ /dev/null
@@ -1,34 +0,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 https://mozilla.org/MPL/2.0/.
-
-# Make sure listed paths do not use unwrap() or panic!()
-
-set -o errexit
-set -o nounset
-set -o pipefail
-
-# cd into repo root to make sure paths work in any case
-cd "$(git rev-parse --show-toplevel)"
-
-# Each path can be either a single file or a directory
-PATHS=(
- "components/compositing/compositor.rs"
- "components/constellation/"
- "ports/winit/headed_window.rs"
- "ports/winit/headless_window.rs"
- "ports/winit/embedder.rs"
-)
-
-# Make sure the paths exist
-ls -1 "${PATHS[@]}"
-
-# Make sure the files do not contain "unwrap" or "panic!"
-! grep \
- --dereference-recursive \
- --line-number \
- --with-filename \
- "unwrap(\|panic!(" \
- "${PATHS[@]}"
diff --git a/python/tidy/servo_tidy/tidy.py b/python/tidy/servo_tidy/tidy.py
index a48fb4adc5d..8426574887b 100644
--- a/python/tidy/servo_tidy/tidy.py
+++ b/python/tidy/servo_tidy/tidy.py
@@ -536,6 +536,18 @@ def check_rust(file_name, lines):
is_lib_rs_file = file_name.endswith("lib.rs")
+ PANIC_NOT_ALLOWED_PATHS = [
+ os.path.join("*", "components", "compositing", "compositor.rs"),
+ os.path.join("*", "components", "constellation", "*"),
+ os.path.join("*", "ports", "winit", "headed_window.rs"),
+ os.path.join("*", "ports", "winit", "headless_window.rs"),
+ os.path.join("*", "ports", "winit", "embedder.rs"),
+ os.path.join("*", "rust_tidy.rs"), # This is for the tests.
+ ]
+ is_panic_not_allowed_rs_file = any([
+ glob.fnmatch.fnmatch(file_name, path) for path in PANIC_NOT_ALLOWED_PATHS])
+ print(file_name)
+
prev_open_brace = False
multi_line_string = False
prev_crate = {}
@@ -547,6 +559,7 @@ def check_rust(file_name, lines):
decl_message = "{} is not in alphabetical order"
decl_expected = "\n\t\033[93mexpected: {}\033[0m"
decl_found = "\n\t\033[91mfound: {}\033[0m"
+ panic_message = "unwrap() or panic!() found in code which should not panic."
for idx, original_line in enumerate(map(lambda line: line.decode("utf-8"), lines)):
# simplify the analysis
@@ -677,6 +690,11 @@ def check_rust(file_name, lines):
# not a feature attribute line, so empty previous name
prev_feature_name = ""
+ if is_panic_not_allowed_rs_file:
+ match = re.search(r"unwrap\(|panic!\(", line)
+ if match:
+ yield (idx + 1, panic_message)
+
# modules must be in the same line and alphabetically sorted
if line.startswith("mod ") or line.startswith("pub mod "):
# strip /(pub )?mod/ from the left and ";" from the right
diff --git a/python/tidy/servo_tidy_tests/rust_tidy.rs b/python/tidy/servo_tidy_tests/rust_tidy.rs
index 1bd0cfa3d27..abd533d5807 100644
--- a/python/tidy/servo_tidy_tests/rust_tidy.rs
+++ b/python/tidy/servo_tidy_tests/rust_tidy.rs
@@ -78,4 +78,8 @@ impl test {
} else {
let xif = 42 in { xif } // Should not trigger
}
+
+ let option = Some(3);
+ println!("{}", option.unwrap());
+ panic!("What a way to end.");
}
diff --git a/python/tidy/servo_tidy_tests/shell_tidy.sh b/python/tidy/servo_tidy_tests/shell_tidy.sh
index 2a887abf206..e38358fc3b6 100644
--- a/python/tidy/servo_tidy_tests/shell_tidy.sh
+++ b/python/tidy/servo_tidy_tests/shell_tidy.sh
@@ -1,4 +1,4 @@
-#!/usr/bin/env bash
+#!/bin/bash
#
# Tests tidy for shell scripts.
diff --git a/python/tidy/servo_tidy_tests/test_tidy.py b/python/tidy/servo_tidy_tests/test_tidy.py
index 3189250143a..6138320cc35 100644
--- a/python/tidy/servo_tidy_tests/test_tidy.py
+++ b/python/tidy/servo_tidy_tests/test_tidy.py
@@ -115,6 +115,8 @@ class CheckTidiness(unittest.TestCase):
self.assertEqual('use &T instead of &DomRoot<T>', next(errors)[2])
self.assertEqual('encountered function signature with -> ()', next(errors)[2])
self.assertEqual('operators should go at the end of the first line', next(errors)[2])
+ self.assertEqual('unwrap() or panic!() found in code which should not panic.', next(errors)[2])
+ self.assertEqual('unwrap() or panic!() found in code which should not panic.', next(errors)[2])
self.assertNoMoreErrors(errors)
feature_errors = tidy.collect_errors_for_files(iterFile('lib.rs'), [], [tidy.check_rust], print_text=False)
diff --git a/python/tidy/setup.py b/python/tidy/setup.py
index 19a1f61d1fc..17c6172acbc 100644
--- a/python/tidy/setup.py
+++ b/python/tidy/setup.py
@@ -17,7 +17,7 @@ install_requires = [
"flake8==3.8.3",
"toml==0.9.2",
"colorama==0.3.7",
- "voluptuous==0.11.5",
+ "voluptuous==0.12.1",
"PyYAML==5.4",
]