diff options
-rw-r--r-- | tests/auto/qml/qmllint/data/SignalHandler.qml | 10 | ||||
-rw-r--r-- | tests/auto/qml/qmllint/main.cpp | 5 | ||||
-rw-r--r-- | tools/qmllint/findunqualified.cpp | 4 | ||||
-rw-r--r-- | tools/qmllint/scopetree.cpp | 25 | ||||
-rw-r--r-- | tools/qmllint/scopetree.h | 5 |
5 files changed, 38 insertions, 11 deletions
diff --git a/tests/auto/qml/qmllint/data/SignalHandler.qml b/tests/auto/qml/qmllint/data/SignalHandler.qml index bdf503e3dc..865277cedb 100644 --- a/tests/auto/qml/qmllint/data/SignalHandler.qml +++ b/tests/auto/qml/qmllint/data/SignalHandler.qml @@ -1,5 +1,13 @@ import QtQuick 2.0 MouseArea { - onDoubleClicked: console.log(mouse); + onDoubleClicked: { + console.log(mouse); + // do further things + } + onClicked: console.info(mouse) + onPositionChanged: { + console.log(mouse) + } + onPressAndHold: console.warn(mouse) } diff --git a/tests/auto/qml/qmllint/main.cpp b/tests/auto/qml/qmllint/main.cpp index 038826790b..1069aa5a33 100644 --- a/tests/auto/qml/qmllint/main.cpp +++ b/tests/auto/qml/qmllint/main.cpp @@ -110,7 +110,10 @@ void TestQmllint::testUnqualified_data() QTest::newRow("FromRootDirectParentDirect") << QStringLiteral("FromRootDirectParent.qml") << QStringLiteral("x: parent.unqualified") << 8 << 12; QTest::newRow("FromRootDirectParentAccess") << QStringLiteral("FromRootDirectParent.qml") << QStringLiteral("property int check: parent.x") << 12 << 29; // access injected name from signal - QTest::newRow("SignalHandler") << QStringLiteral("SignalHandler.qml") << QStringLiteral("onDoubleClicked: function(mouse) {...") << 4 << 34; + QTest::newRow("SignalHandler1") << QStringLiteral("SignalHandler.qml") << QStringLiteral("onDoubleClicked: function(mouse) {...") << 5 << 21; + QTest::newRow("SignalHandler2") << QStringLiteral("SignalHandler.qml") << QStringLiteral("onPositionChanged: function(mouse) {...") << 10 << 21; + QTest::newRow("SignalHandlerShort1") << QStringLiteral("SignalHandler.qml") << QStringLiteral("onClicked: (mouse) => {...") << 8 << 29; + QTest::newRow("SignalHandlerShort2") << QStringLiteral("SignalHandler.qml") << QStringLiteral("onPressAndHold: (mouse) => {...") << 12 << 34; } diff --git a/tools/qmllint/findunqualified.cpp b/tools/qmllint/findunqualified.cpp index 3abdfcfa53..e6a72df44c 100644 --- a/tools/qmllint/findunqualified.cpp +++ b/tools/qmllint/findunqualified.cpp @@ -503,7 +503,9 @@ bool FindUnqualifiedIDVisitor::visit(QQmlJS::AST::UiScriptBinding *uisb) } else { auto method = m_currentScope->methods()[signal]; for (auto const ¶m : method.parameterNames()) { - m_currentScope->insertSignalIdentifier(param, method, uisb->statement->firstSourceLocation()); + auto firstSourceLocation = uisb->statement->firstSourceLocation(); + bool hasMultilineStatementBody = uisb->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 b064117dd4..f58fde061e 100644 --- a/tools/qmllint/scopetree.cpp +++ b/tools/qmllint/scopetree.cpp @@ -30,6 +30,8 @@ #include "qcoloroutput_p.h" +#include <algorithm> + #include <QQueue> ScopeTree::ScopeTree(ScopeType type, QString name, ScopeTree *parentScope) @@ -66,10 +68,10 @@ void ScopeTree::insertQMLIdentifier(QString id) m_currentScopeQMLIdentifiers.insert(id); } -void ScopeTree::insertSignalIdentifier(QString id, LanguageUtils::FakeMetaMethod method, QQmlJS::AST::SourceLocation loc) +void ScopeTree::insertSignalIdentifier(QString id, LanguageUtils::FakeMetaMethod method, QQmlJS::AST::SourceLocation loc, bool hasMultilineHandlerBody) { Q_ASSERT(m_scopeType == ScopeType::QMLScope); - m_injectedSignalIdentifiers.insert(id, {method, loc}); + m_injectedSignalIdentifiers.insert(id, {method, loc, hasMultilineHandlerBody}); } void ScopeTree::insertPropertyIdentifier(QString id) @@ -164,13 +166,24 @@ bool ScopeTree::recheckIdentifiers(const QString& code, const QHash<QString, Lan colorOut.write(issueLocationWithContext.afterText + QLatin1Char('\n'), Normal); } else if (currentScope->isIdInjectedFromSignal(idLocationPair.first)) { auto qmlScope = currentScope->getCurrentQMLScope(); - MethodUsage methodUsage = qmlScope->m_injectedSignalIdentifiers[idLocationPair.first]; + auto methodUsages = qmlScope->m_injectedSignalIdentifiers.values(idLocationPair.first); + auto location = idLocationPair.second; + // sort the list of signal handlers by their occurrence in the source code + // then, we select the first one whose location is after the unqualified id + // and go one step backwards to get the one which we actually need + std::sort(methodUsages.begin(), methodUsages.end(), [](const MethodUsage m1, const MethodUsage m2) { + return m1.loc.startLine < m2.loc.startLine || (m1.loc.startLine == m2.loc.startLine && m1.loc.startColumn < m2.loc.startColumn); + }); + auto oneBehindIt = std::find_if(methodUsages.begin(), methodUsages.end(), [&location](MethodUsage methodUsage) { + return location.startLine < methodUsage.loc.startLine || (location.startLine == methodUsage.loc.startLine && location.startColumn < methodUsage.loc.startColumn); + }); + auto methodUsage = *(--oneBehindIt); colorOut.write("Note:", Info); colorOut.write(idLocationPair.first + QString::asprintf(" is accessible in this scope because you are handling a signal at %d:%d\n", methodUsage.loc.startLine, methodUsage.loc.startColumn), Normal); colorOut.write("Consider using a function instead\n", Normal); IssueLocationWithContext context {code, methodUsage.loc}; colorOut.write(context.beforeText + QLatin1Char(' ')); - colorOut.write("function(", Hint); + colorOut.write(methodUsage.hasMultilineHandlerBody ? "function(" : "(", Hint); const auto parameters = methodUsage.method.parameterNames(); for (int numParams = parameters.size(); numParams > 0; --numParams) { colorOut.write(parameters.at(parameters.size() - numParams), Hint); @@ -178,10 +191,10 @@ bool ScopeTree::recheckIdentifiers(const QString& code, const QHash<QString, Lan colorOut.write(", ", Hint); } } - colorOut.write(")", Hint); + colorOut.write(methodUsage.hasMultilineHandlerBody ? ")" : ") => ", Hint); colorOut.write(" {...", Normal); } - colorOut.write("\n", Normal); + colorOut.write("\n\n\n", Normal); } for (auto const& childScope: currentScope->m_childScopes) { workQueue.enqueue(childScope); diff --git a/tools/qmllint/scopetree.h b/tools/qmllint/scopetree.h index cb495584c1..a65ed30d61 100644 --- a/tools/qmllint/scopetree.h +++ b/tools/qmllint/scopetree.h @@ -56,6 +56,7 @@ struct MethodUsage { LanguageUtils::FakeMetaMethod method; QQmlJS::AST::SourceLocation loc; + bool hasMultilineHandlerBody; }; class ColorOutput; @@ -70,7 +71,7 @@ public: void insertJSIdentifier(QString id, QQmlJS::AST::VariableScope scope); void insertQMLIdentifier(QString id); - void insertSignalIdentifier(QString id, LanguageUtils::FakeMetaMethod method, QQmlJS::AST::SourceLocation loc); + 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 bool isIdInCurrentScope(QString const &id) const; @@ -88,7 +89,7 @@ public: private: QSet<QString> m_currentScopeJSIdentifiers; QSet<QString> m_currentScopeQMLIdentifiers; - QHash<QString, MethodUsage> m_injectedSignalIdentifiers; // need iteration in insertion order for hints, rarely more than 4 parameters needed + QMultiHash<QString, MethodUsage> m_injectedSignalIdentifiers; QMap<QString, LanguageUtils::FakeMetaMethod> m_methods; QVector<QPair<QString, QQmlJS::AST::SourceLocation>> m_accessedIdentifiers; QVector<ScopeTree*> m_childScopes; |