From f0031c949ea6e5e6bc5dfb645e125e12192d425d Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Fri, 25 Sep 2020 12:39:17 +0200 Subject: 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 --- tools/qmllint/checkidentifiers.cpp | 72 ++++++++++++++++++++++------------ tools/qmllint/checkidentifiers.h | 7 ++-- tools/qmllint/findwarnings.cpp | 7 ++-- tools/qmllint/findwarnings.h | 14 +++---- tools/shared/scopetree.cpp | 2 +- tools/shared/typedescriptionreader.cpp | 2 - 6 files changed, 62 insertions(+), 42 deletions(-) (limited to 'tools') 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 &allTypes, + const FindWarningVisitor::ImportedTypes &allTypes, std::function 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 & 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 & 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 &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 & 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 & 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 & 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 &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 &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 &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 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 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: diff --git a/tools/shared/scopetree.cpp b/tools/shared/scopetree.cpp index 6cbabf4b51..4d02c67491 100644 --- a/tools/shared/scopetree.cpp +++ b/tools/shared/scopetree.cpp @@ -167,7 +167,7 @@ void ScopeTree::updateParentProperty(const ScopeTree::ConstPtr &scope) { auto it = m_properties.find(QLatin1String("parent")); if (it != m_properties.end() - && scope->internalName() != QLatin1String("Component") + && scope->baseTypeName() != QLatin1String("Component") && scope->internalName() != QLatin1String("program")) it->setType(scope); } diff --git a/tools/shared/typedescriptionreader.cpp b/tools/shared/typedescriptionreader.cpp index 717dbdf843..3691fa080c 100644 --- a/tools/shared/typedescriptionreader.cpp +++ b/tools/shared/typedescriptionreader.cpp @@ -257,8 +257,6 @@ void TypeDescriptionReader::readComponent(UiObjectDefinition *ast) return; } - // ### add implicit export into the package of c++ types - scope->addExport(scope->internalName(), QStringLiteral(""), ComponentVersion()); m_objects->insert(scope->internalName(), scope); } -- cgit v1.2.3