From 675aea369c0906600fedc7ab8164dfeb05924fdc Mon Sep 17 00:00:00 2001 From: Sami Shalayel Date: Tue, 14 Nov 2023 14:55:49 +0100 Subject: [PATCH] QQmlDomAstCreator: do not call resourceFilesFromBuildFolders twice Its seems that d0fcb75aab1b5a91260fb378d761257e1f2e4787 introduced a bug where the resourceFilesFromBuildFolders was called twice during Dom creation: the resources files were not found in the second call and the Dom was not able to find the qmldir, qmltypes, etc. The problem was that the current test only tests if there are linting errors, but currently, the Dom created by the linting module is not the Dom used by all other modules. Therefore, extend the tests to test this second Dom by asking for completions of the type of the build/source folder. Also change the test to be a data test. Making the linting module of qmlls reuse the Dom that the other modules also uses is tracked in QTBUG-119126. Task-number: QTBUG-119126 Change-Id: I3a8db0f854512323b61b4763e8a2d84d407f46d8 Reviewed-by: Fabian Kosmale Reviewed-by: Qt CI Bot --- src/qmldom/qqmldomastcreator.cpp | 3 +- .../auto/qmlls/modules/tst_qmlls_modules.cpp | 78 ++++++++++++------- tests/auto/qmlls/modules/tst_qmlls_modules.h | 4 +- 3 files changed, 51 insertions(+), 34 deletions(-) diff --git a/src/qmldom/qqmldomastcreator.cpp b/src/qmldom/qqmldomastcreator.cpp index 4ffc5613d5..b51ac47a16 100644 --- a/src/qmldom/qqmldomastcreator.cpp +++ b/src/qmldom/qqmldomastcreator.cpp @@ -2341,8 +2341,7 @@ void createDom(MutableDomItem &&qmlFile, DomCreationOptions options) if (auto environmentPtr = qmlFile.environment().ownerAs()) { const QStringList resourceFiles = resourceFilesFromBuildFolders(environmentPtr->loadPaths()); - mapper = std::make_shared( - resourceFilesFromBuildFolders(resourceFiles)); + mapper = std::make_shared(resourceFiles); } auto importer = std::make_shared(importPathsFrom(qmlFile), mapper.get(), true); diff --git a/tests/auto/qmlls/modules/tst_qmlls_modules.cpp b/tests/auto/qmlls/modules/tst_qmlls_modules.cpp index b2006d10a4..08dac409bf 100644 --- a/tests/auto/qmlls/modules/tst_qmlls_modules.cpp +++ b/tests/auto/qmlls/modules/tst_qmlls_modules.cpp @@ -1221,44 +1221,44 @@ void tst_qmlls_modules::rangeFormatting() QTRY_VERIFY_WITH_TIMEOUT(*didFinish, 10000); } -void tst_qmlls_modules::qmldirImportsFromBuild() +enum AddBuildDirOption : bool { AddBuildDir, DoNotAddBuildDir }; + +void tst_qmlls_modules::qmldirImports_data() { - const QString filePath = u"completions/fromBuildDir.qml"_s; - const auto uri = openFile(filePath); - QVERIFY(uri); + QTest::addColumn("filePath"); + QTest::addColumn("addBuildDirectory"); + QTest::addColumn("line"); + QTest::addColumn("character"); + QTest::addColumn("expectedCompletion"); - Notifications::AddBuildDirsParams bDirs; - UriToBuildDirs ub; - ub.baseUri = *uri; - ub.buildDirs.append(testFile("buildDir").toUtf8()); - bDirs.buildDirsToSet.append(ub); - m_protocol->typedRpc()->sendNotification(QByteArray(Notifications::AddBuildDirsMethod), bDirs); - - bool diagnosticOk = false; - m_protocol->registerPublishDiagnosticsNotificationHandler( - [&diagnosticOk, &uri](const QByteArray &, const PublishDiagnosticsParams &p) { - if (p.uri != *uri) - return; - - if constexpr (enable_debug_output) { - for (const auto &x : p.diagnostics) { - qDebug() << x.message; - } - } - QCOMPARE(p.diagnostics.size(), 0); - diagnosticOk = true; - }); - - QTRY_VERIFY_WITH_TIMEOUT(diagnosticOk, 5000); + QTest::addRow("fromBuildFolder") + << u"completions/fromBuildDir.qml"_s << AddBuildDir << 3 << 1 << u"BuildDirType"_s; + QTest::addRow("fromSourceFolder") + << u"sourceDir/Main.qml"_s << DoNotAddBuildDir << 3 << 1 << u"Button"_s; } -void tst_qmlls_modules::qmldirImportsFromSource() +void tst_qmlls_modules::qmldirImports() { - const QString filePath = u"sourceDir/Main.qml"_s; + QFETCH(QString, filePath); + QFETCH(AddBuildDirOption, addBuildDirectory); + QFETCH(int, line); + QFETCH(int, character); + QFETCH(QString, expectedCompletion); + const auto uri = openFile(filePath); QVERIFY(uri); + if (addBuildDirectory == AddBuildDir) { + Notifications::AddBuildDirsParams bDirs; + UriToBuildDirs ub; + ub.baseUri = *uri; + ub.buildDirs.append(testFile("buildDir").toUtf8()); + bDirs.buildDirsToSet.append(ub); + m_protocol->typedRpc()->sendNotification(QByteArray(Notifications::AddBuildDirsMethod), bDirs); + } + bool diagnosticOk = false; + bool completionOk = false; m_protocol->registerPublishDiagnosticsNotificationHandler( [&diagnosticOk, &uri](const QByteArray &, const PublishDiagnosticsParams &p) { if (p.uri != *uri) @@ -1273,7 +1273,25 @@ void tst_qmlls_modules::qmldirImportsFromSource() diagnosticOk = true; }); - QTRY_VERIFY_WITH_TIMEOUT(diagnosticOk, 5000); + // Currently, the Dom is created twice in qmlls: once for the linting and once for all other + // features. Therefore, also test that this second dom also uses the right resource files. + CompletionParams cParams; + cParams.position.line = line - 1; // LSP is 0 based + cParams.position.character = character - 1; // LSP is 0 based + cParams.textDocument.uri = *uri; + + m_protocol->requestCompletion(cParams, [&completionOk, &expectedCompletion](auto res) { + const QList *cItems = std::get_if>(&res); + + QSet labels; + for (const CompletionItem &c : *cItems) { + labels << c.label; + } + QVERIFY(labels.contains(expectedCompletion)); + completionOk = true; + }); + + QTRY_VERIFY_WITH_TIMEOUT(diagnosticOk && completionOk, 5000); } void tst_qmlls_modules::quickFixes_data() diff --git a/tests/auto/qmlls/modules/tst_qmlls_modules.h b/tests/auto/qmlls/modules/tst_qmlls_modules.h index 58759dae88..b3c3602959 100644 --- a/tests/auto/qmlls/modules/tst_qmlls_modules.h +++ b/tests/auto/qmlls/modules/tst_qmlls_modules.h @@ -58,8 +58,8 @@ private slots: void linting(); void rangeFormatting_data(); void rangeFormatting(); - void qmldirImportsFromBuild(); - void qmldirImportsFromSource(); + void qmldirImports_data(); + void qmldirImports(); void quickFixes_data(); void quickFixes();