old-style-connect: Fix fixit bug when using Q_PRIVATE_SLOT

If the QObject has a signal with the same name of a Q_PRIVATE_SLOT
then the fixit would connect to the signal instead of the private
slot.

The fix is generic, if the user used the SLOT macro we simply
never rewrite to use a signal.

CCMAIL: dfaure@kdab.com

3 files changed, 35 insertions, 17 deletions

diff --git a/src/checks/level2/oldstyleconnect.cpp b/src/checks/level2/oldstyleconnect.cpp
index 18d1743b..1a4fabef 100644
--- a/src/checks/level2/oldstyleconnect.cpp
+++ b/src/checks/level2/oldstyleconnect.cpp
@@ -30,6 +30,8 @@
 #include "FixItUtils.h"
 #include "ContextUtils.h"
 #include "QtUtils.h"
+#include "ClazyContext.h"
+#include "AccessSpecifierManager.h"
 
 #include <clang/AST/Decl.h>
 #include <clang/AST/DeclCXX.h>
@@ -74,6 +76,7 @@ OldStyleConnect::OldStyleConnect(const std::string &name, ClazyContext *context)
     : CheckBase(name, context)
 {
     enablePreProcessorCallbacks();
+    context->enableAccessSpecifierManager();
 }
 
 int OldStyleConnect::classifyConnect(FunctionDecl *connectFunc, CallExpr *connectCall)
@@ -175,7 +178,7 @@ bool OldStyleConnect::isPrivateSlot(const string &name) const
 
 void OldStyleConnect::VisitStmt(Stmt *s)
 {
-    CallExpr *call = dyn_cast<CallExpr>(s);
+    auto call = dyn_cast<CallExpr>(s);
     if (!call)
         return;
 
@@ -186,7 +189,7 @@ void OldStyleConnect::VisitStmt(Stmt *s)
     if (!function)
         return;
 
-    CXXMethodDecl *method = dyn_cast<CXXMethodDecl>(function);
+    auto method = dyn_cast<CXXMethodDecl>(function);
     if (!method)
         return;
 
@@ -331,12 +334,24 @@ vector<FixItHint> OldStyleConnect::fixits(int classification, CallExpr *call)
                              + methodName + " for record " + lastRecordDecl->getNameAsString();
                 queueManualFixitWarning(s, FixItConnects, msg);
                 return {};
+            } else {
+                AccessSpecifierManager *a = m_context->accessSpecifierManager;
+                if (!a)
+                    return {};
+
+                const bool isSignal = a->qtAccessSpecifierType(methods[0]) == QtAccessSpecifier_Signal;
+                if (isSignal && macroName == "SLOT") {
+                    // The method is actually a signal and the user used SLOT()
+                    // bail out with the fixing.
+                    string msg = string("Can't fix. SLOT macro used but method " + methodName + " is a signal");
+                    queueManualFixitWarning(s, FixItConnects, msg);
+                    return {};
+                }
             }
 
             auto methodDecl = methods[0];
-            if (methodDecl->isStatic()) {
+            if (methodDecl->isStatic())
                 return {};
-            }
 
             if (macroNum == 1) {
                 // Save the number of parameters of the signal. The slot should not have more arguments.
diff --git a/tests/old-style-connect/q_private_slot.cpp b/tests/old-style-connect/q_private_slot.cpp
index ef39ddc7..fa6028e4 100644
--- a/tests/old-style-connect/q_private_slot.cpp
+++ b/tests/old-style-connect/q_private_slot.cpp
@@ -7,11 +7,15 @@ class MyObj : public QObject
     {
         connect(this, SIGNAL(destroyed()), SLOT(privSlot()));
         connect(this, SIGNAL(destroyed()), this, SLOT(privSlot()));
+        connect(this, SIGNAL(destroyed()), this, SLOT(privSlot2()));
     }
 public:
     class Private;
     Private * d;
     Q_PRIVATE_SLOT(d, void privSlot())
+    Q_PRIVATE_SLOT(d, void privSlot2())
+signals:
+    void privSlot2(); // Signal with the same name as private slot
 };
 
 class MyObj::Private
@@ -25,14 +29,11 @@ public:
         q->connect(other, SIGNAL(destroyed()), SLOT(privSlot()));
     }
 
-
-
-
-
     void somePrivFunction();
 
 public Q_SLOTS:
     void privSlot();
+    void privSlot2();
 private:
     MyObj *q;
 };
diff --git a/tests/old-style-connect/q_private_slot.cpp.expected b/tests/old-style-connect/q_private_slot.cpp.expected
index 282a373d..1dadb61b 100644
--- a/tests/old-style-connect/q_private_slot.cpp.expected
+++ b/tests/old-style-connect/q_private_slot.cpp.expected
@@ -2,13 +2,15 @@ old-style-connect/q_private_slot.cpp:8:9: warning: Old Style Connect [-Wclazy-ol
 old-style-connect/q_private_slot.cpp:8:44: warning: FixIt failed, requires manual intervention:  Converting Q_PRIVATE_SLOTS not implemented yet
 old-style-connect/q_private_slot.cpp:9:9: warning: Old Style Connect [-Wclazy-old-style-connect]
 old-style-connect/q_private_slot.cpp:9:50: warning: FixIt failed, requires manual intervention:  Converting Q_PRIVATE_SLOTS not implemented yet
-old-style-connect/q_private_slot.cpp:22:9: warning: Old Style Connect [-Wclazy-old-style-connect]
-old-style-connect/q_private_slot.cpp:22:44: warning: FixIt failed, requires manual intervention:  Converting Q_PRIVATE_SLOTS not implemented yet
-old-style-connect/q_private_slot.cpp:23:9: warning: Old Style Connect [-Wclazy-old-style-connect]
-old-style-connect/q_private_slot.cpp:23:47: warning: FixIt failed, requires manual intervention:  Converting Q_PRIVATE_SLOTS not implemented yet
-old-style-connect/q_private_slot.cpp:25:9: warning: Old Style Connect [-Wclazy-old-style-connect]
-old-style-connect/q_private_slot.cpp:25:48: warning: FixIt failed, requires manual intervention:  Converting Q_PRIVATE_SLOTS not implemented yet
-old-style-connect/q_private_slot.cpp:43:5: warning: Old Style Connect [-Wclazy-old-style-connect]
-old-style-connect/q_private_slot.cpp:43:44: warning: FixIt failed, requires manual intervention:  Converting Q_PRIVATE_SLOTS not implemented yet
+old-style-connect/q_private_slot.cpp:10:9: warning: Old Style Connect [-Wclazy-old-style-connect]
+old-style-connect/q_private_slot.cpp:10:50: warning: FixIt failed, requires manual intervention:  Can't fix. SLOT macro used but method privSlot2 is a signal [-Wclazy-old-style-connect]
+old-style-connect/q_private_slot.cpp:26:9: warning: Old Style Connect [-Wclazy-old-style-connect]
+old-style-connect/q_private_slot.cpp:26:44: warning: FixIt failed, requires manual intervention:  Converting Q_PRIVATE_SLOTS not implemented yet
+old-style-connect/q_private_slot.cpp:27:9: warning: Old Style Connect [-Wclazy-old-style-connect]
+old-style-connect/q_private_slot.cpp:27:47: warning: FixIt failed, requires manual intervention:  Converting Q_PRIVATE_SLOTS not implemented yet
+old-style-connect/q_private_slot.cpp:29:9: warning: Old Style Connect [-Wclazy-old-style-connect]
+old-style-connect/q_private_slot.cpp:29:48: warning: FixIt failed, requires manual intervention:  Converting Q_PRIVATE_SLOTS not implemented yet
 old-style-connect/q_private_slot.cpp:44:5: warning: Old Style Connect [-Wclazy-old-style-connect]
-old-style-connect/q_private_slot.cpp:44:23: warning: FixIt failed, requires manual intervention:  No such method doesnt_exist in class QObject [-Wclazy-old-style-connect]
+old-style-connect/q_private_slot.cpp:44:44: warning: FixIt failed, requires manual intervention:  Converting Q_PRIVATE_SLOTS not implemented yet
+old-style-connect/q_private_slot.cpp:45:5: warning: Old Style Connect [-Wclazy-old-style-connect]
+old-style-connect/q_private_slot.cpp:45:23: warning: FixIt failed, requires manual intervention:  No such method doesnt_exist in class QObject [-Wclazy-old-style-connect]