From 678d4ec4b68d43d0c748a8ec62a13716f83a27cf Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Fri, 9 Oct 2020 12:02:22 +0200 Subject: Unify QQmlJSImportVisitor and FindWarningsVisitor They are both pretty much doing the same thing, except that the import visitor is not as thorough. We need the full analysis in QtQmlCompiler, so we successively move the code over. Change-Id: If7fb47f88165fd8b61f4ccc408ccfbb7dad533e6 Reviewed-by: Fabian Kosmale --- tools/qmllint/checkidentifiers.cpp | 4 +- tools/qmllint/findwarnings.cpp | 168 ++++--------------------------------- tools/qmllint/findwarnings.h | 30 +------ tools/qmllint/main.cpp | 3 +- 4 files changed, 21 insertions(+), 184 deletions(-) (limited to 'tools/qmllint') diff --git a/tools/qmllint/checkidentifiers.cpp b/tools/qmllint/checkidentifiers.cpp index b7d331521c..09c39ddd63 100644 --- a/tools/qmllint/checkidentifiers.cpp +++ b/tools/qmllint/checkidentifiers.cpp @@ -373,8 +373,8 @@ bool CheckIdentifiers::operator()( printContext(m_code, m_colorOut, location); - // root(JS) --> program(qml) --> (first element) - const auto firstElement = root->childScopes()[0]->childScopes()[0]; + // root(JS) --> (first element) + const auto firstElement = root->childScopes()[0]; if (firstElement->properties().contains(memberAccessBase.m_name) || firstElement->methods().contains(memberAccessBase.m_name) || firstElement->enums().contains(memberAccessBase.m_name)) { diff --git a/tools/qmllint/findwarnings.cpp b/tools/qmllint/findwarnings.cpp index 962ccaa1c6..6f8c64aabc 100644 --- a/tools/qmllint/findwarnings.cpp +++ b/tools/qmllint/findwarnings.cpp @@ -43,18 +43,6 @@ #include #include -void FindWarningVisitor::enterEnvironment(QQmlJSScope::ScopeType type, const QString &name) -{ - m_currentScope = QQmlJSScope::create(type, m_currentScope); - m_currentScope->setBaseTypeName(name); - m_currentScope->setIsComposite(true); -} - -void FindWarningVisitor::leaveEnvironment() -{ - m_currentScope = m_currentScope->parentScope(); -} - void FindWarningVisitor::importExportedNames(QQmlJSScope::ConstPtr scope) { QList scopes; @@ -123,47 +111,10 @@ void FindWarningVisitor::flushPendingSignalParameters() void FindWarningVisitor::throwRecursionDepthError() { - m_errors.append({ - QStringLiteral("Maximum statement or expression depth exceeded"), - QtCriticalMsg, - QQmlJS::SourceLocation() - }); + QQmlJSImportVisitor::throwRecursionDepthError(); m_visitFailed = true; } -bool FindWarningVisitor::visit(QQmlJS::AST::UiProgram *) -{ - enterEnvironment(QQmlJSScope::QMLScope, "program"); - m_rootScopeImports = m_importer.importBuiltins(); - - if (!m_qmltypesFiles.isEmpty()) { - const auto baseTypes = m_importer.importQmltypes(m_qmltypesFiles); - m_rootScopeImports.insert(baseTypes); - } - - const auto imported = m_importer.importDirectory(QFileInfo(m_filePath).canonicalPath()); - m_rootScopeImports.insert(imported); - - m_errors.append(m_importer.takeWarnings()); - return true; -} - -void FindWarningVisitor::endVisit(QQmlJS::AST::UiProgram *) -{ - leaveEnvironment(); -} - -bool FindWarningVisitor::visit(QQmlJS::AST::ClassExpression *ast) -{ - enterEnvironment(QQmlJSScope::JSFunctionScope, ast->name.toString()); - return true; -} - -void FindWarningVisitor::endVisit(QQmlJS::AST::ClassExpression *) -{ - leaveEnvironment(); -} - bool FindWarningVisitor::visit(QQmlJS::AST::ClassDeclaration *ast) { enterEnvironment(QQmlJSScope::JSFunctionScope, ast->name.toString()); @@ -301,14 +252,12 @@ bool FindWarningVisitor::visit(QQmlJS::AST::UiScriptBinding *uisb) // found id auto expstat = cast(uisb->statement); auto identexp = cast(expstat->expression); - m_qmlid2scope.insert(identexp->name.toString(), m_currentScope); + 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 (auto grandParentScope = parentScope->parentScope()) { - if (!grandParentScope->parentScope()) - m_rootId = identexp->name.toString(); - } + if (!parentScope->parentScope()) + m_rootId = identexp->name.toString(); } } else { const QString signal = signalName(name); @@ -389,21 +338,18 @@ bool FindWarningVisitor::visit(QQmlJS::AST::IdentifierExpression *idexp) } FindWarningVisitor::FindWarningVisitor( - QStringList qmlImportPaths, QStringList qmltypesFiles, QString code, QString fileName, + QQmlJSImporter *importer, QStringList qmltypesFiles, QString code, QString fileName, bool silent, bool warnUnqualified, bool warnWithStatement, bool warnInheritanceCycle) - : m_rootScope(QQmlJSScope::create(QQmlJSScope::JSFunctionScope)), - m_qmltypesFiles(std::move(qmltypesFiles)), + : QQmlJSImportVisitor(importer, QFileInfo {fileName}.canonicalPath(), qmltypesFiles), m_code(std::move(code)), m_rootId(QLatin1String("")), m_filePath(std::move(fileName)), m_colorOut(silent), m_warnUnqualified(warnUnqualified), m_warnWithStatement(warnWithStatement), - m_warnInheritanceCycle(warnInheritanceCycle), - m_importer(qmlImportPaths) + m_warnInheritanceCycle(warnInheritanceCycle) { - m_rootScope->setInternalName("global"); - m_currentScope = m_rootScope; + m_currentScope->setInternalName("global"); // setup color output m_colorOut.insertMapping(Error, ColorOutput::RedForeground); @@ -472,7 +418,7 @@ bool FindWarningVisitor::check() // now that all ids are known, revisit any Connections whose target were perviously unknown for (auto const &outstandingConnection: m_outstandingConnections) { - auto targetScope = m_qmlid2scope[outstandingConnection.targetName]; + auto targetScope = m_scopesById[outstandingConnection.targetName]; if (outstandingConnection.scope && !targetScope.isNull()) outstandingConnection.scope->addMethods(targetScope->methods()); QScopedValueRollback rollback(m_currentScope, outstandingConnection.scope); @@ -483,7 +429,7 @@ bool FindWarningVisitor::check() return true; CheckIdentifiers check(&m_colorOut, m_code, m_rootScopeImports, m_filePath); - return check(m_qmlid2scope, m_signalHandlers, m_memberAccessChains, m_rootScope, m_rootId); + return check(m_scopesById, m_signalHandlers, m_memberAccessChains, m_globalScope, m_rootId); } bool FindWarningVisitor::visit(QQmlJS::AST::VariableDeclarationList *vdl) @@ -502,48 +448,6 @@ bool FindWarningVisitor::visit(QQmlJS::AST::VariableDeclarationList *vdl) return true; } -void FindWarningVisitor::visitFunctionExpressionHelper(QQmlJS::AST::FunctionExpression *fexpr) -{ - using namespace QQmlJS::AST; - auto name = fexpr->name.toString(); - if (!name.isEmpty()) { - if (m_currentScope->scopeType() == QQmlJSScope::QMLScope) { - m_currentScope->addMethod(QQmlJSMetaMethod(name, QLatin1String("void"))); - } else { - m_currentScope->insertJSIdentifier( - name, { - QQmlJSScope::JavaScriptIdentifier::LexicalScoped, - fexpr->firstSourceLocation() - }); - } - enterEnvironment(QQmlJSScope::JSFunctionScope, name); - } else { - enterEnvironment(QQmlJSScope::JSFunctionScope, QLatin1String("")); - } -} - -bool FindWarningVisitor::visit(QQmlJS::AST::FunctionExpression *fexpr) -{ - visitFunctionExpressionHelper(fexpr); - return true; -} - -void FindWarningVisitor::endVisit(QQmlJS::AST::FunctionExpression *) -{ - leaveEnvironment(); -} - -bool FindWarningVisitor::visit(QQmlJS::AST::FunctionDeclaration *fdecl) -{ - visitFunctionExpressionHelper(fdecl); - return true; -} - -void FindWarningVisitor::endVisit(QQmlJS::AST::FunctionDeclaration *) -{ - leaveEnvironment(); -} - bool FindWarningVisitor::visit(QQmlJS::AST::FormalParameterList *fpl) { for (auto const &boundName : fpl->boundNames()) { @@ -556,50 +460,6 @@ bool FindWarningVisitor::visit(QQmlJS::AST::FormalParameterList *fpl) return true; } -bool FindWarningVisitor::visit(QQmlJS::AST::UiImport *import) -{ - // construct path - QString prefix = QLatin1String(""); - if (import->asToken.isValid()) { - prefix += import->importId; - } - auto filename = import->fileName.toString(); - if (!filename.isEmpty()) { - const QFileInfo file(filename); - const QFileInfo path(file.isRelative() ? QFileInfo(m_filePath).dir().filePath(filename) - : filename); - if (path.isDir()) { - m_rootScopeImports.insert(m_importer.importDirectory(path.canonicalFilePath(), prefix)); - } else if (path.isFile()) { - const auto scope = m_importer.importFile(path.canonicalFilePath()); - m_rootScopeImports.insert(prefix.isEmpty() ? scope->internalName() : prefix, scope); - } - - } - - QString path {}; - if (!import->importId.isEmpty()) { - // TODO: do not put imported ids into the same space as qml IDs - const QString importId = import->importId.toString(); - m_qmlid2scope.insert(importId, m_rootScopeImports.value(importId)); - } - auto uri = import->importUri; - while (uri) { - path.append(uri->name); - path.append("/"); - uri = uri->next; - } - path.chop(1); - - const auto imported = m_importer.importModule( - path, prefix, import->version ? import->version->version : QTypeRevision()); - - m_rootScopeImports.insert(imported); - - m_errors.append(m_importer.takeWarnings()); - return true; -} - bool FindWarningVisitor::visit(QQmlJS::AST::UiEnumDeclaration *uied) { QQmlJSMetaEnum qmlEnum(uied->name.toString()); @@ -689,8 +549,8 @@ bool FindWarningVisitor::visit(QQmlJS::AST::UiObjectDefinition *uiod) targetScope = m_rootScopeImports.value(scope->baseTypeName()); } else { // there was a target, check if we already can find it - auto scopeIt = m_qmlid2scope.find(target); - if (scopeIt != m_qmlid2scope.end()) { + auto scopeIt = m_scopesById.find(target); + if (scopeIt != m_scopesById.end()) { targetScope = *scopeIt; } else { m_outstandingConnections.push_back({target, m_currentScope, uiod}); @@ -727,8 +587,8 @@ void FindWarningVisitor::endVisit(QQmlJS::AST::UiObjectDefinition *) auto childScope = m_currentScope; leaveEnvironment(); - if (m_currentScope->baseTypeName() == QStringLiteral("Component") - || m_currentScope->baseTypeName() == QStringLiteral("program")) { + if (m_currentScope == m_globalScope + || m_currentScope->baseTypeName() == QStringLiteral("Component")) { return; } diff --git a/tools/qmllint/findwarnings.h b/tools/qmllint/findwarnings.h index 7d07114b32..da32f4c9d5 100644 --- a/tools/qmllint/findwarnings.h +++ b/tools/qmllint/findwarnings.h @@ -45,6 +45,7 @@ #include #include #include +#include #include #include @@ -52,34 +53,27 @@ #include -class FindWarningVisitor : public QQmlJS::AST::Visitor +class FindWarningVisitor : public QQmlJSImportVisitor { Q_DISABLE_COPY_MOVE(FindWarningVisitor) public: explicit FindWarningVisitor( - QStringList qmlImportPaths, QStringList qmltypesFiles, QString code, QString fileName, + QQmlJSImporter *importer, QStringList qmltypesFiles, QString code, QString fileName, bool silent, bool warnUnqualified, bool warnWithStatement, bool warnInheritanceCycle); ~FindWarningVisitor() override = default; bool check(); private: - QQmlJSImporter::ImportedTypes m_rootScopeImports; - QHash m_signalHandlers; QQmlJS::SourceLocation m_pendingSingalHandler; MemberAccessChains m_memberAccessChains; - QQmlJSScope::Ptr m_rootScope; - QQmlJSScope::Ptr m_currentScope; QQmlJS::AST::ExpressionNode *m_fieldMemberBase = nullptr; - QStringList m_qmltypesFiles; QString m_code; - QHash m_qmlid2scope; QString m_rootId; QString m_filePath; QSet m_unknownImports; - QList m_errors; ColorOutput m_colorOut; bool m_visitFailed = false; @@ -96,11 +90,6 @@ private: QVarLengthArray m_outstandingConnections; // Connections whose target we have not encountered - QQmlJSImporter m_importer; - - void enterEnvironment(QQmlJSScope::ScopeType type, const QString &name); - void leaveEnvironment(); - void importExportedNames(QQmlJSScope::ConstPtr scope); void parseHeaders(QQmlJS::AST::UiHeaderItemList *headers); @@ -110,12 +99,6 @@ private: void throwRecursionDepthError() override; // start block/scope handling - bool visit(QQmlJS::AST::UiProgram *ast) override; - void endVisit(QQmlJS::AST::UiProgram *ast) override; - - bool visit(QQmlJS::AST::ClassExpression *ast) override; - void endVisit(QQmlJS::AST::ClassExpression *ast) override; - bool visit(QQmlJS::AST::ClassDeclaration *ast) override; void endVisit(QQmlJS::AST::ClassDeclaration *ast) override; @@ -140,18 +123,11 @@ private: bool visit(QQmlJS::AST::WithStatement *withStatement) override; void endVisit(QQmlJS::AST::WithStatement *ast) override; - void visitFunctionExpressionHelper(QQmlJS::AST::FunctionExpression *fexpr); - bool visit(QQmlJS::AST::FunctionExpression *fexpr) override; - void endVisit(QQmlJS::AST::FunctionExpression *fexpr) override; - - bool visit(QQmlJS::AST::FunctionDeclaration *fdecl) override; - void endVisit(QQmlJS::AST::FunctionDeclaration *fdecl) override; /* --- end block handling --- */ bool visit(QQmlJS::AST::VariableDeclarationList *vdl) override; bool visit(QQmlJS::AST::FormalParameterList *fpl) override; - bool visit(QQmlJS::AST::UiImport *import) override; bool visit(QQmlJS::AST::UiEnumDeclaration *uied) override; bool visit(QQmlJS::AST::UiObjectBinding *uiob) override; void endVisit(QQmlJS::AST::UiObjectBinding *uiob) override; diff --git a/tools/qmllint/main.cpp b/tools/qmllint/main.cpp index 1112f64d7c..a877b20335 100644 --- a/tools/qmllint/main.cpp +++ b/tools/qmllint/main.cpp @@ -86,7 +86,8 @@ static bool lint_file(const QString &filename, const bool silent, const bool war if (success && !isJavaScript) { auto root = parser.rootNode(); - FindWarningVisitor v { qmlImportPaths, qmltypesFiles, code, filename, silent, + QQmlJSImporter importer(qmlImportPaths); + FindWarningVisitor v { &importer, qmltypesFiles, code, filename, silent, warnUnqualified, warnWithStatement, warnInheritanceCycle }; root->accept(&v); success = v.check(); -- cgit v1.2.3