refactor!: add MenuItemKind enum (#79)

* refactor!: add `MenuItemKind` enum

* remove as_any

.changes/menu-item-kind.md

@@ -0,0 +1,5 @@
+---
+"muda": "minor"
+---
+
+Removed `MenuItemType` enum and replaced with `MenuItemKind` enum. `Menu::items` and `Submenu::items` will now return `Vec<MenuItemKind>` instead of `Vec<Box<dyn MenuItemExt>>`

.changes/trait-helpers.md

@@ -1,5 +0,0 @@
----
-"muda": "patch"
----
-
-Add helper methods on `IsMenuItem` trait to make it easier to get the concrete type back.

src/items/check.rs

@@ -4,7 +4,7 @@
 
 use std::{cell::RefCell, rc::Rc};
 
-use crate::{accelerator::Accelerator, IsMenuItem, MenuItemType};
+use crate::{accelerator::Accelerator, IsMenuItem, MenuItemKind};
 
 /// A check menu item inside a [`Menu`] or [`Submenu`]
 /// and usually contains a text and a check mark or a similar toggle
@@ -16,16 +16,8 @@ use crate::{accelerator::Accelerator, IsMenuItem, MenuItemType};
 pub struct CheckMenuItem(pub(crate) Rc<RefCell<crate::platform_impl::MenuChild>>);
 
 unsafe impl IsMenuItem for CheckMenuItem {
-    fn type_(&self) -> MenuItemType {
+    fn kind(&self) -> MenuItemKind {
-        MenuItemType::Check
+        MenuItemKind::Check(self.clone())
-    }
-
-    fn as_any(&self) -> &(dyn std::any::Any + 'static) {
-        self
-    }
-
-    fn id(&self) -> u32 {
-        self.id()
     }
 }
 

src/items/icon.rs

@@ -7,7 +7,7 @@ use std::{cell::RefCell, rc::Rc};
 use crate::{
     accelerator::Accelerator,
     icon::{Icon, NativeIcon},
-    IsMenuItem, MenuItemType,
+    IsMenuItem, MenuItemKind,
 };
 
 /// An icon menu item inside a [`Menu`] or [`Submenu`]
@@ -19,16 +19,8 @@ use crate::{
 pub struct IconMenuItem(pub(crate) Rc<RefCell<crate::platform_impl::MenuChild>>);
 
 unsafe impl IsMenuItem for IconMenuItem {
-    fn type_(&self) -> MenuItemType {
+    fn kind(&self) -> MenuItemKind {
-        MenuItemType::Icon
+        MenuItemKind::Icon(self.clone())
-    }
-
-    fn as_any(&self) -> &(dyn std::any::Any + 'static) {
-        self
-    }
-
-    fn id(&self) -> u32 {
-        self.id()
     }
 }
 

src/items/normal.rs

@@ -1,6 +1,6 @@
 use std::{cell::RefCell, rc::Rc};
 
-use crate::{accelerator::Accelerator, IsMenuItem, MenuItemType};
+use crate::{accelerator::Accelerator, IsMenuItem, MenuItemKind};
 
 /// A menu item inside a [`Menu`] or [`Submenu`] and contains only text.
 ///
@@ -10,16 +10,8 @@ use crate::{accelerator::Accelerator, IsMenuItem, MenuItemType};
 pub struct MenuItem(pub(crate) Rc<RefCell<crate::platform_impl::MenuChild>>);
 
 unsafe impl IsMenuItem for MenuItem {
-    fn type_(&self) -> MenuItemType {
+    fn kind(&self) -> MenuItemKind {
-        MenuItemType::Normal
+        MenuItemKind::MenuItem(self.clone())
-    }
-
-    fn as_any(&self) -> &(dyn std::any::Any + 'static) {
-        self
-    }
-
-    fn id(&self) -> u32 {
-        self.id()
     }
 }
 

src/items/predefined.rs

@@ -6,24 +6,17 @@ use std::{cell::RefCell, rc::Rc};
 
 use crate::{
     accelerator::{Accelerator, CMD_OR_CTRL},
-    AboutMetadata, IsMenuItem, MenuItemType,
+    AboutMetadata, IsMenuItem, MenuItemKind,
 };
 use keyboard_types::{Code, Modifiers};
 
 /// A predefined (native) menu item which has a predfined behavior by the OS or by this crate.
+#[derive(Clone)]
 pub struct PredefinedMenuItem(pub(crate) Rc<RefCell<crate::platform_impl::MenuChild>>);
 
 unsafe impl IsMenuItem for PredefinedMenuItem {
-    fn type_(&self) -> MenuItemType {
+    fn kind(&self) -> MenuItemKind {
-        MenuItemType::Predefined
+        MenuItemKind::Predefined(self.clone())
-    }
-
-    fn as_any(&self) -> &(dyn std::any::Any + 'static) {
-        self
-    }
-
-    fn id(&self) -> u32 {
-        self.id()
     }
 }
 
@@ -165,7 +158,7 @@ impl PredefinedMenuItem {
         )))
     }
 
-    fn id(&self) -> u32 {
+    pub(crate) fn id(&self) -> u32 {
         self.0.borrow().id()
     }
 

src/items/submenu.rs

@@ -4,7 +4,7 @@
 
 use std::{cell::RefCell, rc::Rc};
 
-use crate::{util::AddOp, ContextMenu, IsMenuItem, MenuItemType, Position};
+use crate::{util::AddOp, ContextMenu, IsMenuItem, MenuItemKind, Position};
 
 /// A menu that can be added to a [`Menu`] or another [`Submenu`].
 ///
@@ -13,16 +13,8 @@ use crate::{util::AddOp, ContextMenu, IsMenuItem, MenuItemType, Position};
 pub struct Submenu(pub(crate) Rc<RefCell<crate::platform_impl::MenuChild>>);
 
 unsafe impl IsMenuItem for Submenu {
-    fn type_(&self) -> MenuItemType {
+    fn kind(&self) -> MenuItemKind {
-        MenuItemType::Submenu
+        MenuItemKind::Submenu(self.clone())
-    }
-
-    fn as_any(&self) -> &(dyn std::any::Any + 'static) {
-        self
-    }
-
-    fn id(&self) -> u32 {
-        self.id()
     }
 }
 
@@ -101,7 +93,7 @@ impl Submenu {
     }
 
     /// Returns a list of menu items that has been added to this submenu.
-    pub fn items(&self) -> Vec<Box<dyn IsMenuItem>> {
+    pub fn items(&self) -> Vec<MenuItemKind> {
         self.0.borrow().items()
     }
 

src/lib.rs

@@ -152,96 +152,136 @@ pub mod icon;
 
 /// An enumeration of all available menu types, useful to match against
 /// the items return from [`Menu::items`] or [`Submenu::items`]
-#[derive(Clone, Debug, PartialEq, Eq)]
+#[derive(Clone)]
-pub enum MenuItemType {
+pub enum MenuItemKind {
+    MenuItem(MenuItem),
+    Submenu(Submenu),
+    Predefined(PredefinedMenuItem),
+    Check(CheckMenuItem),
+    Icon(IconMenuItem),
+}
+
+impl MenuItemKind {
+    /// Returns the id associated with this menu entry
+    fn id(&self) -> u32 {
+        match self {
+            MenuItemKind::MenuItem(i) => i.id(),
+            MenuItemKind::Submenu(i) => i.id(),
+            MenuItemKind::Predefined(i) => i.id(),
+            MenuItemKind::Check(i) => i.id(),
+            MenuItemKind::Icon(i) => i.id(),
+        }
+    }
+
+    /// Casts this item to a [`MenuItem`], and returns `None` if it wasn't.
+    pub fn as_menuitem(&self) -> Option<&MenuItem> {
+        match self {
+            MenuItemKind::MenuItem(i) => Some(i),
+            _ => None,
+        }
+    }
+
+    /// Casts this item to a [`MenuItem`], and panics if it wasn't.
+    pub fn as_menuitem_unchecked(&self) -> &MenuItem {
+        match self {
+            MenuItemKind::MenuItem(i) => i,
+            _ => panic!("Not a MenuItem"),
+        }
+    }
+
+    /// Casts this item to a [`Submenu`], and returns `None` if it wasn't.
+    pub fn as_submenu(&self) -> Option<&Submenu> {
+        match self {
+            MenuItemKind::Submenu(i) => Some(i),
+            _ => None,
+        }
+    }
+
+    /// Casts this item to a [`Submenu`], and panics if it wasn't.
+    pub fn as_submenu_unchecked(&self) -> &Submenu {
+        match self {
+            MenuItemKind::Submenu(i) => i,
+            _ => panic!("Not a Submenu"),
+        }
+    }
+
+    /// Casts this item to a [`PredefinedMenuItem`], and returns `None` if it wasn't.
+    pub fn as_predefined_menuitem(&self) -> Option<&PredefinedMenuItem> {
+        match self {
+            MenuItemKind::Predefined(i) => Some(i),
+            _ => None,
+        }
+    }
+
+    /// Casts this item to a [`PredefinedMenuItem`], and panics if it wasn't.
+    pub fn as_predefined_menuitem_unchecked(&self) -> &PredefinedMenuItem {
+        match self {
+            MenuItemKind::Predefined(i) => i,
+            _ => panic!("Not a PredefinedMenuItem"),
+        }
+    }
+
+    /// Casts this item to a [`CheckMenuItem`], and returns `None` if it wasn't.
+    pub fn as_check_menuitem(&self) -> Option<&CheckMenuItem> {
+        match self {
+            MenuItemKind::Check(i) => Some(i),
+            _ => None,
+        }
+    }
+
+    /// Casts this item to a [`CheckMenuItem`], and panics if it wasn't.
+    pub fn as_check_menuitem_unchecked(&self) -> &CheckMenuItem {
+        match self {
+            MenuItemKind::Check(i) => i,
+            _ => panic!("Not a CheckMenuItem"),
+        }
+    }
+
+    /// Casts this item to a [`IconMenuItem`], and returns `None` if it wasn't.
+    pub fn as_icon_menuitem(&self) -> Option<&IconMenuItem> {
+        match self {
+            MenuItemKind::Icon(i) => Some(i),
+            _ => None,
+        }
+    }
+
+    /// Casts this item to a [`IconMenuItem`], and panics if it wasn't.
+    pub fn as_icon_menuitem_unchecked(&self) -> &IconMenuItem {
+        match self {
+            MenuItemKind::Icon(i) => i,
+            _ => panic!("Not an IconMenuItem"),
+        }
+    }
+}
+
+/// A trait that defines a generic item in a menu, which may be one of [`MenuItemKind`]
+///
+/// # Safety
+///
+/// This trait is ONLY meant to be implemented internally by the crate.
+pub unsafe trait IsMenuItem {
+    fn kind(&self) -> MenuItemKind;
+
+    fn id(&self) -> u32 {
+        self.kind().id()
+    }
+}
+
+#[derive(Debug, PartialEq, PartialOrd, Clone, Copy)]
+pub(crate) enum MenuItemType {
+    MenuItem,
     Submenu,
-    Normal,
     Predefined,
     Check,
     Icon,
 }
-
 impl Default for MenuItemType {
     fn default() -> Self {
-        Self::Normal
+        Self::MenuItem
-    }
-}
-
-/// A trait that defines a generic item in a menu, which may be one of [MenuItemType]
-///
-/// # Safety
-///
-/// This trait is ONLY meant to be implemented internally.
-// TODO(amrbashir): first person to replace this trait with an enum while keeping `Menu.append_items`
-// taking mix of types (`MenuItem`, `CheckMenuItem`, `Submenu`...etc) in the same call, gets a cookie.
-pub unsafe trait IsMenuItem {
-    /// Get the type of this menu entry
-    fn type_(&self) -> MenuItemType;
-
-    /// Casts this menu entry to [`Any`](std::any::Any).
-    ///
-    /// You can use this to get the concrete underlying type
-    /// when calling [`Menu::items`] or [`Submenu::items`]
-    /// by calling [`downcast_ref`](https://doc.rust-lang.org/std/any/trait.Any.html#method.downcast_ref-1)
-    ///
-    /// ## Example
-    ///
-    /// ```no_run
-    /// # use muda::{Submenu, MenuItem};
-    /// let submenu = Submenu::new("Submenu", true);
-    /// let item = MenuItem::new("Text", true, None);
-    /// submenu.append(&item);
-    /// // --snip--
-    /// let item = &submenu.items()[0];
-    /// let item = item.as_any().downcast_ref::<MenuItem>().unwrap();
-    /// item.set_text("New text")
-    /// ````
-    fn as_any(&self) -> &(dyn std::any::Any + 'static);
-
-    /// Returns the id associated with this menu entry
-    fn id(&self) -> u32;
-
-    /// Casts this item to a [`Submenu`], and returns `None` if it wasn't.
-    fn as_submenu(&self) -> Option<&Submenu> {
-        self.as_any().downcast_ref()
-    }
-
-    /// Casts this item to a [`Submenu`], and panics if it wasn't.
-    fn as_submenu_unchecked(&self) -> &Submenu {
-        self.as_any().downcast_ref().unwrap()
-    }
-
-    /// Casts this item to a [`MenuItem`], and returns `None` if it wasn't.
-    fn as_menuitem(&self) -> Option<&MenuItem> {
-        self.as_any().downcast_ref()
-    }
-
-    /// Casts this item to a [`MenuItem`], and panics if it wasn't.
-    fn as_menuitem_unchecked(&self) -> &MenuItem {
-        self.as_any().downcast_ref().unwrap()
-    }
-
-    /// Casts this item to a [`CheckMenuItem`], and returns `None` if it wasn't.
-    fn as_check_menuitem(&self) -> Option<&CheckMenuItem> {
-        self.as_any().downcast_ref()
-    }
-
-    /// Casts this item to a [`CheckMenuItem`], and panics if it wasn't.
-    fn as_check_menuitem_unchecked(&self) -> &CheckMenuItem {
-        self.as_any().downcast_ref().unwrap()
-    }
-
-    /// Casts this item to a [`IconMenuItem`], and returns `None` if it wasn't.
-    fn as_icon_menuitem(&self) -> Option<&IconMenuItem> {
-        self.as_any().downcast_ref()
-    }
-
-    /// Casts this item to a [`IconMenuItem`], and panics if it wasn't.
-    fn as_icon_menuitem_unchecked(&self) -> &IconMenuItem {
-        self.as_any().downcast_ref().unwrap()
     }
 }
 
+/// A helper trait with methods to help creating a context menu.
 pub trait ContextMenu {
     /// Get the popup [`HMENU`] for this menu.
     ///

src/menu.rs

@@ -4,7 +4,7 @@
 
 use std::{cell::RefCell, rc::Rc};
 
-use crate::{util::AddOp, ContextMenu, IsMenuItem, Position};
+use crate::{util::AddOp, ContextMenu, IsMenuItem, MenuItemKind, Position};
 
 /// A root menu that can be added to a Window on Windows and Linux
 /// and used as the app global menu on macOS.
@@ -117,7 +117,7 @@ impl Menu {
     }
 
     /// Returns a list of menu items that has been added to this menu.
-    pub fn items(&self) -> Vec<Box<dyn IsMenuItem>> {
+    pub fn items(&self) -> Vec<MenuItemKind> {
         self.0.borrow().items()
     }
 

src/platform_impl/gtk/mod.rs

@@ -12,7 +12,7 @@ use crate::{
     icon::{Icon, NativeIcon},
     items::*,
     util::{AddOp, Counter},
-    MenuEvent, MenuItemType, Position,
+    IsMenuItem, MenuEvent, MenuItemKind, MenuItemType, Position,
 };
 use accelerator::{from_gtk_mnemonic, parse_accelerator, to_gtk_mnemonic};
 use gtk::{prelude::*, Orientation};
@@ -29,9 +29,9 @@ macro_rules! return_if_predefined_item_not_supported {
     ($item:tt) => {
         let child = $item.child();
         let child_ = child.borrow();
-        match (&child_.type_, &child_.predefined_item_type) {
+        match (&child_.item_type, &child_.predefined_item_type) {
             (
-                crate::MenuItemType::Predefined,
+                MenuItemType::Predefined,
                 PredefinedMenuItemType::Separator
                 | PredefinedMenuItemType::Copy
                 | PredefinedMenuItemType::Cut
@@ -40,10 +40,10 @@ macro_rules! return_if_predefined_item_not_supported {
                 | PredefinedMenuItemType::About(_),
             ) => {}
             (
-                crate::MenuItemType::Submenu
+                MenuItemType::Submenu
-                | crate::MenuItemType::Normal
+                | MenuItemType::MenuItem
-                | crate::MenuItemType::Check
+                | MenuItemType::Check
-                | crate::MenuItemType::Icon,
+                | MenuItemType::Icon,
                 _,
             ) => {}
             _ => return Ok(()),
@@ -156,15 +156,12 @@ impl Menu {
             }
         };
 
-        if item.type_() == crate::MenuItemType::Submenu {
+        if let MenuItemKind::Submenu(i) = item.kind() {
-            let submenu = item.as_any().downcast_ref::<crate::Submenu>().unwrap();
+            let gtk_menus = i.0.borrow().gtk_menus.clone();
-            let gtk_menus = submenu.0.borrow().gtk_menus.clone();
 
             for (menu_id, _) in gtk_menus {
-                for item in submenu.items() {
+                for item in i.items() {
-                    submenu
+                    i.0.borrow_mut()
-                        .0
-                        .borrow_mut()
                         .remove_inner(item.as_ref(), false, Some(menu_id))?;
                 }
             }
@@ -203,10 +200,10 @@ impl Menu {
         Ok(())
     }
 
-    pub fn items(&self) -> Vec<Box<dyn crate::IsMenuItem>> {
+    pub fn items(&self) -> Vec<MenuItemKind> {
         self.children
             .iter()
-            .map(|c| c.borrow().boxed(c.clone()))
+            .map(|c| c.borrow().kind(c.clone()))
             .collect()
     }
 
@@ -353,7 +350,7 @@ impl Menu {
 #[derive(Debug, Default)]
 pub struct MenuChild {
     // shared fields between submenus and menu items
-    pub type_: MenuItemType,
+    item_type: MenuItemType,
     text: String,
     enabled: bool,
     id: u32,
@@ -389,7 +386,7 @@ impl MenuChild {
             enabled,
             accelerator,
             id: COUNTER.next(),
-            type_: MenuItemType::Normal,
+            item_type: MenuItemType::MenuItem,
             gtk_menu_items: Rc::new(RefCell::new(HashMap::new())),
             ..Default::default()
         }
@@ -401,7 +398,7 @@ impl MenuChild {
             enabled,
             id: COUNTER.next(),
             children: Some(Vec::new()),
-            type_: MenuItemType::Submenu,
+            item_type: MenuItemType::Submenu,
             gtk_menu: (COUNTER.next(), None),
             gtk_menu_items: Rc::new(RefCell::new(HashMap::new())),
             gtk_menus: HashMap::new(),
@@ -415,7 +412,7 @@ impl MenuChild {
             enabled: true,
             accelerator: item_type.accelerator(),
             id: COUNTER.next(),
-            type_: MenuItemType::Predefined,
+            item_type: MenuItemType::Predefined,
             predefined_item_type: item_type,
             gtk_menu_items: Rc::new(RefCell::new(HashMap::new())),
             ..Default::default()
@@ -434,7 +431,7 @@ impl MenuChild {
             checked: Rc::new(AtomicBool::new(checked)),
             accelerator,
             id: COUNTER.next(),
-            type_: MenuItemType::Check,
+            item_type: MenuItemType::Check,
             gtk_menu_items: Rc::new(RefCell::new(HashMap::new())),
             is_syncing_checked_state: Rc::new(AtomicBool::new(false)),
             ..Default::default()
@@ -453,7 +450,7 @@ impl MenuChild {
             icon,
             accelerator,
             id: COUNTER.next(),
-            type_: MenuItemType::Icon,
+            item_type: MenuItemType::Icon,
             gtk_menu_items: Rc::new(RefCell::new(HashMap::new())),
             is_syncing_checked_state: Rc::new(AtomicBool::new(false)),
             ..Default::default()
@@ -471,7 +468,7 @@ impl MenuChild {
             enabled,
             accelerator,
             id: COUNTER.next(),
-            type_: MenuItemType::Icon,
+            item_type: MenuItemType::Icon,
             gtk_menu_items: Rc::new(RefCell::new(HashMap::new())),
             is_syncing_checked_state: Rc::new(AtomicBool::new(false)),
             ..Default::default()
@@ -481,6 +478,10 @@ impl MenuChild {
 
 /// Shared methods
 impl MenuChild {
+    pub(crate) fn item_type(&self) -> MenuItemType {
+        self.item_type
+    }
+
     pub fn id(&self) -> u32 {
         self.id
     }
@@ -711,15 +712,12 @@ impl MenuChild {
             }
         };
 
-        if item.type_() == crate::MenuItemType::Submenu {
+        if let MenuItemKind::Submenu(i) = item.kind() {
-            let submenu = item.as_any().downcast_ref::<crate::Submenu>().unwrap();
+            let gtk_menus = i.0.borrow().gtk_menus.clone();
-            let gtk_menus = submenu.0.borrow().gtk_menus.clone();
 
             for (menu_id, _) in gtk_menus {
-                for item in submenu.items() {
+                for item in i.items() {
-                    submenu
+                    i.0.borrow_mut()
-                        .0
-                        .borrow_mut()
                         .remove_inner(item.as_ref(), false, Some(menu_id))?;
                 }
             }
@@ -761,12 +759,12 @@ impl MenuChild {
         Ok(())
     }
 
-    pub fn items(&self) -> Vec<Box<dyn crate::IsMenuItem>> {
+    pub fn items(&self) -> Vec<MenuItemKind> {
         self.children
             .as_ref()
             .unwrap()
             .iter()
-            .map(|c| c.borrow().boxed(c.clone()))
+            .map(|c| c.borrow().kind(c.clone()))
             .collect()
     }
 
@@ -1133,53 +1131,46 @@ impl MenuChild {
     }
 }
 
-impl dyn crate::IsMenuItem + '_ {
+impl MenuItemKind {
     fn make_gtk_menu_item(
         &self,
         menu_id: u32,
         accel_group: Option<&gtk::AccelGroup>,
         add_to_cache: bool,
     ) -> crate::Result<gtk::MenuItem> {
-        match self.type_() {
+        let mut child = self.child_mut();
-            MenuItemType::Submenu => self
+        match child.item_type() {
-                .as_any()
+            MenuItemType::Submenu => {
-                .downcast_ref::<Submenu>()
+                child.create_gtk_item_for_submenu(menu_id, accel_group, add_to_cache)
-                .unwrap()
+            }
-                .0
+            MenuItemType::MenuItem => {
-                .borrow_mut()
+                child.create_gtk_item_for_menu_item(menu_id, accel_group, add_to_cache)
-                .create_gtk_item_for_submenu(menu_id, accel_group, add_to_cache),
+            }
-            MenuItemType::Normal => self
+            MenuItemType::Predefined => {
-                .as_any()
+                child.create_gtk_item_for_predefined_menu_item(menu_id, accel_group, add_to_cache)
-                .downcast_ref::<MenuItem>()
+            }
-                .unwrap()
+            MenuItemType::Check => {
-                .0
+                child.create_gtk_item_for_check_menu_item(menu_id, accel_group, add_to_cache)
-                .borrow_mut()
+            }
-                .create_gtk_item_for_menu_item(menu_id, accel_group, add_to_cache),
+            MenuItemType::Icon => {
-            MenuItemType::Predefined => self
+                child.create_gtk_item_for_icon_menu_item(menu_id, accel_group, add_to_cache)
-                .as_any()
+            }
-                .downcast_ref::<PredefinedMenuItem>()
-                .unwrap()
-                .0
-                .borrow_mut()
-                .create_gtk_item_for_predefined_menu_item(menu_id, accel_group, add_to_cache),
-            MenuItemType::Check => self
-                .as_any()
-                .downcast_ref::<CheckMenuItem>()
-                .unwrap()
-                .0
-                .borrow_mut()
-                .create_gtk_item_for_check_menu_item(menu_id, accel_group, add_to_cache),
-            MenuItemType::Icon => self
-                .as_any()
-                .downcast_ref::<IconMenuItem>()
-                .unwrap()
-                .0
-                .borrow_mut()
-                .create_gtk_item_for_icon_menu_item(menu_id, accel_group, add_to_cache),
         }
     }
 }
 
+impl dyn IsMenuItem + '_ {
+    fn make_gtk_menu_item(
+        &self,
+        menu_id: u32,
+        accel_group: Option<&gtk::AccelGroup>,
+        add_to_cache: bool,
+    ) -> crate::Result<gtk::MenuItem> {
+        self.kind()
+            .make_gtk_menu_item(menu_id, accel_group, add_to_cache)
+    }
+}
+
 fn show_context_menu(
     gtk_menu: gtk::Menu,
     widget: &impl IsA<gtk::Widget>,

src/platform_impl/macos/mod.rs

@@ -28,7 +28,7 @@ use crate::{
     icon::{Icon, NativeIcon},
     items::*,
     util::{AddOp, Counter},
-    IsMenuItem, LogicalPosition, MenuEvent, MenuItemType, Position,
+    IsMenuItem, LogicalPosition, MenuEvent, MenuItemKind, MenuItemType, Position,
 };
 
 static COUNTER: Counter = Counter::new();
@@ -92,32 +92,16 @@ impl Menu {
     }
 
     pub fn remove(&self, item: &dyn crate::IsMenuItem) -> crate::Result<()> {
-        // get a list of instances of the specified NSMenuItem in this menu
+        // get a list of instances of the specified `NSMenuItem` in this menu
-        if let Some(ns_menu_items) = match item.type_() {
+        let child = match item.kind() {
-            MenuItemType::Submenu => {
+            MenuItemKind::Submenu(i) => i.0.clone(),
-                let submenu = item.as_any().downcast_ref::<Submenu>().unwrap();
+            MenuItemKind::MenuItem(i) => i.0.clone(),
-                submenu.0.borrow_mut()
+            MenuItemKind::Predefined(i) => i.0.clone(),
-            }
+            MenuItemKind::Check(i) => i.0.clone(),
-            MenuItemType::Normal => {
+            MenuItemKind::Icon(i) => i.0.clone(),
-                let menuitem = item.as_any().downcast_ref::<MenuItem>().unwrap();
+        };
-                menuitem.0.borrow_mut()
+        let mut child_ = child.borrow_mut();
-            }
+        if let Some(ns_menu_items) = child_.ns_menu_items.remove(&self.id) {
-            MenuItemType::Predefined => {
-                let menuitem = item.as_any().downcast_ref::<PredefinedMenuItem>().unwrap();
-                menuitem.0.borrow_mut()
-            }
-            MenuItemType::Check => {
-                let menuitem = item.as_any().downcast_ref::<CheckMenuItem>().unwrap();
-                menuitem.0.borrow_mut()
-            }
-            MenuItemType::Icon => {
-                let menuitem = item.as_any().downcast_ref::<IconMenuItem>().unwrap();
-                menuitem.0.borrow_mut()
-            }
-        }
-        .ns_menu_items
-        .remove(&self.id)
-        {
             // remove each NSMenuItem from the NSMenu
             unsafe {
                 for item in ns_menu_items {
@@ -137,11 +121,11 @@ impl Menu {
         Ok(())
     }
 
-    pub fn items(&self) -> Vec<Box<dyn crate::IsMenuItem>> {
+    pub fn items(&self) -> Vec<MenuItemKind> {
         self.children
             .borrow()
             .iter()
-            .map(|c| c.borrow().boxed(c.clone()))
+            .map(|c| c.borrow().kind(c.clone()))
             .collect()
     }
 
@@ -166,7 +150,7 @@ impl Menu {
 #[derive(Debug)]
 pub struct MenuChild {
     // shared fields between submenus and menu items
-    pub type_: MenuItemType,
+    item_type: MenuItemType,
     id: u32,
     text: String,
     enabled: bool,
@@ -195,7 +179,7 @@ pub struct MenuChild {
 impl Default for MenuChild {
     fn default() -> Self {
         Self {
-            type_: Default::default(),
+            item_type: Default::default(),
             id: Default::default(),
             text: Default::default(),
             enabled: Default::default(),
@@ -216,7 +200,7 @@ impl Default for MenuChild {
 impl MenuChild {
     pub fn new(text: &str, enabled: bool, accelerator: Option<Accelerator>) -> Self {
         Self {
-            type_: MenuItemType::Normal,
+            item_type: MenuItemType::MenuItem,
             text: strip_mnemonic(text),
             enabled,
             id: COUNTER.next(),
@@ -227,7 +211,7 @@ impl MenuChild {
 
     pub fn new_submenu(text: &str, enabled: bool) -> Self {
         Self {
-            type_: MenuItemType::Submenu,
+            item_type: MenuItemType::Submenu,
             text: strip_mnemonic(text),
             enabled,
             children: Some(Vec::new()),
@@ -260,7 +244,7 @@ impl MenuChild {
         let accelerator = item_type.accelerator();
 
         Self {
-            type_: MenuItemType::Predefined,
+            item_type: MenuItemType::Predefined,
             text,
             enabled: true,
             id: COUNTER.next(),
@@ -278,7 +262,7 @@ impl MenuChild {
         accelerator: Option<Accelerator>,
     ) -> Self {
         Self {
-            type_: MenuItemType::Check,
+            item_type: MenuItemType::Check,
             text: text.to_string(),
             enabled,
             id: COUNTER.next(),
@@ -295,7 +279,7 @@ impl MenuChild {
         accelerator: Option<Accelerator>,
     ) -> Self {
         Self {
-            type_: MenuItemType::Icon,
+            item_type: MenuItemType::Icon,
             text: text.to_string(),
             enabled,
             id: COUNTER.next(),
@@ -312,7 +296,7 @@ impl MenuChild {
         accelerator: Option<Accelerator>,
     ) -> Self {
         Self {
-            type_: MenuItemType::Icon,
+            item_type: MenuItemType::Icon,
             text: text.to_string(),
             enabled,
             id: COUNTER.next(),
@@ -325,6 +309,10 @@ impl MenuChild {
 
 /// Shared methods
 impl MenuChild {
+    pub(crate) fn item_type(&self) -> MenuItemType {
+        self.item_type
+    }
+
     pub fn id(&self) -> u32 {
         self.id
     }
@@ -516,12 +504,12 @@ impl MenuChild {
         Ok(())
     }
 
-    pub fn items(&self) -> Vec<Box<dyn crate::IsMenuItem>> {
+    pub fn items(&self) -> Vec<MenuItemKind> {
         self.children
             .as_ref()
             .unwrap()
             .iter()
-            .map(|c| c.borrow().boxed(c.clone()))
+            .map(|c| c.borrow().kind(c.clone()))
             .collect()
     }
 
@@ -714,9 +702,9 @@ impl MenuChild {
     }
 
     fn make_ns_item_for_menu(&mut self, menu_id: u32) -> crate::Result<*mut Object> {
-        match self.type_ {
+        match self.item_type {
             MenuItemType::Submenu => self.create_ns_item_for_submenu(menu_id),
-            MenuItemType::Normal => self.create_ns_item_for_menu_item(menu_id),
+            MenuItemType::MenuItem => self.create_ns_item_for_menu_item(menu_id),
             MenuItemType::Predefined => self.create_ns_item_for_predefined_menu_item(menu_id),
             MenuItemType::Check => self.create_ns_item_for_check_menu_item(menu_id),
             MenuItemType::Icon => self.create_ns_item_for_icon_menu_item(menu_id),
@@ -752,42 +740,15 @@ impl PredefinedMenuItemType {
 
 impl dyn IsMenuItem + '_ {
     fn make_ns_item_for_menu(&self, menu_id: u32) -> crate::Result<*mut Object> {
-        match self.type_() {
+        match self.kind() {
-            MenuItemType::Submenu => self
+            MenuItemKind::Submenu(i) => i.0.borrow_mut().create_ns_item_for_submenu(menu_id),
-                .as_any()
+            MenuItemKind::MenuItem(i) => i.0.borrow_mut().create_ns_item_for_menu_item(menu_id),
-                .downcast_ref::<Submenu>()
+            MenuItemKind::Predefined(i) => {
-                .unwrap()
+                i.0.borrow_mut()
-                .0
+                    .create_ns_item_for_predefined_menu_item(menu_id)
-                .borrow_mut()
+            }
-                .create_ns_item_for_submenu(menu_id),
+            MenuItemKind::Check(i) => i.0.borrow_mut().create_ns_item_for_check_menu_item(menu_id),
-            MenuItemType::Normal => self
+            MenuItemKind::Icon(i) => i.0.borrow_mut().create_ns_item_for_icon_menu_item(menu_id),
-                .as_any()
-                .downcast_ref::<MenuItem>()
-                .unwrap()
-                .0
-                .borrow_mut()
-                .create_ns_item_for_menu_item(menu_id),
-            MenuItemType::Predefined => self
-                .as_any()
-                .downcast_ref::<PredefinedMenuItem>()
-                .unwrap()
-                .0
-                .borrow_mut()
-                .create_ns_item_for_predefined_menu_item(menu_id),
-            MenuItemType::Check => self
-                .as_any()
-                .downcast_ref::<CheckMenuItem>()
-                .unwrap()
-                .0
-                .borrow_mut()
-                .create_ns_item_for_check_menu_item(menu_id),
-            MenuItemType::Icon => self
-                .as_any()
-                .downcast_ref::<IconMenuItem>()
-                .unwrap()
-                .0
-                .borrow_mut()
-                .create_ns_item_for_icon_menu_item(menu_id),
         }
     }
 }
@@ -895,7 +856,7 @@ extern "C" fn fire_menu_item_click(this: &Object, _: Sel, _item: id) {
             }
         }
 
-        if (*item).type_ == MenuItemType::Check {
+        if (*item).item_type == MenuItemType::Check {
             (*item).set_checked(!(*item).is_checked());
         }
 

src/platform_impl/mod.rs

@@ -12,58 +12,70 @@ mod platform;
 #[path = "macos/mod.rs"]
 mod platform;
 
-use std::{cell::RefCell, rc::Rc};
+use std::{
+    cell::{Ref, RefCell, RefMut},
+    rc::Rc,
+};
 
-use crate::{items::*, IsMenuItem, MenuItemType};
+use crate::{items::*, IsMenuItem, MenuItemKind, MenuItemType};
 
 pub(crate) use self::platform::*;
 
 impl dyn IsMenuItem + '_ {
     fn child(&self) -> Rc<RefCell<MenuChild>> {
-        match self.type_() {
+        match self.kind() {
-            MenuItemType::Submenu => self
+            MenuItemKind::MenuItem(i) => i.0,
-                .as_any()
+            MenuItemKind::Submenu(i) => i.0,
-                .downcast_ref::<crate::Submenu>()
+            MenuItemKind::Predefined(i) => i.0,
-                .unwrap()
+            MenuItemKind::Check(i) => i.0,
-                .0
+            MenuItemKind::Icon(i) => i.0,
-                .clone(),
-            MenuItemType::Normal => self
-                .as_any()
-                .downcast_ref::<crate::MenuItem>()
-                .unwrap()
-                .0
-                .clone(),
-            MenuItemType::Predefined => self
-                .as_any()
-                .downcast_ref::<crate::PredefinedMenuItem>()
-                .unwrap()
-                .0
-                .clone(),
-            MenuItemType::Check => self
-                .as_any()
-                .downcast_ref::<crate::CheckMenuItem>()
-                .unwrap()
-                .0
-                .clone(),
-            MenuItemType::Icon => self
-                .as_any()
-                .downcast_ref::<crate::IconMenuItem>()
-                .unwrap()
-                .0
-                .clone(),
         }
+        .clone()
     }
 }
 
 /// Internal utilities
 impl MenuChild {
-    fn boxed(&self, c: Rc<RefCell<MenuChild>>) -> Box<dyn IsMenuItem> {
+    fn kind(&self, c: Rc<RefCell<MenuChild>>) -> MenuItemKind {
-        match self.type_ {
+        match self.item_type() {
-            MenuItemType::Submenu => Box::new(Submenu(c)),
+            MenuItemType::Submenu => MenuItemKind::Submenu(Submenu(c)),
-            MenuItemType::Normal => Box::new(MenuItem(c)),
+            MenuItemType::MenuItem => MenuItemKind::MenuItem(MenuItem(c)),
-            MenuItemType::Predefined => Box::new(PredefinedMenuItem(c)),
+            MenuItemType::Predefined => MenuItemKind::Predefined(PredefinedMenuItem(c)),
-            MenuItemType::Check => Box::new(CheckMenuItem(c)),
+            MenuItemType::Check => MenuItemKind::Check(CheckMenuItem(c)),
-            MenuItemType::Icon => Box::new(IconMenuItem(c)),
+            MenuItemType::Icon => MenuItemKind::Icon(IconMenuItem(c)),
+        }
+    }
+}
+
+#[allow(unused)]
+impl MenuItemKind {
+    pub(crate) fn as_ref(&self) -> &dyn IsMenuItem {
+        match self {
+            MenuItemKind::MenuItem(i) => i,
+            MenuItemKind::Submenu(i) => i,
+            MenuItemKind::Predefined(i) => i,
+            MenuItemKind::Check(i) => i,
+            MenuItemKind::Icon(i) => i,
+        }
+    }
+
+    pub(crate) fn child(&self) -> Ref<MenuChild> {
+        match self {
+            MenuItemKind::MenuItem(i) => i.0.borrow(),
+            MenuItemKind::Submenu(i) => i.0.borrow(),
+            MenuItemKind::Predefined(i) => i.0.borrow(),
+            MenuItemKind::Check(i) => i.0.borrow(),
+            MenuItemKind::Icon(i) => i.0.borrow(),
+        }
+    }
+
+    pub(crate) fn child_mut(&self) -> RefMut<MenuChild> {
+        match self {
+            MenuItemKind::MenuItem(i) => i.0.borrow_mut(),
+            MenuItemKind::Submenu(i) => i.0.borrow_mut(),
+            MenuItemKind::Predefined(i) => i.0.borrow_mut(),
+            MenuItemKind::Check(i) => i.0.borrow_mut(),
+            MenuItemKind::Icon(i) => i.0.borrow_mut(),
         }
     }
 }

src/platform_impl/windows/mod.rs

@@ -13,8 +13,7 @@ use crate::{
     icon::{Icon, NativeIcon},
     items::PredefinedMenuItemType,
     util::{AddOp, Counter},
-    AboutMetadata, CheckMenuItem, IconMenuItem, IsMenuItem, MenuEvent, MenuItem, MenuItemType,
+    AboutMetadata, IsMenuItem, MenuEvent, MenuItemKind, MenuItemType, Position,
-    Position, PredefinedMenuItem, Submenu,
 };
 use std::{
     cell::{RefCell, RefMut},
@@ -49,19 +48,18 @@ type AccelWrapper = (HACCEL, HashMap<u32, Accel>);
 macro_rules! inner_menu_child_and_flags {
     ($item:ident) => {{
         let mut flags = 0;
-        let child = match $item.type_() {
+        let child = match $item.kind() {
-            MenuItemType::Submenu => {
+            MenuItemKind::Submenu(i) => {
                 flags |= MF_POPUP;
-                &$item.as_any().downcast_ref::<Submenu>().unwrap().0
+                i.0.clone()
             }
-            MenuItemType::Normal => {
+            MenuItemKind::MenuItem(i) => {
                 flags |= MF_STRING;
-                &$item.as_any().downcast_ref::<MenuItem>().unwrap().0
+                i.0.clone()
             }
 
-            MenuItemType::Predefined => {
+            MenuItemKind::Predefined(i) => {
-                let item = $item.as_any().downcast_ref::<PredefinedMenuItem>().unwrap();
+                let child = i.0.clone();
-                let child = &item.0;
                 let child_ = child.borrow();
                 match child_.predefined_item_type {
                     PredefinedMenuItemType::None => return Ok(()),
@@ -72,24 +70,24 @@ macro_rules! inner_menu_child_and_flags {
                         flags |= MF_STRING;
                     }
                 }
+                drop(child_);
                 child
             }
-            MenuItemType::Check => {
+            MenuItemKind::Check(i) => {
-                let item = $item.as_any().downcast_ref::<CheckMenuItem>().unwrap();
+                let child = i.0.clone();
-                let child = &item.0;
                 flags |= MF_STRING;
                 if child.borrow().checked {
                     flags |= MF_CHECKED;
                 }
                 child
             }
-            MenuItemType::Icon => {
+            MenuItemKind::Icon(i) => {
                 flags |= MF_STRING;
-                &$item.as_any().downcast_ref::<IconMenuItem>().unwrap().0
+                i.0.clone()
             }
         };
 
-        (child.clone(), flags)
+        (child, flags)
     }};
 }
 
@@ -184,7 +182,7 @@ impl Menu {
         {
             let child_ = child.borrow();
 
-            if child_.type_ == MenuItemType::Icon {
+            if child_.item_type() == MenuItemType::Icon {
                 let hbitmap = child_
                     .icon
                     .as_ref()
@@ -253,10 +251,10 @@ impl Menu {
         Ok(())
     }
 
-    pub fn items(&self) -> Vec<Box<dyn IsMenuItem>> {
+    pub fn items(&self) -> Vec<MenuItemKind> {
         self.children
             .iter()
-            .map(|c| c.borrow().boxed(c.clone()))
+            .map(|c| c.borrow().kind(c.clone()))
             .collect()
     }
 
@@ -370,7 +368,7 @@ impl Menu {
 #[derive(Debug, Default)]
 pub(crate) struct MenuChild {
     // shared fields between submenus and menu items
-    pub type_: MenuItemType,
+    item_type: MenuItemType,
     text: String,
     enabled: bool,
     parents_hemnu: Vec<HMENU>,
@@ -399,7 +397,7 @@ pub(crate) struct MenuChild {
 impl MenuChild {
     pub fn new(text: &str, enabled: bool, accelerator: Option<Accelerator>) -> Self {
         Self {
-            type_: MenuItemType::Normal,
+            item_type: MenuItemType::MenuItem,
             text: text.to_string(),
             enabled,
             parents_hemnu: Vec::new(),
@@ -412,7 +410,7 @@ impl MenuChild {
 
     pub fn new_submenu(text: &str, enabled: bool) -> Self {
         Self {
-            type_: MenuItemType::Submenu,
+            item_type: MenuItemType::Submenu,
             text: text.to_string(),
             enabled,
             parents_hemnu: Vec::new(),
@@ -426,7 +424,7 @@ impl MenuChild {
 
     pub fn new_predefined(item_type: PredefinedMenuItemType, text: Option<String>) -> Self {
         Self {
-            type_: MenuItemType::Predefined,
+            item_type: MenuItemType::Predefined,
             text: text.unwrap_or_else(|| item_type.text().to_string()),
             enabled: true,
             parents_hemnu: Vec::new(),
@@ -445,7 +443,7 @@ impl MenuChild {
         accelerator: Option<Accelerator>,
     ) -> Self {
         Self {
-            type_: MenuItemType::Check,
+            item_type: MenuItemType::Check,
             text: text.to_string(),
             enabled,
             parents_hemnu: Vec::new(),
@@ -464,7 +462,7 @@ impl MenuChild {
         accelerator: Option<Accelerator>,
     ) -> Self {
         Self {
-            type_: MenuItemType::Icon,
+            item_type: MenuItemType::Icon,
             text: text.to_string(),
             enabled,
             parents_hemnu: Vec::new(),
@@ -483,7 +481,7 @@ impl MenuChild {
         accelerator: Option<Accelerator>,
     ) -> Self {
         Self {
-            type_: MenuItemType::Icon,
+            item_type: MenuItemType::Icon,
             text: text.to_string(),
             enabled,
             parents_hemnu: Vec::new(),
@@ -497,8 +495,12 @@ impl MenuChild {
 
 /// Shared methods
 impl MenuChild {
+    pub fn item_type(&self) -> MenuItemType {
+        self.item_type
+    }
+
     pub fn id(&self) -> u32 {
-        match self.type_ {
+        match self.item_type() {
             MenuItemType::Submenu => self.hmenu as u32,
             _ => self.id,
         }
@@ -705,7 +707,7 @@ impl MenuChild {
         {
             let child_ = child.borrow();
 
-            if child_.type_ == MenuItemType::Icon {
+            if child_.item_type() == MenuItemType::Icon {
                 let hbitmap = child_
                     .icon
                     .as_ref()
@@ -771,12 +773,12 @@ impl MenuChild {
         Ok(())
     }
 
-    pub fn items(&self) -> Vec<Box<dyn IsMenuItem>> {
+    pub fn items(&self) -> Vec<MenuItemKind> {
         self.children
             .as_ref()
             .unwrap()
             .iter()
-            .map(|c| c.borrow().boxed(c.clone()))
+            .map(|c| c.borrow().kind(c.clone()))
             .collect()
     }
 
@@ -818,7 +820,7 @@ fn find_by_id(id: u32, children: &Vec<Rc<RefCell<MenuChild>>>) -> Option<Rc<RefC
             return Some(i.clone());
         }
 
-        if item.type_ == MenuItemType::Submenu {
+        if item.item_type() == MenuItemType::Submenu {
             if let Some(child) = item.find_by_id(id) {
                 return Some(child);
             }
@@ -933,11 +935,11 @@ unsafe extern "system" fn menu_subclass_proc(
             {
                 let mut item = item.borrow_mut();
 
-                if item.type_ == MenuItemType::Predefined {
+                if item.item_type() == MenuItemType::Predefined {
                     dispatch = false;
                 }
 
-                match item.type_ {
+                match item.item_type() {
                     MenuItemType::Check => {
                         let checked = !item.checked;
                         item.set_checked(checked);

src/platform_impl/windows/util.rs

@@ -134,6 +134,7 @@ pub unsafe fn hwnd_dpi(hwnd: HWND) -> u32 {
         // We are on Windows 10 Anniversary Update (1607) or later.
         match GetDpiForWindow(hwnd) {
             0 => BASE_DPI, // 0 is returned if hwnd is invalid
+            #[allow(clippy::unnecessary_cast)]
             dpi => dpi as u32,
         }
     } else if let Some(GetDpiForMonitor) = *GET_DPI_FOR_MONITOR {
@@ -145,6 +146,7 @@ pub unsafe fn hwnd_dpi(hwnd: HWND) -> u32 {
 
         let mut dpi_x = 0;
         let mut dpi_y = 0;
+        #[allow(clippy::unnecessary_cast)]
         if GetDpiForMonitor(monitor, MDT_EFFECTIVE_DPI, &mut dpi_x, &mut dpi_y) == S_OK {
             dpi_x as u32
         } else {