Fix/suppress clang-tidy issues in recently-added code (GTK client) (#4485)

* Fix `readability-convert-member-functions-to-static` clang-tidy issues

* Fix `modernize-use-nodiscard` clang-tidy issues

* Fix `cppcoreguidelines-owning-memory` clang-tidy issues

* Fix `performance-unnecessary-value-param` clang-tidy issues

* Fix `cppcoreguidelines-pro-type-reinterpret-cast` clang-tidy issues

* Fix `bugprone-easily-swappable-parameters` clang-tidy issues

* Fix `readability-named-parameter` clang-tidy issues

* Fix `readability-inconsistent-declaration-parameter-name` clang-tidy issue

* Fix `readability-else-after-return` clang-tidy issues

* Fix `cppcoreguidelines-special-member-functions` clang-tidy issues

* Fix `cppcoreguidelines-pro-type-vararg` clang-tidy issues

* Fix `cppcoreguidelines-pro-type-static-cast-downcast` clang-tidy issues

* Fix `cppcoreguidelines-pro-bounds-pointer-arithmetic` clang-tidy issues

gtk/FilterBar.cc

@@ -75,16 +75,10 @@ private:
     void update_filter_tracker();
     void update_filter_text();
 
-    Glib::RefPtr<Gtk::ListStore> activity_filter_model_new();
     bool activity_filter_model_update();
-    void status_model_update_count(Gtk::TreeModel::iterator const& iter, int n);
-    bool activity_is_it_a_separator(Gtk::TreeModel::const_iterator const& iter);
 
-    Glib::RefPtr<Gtk::TreeStore> tracker_filter_model_new();
     bool tracker_filter_model_update();
-    void tracker_model_update_count(Gtk::TreeModel::iterator const& iter, int n);
-    bool is_it_a_separator(Gtk::TreeModel::const_iterator const& iter);
-    void favicon_ready_cb(Glib::RefPtr<Gdk::Pixbuf> const& pixbuf, Gtk::TreeRowReference& reference);
+    void favicon_ready_cb(Glib::RefPtr<Gdk::Pixbuf> const& pixbuf, Gtk::TreeModel::Path const& path);
 
     void update_filter_models(Torrent::ChangeFlags changes);
     void update_filter_models_idle(Torrent::ChangeFlags changes);
@@ -92,6 +86,14 @@ private:
     void update_count_label_idle();
     bool update_count_label();
 
+    static Glib::RefPtr<Gtk::ListStore> activity_filter_model_new();
+    static void status_model_update_count(Gtk::TreeModel::iterator const& iter, int n);
+    static bool activity_is_it_a_separator(Gtk::TreeModel::const_iterator const& iter);
+
+    static Glib::RefPtr<Gtk::TreeStore> tracker_filter_model_new();
+    static void tracker_model_update_count(Gtk::TreeModel::iterator const& iter, int n);
+    static bool is_it_a_separator(Gtk::TreeModel::const_iterator const& iter);
+
     static Glib::ustring get_name_from_host(std::string const& host);
 
     static Gtk::CellRendererText* number_renderer_new();
@@ -172,14 +174,11 @@ void FilterBar::Impl::tracker_model_update_count(Gtk::TreeModel::iterator const&
     }
 }
 
-void FilterBar::Impl::favicon_ready_cb(Glib::RefPtr<Gdk::Pixbuf> const& pixbuf, Gtk::TreeRowReference& reference)
+void FilterBar::Impl::favicon_ready_cb(Glib::RefPtr<Gdk::Pixbuf> const& pixbuf, Gtk::TreeModel::Path const& path)
 {
     if (pixbuf != nullptr)
     {
-        auto const path = reference.get_path();
-        auto const model = reference.get_model();
-
-        if (auto const iter = model->get_iter(path); iter)
+        if (auto const iter = tracker_model_->get_iter(path); iter)
         {
             iter->set_value(tracker_filter_cols.pixbuf, pixbuf);
         }
@@ -305,9 +304,7 @@ bool FilterBar::Impl::tracker_filter_model_update()
             gtr_get_favicon(
                 core_->get_session(),
                 site.host,
-                [this, ref = Gtk::TreeRowReference(tracker_model_, path)](auto const& pixbuf) mutable
-                { favicon_ready_cb(pixbuf, ref); });
-            // ++iter;
+                [this, path](auto const& pixbuf) { favicon_ready_cb(pixbuf, path); });
             ++i;
         }
         else // update row
@@ -366,7 +363,7 @@ Gtk::CellRendererText* FilterBar::Impl::number_renderer_new()
 void FilterBar::Impl::tracker_combo_box_init(Gtk::ComboBox& combo)
 {
     combo.set_model(tracker_model_);
-    combo.set_row_separator_func(sigc::hide<0>(sigc::mem_fun(*this, &Impl::is_it_a_separator)));
+    combo.set_row_separator_func(sigc::hide<0>(&Impl::is_it_a_separator));
     combo.set_active(0);
 
     {
@@ -511,7 +508,7 @@ void FilterBar::Impl::render_activity_pixbuf_func(
 void FilterBar::Impl::activity_combo_box_init(Gtk::ComboBox& combo)
 {
     combo.set_model(activity_model_);
-    combo.set_row_separator_func(sigc::hide<0>(sigc::mem_fun(*this, &Impl::activity_is_it_a_separator)));
+    combo.set_row_separator_func(sigc::hide<0>(&Impl::activity_is_it_a_separator));
     combo.set_active(0);
 
     {

gtk/ListModelAdapter.cc

@@ -72,6 +72,7 @@ GType ListModelAdapter::get_column_type_vfunc(int index) const
     g_return_val_if_fail(index >= 0, G_TYPE_INVALID);
     g_return_val_if_fail(index < get_n_columns_vfunc(), G_TYPE_INVALID);
 
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
     return columns_.types()[index];
 }
 
@@ -202,17 +203,18 @@ std::optional<guint> ListModelAdapter::find_item_position_by_id(int item_id) con
     return item_position_it != item_positions_.end() ? std::make_optional(item_position_it->second) : std::nullopt;
 }
 
-void ListModelAdapter::adjust_item_positions(guint min_position, int delta)
+void ListModelAdapter::adjust_item_positions(guint min_position, PositionAdjustment adjustment)
 {
     for (auto item_it = std::next(items_.begin(), min_position); item_it != items_.end(); ++item_it)
     {
         if (auto const item_position_it = item_positions_.find(item_it->id); item_position_it != item_positions_.end())
         {
-            item_position_it->second += delta;
+            item_position_it->second += static_cast<int>(adjustment);
         }
     }
 }
 
+// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
 void ListModelAdapter::on_adaptee_items_changed(guint position, guint removed, guint added)
 {
     g_assert(position + removed <= items_.size());
@@ -227,7 +229,7 @@ void ListModelAdapter::on_adaptee_items_changed(guint position, guint removed, g
         info.notify_tag.disconnect();
 
         item_positions_.erase(info.id);
-        adjust_item_positions(removed_position, -1);
+        adjust_item_positions(removed_position, PositionAdjustment::DECREMENT);
 
         auto path = Path();
         path.push_back(removed_position);
@@ -247,7 +249,7 @@ void ListModelAdapter::on_adaptee_items_changed(guint position, guint removed, g
 
         items_.insert(std::next(items_.begin(), added_position), info);
 
-        adjust_item_positions(added_position, 1);
+        adjust_item_positions(added_position, PositionAdjustment::INCREMENT);
         item_positions_.emplace(info.id, added_position);
 
         auto path = Path();

gtk/ListModelAdapter.h

@@ -25,6 +25,12 @@ class ListModelAdapter
     using IdGetter = std::function<int(Glib::RefPtr<Glib::ObjectBase const> const&)>;
     using ValueGetter = std::function<void(Glib::RefPtr<Glib::ObjectBase const> const&, int, Glib::ValueBase&)>;
 
+    enum class PositionAdjustment
+    {
+        DECREMENT = -1,
+        INCREMENT = 1,
+    };
+
     struct ItemInfo
     {
         int id = 0;
@@ -64,7 +70,7 @@ private:
         ValueGetter value_getter);
 
     std::optional<guint> find_item_position_by_id(int item_id) const;
-    void adjust_item_positions(guint min_position, int delta);
+    void adjust_item_positions(guint min_position, PositionAdjustment adjustment);
 
     void on_adaptee_items_changed(guint position, guint removed, guint added);
     void on_adaptee_item_changed(Glib::RefPtr<Glib::ObjectBase const> const& item);

gtk/MainWindow.cc

@@ -107,7 +107,7 @@ private:
 
     Glib::RefPtr<Gio::MenuModel> createStatsMenu();
 
-    void on_popup_menu(double view_x, double view_y);
+    void on_popup_menu(double event_x, double event_y);
 
     void onSpeedToggled(std::string const& action_name, tr_direction dir, bool enabled);
     void onSpeedSet(tr_direction dir, int KBps);
@@ -755,12 +755,12 @@ Glib::RefPtr<Gtk::TreeSelection> MainWindow::Impl::get_selection() const
     return view_->get_selection();
 }
 
-void MainWindow::for_each_selected_torrent(std::function<void(Glib::RefPtr<Torrent> const&)> callback) const
+void MainWindow::for_each_selected_torrent(std::function<void(Glib::RefPtr<Torrent> const&)> const& callback) const
 {
     for_each_selected_torrent_until(sigc::bind_return(callback, false));
 }
 
-bool MainWindow::for_each_selected_torrent_until(std::function<bool(Glib::RefPtr<Torrent> const&)> callback) const
+bool MainWindow::for_each_selected_torrent_until(std::function<bool(Glib::RefPtr<Torrent> const&)> const& callback) const
 {
     static auto const& self_col = Torrent::get_columns().self;
 

gtk/MainWindow.h

@@ -35,8 +35,8 @@ public:
         Glib::RefPtr<Gio::ActionGroup> const& actions,
         Glib::RefPtr<Session> const& core);
 
-    void for_each_selected_torrent(std::function<void(Glib::RefPtr<Torrent> const&)> callback) const;
-    bool for_each_selected_torrent_until(std::function<bool(Glib::RefPtr<Torrent> const&)> callback) const;
+    void for_each_selected_torrent(std::function<void(Glib::RefPtr<Torrent> const&)> const& callback) const;
+    bool for_each_selected_torrent_until(std::function<bool(Glib::RefPtr<Torrent> const&)> const& callback) const;
 
     void select_all();
     void unselect_all();

gtk/Session.cc

@@ -89,6 +89,8 @@ public:
     Impl(Session& core, tr_session* session);
     ~Impl();
 
+    TR_DISABLE_COPY_MOVE(Impl)
+
     tr_session* close();
 
     Glib::RefPtr<Gio::ListStore<Torrent>> get_raw_model() const;
@@ -636,15 +638,14 @@ std::pair<Glib::RefPtr<Torrent>, guint> Session::Impl::find_torrent_by_id(tr_tor
     {
         auto const position = begin_position + (end_position - begin_position) / 2;
         auto const torrent = raw_model_->get_item(position);
+        auto const current_torrent_id = torrent->get_id();
 
-        if (auto const current_torrent_id = torrent->get_id(); current_torrent_id == torrent_id)
+        if (current_torrent_id == torrent_id)
         {
             return { torrent, position };
         }
-        else
-        {
-            (current_torrent_id < torrent_id ? begin_position : end_position) = position;
-        }
+
+        (current_torrent_id < torrent_id ? begin_position : end_position) = position;
     }
 
     return {};

gtk/Torrent.cc

@@ -25,7 +25,7 @@ namespace
 {
 
 template<typename T>
-Glib::Value<T>& column_value_cast(Glib::ValueBase& value, Gtk::TreeModelColumn<T> const&)
+Glib::Value<T>& column_value_cast(Glib::ValueBase& value, Gtk::TreeModelColumn<T> const& /*column*/)
 {
     return static_cast<Glib::Value<T>&>(value);
 }
@@ -167,15 +167,15 @@ public:
 
     void get_value(int column, Glib::ValueBase& value) const;
 
-    Glib::RefPtr<Gio::Icon> get_icon() const;
-    Glib::ustring get_short_status_text() const;
-    Glib::ustring get_long_progress_text() const;
-    Glib::ustring get_long_status_text() const;
+    [[nodiscard]] Glib::RefPtr<Gio::Icon> get_icon() const;
+    [[nodiscard]] Glib::ustring get_short_status_text() const;
+    [[nodiscard]] Glib::ustring get_long_progress_text() const;
+    [[nodiscard]] Glib::ustring get_long_status_text() const;
 
 private:
-    Glib::ustring get_short_transfer_text() const;
-    Glib::ustring get_error_text() const;
-    Glib::ustring get_activity_text() const;
+    [[nodiscard]] Glib::ustring get_short_transfer_text() const;
+    [[nodiscard]] Glib::ustring get_error_text() const;
+    [[nodiscard]] Glib::ustring get_activity_text() const;
 
 private:
     Torrent& torrent_;
@@ -732,6 +732,7 @@ Torrent::ChangeFlags Torrent::update()
 
 Glib::RefPtr<Torrent> Torrent::create(tr_torrent* torrent)
 {
+    // NOLINTNEXTLINE(cppcoreguidelines-owning-memory)
     return Glib::make_refptr_for_instance(new Torrent(torrent));
 }
 

gtk/TorrentFilter.cc

@@ -155,6 +155,7 @@ sigc::signal<void()>& TorrentFilter::signal_changed()
 
 Glib::RefPtr<TorrentFilter> TorrentFilter::create()
 {
+    // NOLINTNEXTLINE(cppcoreguidelines-owning-memory)
     return Glib::make_refptr_for_instance(new TorrentFilter());
 }
 

gtk/TorrentSorter.cc

@@ -262,6 +262,7 @@ sigc::signal<void()>& TorrentSorter::signal_changed()
 
 Glib::RefPtr<TorrentSorter> TorrentSorter::create()
 {
+    // NOLINTNEXTLINE(cppcoreguidelines-owning-memory)
     return Glib::make_refptr_for_instance(new TorrentSorter());
 }
 

gtk/Utils.cc

@@ -465,6 +465,7 @@ void object_signal_notify_callback(GObject* object, GParamSpec* /*param_spec*/,
     {
         if (auto const* const slot = Glib::SignalProxyBase::data_to_slot(data); slot != nullptr)
         {
+            // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast)
             (*static_cast<sigc::slot<TrObjectSignalNotifyCallback> const*>(slot))(Glib::wrap(object, true));
         }
     }
@@ -476,7 +477,9 @@ Glib::SignalProxy<TrObjectSignalNotifyCallback> gtr_object_signal_notify(Glib::O
 {
     static auto const object_signal_notify_info = Glib::SignalProxyInfo{
         .signal_name = "notify",
+        // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
         .callback = reinterpret_cast<GCallback>(&object_signal_notify_callback),
+        // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
         .notify_callback = reinterpret_cast<GCallback>(&object_signal_notify_callback),
     };
 
@@ -485,6 +488,7 @@ Glib::SignalProxy<TrObjectSignalNotifyCallback> gtr_object_signal_notify(Glib::O
 
 void gtr_object_notify_emit(Glib::ObjectBase& object)
 {
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg)
     g_signal_emit_by_name(object.gobj(), "notify", nullptr);
 }