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>

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;