From be7625fc1e329dde1dbe6a2da2f175042c39c61b Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Mon, 11 Aug 2025 13:04:11 +0200 Subject: [PATCH] tidy: Replace custom `panic`/`unwrap` lint with clippy lint (#38593) This change replaces our custom `panic` / `unwrap` lint with the one from clippy. This rule as not properly applied in servoshell, so this change fixes some clippy errors raised by the new configuration. Testing: This change removes the tidy tests for the custom lints, but otherwise the behavior is tested as part of clippy itself. Signed-off-by: Martin Robinson --- components/compositing/Cargo.toml | 4 ++++ components/constellation/Cargo.toml | 4 ++++ ports/servoshell/desktop/headed_window.rs | 23 ++++++++++++++++----- ports/servoshell/desktop/headless_window.rs | 3 +++ python/tidy/test.py | 2 -- python/tidy/tests/rust_tidy.rs | 4 ---- python/tidy/tidy.py | 17 --------------- 7 files changed, 29 insertions(+), 28 deletions(-) diff --git a/components/compositing/Cargo.toml b/components/compositing/Cargo.toml index 648d60b497f..d5792b858db 100644 --- a/components/compositing/Cargo.toml +++ b/components/compositing/Cargo.toml @@ -16,6 +16,10 @@ default = [] tracing = ["dep:tracing"] webxr = ["dep:webxr"] +[lints.clippy] +unwrap_used = "deny" +panic = "deny" + [dependencies] base = { workspace = true } bincode = { workspace = true } diff --git a/components/constellation/Cargo.toml b/components/constellation/Cargo.toml index 4b01b0cbce3..32d76b452db 100644 --- a/components/constellation/Cargo.toml +++ b/components/constellation/Cargo.toml @@ -20,6 +20,10 @@ vello = ["canvas/vello"] vello_cpu = ["canvas/vello_cpu"] raqote = ["canvas/raqote"] +[lints.clippy] +unwrap_used = "deny" +panic = "deny" + [dependencies] background_hang_monitor = { path = "../background_hang_monitor" } background_hang_monitor_api = { workspace = true } diff --git a/ports/servoshell/desktop/headed_window.rs b/ports/servoshell/desktop/headed_window.rs index 5bb97376204..1e1c1cbdb7d 100644 --- a/ports/servoshell/desktop/headed_window.rs +++ b/ports/servoshell/desktop/headed_window.rs @@ -4,6 +4,9 @@ //! A winit window implementation. +#![deny(clippy::panic)] +#![deny(clippy::unwrap_used)] + use std::cell::{Cell, RefCell}; use std::collections::HashMap; use std::env; @@ -118,7 +121,10 @@ impl Window { winit_window.set_window_icon(Some(load_icon(icon_bytes))); } - Window::force_srgb_color_space(winit_window.window_handle().unwrap().as_raw()); + let window_handle = winit_window + .window_handle() + .expect("winit window did not have a window handle"); + Window::force_srgb_color_space(window_handle.as_raw()); let monitor = winit_window .current_monitor() @@ -153,7 +159,9 @@ impl Window { } // Make sure the gl context is made current. - window_rendering_context.make_current().unwrap(); + window_rendering_context + .make_current() + .expect("Could not make window RenderingContext current"); let rendering_context = Rc::new(window_rendering_context.offscreen_context(inner_size)); @@ -405,7 +413,10 @@ impl Window { } }) .shortcut(CMD_OR_CONTROL, 'T', || { - state.create_and_focus_toplevel_webview(Url::parse("servo:newtab").unwrap()); + state.create_and_focus_toplevel_webview( + Url::parse("servo:newtab") + .expect("Should be able to unconditionally parse 'servo:newtab' as URL"), + ); }) .shortcut(CMD_OR_CONTROL, 'Q', || state.servo().start_shutting_down()) .otherwise(|| handled = false); @@ -425,7 +436,7 @@ impl Window { unsafe { let view = handle.ns_view.cast::().as_ref(); view.window() - .unwrap() + .expect("Should have a window") .setColorSpace(Some(&NSColorSpace::sRGBColorSpace())); } } @@ -888,7 +899,9 @@ impl servo::webxr::glwindow::GlWindow for XRWindow { } fn display_handle(&self) -> raw_window_handle::DisplayHandle { - self.winit_window.display_handle().unwrap() + self.winit_window + .display_handle() + .expect("Every window should have a display handle") } } diff --git a/ports/servoshell/desktop/headless_window.rs b/ports/servoshell/desktop/headless_window.rs index 47b20a64e5c..5c0f5113813 100644 --- a/ports/servoshell/desktop/headless_window.rs +++ b/ports/servoshell/desktop/headless_window.rs @@ -4,6 +4,9 @@ //! A headless window implementation. +#![deny(clippy::panic)] +#![deny(clippy::unwrap_used)] + use std::cell::Cell; use std::rc::Rc; diff --git a/python/tidy/test.py b/python/tidy/test.py index 57bbaebc4a0..40ce3ca9495 100644 --- a/python/tidy/test.py +++ b/python/tidy/test.py @@ -124,8 +124,6 @@ class CheckTidiness(unittest.TestCase): self.assertEqual("use &T instead of &DomRoot", 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/tests/rust_tidy.rs b/python/tidy/tests/rust_tidy.rs index abd533d5807..1bd0cfa3d27 100644 --- a/python/tidy/tests/rust_tidy.rs +++ b/python/tidy/tests/rust_tidy.rs @@ -78,8 +78,4 @@ 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/tidy.py b/python/tidy/tidy.py index ebaea2308dc..aef2732f505 100644 --- a/python/tidy/tidy.py +++ b/python/tidy/tidy.py @@ -557,21 +557,9 @@ def check_rust(file_name: str, lines: list[bytes]) -> Iterator[tuple[int, str]]: import_block = False whitespace = False - PANIC_NOT_ALLOWED_PATHS = [ - os.path.join("*", "components", "compositing", "compositor.rs"), - os.path.join("*", "components", "constellation", "*"), - os.path.join("*", "ports", "servoshell", "headed_window.rs"), - os.path.join("*", "ports", "servoshell", "headless_window.rs"), - os.path.join("*", "ports", "servoshell", "embedder.rs"), - os.path.join("*", "rust_tidy.rs"), # This is for the tests. - ] - is_panic_not_allowed_rs_file = any([fnmatch.fnmatch(file_name, path) for path in PANIC_NOT_ALLOWED_PATHS]) - prev_open_brace = False multi_line_string = False - 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 line = original_line.strip() @@ -660,11 +648,6 @@ def check_rust(file_name: str, lines: list[bytes]) -> Iterator[tuple[int, str]]: yield (idx + 1, "found an empty line following a {") prev_open_brace = line.endswith("{") - if is_panic_not_allowed_rs_file: - match = re.search(r"unwrap\(|panic!\(", line) - if match: - yield (idx + 1, panic_message) - # Avoid flagging constructs def is_associated_type(match, line):