diff options
author | Fabian Kosmale <fabian.kosmale@qt.io> | 2019-07-23 16:10:24 +0200 |
---|---|---|
committer | Fabian Kosmale <fabian.kosmale@qt.io> | 2019-07-26 09:00:07 +0200 |
commit | 66368ffdd93e63a182aa6961f821ca14290ecf80 (patch) | |
tree | 55be6dd8472d9b3afa3301a52e1e4c6fd0b1198f /tools/qmllint | |
parent | 53e7927fdf56617cfcd3b0e6c9f6db6e0b3d6483 (diff) |
qmllint: Improve parent handling
- Do not warn about parent access in unknown components
This avoids false positive warnings when an imported component could not
be found (or when it actually was not imported).
We still warn about the component which could not be found, so the user
is still informed that something is not right.
We also still emit a warning when we know the properties of a component,
and parent is not one of them.
- Do not recommend the use of parent to address the root components
properties. For this to work, we would need to know whether the root
component reparents its children or not. Moreover, id lookups are
actually faster than parent lookups.
Change-Id: I83d0e71e4bf20d34a3e6d836c2b123b2bf0d416e
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>
Diffstat (limited to 'tools/qmllint')
-rw-r--r-- | tools/qmllint/findunqualified.cpp | 6 | ||||
-rw-r--r-- | tools/qmllint/findunqualified.h | 1 | ||||
-rw-r--r-- | tools/qmllint/scopetree.cpp | 25 | ||||
-rw-r--r-- | tools/qmllint/scopetree.h | 2 |
4 files changed, 19 insertions, 15 deletions
diff --git a/tools/qmllint/findunqualified.cpp b/tools/qmllint/findunqualified.cpp index e6a72df44c..69e1473975 100644 --- a/tools/qmllint/findunqualified.cpp +++ b/tools/qmllint/findunqualified.cpp @@ -325,7 +325,9 @@ void FindUnqualifiedIDVisitor::importExportedNames(QStringRef prefix, QString na break; } } else { - qDebug() << name << "not found"; + m_colorOut.write(QLatin1String("warning: "), Warning); + m_colorOut.write(name + QLatin1String(" was not found. Did you add all import paths?\n")); + m_unknownImports.insert(name); break; } } @@ -526,7 +528,7 @@ bool FindUnqualifiedIDVisitor::visit(QQmlJS::AST::IdentifierExpression *idexp) auto name = idexp->name; if (!m_exportedName2MetaObject.contains(name.toString())) { m_currentScope->addIdToAccssedIfNotInParentScopes( - { name.toString(), idexp->firstSourceLocation() }); + { name.toString(), idexp->firstSourceLocation() }, m_unknownImports); } return true; } diff --git a/tools/qmllint/findunqualified.h b/tools/qmllint/findunqualified.h index 937194d142..8fc8257bef 100644 --- a/tools/qmllint/findunqualified.h +++ b/tools/qmllint/findunqualified.h @@ -57,6 +57,7 @@ private: QString m_rootId; QString m_filePath; QSet<QPair<QString, QString>> m_alreadySeenImports; + QSet<QString> m_unknownImports; ColorOutput m_colorOut; struct OutstandingConnection {QString targetName; ScopeTree *scope; QQmlJS::AST::UiObjectDefinition *uiod;}; diff --git a/tools/qmllint/scopetree.cpp b/tools/qmllint/scopetree.cpp index f58fde061e..2eff3fa319 100644 --- a/tools/qmllint/scopetree.cpp +++ b/tools/qmllint/scopetree.cpp @@ -86,8 +86,14 @@ bool ScopeTree::isIdInCurrentScope(const QString &id) const return isIdInCurrentQMlScopes(id) || isIdInCurrentJSScopes(id); } -void ScopeTree::addIdToAccssedIfNotInParentScopes(const QPair<QString, QQmlJS::AST::SourceLocation> &id_loc_pair) { - if (!isIdInCurrentScope(id_loc_pair.first)) { +void ScopeTree::addIdToAccssedIfNotInParentScopes(const QPair<QString, QQmlJS::AST::SourceLocation> &id_loc_pair, const QSet<QString>& unknownImports) { + // also do not add id if it is parent + // parent is almost always defined valid in QML, and if we could not find a definition for the current QML component + // not skipping "parent" will lead to many false positives + // Moreover, if the top level item is Item or inherits from it, it will have a parent property to which we would point the user + // which makes for a very nonsensical warning + auto qmlScope = getCurrentQMLScope(); + if (!isIdInCurrentScope(id_loc_pair.first) && !(id_loc_pair.first == QLatin1String("parent") && qmlScope && unknownImports.contains(qmlScope->name()))) { m_accessedIdentifiers.push_back(id_loc_pair); } } @@ -148,20 +154,15 @@ bool ScopeTree::recheckIdentifiers(const QString& code, const QHash<QString, Lan while (parentScope && parentScope->scopeType() != ScopeType::QMLScope) { parentScope = parentScope->m_parentScope; } - bool directChild = parentScope && parentScope->isVisualRootScope(); colorOut.write("Note: ", Info); colorOut.write( idLocationPair.first + QLatin1String(" is a meber of the root element\n"), Normal ); - if (directChild) { - colorOut.write(QLatin1String(" As the current element is a direct child of the root element,\n you can qualify the access with parent to avoid this warning:\n"), Normal); - } else { - colorOut.write(QLatin1String(" You can qualify the access with its id to avoid this warning:\n"), Normal); - if (rootId == QLatin1String("<id>")) { - colorOut.write("Note: ", Warning); - colorOut.write(("You first have to give the root element an id\n")); - } + colorOut.write(QLatin1String(" You can qualify the access with its id to avoid this warning:\n"), Normal); + if (rootId == QLatin1String("<id>")) { + colorOut.write("Note: ", Warning); + colorOut.write(("You first have to give the root element an id\n")); } colorOut.write(issueLocationWithContext.beforeText.toString(), Normal); - colorOut.write( (directChild ? QLatin1String("parent") : rootId) + QLatin1Char('.'), Hint); + colorOut.write(rootId + QLatin1Char('.'), Hint); colorOut.write(issueLocationWithContext.issueText.toString(), Normal); colorOut.write(issueLocationWithContext.afterText + QLatin1Char('\n'), Normal); } else if (currentScope->isIdInjectedFromSignal(idLocationPair.first)) { diff --git a/tools/qmllint/scopetree.h b/tools/qmllint/scopetree.h index a65ed30d61..872a509123 100644 --- a/tools/qmllint/scopetree.h +++ b/tools/qmllint/scopetree.h @@ -75,7 +75,7 @@ public: void insertPropertyIdentifier(QString id); // inserts property as qml identifier as well as the corresponding bool isIdInCurrentScope(QString const &id) const; - void addIdToAccssedIfNotInParentScopes(QPair<QString, QQmlJS::AST::SourceLocation> const& id_loc_pair); + void addIdToAccssedIfNotInParentScopes(QPair<QString, QQmlJS::AST::SourceLocation> const& id_loc_pair, const QSet<QString>& unknownImports); bool isVisualRootScope() const; QString name() const; |