diff options
author | Ulf Hermann <ulf.hermann@qt.io> | 2020-10-09 12:02:22 +0200 |
---|---|---|
committer | Ulf Hermann <ulf.hermann@qt.io> | 2020-10-15 10:44:25 +0200 |
commit | 678d4ec4b68d43d0c748a8ec62a13716f83a27cf (patch) | |
tree | 863c228d5f0c46d494f2aee24e65f7ef21ee9e77 /tools/qmllint | |
parent | f98c961da6d039621ae40ab6c1a79c4b06efb83f (diff) |
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 <fabian.kosmale@qt.io>
Diffstat (limited to 'tools/qmllint')
-rw-r--r-- | tools/qmllint/checkidentifiers.cpp | 4 | ||||
-rw-r--r-- | tools/qmllint/findwarnings.cpp | 168 | ||||
-rw-r--r-- | tools/qmllint/findwarnings.h | 30 | ||||
-rw-r--r-- | tools/qmllint/main.cpp | 3 |
4 files changed, 21 insertions, 184 deletions
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 <QtCore/qdiriterator.h> #include <QtCore/qscopedvaluerollback.h> -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<QQmlJSScope::ConstPtr> 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<ExpressionStatement *>(uisb->statement); auto identexp = cast<IdentifierExpression *>(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("<id>")), 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<QQmlJSScope::Ptr> 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("<anon>")); - } -} - -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 <QtQmlCompiler/private/qqmljstypedescriptionreader_p.h> #include <QtQmlCompiler/private/qqmljsscope_p.h> #include <QtQmlCompiler/private/qqmljsimporter_p.h> +#include <QtQmlCompiler/private/qqmljsimportvisitor_p.h> #include <QtQml/private/qqmldirparser_p.h> #include <QtQml/private/qqmljsastvisitor_p.h> @@ -52,34 +53,27 @@ #include <QtCore/qscopedpointer.h> -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<QQmlJS::SourceLocation, SignalHandler> 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<QString, QQmlJSScope::ConstPtr> m_qmlid2scope; QString m_rootId; QString m_filePath; QSet<QString> m_unknownImports; - QList<QQmlJS::DiagnosticMessage> m_errors; ColorOutput m_colorOut; bool m_visitFailed = false; @@ -96,11 +90,6 @@ private: QVarLengthArray<OutstandingConnection, 3> 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(); |