QmlCompiler: Properly discern between inherited and own names

Previously, we would mix them up on importExportedNames(), which was
also misnamed. Now we keep them in their proper place in the scope
hierarchy, so that we can identify which scope a property came from.
Exceptions are the qmllint-specific treatment of parent properties and
Connections elements.

Change-Id: I7c012388b16c83439d6f2de2e83fac0da4940d30
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

2 files changed, 48 insertions, 43 deletions

diff --git a/tools/qmllint/checkidentifiers.cpp b/tools/qmllint/checkidentifiers.cpp
index 09c39ddd63..86ac84fcf1 100644
--- a/tools/qmllint/checkidentifiers.cpp
+++ b/tools/qmllint/checkidentifiers.cpp
@@ -149,12 +149,11 @@ bool CheckIdentifiers::checkMemberAccess(const QVector<FieldMember> &members,
             return false;
         }
 
-        const auto properties = scope->properties();
-        const auto scopeIt = properties.find(access.m_name);
-        if (scopeIt != properties.end()) {
-            const QString typeName = access.m_parentType.isEmpty() ? scopeIt->typeName()
+        const auto property = scope->property(access.m_name);
+        if (!property.propertyName().isEmpty()) {
+            const QString typeName = access.m_parentType.isEmpty() ? property.typeName()
                                                                    : access.m_parentType;
-            if (scopeIt->isList()) {
+            if (property.isList()) {
                 detectedRestrictiveKind = QLatin1String("list");
                 detectedRestrictiveName = access.m_name;
                 expectedNext.append(QLatin1String("length"));
@@ -169,7 +168,7 @@ bool CheckIdentifiers::checkMemberAccess(const QVector<FieldMember> &members,
             }
 
             if (access.m_parentType.isEmpty()) {
-                scope = scopeIt->type();
+                scope = property.type();
                 if (scope.isNull()) {
                     // Properties should always have a type. Otherwise something
                     // was missing from the import already.
@@ -194,9 +193,7 @@ bool CheckIdentifiers::checkMemberAccess(const QVector<FieldMember> &members,
             continue;
         }
 
-        const auto methods = scope->methods();
-        const auto scopeMethodIt = methods.find(access.m_name);
-        if (scopeMethodIt != methods.end())
+        if (scope->hasMethod(access.m_name))
             return true; // Access to property of JS function
 
         auto checkEnums = [&](const QQmlJSScope::ConstPtr &scope) {
@@ -232,14 +229,14 @@ bool CheckIdentifiers::checkMemberAccess(const QVector<FieldMember> &members,
 
         bool typeFound =
                 walkViaParentAndAttachedScopes(rootType, [&](QQmlJSScope::ConstPtr type) {
-                    const auto typeProperties = type->properties();
+                    const auto typeProperties = type->ownProperties();
                     const auto typeIt = typeProperties.find(access.m_name);
                     if (typeIt != typeProperties.end()) {
                         scope = typeIt->type();
                         return true;
                     }
 
-                    const auto typeMethods = type->methods();
+                    const auto typeMethods = type->ownMethods();
                     const auto typeMethodIt = typeMethods.find(access.m_name);
                     if (typeMethodIt != typeMethods.end()) {
                         detectedRestrictiveName = access.m_name;
@@ -326,18 +323,17 @@ bool CheckIdentifiers::operator()(
             }
 
             auto qmlScope = QQmlJSScope::findCurrentQMLScope(currentScope);
-            if (qmlScope->methods().contains(memberAccessBase.m_name)) {
-                // a property of a JavaScript function
+            if (qmlScope->hasMethod(memberAccessBase.m_name)) {
+                // a property of a JavaScript function, or a method
                 continue;
             }
 
-            const auto properties = qmlScope->properties();
-            const auto qmlIt = properties.find(memberAccessBase.m_name);
-            if (qmlIt != properties.end()) {
-                if (memberAccessChain.isEmpty() || unknownBuiltins.contains(qmlIt->typeName()))
+            const auto property = qmlScope->property(memberAccessBase.m_name);
+            if (!property.propertyName().isEmpty()) {
+                if (memberAccessChain.isEmpty() || unknownBuiltins.contains(property.typeName()))
                     continue;
 
-                if (!qmlIt->type()) {
+                if (!property.type()) {
                     m_colorOut->writePrefixedMessage(QString::fromLatin1(
                                            "Type of property \"%2\" not found at %3:%4:%5\n")
                                            .arg(memberAccessBase.m_name)
@@ -346,7 +342,7 @@ bool CheckIdentifiers::operator()(
                                            .arg(memberAccessBase.m_location.startColumn), Warning);
                     printContext(m_code, m_colorOut, memberAccessBase.m_location);
                     noUnqualifiedIdentifier = false;
-                } else if (!checkMemberAccess(memberAccessChain, qmlIt->type(), &*qmlIt)) {
+                } else if (!checkMemberAccess(memberAccessChain, property.type(), &property)) {
                     noUnqualifiedIdentifier = false;
                 }
 
@@ -375,8 +371,8 @@ bool CheckIdentifiers::operator()(
 
             // root(JS) --> (first element)
             const auto firstElement = root->childScopes()[0];
-            if (firstElement->properties().contains(memberAccessBase.m_name)
-                    || firstElement->methods().contains(memberAccessBase.m_name)
+            if (firstElement->hasProperty(memberAccessBase.m_name)
+                    || firstElement->hasMethod(memberAccessBase.m_name)
                     || firstElement->enums().contains(memberAccessBase.m_name)) {
                 m_colorOut->writePrefixedMessage(
                             memberAccessBase.m_name
diff --git a/tools/qmllint/findwarnings.cpp b/tools/qmllint/findwarnings.cpp
index 9e8fbb853b..06e647dcd5 100644
--- a/tools/qmllint/findwarnings.cpp
+++ b/tools/qmllint/findwarnings.cpp
@@ -192,7 +192,7 @@ bool FindWarningVisitor::visit(QQmlJS::AST::UiScriptBinding *uisb)
     if (signal.isEmpty())
         return true;
 
-    if (!m_currentScope->methods().contains(signal) && m_warnUnqualified) {
+    if (!m_currentScope->hasMethod(signal) && m_warnUnqualified) {
         m_errors.append({
                             QStringLiteral("no matching signal found for handler \"%1\"\n")
                                .arg(name.toString()),
@@ -212,18 +212,20 @@ bool FindWarningVisitor::visit(QQmlJS::AST::UiScriptBinding *uisb)
         }
     }
 
-    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.
+    for (QQmlJSScope::ConstPtr scope = m_currentScope; scope; scope = scope->baseType()) {
+        const auto methods = scope->ownMethods();
+        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});
+            return true; // If there are multiple candidates for the signal, it's a mess anyway.
+        }
     }
 
     return true;
@@ -319,8 +321,14 @@ 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 && !targetScope.isNull())
-            outstandingConnection.scope->addMethods(targetScope->methods());
+        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);
     }
@@ -392,8 +400,11 @@ bool FindWarningVisitor::visit(QQmlJS::AST::UiObjectDefinition *uiod)
                 return false; // visit children later once target is known
             }
         }
-        if (targetScope)
-            m_currentScope->addMethods(targetScope->methods());
+        for (const auto scope = targetScope; targetScope; targetScope = targetScope->baseType()) {
+            const auto connectionMethods = targetScope->ownMethods();
+            for (const auto &method : connectionMethods)
+                m_currentScope->addOwnMethod(method);
+        }
     }
     return true;
 }
@@ -427,12 +438,10 @@ void FindWarningVisitor::endVisit(QQmlJS::AST::UiObjectDefinition *uiod)
         return;
     }
 
-    const auto properties = childScope->properties();
-    const auto it = properties.find(QStringLiteral("parent"));
-    if (it != properties.end()) {
-        auto property = *it;
+    auto property = childScope->property(QStringLiteral("parent"));
+    if (!property.propertyName().isEmpty()) {
         property.setType(QQmlJSScope::ConstPtr(m_currentScope));
-        childScope->addProperty(property);
+        childScope->addOwnProperty(property);
     }
 }