diff options
author | Ulf Hermann <ulf.hermann@qt.io> | 2020-10-15 11:16:34 +0200 |
---|---|---|
committer | Ulf Hermann <ulf.hermann@qt.io> | 2020-10-15 13:17:12 +0200 |
commit | ea7d11a3c73b6054852092ab268466f7639b8dbf (patch) | |
tree | a9137ca1a34466bb95224c9a37adf3e20b8b14ab /tools/qmllint | |
parent | fa48382c492097d06d7ce066ac36dba04fd5e801 (diff) |
Further unify findwarnings.cpp and qqmljsimportvisitor.cpp
Most of the logic in findwarnings.cpp applies also to imported files and
should live in the base class.
Change-Id: I65f326f50a8bfab0dff4b5b31f7bee7300b20704
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Diffstat (limited to 'tools/qmllint')
-rw-r--r-- | tools/qmllint/findwarnings.cpp | 279 | ||||
-rw-r--r-- | tools/qmllint/findwarnings.h | 27 |
2 files changed, 58 insertions, 248 deletions
diff --git a/tools/qmllint/findwarnings.cpp b/tools/qmllint/findwarnings.cpp index 6f8c64aabc..7325c26a58 100644 --- a/tools/qmllint/findwarnings.cpp +++ b/tools/qmllint/findwarnings.cpp @@ -43,7 +43,7 @@ #include <QtCore/qdiriterator.h> #include <QtCore/qscopedvaluerollback.h> -void FindWarningVisitor::importExportedNames(QQmlJSScope::ConstPtr scope) +void FindWarningVisitor::checkInheritanceCycle(QQmlJSScope::ConstPtr scope) { QList<QQmlJSScope::ConstPtr> scopes; while (!scope.isNull()) { @@ -72,12 +72,6 @@ void FindWarningVisitor::importExportedNames(QQmlJSScope::ConstPtr scope) scopes.append(scope); - const auto properties = scope->properties(); - for (auto property : properties) - m_currentScope->insertPropertyIdentifier(property); - - m_currentScope->addMethods(scope->methods()); - if (scope->baseTypeName().isEmpty()) { break; } else if (auto newScope = scope->baseType()) { @@ -115,39 +109,6 @@ void FindWarningVisitor::throwRecursionDepthError() m_visitFailed = true; } -bool FindWarningVisitor::visit(QQmlJS::AST::ClassDeclaration *ast) -{ - enterEnvironment(QQmlJSScope::JSFunctionScope, ast->name.toString()); - return true; -} - -void FindWarningVisitor::endVisit(QQmlJS::AST::ClassDeclaration *) -{ - leaveEnvironment(); -} - -bool FindWarningVisitor::visit(QQmlJS::AST::ForStatement *) -{ - enterEnvironment(QQmlJSScope::JSLexicalScope, "forloop"); - return true; -} - -void FindWarningVisitor::endVisit(QQmlJS::AST::ForStatement *) -{ - leaveEnvironment(); -} - -bool FindWarningVisitor::visit(QQmlJS::AST::ForEachStatement *) -{ - enterEnvironment(QQmlJSScope::JSLexicalScope, "foreachloop"); - return true; -} - -void FindWarningVisitor::endVisit(QQmlJS::AST::ForEachStatement *) -{ - leaveEnvironment(); -} - bool FindWarningVisitor::visit(QQmlJS::AST::ExpressionStatement *) { if (m_pendingSingalHandler.isValid()) { @@ -165,46 +126,15 @@ void FindWarningVisitor::endVisit(QQmlJS::AST::ExpressionStatement *) } } -bool FindWarningVisitor::visit(QQmlJS::AST::Block *) +bool FindWarningVisitor::visit(QQmlJS::AST::Block *block) { - enterEnvironment(QQmlJSScope::JSLexicalScope, "block"); + if (!QQmlJSImportVisitor::visit(block)) + return false; if (m_pendingSingalHandler.isValid()) flushPendingSignalParameters(); return true; } -void FindWarningVisitor::endVisit(QQmlJS::AST::Block *) -{ - leaveEnvironment(); -} - -bool FindWarningVisitor::visit(QQmlJS::AST::CaseBlock *) -{ - enterEnvironment(QQmlJSScope::JSLexicalScope, "case"); - return true; -} - -void FindWarningVisitor::endVisit(QQmlJS::AST::CaseBlock *) -{ - leaveEnvironment(); -} - -bool FindWarningVisitor::visit(QQmlJS::AST::Catch *catchStatement) -{ - enterEnvironment(QQmlJSScope::JSLexicalScope, "catch"); - m_currentScope->insertJSIdentifier( - catchStatement->patternElement->bindingIdentifier.toString(), { - QQmlJSScope::JavaScriptIdentifier::LexicalScoped, - catchStatement->patternElement->firstSourceLocation() - }); - return true; -} - -void FindWarningVisitor::endVisit(QQmlJS::AST::Catch *) -{ - leaveEnvironment(); -} - bool FindWarningVisitor::visit(QQmlJS::AST::WithStatement *withStatement) { if (m_warnWithStatement) { @@ -218,13 +148,7 @@ bool FindWarningVisitor::visit(QQmlJS::AST::WithStatement *withStatement) }); } - enterEnvironment(QQmlJSScope::JSLexicalScope, "with"); - return true; -} - -void FindWarningVisitor::endVisit(QQmlJS::AST::WithStatement *) -{ - leaveEnvironment(); + return QQmlJSImportVisitor::visit(withStatement); } static QString signalName(QStringView handlerName) @@ -247,85 +171,61 @@ static QString signalName(QStringView handlerName) bool FindWarningVisitor::visit(QQmlJS::AST::UiScriptBinding *uisb) { using namespace QQmlJS::AST; + + if (!QQmlJSImportVisitor::visit(uisb)) + return false; + auto name = uisb->qualifiedId->name; if (name == QLatin1String("id")) { - // found id - auto expstat = cast<ExpressionStatement *>(uisb->statement); - auto identexp = cast<IdentifierExpression *>(expstat->expression); - m_scopesById.insert(identexp->name.toString(), m_currentScope); - // Figure out whether the current scope is the root scope. if (auto parentScope = m_currentScope->parentScope()) { - if (!parentScope->parentScope()) + if (!parentScope->parentScope()) { + const auto expstat = cast<ExpressionStatement *>(uisb->statement); + const auto identexp = cast<IdentifierExpression *>(expstat->expression); m_rootId = identexp->name.toString(); - } - } else { - const QString signal = signalName(name); - if (signal.isEmpty()) - return true; - - if (!m_currentScope->methods().contains(signal) && m_warnUnqualified) { - m_errors.append({ - QStringLiteral("no matching signal found for handler \"%1\"\n") - .arg(name.toString()), - QtWarningMsg, - uisb->firstSourceLocation() - }); - return true; - } - - const auto statement = uisb->statement; - if (statement->kind == Node::Kind::Kind_ExpressionStatement) { - if (cast<ExpressionStatement *>(statement)->expression->asFunctionDefinition()) { - // functions are already handled - // they do not get names inserted according to the signal, but access their formal - // parameters - return true; } } + return true; + } - const auto methods = m_currentScope->methods(); - const auto methodsRange = methods.equal_range(signal); - for (auto method = methodsRange.first; method != methodsRange.second; ++method) { - if (method->methodType() != QQmlJSMetaMethod::Signal) - continue; - - 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. - } + const QString signal = signalName(name); + if (signal.isEmpty()) + return true; + + if (!m_currentScope->methods().contains(signal) && m_warnUnqualified) { + m_errors.append({ + QStringLiteral("no matching signal found for handler \"%1\"\n") + .arg(name.toString()), + QtWarningMsg, + uisb->firstSourceLocation() + }); return true; } - return true; -} -bool FindWarningVisitor::visit(QQmlJS::AST::UiPublicMember *uipm) -{ - if (uipm->type == QQmlJS::AST::UiPublicMember::Signal) { - QQmlJSMetaMethod method; - method.setMethodType(QQmlJSMetaMethod::Signal); - method.setMethodName(uipm->name.toString()); - QQmlJS::AST::UiParameterList *param = uipm->parameters; - while (param) { - method.addParameter(param->name.toString(), param->type->name.toString()); - param = param->next; + const auto statement = uisb->statement; + if (statement->kind == Node::Kind::Kind_ExpressionStatement) { + if (cast<ExpressionStatement *>(statement)->expression->asFunctionDefinition()) { + // functions are already handled + // they do not get names inserted according to the signal, but access their formal + // parameters + return true; } - m_currentScope->addMethod(method); - } else { - // property bool inactive: !active - // extract name inactive - QQmlJSMetaProperty property( - uipm->name.toString(), - // TODO: complex types etc. - uipm->memberType ? uipm->memberType->name.toString() : QString(), - uipm->typeModifier == QLatin1String("list"), !uipm->isReadonlyMember, false, - uipm->memberType ? (uipm->memberType->name == QLatin1String("alias")) : false, 0); - property.setType(m_rootScopeImports.value(property.typeName())); - m_currentScope->insertPropertyIdentifier(property); } + + const auto methods = m_currentScope->methods(); + const auto methodsRange = methods.equal_range(signal); + for (auto method = methodsRange.first; method != methodsRange.second; ++method) { + if (method->methodType() != QQmlJSMetaMethod::Signal) + continue; + + 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; } @@ -432,92 +332,27 @@ bool FindWarningVisitor::check() return check(m_scopesById, m_signalHandlers, m_memberAccessChains, m_globalScope, m_rootId); } -bool FindWarningVisitor::visit(QQmlJS::AST::VariableDeclarationList *vdl) -{ - while (vdl) { - m_currentScope->insertJSIdentifier( - vdl->declaration->bindingIdentifier.toString(), - { - (vdl->declaration->scope == QQmlJS::AST::VariableScope::Var) - ? QQmlJSScope::JavaScriptIdentifier::FunctionScoped - : QQmlJSScope::JavaScriptIdentifier::LexicalScoped, - vdl->declaration->firstSourceLocation() - }); - vdl = vdl->next; - } - return true; -} - -bool FindWarningVisitor::visit(QQmlJS::AST::FormalParameterList *fpl) -{ - for (auto const &boundName : fpl->boundNames()) { - m_currentScope->insertJSIdentifier( - boundName.id, { - QQmlJSScope::JavaScriptIdentifier::Parameter, - fpl->firstSourceLocation() - }); - } - return true; -} - -bool FindWarningVisitor::visit(QQmlJS::AST::UiEnumDeclaration *uied) -{ - QQmlJSMetaEnum qmlEnum(uied->name.toString()); - for (const auto *member = uied->members; member; member = member->next) - qmlEnum.addKey(member->member.toString()); - m_currentScope->addEnum(qmlEnum); - return true; -} - bool FindWarningVisitor::visit(QQmlJS::AST::UiObjectBinding *uiob) { - // property QtObject __styleData: QtObject {...} - - QString name; - for (auto id = uiob->qualifiedTypeNameId; id; id = id->next) - name += id->name.toString() + QLatin1Char('.'); - - name.chop(1); - - QQmlJSMetaProperty prop(uiob->qualifiedId->name.toString(), name, false, true, true, - name == QLatin1String("alias"), 0); - prop.setType(m_rootScopeImports.value(uiob->qualifiedTypeNameId->name.toString())); - m_currentScope->addProperty(prop); + if (!QQmlJSImportVisitor::visit(uiob)) + return false; - enterEnvironment(QQmlJSScope::QMLScope, name); - m_currentScope->resolveTypes(m_rootScopeImports); - importExportedNames(m_currentScope); + checkInheritanceCycle(m_currentScope); return true; } -void FindWarningVisitor::endVisit(QQmlJS::AST::UiObjectBinding *uiob) -{ - const QQmlJSScope::ConstPtr childScope = m_currentScope; - leaveEnvironment(); - QQmlJSMetaProperty property(uiob->qualifiedId->name.toString(), - uiob->qualifiedTypeNameId->name.toString(), - false, true, true, - uiob->qualifiedTypeNameId->name == QLatin1String("alias"), - 0); - property.setType(childScope); - m_currentScope->addProperty(property); -} - bool FindWarningVisitor::visit(QQmlJS::AST::UiObjectDefinition *uiod) { using namespace QQmlJS::AST; - QString name; - for (auto id = uiod->qualifiedTypeNameId; id; id = id->next) - name += id->name.toString() + QLatin1Char('.'); + if (!QQmlJSImportVisitor::visit(uiod)) + return false; - name.chop(1); - enterEnvironment(QQmlJSScope::QMLScope, name); - if (name.isLower()) + const QString name = m_currentScope->baseTypeName(); + if (name.isEmpty() || name.front().isLower()) return false; // Ignore grouped properties for now - m_currentScope->resolveTypes(m_rootScopeImports); - importExportedNames(m_currentScope); + checkInheritanceCycle(m_currentScope); if (name.endsWith("Connections")) { QString target; @@ -582,10 +417,10 @@ bool FindWarningVisitor::visit(QQmlJS::AST::PatternElement *element) return true; } -void FindWarningVisitor::endVisit(QQmlJS::AST::UiObjectDefinition *) +void FindWarningVisitor::endVisit(QQmlJS::AST::UiObjectDefinition *uiod) { auto childScope = m_currentScope; - leaveEnvironment(); + QQmlJSImportVisitor::endVisit(uiod); if (m_currentScope == m_globalScope || m_currentScope->baseTypeName() == QStringLiteral("Component")) { diff --git a/tools/qmllint/findwarnings.h b/tools/qmllint/findwarnings.h index da32f4c9d5..f60a003a4d 100644 --- a/tools/qmllint/findwarnings.h +++ b/tools/qmllint/findwarnings.h @@ -90,7 +90,7 @@ private: QVarLengthArray<OutstandingConnection, 3> m_outstandingConnections; // Connections whose target we have not encountered - void importExportedNames(QQmlJSScope::ConstPtr scope); + void checkInheritanceCycle(QQmlJSScope::ConstPtr scope); void parseHeaders(QQmlJS::AST::UiHeaderItemList *headers); QQmlJSScope::Ptr parseProgram(QQmlJS::AST::Program *program, const QString &name); @@ -99,42 +99,17 @@ private: void throwRecursionDepthError() override; // start block/scope handling - bool visit(QQmlJS::AST::ClassDeclaration *ast) override; - void endVisit(QQmlJS::AST::ClassDeclaration *ast) override; - - bool visit(QQmlJS::AST::ForStatement *ast) override; - void endVisit(QQmlJS::AST::ForStatement *ast) override; - - 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; - - bool visit(QQmlJS::AST::CaseBlock *ast) override; - void endVisit(QQmlJS::AST::CaseBlock *ast) override; - - bool visit(QQmlJS::AST::Catch *ast) override; - void endVisit(QQmlJS::AST::Catch *ast) override; - bool visit(QQmlJS::AST::WithStatement *withStatement) override; - void endVisit(QQmlJS::AST::WithStatement *ast) override; /* --- end block handling --- */ - bool visit(QQmlJS::AST::VariableDeclarationList *vdl) override; - bool visit(QQmlJS::AST::FormalParameterList *fpl) override; - - bool visit(QQmlJS::AST::UiEnumDeclaration *uied) override; bool visit(QQmlJS::AST::UiObjectBinding *uiob) override; - void endVisit(QQmlJS::AST::UiObjectBinding *uiob) override; bool visit(QQmlJS::AST::UiObjectDefinition *uiod) override; void endVisit(QQmlJS::AST::UiObjectDefinition *) override; bool visit(QQmlJS::AST::UiScriptBinding *uisb) override; - bool visit(QQmlJS::AST::UiPublicMember *uipm) override; // expression handling bool visit(QQmlJS::AST::IdentifierExpression *idexp) override; |