Switch Arc<Mutex<Keymap>> to Rc<RefCell<Keymap>>, a relic of the GPUI2 port.

Mikayla created

Make gpui pass clippy

Change summary

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.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(-)

Detailed changes

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<AppCell>);
 /// 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<TypeId, NewViewListener>,
     pub(crate) windows: SlotMap<WindowId, Option<Window>>,
-    pub(crate) keymap: Arc<Mutex<Keymap>>,
+    pub(crate) keymap: Rc<RefCell<Keymap>>,
     pub(crate) global_action_listeners:
         FxHashMap<TypeId, Vec<Rc<dyn Fn(&dyn Any, DispatchPhase, &mut Self)>>>,
     pending_effects: VecDeque<Effect>,
@@ -242,6 +242,7 @@ pub struct AppContext {
 }
 
 impl AppContext {
+    #[allow(clippy::new_ret_no_self)]
     pub(crate) fn new(
         platform: Rc<dyn Platform>,
         asset_source: Arc<dyn AssetSource>,
@@ -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<Item = KeyBinding>) {
-        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<Menu>) {
-        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

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
+        }
     }
 }
 

crates/gpui/src/color.rs 🔗

@@ -203,20 +203,16 @@ impl PartialEq for Hsla {
 
 impl PartialOrd for Hsla {
     fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
-        // 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)))
     }
 }
 

crates/gpui/src/element.rs 🔗

@@ -133,9 +133,7 @@ pub trait Render: 'static + Sized {
 }
 
 impl Render for () {
-    fn render(&mut self, _cx: &mut ViewContext<Self>) -> impl IntoElement {
-        ()
-    }
+    fn render(&mut self, _cx: &mut ViewContext<Self>) -> impl IntoElement {}
 }
 
 /// You can derive [`IntoElement`] on any type that implements this trait.

crates/gpui/src/elements/div.rs 🔗

@@ -1008,10 +1008,9 @@ pub(crate) type ActionListener = Box<dyn Fn(&dyn Any, DispatchPhase, &mut Window
 #[track_caller]
 pub fn div() -> 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))]

crates/gpui/src/elements/overlay.rs 🔗

@@ -222,7 +222,7 @@ impl OverlayPositionMode {
     ) -> (Point<Pixels>, Bounds<Pixels>) {
         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)
             }

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;

crates/gpui/src/geometry.rs 🔗

@@ -2020,13 +2020,13 @@ impl Eq for Pixels {}
 
 impl PartialOrd for Pixels {
     fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
-        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)
     }
 }
 

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;

crates/gpui/src/interactive.rs 🔗

@@ -343,7 +343,7 @@ impl ExternalPaths {
 
 impl Render for ExternalPaths {
     fn render(&mut self, _: &mut ViewContext<Self>) -> 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
     }
 }
 

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<FocusId, DispatchNodeId>,
     view_node_ids: FxHashMap<EntityId, DispatchNodeId>,
     keystroke_matchers: FxHashMap<SmallVec<[KeyContext; 4]>, KeystrokeMatcher>,
-    keymap: Arc<Mutex<Keymap>>,
+    keymap: Rc<RefCell<Keymap>>,
     action_registry: Rc<ActionRegistry>,
 }
 
@@ -96,7 +95,7 @@ pub(crate) struct DispatchActionListener {
 }
 
 impl DispatchTree {
-    pub fn new(keymap: Arc<Mutex<Keymap>>, action_registry: Rc<ActionRegistry>) -> Self {
+    pub fn new(keymap: Rc<RefCell<Keymap>>, action_registry: Rc<ActionRegistry>) -> Self {
         Self {
             node_stack: Vec::new(),
             context_stack: Vec::new(),
@@ -307,7 +306,7 @@ impl DispatchTree {
         action: &dyn Action,
         context_stack: &Vec<KeyContext>,
     ) -> Vec<KeyBinding> {
-        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::<TestAction>();
 
-        let keymap = Arc::new(Mutex::new(keymap));
+        let keymap = Rc::new(RefCell::new(keymap));
 
         let tree = DispatchTree::new(keymap, Rc::new(registry));
 

crates/gpui/src/keymap/keymap.rs → 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::{

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 ')'"))

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<Keystroke>,
-    keymap: Arc<Mutex<Keymap>>,
+    keymap: Rc<RefCell<Keymap>>,
     keymap_version: KeymapVersion,
 }
 
@@ -15,8 +14,8 @@ pub struct KeymatchResult {
 }
 
 impl KeystrokeMatcher {
-    pub fn new(keymap: Arc<Mutex<Keymap>>) -> Self {
-        let keymap_version = keymap.lock().version();
+    pub fn new(keymap: Rc<RefCell<Keymap>>) -> 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();

crates/gpui/src/keymap/mod.rs 🔗

@@ -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::*;

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,

crates/gpui/src/taffy.rs 🔗

@@ -12,22 +12,16 @@ use taffy::{
     Taffy,
 };
 
+type NodeMeasureFn =
+    Box<dyn FnMut(Size<Option<Pixels>>, Size<AvailableSpace>, &mut WindowContext) -> Size<Pixels>>;
+
 pub struct TaffyLayoutEngine {
     taffy: Taffy,
     styles: FxHashMap<LayoutId, Style>,
     children_to_parents: FxHashMap<LayoutId, LayoutId>,
     absolute_layout_bounds: FxHashMap<LayoutId, Bounds<Pixels>>,
     computed_layouts: FxHashSet<LayoutId>,
-    nodes_to_measure: FxHashMap<
-        LayoutId,
-        Box<
-            dyn FnMut(
-                Size<Option<Pixels>>,
-                Size<AvailableSpace>,
-                &mut WindowContext,
-            ) -> Size<Pixels>,
-        >,
-    >,
+    nodes_to_measure: FxHashMap<LayoutId, NodeMeasureFn>,
 }
 
 static EXPECT_MESSAGE: &str = "we should avoid taffy layout errors by construction if possible";

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()

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()
     }

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)?,
         }
 

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<Self> {
             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,