Batch of clang-tidy fixes

.clang-format

+---
+BasedOnStyle: LLVM
+
+Language: Cpp
+IndentWidget: 4
+ColumnLimit: 120
+
+AlignTrailingcomments: false
+AllowAllConstructorInitializersOnNextLine: false
+AllowShortBlocksOnASingleLine: Empty
+AlwaysBreakTemplateDeclarations: Yes
+BraceWrapping:
+  AfterFunction: true
+  AfterNamespace: true
+  AfterStruct: true
+  AfterUnion: true
+  SplitEmptyFunction: false
+  SplitEmptyNamespace: false
+  SplitEmptyRecord: false
+BreakBeforeBraces: Mozilla
+BreakBeforeInheritanceComma: true
+BreakConstructorInitializers: BeforeComma
+BreakInheritanceList: BeforeComma
+BreakStringLiterals: true
+IndentWrappedFunctionNames: true
+...

.clang-tidy

@@ -2,14 +2,22 @@
 # readability-redundant-access-specifiers triggered by Q_SLOTS
 # readability-inconsistent-declaration-parameter-name trigered by generated Qt code
 # performance-no-automatic-move is triggered by constness of qstring_literal_tmp in QStringLiteral macro
+#
+# TODO make those pass:
+# readability-function-size - for now some tests and the generated code contains extremely long
+#                             functions, which should be split into smaller functions
 
-Checks: -*
+Checks: -*,
+        bugprone-*,
         clang-analyzer-*,
         -clang-analyzer-osx,
         -clang-analyzer-cplusplus.NewDeleteLeaks,
-        google-explicit-constructor,
-        google-global-names-in-headers,
-        hicpp-signed-bitwise,
+        google-*,
+        -google-build-using-namespace,
+        -google-readability-todo,
+        -google-runtime-references,
+        -google-readability-function-size,
+        -google-default-arguments,
         misc-*,
         -misc-definitions-in-headers,
         -*-non-private-member-variables-in-classes,
@@ -22,10 +30,31 @@ Checks: -*
         -readability-redundant-access-specifiers,
         -readability-implicit-bool-conversion,
         -readability-static-accessed-through-instance,
-        -readability-inconsistent-declaration-parameter-name
+        -readability-inconsistent-declaration-parameter-name,
+        -readability-magic-numbers,
+        -readability-make-member-function-const,
+        -readability-function-size
 AnalyzeTemporaryDtors: true
 CheckOptions:
     - key:  bugprone-assert-side-effects.AssertMacros
       value: 'Q_ASSERT'
     - key:  CheckFunctionCalls
       value: true
+    - key:  StringCompareLikeFuctions
+      value: QString::compare;QString::localeAwareCompare
+    - key: WarnOnSizeOfIntegerExpression
+      value: 1
+    - key: bugprone-dangling-handle.HandleClasses
+      value: std::string_view;QStringView
+    - key: IgnoreClassesWithAllMemberVariablesBeingPublic
+      value: true
+    - key: VectorLikeClasses
+      value: std::vector;QVector
+WarningsAsErrors: bugprone-*,
+                  clang-*,
+                  google-*,
+                  misc-*,
+                  performance-*,
+                  readability-*,
+                  -readability-magic-numbers,
+                  -readability-make-member-function-const

.gitlab-ci.yml

@@ -24,7 +24,7 @@ build_clazy_clang_tidy:
     - python3 -u $CI_TOOLING/helpers/prepare-dependencies.py --product $PRODUCT --project $PROJECT --branchGroup $BRANCH_GROUP --environment production --platform $PLATFORM --installTo $INSTALL_PREFIX
     - python3 -u $CI_TOOLING/helpers/configure-build.py --product $PRODUCT --project $PROJECT --branchGroup $BRANCH_GROUP --platform $PLATFORM --installTo $INSTALL_PREFIX
     - python3 -u $CI_TOOLING/helpers/compile-build.py --product $PRODUCT --project $PROJECT --branchGroup $BRANCH_GROUP --platform $PLATFORM --usingInstall $INSTALL_PREFIX
-    #- cd build && run-clang-tidy
+    - cd build && run-clang-tidy -j$(nproc) -q
   variables:
     PLATFORM: SUSEQt5.14
     BRANCH_GROUP: kf5-qt5

autotests/.clang-tidy

-# Same base checks as in top-level .clang-tidy, but with some additional
-# checks disabled to give us some leeway in autotests (e.g. no magic numbers)
-
-Checks: -*
-        clang-analyzer-*,
-        -clang-analyzer-osx,
-        -clang-analyzer-cplusplus.NewDeleteLeaks,
-        google-explicit-constructor,
-        google-global-names-in-headers,
-        hicpp-signed-bitwise,
-        misc-*,
-        -misc-definitions-in-headers,
-        -*-non-private-member-variables-in-classes,
-        performance-*,
-        -performance-no-automatic-move,
-        readability-*,
-        -readability-avoid-const-params-in-decls,
-        -readability-convert-member-functions-to-static,
-        -readability-else-after-return,
-        -readability-redundant-access-specifiers,
-        -readability-implicit-bool-conversion,
-        -readability-static-accessed-through-instance,
-        -readability-inconsistent-declaration-parameter-name,
-        -readability-magic-numbers
-AnalyzeTemporaryDtors: true
-CheckOptions:
-    - key:  bugprone-assert-side-effects.AssertMacros
-      value: 'Q_ASSERT'
-    - key:  CheckFunctionCalls
-      value: true

autotests/libs/autoincrementtest.cpp

@@ -92,7 +92,6 @@ void AutoIncrementTest::testItemAutoIncrement()
     Item newItem = job->item();
 
     QVERIFY(newItem.id() > lastId);
-    lastId = newItem.id();
 }
 
 void AutoIncrementTest::testCollectionAutoIncrement()
@@ -125,5 +124,4 @@ void AutoIncrementTest::testCollectionAutoIncrement()
     Collection newCollection = job->collection();
 
     QVERIFY(newCollection.id() > lastId);
-    lastId = newCollection.id();
 }

autotests/libs/entitytreemodeltest.cpp

@@ -124,7 +124,7 @@ private:
 
 };
 
-QModelIndex firstMatchedIndex(const QAbstractItemModel &model, const QString pattern)
+QModelIndex firstMatchedIndex(const QAbstractItemModel &model, const QString &pattern)
 {
     if (pattern.isEmpty()) {
         return {};

autotests/libs/fakeakonadiservercommand.cpp

@@ -57,8 +57,9 @@ bool FakeAkonadiServerCommand::isCollectionSignal(const QByteArray &signal) cons
 
 void FakeAkonadiServerCommand::connectForwardingSignals()
 {
-    for (int methodIndex = 0; methodIndex < metaObject()->methodCount(); ++methodIndex) {
-        const QMetaMethod mm = metaObject()->method(methodIndex);
+    const auto *mo = FakeAkonadiServerCommand::metaObject();
+    for (int methodIndex = 0; methodIndex < mo->methodCount(); ++methodIndex) {
+        const QMetaMethod mm = mo->method(methodIndex);
         QByteArray signature = mm.methodSignature();
         if (mm.methodType() == QMetaMethod::Signal) {
             if ((qobject_cast<TagModel*>(m_model) && isTagSignal(signature)) ||
@@ -66,7 +67,7 @@ void FakeAkonadiServerCommand::connectForwardingSignals()
             {
                 const int modelSlotIndex = m_model->metaObject()->indexOfSlot(signature.remove(0, 5).constData());
                 Q_ASSERT(modelSlotIndex >= 0);
-                metaObject()->connect(this, methodIndex, m_model, modelSlotIndex);
+                mo->connect(this, methodIndex, m_model, modelSlotIndex);
             }
         }
     }

autotests/libs/fakeakonadiservercommand.h

@@ -30,7 +30,7 @@
 
 class FakeServerData;
 
-class AKONADITESTFAKE_EXPORT FakeAkonadiServerCommand : public QObject
+class AKONADITESTFAKE_EXPORT FakeAkonadiServerCommand: public QObject
 {
     Q_OBJECT
 public:
@@ -43,9 +43,7 @@ public:
 
     FakeAkonadiServerCommand(Type type, FakeServerData *serverData);
 
-    virtual ~FakeAkonadiServerCommand()
-    {
-    }
+    virtual ~FakeAkonadiServerCommand() = default;
 
     Type respondTo() const
     {
@@ -112,9 +110,6 @@ public:
         : FakeAkonadiServerCommand(Notification, serverData)
     {
     }
-    virtual ~FakeMonitorCommand()
-    {
-    }
 };
 
 class AKONADITESTFAKE_EXPORT FakeCollectionMovedCommand : public FakeMonitorCommand
@@ -129,10 +124,6 @@ public:
     {
     }
 
-    ~FakeCollectionMovedCommand() override
-    {
-    }
-
     void doCommand() override;
 
 private:
@@ -152,10 +143,6 @@ public:
     {
     }
 
-    ~FakeCollectionAddedCommand() override
-    {
-    }
-
     void doCommand() override;
 
 private:
@@ -174,10 +161,6 @@ public:
     {
     }
 
-    ~FakeCollectionRemovedCommand() override
-    {
-    }
-
     void doCommand() override;
 
 private:
@@ -202,10 +185,6 @@ public:
     {
     }
 
-    ~FakeCollectionChangedCommand() override
-    {
-    }
-
     void doCommand() override;
 
 private:
@@ -226,10 +205,6 @@ public:
     {
     }
 
-    ~FakeItemMovedCommand() override
-    {
-    }
-
     void doCommand() override;
 
 private:
@@ -249,10 +224,6 @@ public:
     {
     }
 
-    ~FakeItemAddedCommand() override
-    {
-    }
-
     void doCommand() override;
 
 private:
@@ -271,10 +242,6 @@ public:
     {
     }
 
-    ~FakeItemRemovedCommand() override
-    {
-    }
-
     void doCommand() override;
 
 private:
@@ -294,10 +261,6 @@ public:
     {
     }
 
-    ~FakeItemChangedCommand() override
-    {
-    }
-
     void doCommand() override;
 
 private:
@@ -316,10 +279,6 @@ public:
     {
     }
 
-    ~FakeTagAddedCommand() override
-    {
-    }
-
     void doCommand() override;
 
 private:
@@ -338,10 +297,6 @@ public:
     {
     }
 
-    ~FakeTagChangedCommand() override
-    {
-    }
-
     void doCommand() override;
 
 private:
@@ -361,10 +316,6 @@ public:
     {
     }
 
-    ~FakeTagMovedCommand() override
-    {
-    }
-
     void doCommand() override;
 
 private:
@@ -384,10 +335,6 @@ public:
     {
     }
 
-    ~FakeTagRemovedCommand() override
-    {
-    }
-
     void doCommand() override;
 
 private:
@@ -419,10 +366,6 @@ public:
         m_parentTag = parentTag;
     }
 
-    ~FakeJobResponse() override
-    {
-    }
-
     void appendCollection(const Akonadi::Collection &collection)
     {
         m_collections.insert(collection.id(), collection);

autotests/libs/itemappendtest.cpp

@@ -108,10 +108,10 @@ void ItemAppendTest::testContent_data()
     QTest::newRow("utf8") << utf8string.toUtf8();
     QTest::newRow("newlines") << QByteArray("\nsome\n\nbreaked\ncontent\n\n");
     QByteArray b;
-    QTest::newRow("big") << b.fill('a', 1 << 20);
-    QTest::newRow("bignull") << b.fill('\0', 1 << 20);
-    QTest::newRow("bigcr") << b.fill('\r', 1 << 20);
-    QTest::newRow("biglf") << b.fill('\n', 1 << 20);
+    QTest::newRow("big") << b.fill('a', 1u << 20);
+    QTest::newRow("bignull") << b.fill('\0', 1U << 20);
+    QTest::newRow("bigcr") << b.fill('\r', 1U << 20);
+    QTest::newRow("biglf") << b.fill('\n', 1U << 20);
 }
 
 void ItemAppendTest::testContent()

autotests/libs/sharedvaluepooltest.cpp

@@ -45,7 +45,7 @@ private Q_SLOTS:
         Internal::SharedValuePool<QByteArray, QVector> pool;
 
         for (int i = 0; i < size; ++i) {
-            QByteArray b(10, (char)i);
+            QByteArray b(10, static_cast<char>(i));
             data.push_back(b);
             QCOMPARE(pool.sharedValue(b), b);
             QCOMPARE(pool.sharedValue(b), b);

autotests/libs/tagmodeltest.cpp

@@ -86,7 +86,7 @@ private:
     QByteArray m_sessionName;
 };
 
-QModelIndex firstMatchedIndex(const QAbstractItemModel &model, const QString pattern)
+QModelIndex firstMatchedIndex(const QAbstractItemModel &model, const QString &pattern)
 {
     if (pattern.isEmpty()) {
         return {};

autotests/libs/testresource/knutresource.cpp

@@ -224,9 +224,8 @@ void KnutResource::collectionChanged(const Akonadi::Collection &collection)
         return;
     }
 
-    Collection c(collection);
     QDomElement newElem;
-    newElem = XmlWriter::collectionToElement(c, mDocument.document());
+    newElem = XmlWriter::collectionToElement(collection, mDocument.document());
     // move all items/collections over to the new node
     const QDomNodeList children = oldElem.childNodes();
     const int numberOfChildren = children.count();
@@ -243,7 +242,7 @@ void KnutResource::collectionChanged(const Akonadi::Collection &collection)
     }
     oldElem.parentNode().replaceChild(newElem, oldElem);
     save();
-    changeCommitted(c);
+    changeCommitted(collection);
 }
 
 void KnutResource::collectionRemoved(const Akonadi::Collection &collection)
@@ -291,11 +290,10 @@ void KnutResource::itemChanged(const Akonadi::Item &item, const QSet<QByteArray>
         return;
     }
 
-    Item i(item);
-    const QDomElement newElem = XmlWriter::itemToElement(i, mDocument.document());
+    const QDomElement newElem = XmlWriter::itemToElement(item, mDocument.document());
     oldElem.parentNode().replaceChild(newElem, oldElem);
     save();
-    changeCommitted(i);
+    changeCommitted(item);
 }
 
 void KnutResource::itemRemoved(const Akonadi::Item &item)

autotests/libs/testrunner/config.cpp

@@ -24,7 +24,7 @@
 #include <QFileInfo>
 #include <QXmlStreamReader>
 
-Q_GLOBAL_STATIC(Config, globalConfig)
+Q_GLOBAL_STATIC(Config, globalConfig) // NOLINT(readability-redundant-member-init)
 
 Config::Config()
 {

autotests/private/protocoltest.cpp

@@ -226,7 +226,7 @@ void ProtocolTest::testFetchScope()
     QFETCH(QVector<QByteArray>, expectedPayloads);
 
     ItemFetchScope in;
-    for (int i = ItemFetchScope::CacheOnly; i <= ItemFetchScope::VirtReferences; i = i << 1) {
+    for (unsigned i = ItemFetchScope::CacheOnly; i <= ItemFetchScope::VirtReferences; i = i << 1) {
         QVERIFY(!in.fetch(static_cast<ItemFetchScope::FetchFlag>(i)));
     }
     QVERIFY(in.fetch(ItemFetchScope::None));

autotests/server/fakeclient.cpp

@@ -107,7 +107,7 @@ void FakeClient::readServerPart()
             expectedStream >> expectedTag;
             const auto expectedCommand = Protocol::deserialize(expectedStream.device());
             try {
-                while ((size_t)mSocket->bytesAvailable() < sizeof(qint64)) {
+                while (static_cast<size_t>(mSocket->bytesAvailable()) < sizeof(qint64)) {
                     Protocol::DataStream::waitForData(mSocket, 5000);
                 }
             } catch (const ProtocolException &e) {

autotests/server/fakeitemretrievalmanager.cpp

@@ -32,7 +32,7 @@ FakeItemRetrievalManager::FakeItemRetrievalManager()
 void FakeItemRetrievalManager::requestItemDelivery(ItemRetrievalRequest request)
 {
     QMetaObject::invokeMethod(this, [this, r = std::move(request)] {
-            Q_EMIT requestFinished({std::move(r)});
+            Q_EMIT requestFinished({r});
     }, Qt::QueuedConnection);
 }
 

autotests/server/handlertest.cpp

@@ -52,7 +52,7 @@
 using namespace Akonadi;
 using namespace Akonadi::Server;
 
-#define MAKE_CMD_ROW( command, class ) QTest::newRow(#command) << command << QByteArray(typeid(Akonadi::Server::class).name());
+#define MAKE_CMD_ROW( command, class ) QTest::newRow(#command) << (command) << QByteArray(typeid(Akonadi::Server::class).name());
 
 class HandlerTest : public QObject
 {

autotests/server/notificationsubscribertest.cpp

@@ -128,7 +128,7 @@ private Q_SLOTS:
         QTest::addColumn<bool>("accepted");
 
 #define EmptyList(T) (QVector<T>())
-#define List(T,x) (QVector<T>() << x)
+#define List(T,x) (QVector<T>() << (x))
 
         auto itemMsg = Protocol::ItemChangeNotificationPtr::create();
         itemMsg->setOperation(Protocol::ItemChangeNotification::Add);

autotests/shared/akrangestest.cpp

@@ -66,7 +66,7 @@ public:
     }
 };
 
-}
+} // namespace
 
 class AkRangesTest : public QObject
 {

src/agentbase/agentbase.cpp

@@ -1116,7 +1116,7 @@ WId AgentBase::winIdForDialogs() const
         return 0;
     }
 
-    const WId winid = (WId)reply.arguments().at(0).toLongLong();
+    const WId winid = static_cast<WId>(reply.arguments().at(0).toLongLong());
 
     return winid;
 }