Introducing overridden-signal

Warns when overriding a signal, which might make existing connects
not work, if done unintentionally.

Warns for:
- Overriding signal with non-signal
- Overriding non-signal with signal
- Overriding signal with signal

6 files changed, 171 insertions, 7 deletions

diff --git a/src/AccessSpecifierManager.cpp b/src/AccessSpecifierManager.cpp
index b3ee0ae9..eea709fa 100644
--- a/src/AccessSpecifierManager.cpp
+++ b/src/AccessSpecifierManager.cpp
@@ -130,10 +130,10 @@ ClazySpecifierList& AccessSpecifierManager::entryForClassDefinition(CXXRecordDec
     return specifiers;
 }
 
-CXXRecordDecl *AccessSpecifierManager::classDefinitionForLoc(SourceLocation loc) const
+const CXXRecordDecl *AccessSpecifierManager::classDefinitionForLoc(SourceLocation loc) const
 {
     for (const auto &it : m_specifiersMap) {
-        CXXRecordDecl *record = it.first;
+        const CXXRecordDecl *record = it.first;
         if (record->getLocStart() < loc && loc < record->getLocEnd())
             return record;
     }
@@ -172,7 +172,7 @@ void AccessSpecifierManager::VisitDeclaration(Decl *decl)
     }
 }
 
-QtAccessSpecifierType AccessSpecifierManager::qtAccessSpecifierType(CXXMethodDecl *method) const
+QtAccessSpecifierType AccessSpecifierManager::qtAccessSpecifierType(const CXXMethodDecl *method) const
 {
     if (!method || method->getLocStart().isMacroID())
         return QtAccessSpecifier_Unknown;
@@ -197,7 +197,7 @@ QtAccessSpecifierType AccessSpecifierManager::qtAccessSpecifierType(CXXMethodDec
 
     // Process Q_SLOTS and Q_SIGNALS:
 
-    CXXRecordDecl *record = method->getParent();
+    const CXXRecordDecl *record = method->getParent();
     if (!record || isa<clang::ClassTemplateSpecializationDecl>(record))
         return QtAccessSpecifier_None;
 
diff --git a/src/AccessSpecifierManager.h b/src/AccessSpecifierManager.h
index c4e4a9a4..44f4ce48 100644
--- a/src/AccessSpecifierManager.h
+++ b/src/AccessSpecifierManager.h
@@ -78,13 +78,13 @@ public:
     /**
      * Returns if a method is a signal, a slot, or neither.
      */
-    QtAccessSpecifierType qtAccessSpecifierType(clang::CXXMethodDecl*) const;
+    QtAccessSpecifierType qtAccessSpecifierType(const clang::CXXMethodDecl*) const;
 
 private:
     ClazySpecifierList &entryForClassDefinition(clang::CXXRecordDecl*);
     const clang::CompilerInstance &m_ci;
-    clang::CXXRecordDecl *classDefinitionForLoc(clang::SourceLocation loc) const;
-    std::unordered_map<clang::CXXRecordDecl*, ClazySpecifierList> m_specifiersMap;
+    const clang::CXXRecordDecl *classDefinitionForLoc(clang::SourceLocation loc) const;
+    std::unordered_map<const clang::CXXRecordDecl*, ClazySpecifierList> m_specifiersMap;
     AccessSpecifierPreprocessorCallbacks *const m_preprocessorCallbacks;
 };
 
diff --git a/src/FunctionUtils.h b/src/FunctionUtils.h
index a88dcf56..327078da 100644
--- a/src/FunctionUtils.h
+++ b/src/FunctionUtils.h
@@ -73,5 +73,28 @@ inline clang::ValueDecl *valueDeclForCallArgument(clang::CallExpr *call, unsigne
 }
 
 
+inline bool parametersMatch(clang::FunctionDecl *f1, clang::FunctionDecl *f2)
+{
+    if (!f1 || !f2)
+        return false;
+
+    auto params1 = f1->parameters();
+    auto params2 = f2->parameters();
+
+    if (params1.size() != params2.size())
+        return false;
+
+    for (int i = 0, e = params1.size(); i < e; ++i) {
+        clang::ParmVarDecl *p1 = params1[i];
+        clang::ParmVarDecl *p2 = params2[i];
+
+        if (p1->getType() != p2->getType())
+            return false;
+    }
+
+    return true;
+}
+
+
 }
 #endif
diff --git a/src/checks/level1/README-overridden-signal.md b/src/checks/level1/README-overridden-signal.md
new file mode 100644
index 00000000..cc452f4d
--- /dev/null
+++ b/src/checks/level1/README-overridden-signal.md
@@ -0,0 +1,9 @@
+# overridden-signal
+
+Warns when overriding a signal, which might make existing connects not work, if done unintentionally.
+Doesn't warn when the overriden signal has a different signature.
+
+Warns for:
+- Overriding signal with non-signal
+- Overriding non-signal with signal
+- Overriding signal with signal
diff --git a/src/checks/level1/overridden-signal.cpp b/src/checks/level1/overridden-signal.cpp
new file mode 100644
index 00000000..ea9d2c6c
--- /dev/null
+++ b/src/checks/level1/overridden-signal.cpp
@@ -0,0 +1,93 @@
+/*
+  This file is part of the clazy static checker.
+
+  Copyright (C) 2017 Sergio Martins <smartins@kde.org>
+
+  This library is free software; you can redistribute it and/or
+  modify it under the terms of the GNU Library General Public
+  License as published by the Free Software Foundation; either
+  version 2 of the License, or (at your option) any later version.
+
+  This library is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  Library General Public License for more details.
+
+  You should have received a copy of the GNU Library General Public License
+  along with this library; see the file COPYING.LIB.  If not, write to
+  the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+  Boston, MA 02110-1301, USA.
+*/
+
+#include "overridden-signal.h"
+#include "Utils.h"
+#include "HierarchyUtils.h"
+#include "QtUtils.h"
+#include "TypeUtils.h"
+#include "checkmanager.h"
+#include "AccessSpecifierManager.h"
+#include "ClazyContext.h"
+#include "FunctionUtils.h"
+
+#include <clang/AST/AST.h>
+
+using namespace clang;
+using namespace std;
+
+
+OverriddenSignal::OverriddenSignal(const std::string &name, ClazyContext *context)
+    : CheckBase(name, context)
+{
+    context->enableAccessSpecifierManager();
+}
+
+void OverriddenSignal::VisitDecl(clang::Decl *decl)
+{
+    AccessSpecifierManager *accessSpecifierManager = m_context->accessSpecifierManager;
+    auto method = dyn_cast<CXXMethodDecl>(decl);
+    if (!accessSpecifierManager || !method)
+        return;
+
+    if (method->isThisDeclarationADefinition() && !method->hasInlineBody())
+        return;
+
+    CXXRecordDecl *record = method->getParent();
+    if (!QtUtils::isQObject(record))
+        return;
+
+    const bool methodIsSignal = accessSpecifierManager->qtAccessSpecifierType(method) == QtAccessSpecifier_Signal;
+    const std::string methodName = method->getNameAsString();
+
+
+    CXXRecordDecl *baseClass = QtUtils::getQObjectBaseClass(record);
+    std::string warningMsg;
+    while (baseClass) {
+        for (auto baseMethod : baseClass->methods()) {
+            if (baseMethod->getNameAsString() == methodName) {
+
+                if (!FunctionUtils::parametersMatch(method, baseMethod)) // overloading is permitted.
+                    continue;
+
+                const bool baseMethodIsSignal = accessSpecifierManager->qtAccessSpecifierType(baseMethod) == QtAccessSpecifier_Signal;
+
+                if (methodIsSignal && baseMethodIsSignal) {
+                    warningMsg = "Overriding signal with signal: " + method->getQualifiedNameAsString();
+                } else if (methodIsSignal && !baseMethodIsSignal) {
+                    warningMsg = "Overriding non-signal with signal: " + method->getQualifiedNameAsString();
+                } else if (!methodIsSignal && baseMethodIsSignal) {
+                    warningMsg = "Overriding signal with non-signal: " + method->getQualifiedNameAsString();
+                }
+
+                if (!warningMsg.empty()) {
+                    emitWarning(decl, warningMsg);
+                    return;
+                }
+            }
+        }
+
+        baseClass = QtUtils::getQObjectBaseClass(baseClass);
+    }
+}
+
+
+REGISTER_CHECK("overridden-signal", OverriddenSignal, CheckLevel1)
diff --git a/src/checks/level1/overridden-signal.h b/src/checks/level1/overridden-signal.h
new file mode 100644
index 00000000..805b2fd5
--- /dev/null
+++ b/src/checks/level1/overridden-signal.h
@@ -0,0 +1,39 @@
+/*
+  This file is part of the clazy static checker.
+
+  Copyright (C) 2017 Sergio Martins <smartins@kde.org>
+
+  This library is free software; you can redistribute it and/or
+  modify it under the terms of the GNU Library General Public
+  License as published by the Free Software Foundation; either
+  version 2 of the License, or (at your option) any later version.
+
+  This library is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  Library General Public License for more details.
+
+  You should have received a copy of the GNU Library General Public License
+  along with this library; see the file COPYING.LIB.  If not, write to
+  the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+  Boston, MA 02110-1301, USA.
+*/
+
+#ifndef CLAZY_OVERRIDDEN_SIGNAL_H
+#define CLAZY_OVERRIDDEN_SIGNAL_H
+
+#include "checkbase.h"
+
+
+/**
+ * See README-overridden-signal.md for more info.
+ */
+class OverriddenSignal : public CheckBase
+{
+public:
+    explicit OverriddenSignal(const std::string &name, ClazyContext *context);
+    void VisitDecl(clang::Decl *decl) override;
+private:
+};
+
+#endif