qmlls: show fix suggestions and enable quick fixes

Display the fix suggestions from qmllint in qmlls instead of just
showing the warning.
For example,
'Unqualified access: xxx is accessible in this scope because you are
handling a signal at 18:10. Use a function instead.'
sounds so much more helpful than the previous 'Unqualified access'.

Add an ending '.' in QQmlJSFixSuggestion's messages where missing.

Enable quick-fixes in VS Code and QtC by labeling quickfixes as
quickfixes. Everything that is not labelled a quickfix might need a
special language client implementation to work.
Also, add a test for quickfixes: it seems someone broke the quick
fix functionality in qmlls by mistake by relabeling a quickfix to
'refactor.rewrite'.

Task-number: QTBUG-115213
Change-Id: I350e23901b97d16a60bb39fdb4ab566b0d7fbdfb
Reviewed-by: Semih Yavuz <semih.yavuz@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

src/qmlcompiler/qqmljsimportvisitor.cpp

@@ -872,12 +872,12 @@ void QQmlJSImportVisitor::checkRequiredProperties()
                             if (requiredScope != scope) {
                                 if (!prevRequiredScope.isNull()) {
                                     auto sourceScope = prevRequiredScope->baseType();
-                                    suggestion = QQmlJSFixSuggestion {
-                                        "%1:%2:%3: Property marked as required in %4"_L1
-                                            .arg(sourceScope->filePath())
-                                            .arg(sourceScope->sourceLocation().startLine)
-                                            .arg(sourceScope->sourceLocation().startColumn)
-                                            .arg(requiredScopeName),
+                                    suggestion = QQmlJSFixSuggestion{
+                                        "%1:%2:%3: Property marked as required in %4."_L1
+                                                .arg(sourceScope->filePath())
+                                                .arg(sourceScope->sourceLocation().startLine)
+                                                .arg(sourceScope->sourceLocation().startColumn)
+                                                .arg(requiredScopeName),
                                         sourceScope->sourceLocation()
                                     };
                                     suggestion->setFilename(sourceScope->filePath());
@@ -1003,12 +1003,12 @@ void QQmlJSImportVisitor::checkSignal(
             const qsizetype newLength = m_logger->code().indexOf(u'\n', location.end())
                     - location.offset;
 
-            fix = QQmlJSFixSuggestion {
-                    "Implicitly defining %1 as signal handler in Connections is deprecated. "
-                    "Create a function instead"_L1.arg(handlerName),
-                    QQmlJS::SourceLocation(location.offset, newLength, location.startLine,
-                                           location.startColumn),
-                    "function %1(%2) { ... }"_L1.arg(handlerName, handlerParameters.join(u", "))
+            fix = QQmlJSFixSuggestion{
+                "Implicitly defining %1 as signal handler in Connections is deprecated. "
+                "Create a function instead."_L1.arg(handlerName),
+                QQmlJS::SourceLocation(location.offset, newLength, location.startLine,
+                                       location.startColumn),
+                "function %1(%2) { ... }"_L1.arg(handlerName, handlerParameters.join(u", "))
             };
         }
 
@@ -1411,11 +1411,8 @@ bool QQmlJSImportVisitor::visit(QQmlJS::AST::StringLiteral *sl)
             templateString += c;
         }
 
-        QQmlJSFixSuggestion suggestion = {
-            "Use a template literal instead"_L1,
-            sl->literalToken,
-            u"`" % templateString % u"`"
-        };
+        QQmlJSFixSuggestion suggestion = { "Use a template literal instead."_L1, sl->literalToken,
+                                           u"`" % templateString % u"`" };
         suggestion.setAutoApplicable();
         m_logger->log(QStringLiteral("String contains unescaped line terminator which is "
                                      "deprecated."),

src/qmlcompiler/qqmljstypepropagator.cpp

@@ -381,11 +381,11 @@ void QQmlJSTypePropagator::handleUnqualifiedAccess(const QString &name, bool isM
 
                 QQmlJS::SourceLocation fixLocation = location;
                 fixLocation.length = 0;
-                suggestion = QQmlJSFixSuggestion {
-                    name + " is a member of a parent element.\n      You can qualify the access "
-                           "with its id to avoid this warning:\n"_L1,
-                    fixLocation,
-                    (id.isEmpty() ? u"<id>."_s : (id + u'.'))
+                suggestion = QQmlJSFixSuggestion{
+                    name
+                            + " is a member of a parent element.\n      You can qualify the access "
+                              "with its id to avoid this warning.\n"_L1,
+                    fixLocation, (id.isEmpty() ? u"<id>."_s : (id + u'.'))
                 };
 
                 if (id.isEmpty())

src/qmlls/qqmllintsuggestions.cpp

@@ -80,7 +80,8 @@ static void codeActionHandler(
         edit.documentChanges = edits;
 
         CodeAction action;
-        action.kind = u"refactor.rewrite"_s.toUtf8();
+        // VS Code and QtC ignore everything that is not a 'quickfix'.
+        action.kind = u"quickfix"_s.toUtf8();
         action.edit = edit;
         action.title = message.toUtf8();
 
@@ -165,7 +166,16 @@ static Diagnostic messageToDiagnostic_helper(AdvanceFunc advancePositionPastLoca
                                     position);
     }
     range.end = position;
-    diagnostic.message = message.message.toUtf8();
+    if (message.fixSuggestion && !message.fixSuggestion->fixDescription().isEmpty()) {
+        diagnostic.message = QString(message.message)
+                                     .append(u": "_s)
+                                     .append(message.fixSuggestion->fixDescription())
+                                     .simplified()
+                                     .toUtf8();
+    } else {
+        diagnostic.message = message.message.toUtf8();
+    }
+
     diagnostic.source = QByteArray("qmllint");
 
     auto suggestion = message.fixSuggestion;

tests/auto/qmlls/modules/data/quickfixes/INeedAQuickFix.qml

@@ -0,0 +1,19 @@
+import QtQuick
+
+Item {
+    id: hello
+
+    signal s(xxx: string)
+    property int i: 42
+
+    // fix me! add '(xxx) =>' in front of console.log()
+    onS: console.log(xxx)
+
+    Item {
+        // fix me! prepend 'hello.' to 'i'!
+        function f() {
+            return i
+        }
+    }
+
+}

tests/auto/qmlls/modules/tst_qmlls_modules.cpp

@@ -3,9 +3,11 @@
 
 #include "tst_qmlls_modules.h"
 #include "QtQmlLS/private/qqmllsutils_p.h"
+#include <algorithm>
 #include <memory>
 #include <optional>
 #include <string>
+#include <tuple>
 #include <type_traits>
 #include <variant>
 
@@ -1397,6 +1399,130 @@ void tst_qmlls_modules::qmldirImportsFromSource()
     QTRY_VERIFY_WITH_TIMEOUT(diagnosticOk, 5000);
 }
 
+void tst_qmlls_modules::quickFixes_data()
+{
+    QTest::addColumn<QString>("filePath");
+    QTest::addColumn<CodeActionParams>("codeActionParams");
+    QTest::addColumn<int>("diagnosticIndex");
+    QTest::addColumn<Range>("replacementRange");
+    QTest::addColumn<QString>("replacementText");
+
+    const QString filePath = u"quickfixes/INeedAQuickFix.qml"_s;
+
+    QString fileContent;
+    {
+        QFile file(testFile(filePath));
+        QVERIFY(file.open(QFile::Text | QFile::ReadOnly));
+        fileContent = file.readAll();
+    }
+
+    CodeActionParams firstCodeAction;
+    firstCodeAction.range = rangeFrom(fileContent, 10, 23, 1);
+    firstCodeAction.textDocument.uri = testFileUrl(filePath).toEncoded();
+
+    const Range firstRange = rangeFrom(fileContent, 10, 10, 0);
+    const QString firstReplacement = u"(xxx) => "_s;
+
+    QTest::addRow("injectedParameters")
+            << filePath << firstCodeAction << 0 << firstRange << firstReplacement;
+
+    CodeActionParams secondCodeAction;
+    secondCodeAction.textDocument.uri = testFileUrl(filePath).toEncoded();
+    secondCodeAction.range = rangeFrom(fileContent, 15, 20, 1);
+
+    const Range secondRange = rangeFrom(fileContent, 15, 20, 0);
+    const QString secondReplacement = u"hello."_s;
+
+    QTest::addRow("parentProperty")
+            << filePath << secondCodeAction << 1 << secondRange << secondReplacement;
+}
+
+std::tuple<int, int, int, int> rangeAsTuple(const Range &range)
+{
+    return std::make_tuple(range.start.line, range.start.character, range.end.line,
+                           range.end.character);
+}
+
+void tst_qmlls_modules::quickFixes()
+{
+    QFETCH(QString, filePath);
+    QFETCH(CodeActionParams, codeActionParams);
+    // The index of the diagnostic that the quickFix belongs to.
+    // diagnostics are sorted by their range (= text position in the current file).
+    QFETCH(int, diagnosticIndex);
+    QFETCH(Range, replacementRange);
+    QFETCH(QString, replacementText);
+
+    const auto uri = openFile(filePath);
+    QVERIFY(uri);
+
+    bool diagnosticOk = false;
+    QList<Diagnostic> diagnostics;
+
+    // run first the diagnostic that proposes a quickfix
+    m_protocol->registerPublishDiagnosticsNotificationHandler(
+            [&diagnosticOk, &uri, &diagnostics,
+             &diagnosticIndex](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_GE(p.diagnostics.size(), diagnosticIndex);
+
+                QList<Diagnostic> partially_sorted{ p.diagnostics };
+                std::nth_element(partially_sorted.begin(),
+                                 std::next(partially_sorted.begin(), diagnosticIndex),
+                                 partially_sorted.end(),
+                                 [](const Diagnostic &a, const Diagnostic &b) {
+                                     return rangeAsTuple(a.range) < rangeAsTuple(b.range);
+                                 });
+                diagnostics.append(partially_sorted[diagnosticIndex]);
+
+                diagnosticOk = true;
+            });
+
+    QTRY_VERIFY_WITH_TIMEOUT(diagnosticOk, 5000);
+
+    codeActionParams.context.diagnostics = diagnostics;
+
+    using InnerT = QList<std::variant<Command, CodeAction>>;
+    using T = std::variant<InnerT, std::nullptr_t>;
+
+    bool codeActionOk = false;
+
+    // request a quickfix with the obtained diagnostic
+    m_protocol->requestCodeAction(codeActionParams, [&](const T &result) {
+        QVERIFY(std::holds_alternative<InnerT>(result));
+        InnerT inner = std::get<InnerT>(result);
+        QCOMPARE(inner.size(), 1);
+        QVERIFY(std::holds_alternative<CodeAction>(inner.front()));
+        CodeAction codeAction = std::get<CodeAction>(inner.front());
+
+        QCOMPARE(codeAction.kind, "quickfix"); // everything else is ignored by QtC, VS Code, ...
+
+        QVERIFY(codeAction.edit);
+        QVERIFY(codeAction.edit->documentChanges);
+        QVERIFY(std::holds_alternative<QList<TextDocumentEdit>>(*codeAction.edit->documentChanges));
+        auto edits = std::get<QList<TextDocumentEdit>>(*codeAction.edit->documentChanges);
+        QCOMPARE(edits.size(), 1);
+        QCOMPARE(edits.front().edits.size(), 1);
+        QVERIFY(std::holds_alternative<TextEdit>(edits.front().edits.front()));
+        auto textEdit = std::get<TextEdit>(edits.front().edits.front());
+
+        // make sure that the quick fix does something
+        QCOMPARE(textEdit.newText, replacementText);
+        QCOMPARE(rangeAsTuple(textEdit.range), rangeAsTuple(replacementRange));
+
+        codeActionOk = true;
+    });
+
+    QTRY_VERIFY_WITH_TIMEOUT(codeActionOk, 5000);
+}
+
 QTEST_MAIN(tst_qmlls_modules)
 
 #include <tst_qmlls_modules.moc>

tests/auto/qmlls/modules/tst_qmlls_modules.h

@@ -62,6 +62,8 @@ private slots:
     void rangeFormatting();
     void qmldirImportsFromBuild();
     void qmldirImportsFromSource();
+    void quickFixes_data();
+    void quickFixes();
 
 private:
     QProcess m_server;

tests/auto/qmlls/qmlls/tst_qmlls.cpp

@@ -214,8 +214,7 @@ void tst_Qmlls::didOpenTextDocument()
 
                     QString title = QString::fromUtf8(action.title);
                     QVERIFY(action.kind.has_value());
-                    QCOMPARE(QString::fromUtf8(action.kind.value()),
-                             QLatin1StringView("refactor.rewrite"));
+                    QCOMPARE(QString::fromUtf8(action.kind.value()), QLatin1StringView("quickfix"));
                     QVERIFY(action.edit.has_value());
                     WorkspaceEdit edit = action.edit.value();