TypeDescriptionReader: Don't export C++ names as QML names

This is just wrong. The types are not visible in QML under their C++
names. Indeed, this way we reveal a number of places where we confuse
the names. Fix those in turn.

Furthermore, one of the tests was incorrect. The qmltypes files did not
contain an export entry for one of the types, and therefore the type was
formally anonymous in QML. However, we did access it via its C++ name.
Fix that by exporting the C++ name.

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

4 files changed, 61 insertions, 39 deletions

diff --git a/tools/qmllint/checkidentifiers.cpp b/tools/qmllint/checkidentifiers.cpp
index fdffb556b6..a75c9c9b83 100644
--- a/tools/qmllint/checkidentifiers.cpp
+++ b/tools/qmllint/checkidentifiers.cpp
@@ -84,7 +84,7 @@ void CheckIdentifiers::printContext(const QQmlJS::SourceLocation &location) cons
 }
 
 static bool walkViaParentAndAttachedScopes(ScopeTree::ConstPtr rootType,
-                                           const QHash<QString, ScopeTree::Ptr> &allTypes,
+                                           const FindWarningVisitor::ImportedTypes &allTypes,
                                            std::function<bool(ScopeTree::ConstPtr)> visit)
 {
     if (rootType == nullptr)
@@ -98,10 +98,15 @@ static bool walkViaParentAndAttachedScopes(ScopeTree::ConstPtr rootType,
         if (visit(type))
             return true;
 
-        if (auto superType = allTypes.value(type->baseTypeName()))
-            stack.push(superType);
+        if (type->isComposite()) {
+            if (auto superType = allTypes.importedQmlNames.value(type->baseTypeName()))
+                stack.push(superType);
+        } else {
+            if (auto superType = allTypes.cppNames.value(type->baseTypeName()))
+                stack.push(superType);
+        }
 
-        if (auto attachedType = allTypes.value(type->attachedTypeName()))
+        if (auto attachedType = allTypes.cppNames.value(type->attachedTypeName()))
             stack.push(attachedType);
     }
     return false;
@@ -137,8 +142,6 @@ bool CheckIdentifiers::checkMemberAccess(const QVector<ScopeTree::FieldMember> &
             return false;
         }
 
-        const QString scopeName = scope->internalName();
-
         if (!detectedRestrictiveKind.isEmpty()) {
             if (expectedNext.contains(access.m_name)) {
                 expectedNext.clear();
@@ -187,14 +190,21 @@ bool CheckIdentifiers::checkMemberAccess(const QVector<ScopeTree::FieldMember> &
             if (unknownBuiltins.contains(typeName))
                 return true;
 
-            const auto it = m_types.find(typeName);
-            if (it == m_types.end()) {
-                detectedRestrictiveKind = typeName;
-                detectedRestrictiveName = access.m_name;
-                scope = nullptr;
-            } else {
-                scope = *it;
-            }
+            auto findNextScope = [&](const QHash<QString, ScopeTree::Ptr> &types) {
+                const auto it = types.find(typeName);
+                if (it == types.end()) {
+                    detectedRestrictiveKind = typeName;
+                    detectedRestrictiveName = access.m_name;
+                    scope = nullptr;
+                } else {
+                    scope = *it;
+                }
+            };
+
+            if (access.m_parentType.isEmpty() && scope->isComposite())
+                findNextScope(m_types.cppNames);
+            else
+                findNextScope(m_types.importedQmlNames);
             continue;
         }
 
@@ -228,15 +238,24 @@ bool CheckIdentifiers::checkMemberAccess(const QVector<ScopeTree::FieldMember> &
         if (!detectedRestrictiveName.isEmpty())
             continue;
 
-        auto rootType =
-                m_types.value(access.m_parentType.isEmpty() ? scopeName : access.m_parentType);
+        ScopeTree::ConstPtr rootType;
+        if (!access.m_parentType.isEmpty())
+            rootType = m_types.importedQmlNames.value(access.m_parentType);
+        else
+            rootType = scope;
+
         bool typeFound =
                 walkViaParentAndAttachedScopes(rootType, m_types, [&](ScopeTree::ConstPtr type) {
                     const auto typeProperties = type->properties();
                     const auto typeIt = typeProperties.find(access.m_name);
                     if (typeIt != typeProperties.end()) {
                         const ScopeTree::ConstPtr propType = typeIt->type();
-                        scope = propType ? propType : m_types.value(typeIt->typeName());
+                        if (propType)
+                            scope = propType;
+                        else if (scope->isComposite())
+                            scope = m_types.importedQmlNames.value(typeIt->typeName());
+                        else
+                            scope = m_types.cppNames.value(typeIt->typeName());
                         return true;
                     }
 
@@ -255,10 +274,10 @@ bool CheckIdentifiers::checkMemberAccess(const QVector<ScopeTree::FieldMember> &
 
         if (access.m_name.front().isUpper() && scope->scopeType() == ScopeType::QMLScope) {
             // may be an attached type
-            const auto it = m_types.find(access.m_name);
-            if (it != m_types.end() && !(*it)->attachedTypeName().isEmpty()) {
-                const auto attached = m_types.find((*it)->attachedTypeName());
-                if (attached != m_types.end()) {
+            const auto it = m_types.importedQmlNames.find(access.m_name);
+            if (it != m_types.importedQmlNames.end() && !(*it)->attachedTypeName().isEmpty()) {
+                const auto attached = m_types.cppNames.find((*it)->attachedTypeName());
+                if (attached != m_types.cppNames.end()) {
                     scope = *attached;
                     continue;
                 }
@@ -269,7 +288,8 @@ bool CheckIdentifiers::checkMemberAccess(const QVector<ScopeTree::FieldMember> &
         m_colorOut->write(QString::fromLatin1(
                                "Property \"%1\" not found on type \"%2\" at %3:%4:%5\n")
                                .arg(access.m_name)
-                               .arg(scopeName)
+                               .arg(scope->internalName().isEmpty()
+                                    ? scope->baseTypeName() : scope->internalName())
                                .arg(m_fileName)
                                .arg(access.m_location.startLine)
                                .arg(access.m_location.startColumn), Normal);
@@ -321,8 +341,8 @@ bool CheckIdentifiers::operator()(const QHash<QString, ScopeTree::ConstPtr> &qml
                     if (scopedName.front().isUpper()) {
                         const QString qualified = memberAccessBase.m_name + QLatin1Char('.')
                                 + scopedName;
-                        const auto typeIt = m_types.find(qualified);
-                        if (typeIt != m_types.end()) {
+                        const auto typeIt = m_types.importedQmlNames.find(qualified);
+                        if (typeIt != m_types.importedQmlNames.end()) {
                             memberAccessChain.takeFirst();
                             if (!checkMemberAccess(memberAccessChain, *typeIt))
                                 noUnqualifiedIdentifier = false;
@@ -365,8 +385,8 @@ bool CheckIdentifiers::operator()(const QHash<QString, ScopeTree::ConstPtr> &qml
             if (memberAccessBase.m_name == QLatin1String("Qt"))
                 continue;
 
-            const auto typeIt = m_types.find(memberAccessBase.m_name);
-            if (typeIt != m_types.end()) {
+            const auto typeIt = m_types.importedQmlNames.find(memberAccessBase.m_name);
+            if (typeIt != m_types.importedQmlNames.end()) {
                 if (!checkMemberAccess(memberAccessChain, *typeIt))
                     noUnqualifiedIdentifier = false;
                 continue;
diff --git a/tools/qmllint/checkidentifiers.h b/tools/qmllint/checkidentifiers.h
index 8fd605f454..653b398301 100644
--- a/tools/qmllint/checkidentifiers.h
+++ b/tools/qmllint/checkidentifiers.h
@@ -30,14 +30,15 @@
 #define CHECKIDENTIFIERS_H
 
 #include "scopetree.h"
+#include "findwarnings.h"
 
 class ColorOutput;
 
 class CheckIdentifiers
 {
 public:
-    CheckIdentifiers(ColorOutput *colorOut, const QString &code, const QHash<QString,
-                     ScopeTree::Ptr> &types, const QString &fileName) :
+    CheckIdentifiers(ColorOutput *colorOut, const QString &code,
+                     const FindWarningVisitor::ImportedTypes &types, const QString &fileName) :
         m_colorOut(colorOut), m_code(code), m_types(types), m_fileName(fileName)
     {}
 
@@ -52,7 +53,7 @@ private:
 
     ColorOutput *m_colorOut = nullptr;
     QString m_code;
-    QHash<QString, ScopeTree::Ptr> m_types;
+    FindWarningVisitor::ImportedTypes m_types;
     QString m_fileName;
 };
 
diff --git a/tools/qmllint/findwarnings.cpp b/tools/qmllint/findwarnings.cpp
index 3e3d63f8ad..7e1dfc743e 100644
--- a/tools/qmllint/findwarnings.cpp
+++ b/tools/qmllint/findwarnings.cpp
@@ -61,7 +61,8 @@ static QQmlDirParser createQmldirParserForFile(const QString &filename)
 void FindWarningVisitor::enterEnvironment(ScopeType type, const QString &name)
 {
     m_currentScope = ScopeTree::create(type, m_currentScope);
-    m_currentScope->setInternalName(name);
+    m_currentScope->setBaseTypeName(name);
+    m_currentScope->setIsComposite(true);
 }
 
 void FindWarningVisitor::leaveEnvironment()
@@ -641,7 +642,7 @@ bool FindWarningVisitor::check()
     if (!m_warnUnqualified)
         return true;
 
-    CheckIdentifiers check(&m_colorOut, m_code, m_rootScopeImports.importedQmlNames, m_filePath);
+    CheckIdentifiers check(&m_colorOut, m_code, m_rootScopeImports, m_filePath);
     return check(m_qmlid2scope, m_rootScope, m_rootId);
 }
 
@@ -834,7 +835,7 @@ bool FindWarningVisitor::visit(QQmlJS::AST::UiObjectDefinition *uiod)
             do {
                 scope = scope->parentScope(); // TODO: rename method
             } while (scope->scopeType() != ScopeType::QMLScope);
-            targetScope = m_rootScopeImports.importedQmlNames.value(scope->internalName());
+            targetScope = m_rootScopeImports.importedQmlNames.value(scope->baseTypeName());
         } else {
             // there was a target, check if we already can find it
             auto scopeIt =  m_qmlid2scope.find(target);
diff --git a/tools/qmllint/findwarnings.h b/tools/qmllint/findwarnings.h
index 7e3ea14128..2c7fe5ce05 100644
--- a/tools/qmllint/findwarnings.h
+++ b/tools/qmllint/findwarnings.h
@@ -53,13 +53,6 @@ class FindWarningVisitor : public QQmlJS::AST::Visitor
 {
     Q_DISABLE_COPY_MOVE(FindWarningVisitor)
 public:
-    explicit FindWarningVisitor(
-            QStringList qmltypeDirs, QStringList qmltypesFiles, QString code, QString fileName,
-            bool silent, bool warnUnqualified, bool warnWithStatement, bool warnInheritanceCycle);
-    ~FindWarningVisitor() override = default;
-    bool check();
-
-private:
     struct ImportedTypes
     {
         // C++ names used in qmltypes files for non-composite types
@@ -72,6 +65,13 @@ private:
         QHash<QString, ScopeTree::Ptr> importedQmlNames;
     };
 
+    explicit FindWarningVisitor(
+            QStringList qmltypeDirs, QStringList qmltypesFiles, QString code, QString fileName,
+            bool silent, bool warnUnqualified, bool warnWithStatement, bool warnInheritanceCycle);
+    ~FindWarningVisitor() override = default;
+    bool check();
+
+private:
     class Importer
     {
     public: