From f1a084c5e5d2d20f105d4e3fe1a762248c0b7460 Mon Sep 17 00:00:00 2001 From: Sami Shalayel Date: Tue, 20 Jun 2023 13:50:14 +0200 Subject: [PATCH] qmlls: fix the order in which the build directory is obtained Make sure that the order in which the build directory is obtained is intuitive. The preference should be: 1. Command line option 2. If there is no command line option, search for the environment variable. 3. If there environment variable, check for .qmlls.ini files Add a test to test all combinations of this. Check that the paths passed via the --build-dir argument to qmlls does point to a directory and complain when it does not. Same for the paths passed via the QMLLS_BUILD_DIRS environment variable. Also inform the user when none of --build-dir nor QMLLS_BUILD_DIRS was passed that the build dir will be set by the .qmlls.ini files. Also, remove the broken #if QT_CONFIG(commandlineparser)-block in the code: the parser defined in this #if-block is also used outside the #if. Instead, do not compile qmlls if the commandlineparser feature is disabled. Also use platform-specific separators when passing multiple build directories. [ChangeLog][Qmlls] Qmlls warns when inexisting build folder paths are passed to the --build-dir argument or the QMLLS_BUILD_DIRS environment variable. Also, it will inform the user if the build directory was set using the command line option, the environment variable or an .qmlls.ini settings file, if existent in the source folder. Task-number: QTBUG-114474 Change-Id: I6cb4f2db47945f8ea9549b7519a92f9d15d5ce66 Reviewed-by: Ulf Hermann (cherry picked from commit 13dc5b3c9d40d10cc63c3893b454ccd9cb50a9b8) Reviewed-by: Qt Cherry-pick Bot --- src/qmlls/qqmlcodemodel.cpp | 30 ++++++--- tests/auto/qmlls/CMakeLists.txt | 1 + tests/auto/qmlls/qqmlcodemodel/CMakeLists.txt | 27 ++++++++ .../qqmlcodemodel/tst_qmlls_qqmlcodemodel.cpp | 63 +++++++++++++++++++ .../qqmlcodemodel/tst_qmlls_qqmlcodemodel.h | 33 ++++++++++ tools/CMakeLists.txt | 2 +- tools/qmlls/qmllanguageservertool.cpp | 49 +++++++++++++-- 7 files changed, 190 insertions(+), 15 deletions(-) create mode 100644 tests/auto/qmlls/qqmlcodemodel/CMakeLists.txt create mode 100644 tests/auto/qmlls/qqmlcodemodel/tst_qmlls_qqmlcodemodel.cpp create mode 100644 tests/auto/qmlls/qqmlcodemodel/tst_qmlls_qqmlcodemodel.h diff --git a/src/qmlls/qqmlcodemodel.cpp b/src/qmlls/qqmlcodemodel.cpp index dc912f4b3a..efb2bc0695 100644 --- a/src/qmlls/qqmlcodemodel.cpp +++ b/src/qmlls/qqmlcodemodel.cpp @@ -556,22 +556,32 @@ QStringList QQmlCodeModel::buildPathsForFileUrl(const QByteArray &url) } } QString path = url2Path(url); + + // fallback to the empty root, if is has an entry. + // This is the buildPath that is passed to qmlls via --build-dir. + if (buildPaths.isEmpty()) { + buildPaths += buildPathsForRootUrl(QByteArray()); + } + + // look in the QMLLS_BUILD_DIRS environment variable + if (buildPaths.isEmpty()) { + QStringList envPaths = qEnvironmentVariable("QMLLS_BUILD_DIRS") + .split(QDir::listSeparator(), Qt::SkipEmptyParts); + buildPaths += envPaths; + } + + // look in the settings. + // This is the one that is passed via the .qmlls.ini file. if (buildPaths.isEmpty() && m_settings) { - // look in the settings m_settings->search(path); QString buildDir = QStringLiteral(u"buildDir"); if (m_settings->isSet(buildDir)) - buildPaths += m_settings->value(buildDir).toString().split(u',', Qt::SkipEmptyParts); + buildPaths += m_settings->value(buildDir).toString().split(QDir::listSeparator(), + Qt::SkipEmptyParts); } + + // heuristic to find build directory if (buildPaths.isEmpty()) { - // default values - buildPaths += buildPathsForRootUrl(QByteArray()); - } - // env variable - QStringList envPaths = qEnvironmentVariable("QMLLS_BUILD_DIRS").split(u',', Qt::SkipEmptyParts); - buildPaths += envPaths; - if (buildPaths.isEmpty()) { - // heuristic to find build dir QDir d(path); d.setNameFilters(QStringList({ u"build*"_s })); const int maxDirDepth = 8; diff --git a/tests/auto/qmlls/CMakeLists.txt b/tests/auto/qmlls/CMakeLists.txt index 2865b9f7f4..c4b3fab6c7 100644 --- a/tests/auto/qmlls/CMakeLists.txt +++ b/tests/auto/qmlls/CMakeLists.txt @@ -7,5 +7,6 @@ if (TARGET Qt::LanguageServerPrivate AND TARGET Qt::qmlls) add_subdirectory(qmlls) add_subdirectory(utils) add_subdirectory(modules) + add_subdirectory(qqmlcodemodel) endif() endif() diff --git a/tests/auto/qmlls/qqmlcodemodel/CMakeLists.txt b/tests/auto/qmlls/qqmlcodemodel/CMakeLists.txt new file mode 100644 index 0000000000..a4437373d8 --- /dev/null +++ b/tests/auto/qmlls/qqmlcodemodel/CMakeLists.txt @@ -0,0 +1,27 @@ +# Copyright (C) 2023 The Qt Company Ltd. +# SPDX-License-Identifier: BSD-3-Clause + +file(GLOB_RECURSE test_data_glob + RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} + data) +list(APPEND test_data ${test_data_glob}) + +qt_internal_add_test(tst_qmlls_qqmlcodemodel + SOURCES + tst_qmlls_qqmlcodemodel.cpp tst_qmlls_qqmlcodemodel.h + DEFINES + QT_QMLLS_QQMLCODEMODEL_DATADIR="${CMAKE_CURRENT_SOURCE_DIR}/data" + LIBRARIES + Qt::Core + Qt::QmlDomPrivate + Qt::LanguageServerPrivate + Qt::Test + Qt::QuickTestUtilsPrivate + Qt::QmlLSPrivate + TESTDATA ${test_data} +) + +qt_internal_extend_target(tst_qmlls_qqmlcodemodel CONDITION ANDROID OR IOS + DEFINES + QT_QMLLS_QQMLCODEMODEL_DATADIR=":/domdata" +) diff --git a/tests/auto/qmlls/qqmlcodemodel/tst_qmlls_qqmlcodemodel.cpp b/tests/auto/qmlls/qqmlcodemodel/tst_qmlls_qqmlcodemodel.cpp new file mode 100644 index 0000000000..73c0d0b2c0 --- /dev/null +++ b/tests/auto/qmlls/qqmlcodemodel/tst_qmlls_qqmlcodemodel.cpp @@ -0,0 +1,63 @@ +// Copyright (C) 2023 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#include "tst_qmlls_qqmlcodemodel.h" + +#include +#include + +void tst_qmlls_qqmlcodemodel::buildPathsForFileUrl_data() +{ + QTest::addColumn("pathFromIniFile"); + QTest::addColumn("pathFromEnvironmentVariable"); + QTest::addColumn("pathFromCommandLine"); + QTest::addColumn("expectedPath"); + + const QString path1 = u"/Users/helloWorld/build-myProject"_s; + const QString path2 = u"/Users/helloWorld/build-custom"_s; + const QString path3 = u"/Users/helloWorld/build-12345678"_s; + + QTest::addRow("justCommandLine") << QString() << QString() << path1 << path1; + QTest::addRow("all3") << path1 << path2 << path3 << path3; + + QTest::addRow("commandLineOverridesEnvironmentVariable") + << QString() << path2 << path3 << path3; + QTest::addRow("commandLineOverridesIniFile") << path2 << QString() << path3 << path3; + + QTest::addRow("EnvironmentVariableOverridesIniFile") << path1 << path2 << QString() << path2; + QTest::addRow("iniFile") << path1 << QString() << QString() << path1; + QTest::addRow("environmentVariable") << QString() << path3 << QString() << path3; +} + +void tst_qmlls_qqmlcodemodel::buildPathsForFileUrl() +{ + QFETCH(QString, pathFromIniFile); + QFETCH(QString, pathFromEnvironmentVariable); + QFETCH(QString, pathFromCommandLine); + QFETCH(QString, expectedPath); + + QQmlToolingSettings settings(u"qmlls"_s); + if (!pathFromIniFile.isEmpty()) + settings.addOption("buildDir", pathFromIniFile); + + constexpr char environmentVariable[] = "QMLLS_BUILD_DIRS"; + qunsetenv(environmentVariable); + if (!pathFromEnvironmentVariable.isEmpty()) { + qputenv(environmentVariable, pathFromEnvironmentVariable.toUtf8()); + } + + QmlLsp::QQmlCodeModel model(nullptr, &settings); + if (!pathFromCommandLine.isEmpty()) + model.setBuildPathsForRootUrl(QByteArray(), QStringList{ pathFromCommandLine }); + + // use nonexistent path to avoid loading random .qmlls.ini files that might be laying around. + // in this case, it should abort the search and the standard value we set in the settings + const QByteArray nonExistentUrl = + QUrl::fromLocalFile(u"./___thispathdoesnotexist123___/abcdefghijklmnop"_s).toEncoded(); + + QStringList result = model.buildPathsForFileUrl(nonExistentUrl); + QCOMPARE(result.size(), 1); + QCOMPARE(result.front(), expectedPath); +} + +QTEST_MAIN(tst_qmlls_qqmlcodemodel) diff --git a/tests/auto/qmlls/qqmlcodemodel/tst_qmlls_qqmlcodemodel.h b/tests/auto/qmlls/qqmlcodemodel/tst_qmlls_qqmlcodemodel.h new file mode 100644 index 0000000000..6b1a757b40 --- /dev/null +++ b/tests/auto/qmlls/qqmlcodemodel/tst_qmlls_qqmlcodemodel.h @@ -0,0 +1,33 @@ +// Copyright (C) 2023 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#ifndef TST_QMLLS_QQMLCODEMODEL_H +#define TST_QMLLS_QQMLCODEMODEL_H + +#include +#include +#include + +#include +#include +#include + +#include + +#include + +using namespace Qt::StringLiterals; + +class tst_qmlls_qqmlcodemodel : public QQmlDataTest +{ + Q_OBJECT + +public: + tst_qmlls_qqmlcodemodel() : QQmlDataTest(QT_QMLLS_QQMLCODEMODEL_DATADIR) { } + +private slots: + void buildPathsForFileUrl_data(); + void buildPathsForFileUrl(); +}; + +#endif // TST_QMLLS_QQMLCODEMODEL_H diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt index 75b240aeea..b86d3251a3 100644 --- a/tools/CMakeLists.txt +++ b/tools/CMakeLists.txt @@ -11,7 +11,7 @@ if(QT_FEATURE_commandlineparser) endif() add_subdirectory(qmlimportscanner) add_subdirectory(qmlformat) -if (TARGET Qt::LanguageServerPrivate +if (TARGET Qt::LanguageServerPrivate AND QT_FEATURE_commandlineparser AND NOT WASM AND NOT IOS AND NOT ANDROID AND NOT QNX AND NOT INTEGRITY AND NOT WEBOS) add_subdirectory(qmlls) endif() diff --git a/tools/qmlls/qmllanguageservertool.cpp b/tools/qmlls/qmllanguageservertool.cpp index d1d9e53810..f30cf1655c 100644 --- a/tools/qmlls/qmllanguageservertool.cpp +++ b/tools/qmlls/qmllanguageservertool.cpp @@ -132,7 +132,7 @@ int main(int argv, char *argc[]) QCoreApplication app(argv, argc); QCoreApplication::setApplicationName("qmlls"); QCoreApplication::setApplicationVersion(QT_VERSION_STR); -#if QT_CONFIG(commandlineparser) + QCommandLineParser parser; QQmlToolingSettings settings(QLatin1String("qmlls")); parser.setApplicationDescription(QLatin1String(R"(QML languageserver)")); @@ -204,7 +204,6 @@ int main(int argv, char *argc[]) QThread::sleep(waitSeconds); qDebug() << "starting"; } -#endif QMutex writeMutex; QQmlLanguageServer qmlServer( [&writeMutex](const QByteArray &data) { @@ -213,8 +212,50 @@ int main(int argv, char *argc[]) std::cout.flush(); }, (parser.isSet(ignoreSettings) ? nullptr : &settings)); - if (parser.isSet(buildDirOption)) - qmlServer.codeModel()->setBuildPathsForRootUrl(QByteArray(), parser.values(buildDirOption)); + + const QStringList envPaths = + qEnvironmentVariable("QMLLS_BUILD_DIRS").split(u',', Qt::SkipEmptyParts); + for (const QString &envPath : envPaths) { + QFileInfo info(envPath); + if (!info.exists()) { + qWarning() << "Argument" << buildDir << "passed via QMLLS_BUILD_DIRS does not exist."; + } else if (!info.isDir()) { + qWarning() << "Argument" << buildDir + << "passed via QMLLS_BUILD_DIRS is not a directory."; + } + } + + QStringList buildDirs; + if (parser.isSet(buildDirOption)) { + buildDirs = parser.values(buildDirOption); + for (const QString &buildDir : buildDirs) { + QFileInfo info(buildDir); + if (!info.exists()) { + qWarning() << "Argument" << buildDir << "passed to --build-dir does not exist."; + } else if (!info.isDir()) { + qWarning() << "Argument" << buildDir << "passed to --build-dir is not a directory."; + } + } + qmlServer.codeModel()->setBuildPathsForRootUrl(QByteArray(), buildDirs); + } + + if (!buildDirs.isEmpty()) { + qInfo() << "Using the build directories passed via the --build-dir option:" + << buildDirs.join(", "); + } else if (!envPaths.isEmpty()) { + qInfo() << "Using the build directories passed via the QMLLS_BUILD_DIRS environment " + "variable" + << buildDirs.join(", "); + } else { + qInfo() << "Using the build directories found in the .qmlls.ini file. Your build folder " + "might not be found if no .qmlls.ini files are present in the root source " + "folder."; + } + + if (buildDirs.isEmpty() && envPaths.isEmpty()) { + qInfo() << "Build directory path omitted: Your source folders will be searched for " + ".qmlls.ini files."; + } StdinReader r; QObject::connect(&r, &StdinReader::receivedData, qmlServer.server(), &QLanguageServer::receiveData);