qmllint: Re-enable warning about automatched signal handler

... and give a hint on what to do about it. In order to not duplicate
all the warnings from the importer, make sure it runs only once.

Change-Id: Ie2b314ff659664f7c84c20cc7971c094e15c59cf
Reviewed-by: Maximilian Goldstein <max.goldstein@qt.io>

5 files changed, 22 insertions, 107 deletions

diff --git a/src/qmllint/codegen.cpp b/src/qmllint/codegen.cpp
index 9bc8cbbe0d..a4908ba1b9 100644
--- a/src/qmllint/codegen.cpp
+++ b/src/qmllint/codegen.cpp
@@ -36,12 +36,10 @@
 
 QT_BEGIN_NAMESPACE
 
-Codegen::Codegen(QQmlJSImporter *importer, const QString &fileName,
-                 const QStringList &qmltypesFiles, QQmlJSLogger *logger, QQmlJSTypeInfo *typeInfo,
-                 const QString &code)
+Codegen::Codegen(const QString &fileName, const QStringList &qmltypesFiles, QQmlJSLogger *logger,
+                 QQmlJSTypeInfo *typeInfo, const QString &code)
     : m_fileName(fileName),
       m_qmltypesFiles(qmltypesFiles),
-      m_importer(importer),
       m_logger(logger),
       m_typeInfo(typeInfo),
       m_code(code)
@@ -55,26 +53,6 @@ void Codegen::setDocument(QmlIR::JSCodeGen *codegen, QmlIR::Document *document)
     m_pool = document->jsParserEngine.pool();
     m_unitGenerator = &document->jsGenerator;
     m_entireSourceCodeLines = document->code.split(u'\n');
-    m_typeResolver = std::make_unique<QQmlJSTypeResolver>(
-            m_importer, document, QQmlJSTypeResolver::Indirect, m_logger);
-
-    // Type resolving is using document parent mode here so that it produces fewer false positives
-    // on the "parent" property of QQuickItem. It does produce a few false negatives this way
-    // because items can be reparented. Furthermore, even if items are not reparented, the document
-    // parent may indeed not be their visual parent. See QTBUG-95530. Eventually, we'll need
-    // cleverer logic to deal with this.
-    m_typeResolver->setParentMode(QQmlJSTypeResolver::UseDocumentParent);
-
-    // TODO: using silentLogger for visitor actually hides potential issues but
-    // using m_logger instead fails some tests, so for now let's leave the old
-    // behavior for consistency. the proper fix is anyway to remove this visitor
-    // and use FindWarningsVisitor instead.
-    QQmlJSLogger silentLogger(m_fileName, document->code, /* silent */ true);
-    QQmlJSImportVisitor visitor(m_importer, &silentLogger,
-                                QQmlJSImportVisitor::implicitImportDirectory(
-                                        m_fileName, m_importer->resourceFileMapper()),
-                                m_qmltypesFiles);
-    m_typeResolver->init(visitor);
 }
 
 void Codegen::setScope(const QmlIR::Object *object, const QmlIR::Object *scope)
diff --git a/src/qmllint/codegen_p.h b/src/qmllint/codegen_p.h
index 481f8e2341..b59a97da2e 100644
--- a/src/qmllint/codegen_p.h
+++ b/src/qmllint/codegen_p.h
@@ -59,7 +59,7 @@ QT_BEGIN_NAMESPACE
 class Codegen : public QQmlJSAotCompiler
 {
 public:
-    Codegen(QQmlJSImporter *importer, const QString &fileName, const QStringList &qmltypesFiles,
+    Codegen(const QString &fileName, const QStringList &qmltypesFiles,
             QQmlJSLogger *logger, QQmlJSTypeInfo *typeInfo, const QString &m_code);
 
     void setDocument(QmlIR::JSCodeGen *codegen, QmlIR::Document *document) override;
@@ -72,6 +72,11 @@ public:
 
     QQmlJSAotFunction globalCode() const override;
 
+    void setTypeResolver(std::unique_ptr<QQmlJSTypeResolver> typeResolver)
+    {
+        m_typeResolver = std::move(typeResolver);
+    }
+
 private:
     using Function = QQmlJSCompilePass::Function;
 
@@ -79,7 +84,6 @@ private:
     const QString m_fileName;
     const QStringList m_resourceFiles;
     const QStringList m_qmltypesFiles;
-    QQmlJSImporter *m_importer = nullptr;
 
     QQmlJS::MemoryPool *m_pool = nullptr;
     const QmlIR::Object *m_currentObject = nullptr;
diff --git a/src/qmllint/findwarnings.cpp b/src/qmllint/findwarnings.cpp
index d06dc59c5c..60f465a97b 100644
--- a/src/qmllint/findwarnings.cpp
+++ b/src/qmllint/findwarnings.cpp
@@ -43,66 +43,6 @@
 
 QT_BEGIN_NAMESPACE
 
-bool FindWarningVisitor::visit(QQmlJS::AST::UiObjectDefinition *uiod)
-{
-    QQmlJSImportVisitor::visit(uiod);
-
-    const QString name = m_currentScope->baseTypeName();
-    if (name.endsWith(u"Connections"_qs)) {
-        QString target;
-        auto member = uiod->initializer->members;
-        while (member) {
-            if (member->member->kind == QQmlJS::AST::Node::Kind_UiScriptBinding) {
-                auto asBinding = static_cast<QQmlJS::AST::UiScriptBinding *>(member->member);
-                if (asBinding->qualifiedId->name == u"target"_qs) {
-                    if (asBinding->statement->kind == QQmlJS::AST::Node::Kind_ExpressionStatement) {
-                        auto expr = static_cast<QQmlJS::AST::ExpressionStatement *>(
-                                            asBinding->statement)
-                                            ->expression;
-                        if (auto idexpr =
-                                    QQmlJS::AST::cast<QQmlJS::AST::IdentifierExpression *>(expr)) {
-                            target = idexpr->name.toString();
-                        } else {
-                            // more complex expressions are not supported
-                        }
-                    }
-                    break;
-                }
-            }
-            member = member->next;
-        }
-        QQmlJSScope::ConstPtr targetScope;
-        if (target.isEmpty()) {
-            // no target set, connection comes from parentF
-            QQmlJSScope::Ptr scope = m_currentScope;
-            do {
-                if (auto parentScope = scope->parentScope(); !parentScope.isNull())
-                    scope = parentScope;
-                else
-                    break;
-            } while (scope->scopeType() != QQmlJSScope::QMLScope);
-            targetScope = m_rootScopeImports.value(scope->baseTypeName());
-        } else {
-            // there was a target, check if we already can find it
-            auto scopeIt = m_scopesById.find(target);
-            if (scopeIt != m_scopesById.end()) {
-                targetScope = *scopeIt;
-            } else {
-                m_outstandingConnections.push_back({ target, m_currentScope, uiod });
-                return false; // visit children later once target is known
-            }
-        }
-        for (const auto scope = targetScope; targetScope; targetScope = targetScope->baseType()) {
-            const auto connectionMethods = targetScope->ownMethods();
-            for (const auto &method : connectionMethods)
-                m_currentScope->addOwnMethod(method);
-        }
-    }
-
-    m_objectDefinitionScopes << m_currentScope;
-    return true;
-}
-
 void FindWarningVisitor::endVisit(QQmlJS::AST::UiObjectDefinition *uiod)
 {
     auto childScope = m_currentScope;
@@ -223,22 +163,6 @@ void FindWarningVisitor::parseComments(const QList<QQmlJS::SourceLocation> &comm
 
 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_scopesById[outstandingConnection.targetName];
-        if (outstandingConnection.scope) {
-            for (const auto scope = targetScope; targetScope;
-                 targetScope = targetScope->baseType()) {
-                const auto connectionMethods = targetScope->ownMethods();
-                for (const auto &method : connectionMethods)
-                    outstandingConnection.scope->addOwnMethod(method);
-            }
-        }
-        QScopedValueRollback<QQmlJSScope::Ptr> rollback(m_currentScope,
-                                                        outstandingConnection.scope);
-        outstandingConnection.uiod->initializer->accept(this);
-    }
-
     return !m_logger->hasWarnings() && !m_logger->hasErrors();
 }
 
diff --git a/src/qmllint/findwarnings_p.h b/src/qmllint/findwarnings_p.h
index 599eaf2dc8..48addf1804 100644
--- a/src/qmllint/findwarnings_p.h
+++ b/src/qmllint/findwarnings_p.h
@@ -69,7 +69,6 @@ private:
     using QQmlJSImportVisitor::endVisit;
     using QQmlJSImportVisitor::visit;
 
-    bool visit(QQmlJS::AST::UiObjectDefinition *uiod) override;
     void endVisit(QQmlJS::AST::UiObjectDefinition *uiod) override;
 };
 
diff --git a/src/qmllint/qqmllinter.cpp b/src/qmllint/qqmllinter.cpp
index 2a4fa513e3..a6fa6b8101 100644
--- a/src/qmllint/qqmllinter.cpp
+++ b/src/qmllint/qqmllinter.cpp
@@ -184,16 +184,26 @@ bool QQmlLinter::lintFile(const QString &filename, const QString *fileContents,
                 m_logger->setCategoryLevel(it.value().m_category, it.value().m_level);
             }
 
-            parser.rootNode()->accept(&v);
+            std::unique_ptr<QQmlJSTypeResolver> typeResolver
+                    = std::make_unique<QQmlJSTypeResolver>(
+                        &m_importer, QQmlJSTypeResolver::TypeStorage::Indirect, m_logger.get());
+
+            // Type resolving is using document parent mode here so that it produces fewer false positives
+            // on the "parent" property of QQuickItem. It does produce a few false negatives this way
+            // because items can be reparented. Furthermore, even if items are not reparented, the document
+            // parent may indeed not be their visual parent. See QTBUG-95530. Eventually, we'll need
+            // cleverer logic to deal with this.
+            typeResolver->setParentMode(QQmlJSTypeResolver::UseDocumentParent);
+
+            typeResolver->init(&v, parser.rootNode());
             success = v.check();
 
             if (m_logger->hasErrors())
                 return;
 
             QQmlJSTypeInfo typeInfo;
-            Codegen codegen {
-                &m_importer, filename, qmltypesFiles, m_logger.get(), &typeInfo, code
-            };
+            Codegen codegen { filename, qmltypesFiles, m_logger.get(), &typeInfo, code };
+            codegen.setTypeResolver(std::move(typeResolver));
             QQmlJSSaveFunction saveFunction = [](const QV4::CompiledData::SaveableUnitPointer &,
                                                  const QQmlJSAotFunctionMap &,
                                                  QString *) { return true; };