diff options
author | Maximilian Goldstein <max.goldstein@qt.io> | 2021-03-24 20:13:14 +0100 |
---|---|---|
committer | Maximilian Goldstein <max.goldstein@qt.io> | 2021-03-25 11:34:41 +0100 |
commit | eb74d99d02f61007443bcdf83033b05d525326b8 (patch) | |
tree | e7d768e70b154daa9418aad3d3d3df138908c201 | |
parent | 8fe127129b4f31e7e13dbf727687d98b95984763 (diff) |
qmllint: Add support for warning about unused imports
qmllint will now show an information line pointing out import statements that are not needed.
It will not warn about imports that are redundant however, since there are legitimate reasons for having these.
Fixes: QTBUG-83237
Change-Id: I9588b5fa8a8efd37b48c9b349a448e580efb1452
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
-rw-r--r-- | src/qmlcompiler/qqmljsimportvisitor.cpp | 47 | ||||
-rw-r--r-- | src/qmlcompiler/qqmljsimportvisitor_p.h | 8 | ||||
-rw-r--r-- | src/qmlcompiler/qqmljsscope.cpp | 19 | ||||
-rw-r--r-- | src/qmlcompiler/qqmljsscope_p.h | 3 | ||||
-rw-r--r-- | tests/auto/qml/qmllint/data/FormUser.qml | 2 | ||||
-rw-r--r-- | tests/auto/qml/qmllint/data/ListProperty.qml | 1 | ||||
-rw-r--r-- | tests/auto/qml/qmllint/data/importing_js.qml | 2 | ||||
-rw-r--r-- | tests/auto/qml/qmllint/data/overridescript.qml | 1 | ||||
-rw-r--r-- | tests/auto/qml/qmllint/data/unused_multi.qml | 7 | ||||
-rw-r--r-- | tests/auto/qml/qmllint/data/unused_prefix.qml | 5 | ||||
-rw-r--r-- | tests/auto/qml/qmllint/data/unused_simple.qml | 5 | ||||
-rw-r--r-- | tests/auto/qml/qmllint/data/used.qml | 12 | ||||
-rw-r--r-- | tests/auto/qml/qmllint/tst_qmllint.cpp | 153 | ||||
-rw-r--r-- | tools/qmllint/checkidentifiers.cpp | 11 | ||||
-rw-r--r-- | tools/qmllint/findwarnings.cpp | 37 |
15 files changed, 239 insertions, 74 deletions
diff --git a/src/qmlcompiler/qqmljsimportvisitor.cpp b/src/qmlcompiler/qqmljsimportvisitor.cpp index 2483c1c95d..a452327377 100644 --- a/src/qmlcompiler/qqmljsimportvisitor.cpp +++ b/src/qmlcompiler/qqmljsimportvisitor.cpp @@ -270,13 +270,13 @@ bool QQmlJSImportVisitor::visit(UiObjectDefinition *definition) m_currentScope->setAnnotations(parseAnnotations(definition->annotations)); - QQmlJSScope::resolveTypes(m_currentScope, m_rootScopeImports); + QQmlJSScope::resolveTypes(m_currentScope, m_rootScopeImports, &m_usedTypes); return true; } void QQmlJSImportVisitor::endVisit(UiObjectDefinition *) { - QQmlJSScope::resolveTypes(m_currentScope, m_rootScopeImports); + QQmlJSScope::resolveTypes(m_currentScope, m_rootScopeImports, &m_usedTypes); leaveEnvironment(); } @@ -488,6 +488,14 @@ bool QQmlJSImportVisitor::visit(QQmlJS::AST::UiEnumDeclaration *uied) bool QQmlJSImportVisitor::visit(QQmlJS::AST::UiImport *import) { + auto addImportLocation = [this, import](const QString &name) { + if (m_importTypeLocationMap.contains(name) + && m_importTypeLocationMap.values(name).contains(import->firstSourceLocation())) + return; + + m_importTypeLocationMap.insert(name, import->firstSourceLocation()); + m_importLocations.insert(import->firstSourceLocation()); + }; // construct path QString prefix = QLatin1String(""); if (import->asToken.isValid()) { @@ -506,13 +514,17 @@ bool QQmlJSImportVisitor::visit(QQmlJS::AST::UiImport *import) const auto entry = m_importer->resourceFileMapper()->entry( QQmlJSResourceFileMapper::resourceFileFilter(absolute.mid(1))); const auto scope = m_importer->importFile(entry.filePath); - m_rootScopeImports.insert( - prefix.isEmpty() - ? QFileInfo(entry.resourcePath).baseName() - : prefix, - scope); + const QString actualPrefix = prefix.isEmpty() + ? QFileInfo(entry.resourcePath).baseName() + : prefix; + m_rootScopeImports.insert(actualPrefix, scope); + + addImportLocation(actualPrefix); } else { - m_rootScopeImports.insert(m_importer->importDirectory(absolute, prefix)); + const auto scopes = m_importer->importDirectory(absolute, prefix); + m_rootScopeImports.insert(scopes); + for (const QString &key : scopes.keys()) + addImportLocation(key); } } return true; @@ -520,10 +532,15 @@ bool QQmlJSImportVisitor::visit(QQmlJS::AST::UiImport *import) QFileInfo path(absolute); if (path.isDir()) { - m_rootScopeImports.insert(m_importer->importDirectory(path.canonicalFilePath(), prefix)); + const auto scopes = m_importer->importDirectory(path.canonicalFilePath(), prefix); + m_rootScopeImports.insert(scopes); + for (const QString &key : scopes.keys()) + addImportLocation(key); } else if (path.isFile()) { const auto scope = m_importer->importFile(path.canonicalFilePath()); - m_rootScopeImports.insert(prefix.isEmpty() ? scope->internalName() : prefix, scope); + const QString actualPrefix = prefix.isEmpty() ? scope->internalName() : prefix; + m_rootScopeImports.insert(actualPrefix, scope); + addImportLocation(actualPrefix); } return true; } @@ -541,6 +558,8 @@ bool QQmlJSImportVisitor::visit(QQmlJS::AST::UiImport *import) path, prefix, import->version ? import->version->version : QTypeRevision(), import->firstSourceLocation()); m_rootScopeImports.insert(imported); + for (const QString &key : imported.keys()) + addImportLocation(key); m_errors.append(m_importer->takeWarnings()); return true; @@ -696,13 +715,13 @@ bool QQmlJSImportVisitor::visit(QQmlJS::AST::UiObjectBinding *uiob) enterEnvironment(QQmlJSScope::QMLScope, name, uiob->qualifiedTypeNameId->identifierToken); - QQmlJSScope::resolveTypes(m_currentScope, m_rootScopeImports); + QQmlJSScope::resolveTypes(m_currentScope, m_rootScopeImports, &m_usedTypes); return true; } void QQmlJSImportVisitor::endVisit(QQmlJS::AST::UiObjectBinding *uiob) { - QQmlJSScope::resolveTypes(m_currentScope, m_rootScopeImports); + QQmlJSScope::resolveTypes(m_currentScope, m_rootScopeImports, &m_usedTypes); const QQmlJSScope::ConstPtr childScope = m_currentScope; leaveEnvironment(); @@ -743,7 +762,7 @@ bool QQmlJSImportVisitor::visit(ESModule *module) void QQmlJSImportVisitor::endVisit(ESModule *) { - QQmlJSScope::resolveTypes(m_exportedRootScope, m_rootScopeImports); + QQmlJSScope::resolveTypes(m_exportedRootScope, m_rootScopeImports, &m_usedTypes); } bool QQmlJSImportVisitor::visit(Program *) @@ -758,7 +777,7 @@ bool QQmlJSImportVisitor::visit(Program *) void QQmlJSImportVisitor::endVisit(Program *) { - QQmlJSScope::resolveTypes(m_exportedRootScope, m_rootScopeImports); + QQmlJSScope::resolveTypes(m_exportedRootScope, m_rootScopeImports, &m_usedTypes); } QT_END_NAMESPACE diff --git a/src/qmlcompiler/qqmljsimportvisitor_p.h b/src/qmlcompiler/qqmljsimportvisitor_p.h index e93080c28d..cecf4993b2 100644 --- a/src/qmlcompiler/qqmljsimportvisitor_p.h +++ b/src/qmlcompiler/qqmljsimportvisitor_p.h @@ -119,6 +119,14 @@ protected: QQmlJSScope::ConstPtr m_globalScope; QHash<QString, QQmlJSScope::ConstPtr> m_scopesById; QHash<QString, QQmlJSScope::ConstPtr> m_rootScopeImports; + + // Maps all qmlNames to the source location of their import + QMultiHash<QString, QQmlJS::SourceLocation> m_importTypeLocationMap; + // Contains all import source locations (could be extracted from above but that is expensive) + QSet<QQmlJS::SourceLocation> m_importLocations; + // A set of all types that have been used during type resolution + QSet<QString> m_usedTypes; + QQmlJSImporter *m_importer; QList<QQmlJS::DiagnosticMessage> m_errors; diff --git a/src/qmlcompiler/qqmljsscope.cpp b/src/qmlcompiler/qqmljsscope.cpp index aaeda2431f..15c3109b61 100644 --- a/src/qmlcompiler/qqmljsscope.cpp +++ b/src/qmlcompiler/qqmljsscope.cpp @@ -191,20 +191,29 @@ QQmlJSScope::findJSIdentifier(const QString &id) const } void QQmlJSScope::resolveTypes(const QQmlJSScope::Ptr &self, - const QHash<QString, QQmlJSScope::ConstPtr> &contextualTypes) + const QHash<QString, QQmlJSScope::ConstPtr> &contextualTypes, + QSet<QString> *usedTypes) { auto findType = [&](const QString &name) -> QQmlJSScope::ConstPtr { auto type = contextualTypes.constFind(name); - if (type != contextualTypes.constEnd()) + + if (type != contextualTypes.constEnd()) { + if (usedTypes != nullptr) + usedTypes->insert(name); return *type; + } const auto colonColon = name.indexOf(QStringLiteral("::")); if (colonColon > 0) { - const auto outerType = contextualTypes.constFind(name.left(colonColon)); + const QString outerTypeName = name.left(colonColon); + const auto outerType = contextualTypes.constFind(outerTypeName); if (outerType != contextualTypes.constEnd()) { for (const auto &innerType : qAsConst((*outerType)->m_childScopes)) { - if (innerType->m_internalName == name) + if (innerType->m_internalName == name) { + if (usedTypes != nullptr) + usedTypes->insert(name); return innerType; + } } } } @@ -296,7 +305,7 @@ void QQmlJSScope::resolveTypes(const QQmlJSScope::Ptr &self, default: break; } - resolveTypes(childScope, contextualTypes); + resolveTypes(childScope, contextualTypes, usedTypes); } } diff --git a/src/qmlcompiler/qqmljsscope_p.h b/src/qmlcompiler/qqmljsscope_p.h index 484f8ce1d3..80129296fe 100644 --- a/src/qmlcompiler/qqmljsscope_p.h +++ b/src/qmlcompiler/qqmljsscope_p.h @@ -270,7 +270,8 @@ public: } static void resolveTypes(const QQmlJSScope::Ptr &self, - const QHash<QString, ConstPtr> &contextualTypes); + const QHash<QString, ConstPtr> &contextualTypes, + QSet<QString> *usedTypes = nullptr); void setSourceLocation(const QQmlJS::SourceLocation &sourceLocation) { diff --git a/tests/auto/qml/qmllint/data/FormUser.qml b/tests/auto/qml/qmllint/data/FormUser.qml index ea3621586f..46a5c4fd6d 100644 --- a/tests/auto/qml/qmllint/data/FormUser.qml +++ b/tests/auto/qml/qmllint/data/FormUser.qml @@ -1,5 +1,3 @@ -import QtQuick 2.0 - Form { x: 12 y: 13 diff --git a/tests/auto/qml/qmllint/data/ListProperty.qml b/tests/auto/qml/qmllint/data/ListProperty.qml index d2bbc58cc5..0290f51ce9 100644 --- a/tests/auto/qml/qmllint/data/ListProperty.qml +++ b/tests/auto/qml/qmllint/data/ListProperty.qml @@ -1,4 +1,3 @@ -import QtQuick 2.12 import Things 1.0 Frame { diff --git a/tests/auto/qml/qmllint/data/importing_js.qml b/tests/auto/qml/qmllint/data/importing_js.qml index b2d7a34fe6..3f226f1566 100644 --- a/tests/auto/qml/qmllint/data/importing_js.qml +++ b/tests/auto/qml/qmllint/data/importing_js.qml @@ -3,4 +3,6 @@ import QtQuick Item { id: root + + property var foo: JSTest.foo("Test") } diff --git a/tests/auto/qml/qmllint/data/overridescript.qml b/tests/auto/qml/qmllint/data/overridescript.qml index b85333b95f..d88f905aa7 100644 --- a/tests/auto/qml/qmllint/data/overridescript.qml +++ b/tests/auto/qml/qmllint/data/overridescript.qml @@ -1,5 +1,4 @@ import QtQml -import QtTest import "testlogger.js" as TestLogger QtObject { diff --git a/tests/auto/qml/qmllint/data/unused_multi.qml b/tests/auto/qml/qmllint/data/unused_multi.qml new file mode 100644 index 0000000000..f078eadf0c --- /dev/null +++ b/tests/auto/qml/qmllint/data/unused_multi.qml @@ -0,0 +1,7 @@ +// This is the one kind of unused import we currently do not warn about: +// If two imports provide the same type, one of them could be considered unused. +// We do not however warn here because there might be legitimate reasons why a user would want this. +import QtQuick +import QtQml + +QtObject {} diff --git a/tests/auto/qml/qmllint/data/unused_prefix.qml b/tests/auto/qml/qmllint/data/unused_prefix.qml new file mode 100644 index 0000000000..a54d74390a --- /dev/null +++ b/tests/auto/qml/qmllint/data/unused_prefix.qml @@ -0,0 +1,5 @@ +import QtQml as Qml // Unused due to being prefixed +import QtQuick + +QtObject { +} diff --git a/tests/auto/qml/qmllint/data/unused_simple.qml b/tests/auto/qml/qmllint/data/unused_simple.qml new file mode 100644 index 0000000000..c2a931f3ed --- /dev/null +++ b/tests/auto/qml/qmllint/data/unused_simple.qml @@ -0,0 +1,5 @@ +import QtTest // This import is not used. +import QtQuick + +QtObject { +} diff --git a/tests/auto/qml/qmllint/data/used.qml b/tests/auto/qml/qmllint/data/used.qml new file mode 100644 index 0000000000..ebb5a4884f --- /dev/null +++ b/tests/auto/qml/qmllint/data/used.qml @@ -0,0 +1,12 @@ +import QtQml as Qml +import QtQuick as Quick +import QtQuick 2.0 as QuickLegacy +import QtQuick + +Item { + property var bar: Qml.QtObject {} + Item { + property var foo: Quick.Screen.pixelDensity + property var foo2: parent.QuickLegacy.Screen.pixelDensity + } +} diff --git a/tests/auto/qml/qmllint/tst_qmllint.cpp b/tests/auto/qml/qmllint/tst_qmllint.cpp index b83fba5f7c..ccf8e4cfad 100644 --- a/tests/auto/qml/qmllint/tst_qmllint.cpp +++ b/tests/auto/qml/qmllint/tst_qmllint.cpp @@ -283,188 +283,243 @@ void TestQmllint::dirtyQmlCode_data() QTest::addColumn<QString>("filename"); QTest::addColumn<QString>("warningMessage"); QTest::addColumn<QString>("notContained"); + QTest::addColumn<bool>("exitsNormally"); QTest::newRow("Invalid_syntax_QML") << QStringLiteral("failure1.qml") << QStringLiteral("%1:4 : Expected token `:'") - << QString(); + << QString() + << false; QTest::newRow("Invalid_syntax_JS") << QStringLiteral("failure1.js") << QStringLiteral("%1:4 : Expected token `;'") - << QString(); + << QString() + << false; QTest::newRow("AutomatchedSignalHandler") << QStringLiteral("AutomatchedSignalHandler.qml") << QString("Warning: unqualified access at %1:12:36") - << QStringLiteral("no matching signal found"); + << QStringLiteral("no matching signal found") + << false; QTest::newRow("MemberNotFound") << QStringLiteral("memberNotFound.qml") << QString("Warning: Property \"foo\" not found on type \"QtObject\" at %1:6:31") - << QString(); + << QString() + << false; QTest::newRow("UnknownJavascriptMethd") << QStringLiteral("unknownJavascriptMethod.qml") << QString("Warning: Property \"foo2\" not found on type \"Methods\" at %1:5:25") - << QString(); + << QString() + << false; QTest::newRow("badAlias1") << QStringLiteral("badAlias.qml") << QString("Warning: 3:1: Cannot deduce type of alias \"wrong\"") - << QString(); + << QString() + << false; QTest::newRow("badAlias2") << QStringLiteral("badAlias.qml") << QString("Warning: unqualified access at %1:4:27") - << QString(); + << QString() + << false; QTest::newRow("badAliasProperty1") << QStringLiteral("badAliasProperty.qml") << QString("Warning: 3:1: Cannot deduce type of alias \"wrong\"") - << QString(); + << QString() + << false; QTest::newRow("badAliasProperty2") << QStringLiteral("badAliasProperty.qml") << QString("Warning: Property \"nowhere\" not found on type \"QtObject\" at %1:5:32") - << QString(); + << QString() + << false; QTest::newRow("badAliasExpression") << QStringLiteral("badAliasExpression.qml") << QString("Warning: 5:26: Invalid alias expression. Only IDs and field member " "expressions can be aliased") - << QString(); + << QString() + << false; QTest::newRow("aliasCycle1") << QStringLiteral("aliasCycle.qml") << QString("Warning: 3:1: Alias \"b\" is part of an alias cycle") - << QString(); + << QString() + << false; QTest::newRow("aliasCycle2") << QStringLiteral("aliasCycle.qml") << QString("Warning: 3:1: Alias \"a\" is part of an alias cycle") - << QString(); + << QString() + << false; QTest::newRow("badParent") << QStringLiteral("badParent.qml") << QString("Warning: Property \"rrr\" not found on type \"Item\" at %1:5:34") - << QString(); + << QString() + << false; QTest::newRow("parentIsComponent") << QStringLiteral("parentIsComponent.qml") << QString("Warning: Property \"progress\" not found on type \"QQuickItem\" at %1:7:39") - << QString(); + << QString() + << false; QTest::newRow("badTypeAssertion") << QStringLiteral("badTypeAssertion.qml") << QString("Warning: Property \"rrr\" not found on type \"Item\" at %1:5:39") - << QString(); + << QString() + << false; QTest::newRow("incompleteQmltypes") << QStringLiteral("incompleteQmltypes.qml") << QString("Warning: Type \"QPalette\" of base \"palette\" not found when accessing member \"weDontKnowIt\" at %1:5:34") - << QString(); + << QString() + << false; QTest::newRow("inheritanceCylce") << QStringLiteral("Cycle1.qml") << QString("Warning: Cycle2 is part of an inheritance cycle: Cycle2 -> Cycle3 -> Cycle1 -> Cycle2") - << QString(); + << QString() + << false; QTest::newRow("badQmldirImportAndDepend") << QStringLiteral("qmldirImportAndDepend/bad.qml") << QString("Warning: Item was not found. Did you add all import paths?") - << QString(); + << QString() + << false; QTest::newRow("javascriptMethodsInModule") << QStringLiteral("javascriptMethodsInModuleBad.qml") << QString("Warning: Property \"unknownFunc\" not found on type \"Foo\"") - << QString(); + << QString() + << false; QTest::newRow("badEnumFromQtQml") << QStringLiteral("badEnumFromQtQml.qml") << QString("Warning: Property \"Linear123\" not found on type \"QQmlEasingEnums\"") - << QString(); + << QString() + << false; QTest::newRow("anchors3") << QStringLiteral("anchors3.qml") << QString() - << QString(); + << QString() + << false; QTest::newRow("nanchors1") << QStringLiteral("nanchors1.qml") << QString() - << QString(); + << QString() + << false; QTest::newRow("nanchors2") << QStringLiteral("nanchors2.qml") << QString("unknown grouped property scope nanchors.") - << QString(); + << QString() + << false; QTest::newRow("nanchors3") << QStringLiteral("nanchors3.qml") << QString("unknown grouped property scope nanchors.") - << QString(); + << QString() + << false; QTest::newRow("badAliasObject") << QStringLiteral("badAliasObject.qml") << QString("Warning: Property \"wrongwrongwrong\" not found on type \"QtObject\"") - << QString(); + << QString() + << false; QTest::newRow("badScript") << QStringLiteral("badScript.qml") << QString("Warning: Property \"stuff\" not found on type \"Empty\"") - << QString(); + << QString() + << false; QTest::newRow("brokenNamespace") << QStringLiteral("brokenNamespace.qml") << QString("Warning: type not found in namespace at %1:4:17") - << QString(); + << QString() + << false; QTest::newRow("segFault (bad)") << QStringLiteral("SegFault.bad.qml") << QStringLiteral("Property \"foobar\" not found on type \"QQuickScreenAttached\"") - << QString(); + << QString() + << false; QTest::newRow("VariableUsedBeforeDeclaration") << QStringLiteral("useBeforeDeclaration.qml") << QStringLiteral("Variable \"argq\" is used before its declaration at 5:9. " "The declaration is at 6:13.") - << QString(); + << QString() + << false; QTest::newRow("SignalParameterMismatch") << QStringLiteral("namedSignalParameters.qml") << QStringLiteral("Parameter 1 to signal handler for \"onSig\" is called \"argarg\". " "The signal has a parameter of the same name in position 2.") - << QStringLiteral("onSig2"); + << QStringLiteral("onSig2") + << false; QTest::newRow("TooManySignalParameters") << QStringLiteral("tooManySignalParameters.qml") << QStringLiteral("Signal handler for \"onSig\" has more formal parameters " "than the signal it handles.") - << QString(); + << QString() + << false; QTest::newRow("OnAssignment") << QStringLiteral("onAssignment.qml") << QStringLiteral("Property \"loops\" not found on type \"bool\"") - << QString(); + << QString() + << false; QTest::newRow("BadAttached") << QStringLiteral("badAttached.qml") << QStringLiteral("unknown attached property scope WrongAttached.") - << QString(); + << QString() + << false; QTest::newRow("BadBinding") << QStringLiteral("badBinding.qml") << QStringLiteral("Binding assigned to \"doesNotExist\", but no property " "\"doesNotExist\" exists in the current element.") - << QString(); + << QString() + << false; QTest::newRow("BadPropertyType") << QStringLiteral("badPropertyType.qml") << QStringLiteral("No type found for property \"bad\". This may be due to a missing " "import statement or incomplete qmltypes files.") - << QString(); + << QString() + << false; QTest::newRow("Deprecation (Property, with reason)") << QStringLiteral("deprecatedPropertyReason.qml") << QStringLiteral("Property \"deprecated\" is deprecated (Reason: Test)") - << QString(); + << QString() + << false; QTest::newRow("Deprecation (Property, no reason)") << QStringLiteral("deprecatedProperty.qml") << QStringLiteral("Property \"deprecated\" is deprecated") - << QString(); + << QString() + << false; QTest::newRow("Deprecation (Type, with reason)") << QStringLiteral("deprecatedTypeReason.qml") << QStringLiteral("Type \"TypeDeprecatedReason\" is deprecated (Reason: Test)") - << QString(); + << QString() + << false; QTest::newRow("Deprecation (Type, no reason)") << QStringLiteral("deprecatedType.qml") << QStringLiteral("Type \"TypeDeprecated\" is deprecated") - << QString(); + << QString() + << false; QTest::newRow("MissingDefaultProperty") << QStringLiteral("defaultPropertyWithoutKeyword.qml") - << QStringLiteral("Cannot assign to non-existent default property") << QString(); + << QStringLiteral("Cannot assign to non-existent default property") << QString() << false; + QTest::newRow("DoubleAssignToDefaultProperty") << QStringLiteral("defaultPropertyWithDoubleAssignment.qml") << QStringLiteral("Cannot assign multiple objects to a default non-list property") - << QString(); + << QString() + << false; QTest::newRow("DefaultPropertyWithWrongType(string)") << QStringLiteral("defaultPropertyWithWrongType.qml") << QStringLiteral("Cannot assign to default property of incompatible type") - << QStringLiteral("Cannot assign to non-existent default property"); + << QStringLiteral("Cannot assign to non-existent default property") + << false; QTest::newRow("DefaultPropertyWithWrongType(var)") << QStringLiteral("defaultPropertyWithWrongType2.qml") << QStringLiteral("Cannot assign to default property of incompatible type") - << QStringLiteral("Cannot assign to non-existent default property"); + << QStringLiteral("Cannot assign to non-existent default property") + << false; QTest::newRow("InvalidImport") << QStringLiteral("invalidImport.qml") << QStringLiteral("Failed to import FooBar. Are your include paths set up properly?") - << QString(); + << QString() + << false; + QTest::newRow("Unused Import (simple)") + << QStringLiteral("unused_simple.qml") + << QStringLiteral("Unused import at %1:1:1") + << QString() + << true; + QTest::newRow("Unused Import (prefix)") + << QStringLiteral("unused_prefix.qml") + << QStringLiteral("Unused import at %1:1:1") + << QString() + << true; } void TestQmllint::dirtyQmlCode() @@ -472,6 +527,8 @@ void TestQmllint::dirtyQmlCode() QFETCH(QString, filename); QFETCH(QString, warningMessage); QFETCH(QString, notContained); + QFETCH(bool, exitsNormally); + if (warningMessage.contains(QLatin1String("%1"))) warningMessage = warningMessage.arg(testFile(filename)); @@ -480,7 +537,11 @@ void TestQmllint::dirtyQmlCode() QCOMPARE(process.exitStatus(), QProcess::NormalExit); QEXPECT_FAIL("anchors3", "We don't see that QQuickItem cannot be assigned to QQuickAnchorLine", Abort); QEXPECT_FAIL("nanchors1", "Invalid grouped properties are not always detected", Abort); - QVERIFY(process.exitCode() != 0); + + if (exitsNormally) + QVERIFY(process.exitCode() == 0); + else + QVERIFY(process.exitCode() != 0); }); QVERIFY(output.contains(warningMessage)); @@ -537,6 +598,8 @@ void TestQmllint::cleanQmlCode_data() QTest::newRow("defaultProperty") << QStringLiteral("defaultProperty.qml"); QTest::newRow("defaultPropertyList") << QStringLiteral("defaultPropertyList.qml"); QTest::newRow("duplicateQmldirImport") << QStringLiteral("qmldirImport/duplicate.qml"); + QTest::newRow("Used imports") << QStringLiteral("used.qml"); + QTest::newRow("Unused imports (multi)") << QStringLiteral("unused_multi.qml"); } void TestQmllint::cleanQmlCode() diff --git a/tools/qmllint/checkidentifiers.cpp b/tools/qmllint/checkidentifiers.cpp index 1ade63c1c0..7350a1f76e 100644 --- a/tools/qmllint/checkidentifiers.cpp +++ b/tools/qmllint/checkidentifiers.cpp @@ -38,7 +38,10 @@ class IssueLocationWithContext public: IssueLocationWithContext(const QString &code, const QQmlJS::SourceLocation &location) { int before = qMax(0,code.lastIndexOf(QLatin1Char('\n'), location.offset)); - m_beforeText = QStringView{code}.mid(before + 1, int(location.offset - (before + 1))); + + if (before != 0) before++; + + m_beforeText = QStringView{code}.mid(before, int(location.offset - before)); m_issueText = QStringView{code}.mid(location.offset, location.length); int after = code.indexOf(QLatin1Char('\n'), int(location.offset + location.length)); m_afterText = QStringView{code}.mid(int(location.offset + location.length), @@ -68,9 +71,11 @@ void CheckIdentifiers::printContext( const QString &code, ColorOutput *output, const QQmlJS::SourceLocation &location) { IssueLocationWithContext issueLocationWithContext { code, location }; - output->write(issueLocationWithContext.beforeText().toString(), Normal); + if (const QString beforeText = issueLocationWithContext.beforeText().toString(); !beforeText.isEmpty()) + output->write(beforeText, Normal); output->write(issueLocationWithContext.issueText().toString(), Error); - output->write(issueLocationWithContext.afterText().toString() + QLatin1Char('\n'), Normal); + if (const QString afterText = issueLocationWithContext.afterText().toString(); !afterText.isEmpty()) + output->write(afterText + QLatin1Char('\n'), Normal); int tabCount = issueLocationWithContext.beforeText().count(QLatin1Char('\t')); output->write(QString::fromLatin1(" ").repeated( issueLocationWithContext.beforeText().length() - tabCount) diff --git a/tools/qmllint/findwarnings.cpp b/tools/qmllint/findwarnings.cpp index c7f2a557f6..dd87293eb6 100644 --- a/tools/qmllint/findwarnings.cpp +++ b/tools/qmllint/findwarnings.cpp @@ -390,8 +390,13 @@ bool FindWarningVisitor::visit(QQmlJS::AST::UiScriptBinding *uisb) bool FindWarningVisitor::visit(QQmlJS::AST::IdentifierExpression *idexp) { + const QString name = idexp->name.toString(); + if (name.front().isUpper() && m_importTypeLocationMap.contains(name)) { + m_usedTypes.insert(name); + } + m_memberAccessChains[m_currentScope].append( - {{idexp->name.toString(), QString(), idexp->firstSourceLocation()}}); + {{name, QString(), idexp->firstSourceLocation()}}); m_fieldMemberBase = idexp; return true; } @@ -489,6 +494,25 @@ bool FindWarningVisitor::check() outstandingConnection.uiod->initializer->accept(this); } + auto unusedImports = m_importLocations; + for (const QString &type : m_usedTypes) { + for (const auto &importLocation : m_importTypeLocationMap.values(type)) + unusedImports.remove(importLocation); + + // If there are no more unused imports left we can abort early + if (unusedImports.isEmpty()) + break; + } + + for (const auto &import : unusedImports) { + m_colorOut.writePrefixedMessage( + QString::fromLatin1("Unused import at %1:%2:%3\n") + .arg(m_filePath) + .arg(import.startLine).arg(import.startColumn), + Info); + CheckIdentifiers::printContext(m_code, &m_colorOut, import); + } + if (!m_warnUnqualified) return m_errors.isEmpty(); @@ -617,6 +641,7 @@ void FindWarningVisitor::endVisit(QQmlJS::AST::FieldMemberExpression *fieldMembe { using namespace QQmlJS::AST; ExpressionNode *base = fieldMember->base; + while (auto *nested = cast<NestedExpression *>(base)) base = nested->expression; @@ -631,9 +656,17 @@ void FindWarningVisitor::endVisit(QQmlJS::AST::FieldMemberExpression *fieldMembe auto &chain = m_memberAccessChains[m_currentScope]; + Q_ASSERT(!chain.last().isEmpty()); + + const QString name = fieldMember->name.toString(); + if (m_importTypeLocationMap.contains(name)) { + if (auto it = m_rootScopeImports.find(name); it != m_rootScopeImports.end() && !*(it)) + m_usedTypes.insert(name); + } + chain.last().append(FieldMember { - fieldMember->name.toString(), type, fieldMember->identifierToken + name, type, fieldMember->identifierToken }); m_fieldMemberBase = fieldMember; } else { |