From 4c6e3d0c591e53efc6fe259c63223ab77d8fd779 Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Sun, 9 May 2021 11:09:58 +0200
Subject: [PATCH] WindowServer: Fix a handful of clang-tidy warnings in the
 menu code

Some avoidable signed/unsigned mismatch, String move construction,
and functions that can be static constexpr.
---
 Userland/Services/WindowServer/Menu.cpp       |  4 ++--
 Userland/Services/WindowServer/Menu.h         | 24 +++++++++----------
 Userland/Services/WindowServer/MenuItem.h     |  4 ++--
 .../Services/WindowServer/MenuManager.cpp     |  2 +-
 4 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/Userland/Services/WindowServer/Menu.cpp b/Userland/Services/WindowServer/Menu.cpp
index 94792eb071..52faea1c48 100644
--- a/Userland/Services/WindowServer/Menu.cpp
+++ b/Userland/Services/WindowServer/Menu.cpp
@@ -153,7 +153,7 @@ Window& Menu::ensure_menu_window()
     return *m_menu_window;
 }
 
-int Menu::visible_item_count() const
+size_t Menu::visible_item_count() const
 {
     if (!is_scrollable())
         return m_items.size();
@@ -441,7 +441,7 @@ void Menu::event(Core::Event& event)
             VERIFY(new_index >= 0);
             VERIFY(new_index <= static_cast<int>(m_items.size()) - 1);
 
-            if (is_scrollable() && new_index >= (m_scroll_offset + visible_item_count()))
+            if (is_scrollable() && new_index >= (m_scroll_offset + static_cast<int>(visible_item_count())))
                 ++m_scroll_offset;
 
             set_hovered_index(new_index);
diff --git a/Userland/Services/WindowServer/Menu.h b/Userland/Services/WindowServer/Menu.h
index dcc72d8a27..90b0bd962c 100644
--- a/Userland/Services/WindowServer/Menu.h
+++ b/Userland/Services/WindowServer/Menu.h
@@ -37,9 +37,9 @@ public:
     u32 alt_shortcut_character() const { return m_alt_shortcut_character; }
 
     bool is_empty() const { return m_items.is_empty(); }
-    int item_count() const { return m_items.size(); }
-    const MenuItem& item(int index) const { return m_items.at(index); }
-    MenuItem& item(int index) { return m_items.at(index); }
+    size_t item_count() const { return m_items.size(); }
+    const MenuItem& item(size_t index) const { return m_items.at(index); }
+    MenuItem& item(size_t index) { return m_items.at(index); }
 
     MenuItem* item_by_identifier(unsigned identifier)
     {
@@ -56,7 +56,7 @@ public:
 
     void add_item(NonnullOwnPtr<MenuItem>);
 
-    String name() const { return m_name; }
+    String const& name() const { return m_name; }
 
     template<typename Callback>
     IterationDecision for_each_item(Callback callback)
@@ -77,18 +77,18 @@ public:
 
     Window* window_menu_of() { return m_window_menu_of; }
     void set_window_menu_of(Window& window) { m_window_menu_of = window; }
-    bool is_window_menu_open() { return m_is_window_menu_open; }
+    bool is_window_menu_open() const { return m_is_window_menu_open; }
     void set_window_menu_open(bool is_open) { m_is_window_menu_open = is_open; }
 
     bool activate_default();
 
     int content_width() const;
 
-    int item_height() const { return 22; }
-    int frame_thickness() const { return 2; }
-    int horizontal_padding() const { return left_padding() + right_padding(); }
-    int left_padding() const { return 14; }
-    int right_padding() const { return 14; }
+    static constexpr int item_height() { return 22; }
+    static constexpr int frame_thickness() { return 2; }
+    static constexpr int horizontal_padding() { return left_padding() + right_padding(); }
+    static constexpr int left_padding() { return 14; }
+    static constexpr int right_padding() { return 14; }
 
     void draw();
     const Gfx::Font& font() const;
@@ -127,10 +127,10 @@ private:
     virtual void event(Core::Event&) override;
 
     void handle_mouse_move_event(const MouseEvent&);
-    int visible_item_count() const;
+    size_t visible_item_count() const;
 
     int item_index_at(const Gfx::IntPoint&);
-    int padding_between_text_and_shortcut() const { return 50; }
+    static constexpr int padding_between_text_and_shortcut() { return 50; }
     void did_activate(MenuItem&, bool leave_menu_open);
     void update_for_new_hovered_item(bool make_input = false);
 
diff --git a/Userland/Services/WindowServer/MenuItem.h b/Userland/Services/WindowServer/MenuItem.h
index 8cf931864a..310e574274 100644
--- a/Userland/Services/WindowServer/MenuItem.h
+++ b/Userland/Services/WindowServer/MenuItem.h
@@ -42,10 +42,10 @@ public:
     void set_default(bool);
 
     String text() const { return m_text; }
-    void set_text(const String& text) { m_text = text; }
+    void set_text(String text) { m_text = move(text); }
 
     String shortcut_text() const { return m_shortcut_text; }
-    void set_shortcut_text(const String& text) { m_shortcut_text = text; }
+    void set_shortcut_text(String text) { m_shortcut_text = move(text); }
 
     void set_rect(const Gfx::IntRect& rect) { m_rect = rect; }
     Gfx::IntRect rect() const;
diff --git a/Userland/Services/WindowServer/MenuManager.cpp b/Userland/Services/WindowServer/MenuManager.cpp
index 9e2e78156f..a094c789e2 100644
--- a/Userland/Services/WindowServer/MenuManager.cpp
+++ b/Userland/Services/WindowServer/MenuManager.cpp
@@ -255,7 +255,7 @@ void MenuManager::close_menus(const Vector<Menu*>& menus)
 static void collect_menu_subtree(Menu& menu, Vector<Menu*>& menus)
 {
     menus.append(&menu);
-    for (int i = 0; i < menu.item_count(); ++i) {
+    for (size_t i = 0; i < menu.item_count(); ++i) {
         auto& item = menu.item(i);
         if (!item.is_submenu())
             continue;