diff options
Diffstat (limited to 'tools')
-rw-r--r-- | tools/qmllint/checkidentifiers.cpp | 48 | ||||
-rw-r--r-- | tools/qmllint/checkidentifiers.h | 8 | ||||
-rw-r--r-- | tools/qmllint/findwarnings.cpp | 96 | ||||
-rw-r--r-- | tools/qmllint/findwarnings.h | 8 | ||||
-rw-r--r-- | tools/shared/scopetree.cpp | 38 | ||||
-rw-r--r-- | tools/shared/scopetree.h | 31 |
6 files changed, 142 insertions, 87 deletions
diff --git a/tools/qmllint/checkidentifiers.cpp b/tools/qmllint/checkidentifiers.cpp index 9334c13e43..6df5a3fd1a 100644 --- a/tools/qmllint/checkidentifiers.cpp +++ b/tools/qmllint/checkidentifiers.cpp @@ -285,8 +285,10 @@ bool CheckIdentifiers::checkMemberAccess(const QVector<ScopeTree::FieldMember> & return true; } -bool CheckIdentifiers::operator()(const QHash<QString, ScopeTree::ConstPtr> &qmlIDs, - const ScopeTree::ConstPtr &root, const QString &rootId) const +bool CheckIdentifiers::operator()( + const QHash<QString, ScopeTree::ConstPtr> &qmlIDs, + const QHash<QQmlJS::SourceLocation, SignalHandler> &signalHandlers, + const ScopeTree::ConstPtr &root, const QString &rootId) const { bool noUnqualifiedIdentifier = true; @@ -311,7 +313,8 @@ bool CheckIdentifiers::operator()(const QHash<QString, ScopeTree::ConstPtr> &qml continue; const auto memberAccessBase = memberAccessChain.takeFirst(); - if (currentScope->isIdInCurrentJSScopes(memberAccessBase.m_name)) + const auto jsId = currentScope->findJSIdentifier(memberAccessBase.m_name); + if (jsId.has_value() && jsId->kind != JavaScriptIdentifier::Injected) continue; auto it = qmlIDs.find(memberAccessBase.m_name); @@ -404,49 +407,30 @@ bool CheckIdentifiers::operator()(const QHash<QString, ScopeTree::ConstPtr> &qml m_colorOut->write(rootId + QLatin1Char('.'), Hint); m_colorOut->write(issueLocationWithContext.issueText().toString(), Normal); m_colorOut->write(issueLocationWithContext.afterText() + QLatin1Char('\n'), Normal); - } else if (currentScope->isIdInjectedFromSignal(memberAccessBase.m_name)) { - auto methodUsages = ScopeTree::findCurrentQMLScope(currentScope) - ->injectedSignalIdentifiers().values(memberAccessBase.m_name); - auto location = memberAccessBase.m_location; - // 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](const MethodUsage &methodUsage) { - return location.startLine < methodUsage.loc.startLine - || (location.startLine == methodUsage.loc.startLine - && location.startColumn < methodUsage.loc.startColumn); - }); - auto methodUsage = *(--oneBehindIt); + } else if (jsId.has_value() && jsId->kind == JavaScriptIdentifier::Injected) { + const JavaScriptIdentifier id = jsId.value(); m_colorOut->write(QLatin1String("Note: "), Info); m_colorOut->write( memberAccessBase.m_name + QString::fromLatin1( " is accessible in this scope because " "you are handling a signal at %1:%2:%3\n") .arg(m_fileName) - .arg(methodUsage.loc.startLine).arg(methodUsage.loc.startColumn), + .arg(id.location.startLine).arg(id.location.startColumn), Normal); m_colorOut->write(QLatin1String("Consider using a function instead\n"), Normal); - IssueLocationWithContext context {m_code, methodUsage.loc}; + IssueLocationWithContext context {m_code, id.location}; m_colorOut->write(context.beforeText() + QLatin1Char(' ')); - m_colorOut->write(QLatin1String(methodUsage.hasMultilineHandlerBody - ? "function(" - : "("), - Hint); - const auto parameters = methodUsage.method.parameterNames(); + + const auto handler = signalHandlers[id.location]; + + m_colorOut->write(QLatin1String(handler.isMultiline ? "function(" : "("), Hint); + const auto parameters = handler.signal.parameterNames(); for (int numParams = parameters.size(); numParams > 0; --numParams) { m_colorOut->write(parameters.at(parameters.size() - numParams), Hint); if (numParams > 1) m_colorOut->write(QLatin1String(", "), Hint); } - m_colorOut->write(QLatin1String(methodUsage.hasMultilineHandlerBody ? ")" : ") => "), - Hint); + m_colorOut->write(QLatin1String(handler.isMultiline ? ")" : ") => "), Hint); m_colorOut->write(QLatin1String(" {..."), Normal); } m_colorOut->write(QLatin1String("\n\n\n"), Normal); diff --git a/tools/qmllint/checkidentifiers.h b/tools/qmllint/checkidentifiers.h index bdd83f1e57..373767420b 100644 --- a/tools/qmllint/checkidentifiers.h +++ b/tools/qmllint/checkidentifiers.h @@ -30,10 +30,15 @@ #define CHECKIDENTIFIERS_H #include "scopetree.h" -#include "findwarnings.h" +#include "qmljsimporter.h" class ColorOutput; +struct SignalHandler { + MetaMethod signal; + bool isMultiline; +}; + class CheckIdentifiers { public: @@ -43,6 +48,7 @@ public: {} bool operator ()(const QHash<QString, ScopeTree::ConstPtr> &qmlIDs, + const QHash<QQmlJS::SourceLocation, SignalHandler> &signalHandlers, const ScopeTree::ConstPtr &root, const QString &rootId) const; private: diff --git a/tools/qmllint/findwarnings.cpp b/tools/qmllint/findwarnings.cpp index 9fe3bf99bd..309c7aa6ed 100644 --- a/tools/qmllint/findwarnings.cpp +++ b/tools/qmllint/findwarnings.cpp @@ -103,6 +103,16 @@ void FindWarningVisitor::importExportedNames(ScopeTree::ConstPtr scope) } } +void FindWarningVisitor::flushPendingSignalParameters() +{ + const SignalHandler handler = m_signalHandlers[m_pendingSingalHandler]; + for (const QString ¶meter : handler.signal.parameterNames()) { + m_currentScope->insertJSIdentifier( + parameter, { JavaScriptIdentifier::Injected, m_pendingSingalHandler}); + } + m_pendingSingalHandler = QQmlJS::SourceLocation(); +} + void FindWarningVisitor::throwRecursionDepthError() { m_colorOut.write(QStringLiteral("Error"), Error); @@ -185,9 +195,28 @@ void FindWarningVisitor::endVisit(QQmlJS::AST::ForEachStatement *) leaveEnvironment(); } +bool FindWarningVisitor::visit(QQmlJS::AST::ExpressionStatement *) +{ + if (m_pendingSingalHandler.isValid()) { + enterEnvironment(ScopeType::JSFunctionScope, "signalhandler"); + flushPendingSignalParameters(); + } + return true; +} + +void FindWarningVisitor::endVisit(QQmlJS::AST::ExpressionStatement *) +{ + if (m_currentScope->scopeType() == ScopeType::JSFunctionScope + && m_currentScope->baseTypeName() == "signalhandler") { + leaveEnvironment(); + } +} + bool FindWarningVisitor::visit(QQmlJS::AST::Block *) { enterEnvironment(ScopeType::JSLexicalScope, "block"); + if (m_pendingSingalHandler.isValid()) + flushPendingSignalParameters(); return true; } @@ -210,8 +239,11 @@ void FindWarningVisitor::endVisit(QQmlJS::AST::CaseBlock *) bool FindWarningVisitor::visit(QQmlJS::AST::Catch *catchStatement) { enterEnvironment(ScopeType::JSLexicalScope, "catch"); - m_currentScope->insertJSIdentifier(catchStatement->patternElement->bindingIdentifier.toString(), - ScopeType::JSLexicalScope); + m_currentScope->insertJSIdentifier( + catchStatement->patternElement->bindingIdentifier.toString(), { + JavaScriptIdentifier::LexicalScoped, + catchStatement->patternElement->firstSourceLocation() + }); return true; } @@ -300,13 +332,13 @@ bool FindWarningVisitor::visit(QQmlJS::AST::UiScriptBinding *uisb) for (auto method = methodsRange.first; method != methodsRange.second; ++method) { if (method->methodType() != MetaMethod::Signal) continue; - for (auto const ¶m : method->parameterNames()) { - const auto firstSourceLocation = statement->firstSourceLocation(); - bool hasMultilineStatementBody - = statement->lastSourceLocation().startLine > firstSourceLocation.startLine; - m_currentScope->insertSignalIdentifier(param, *method, firstSourceLocation, - hasMultilineStatementBody); - } + + const auto firstSourceLocation = statement->firstSourceLocation(); + bool hasMultilineStatementBody + = statement->lastSourceLocation().startLine > firstSourceLocation.startLine; + m_pendingSingalHandler = firstSourceLocation; + m_signalHandlers.insert(firstSourceLocation, {*method, hasMultilineStatementBody}); + break; // If there are multiple candidates for the signal, it's a mess anyway. } return true; } @@ -383,14 +415,18 @@ FindWarningVisitor::FindWarningVisitor( // XMLHttpRequest QLatin1String("XMLHttpRequest") }; + + JavaScriptIdentifier globalJavaScript = { + JavaScriptIdentifier::LexicalScoped, + QQmlJS::SourceLocation() + }; for (const char **globalName = QV4::Compiler::Codegen::s_globalNames; *globalName != nullptr; ++globalName) { - m_currentScope->insertJSIdentifier(QString::fromLatin1(*globalName), - ScopeType::JSLexicalScope); + m_currentScope->insertJSIdentifier(QString::fromLatin1(*globalName), globalJavaScript); } for (const auto& jsGlobVar: jsGlobVars) - m_currentScope->insertJSIdentifier(jsGlobVar, ScopeType::JSLexicalScope); + m_currentScope->insertJSIdentifier(jsGlobVar, globalJavaScript); } bool FindWarningVisitor::check() @@ -411,7 +447,7 @@ bool FindWarningVisitor::check() return true; CheckIdentifiers check(&m_colorOut, m_code, m_rootScopeImports, m_filePath); - return check(m_qmlid2scope, m_rootScope, m_rootId); + return check(m_qmlid2scope, m_signalHandlers, m_rootScope, m_rootId); } bool FindWarningVisitor::visit(QQmlJS::AST::VariableDeclarationList *vdl) @@ -419,9 +455,12 @@ bool FindWarningVisitor::visit(QQmlJS::AST::VariableDeclarationList *vdl) while (vdl) { m_currentScope->insertJSIdentifier( vdl->declaration->bindingIdentifier.toString(), - (vdl->declaration->scope == QQmlJS::AST::VariableScope::Var) - ? ScopeType::JSFunctionScope - : ScopeType::JSLexicalScope); + { + (vdl->declaration->scope == QQmlJS::AST::VariableScope::Var) + ? JavaScriptIdentifier::FunctionScoped + : JavaScriptIdentifier::LexicalScoped, + vdl->declaration->firstSourceLocation() + }); vdl = vdl->next; } return true; @@ -432,10 +471,13 @@ void FindWarningVisitor::visitFunctionExpressionHelper(QQmlJS::AST::FunctionExpr using namespace QQmlJS::AST; auto name = fexpr->name.toString(); if (!name.isEmpty()) { - if (m_currentScope->scopeType() == ScopeType::QMLScope) + if (m_currentScope->scopeType() == ScopeType::QMLScope) { m_currentScope->addMethod(MetaMethod(name, QLatin1String("void"))); - else - m_currentScope->insertJSIdentifier(name, ScopeType::JSLexicalScope); + } else { + m_currentScope->insertJSIdentifier( + name, + { JavaScriptIdentifier::LexicalScoped, fexpr->firstSourceLocation() }); + } enterEnvironment(ScopeType::JSFunctionScope, name); } else { enterEnvironment(ScopeType::JSFunctionScope, QLatin1String("<anon>")); @@ -466,8 +508,11 @@ void FindWarningVisitor::endVisit(QQmlJS::AST::FunctionDeclaration *) bool FindWarningVisitor::visit(QQmlJS::AST::FormalParameterList *fpl) { - for (auto const &boundName : fpl->boundNames()) - m_currentScope->insertJSIdentifier(boundName.id, ScopeType::JSLexicalScope); + for (auto const &boundName : fpl->boundNames()) { + m_currentScope->insertJSIdentifier( + boundName.id, + {JavaScriptIdentifier::Parameter, fpl->firstSourceLocation() }); + } return true; } @@ -625,9 +670,12 @@ bool FindWarningVisitor::visit(QQmlJS::AST::PatternElement *element) element->boundNames(&names); for (const auto &name : names) { m_currentScope->insertJSIdentifier( - name.id, (element->scope == QQmlJS::AST::VariableScope::Var) - ? ScopeType::JSFunctionScope - : ScopeType::JSLexicalScope); + name.id, { + (element->scope == QQmlJS::AST::VariableScope::Var) + ? JavaScriptIdentifier::FunctionScoped + : JavaScriptIdentifier::LexicalScoped, + element->firstSourceLocation() + }); } } diff --git a/tools/qmllint/findwarnings.h b/tools/qmllint/findwarnings.h index 382c4cab11..e160e8751c 100644 --- a/tools/qmllint/findwarnings.h +++ b/tools/qmllint/findwarnings.h @@ -43,6 +43,7 @@ #include "scopetree.h" #include "qcoloroutput.h" #include "qmljsimporter.h" +#include "checkidentifiers.h" #include <QtQml/private/qqmldirparser_p.h> #include <QtQml/private/qqmljsastvisitor_p.h> @@ -63,6 +64,9 @@ public: private: QmlJSImporter::ImportedTypes m_rootScopeImports; + QHash<QQmlJS::SourceLocation, SignalHandler> m_signalHandlers; + QQmlJS::SourceLocation m_pendingSingalHandler; + ScopeTree::Ptr m_rootScope; ScopeTree::Ptr m_currentScope; QQmlJS::AST::ExpressionNode *m_fieldMemberBase = nullptr; @@ -97,6 +101,7 @@ private: void parseHeaders(QQmlJS::AST::UiHeaderItemList *headers); ScopeTree::Ptr parseProgram(QQmlJS::AST::Program *program, const QString &name); + void flushPendingSignalParameters(); void throwRecursionDepthError() override; @@ -116,6 +121,9 @@ private: bool visit(QQmlJS::AST::ForEachStatement *ast) override; void endVisit(QQmlJS::AST::ForEachStatement *ast) override; + bool visit(QQmlJS::AST::ExpressionStatement *ast) override; + void endVisit(QQmlJS::AST::ExpressionStatement *ast) override; + bool visit(QQmlJS::AST::Block *ast) override; void endVisit(QQmlJS::AST::Block *ast) override; diff --git a/tools/shared/scopetree.cpp b/tools/shared/scopetree.cpp index e7ce631b1d..933f3f88d7 100644 --- a/tools/shared/scopetree.cpp +++ b/tools/shared/scopetree.cpp @@ -49,28 +49,21 @@ ScopeTree::Ptr ScopeTree::create(ScopeType type, const ScopeTree::Ptr &parentSco return childScope; } -void ScopeTree::insertJSIdentifier(const QString &id, ScopeType scope) +void ScopeTree::insertJSIdentifier(const QString &name, const JavaScriptIdentifier &identifier) { Q_ASSERT(m_scopeType != ScopeType::QMLScope); - Q_ASSERT(scope != ScopeType::QMLScope); - if (scope != ScopeType::JSFunctionScope || m_scopeType == ScopeType::JSFunctionScope) { - m_jsIdentifiers.insert(id); + if (identifier.kind == JavaScriptIdentifier::LexicalScoped + || identifier.kind == JavaScriptIdentifier::Injected + || m_scopeType == ScopeType::JSFunctionScope) { + m_jsIdentifiers.insert(name, identifier); } else { auto targetScope = parentScope(); while (targetScope->m_scopeType != ScopeType::JSFunctionScope) targetScope = targetScope->parentScope(); - targetScope->m_jsIdentifiers.insert(id); + targetScope->m_jsIdentifiers.insert(name, identifier); } } -void ScopeTree::insertSignalIdentifier(const QString &id, const MetaMethod &method, - const QQmlJS::SourceLocation &loc, - bool hasMultilineHandlerBody) -{ - Q_ASSERT(m_scopeType == ScopeType::QMLScope); - m_injectedSignalIdentifiers.insert(id, {method, loc, hasMultilineHandlerBody}); -} - void ScopeTree::insertPropertyIdentifier(const MetaProperty &property) { addProperty(property); @@ -127,9 +120,22 @@ bool ScopeTree::isIdInCurrentJSScopes(const QString &id) const bool ScopeTree::isIdInjectedFromSignal(const QString &id) const { - if (m_scopeType == ScopeType::QMLScope) - return m_injectedSignalIdentifiers.contains(id); - return findCurrentQMLScope(parentScope())->m_injectedSignalIdentifiers.contains(id); + const auto found = findJSIdentifier(id); + return found.has_value() && found->kind == JavaScriptIdentifier::Injected; +} + +std::optional<JavaScriptIdentifier> ScopeTree::findJSIdentifier(const QString &id) const +{ + for (const auto *scope = this; scope; scope = scope->parentScope().data()) { + if (scope->m_scopeType == ScopeType::JSFunctionScope + || scope->m_scopeType == ScopeType::JSLexicalScope) { + auto it = scope->m_jsIdentifiers.find(id); + if (it != scope->m_jsIdentifiers.end()) + return *it; + } + } + + return std::optional<JavaScriptIdentifier>{}; } void ScopeTree::resolveTypes(const QHash<QString, ScopeTree::ConstPtr> &contextualTypes) diff --git a/tools/shared/scopetree.h b/tools/shared/scopetree.h index 37eee26d16..b751815172 100644 --- a/tools/shared/scopetree.h +++ b/tools/shared/scopetree.h @@ -48,6 +48,8 @@ #include <QtCore/qstring.h> #include <QtCore/qversionnumber.h> +#include <optional> + enum class ScopeType { JSFunctionScope, @@ -55,11 +57,17 @@ enum class ScopeType QMLScope }; -struct MethodUsage +struct JavaScriptIdentifier { - MetaMethod method; - QQmlJS::SourceLocation loc; - bool hasMultilineHandlerBody; + enum Kind { + Parameter, + FunctionScoped, + LexicalScoped, + Injected + }; + + Kind kind = FunctionScoped; + QQmlJS::SourceLocation location; }; class ScopeTree @@ -115,9 +123,8 @@ public: ScopeTree::Ptr parentScope() const { return m_parentScope.toStrongRef(); } - void insertJSIdentifier(const QString &id, ScopeType scope); - void insertSignalIdentifier(const QString &id, const MetaMethod &method, - const QQmlJS::SourceLocation &loc, bool hasMultilineHandlerBody); + void insertJSIdentifier(const QString &name, const JavaScriptIdentifier &identifier); + // inserts property as qml identifier as well as the corresponding void insertPropertyIdentifier(const MetaProperty &prop); void addUnmatchedSignalHandler(const QString &handler, @@ -196,23 +203,19 @@ public: bool isIdInCurrentJSScopes(const QString &id) const; bool isIdInjectedFromSignal(const QString &id) const; + std::optional<JavaScriptIdentifier> findJSIdentifier(const QString &id) const; + QVector<ScopeTree::Ptr> childScopes() const { return m_childScopes; } - QMultiHash<QString, MethodUsage> injectedSignalIdentifiers() const - { - return m_injectedSignalIdentifiers; - } - void resolveTypes(const QHash<QString, ConstPtr> &contextualTypes); private: ScopeTree(ScopeType type, const ScopeTree::Ptr &parentScope = ScopeTree::Ptr()); - QSet<QString> m_jsIdentifiers; - QMultiHash<QString, MethodUsage> m_injectedSignalIdentifiers; + QHash<QString, JavaScriptIdentifier> m_jsIdentifiers; QMultiHash<QString, MetaMethod> m_methods; QHash<QString, MetaProperty> m_properties; |