qmllint: Warn about magic signal handlers

Those are typically part of Connections elements. We want to use
functions instead.

Change-Id: I08b65eae8b8a6ba95f7a3570f5465961abbb506e
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

3 files changed, 76 insertions, 20 deletions

diff --git a/tools/qmllint/findunqualified.cpp b/tools/qmllint/findunqualified.cpp
index 27939608d7..fffb1d06fa 100644
--- a/tools/qmllint/findunqualified.cpp
+++ b/tools/qmllint/findunqualified.cpp
@@ -476,6 +476,23 @@ void FindUnqualifiedIDVisitor::endVisit(QQmlJS::AST::WithStatement *)
     leaveEnvironment();
 }
 
+static QString signalName(const QStringRef &handlerName)
+{
+    if (handlerName.startsWith("on") && handlerName.size() > 2) {
+        QString signal = handlerName.mid(2).toString();
+        for (int i = 0; i < signal.length(); ++i) {
+            QCharRef ch = signal[i];
+            if (ch.isLower())
+                return QString();
+            if (ch.isUpper()) {
+                ch = ch.toLower();
+                return signal;
+            }
+        }
+    }
+    return QString();
+}
+
 bool FindUnqualifiedIDVisitor::visit(QQmlJS::AST::UiScriptBinding *uisb)
 {
     using namespace QQmlJS::AST;
@@ -489,8 +506,17 @@ bool FindUnqualifiedIDVisitor::visit(QQmlJS::AST::UiScriptBinding *uisb)
         if (m_currentScope->isVisualRootScope()) {
             m_rootId = identexp->name.toString();
         }
-    } else if (name.startsWith("on") && name.size() > 2 && name.at(2).isUpper()) {
-        auto statement = uisb->statement;
+    } else {
+        const QString signal = signalName(name);
+        if (signal.isEmpty())
+            return true;
+
+        if (!m_currentScope->methods().contains(signal)) {
+            m_currentScope->addUnmatchedSignalHandler(name.toString(), uisb->firstSourceLocation());
+            return true;
+        }
+
+        const auto statement = uisb->statement;
         if (statement->kind == Node::Kind::Kind_ExpressionStatement) {
             if (static_cast<ExpressionStatement *>(statement)->expression->asFunctionDefinition()) {
                 // functions are already handled
@@ -499,17 +525,14 @@ bool FindUnqualifiedIDVisitor::visit(QQmlJS::AST::UiScriptBinding *uisb)
                 return true;
             }
         }
-        QString signal = name.mid(2).toString();
-        signal[0] = signal[0].toLower();
-        if (!m_currentScope->methods().contains(signal)) {
-            qDebug() << "Info file does not contain signal" << signal;
-        } else {
-            auto method = m_currentScope->methods()[signal];
-            for (auto const &param : method.parameterNames()) {
-                auto firstSourceLocation = uisb->statement->firstSourceLocation();
-                bool hasMultilineStatementBody = uisb->statement->lastSourceLocation().startLine > firstSourceLocation.startLine;
-                m_currentScope->insertSignalIdentifier(param, method, firstSourceLocation, hasMultilineStatementBody);
-            }
+
+        auto method = m_currentScope->methods()[signal];
+        for (auto const &param : method.parameterNames()) {
+            const auto firstSourceLocation = statement->firstSourceLocation();
+            bool hasMultilineStatementBody
+                    = statement->lastSourceLocation().startLine > firstSourceLocation.startLine;
+            m_currentScope->insertSignalIdentifier(param, method, firstSourceLocation,
+                                                   hasMultilineStatementBody);
         }
         return true;
     }
diff --git a/tools/qmllint/scopetree.cpp b/tools/qmllint/scopetree.cpp
index 2eff3fa319..0ba4f9e38e 100644
--- a/tools/qmllint/scopetree.cpp
+++ b/tools/qmllint/scopetree.cpp
@@ -81,6 +81,12 @@ void ScopeTree::insertPropertyIdentifier(QString id)
     this->addMethod(method);
 }
 
+void ScopeTree::addUnmatchedSignalHandler(const QString &handler,
+                                          const QQmlJS::AST::SourceLocation &location)
+{
+    m_unmatchedSignalHandlers.append(qMakePair(handler, location));
+}
+
 bool ScopeTree::isIdInCurrentScope(const QString &id) const
 {
     return isIdInCurrentQMlScopes(id) || isIdInCurrentJSScopes(id);
@@ -132,6 +138,15 @@ bool ScopeTree::recheckIdentifiers(const QString& code, const QHash<QString, Lan
     workQueue.enqueue(this);
     while (!workQueue.empty()) {
         const ScopeTree* currentScope = workQueue.dequeue();
+        for (const auto &handler : currentScope->m_unmatchedSignalHandlers) {
+            colorOut.write("Warning: ", Warning);
+            colorOut.write(QString::fromLatin1(
+                                   "no matching signal found for handler \"%1\" at %2:%3\n")
+                                   .arg(handler.first).arg(handler.second.startLine)
+                                   .arg(handler.second.startColumn), Normal);
+            printContext(colorOut, code, handler.second);
+        }
+
         for (auto idLocationPair : currentScope->m_accessedIdentifiers) {
             if (qmlIDs.contains(idLocationPair.first))
                 continue;
@@ -141,13 +156,11 @@ bool ScopeTree::recheckIdentifiers(const QString& code, const QHash<QString, Lan
             noUnqualifiedIdentifier = false;
             colorOut.write("Warning: ", Warning);
             auto location = idLocationPair.second;
-            colorOut.write(QString::asprintf("unqualified access at %d:%d\n", location.startLine, location.startColumn), Normal);
-            IssueLocationWithContext issueLocationWithContext {code, location};
-            colorOut.write(issueLocationWithContext.beforeText.toString(), Normal);
-            colorOut.write(issueLocationWithContext.issueText.toString(), Error);
-            colorOut.write(issueLocationWithContext.afterText.toString() + QLatin1Char('\n'), Normal);
-            int tabCount = issueLocationWithContext.beforeText.count(QLatin1Char('\t'));
-            colorOut.write(QString(" ").repeated(issueLocationWithContext.beforeText.length() - tabCount) + QString("\t").repeated(tabCount) + QString("^").repeated(location.length) + QLatin1Char('\n'), Normal);
+            colorOut.write(QString::asprintf("unqualified access at %d:%d\n", location.startLine,
+                                             location.startColumn), Normal);
+
+            printContext(colorOut, code, location);
+
             // root(JS) --> program(qml) --> (first element)
             if (root->m_childScopes[0]->m_childScopes[0]->m_currentScopeQMLIdentifiers.contains(idLocationPair.first)) {
                 ScopeTree *parentScope = currentScope->m_parentScope;
@@ -161,6 +174,7 @@ bool ScopeTree::recheckIdentifiers(const QString& code, const QHash<QString, Lan
                     colorOut.write("Note: ", Warning);
                     colorOut.write(("You first have to give the root element an id\n"));
                 }
+                IssueLocationWithContext issueLocationWithContext {code, location};
                 colorOut.write(issueLocationWithContext.beforeText.toString(), Normal);
                 colorOut.write(rootId + QLatin1Char('.'), Hint);
                 colorOut.write(issueLocationWithContext.issueText.toString(), Normal);
@@ -250,6 +264,20 @@ ScopeTree *ScopeTree::getCurrentQMLScope()
     return qmlScope;
 }
 
+void ScopeTree::printContext(ColorOutput &colorOut, const QString &code,
+                             const QQmlJS::AST::SourceLocation &location) const
+{
+    IssueLocationWithContext issueLocationWithContext {code, location};
+    colorOut.write(issueLocationWithContext.beforeText.toString(), Normal);
+    colorOut.write(issueLocationWithContext.issueText.toString(), Error);
+    colorOut.write(issueLocationWithContext.afterText.toString() + QLatin1Char('\n'), Normal);
+    int tabCount = issueLocationWithContext.beforeText.count(QLatin1Char('\t'));
+    colorOut.write(QString(" ").repeated(issueLocationWithContext.beforeText.length() - tabCount)
+                           + QString("\t").repeated(tabCount)
+                           + QString("^").repeated(location.length)
+                           + QLatin1Char('\n'), Normal);
+}
+
 ScopeType ScopeTree::scopeType() {return m_scopeType;}
 
 void ScopeTree::addMethod(LanguageUtils::FakeMetaMethod method)
diff --git a/tools/qmllint/scopetree.h b/tools/qmllint/scopetree.h
index 872a509123..52cdc45e96 100644
--- a/tools/qmllint/scopetree.h
+++ b/tools/qmllint/scopetree.h
@@ -73,6 +73,8 @@ public:
     void insertQMLIdentifier(QString id);
     void insertSignalIdentifier(QString id, LanguageUtils::FakeMetaMethod method, QQmlJS::AST::SourceLocation loc, bool hasMultilineHandlerBody);
     void insertPropertyIdentifier(QString id); // inserts property as qml identifier as well as the corresponding
+    void addUnmatchedSignalHandler(const QString &handler,
+                                   const QQmlJS::AST::SourceLocation &location);
 
     bool isIdInCurrentScope(QString const &id) const;
     void addIdToAccssedIfNotInParentScopes(QPair<QString, QQmlJS::AST::SourceLocation> const& id_loc_pair, const QSet<QString>& unknownImports);
@@ -96,11 +98,14 @@ private:
     ScopeTree *m_parentScope;
     QString m_name;
     ScopeType m_scopeType;
+    QVector<QPair<QString, QQmlJS::AST::SourceLocation>> m_unmatchedSignalHandlers;
 
     bool isIdInCurrentQMlScopes(QString id) const;
     bool isIdInCurrentJSScopes(QString id) const;
     bool isIdInjectedFromSignal(QString id) const;
     const ScopeTree* getCurrentQMLScope() const;
     ScopeTree* getCurrentQMLScope();
+    void printContext(ColorOutput& colorOut, const QString &code,
+                      const QQmlJS::AST::SourceLocation &location) const;
 };
 #endif // SCOPETREE_H