refactor: common/Credentials (#4979)

Use full path in includes
Sort includes
Move anon namespace out of chatterino namespace
Use auto * where possible
Disable convert-member-function-to-static check for all member functions
Don't use else after return
Removed empty constructor
Replace use of `boost::variant` with `std::variant`

Co-authored-by: nerix <nerixdev@outlook.de>

CHANGELOG.md

@@ -63,6 +63,7 @@
 - Dev: Removed direct dependency on Qt 5 compatibility module. (#4906)
 - Dev: Refactor `Emoji`'s EmojiMap into a vector. (#4980)
 - Dev: Refactor `DebugCount` and add copy button to debug popup. (#4921)
+- Dev: Refactor `common/Credentials`. (#4979)
 - Dev: Changed lifetime of context menus. (#4924)
 - Dev: Refactor `ChannelView`, removing a bunch of clang-tidy warnings. (#4926)
 - Dev: Refactor `IrcMessageHandler`, removing a bunch of clang-tidy warnings & changing its public API. (#4927)

src/common/Credentials.cpp

@@ -1,13 +1,17 @@
-#include "Credentials.hpp"
+#include "common/Credentials.hpp"
 
 #include "debug/AssertInGuiThread.hpp"
 #include "singletons/Paths.hpp"
 #include "singletons/Settings.hpp"
 #include "util/CombinePath.hpp"
 #include "util/Overloaded.hpp"
+#include "util/Variant.hpp"
 
 #include <QJsonDocument>
 #include <QJsonObject>
+#include <QSaveFile>
+
+#include <variant>
 
 #ifndef NO_QTKEYCHAIN
 #    ifdef CMAKE_BUILD
@@ -20,8 +24,6 @@
 #        include "keychain.h"
 #    endif
 #endif
-#include <boost/variant.hpp>
-#include <QSaveFile>
 
 #define FORMAT_NAME                                                  \
     ([&] {                                                           \
@@ -29,152 +31,151 @@
         return QString("chatterino:%1:%2").arg(provider).arg(name_); \
     })()
 
-namespace chatterino {
-
 namespace {
-    bool useKeyring()
+
-    {
+using namespace chatterino;
+
+bool useKeyring()
+{
 #ifdef NO_QTKEYCHAIN
+    return false;
+#endif
+    if (getPaths()->isPortable())
+    {
         return false;
-#endif
+    }
-        if (getPaths()->isPortable())
+
-        {
-            return false;
-        }
-        else
-        {
 #ifdef Q_OS_LINUX
-            return getSettings()->useKeyring;
+    return getSettings()->useKeyring;
 #else
-            return true;
+    return true;
 #endif
-        }
+}
-    }
 
-    // Insecure storage:
+// Insecure storage:
-    QString insecurePath()
+QString insecurePath()
+{
+    return combinePath(getPaths()->settingsDirectory, "credentials.json");
+}
+
+QJsonDocument loadInsecure()
+{
+    QFile file(insecurePath());
+    file.open(QIODevice::ReadOnly);
+    return QJsonDocument::fromJson(file.readAll());
+}
+
+void storeInsecure(const QJsonDocument &doc)
+{
+    QSaveFile file(insecurePath());
+    file.open(QIODevice::WriteOnly);
+    file.write(doc.toJson());
+    file.commit();
+}
+
+QJsonDocument &insecureInstance()
+{
+    static auto store = loadInsecure();
+    return store;
+}
+
+void queueInsecureSave()
+{
+    static bool isQueued = false;
+
+    if (!isQueued)
     {
-        return combinePath(getPaths()->settingsDirectory, "credentials.json");
+        isQueued = true;
+        QTimer::singleShot(200, qApp, [] {
+            storeInsecure(insecureInstance());
+            isQueued = false;
+        });
     }
+}
 
-    QJsonDocument loadInsecure()
+// QKeychain runs jobs asyncronously, so we have to assure that set/erase
-    {
+// jobs gets executed in order.
-        QFile file(insecurePath());
+struct SetJob {
-        file.open(QIODevice::ReadOnly);
+    QString name;
-        return QJsonDocument::fromJson(file.readAll());
+    QString credential;
-    }
+};
 
-    void storeInsecure(const QJsonDocument &doc)
+struct EraseJob {
-    {
+    QString name;
-        QSaveFile file(insecurePath());
+};
-        file.open(QIODevice::WriteOnly);
-        file.write(doc.toJson());
-        file.commit();
-    }
 
-    QJsonDocument &insecureInstance()
+using Job = std::variant<SetJob, EraseJob>;
-    {
-        static auto store = loadInsecure();
-        return store;
-    }
 
-    void queueInsecureSave()
+std::queue<Job> &jobQueue()
-    {
+{
-        static bool isQueued = false;
+    static std::queue<Job> jobs;
+    return jobs;
+}
 
-        if (!isQueued)
+void runNextJob()
-        {
+{
-            isQueued = true;
-            QTimer::singleShot(200, qApp, [] {
-                storeInsecure(insecureInstance());
-                isQueued = false;
-            });
-        }
-    }
-
-    // QKeychain runs jobs asyncronously, so we have to assure that set/erase
-    // jobs gets executed in order.
-    struct SetJob {
-        QString name;
-        QString credential;
-    };
-
-    struct EraseJob {
-        QString name;
-    };
-
-    using Job = boost::variant<SetJob, EraseJob>;
-
-    static std::queue<Job> &jobQueue()
-    {
-        static std::queue<Job> jobs;
-        return jobs;
-    }
-
-    static void runNextJob()
-    {
 #ifndef NO_QTKEYCHAIN
-        auto &&queue = jobQueue();
+    auto &&queue = jobQueue();
 
-        if (!queue.empty())
+    if (!queue.empty())
-        {
-            // we were gonna use std::visit here but macos is shit
-
-            auto &&item = queue.front();
-
-            if (item.which() == 0)  // set job
-            {
-                auto set = boost::get<SetJob>(item);
-                auto job = new QKeychain::WritePasswordJob("chatterino");
-                job->setAutoDelete(true);
-                job->setKey(set.name);
-                job->setTextData(set.credential);
-                QObject::connect(job, &QKeychain::Job::finished, qApp,
-                                 [](auto) {
-                                     runNextJob();
-                                 });
-                job->start();
-            }
-            else  // erase job
-            {
-                auto erase = boost::get<EraseJob>(item);
-                auto job = new QKeychain::DeletePasswordJob("chatterino");
-                job->setAutoDelete(true);
-                job->setKey(erase.name);
-                QObject::connect(job, &QKeychain::Job::finished, qApp,
-                                 [](auto) {
-                                     runNextJob();
-                                 });
-                job->start();
-            }
-
-            queue.pop();
-        }
-#endif
-    }
-
-    static void queueJob(Job &&job)
     {
-        auto &&queue = jobQueue();
+        // we were gonna use std::visit here but macos is shit
 
-        queue.push(std::move(job));
+        auto &&item = queue.front();
-        if (queue.size() == 1)
+
-        {
+        std::visit(
-            runNextJob();
+            variant::Overloaded{
-        }
+                [](const SetJob &set) {
+                    auto *job = new QKeychain::WritePasswordJob("chatterino");
+                    job->setAutoDelete(true);
+                    job->setKey(set.name);
+                    job->setTextData(set.credential);
+                    QObject::connect(job, &QKeychain::Job::finished, qApp,
+                                     [](auto) {
+                                         runNextJob();
+                                     });
+                    job->start();
+                },
+                [](const EraseJob &erase) {
+                    auto *job = new QKeychain::DeletePasswordJob("chatterino");
+                    job->setAutoDelete(true);
+                    job->setKey(erase.name);
+                    QObject::connect(job, &QKeychain::Job::finished, qApp,
+                                     [](auto) {
+                                         runNextJob();
+                                     });
+                    job->start();
+                },
+            },
+            item);
+
+        queue.pop();
     }
+#endif
+}
+
+void queueJob(Job &&job)
+{
+    auto &&queue = jobQueue();
+
+    queue.push(std::move(job));
+    if (queue.size() == 1)
+    {
+        runNextJob();
+    }
+}
+
 }  // namespace
 
+namespace chatterino {
+
 Credentials &Credentials::instance()
 {
     static Credentials creds;
     return creds;
 }
 
-Credentials::Credentials()
+// NOLINTNEXTLINE(readability-convert-member-functions-to-static)
-{
-}
-
 void Credentials::get(const QString &provider, const QString &name_,
                       QObject *receiver,
                       std::function<void(const QString &)> &&onLoaded)
@@ -187,7 +188,7 @@ void Credentials::get(const QString &provider, const QString &name_,
     {
 #ifndef NO_QTKEYCHAIN
         // if NO_QTKEYCHAIN is set, then this code is never used either way
-        auto job = new QKeychain::ReadPasswordJob("chatterino");
+        auto *job = new QKeychain::ReadPasswordJob("chatterino");
         job->setAutoDelete(true);
         job->setKey(name);
         QObject::connect(
@@ -207,6 +208,7 @@ void Credentials::get(const QString &provider, const QString &name_,
     }
 }
 
+// NOLINTNEXTLINE(readability-convert-member-functions-to-static)
 void Credentials::set(const QString &provider, const QString &name_,
                       const QString &credential)
 {
@@ -233,6 +235,7 @@ void Credentials::set(const QString &provider, const QString &name_,
     }
 }
 
+// NOLINTNEXTLINE(readability-convert-member-functions-to-static)
 void Credentials::erase(const QString &provider, const QString &name_)
 {
     assertInGuiThread();

src/common/Credentials.hpp

@@ -19,7 +19,7 @@ public:
     void erase(const QString &provider, const QString &name);
 
 private:
-    Credentials();
+    Credentials() = default;
 };
 
 }  // namespace chatterino