From c2fb3933905e36f63b963f06095719ac2059bb7d Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 9 Sep 2020 09:24:39 -0500 Subject: [PATCH] chore: fix clang-tidy-11 warnings (#1436) * refactor: mark subclass' destructors as override. * refactor: use QUrl to parse announce URL strings. The prompt for this was to work around a clang-tidy issue where "char* host = nullptr;" triggers a "don't use varargs" warning, but on the other hand it's also terser / cleaner. * refactor: make the TorrentDelegate brushes const. * refactor: s/auto/auto const*/ where appropriate * chore: add some nonconst global warning exemptions * chore: turn off warnings in GTest * refactor: just disable the clang-tidy warning. Apparently a std::array::iterator is a T* on clang, since clang-tidy's readability warning says we should use 'auto*' instead of 'auto'. However adding that annotation fails on MSVC, where the is apparently _not_ a raw pointer. Since there's not a way to satisfy both of them at the same time, disable the warning. --- qt/Application.h | 2 +- qt/DetailsDialog.h | 2 +- qt/FileTreeModel.h | 2 +- qt/Formatter.cc | 1 + qt/IconCache.cc | 1 + qt/OptionsDialog.h | 2 +- qt/Prefs.cc | 88 ++++++++++++++++++------------- qt/Prefs.h | 4 +- qt/RelocateDialog.cc | 1 + qt/RpcClient.cc | 2 +- qt/RpcQueue.cc | 8 +-- qt/RpcQueue.h | 1 + qt/Session.h | 2 +- qt/TorrentDelegate.cc | 24 +++------ qt/TorrentDelegate.h | 12 ++--- qt/TorrentModel.h | 2 +- qt/TrackerDelegate.cc | 7 +-- tests/libtransmission/.clang-tidy | 1 + 18 files changed, 81 insertions(+), 81 deletions(-) diff --git a/qt/Application.h b/qt/Application.h index 77bd8ebca..55b20d890 100644 --- a/qt/Application.h +++ b/qt/Application.h @@ -34,7 +34,7 @@ class Application : public QApplication public: Application(int& argc, char** argv); - virtual ~Application(); + ~Application() override; void raise(); bool notifyApp(QString const& title, QString const& body) const; diff --git a/qt/DetailsDialog.h b/qt/DetailsDialog.h index 03414a82c..fb4fa6777 100644 --- a/qt/DetailsDialog.h +++ b/qt/DetailsDialog.h @@ -37,7 +37,7 @@ class DetailsDialog : public BaseDialog public: DetailsDialog(Session&, Prefs&, TorrentModel const&, QWidget* parent = nullptr); - virtual ~DetailsDialog(); + ~DetailsDialog() override; void setIds(torrent_ids_t const& ids); diff --git a/qt/FileTreeModel.h b/qt/FileTreeModel.h index 14074756f..2e6d35402 100644 --- a/qt/FileTreeModel.h +++ b/qt/FileTreeModel.h @@ -45,7 +45,7 @@ public: public: FileTreeModel(QObject* parent = nullptr, bool is_editable = true); - ~FileTreeModel(); + ~FileTreeModel() override; void setEditable(bool editable); diff --git a/qt/Formatter.cc b/qt/Formatter.cc index aa2012da6..9279af8e0 100644 --- a/qt/Formatter.cc +++ b/qt/Formatter.cc @@ -17,6 +17,7 @@ Formatter& Formatter::get() { + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) static auto& singleton = *new Formatter(); return singleton; } diff --git a/qt/IconCache.cc b/qt/IconCache.cc index f697e39d2..6f500f62c 100644 --- a/qt/IconCache.cc +++ b/qt/IconCache.cc @@ -35,6 +35,7 @@ IconCache& IconCache::get() { + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) static auto& singleton = *new IconCache(); return singleton; } diff --git a/qt/OptionsDialog.h b/qt/OptionsDialog.h index 0d8af5ee2..917e3925d 100644 --- a/qt/OptionsDialog.h +++ b/qt/OptionsDialog.h @@ -37,7 +37,7 @@ class OptionsDialog : public BaseDialog public: OptionsDialog(Session& session, Prefs const& prefs, AddData addme, QWidget* parent = nullptr); - virtual ~OptionsDialog(); + ~OptionsDialog() override; private: using mybins_t = QMap; diff --git a/qt/Prefs.cc b/qt/Prefs.cc index 87230194f..8501c649d 100644 --- a/qt/Prefs.cc +++ b/qt/Prefs.cc @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -125,34 +126,43 @@ std::array const Prefs::Items { UPLOAD_SLOTS_PER_TORRENT, TR_KEY_upload_slots_per_torrent, QVariant::Int } }; +namespace +{ + +auto const FilterModes = std::array, FilterMode::NUM_MODES> +{{ + { FilterMode::SHOW_ALL, "show-all" }, + { FilterMode::SHOW_ACTIVE, "show-active" }, + { FilterMode::SHOW_DOWNLOADING, "show-downloading" }, + { FilterMode::SHOW_SEEDING, "show-seeding" }, + { FilterMode::SHOW_PAUSED, "show-paused" }, + { FilterMode::SHOW_FINISHED, "show-finished" }, + { FilterMode::SHOW_VERIFYING, "show-verifying" }, + { FilterMode::SHOW_ERROR, "show-error" } +}}; + +auto const SortModes = std::array, SortMode::NUM_MODES> +{{ + { SortMode::SORT_BY_NAME, "sort-by-name" }, + { SortMode::SORT_BY_ACTIVITY, "sort-by-activity" }, + { SortMode::SORT_BY_AGE, "sort-by-age" }, + { SortMode::SORT_BY_ETA, "sort-by-eta" }, + { SortMode::SORT_BY_PROGRESS, "sort-by-progress" }, + { SortMode::SORT_BY_QUEUE, "sort-by-queue" }, + { SortMode::SORT_BY_RATIO, "sort-by-ratio" }, + { SortMode::SORT_BY_SIZE, "sort-by-size" }, + { SortMode::SORT_BY_STATE, "sort-by-state" }, + { SortMode::SORT_BY_ID, "sort-by-id" } +}}; + +} // namespace + /*** **** ***/ Prefs::Prefs(QString config_dir) : - config_dir_(std::move(config_dir)), - FilterModes{ - std::make_pair(FilterMode::SHOW_ALL, QStringLiteral("show-all")), - std::make_pair(FilterMode::SHOW_ACTIVE, QStringLiteral("show-active")), - std::make_pair(FilterMode::SHOW_DOWNLOADING, QStringLiteral("show-downloading")), - std::make_pair(FilterMode::SHOW_SEEDING, QStringLiteral("show-seeding")), - std::make_pair(FilterMode::SHOW_PAUSED, QStringLiteral("show-paused")), - std::make_pair(FilterMode::SHOW_FINISHED, QStringLiteral("show-finished")), - std::make_pair(FilterMode::SHOW_VERIFYING, QStringLiteral("show-verifying")), - std::make_pair(FilterMode::SHOW_ERROR, QStringLiteral("show-error")) - }, - SortModes{ - std::make_pair(SortMode::SORT_BY_NAME, QStringLiteral("sort-by-name")), - std::make_pair(SortMode::SORT_BY_ACTIVITY, QStringLiteral("sort-by-activity")), - std::make_pair(SortMode::SORT_BY_AGE, QStringLiteral("sort-by-age")), - std::make_pair(SortMode::SORT_BY_ETA, QStringLiteral("sort-by-eta")), - std::make_pair(SortMode::SORT_BY_PROGRESS, QStringLiteral("sort-by-progress")), - std::make_pair(SortMode::SORT_BY_QUEUE, QStringLiteral("sort-by-queue")), - std::make_pair(SortMode::SORT_BY_RATIO, QStringLiteral("sort-by-ratio")), - std::make_pair(SortMode::SORT_BY_SIZE, QStringLiteral("sort-by-size")), - std::make_pair(SortMode::SORT_BY_STATE, QStringLiteral("sort-by-state")), - std::make_pair(SortMode::SORT_BY_ID, QStringLiteral("sort-by-id")) - } + config_dir_(std::move(config_dir)) { static_assert(sizeof(Items) / sizeof(Items[0]) == PREFS_COUNT); @@ -191,12 +201,13 @@ Prefs::Prefs(QString config_dir) : case CustomVariantType::SortModeType: { - auto const value = getValue(b); + auto const value = getValue(b); if (value) { - auto test = [&value](auto const& item) { return item.second == *value; }; - auto it = std::find_if(std::cbegin(SortModes), std::cend(SortModes), test); - auto const& pair = it == std::end(SortModes) ? SortModes[0] : *it; + auto const test = [&value](auto const& item) { return item.second == *value; }; + // NOLINTNEXTLINE(readability-qualified-auto) + auto const it = std::find_if(std::cbegin(SortModes), std::cend(SortModes), test); + auto const& pair = it == std::end(SortModes) ? SortModes.front() : *it; values_[i] = QVariant::fromValue(SortMode(pair.first)); } } @@ -204,12 +215,13 @@ Prefs::Prefs(QString config_dir) : case CustomVariantType::FilterModeType: { - auto const value = getValue(b); + auto const value = getValue(b); if (value) { - auto test = [&value](auto const& item) { return item.second == *value; }; - auto it = std::find_if(std::cbegin(FilterModes), std::cend(FilterModes), test); - auto const& pair = it == std::end(FilterModes) ? FilterModes[0] : *it; + auto const test = [&value](auto const& item) { return item.second == *value; }; + // NOLINTNEXTLINE(readability-qualified-auto) + auto const it = std::find_if(std::cbegin(FilterModes), std::cend(FilterModes), test); + auto const& pair = it == std::end(FilterModes) ? FilterModes.front() : *it; values_[i] = QVariant::fromValue(FilterMode(pair.first)); } } @@ -289,9 +301,10 @@ Prefs::~Prefs() case CustomVariantType::SortModeType: { auto const mode = val.value().mode(); - auto test = [&mode](auto const& item) { return item.first == mode; }; - auto it = std::find_if(std::cbegin(SortModes), std::cend(SortModes), test); - auto const& pair = it == std::end(SortModes) ? SortModes[0] : *it; + auto const test = [&mode](auto const& item) { return item.first == mode; }; + // NOLINTNEXTLINE(readability-qualified-auto) + auto const it = std::find_if(std::cbegin(SortModes), std::cend(SortModes), test); + auto const& pair = it == std::end(SortModes) ? SortModes.front() : *it; dictAdd(¤t_settings, key, pair.second); break; } @@ -299,9 +312,10 @@ Prefs::~Prefs() case CustomVariantType::FilterModeType: { auto const mode = val.value().mode(); - auto test = [&mode](auto const& item) { return item.first == mode; }; - auto it = std::find_if(std::cbegin(FilterModes), std::cend(FilterModes), test); - auto const& pair = it == std::end(FilterModes) ? FilterModes[0] : *it; + auto const test = [&mode](auto const& item) { return item.first == mode; }; + // NOLINTNEXTLINE(readability-qualified-auto) + auto const it = std::find_if(std::cbegin(FilterModes), std::cend(FilterModes), test); + auto const& pair = it == std::end(FilterModes) ? FilterModes.front() : *it; dictAdd(¤t_settings, key, pair.second); break; } diff --git a/qt/Prefs.h b/qt/Prefs.h index b0e7a66e8..6e6450eac 100644 --- a/qt/Prefs.h +++ b/qt/Prefs.h @@ -134,7 +134,7 @@ public: public: Prefs(QString config_dir); - virtual ~Prefs(); + ~Prefs() override; bool isCore(int key) const { @@ -204,8 +204,6 @@ private: void set(int key, char const* value) = delete; QString const config_dir_; - std::array, FilterMode::NUM_MODES> const FilterModes; - std::array, SortMode::NUM_MODES> const SortModes; QSet temporary_prefs_; QVariant mutable values_[PREFS_COUNT]; diff --git a/qt/RelocateDialog.cc b/qt/RelocateDialog.cc index cce5700f9..d13327c6b 100644 --- a/qt/RelocateDialog.cc +++ b/qt/RelocateDialog.cc @@ -13,6 +13,7 @@ #include "Torrent.h" #include "TorrentModel.h" +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) bool RelocateDialog::move_flag = true; void RelocateDialog::onSetLocation() diff --git a/qt/RpcClient.cc b/qt/RpcClient.cc index d50af5bde..293d18110 100644 --- a/qt/RpcClient.cc +++ b/qt/RpcClient.cc @@ -144,7 +144,7 @@ void RpcClient::sendNetworkRequest(TrVariantPtr json, QFutureInterface promise_; QQueue> queue_; diff --git a/qt/Session.h b/qt/Session.h index cceb5ba9d..7ba4f55d8 100644 --- a/qt/Session.h +++ b/qt/Session.h @@ -41,7 +41,7 @@ class Session : public QObject public: Session(QString config_dir, Prefs& prefs); - virtual ~Session(); + ~Session() override; void stop(); void restart(); diff --git a/qt/TorrentDelegate.cc b/qt/TorrentDelegate.cc index ad627d798..cd95c60de 100644 --- a/qt/TorrentDelegate.cc +++ b/qt/TorrentDelegate.cc @@ -28,13 +28,6 @@ enum BAR_HEIGHT = 12 }; -QColor TorrentDelegate::green_brush; -QColor TorrentDelegate::blue_brush; -QColor TorrentDelegate::silver_brush; -QColor TorrentDelegate::green_back; -QColor TorrentDelegate::blue_back; -QColor TorrentDelegate::silver_back; - namespace { @@ -127,19 +120,16 @@ ItemLayout::ItemLayout(QString name_text, QString status_text, QString progress_ } // namespace TorrentDelegate::TorrentDelegate(QObject* parent) : - QStyledItemDelegate(parent) + QStyledItemDelegate{parent}, + blue_back{"lightgrey"}, + blue_brush{"steelblue"}, + green_back{"darkseagreen"}, + green_brush{"forestgreen"}, + silver_back{"grey"}, + silver_brush{"silver"} { progress_bar_style_.minimum = 0; progress_bar_style_.maximum = 1000; - - green_brush = QColor("forestgreen"); - green_back = QColor("darkseagreen"); - - blue_brush = QColor("steelblue"); - blue_back = QColor("lightgrey"); - - silver_brush = QColor("silver"); - silver_back = QColor("grey"); } /*** diff --git a/qt/TorrentDelegate.h b/qt/TorrentDelegate.h index f633e5506..ec58ca478 100644 --- a/qt/TorrentDelegate.h +++ b/qt/TorrentDelegate.h @@ -45,12 +45,12 @@ protected: static QString shortStatusString(Torrent const& tor); static QString shortTransferString(Torrent const& tor); - static QColor blue_brush; - static QColor green_brush; - static QColor silver_brush; - static QColor blue_back; - static QColor green_back; - static QColor silver_back; + QColor const blue_back; + QColor const blue_brush; + QColor const green_back; + QColor const green_brush; + QColor const silver_back; + QColor const silver_brush; mutable QStyleOptionProgressBar progress_bar_style_ = {}; diff --git a/qt/TorrentModel.h b/qt/TorrentModel.h index b8dbdf3bd..0e25ac6ff 100644 --- a/qt/TorrentModel.h +++ b/qt/TorrentModel.h @@ -38,7 +38,7 @@ public: }; explicit TorrentModel(Prefs const& prefs); - virtual ~TorrentModel() override; + ~TorrentModel() override; void clear(); bool hasTorrent(TorrentHash const& hash) const; diff --git a/qt/TrackerDelegate.cc b/qt/TrackerDelegate.cc index 2fab45a24..d01d7eb19 100644 --- a/qt/TrackerDelegate.cc +++ b/qt/TrackerDelegate.cc @@ -185,11 +185,8 @@ QString TrackerDelegate::getText(TrackerInfo const& inf) const // hostname str += inf.st.is_backup ? QStringLiteral("") : QStringLiteral(""); - char* host = nullptr; - int port = 0; - tr_urlParse(inf.st.announce.toUtf8().constData(), TR_BAD_SIZE, nullptr, &host, &port, nullptr); - str += QStringLiteral("%1:%2").arg(QString::fromUtf8(host)).arg(port); - tr_free(host); + auto const url = QUrl(inf.st.announce); + str += QStringLiteral("%1:%2").arg(url.host()).arg(url.port(80)); if (!key.isEmpty()) { diff --git a/tests/libtransmission/.clang-tidy b/tests/libtransmission/.clang-tidy index 2b8b6aafa..8aa8b0d5d 100644 --- a/tests/libtransmission/.clang-tidy +++ b/tests/libtransmission/.clang-tidy @@ -19,6 +19,7 @@ Checks: > -clang-diagnostic-sign-conversion, cppcoreguidelines-*, -cppcoreguidelines-avoid-magic-numbers, + -cppcoreguidelines-avoid-non-const-global-variables, -cppcoreguidelines-init-variables, -cppcoreguidelines-macro-usage, -cppcoreguidelines-narrowing-conversions,