From b0196ad3734149c98cfad89df0864fca3bdf92ce Mon Sep 17 00:00:00 2001 From: Ekta Siwach <137225906+ektuu@users.noreply.github.com> Date: Fri, 29 Mar 2024 20:13:10 +0530 Subject: [PATCH] clippy: Fix a variety of warnings in components/script/dom (#31894) --- components/script/dom/bindings/root.rs | 17 +++++++++++------ components/script/dom/bindings/str.rs | 11 ++++++++--- .../script/dom/bindings/structuredclone.rs | 6 ++++-- components/script/dom/bindings/utils.rs | 2 +- components/script/dom/bindings/xmlname.rs | 7 ++----- components/script/dom/htmlscriptelement.rs | 10 +++++----- components/script/dom/htmlselectelement.rs | 7 +++---- components/script/dom/htmlvideoelement.rs | 9 +++------ components/script/dom/navigator.rs | 6 ++---- components/script/dom/node.rs | 13 ++++++------- 10 files changed, 45 insertions(+), 43 deletions(-) diff --git a/components/script/dom/bindings/root.rs b/components/script/dom/bindings/root.rs index ed74173c7a3..da8113afefd 100644 --- a/components/script/dom/bindings/root.rs +++ b/components/script/dom/bindings/root.rs @@ -76,8 +76,16 @@ where } } -/// Represents values that can be rooted through a stable address that will +/// `StableTraceObject` represents values that can be rooted through a stable address that will /// not change for their whole lifetime. +/// It is an unsafe trait that requires implementors to ensure certain safety guarantees. +/// +/// # Safety +/// +/// Implementors of this trait must ensure that the `trace` method correctly accounts for all +/// owned and referenced objects, so that the garbage collector can accurately determine which +/// objects are still in use. Failing to adhere to this contract may result in undefined behavior, +/// such as use-after-free errors. pub unsafe trait StableTraceObject { /// Returns a stable trace object which address won't change for the whole /// lifetime of the value. @@ -267,10 +275,7 @@ impl RootCollection { unsafe fn unroot(&self, object: *const dyn JSTraceable) { assert_in_script(); let roots = &mut *self.roots.get(); - match roots - .iter() - .rposition(|r| *r as *const () == object as *const ()) - { + match roots.iter().rposition(|r| std::ptr::eq(*r, object)) { Some(idx) => { roots.remove(idx); }, @@ -486,7 +491,7 @@ impl Eq for Dom {} impl PartialEq for LayoutDom<'_, T> { fn eq(&self, other: &Self) -> bool { - self.value as *const T == other.value as *const T + std::ptr::eq(self.value, other.value) } } diff --git a/components/script/dom/bindings/str.rs b/components/script/dom/bindings/str.rs index 63866eb6bf4..70a2cc2f7fb 100644 --- a/components/script/dom/bindings/str.rs +++ b/components/script/dom/bindings/str.rs @@ -40,6 +40,11 @@ impl ByteString { self.0.len() } + /// Checks if the ByteString is empty. + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + /// Returns `self` with A–Z replaced by a–z. pub fn to_lower(&self) -> ByteString { ByteString::new(self.0.to_ascii_lowercase()) @@ -365,7 +370,7 @@ impl DOMString { let (year_int, month_int) = parse_month_component(value)?; // Step 4 - if value.split("-").nth(2).is_some() { + if value.split('-').nth(2).is_some() { return None; } // Step 5 @@ -443,7 +448,7 @@ impl DOMString { if !( // A valid number is the same as what rust considers to be valid, // except for +1., NaN, and Infinity. - val.is_infinite() || val.is_nan() || input.ends_with(".") || input.starts_with('+') + val.is_infinite() || val.is_nan() || input.ends_with('.') || input.starts_with('+') ) { return Some(val); } @@ -677,7 +682,7 @@ fn parse_month_component(value: &str) -> Option<(i32, u32)> { // Step 4, 5 let month_int = month.parse::().ok()?; - if month.len() != 2 || month_int > 12 || month_int < 1 { + if month.len() != 2 || !(1..=12).contains(&month_int) { return None; } diff --git a/components/script/dom/bindings/structuredclone.rs b/components/script/dom/bindings/structuredclone.rs index b37c9ab2093..209a499ee59 100644 --- a/components/script/dom/bindings/structuredclone.rs +++ b/components/script/dom/bindings/structuredclone.rs @@ -169,12 +169,14 @@ unsafe extern "C" fn read_transfer_callback( let sc_holder = &mut *(closure as *mut StructuredDataHolder); let in_realm_proof = AlreadyInRealm::assert_for_cx(SafeJSContext::from_ptr(cx)); let owner = GlobalScope::from_context(cx, InRealm::Already(&in_realm_proof)); - if let Ok(_) = ::transfer_receive( + if ::transfer_receive( &owner, sc_holder, extra_data, return_object, - ) { + ) + .is_ok() + { return true; } } diff --git a/components/script/dom/bindings/utils.rs b/components/script/dom/bindings/utils.rs index 402b60fc2b6..e5f5f8ab41f 100644 --- a/components/script/dom/bindings/utils.rs +++ b/components/script/dom/bindings/utils.rs @@ -197,7 +197,7 @@ pub unsafe fn get_array_index_from_id(_cx: *mut JSContext, id: HandleId) -> Opti let first_char = char::decode_utf16(chars.iter().cloned()) .next() .map_or('\0', |r| r.unwrap_or('\0')); - if first_char < 'a' || first_char > 'z' { + if !('a'..='z').contains(&first_char) { return None; } diff --git a/components/script/dom/bindings/xmlname.rs b/components/script/dom/bindings/xmlname.rs index 7fd7a7ec4de..aac839f53fd 100644 --- a/components/script/dom/bindings/xmlname.rs +++ b/components/script/dom/bindings/xmlname.rs @@ -90,8 +90,7 @@ pub enum XMLName { /// for details. pub fn xml_name_type(name: &str) -> XMLName { fn is_valid_start(c: char) -> bool { - match c { - ':' | + matches!(c, ':' | 'A'..='Z' | '_' | 'a'..='z' | @@ -106,9 +105,7 @@ pub fn xml_name_type(name: &str) -> XMLName { '\u{3001}'..='\u{D7FF}' | '\u{F900}'..='\u{FDCF}' | '\u{FDF0}'..='\u{FFFD}' | - '\u{10000}'..='\u{EFFFF}' => true, - _ => false, - } + '\u{10000}'..='\u{EFFFF}') } fn is_valid_continuation(c: char) -> bool { diff --git a/components/script/dom/htmlscriptelement.rs b/components/script/dom/htmlscriptelement.rs index 2c3faa899bd..2e157a896d9 100644 --- a/components/script/dom/htmlscriptelement.rs +++ b/components/script/dom/htmlscriptelement.rs @@ -898,15 +898,15 @@ impl HTMLScriptElement { warn!("Error creating input and output files for unminify"); } - let path; - match window_from_node(self).unminified_js_dir() { - Some(unminified_js_dir) => path = PathBuf::from(unminified_js_dir), + let path = match window_from_node(self).unminified_js_dir() { + Some(unminified_js_dir) => PathBuf::from(unminified_js_dir), None => { warn!("Unminified script directory not found"); return; }, - } - let (base, has_name) = match script.url.as_str().ends_with("/") { + }; + + let (base, has_name) = match script.url.as_str().ends_with('/') { true => ( path.join(&script.url[url::Position::BeforeHost..]) .as_path() diff --git a/components/script/dom/htmlselectelement.rs b/components/script/dom/htmlselectelement.rs index ed50de11a53..74964bfe25c 100755 --- a/components/script/dom/htmlselectelement.rs +++ b/components/script/dom/htmlselectelement.rs @@ -518,11 +518,10 @@ impl Validatable for HTMLSelectElement { // https://html.spec.whatwg.org/multipage/#the-select-element%3Asuffering-from-being-missing if validate_flags.contains(ValidationFlags::VALUE_MISSING) && self.Required() { let placeholder = self.get_placeholder_label_option(); - let selected_option = self + let is_value_missing = !self .list_of_options() - .filter(|e| e.Selected() && placeholder.as_ref() != Some(e)) - .next(); - failed_flags.set(ValidationFlags::VALUE_MISSING, selected_option.is_none()); + .any(|e| e.Selected() && placeholder != Some(e)); + failed_flags.set(ValidationFlags::VALUE_MISSING, is_value_missing); } failed_flags diff --git a/components/script/dom/htmlvideoelement.rs b/components/script/dom/htmlvideoelement.rs index 89574d78dbe..b09503627c5 100644 --- a/components/script/dom/htmlvideoelement.rs +++ b/components/script/dom/htmlvideoelement.rs @@ -279,12 +279,9 @@ impl VirtualMethods for HTMLVideoElement { self.super_type().unwrap().attribute_mutated(attr, mutation); if let Some(new_value) = mutation.new_value(attr) { - match attr.local_name() { - &local_name!("poster") => { - self.fetch_poster_frame(&new_value); - }, - _ => (), - }; + if attr.local_name() == &local_name!("poster") { + self.fetch_poster_frame(&new_value); + } } } } diff --git a/components/script/dom/navigator.rs b/components/script/dom/navigator.rs index ad9191232b8..3732a1face7 100644 --- a/components/script/dom/navigator.rs +++ b/components/script/dom/navigator.rs @@ -82,10 +82,8 @@ impl Navigator { } pub fn gamepads(&self) -> DomRoot { - let gamepads = self - .gamepads - .or_init(|| GamepadList::new(&self.global(), &[])); - gamepads + self.gamepads + .or_init(|| GamepadList::new(&self.global(), &[])) } pub fn has_gamepad_gesture(&self) -> bool { diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index cb5c97afb52..aea3bc96366 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -10,7 +10,7 @@ use std::default::Default; use std::ops::Range; use std::slice::from_ref; use std::sync::Arc as StdArc; -use std::{cmp, iter, mem}; +use std::{cmp, iter}; use app_units::Au; use bitflags::bitflags; @@ -454,7 +454,7 @@ pub struct QuerySelectorIterator { iterator: TreeIterator, } -impl<'a> QuerySelectorIterator { +impl QuerySelectorIterator { fn new(iter: TreeIterator, selectors: SelectorList) -> QuerySelectorIterator { QuerySelectorIterator { selectors, @@ -463,7 +463,7 @@ impl<'a> QuerySelectorIterator { } } -impl<'a> Iterator for QuerySelectorIterator { +impl Iterator for QuerySelectorIterator { type Item = DomRoot; fn next(&mut self) -> Option> { @@ -1203,8 +1203,7 @@ impl Node { match last_child.and_then(|node| { node.inclusively_preceding_siblings() .filter_map(DomRoot::downcast::) - .filter(|elem| is_delete_type(elem)) - .next() + .find(|elem| is_delete_type(elem)) }) { Some(element) => element, None => return Ok(()), @@ -1314,10 +1313,10 @@ where #[allow(unsafe_code)] pub unsafe fn from_untrusted_node_address(candidate: UntrustedNodeAddress) -> DomRoot { // https://github.com/servo/servo/issues/6383 - let candidate: uintptr_t = mem::transmute(candidate.0); + let candidate = candidate.0 as usize; // let object: *mut JSObject = jsfriendapi::bindgen::JS_GetAddressableObject(runtime, // candidate); - let object: *mut JSObject = mem::transmute(candidate); + let object = candidate as *mut JSObject; if object.is_null() { panic!("Attempted to create a `Dom` from an invalid pointer!") }