From 892ff6c8e2f47abad92b99f48b6c045a90f26d8f Mon Sep 17 00:00:00 2001 From: Sami Shalayel Date: Mon, 7 Apr 2025 16:57:53 +0200 Subject: [PATCH] qqmljscontextproperties: "grep" for setContextProperty calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add code to Grep the project folder for "setContextProperty" calls, so that a later commit can warn about usages of those context properties. Use grep to find files with setContextProperty calls, and then run a QRegularExpression on the files to extract the names and locations of the context properties. Add a fallback in case there is no grep on the system, and don't run grep on windows. Also avoid unused functions errors on windows where grep is not run so its output also does not need to be parsed. Task-number: QTBUG-128232 Change-Id: I15ceb2bbeb6ae6ac83f4bfa85b8a44d5a897d0a7 Reviewed-by: Olivier De Cannière --- src/qmlcompiler/CMakeLists.txt | 1 + src/qmlcompiler/qqmljscontextproperties.cpp | 123 ++++++++++++++++++ src/qmlcompiler/qqmljscontextproperties_p.h | 45 +++++++ .../qml/MyContextProperties.qml | 8 ++ .../src/Sub/Sub2/Sub3/register.cpp | 23 ++++ .../data/ContextProperties/src/main.cpp | 24 ++++ tests/auto/qml/qmllint/tst_qmllint.cpp | 42 ++++++ 7 files changed, 266 insertions(+) create mode 100644 src/qmlcompiler/qqmljscontextproperties.cpp create mode 100644 src/qmlcompiler/qqmljscontextproperties_p.h create mode 100644 tests/auto/qml/qmllint/data/ContextProperties/qml/MyContextProperties.qml create mode 100644 tests/auto/qml/qmllint/data/ContextProperties/src/Sub/Sub2/Sub3/register.cpp create mode 100644 tests/auto/qml/qmllint/data/ContextProperties/src/main.cpp diff --git a/src/qmlcompiler/CMakeLists.txt b/src/qmlcompiler/CMakeLists.txt index 32fa2cf1d8..07097f451e 100644 --- a/src/qmlcompiler/CMakeLists.txt +++ b/src/qmlcompiler/CMakeLists.txt @@ -48,6 +48,7 @@ qt_internal_add_module(QmlCompiler qqmlsasourcelocation.cpp qqmlsasourcelocation.h qqmlsasourcelocation_p.h qresourcerelocater.cpp qresourcerelocater_p.h qqmljstranslationfunctionmismatchcheck_p.h qqmljstranslationfunctionmismatchcheck.cpp + qqmljscontextproperties_p.h qqmljscontextproperties.cpp NO_UNITY_BUILD_SOURCES qqmljsoptimizations.cpp PUBLIC_LIBRARIES diff --git a/src/qmlcompiler/qqmljscontextproperties.cpp b/src/qmlcompiler/qqmljscontextproperties.cpp new file mode 100644 index 0000000000..67be7c8661 --- /dev/null +++ b/src/qmlcompiler/qqmljscontextproperties.cpp @@ -0,0 +1,123 @@ +// Copyright (C) 2025 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#include "qqmljscontextproperties_p.h" +#include +#include +#include +#include + +#if QT_CONFIG(process) +# include +#endif + +QT_BEGIN_NAMESPACE + +namespace QQmlJS { + +using namespace Qt::StringLiterals; + +// There are many ways to set context properties without triggering the regexp in s_pattern, +// but its supposed to catch most context properties set via "setContextProperty". +static constexpr QLatin1StringView s_pattern = + R"x((\.|->)setContextProperty\s*\(\s*(QStringLiteral\s*\(|QString\s*\(|QLatin1String(View)?\s*\(|u)?\s*"([^"]*)")x"_L1; +static constexpr int s_contextPropertyNameIdxInPattern = 4; + +// TODO: use a central list of file extensions that can also be used by qmetatypesjsonprocessor.cpp +// (that needs header file extensions) and Qt6QmlMacros.cmake. +static constexpr std::array s_fileFilters = { + "*.cpp"_L1, "*.cxx"_L1, "*.cc"_L1, "*.c"_L1, "*.c++"_L1, + "*.hpp"_L1, "*.hxx"_L1, "*.hh"_L1, "*.h"_L1, "*.h++"_L1, +}; + +static const QRegularExpression s_matchSetContextProperty{ s_pattern, + QRegularExpression::MultilineOption }; + +static void collectAllFromFile(const QString &filePath, ContextProperties *output) +{ + Q_ASSERT(output); + + QFile file(filePath); + if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) + return; + + const QString fileContent = QString::fromUtf8(file.readAll()); + for (const auto &match : s_matchSetContextProperty.globalMatch(fileContent)) { + const quint32 offset = match.capturedStart(s_contextPropertyNameIdxInPattern); + const quint32 length = match.capturedLength(s_contextPropertyNameIdxInPattern); + const auto [row, column] = QQmlJS::SourceLocation::rowAndColumnFrom(fileContent, offset); + const QQmlJS::SourceLocation sourceLocation{ offset, length, row, column }; + + (*output)[match.captured(s_contextPropertyNameIdxInPattern)].append( + ContextProperty{ filePath, sourceLocation }); + } +} + +static ContextProperties grepFallback(const QList &rootUrls) +{ + ContextProperties result; + + const QStringList fileFilters{ s_fileFilters.begin(), s_fileFilters.end() }; + + for (const QString &url : rootUrls) { + for (const auto &dirEntry : QDirListing{ url, fileFilters, + QDirListing::IteratorFlag::Recursive + | QDirListing::IteratorFlag::FilesOnly }) { + + const QString filePath = dirEntry.filePath(); + collectAllFromFile(filePath, &result); + } + } + + return result; +} + +#if QT_CONFIG(process) && !defined(Q_OS_WINDOWS) +static ContextProperties parseGrepOutput(const QString &output) +{ + ContextProperties result; + + for (const auto line : QStringTokenizer{ output, "\n"_L1, Qt::SkipEmptyParts }) + collectAllFromFile(line.toString(), &result); + + return result; +} +#endif + +/*! + \internal + Uses grep to find files that have setContextProperty()-calls, and then search matching files + with QRegularExpression to extract the location and name of the found context properties. +*/ +ContextProperties ContextProperty::collectAllFrom(const QList &rootUrls) +{ +#if QT_CONFIG(process) && !defined(Q_OS_WINDOWS) + if (qEnvironmentVariableIsSet("QT_QML_NO_GREP")) + return grepFallback(rootUrls); + + QProcess grep; + QStringList arguments{ "--recursive"_L1, + "--null-data"_L1, // match multiline patterns + "--files-with-matches"_L1, + "--extended-regexp"_L1, // the pattern is "extended" + "-e"_L1, + s_pattern }; + + // don't search non-cpp files + for (const auto fileFilter : s_fileFilters) + arguments << "--include"_L1 << fileFilter; + + arguments.append(rootUrls); + grep.start("grep"_L1, arguments); + grep.waitForFinished(); + if (grep.exitStatus() == QProcess::NormalExit && grep.exitCode() == 0) { + const QString output = QString::fromUtf8(grep.readAllStandardOutput()); + return parseGrepOutput(output); + } +#endif + return grepFallback(rootUrls); +} + +} // namespace QQmlJS + +QT_END_NAMESPACE diff --git a/src/qmlcompiler/qqmljscontextproperties_p.h b/src/qmlcompiler/qqmljscontextproperties_p.h new file mode 100644 index 0000000000..07dfcc1b23 --- /dev/null +++ b/src/qmlcompiler/qqmljscontextproperties_p.h @@ -0,0 +1,45 @@ +// Copyright (C) 2025 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#ifndef QQMLJSCONTEXTPROPERTIES_P_H +#define QQMLJSCONTEXTPROPERTIES_P_H + +// +// W A R N I N G +// ------------- +// +// This file is not part of the Qt API. It exists purely as an +// implementation detail. This header file may change from version to +// version without notice, or even be removed. +// +// We mean it. + +#include + +#include +#include +#include + +#include + +QT_BEGIN_NAMESPACE + +namespace QQmlJS { +class LoggerCategory; + +struct ContextProperty; +using ContextProperties = QHash>; + +struct Q_QMLCOMPILER_EXPORT ContextProperty +{ + QString filename; + QQmlJS::SourceLocation location; + + static ContextProperties collectAllFrom(const QList &rootUrls); +}; + +} // namespace QQmlJS + +QT_END_NAMESPACE + +#endif // QQMLJSCONTEXTPROPERTIES_P_H diff --git a/tests/auto/qml/qmllint/data/ContextProperties/qml/MyContextProperties.qml b/tests/auto/qml/qmllint/data/ContextProperties/qml/MyContextProperties.qml new file mode 100644 index 0000000000..02b76e6730 --- /dev/null +++ b/tests/auto/qml/qmllint/data/ContextProperties/qml/MyContextProperties.qml @@ -0,0 +1,8 @@ +import QtQuick + +Item { + property var a: myContextProperty1 + property var b: myContextProperty2 + property var c: myContextProperty3 + property var d: myContextProperty4 +} diff --git a/tests/auto/qml/qmllint/data/ContextProperties/src/Sub/Sub2/Sub3/register.cpp b/tests/auto/qml/qmllint/data/ContextProperties/src/Sub/Sub2/Sub3/register.cpp new file mode 100644 index 0000000000..49db5fb187 --- /dev/null +++ b/tests/auto/qml/qmllint/data/ContextProperties/src/Sub/Sub2/Sub3/register.cpp @@ -0,0 +1,23 @@ +// Copyright (C) 2025 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only + +#include +#include + +void registerContextProperties(QQuickView &view) +{ + view.rootContext()->setContextProperty( + + + + QLatin1StringView + ( "myContextProperty3") , + QDateTime::currentDateTime()) + ; + view.rootContext()->setContextProperty( QString + ( "myContextProperty4"), + QDateTime::currentDateTime()); + + view.rootContext()->setContextProperty(QString("myContextProperty4"), + QDateTime::currentDateTime()); +} diff --git a/tests/auto/qml/qmllint/data/ContextProperties/src/main.cpp b/tests/auto/qml/qmllint/data/ContextProperties/src/main.cpp new file mode 100644 index 0000000000..b50f50112c --- /dev/null +++ b/tests/auto/qml/qmllint/data/ContextProperties/src/main.cpp @@ -0,0 +1,24 @@ +// Copyright (C) 2025 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only + +#include +#include +#include + +int main(int argc, char *argv[]) +{ + QGuiApplication app(argc, argv); + + QQuickView view; + + view.rootContext()->setContextProperty("myContextProperty1", QDateTime::currentDateTime()); + view.rootContext()->setContextProperty( + u"myContextProperty2"_s, + QDateTime::currentDateTime()); + registerContentProperties(view); + + view.loadFromModule("ContextProperty", "Main"); + view.show(); + + return app.exec(); +} diff --git a/tests/auto/qml/qmllint/tst_qmllint.cpp b/tests/auto/qml/qmllint/tst_qmllint.cpp index 1785a1ea0e..0f827e82aa 100644 --- a/tests/auto/qml/qmllint/tst_qmllint.cpp +++ b/tests/auto/qml/qmllint/tst_qmllint.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -85,6 +86,9 @@ private Q_SLOTS: void cleanJsSnippet_data(); void cleanJsSnippet(); + void contextPropertiesFromRootUrls_data(); + void contextPropertiesFromRootUrls(); + void compilerWarnings_data(); void compilerWarnings(); @@ -1619,6 +1623,44 @@ void TestQmllint::cleanJsSnippet() checkResult(warnings, result, [] { }, [] { }, [] { }); } +using ExpectedProperties = QHash; + +void TestQmllint::contextPropertiesFromRootUrls_data() +{ + QTest::addColumn("rootUrls"); + QTest::addColumn("expectedProperties"); + QTest::addColumn("disableGrep"); + + for (const bool disableGrep : { false, true }) { + QTest::addRow("grep%s", disableGrep ? "-fallback" : "") + << QStringList{ testFile("ContextProperties/src"_L1) } + << ExpectedProperties{ { "myContextProperty1", 1 }, + { "myContextProperty2", 1 }, + { "myContextProperty3", 1 }, + { "myContextProperty4", 2 } } + << disableGrep; + } +} + +void TestQmllint::contextPropertiesFromRootUrls() +{ + QFETCH(QStringList, rootUrls); + QFETCH(ExpectedProperties, expectedProperties); + QFETCH(bool, disableGrep); + + if (disableGrep) + qputenv("QT_QML_NO_GREP", "1"); + const auto properties = QQmlJS::ContextProperty::collectAllFrom(rootUrls); + if (disableGrep) + qunsetenv("QT_QML_NO_GREP"); + + QCOMPARE(properties.size(), expectedProperties.size()); + + for (auto [key, value] : properties.asKeyValueRange()) { + QVERIFY(expectedProperties.contains(key)); + QCOMPARE(value.size(), expectedProperties[key]); + } +} void TestQmllint::cleanQmlCode_data() {