aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSami Shalayel <sami.shalayel@qt.io>2023-12-21 11:04:19 +0100
committerSami Shalayel <sami.shalayel@qt.io>2024-01-16 08:07:18 +0100
commitd06ed7fdd8beec490e453ce493e53041f58a2107 (patch)
treeba1ad3a2a47df8eb8226f76f8c16c61222878016
parent1a545f4bcc28909d57d6e265f88c6b9eba52096c (diff)
qmlls: insert semicolons behind dots as completion workaround
IDE's usually ask completion requests whenever users starts typing a '.'. For this patch, we assume that the user is currently writing code, that the '.' he just entered is followed by a newline and that the line below the cursor is unrelated, which should be a pretty realistic scenario of writing code. For example, imagine a user typing in following code: ``` x: root.<current cursor> SomeQualifiedModule.Item {} ``` The LSP client will request completion suggestions when the user types in `root.` and qmlls will be confused about the completion, as it will see a QmlObject binding in the Dom representation. Another example are JS statements, where the parser stops working when he sees ``` root.<current cursor> for (;;) {} ``` because it tries to construct the function call `root.for` and has troubles with the semicolons inside the function call argument. To go around this problem, insert semicolons when the symbol that requested the completion is a dot and when this dot is followed by a newline. In qqmlcompletionsupport, create a new Dom representation with the patched code, and keep it out the QQmlCodeModel to avoid bothering other modules, like the linting module, with the inserted "invisible" semicolon (invisible in the eyes of the user). Now, when the parser is run again on the patched code, it will see ``` root.; ``` and will happily create the correct field member expression using its recovery mode. One could also have inserted any identifier like '_dummyIdentifier' followed by a semicolon to get: ``` root._dummyIdentifier; ``` which would have worked too I believe. Add two tests, one in tst_qmlls_modules to test if the patched Dom can be used to provide the completions, and on in tst_qmlls_utils to test if the "normal" dom construction works on (hand)patched versions of the files. tst_qmlls_utils cannot test the patching itself as that happens in the qqmlcompletionsupport module. The afterDots.qml files are invalid and as such should be ignored by tst_qmlformat. Pick-to: 6.7 Fixes: QTBUG-119839 Change-Id: Ib914b78512894c423792588842d635d3d1467922 Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
-rw-r--r--src/qmlls/qqmlcompletionsupport.cpp90
-rw-r--r--src/qmlls/qqmlcompletionsupport_p.h1
-rw-r--r--tests/auto/qml/qmlformat/tst_qmlformat.cpp3
-rw-r--r--tests/auto/qmlls/modules/data/completions/bindingAfterDot.qml14
-rw-r--r--tests/auto/qmlls/modules/data/completions/defaultBindingAfterDot.qml14
-rw-r--r--tests/auto/qmlls/modules/tst_qmlls_modules.cpp34
-rw-r--r--tests/auto/qmlls/modules/tst_qmlls_modules.h2
-rw-r--r--tests/auto/qmlls/utils/data/completions/afterDots.qml6
-rw-r--r--tests/auto/qmlls/utils/tst_qmlls_utils.cpp18
9 files changed, 166 insertions, 16 deletions
diff --git a/src/qmlls/qqmlcompletionsupport.cpp b/src/qmlls/qqmlcompletionsupport.cpp
index 34392c9b3b..3666b8e062 100644
--- a/src/qmlls/qqmlcompletionsupport.cpp
+++ b/src/qmlls/qqmlcompletionsupport.cpp
@@ -84,26 +84,86 @@ void CompletionRequest::sendCompletions(QmlLsp::OpenDocumentSnapshot &doc)
m_response.sendResponse(res);
}
+static bool positionIsFollowedBySpaces(qsizetype position, const QString &code)
+{
+ if (position >= code.size())
+ return false;
+
+ auto newline =
+ std::find_if(std::next(code.cbegin(), position), code.cend(),
+ [](const QChar &c) { return c == u'\n' || c == u'\r' || !c.isSpace(); });
+
+ return newline == code.cend() || newline->isSpace();
+}
+
+/*!
+\internal
+
+\note Remove this method and all its usages once the new fault-tolerant parser from QTBUG-118053 is
+introduced!!!
+
+Tries to make the document valid for the parser, to be able to provide completions after dots.
+The created DomItem is not in the qqmlcodemodel which mean it cannot be seen and cannot bother
+other modules: it would be bad to have the linting module complain about code that was modified
+here, but cannot be seen by the user.
+*/
+DomItem CompletionRequest::patchInvalidFileForParser(const DomItem &file, qsizetype position) const
+{
+ // automatic semicolon insertion after dots, if there is nothing behind the dot!
+ if (position > 0 && code[position - 1] == u'.' && positionIsFollowedBySpaces(position, code)) {
+ qCWarning(QQmlLSCompletionLog)
+ << "Patching invalid document: adding a semicolon after '.' for "
+ << QString::fromUtf8(m_parameters.textDocument.uri);
+
+ const QString patchedCode =
+ code.first(position).append(u"_dummyIdentifier;").append(code.sliced(position));
+
+ // create a new (local) Dom only for the completions.
+ // This avoids weird behaviors, like the linting module complaining about the inserted
+ // semicolon that the user cannot see, for example.
+ DomItem newCurrent = file.environment().makeCopy(DomItem::CopyOption::EnvConnected).item();
+
+ DomItem result;
+ DomCreationOptions options;
+ options.setFlag(DomCreationOption::WithScriptExpressions);
+ options.setFlag(DomCreationOption::WithSemanticAnalysis);
+ options.setFlag(DomCreationOption::WithRecovery);
+ newCurrent.loadFile(FileToLoad::fromMemory(newCurrent.ownerAs<DomEnvironment>(),
+ file.canonicalFilePath(), patchedCode, options),
+ [&result](Path, const DomItem &, const DomItem &newValue) {
+ result = newValue.fileObject();
+ },
+ {});
+ newCurrent.loadPendingDependencies();
+ return result;
+ }
+
+ qCWarning(QQmlLSCompletionLog) << "No valid document for completions for "
+ << QString::fromUtf8(m_parameters.textDocument.uri);
+
+ return file;
+}
+
QList<CompletionItem> CompletionRequest::completions(QmlLsp::OpenDocumentSnapshot &doc) const
{
QList<CompletionItem> res;
- if (!doc.validDoc) {
- qCWarning(QQmlLSCompletionLog) << "No valid document for completions for "
- << QString::fromUtf8(m_parameters.textDocument.uri);
- // try to add some import and global completions?
- return res;
- }
- if (!doc.docVersion || *doc.docVersion < m_minVersion) {
- qCWarning(QQmlLSCompletionLog) << "sendCompletions on older doc version";
- } else if (!doc.validDocVersion || *doc.validDocVersion < m_minVersion) {
- qCWarning(QQmlLSCompletionLog) << "using outdated valid doc, position might be incorrect";
- }
- DomItem file = doc.validDoc.fileObject(QQmlJS::Dom::GoTo::MostLikely);
+
+
+ const qsizetype pos = QQmlLSUtils::textOffsetFrom(code, m_parameters.position.line,
+ m_parameters.position.character);
+
+ const bool useValidDoc =
+ doc.validDoc && doc.validDocVersion && *doc.validDocVersion >= m_minVersion;
+
+ const DomItem file = useValidDoc
+ ? doc.validDoc.fileObject(QQmlJS::Dom::GoTo::MostLikely)
+ : patchInvalidFileForParser(doc.doc.fileObject(QQmlJS::Dom::GoTo::MostLikely), pos);
+
// clear reference cache to resolve latest versions (use a local env instead?)
if (std::shared_ptr<DomEnvironment> envPtr = file.environment().ownerAs<DomEnvironment>())
envPtr->clearReferenceCache();
- qsizetype pos = QQmlLSUtils::textOffsetFrom(code, m_parameters.position.line,
- m_parameters.position.character);
+
+
CompletionContextStrings ctx(code, pos);
auto itemsFound = QQmlLSUtils::itemsFromTextLocation(file, m_parameters.position.line,
m_parameters.position.character
@@ -118,6 +178,8 @@ QList<CompletionItem> CompletionRequest::completions(QmlLsp::OpenDocumentSnapsho
DomItem currentItem;
if (!itemsFound.isEmpty())
currentItem = itemsFound.first().domItem;
+ else
+ qCDebug(QQmlLSCompletionLog) << "No items found for completions at" << urlAndPos();
qCDebug(QQmlLSCompletionLog) << "Completion at " << urlAndPos() << " "
<< m_parameters.position.line << ":"
<< m_parameters.position.character << "offset:" << pos
diff --git a/src/qmlls/qqmlcompletionsupport_p.h b/src/qmlls/qqmlcompletionsupport_p.h
index acbbd2e263..f2bf0e0d62 100644
--- a/src/qmlls/qqmlcompletionsupport_p.h
+++ b/src/qmlls/qqmlcompletionsupport_p.h
@@ -37,6 +37,7 @@ struct CompletionRequest
void sendCompletions(QmlLsp::OpenDocumentSnapshot &);
QString urlAndPos() const;
QList<QLspSpecification::CompletionItem> completions(QmlLsp::OpenDocumentSnapshot &doc) const;
+ DomItem patchInvalidFileForParser(const DomItem& file, qsizetype position) const;
};
class QmlCompletionSupport : public QQmlBaseModule<CompletionRequest>
diff --git a/tests/auto/qml/qmlformat/tst_qmlformat.cpp b/tests/auto/qml/qmlformat/tst_qmlformat.cpp
index b2a44a18c5..f5486362d0 100644
--- a/tests/auto/qml/qmlformat/tst_qmlformat.cpp
+++ b/tests/auto/qml/qmlformat/tst_qmlformat.cpp
@@ -139,6 +139,9 @@ void TestQmlformat::initTestCase()
m_invalidFiles << "tests/auto/qmlls/utils/data/completions/missingRHS.parserfail.qml";
m_invalidFiles << "tests/auto/qmlls/utils/data/completions/attachedPropertyMissingRHS.qml";
m_invalidFiles << "tests/auto/qmlls/utils/data/completions/groupedPropertyMissingRHS.qml";
+ m_invalidFiles << "tests/auto/qmlls/utils/data/completions/afterDots.qml";
+ m_invalidFiles << "tests/auto/qmlls/modules/data/completions/bindingAfterDot.qml";
+ m_invalidFiles << "tests/auto/qmlls/modules/data/completions/defaultBindingAfterDot.qml";
// Files that get changed:
// rewrite of import "bla/bla/.." to import "bla"
diff --git a/tests/auto/qmlls/modules/data/completions/bindingAfterDot.qml b/tests/auto/qmlls/modules/data/completions/bindingAfterDot.qml
new file mode 100644
index 0000000000..e726f7e71e
--- /dev/null
+++ b/tests/auto/qmlls/modules/data/completions/bindingAfterDot.qml
@@ -0,0 +1,14 @@
+// Copyright (C) 2023 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+
+import QtQuick
+
+QtObject {
+ id: root
+ property int good
+ property var i: Item {
+ property int bad
+ property int myP: root.
+ Item { }
+ }
+}
diff --git a/tests/auto/qmlls/modules/data/completions/defaultBindingAfterDot.qml b/tests/auto/qmlls/modules/data/completions/defaultBindingAfterDot.qml
new file mode 100644
index 0000000000..74c3214286
--- /dev/null
+++ b/tests/auto/qmlls/modules/data/completions/defaultBindingAfterDot.qml
@@ -0,0 +1,14 @@
+// Copyright (C) 2024 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+
+import QtQuick
+
+QtObject {
+ id: root
+ property int good
+ property var i: Item {
+ property int bad
+ property int myP2: root.
+ bad: 43
+ }
+}
diff --git a/tests/auto/qmlls/modules/tst_qmlls_modules.cpp b/tests/auto/qmlls/modules/tst_qmlls_modules.cpp
index 756fa503d7..72865fc1be 100644
--- a/tests/auto/qmlls/modules/tst_qmlls_modules.cpp
+++ b/tests/auto/qmlls/modules/tst_qmlls_modules.cpp
@@ -39,9 +39,10 @@ tst_qmlls_modules::tst_qmlls_modules() : QQmlDataTest(QT_QMLTEST_DATADIR)
m_qmllsPath = qEnvironmentVariable("QMLLS", m_qmllsPath);
// qputenv("QT_LOGGING_RULES",
// "qt.languageserver.codemodel.debug=true;qt.languageserver.codemodel.warning=true"); // helps
+ qputenv("QT_LOGGING_RULES", "*.debug=true;*.warning=true");
// when using EditingRecorder
m_server.setProgram(m_qmllsPath);
- // m_server.setArguments(QStringList() << u"-v"_s << u"-w"_s << u"8"_s);
+ // m_server.setArguments(QStringList() << u"-v"_s << u"-w"_s << u"7"_s);
}
void tst_qmlls_modules::init()
@@ -194,7 +195,6 @@ void tst_qmlls_modules::checkCompletions(const QByteArray &uri, int lineNr, int
} else if (c.kind->toInt() == int(CompletionItemKind::Property)) {
QVERIFY2(!propertiesTracker.hasSeen(c.label),
"Duplicate property: " + c.label);
- QVERIFY(c.insertText->isEmpty());
}
labels << c.label;
}
@@ -390,6 +390,36 @@ void tst_qmlls_modules::buildDir()
QStringList({ u"QtQuick"_s, u"vector4d"_s })));
}
+void tst_qmlls_modules::automaticSemicolonInsertionForCompletions_data()
+{
+ QTest::addColumn<QString>("filePath");
+ QTest::addColumn<int>("row");
+ QTest::addColumn<int>("column");
+
+ QTest::addRow("bindingAfterDot") << u"completions/bindingAfterDot.qml"_s << 11 << 32;
+ QTest::addRow("defaultBindingAfterDot")
+ << u"completions/defaultBindingAfterDot.qml"_s << 11 << 32;
+}
+
+void tst_qmlls_modules::automaticSemicolonInsertionForCompletions()
+{
+ ignoreDiagnostics();
+ QFETCH(QString, filePath);
+ QFETCH(int, row);
+ QFETCH(int, column);
+ row--;
+ column--;
+ const auto uri = openFile(filePath);
+ QVERIFY(uri);
+
+ QTEST_CHECKED(checkCompletions(
+ *uri, row, column,
+ ExpectedCompletions({
+ { u"good"_s, CompletionItemKind::Property },
+ }),
+ QStringList({ u"bad"_s, u"BuildDirType"_s, u"QtQuick"_s, u"width"_s, u"vector4d"_s })));
+}
+
void tst_qmlls_modules::goToTypeDefinition_data()
{
QTest::addColumn<QString>("filePath");
diff --git a/tests/auto/qmlls/modules/tst_qmlls_modules.h b/tests/auto/qmlls/modules/tst_qmlls_modules.h
index b3c3602959..df8c8f4eff 100644
--- a/tests/auto/qmlls/modules/tst_qmlls_modules.h
+++ b/tests/auto/qmlls/modules/tst_qmlls_modules.h
@@ -62,6 +62,8 @@ private slots:
void qmldirImports();
void quickFixes_data();
void quickFixes();
+ void automaticSemicolonInsertionForCompletions_data();
+ void automaticSemicolonInsertionForCompletions();
private:
QProcess m_server;
diff --git a/tests/auto/qmlls/utils/data/completions/afterDots.qml b/tests/auto/qmlls/utils/data/completions/afterDots.qml
index e726f7e71e..8d46864530 100644
--- a/tests/auto/qmlls/utils/data/completions/afterDots.qml
+++ b/tests/auto/qmlls/utils/data/completions/afterDots.qml
@@ -10,5 +10,11 @@ QtObject {
property int bad
property int myP: root.
Item { }
+ property int myP2: root.;
+ bad: 43
+ function f() {
+ root.;
+ for (;;) {}
+ }
}
}
diff --git a/tests/auto/qmlls/utils/tst_qmlls_utils.cpp b/tests/auto/qmlls/utils/tst_qmlls_utils.cpp
index 5569b3c715..81dc2a62cc 100644
--- a/tests/auto/qmlls/utils/tst_qmlls_utils.cpp
+++ b/tests/auto/qmlls/utils/tst_qmlls_utils.cpp
@@ -3195,6 +3195,24 @@ void tst_qmlls_utils::completions_data()
<< QStringList{ u"bad"_s, u"QtQuick"_s, u"vector4d"_s,
attachedTypeName, u"Rectangle"_s, u"onCompleted"_s };
+ QTest::newRow("dotFollowedByBinding")
+ << testFile("completions/afterDots.qml") << 13 << 32
+ << ExpectedCompletions({
+ { u"good"_s, CompletionItemKind::Property },
+ })
+ << QStringList{ u"bad"_s, u"QtQuick"_s, u"vector4d"_s,
+ attachedTypeName, u"Rectangle"_s, u"onCompleted"_s };
+
+ QTest::newRow("dotFollowedByForStatement")
+ << testFile("completions/afterDots.qml") << 16 << 17
+ << ExpectedCompletions({
+ { u"good"_s, CompletionItemKind::Property },
+ })
+ << QStringList{
+ u"bad"_s, u"QtQuick"_s, u"vector4d"_s, attachedTypeName,
+ u"Rectangle"_s, u"onCompleted"_s, forStatementCompletion
+ };
+
QTest::newRow("qualifiedTypeCompletionWithoutQualifier")
<< testFile("completions/qualifiedTypesCompletion.qml") << 9 << 5
<< ExpectedCompletions({