fix: more clang-tidy warnings (#6608)

* fix: readability-redundant-casting warnings in gtk

* fix: bugprone-move-forwarding-reference warnings in gtk

* fix: readability-redundant-casting warnings in qt

* fix: bugprone-switch-missing-default-case warnings in qt

* fix: readability-use-std-min-max warning in qt client

* fix: readability-static-accessed-through-instance warning in qt client

* fix: cppcoreguidelines-avoid-const-or-ref-data-members warning in qt client

* fix: readability-avoid-nested-conditional-operator warning in qt client

* fixup! fix: readability-use-std-min-max warning in qt client

* fix: readability-redundant-member-init warnings in gtk client

* fix: performance-avoid-endl warnings in gtk client

* chore: disable readability-qualified-auto

too many false warnings

* chore: disable cppcoreguidelines-avoid-const-or-ref-data-members

* chore: fix readability-duplicate-include warning in gtk client

* chore: fix modernize-use-nodiscard warning in gtk client

* chore: fix readability-convert-member-functions-to-static warning in gtk client

* fixup! fix: bugprone-move-forwarding-reference warnings in gtk

* chore: fix performance-enum-size warning in gtk client

* fix: cppcoreguidelines-prefer-member-initializer warning in gtk client

* fix: readability-identifier-naming warning in qt client

* Revert "chore: fix performance-enum-size warning in gtk client"

This reverts commit 5ce6b562f8.

* fix: readability-redundant-member-init warning in move tests

* fix: readability-redundant-casting warnings in tests

* fixup! fix: readability-identifier-naming warning in qt client

* fixup! fix: readability-avoid-nested-conditional-operator warning in qt client

* fix: readability-static-accessed-through-instance warning in qt client

* fix: readability-redundant-casting warning in watchdir tests

gtk/.clang-tidy

@@ -7,6 +7,7 @@ Checks: >
   -bugprone-narrowing-conversions,
   cert-*,
   cppcoreguidelines-*,
+  -cppcoreguidelines-avoid-const-or-ref-data-members,
   -cppcoreguidelines-avoid-magic-numbers,
   -cppcoreguidelines-avoid-non-const-global-variables,
   -cppcoreguidelines-narrowing-conversions,
@@ -20,6 +21,7 @@ Checks: >
   -readability-function-cognitive-complexity,
   -readability-identifier-length,
   -readability-magic-numbers,
+  -readability-qualified-auto,
   -readability-redundant-access-specifiers
 
 CheckOptions:

gtk/DetailsDialog.cc

@@ -1389,7 +1389,7 @@ void DetailsDialog::Impl::refreshPeerList(std::vector<tr_torrent*> const& torren
 
 void DetailsDialog::Impl::refreshWebseedList(std::vector<tr_torrent*> const& torrents)
 {
-    auto has_any_webseeds = bool{ false };
+    auto has_any_webseeds = false;
     auto& hash = webseed_hash_;
     auto const& store = webseed_store_;
 

gtk/DynamicPropertyStore.h

@@ -80,7 +80,7 @@ public:
     }
 
 private:
-    PropertyInfo const& get_property(PropertyType index) const noexcept
+    [[nodiscard]] PropertyInfo const& get_property(PropertyType index) const noexcept
     {
         auto const id = static_cast<PropertyIdType>(index);
         g_assert(id > 0);

gtk/FilterBar.cc

@@ -32,8 +32,6 @@
 
 #if GTKMM_CHECK_VERSION(4, 0, 0)
 #include <gtkmm/filterlistmodel.h>
-#else
-#include <gtkmm/treemodelfilter.h>
 #endif
 
 #include <fmt/core.h>
@@ -203,7 +201,7 @@ bool FilterBar::Impl::tracker_filter_model_update()
     /* Walk through all the torrents, tallying how many matches there are
      * for the various categories. Also make a sorted list of all tracker
      * hosts s.t. we can merge it with the existing list */
-    auto n_torrents = int{ 0 };
+    auto n_torrents = 0;
     auto site_infos = std::unordered_map<std::string /*site*/, site_info>{};
     for (auto i = 0U, count = torrents_model->get_n_items(); i < count; ++i)
     {

gtk/FilterListModel.hh

@@ -30,6 +30,13 @@ FilterListModel<ItemT>::FilterListModel(Glib::RefPtr<Gtk::TreeModel> const& mode
     : Gtk::TreeModelFilter(model)
     , matches_all_(filter->matches_all())
     , matches_none_(filter->matches_none())
+    , signal_changed_tag_{ filter->signal_changed().connect(
+          [this, filter](auto /*changes*/)
+          {
+              matches_all_ = filter->matches_all();
+              matches_none_ = filter->matches_none();
+              refilter();
+          }) }
 {
     static auto const& self_col = ItemT::get_columns().self;
 
@@ -53,14 +60,6 @@ FilterListModel<ItemT>::FilterListModel(Glib::RefPtr<Gtk::TreeModel> const& mode
 
     set_visible_func(filter_func);
 
-    signal_changed_tag_ = filter->signal_changed().connect(
-        [this, filter](auto /*changes*/)
-        {
-            matches_all_ = filter->matches_all();
-            matches_none_ = filter->matches_none();
-            refilter();
-        });
-
     signal_row_inserted().connect([this](auto const& path, auto const& /*iter*/)
                                   { signal_items_changed_.emit(path.front(), 0, 1); });
     signal_row_deleted().connect([this](auto const& path) { signal_items_changed_.emit(path.front(), 1, 0); });

gtk/HigWorkarea.h

@@ -5,7 +5,7 @@
 
 #pragma once
 
-auto inline constexpr GUI_PAD_SMALL = int{ 3 };
-auto inline constexpr GUI_PAD = int{ 6 };
-auto inline constexpr GUI_PAD_BIG = int{ 12 };
-auto inline constexpr GUI_PAD_LARGE = int{ 12 };
+auto inline constexpr GUI_PAD_SMALL = 3;
+auto inline constexpr GUI_PAD = 6;
+auto inline constexpr GUI_PAD_BIG = 12;
+auto inline constexpr GUI_PAD_LARGE = 12;

gtk/MessageLogWindow.cc

@@ -80,7 +80,7 @@ private:
 
     void scroll_to_bottom();
     void level_combo_changed_cb(Gtk::ComboBox* combo_box);
-    void level_combo_init(Gtk::ComboBox* level_combo) const;
+    static void level_combo_init(Gtk::ComboBox* level_combo);
 
     [[nodiscard]] bool is_pinned_to_new() const;
     [[nodiscard]] bool isRowVisible(Gtk::TreeModel::const_iterator const& iter) const;
@@ -164,7 +164,8 @@ void MessageLogWindow::Impl::scroll_to_bottom()
 *****
 ****/
 
-void MessageLogWindow::Impl::level_combo_init(Gtk::ComboBox* level_combo) const
+// static
+void MessageLogWindow::Impl::level_combo_init(Gtk::ComboBox* level_combo)
 {
     auto const pref_level = static_cast<tr_log_level>(gtr_pref_int_get(TR_KEY_message_level));
     auto const default_level = TR_LOG_INFO;

gtk/PrefsDialog.h

@@ -35,5 +35,5 @@ private:
     std::unique_ptr<Impl> const impl_;
 };
 
-auto inline constexpr MAIN_WINDOW_REFRESH_INTERVAL_SECONDS = int{ 2 };
-auto inline constexpr SECONDARY_WINDOW_REFRESH_INTERVAL_SECONDS = int{ 2 };
+auto inline constexpr MAIN_WINDOW_REFRESH_INTERVAL_SECONDS = 2;
+auto inline constexpr SECONDARY_WINDOW_REFRESH_INTERVAL_SECONDS = 2;

gtk/Session.cc

@@ -846,8 +846,7 @@ bool Session::Impl::add_file(Glib::RefPtr<Gio::File> const& file, bool do_start,
     else
     {
         tr_ctorFree(ctor);
-        std::cerr << fmt::format(_("Couldn't add torrent file '{path}'"), fmt::arg("path", file->get_parse_name()))
-                  << std::endl;
+        std::cerr << fmt::format(_("Couldn't add torrent file '{path}'"), fmt::arg("path", file->get_parse_name())) << '\n';
     }
 
     return handled;

gtk/StatsDialog.cc

@@ -20,7 +20,7 @@
 
 #include <memory>
 
-static auto constexpr TR_RESPONSE_RESET = int{ 1 };
+static auto constexpr TR_RESPONSE_RESET = 1;
 
 class StatsDialog::Impl
 {

gtk/Torrent.cc

@@ -140,15 +140,15 @@ public:
 
         std::string_view mime_type;
 
-        Storage have_unchecked = {};
-        Storage have_valid = {};
-        Storage left_until_done = {};
-        Storage size_when_done = {};
-        Storage total_size = {};
-        Storage uploaded_ever = {};
+        Storage have_unchecked;
+        Storage have_valid;
+        Storage left_until_done;
+        Storage size_when_done;
+        Storage total_size;
+        Storage uploaded_ever;
 
-        Speed speed_down = {};
-        Speed speed_up = {};
+        Speed speed_down;
+        Speed speed_up;
 
         size_t queue_position = {};
 
@@ -163,12 +163,12 @@ public:
         int active_peers_up = {};
         int error_code = {};
 
-        Percents activity_percent_done = {};
-        Percents metadata_percent_complete = {};
-        Percents percent_complete = {};
-        Percents percent_done = {};
-        Percents recheck_progress = {};
-        Percents seed_ratio_percent_done = {};
+        Percents activity_percent_done;
+        Percents metadata_percent_complete;
+        Percents percent_complete;
+        Percents percent_done;
+        Percents recheck_progress;
+        Percents seed_ratio_percent_done;
 
         uint16_t peers_connected = {};
         uint16_t peers_getting_from_us = {};

gtk/Utils.h

@@ -261,7 +261,7 @@ T* gtr_get_widget_derived(Glib::RefPtr<Gtk::Builder> const& builder, Glib::ustri
 template<typename F>
 void gtr_window_on_close(Gtk::Window& widget, F&& callback)
 {
-    auto bool_callback = [callback = std::move(callback)]() mutable -> bool
+    auto bool_callback = [callback = std::forward<F>(callback)]() mutable -> bool
     {
         if constexpr (std::is_same_v<void, std::invoke_result_t<decltype(callback)>>)
         {

qt/Application.cc

@@ -66,9 +66,9 @@ char const* getUsage()
            "  transmission [OPTIONS...] [torrent files]";
 }
 
-auto constexpr StatsRefreshIntervalMsec = int{ 3000 };
-auto constexpr SessionRefreshIntervalMsec = int{ 3000 };
-auto constexpr ModelRefreshIntervalMsec = int{ 3000 };
+auto constexpr StatsRefreshIntervalMsec = 3000;
+auto constexpr SessionRefreshIntervalMsec = 3000;
+auto constexpr ModelRefreshIntervalMsec = 3000;
 
 bool loadTranslation(QTranslator& translator, QString const& name, QLocale const& locale, QStringList const& search_directories)
 {
@@ -626,7 +626,7 @@ void Application::addWatchdirTorrent(QString const& filename) const
 
 void Application::addTorrent(AddData addme) const
 {
-    if (addme.type == addme.NONE)
+    if (addme.type == AddData::NONE)
     {
         return;
     }

qt/DetailsDialog.cc

@@ -759,14 +759,9 @@ void DetailsDialog::refreshUI()
     {
         auto latest = torrents[0]->lastActivity();
 
-        for (Torrent const* const t : torrents)
+        for (Torrent const* const tor : torrents)
         {
-            auto const dt = t->lastActivity();
-
-            if (latest < dt)
-            {
-                latest = dt;
-            }
+            latest = std::max(latest, tor->lastActivity());
         }
 
         auto const seconds = static_cast<int>(std::difftime(now, latest));

qt/FileTreeItem.cc

@@ -165,6 +165,9 @@ QVariant FileTreeItem::data(int column, int role) const
             }
 
             break;
+
+        default:
+            break;
         }
 
         break;
@@ -184,6 +187,9 @@ QVariant FileTreeItem::data(int column, int role) const
         }
 
         break;
+
+    default:
+        break;
     }
 
     return value;
@@ -191,8 +197,8 @@ QVariant FileTreeItem::data(int column, int role) const
 
 std::pair<uint64_t, uint64_t> FileTreeItem::get_subtree_wanted_size() const
 {
-    auto have = uint64_t{ is_wanted_ ? have_size_ : 0U };
-    auto total = uint64_t{ is_wanted_ ? total_size_ : 0U };
+    auto have = is_wanted_ ? have_size_ : 0U;
+    auto total = is_wanted_ ? total_size_ : 0U;
 
     for (auto const* const child : children_)
     {

qt/FileTreeView.cc

@@ -110,6 +110,9 @@ void FileTreeView::resizeEvent(QResizeEvent* event)
             item_texts << FileTreeItem::tr("Low") << FileTreeItem::tr("Normal") << FileTreeItem::tr("High")
                        << FileTreeItem::tr("Mixed");
             break;
+
+        default:
+            break;
         }
 
         int item_width = 0;
@@ -407,6 +410,9 @@ Qt::CheckState FileTreeView::getCumulativeCheckState(QModelIndexList const& indi
 
         case Qt::PartiallyChecked:
             return Qt::PartiallyChecked;
+
+        default:
+            break;
         }
 
         if (have_checked && have_unchecked)

qt/FilterBar.cc

@@ -309,6 +309,9 @@ void FilterBar::refreshPref(int key)
 
             break;
         }
+
+    default:
+        break;
     }
 }
 

qt/InteropObject.cc

@@ -22,7 +22,7 @@ bool InteropObject::PresentWindow() const
 // NOLINTNEXTLINE(readability-identifier-naming)
 bool InteropObject::AddMetainfo(QString const& metainfo) const
 {
-    if (auto addme = AddData(metainfo); addme.type != addme.NONE)
+    if (auto addme = AddData(metainfo); addme.type != AddData::NONE)
     {
         trApp->addTorrent(addme);
     }

qt/MakeDialog.cc

@@ -111,7 +111,7 @@ void MakeProgressDialog::onProgress()
     }
 
     // progress bar
-    auto progress = int{ 100 }; // [0..100]
+    auto progress = 100; // [0..100]
     if (!is_done)
     {
         auto const [current, total] = builder_.checksum_status();

qt/Prefs.cc

@@ -201,7 +201,7 @@ bool isValidUtf8(QByteArray const& byteArray)
 #endif
 }
 
-[[nodiscard]] constexpr auto pref_is_savable(int pref)
+[[nodiscard]] constexpr auto prefIsSavable(int pref)
 {
     switch (pref)
     {
@@ -332,7 +332,7 @@ Prefs::~Prefs()
 
     for (int i = 0; i < PREFS_COUNT; ++i)
     {
-        if (!pref_is_savable(i))
+        if (!prefIsSavable(i))
         {
             continue;
         }

qt/PrefsDialog.cc

@@ -83,7 +83,7 @@ public:
     }
 
 private:
-    QObject* const object_;
+    QObject* object_;
 };
 
 char const* const PreferenceWidget::PrefKey = "pref-key";
@@ -470,12 +470,25 @@ void PrefsDialog::portTestSetEnabled()
 
 void PrefsDialog::onPortTested(std::optional<bool> result, Session::PortTestIpProtocol ip_protocol)
 {
+    constexpr auto StatusFromResult = [](const std::optional<bool> result)
+    {
+        if (!result)
+        {
+            return PORT_TEST_ERROR;
+        }
+        if (!*result)
+        {
+            return PORT_TEST_CLOSED;
+        }
+        return PORT_TEST_OPEN;
+    };
+
     // Only update the UI if the current status is "checking", so that
     // we won't show the port test results for the old peer port if it
     // changed while we have port-test RPC call(s) in-flight.
     if (port_test_status_[ip_protocol] == PORT_TEST_CHECKING)
     {
-        port_test_status_[ip_protocol] = result ? (*result ? PORT_TEST_OPEN : PORT_TEST_CLOSED) : PORT_TEST_ERROR;
+        port_test_status_[ip_protocol] = StatusFromResult(result);
         updatePortStatusLabel();
     }
     portTestSetEnabled();

qt/Session.cc

@@ -233,6 +233,9 @@ void Session::updatePref(int key)
             case 2:
                 sessionSet(prefs_.getKey(key), QStringLiteral("required"));
                 break;
+
+            default:
+                break;
             }
 
             break;

qt/Torrent.cc

@@ -283,6 +283,9 @@ Torrent::fields_t Torrent::update(tr_quark const* keys, tr_variant const* const*
                     sitenames_ = std::vector<QString>{ std::begin(tmp), std::end(tmp) };
                     break;
                 }
+
+            default:
+                break;
             }
         }
     }

qt/TorrentFilter.cc

@@ -52,6 +52,9 @@ void TorrentFilter::onPrefChanged(int key)
     case Prefs::SORT_REVERSED:
         msec = FastMSec;
         break;
+
+    default:
+        break;
     }
 
     // if this pref change affects filtering, ensure that a refilter is queued

qt/TrackerDelegate.cc

@@ -24,7 +24,7 @@
 
 namespace
 {
-auto constexpr Spacing = int{ 6 };
+auto constexpr Spacing = 6;
 
 auto constexpr Margin = QSize{ 10, 10 };
 
@@ -257,6 +257,9 @@ QString TrackerDelegate::getText(TrackerInfo const& inf) const
             //: %1 is duration
             str += tr("Asking for more peers now… <small>%1</small>").arg(time_since(inf.st.last_announce_start_time));
             break;
+
+        default:
+            break;
         }
 
         if (!show_more_)
@@ -305,9 +308,6 @@ QString TrackerDelegate::getText(TrackerInfo const& inf) const
 
         switch (inf.st.scrape_state)
         {
-        case TR_TRACKER_INACTIVE:
-            break;
-
         case TR_TRACKER_WAITING:
             str += QStringLiteral("<br/>\n");
             //: %1 is duration
@@ -324,6 +324,9 @@ QString TrackerDelegate::getText(TrackerInfo const& inf) const
             //: %1 is duration
             str += tr("Asking for peer counts now… <small>%1</small>").arg(time_since(inf.st.last_scrape_start_time));
             break;
+
+        default: // TR_TRACKER_INACTIVE
+            break;
         }
     }
 

qt/VariantHelpers.cc

@@ -45,7 +45,7 @@ bool change(TorrentHash& setme, tr_variant const* value)
 
 bool change(Peer& setme, tr_variant const* value)
 {
-    auto changed = bool{ false };
+    auto changed = false;
 
     auto pos = size_t{ 0 };
     auto key = tr_quark{};
@@ -85,7 +85,7 @@ bool change(Peer& setme, tr_variant const* value)
 
 bool change(TorrentFile& setme, tr_variant const* value)
 {
-    auto changed = bool{ false };
+    auto changed = false;
 
     auto pos = size_t{ 0 };
     auto key = tr_quark{};

tests/libtransmission/handshake-test.cc

@@ -52,7 +52,7 @@ using namespace std::literals;
 namespace libtransmission::test
 {
 
-auto constexpr MaxWaitMsec = int{ 5000 };
+auto constexpr MaxWaitMsec = 5000;
 
 class HandshakeTest : public SessionTest
 {

tests/libtransmission/move-test.cc

@@ -81,7 +81,7 @@ TEST_P(IncompleteDirTest, incompleteDir)
         tr_torrent* tor = {};
         tr_block_index_t block = {};
         tr_piece_index_t pieceIndex = {};
-        std::unique_ptr<Cache::BlockData> buf = {};
+        std::unique_ptr<Cache::BlockData> buf;
         bool done = {};
     };
 
@@ -165,7 +165,7 @@ TEST_F(MoveTest, setLocation)
     EXPECT_EQ(0, tr_torrentStat(tor)->leftUntilDone);
 
     // now move it
-    auto state = int{ -1 };
+    auto state = -1;
     tr_torrentSetLocation(tor, target_dir, true, &state);
     auto test = [&state]()
     {

tests/libtransmission/torrent-magnet-test.cc

@@ -27,7 +27,7 @@ TEST_F(TorrentMagnetTest, getMetadataPiece)
     auto benc = std::string{
         "d8:announce31:http://www.example.com/announce10:created by25:Transmission/2.61 (13407)13:creation datei1358704075e8:encoding5:UTF-84:info"
     };
-    auto piece = int{ 0 };
+    auto piece = 0;
     auto info_dict_size = size_t{ 0U };
     for (;;)
     {

tests/libtransmission/variant-test.cc

@@ -463,9 +463,9 @@ TEST_F(VariantTest, boolAndIntRecast)
 TEST_F(VariantTest, dictFindType)
 {
     auto constexpr ExpectedStr = "this-is-a-string"sv;
-    auto constexpr ExpectedBool = bool{ true };
-    auto constexpr ExpectedInt = int{ 1234 };
-    auto constexpr ExpectedReal = double{ 0.3 };
+    auto constexpr ExpectedBool = true;
+    auto constexpr ExpectedInt = 1234;
+    auto constexpr ExpectedReal = 0.3;
 
     auto const key_bool = tr_quark_new("this-is-a-bool"sv);
     auto const key_real = tr_quark_new("this-is-a-real"sv);

tests/libtransmission/watchdir-test.cc

@@ -152,7 +152,7 @@ TEST_P(WatchDirTest, initialScan)
     // setup: start with an empty directory.
     // this block confirms that it's empty
     {
-        auto called = bool{ false };
+        auto called = false;
         auto callback = [&called](std::string_view /*dirname*/, std::string_view /*basename*/)
         {
             called = true;