From 1f94463ce2a23f0cf128fd62fbad5492da5430aa Mon Sep 17 00:00:00 2001 From: Mikayla Date: Sun, 21 Jan 2024 15:35:47 -0800 Subject: [PATCH] Switch Arc> to Rc>, a relic of the GPUI2 port. Make gpui pass clippy --- crates/gpui/src/app.rs | 17 +++++++++-------- crates/gpui/src/app/entity_map.rs | 5 ++--- crates/gpui/src/color.rs | 14 +++++--------- crates/gpui/src/element.rs | 4 +--- crates/gpui/src/elements/div.rs | 7 +++---- crates/gpui/src/elements/overlay.rs | 2 +- crates/gpui/src/elements/uniform_list.rs | 2 +- crates/gpui/src/geometry.rs | 4 ++-- crates/gpui/src/gpui.rs | 3 +++ crates/gpui/src/interactive.rs | 2 +- crates/gpui/src/key_dispatch.rs | 15 ++++++--------- crates/gpui/src/{keymap => }/keymap.rs | 10 +++++++++- crates/gpui/src/keymap/context.rs | 4 ++-- crates/gpui/src/keymap/matcher.rs | 12 ++++++------ crates/gpui/src/keymap/mod.rs | 9 --------- crates/gpui/src/subscription.rs | 1 - crates/gpui/src/taffy.rs | 14 ++++---------- crates/gpui/src/text_system.rs | 2 +- crates/gpui/src/text_system/line.rs | 2 ++ crates/gpui/src/text_system/line_layout.rs | 1 + crates/gpui/src/window.rs | 2 +- crates/media/src/media.rs | 15 +++++++++++++++ 22 files changed, 75 insertions(+), 72 deletions(-) rename crates/gpui/src/{keymap => }/keymap.rs (97%) delete mode 100644 crates/gpui/src/keymap/mod.rs diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 7b349d9256..dfecffb80a 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -25,13 +25,12 @@ use crate::{ use anyhow::{anyhow, Result}; use collections::{FxHashMap, FxHashSet, VecDeque}; use futures::{channel::oneshot, future::LocalBoxFuture, Future}; -use parking_lot::Mutex; + use slotmap::SlotMap; use std::{ any::{type_name, TypeId}, cell::{Ref, RefCell, RefMut}, marker::PhantomData, - mem, ops::{Deref, DerefMut}, path::{Path, PathBuf}, rc::{Rc, Weak}, @@ -109,6 +108,7 @@ pub struct App(Rc); /// configured, you'll start the app with `App::run`. impl App { /// Builds an app with the given asset source. + #[allow(clippy::new_without_default)] pub fn new() -> Self { Self(AppContext::new( current_platform(), @@ -224,7 +224,7 @@ pub struct AppContext { pub(crate) entities: EntityMap, pub(crate) new_view_observers: SubscriberSet, pub(crate) windows: SlotMap>, - pub(crate) keymap: Arc>, + pub(crate) keymap: Rc>, pub(crate) global_action_listeners: FxHashMap>>, pending_effects: VecDeque, @@ -242,6 +242,7 @@ pub struct AppContext { } impl AppContext { + #[allow(clippy::new_ret_no_self)] pub(crate) fn new( platform: Rc, asset_source: Arc, @@ -285,7 +286,7 @@ impl AppContext { entities, new_view_observers: SubscriberSet::new(), windows: SlotMap::with_key(), - keymap: Arc::new(Mutex::new(Keymap::default())), + keymap: Rc::new(RefCell::new(Keymap::default())), global_action_listeners: FxHashMap::default(), pending_effects: VecDeque::new(), pending_notifications: FxHashSet::default(), @@ -763,7 +764,7 @@ impl AppContext { /// so it can be held across `await` points. pub fn to_async(&self) -> AsyncAppContext { AsyncAppContext { - app: unsafe { mem::transmute(self.this.clone()) }, + app: self.this.clone(), background_executor: self.background_executor.clone(), foreground_executor: self.foreground_executor.clone(), } @@ -996,13 +997,13 @@ impl AppContext { /// Register key bindings. pub fn bind_keys(&mut self, bindings: impl IntoIterator) { - self.keymap.lock().add_bindings(bindings); + self.keymap.borrow_mut().add_bindings(bindings); self.pending_effects.push_back(Effect::Refresh); } /// Clear all key bindings in the app. pub fn clear_key_bindings(&mut self) { - self.keymap.lock().clear(); + self.keymap.borrow_mut().clear(); self.pending_effects.push_back(Effect::Refresh); } @@ -1106,7 +1107,7 @@ impl AppContext { /// Sets the menu bar for this application. This will replace any existing menu bar. pub fn set_menus(&mut self, menus: Vec) { - self.platform.set_menus(menus, &self.keymap.lock()); + self.platform.set_menus(menus, &self.keymap.borrow()); } /// Dispatch an action to the currently active window or global action handler diff --git a/crates/gpui/src/app/entity_map.rs b/crates/gpui/src/app/entity_map.rs index db5d844d17..6383cdad7b 100644 --- a/crates/gpui/src/app/entity_map.rs +++ b/crates/gpui/src/app/entity_map.rs @@ -242,7 +242,7 @@ impl Clone for AnyModel { assert_ne!(prev_count, 0, "Detected over-release of a model."); } - let this = Self { + Self { entity_id: self.entity_id, entity_type: self.entity_type, entity_map: self.entity_map.clone(), @@ -254,8 +254,7 @@ impl Clone for AnyModel { .write() .leak_detector .handle_created(self.entity_id), - }; - this + } } } diff --git a/crates/gpui/src/color.rs b/crates/gpui/src/color.rs index e76c31d6f1..dadc3d5ad3 100644 --- a/crates/gpui/src/color.rs +++ b/crates/gpui/src/color.rs @@ -203,20 +203,16 @@ impl PartialEq for Hsla { impl PartialOrd for Hsla { fn partial_cmp(&self, other: &Self) -> Option { - // SAFETY: The total ordering relies on this always being Some() - Some( - self.h - .total_cmp(&other.h) - .then(self.s.total_cmp(&other.s)) - .then(self.l.total_cmp(&other.l).then(self.a.total_cmp(&other.a))), - ) + Some(self.cmp(other)) } } impl Ord for Hsla { fn cmp(&self, other: &Self) -> std::cmp::Ordering { - // SAFETY: The partial comparison is a total comparison - unsafe { self.partial_cmp(other).unwrap_unchecked() } + self.h + .total_cmp(&other.h) + .then(self.s.total_cmp(&other.s)) + .then(self.l.total_cmp(&other.l).then(self.a.total_cmp(&other.a))) } } diff --git a/crates/gpui/src/element.rs b/crates/gpui/src/element.rs index cd5e6ea9dc..4dbc7be652 100644 --- a/crates/gpui/src/element.rs +++ b/crates/gpui/src/element.rs @@ -133,9 +133,7 @@ pub trait Render: 'static + Sized { } impl Render for () { - fn render(&mut self, _cx: &mut ViewContext) -> impl IntoElement { - () - } + fn render(&mut self, _cx: &mut ViewContext) -> impl IntoElement {} } /// You can derive [`IntoElement`] on any type that implements this trait. diff --git a/crates/gpui/src/elements/div.rs b/crates/gpui/src/elements/div.rs index b1d936546b..7ccdf8c2bf 100644 --- a/crates/gpui/src/elements/div.rs +++ b/crates/gpui/src/elements/div.rs @@ -1008,10 +1008,9 @@ pub(crate) type ActionListener = Box Div { #[cfg(debug_assertions)] - let interactivity = { - let mut interactivity = Interactivity::default(); - interactivity.location = Some(*core::panic::Location::caller()); - interactivity + let interactivity = Interactivity { + location: Some(*core::panic::Location::caller()), + ..Default::default() }; #[cfg(not(debug_assertions))] diff --git a/crates/gpui/src/elements/overlay.rs b/crates/gpui/src/elements/overlay.rs index 61c34bd938..9db75b75ba 100644 --- a/crates/gpui/src/elements/overlay.rs +++ b/crates/gpui/src/elements/overlay.rs @@ -222,7 +222,7 @@ impl OverlayPositionMode { ) -> (Point, Bounds) { match self { OverlayPositionMode::Window => { - let anchor_position = anchor_position.unwrap_or_else(|| bounds.origin); + let anchor_position = anchor_position.unwrap_or(bounds.origin); let bounds = anchor_corner.get_bounds(anchor_position, size); (anchor_position, bounds) } diff --git a/crates/gpui/src/elements/uniform_list.rs b/crates/gpui/src/elements/uniform_list.rs index 108e669f75..ce32b993a5 100644 --- a/crates/gpui/src/elements/uniform_list.rs +++ b/crates/gpui/src/elements/uniform_list.rs @@ -181,7 +181,7 @@ impl Element for UniformList { let shared_scroll_offset = element_state .interactive .scroll_offset - .get_or_insert_with(|| Rc::default()) + .get_or_insert_with(Rc::default) .clone(); let item_height = self.measure_item(Some(padded_bounds.size.width), cx).height; diff --git a/crates/gpui/src/geometry.rs b/crates/gpui/src/geometry.rs index d986f26be4..0a22611fac 100644 --- a/crates/gpui/src/geometry.rs +++ b/crates/gpui/src/geometry.rs @@ -2020,13 +2020,13 @@ impl Eq for Pixels {} impl PartialOrd for Pixels { fn partial_cmp(&self, other: &Self) -> Option { - self.0.partial_cmp(&other.0) + Some(self.cmp(other)) } } impl Ord for Pixels { fn cmp(&self, other: &Self) -> cmp::Ordering { - self.partial_cmp(other).unwrap() + self.0.total_cmp(&other.0) } } diff --git a/crates/gpui/src/gpui.rs b/crates/gpui/src/gpui.rs index 929e9ebfb4..a64d043ab8 100644 --- a/crates/gpui/src/gpui.rs +++ b/crates/gpui/src/gpui.rs @@ -26,6 +26,9 @@ //! TODO!(docs): Wrap up with a conclusion and links to other places? Zed / GPUI website? //! Discord for chatting about it? Other tutorials or references? +// #![deny(missing_docs)] +#![allow(clippy::type_complexity)] + #[macro_use] mod action; mod app; diff --git a/crates/gpui/src/interactive.rs b/crates/gpui/src/interactive.rs index 4faa92ce04..1bc32717c4 100644 --- a/crates/gpui/src/interactive.rs +++ b/crates/gpui/src/interactive.rs @@ -343,7 +343,7 @@ impl ExternalPaths { impl Render for ExternalPaths { fn render(&mut self, _: &mut ViewContext) -> impl IntoElement { - () // Intentionally left empty because the platform will render icons for the dragged files + // Intentionally left empty because the platform will render icons for the dragged files } } diff --git a/crates/gpui/src/key_dispatch.rs b/crates/gpui/src/key_dispatch.rs index dd5a7ab84e..b4de09e36c 100644 --- a/crates/gpui/src/key_dispatch.rs +++ b/crates/gpui/src/key_dispatch.rs @@ -54,13 +54,12 @@ use crate::{ KeyContext, Keymap, KeymatchResult, Keystroke, KeystrokeMatcher, WindowContext, }; use collections::FxHashMap; -use parking_lot::Mutex; use smallvec::{smallvec, SmallVec}; use std::{ any::{Any, TypeId}, + cell::RefCell, mem, rc::Rc, - sync::Arc, }; #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] @@ -73,7 +72,7 @@ pub(crate) struct DispatchTree { focusable_node_ids: FxHashMap, view_node_ids: FxHashMap, keystroke_matchers: FxHashMap, KeystrokeMatcher>, - keymap: Arc>, + keymap: Rc>, action_registry: Rc, } @@ -96,7 +95,7 @@ pub(crate) struct DispatchActionListener { } impl DispatchTree { - pub fn new(keymap: Arc>, action_registry: Rc) -> Self { + pub fn new(keymap: Rc>, action_registry: Rc) -> Self { Self { node_stack: Vec::new(), context_stack: Vec::new(), @@ -307,7 +306,7 @@ impl DispatchTree { action: &dyn Action, context_stack: &Vec, ) -> Vec { - let keymap = self.keymap.lock(); + let keymap = self.keymap.borrow(); keymap .bindings_for_action(action) .filter(|binding| { @@ -440,9 +439,7 @@ impl DispatchTree { #[cfg(test)] mod tests { - use std::{rc::Rc, sync::Arc}; - - use parking_lot::Mutex; + use std::{cell::RefCell, rc::Rc}; use crate::{Action, ActionRegistry, DispatchTree, KeyBinding, KeyContext, Keymap}; @@ -496,7 +493,7 @@ mod tests { registry.load_action::(); - let keymap = Arc::new(Mutex::new(keymap)); + let keymap = Rc::new(RefCell::new(keymap)); let tree = DispatchTree::new(keymap, Rc::new(registry)); diff --git a/crates/gpui/src/keymap/keymap.rs b/crates/gpui/src/keymap.rs similarity index 97% rename from crates/gpui/src/keymap/keymap.rs rename to crates/gpui/src/keymap.rs index 2eefbb841e..45e0ebbe95 100644 --- a/crates/gpui/src/keymap/keymap.rs +++ b/crates/gpui/src/keymap.rs @@ -1,4 +1,12 @@ -use crate::{Action, KeyBinding, KeyBindingContextPredicate, KeyContext, Keystroke, NoAction}; +mod binding; +mod context; +mod matcher; + +pub use binding::*; +pub use context::*; +pub(crate) use matcher::*; + +use crate::{Action, Keystroke, NoAction}; use collections::HashSet; use smallvec::SmallVec; use std::{ diff --git a/crates/gpui/src/keymap/context.rs b/crates/gpui/src/keymap/context.rs index 05ef67ba2b..6c22fa9fd6 100644 --- a/crates/gpui/src/keymap/context.rs +++ b/crates/gpui/src/keymap/context.rs @@ -267,8 +267,8 @@ impl KeyBindingContextPredicate { '(' => { source = skip_whitespace(&source[1..]); let (predicate, rest) = Self::parse_expr(source, 0)?; - if rest.starts_with(')') { - source = skip_whitespace(&rest[1..]); + if let Some(stripped) = rest.strip_prefix(')') { + source = skip_whitespace(stripped); Ok((predicate, source)) } else { Err(anyhow!("expected a ')'")) diff --git a/crates/gpui/src/keymap/matcher.rs b/crates/gpui/src/keymap/matcher.rs index 09ba281a0d..88a8826e65 100644 --- a/crates/gpui/src/keymap/matcher.rs +++ b/crates/gpui/src/keymap/matcher.rs @@ -1,11 +1,10 @@ use crate::{KeyBinding, KeyContext, Keymap, KeymapVersion, Keystroke}; -use parking_lot::Mutex; use smallvec::SmallVec; -use std::sync::Arc; +use std::{cell::RefCell, rc::Rc}; pub(crate) struct KeystrokeMatcher { pending_keystrokes: Vec, - keymap: Arc>, + keymap: Rc>, keymap_version: KeymapVersion, } @@ -15,8 +14,8 @@ pub struct KeymatchResult { } impl KeystrokeMatcher { - pub fn new(keymap: Arc>) -> Self { - let keymap_version = keymap.lock().version(); + pub fn new(keymap: Rc>) -> Self { + let keymap_version = keymap.borrow().version(); Self { pending_keystrokes: Vec::new(), keymap_version, @@ -42,7 +41,8 @@ impl KeystrokeMatcher { keystroke: &Keystroke, context_stack: &[KeyContext], ) -> KeymatchResult { - let keymap = self.keymap.lock(); + let keymap = self.keymap.borrow(); + // Clear pending keystrokes if the keymap has changed since the last matched keystroke. if keymap.version() != self.keymap_version { self.keymap_version = keymap.version(); diff --git a/crates/gpui/src/keymap/mod.rs b/crates/gpui/src/keymap/mod.rs deleted file mode 100644 index 6f1a018322..0000000000 --- a/crates/gpui/src/keymap/mod.rs +++ /dev/null @@ -1,9 +0,0 @@ -mod binding; -mod context; -mod keymap; -mod matcher; - -pub use binding::*; -pub use context::*; -pub use keymap::*; -pub(crate) use matcher::*; diff --git a/crates/gpui/src/subscription.rs b/crates/gpui/src/subscription.rs index 9dca2e3a48..c75d10ee62 100644 --- a/crates/gpui/src/subscription.rs +++ b/crates/gpui/src/subscription.rs @@ -41,7 +41,6 @@ where /// are inert, meaning that they won't be listed when calling `[SubscriberSet::remove]` or `[SubscriberSet::retain]`. /// This method returns a tuple of a [`Subscription`] and an `impl FnOnce`, and you can use the latter /// to activate the [`Subscription`]. - #[must_use] pub fn insert( &self, emitter_key: EmitterKey, diff --git a/crates/gpui/src/taffy.rs b/crates/gpui/src/taffy.rs index ea7a4575cc..0797c8f3b4 100644 --- a/crates/gpui/src/taffy.rs +++ b/crates/gpui/src/taffy.rs @@ -12,22 +12,16 @@ use taffy::{ Taffy, }; +type NodeMeasureFn = + Box>, Size, &mut WindowContext) -> Size>; + pub struct TaffyLayoutEngine { taffy: Taffy, styles: FxHashMap, children_to_parents: FxHashMap, absolute_layout_bounds: FxHashMap>, computed_layouts: FxHashSet, - nodes_to_measure: FxHashMap< - LayoutId, - Box< - dyn FnMut( - Size>, - Size, - &mut WindowContext, - ) -> Size, - >, - >, + nodes_to_measure: FxHashMap, } static EXPECT_MESSAGE: &str = "we should avoid taffy layout errors by construction if possible"; diff --git a/crates/gpui/src/text_system.rs b/crates/gpui/src/text_system.rs index cadc000f9a..1d097ca1ee 100644 --- a/crates/gpui/src/text_system.rs +++ b/crates/gpui/src/text_system.rs @@ -71,7 +71,7 @@ impl TextSystem { .all_font_names() .into_iter() .collect(); - names.extend(self.platform_text_system.all_font_families().into_iter()); + names.extend(self.platform_text_system.all_font_families()); names.extend( self.fallback_font_stack .iter() diff --git a/crates/gpui/src/text_system/line.rs b/crates/gpui/src/text_system/line.rs index 8013f5a1e0..d4f23c9539 100644 --- a/crates/gpui/src/text_system/line.rs +++ b/crates/gpui/src/text_system/line.rs @@ -25,6 +25,7 @@ pub struct ShapedLine { impl ShapedLine { /// The length of the line in utf-8 bytes. + #[allow(clippy::len_without_is_empty)] pub fn len(&self) -> usize { self.layout.len } @@ -58,6 +59,7 @@ pub struct WrappedLine { } impl WrappedLine { + #[allow(clippy::len_without_is_empty)] pub fn len(&self) -> usize { self.layout.len() } diff --git a/crates/gpui/src/text_system/line_layout.rs b/crates/gpui/src/text_system/line_layout.rs index 6c466f9680..166fefd26a 100644 --- a/crates/gpui/src/text_system/line_layout.rs +++ b/crates/gpui/src/text_system/line_layout.rs @@ -162,6 +162,7 @@ pub struct WrapBoundary { } impl WrappedLineLayout { + #[allow(clippy::len_without_is_empty)] pub fn len(&self) -> usize { self.unwrapped_layout.len } diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index 5796d139f9..5ffe753dc1 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -2560,7 +2560,7 @@ impl Display for ElementId { ElementId::View(entity_id) => write!(f, "view-{}", entity_id)?, ElementId::Integer(ix) => write!(f, "{}", ix)?, ElementId::Name(name) => write!(f, "{}", name)?, - ElementId::FocusHandle(__) => write!(f, "FocusHandle")?, + ElementId::FocusHandle(_) => write!(f, "FocusHandle")?, ElementId::NamedInteger(s, i) => write!(f, "{}-{}", s, i)?, } diff --git a/crates/media/src/media.rs b/crates/media/src/media.rs index 650af06c37..269244dbf9 100644 --- a/crates/media/src/media.rs +++ b/crates/media/src/media.rs @@ -108,6 +108,9 @@ pub mod core_video { impl_CFTypeDescription!(CVMetalTextureCache); impl CVMetalTextureCache { + /// # Safety + /// + /// metal_device must be valid according to CVMetalTextureCacheCreate pub unsafe fn new(metal_device: *mut MTLDevice) -> Result { let mut this = ptr::null(); let result = CVMetalTextureCacheCreate( @@ -124,6 +127,9 @@ pub mod core_video { } } + /// # Safety + /// + /// The arguments to this function must be valid according to CVMetalTextureCacheCreateTextureFromImage pub unsafe fn create_texture_from_image( &self, source: CVImageBufferRef, @@ -434,6 +440,12 @@ pub mod video_toolbox { impl_CFTypeDescription!(VTCompressionSession); impl VTCompressionSession { + /// Create a new compression session. + /// + /// # Safety + /// + /// The callback must be a valid function pointer. and the callback_data must be valid + /// in whatever terms that callback expects. pub unsafe fn new( width: usize, height: usize, @@ -465,6 +477,9 @@ pub mod video_toolbox { } } + /// # Safety + /// + /// The arguments to this function must be valid according to VTCompressionSessionEncodeFrame pub unsafe fn encode_frame( &self, buffer: CVImageBufferRef,