qmlls: disable features on invalid QML documents

Some features like "go to definition" or "find usages" or "rename
symbols" do not work like expected on invalid documents, so disable them
instead of showing outdated or incorrect information to the user.

Before this commit, these features would use the last known valid
document to perform their search, but that leads to weird behavior:
wrong locations are highlighted or jumped to, and rename might ask to
replace text at positions that do not exist.

An easy way to reproduce this is just to add a newline inside a keyword,
e.g. "property int helloProperty" -> "prop\nperty int helloProperty",
because then all the findings under the changed line will be off by one
line. It looks very weird and unprofessional and its not clear at all
for the user what causes this.

Therefore, when trying to find or rename usages, going to definitions or
type definition, print a warning in the IDE that these functionalities
do not work for invalid QML documents, and that they have to try again
after removing all the invalid QML code.

For reference: clangd seems to also disable its code navigation features
on invalid C++ code.

As a drive-by, to avoid code duplication, add a ResponseScopeGuard class
that sends a response at the end of the scope. It will also help in the
future with error-handling using its setErrorFrom()-methods that checks
a result for errors.

Change-Id: I460f3891df17b79bf49c9b6a5f5798165250a16b
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

6 files changed, 128 insertions, 54 deletions

diff --git a/src/qmlls/qqmlbasemodule_p.h b/src/qmlls/qqmlbasemodule_p.h
index ae15c3fb59..47596914cb 100644
--- a/src/qmlls/qqmlbasemodule_p.h
+++ b/src/qmlls/qqmlbasemodule_p.h
@@ -18,8 +18,10 @@
 #include "qlanguageserver_p.h"
 #include "qqmlcodemodel_p.h"
 #include "qqmllsutils_p.h"
+#include <QtQmlDom/private/qqmldom_utils_p.h>
 
 #include <QObject>
+#include <type_traits>
 #include <unordered_map>
 
 template<typename ParametersT, typename ResponseT>
@@ -41,6 +43,72 @@ struct BaseRequest
     bool fillFrom(QmlLsp::OpenDocument doc, const Parameters &params, Response &&response);
 };
 
+/*!
+\internal
+\brief This class sends a result or an error when going out of scope.
+
+It has a helper method \c setErrorFrom that sets an error from variant and optionals.
+*/
+
+template<typename Result, typename ResponseCallback>
+struct ResponseScopeGuard
+{
+    Q_DISABLE_COPY_MOVE(ResponseScopeGuard)
+
+    std::variant<Result *, QQmlLSUtilsErrorMessage> m_response;
+    ResponseCallback &m_callback;
+
+    ResponseScopeGuard(Result &results, ResponseCallback &callback)
+        : m_response(&results), m_callback(callback)
+    {
+    }
+
+    // note: discards the current result or error message, if there is any
+    void setError(const QQmlLSUtilsErrorMessage &error) { m_response = error; }
+
+    template<typename... T>
+    bool setErrorFrom(const std::variant<T...> &variant)
+    {
+        static_assert(std::disjunction_v<std::is_same<T, QQmlLSUtilsErrorMessage>...>,
+                      "ResponseScopeGuard::setErrorFrom was passed a variant that never contains"
+                      " an error message.");
+        if (auto x = std::get_if<QQmlLSUtilsErrorMessage>(&variant)) {
+            setError(*x);
+            return true;
+        }
+        return false;
+    }
+
+    /*!
+        \internal
+        Note: use it as follows:
+        \badcode
+        if (scopeGuard.setErrorFrom(xxx)) {
+            // do early exit
+        }
+        // xxx was not an error, continue
+        \endcode
+    */
+    bool setErrorFrom(const std::optional<QQmlLSUtilsErrorMessage> &error)
+    {
+        if (error) {
+            setError(*error);
+            return true;
+        }
+        return false;
+    }
+
+    ~ResponseScopeGuard()
+    {
+        std::visit(qOverloadedVisitor{ [this](Result *result) { m_callback.sendResponse(*result); },
+                                       [this](const QQmlLSUtilsErrorMessage &error) {
+                                           m_callback.sendErrorResponse(error.code,
+                                                                        error.message.toUtf8());
+                                       } },
+                   m_response);
+    }
+};
+
 template<typename RequestType>
 struct QQmlBaseModule : public QLanguageServerModule
 {
@@ -57,7 +125,8 @@ struct QQmlBaseModule : public QLanguageServerModule
     decltype(auto) getRequestHandler();
     // processes a request in a different thread.
     virtual void process(RequestPointerArgument toBeProcessed) = 0;
-    std::optional<QList<QQmlLSUtilsItemLocation>> itemsForRequest(const RequestPointer &request);
+    std::variant<QList<QQmlLSUtilsItemLocation>, QQmlLSUtilsErrorMessage>
+    itemsForRequest(const RequestPointer &request);
 
 public Q_SLOTS:
     void updatedSnapshot(const QByteArray &uri);
@@ -159,29 +228,38 @@ void QQmlBaseModule<RequestType>::updatedSnapshot(const QByteArray &url)
 }
 
 template<typename RequestType>
-std::optional<QList<QQmlLSUtilsItemLocation>>
+std::variant<QList<QQmlLSUtilsItemLocation>, QQmlLSUtilsErrorMessage>
 QQmlBaseModule<RequestType>::itemsForRequest(const RequestPointer &request)
 {
 
     QmlLsp::OpenDocument doc = m_codeModel->openDocumentByUrl(
             QQmlLSUtils::lspUriToQmlUrl(request->m_parameters.textDocument.uri));
 
+    if (!doc.snapshot.validDocVersion || doc.snapshot.validDocVersion != doc.snapshot.docVersion) {
+        return QQmlLSUtilsErrorMessage{ 0,
+                                        u"Cannot proceed: current QML document is invalid! Fix"
+                                        u" all the errors in your QML code and try again."_s };
+    }
+
     QQmlJS::Dom::DomItem file = doc.snapshot.validDoc.fileObject(QQmlJS::Dom::GoTo::MostLikely);
     // clear reference cache to resolve latest versions (use a local env instead?)
     if (auto envPtr = file.environment().ownerAs<QQmlJS::Dom::DomEnvironment>())
         envPtr->clearReferenceCache();
     if (!file) {
-        qWarning() << u"Could not find file in Dom Environment from Codemodel :"_s
-                   << doc.snapshot.doc.toString();
-        return {};
+        return QQmlLSUtilsErrorMessage{
+            0,
+            u"Could not find file %1 in project."_s.arg(doc.snapshot.doc.toString()),
+        };
     }
 
     auto itemsFound = QQmlLSUtils::itemsFromTextLocation(file, request->m_parameters.position.line,
                                                          request->m_parameters.position.character);
 
     if (itemsFound.isEmpty()) {
-        qWarning() << u"Could not find any items at given text location."_s;
-        return {};
+        return QQmlLSUtilsErrorMessage{
+            0,
+            u"Could not find any items at given text location."_s,
+        };
     }
     return itemsFound;
 }
diff --git a/src/qmlls/qqmlfindusagessupport.cpp b/src/qmlls/qqmlfindusagessupport.cpp
index 270df92c53..8a3dca7e3a 100644
--- a/src/qmlls/qqmlfindusagessupport.cpp
+++ b/src/qmlls/qqmlfindusagessupport.cpp
@@ -6,6 +6,7 @@
 #include <QtLanguageServer/private/qlanguageserverspectypes_p.h>
 #include <QtQmlDom/private/qqmldomexternalitems_p.h>
 #include <QtQmlDom/private/qqmldomtop_p.h>
+#include <variant>
 
 QT_BEGIN_NAMESPACE
 
@@ -37,17 +38,17 @@ void QQmlFindUsagesSupport::registerHandlers(QLanguageServer *, QLanguageServerP
 void QQmlFindUsagesSupport::process(QQmlFindUsagesSupport::RequestPointerArgument request)
 {
     QList<QLspSpecification::Location> results;
-    QScopeGuard onExit([&results, &request]() { request->m_response.sendResponse(results); });
+    ResponseScopeGuard guard(results, request->m_response);
 
     auto itemsFound = itemsForRequest(request);
-    if (!itemsFound) {
+    if (guard.setErrorFrom(itemsFound))
         return;
-    }
 
-    auto usages = QQmlLSUtils::findUsagesOf(itemsFound->front().domItem);
+    QQmlLSUtilsItemLocation &front = std::get<QList<QQmlLSUtilsItemLocation>>(itemsFound).front();
+
+    auto usages = QQmlLSUtils::findUsagesOf(front.domItem);
 
-    QQmlJS::Dom::DomItem files =
-            itemsFound->front().domItem.top().field(QQmlJS::Dom::Fields::qmlFileWithPath);
+    QQmlJS::Dom::DomItem files = front.domItem.top().field(QQmlJS::Dom::Fields::qmlFileWithPath);
 
     QHash<QString, QString> codeCache;
 
diff --git a/src/qmlls/qqmlformatting.cpp b/src/qmlls/qqmlformatting.cpp
index 435175f6bb..cafc2dba57 100644
--- a/src/qmlls/qqmlformatting.cpp
+++ b/src/qmlls/qqmlformatting.cpp
@@ -38,17 +38,21 @@ void QQmlDocumentFormatting::setupCapabilities(
 
 void QQmlDocumentFormatting::process(RequestPointerArgument request)
 {
+    QList<QLspSpecification::TextEdit> result;
+    ResponseScopeGuard guard(result, request->m_response);
+
     using namespace QQmlJS::Dom;
     QmlLsp::OpenDocument doc = m_codeModel->openDocumentByUrl(
                 QQmlLSUtils::lspUriToQmlUrl(request->m_parameters.textDocument.uri));
 
     DomItem file = doc.snapshot.doc.fileObject(GoTo::MostLikely);
     if (!file) {
-        qWarning() << u"Could not find the file"_s << doc.snapshot.doc.toString();
+        guard.setError(QQmlLSUtilsErrorMessage{
+                0, u"Could not find the file %1"_s.arg(doc.snapshot.doc.canonicalFilePath()) });
         return;
     }
     if (!file.field(Fields::isValid).value().toBool(false)) {
-        qWarning() << u"Invalid document will not be formatted"_s;
+        guard.setError(QQmlLSUtilsErrorMessage{ 0, u"Cannot format invalid documents!"_s });
         return;
     }
     if (auto envPtr = file.environment().ownerAs<DomEnvironment>())
@@ -62,7 +66,8 @@ void QQmlDocumentFormatting::process(RequestPointerArgument request)
                     return true;
                 },
                 true);
-        qWarning().noquote() << "Failed to parse" << file;
+        guard.setError(QQmlLSUtilsErrorMessage{
+                0, u"Failed to parse %1"_s.arg(file.canonicalFilePath()) });
         return;
     }
 
@@ -75,7 +80,7 @@ void QQmlDocumentFormatting::process(RequestPointerArgument request)
     QLspSpecification::TextEdit formattedText;
     LineWriter lw([&formattedText](QStringView s) {formattedText.newText += s.toUtf8(); }, QString(), options);
     OutWriter ow(lw);
-    MutableDomItem result = file.writeOutForFile(ow, WriteOutCheck::None);
+    MutableDomItem formatted = file.writeOutForFile(ow, WriteOutCheck::None);
     ow.flush();
 
     const auto &code = qmlFile->code();
@@ -85,7 +90,7 @@ void QQmlDocumentFormatting::process(RequestPointerArgument request)
     formattedText.range = QLspSpecification::Range{ QLspSpecification::Position{ 0, 0 },
                                                     QLspSpecification::Position{ endLine + 1, 0 } };
 
-    request->m_response.sendResponse(QList<QLspSpecification::TextEdit>{ formattedText });
+    result.append(formattedText);
 }
 
 QT_END_NAMESPACE
diff --git a/src/qmlls/qqmlgotodefinitionsupport.cpp b/src/qmlls/qqmlgotodefinitionsupport.cpp
index 7d771f60d8..0a5579a057 100644
--- a/src/qmlls/qqmlgotodefinitionsupport.cpp
+++ b/src/qmlls/qqmlgotodefinitionsupport.cpp
@@ -40,24 +40,26 @@ void QmlGoToDefinitionSupport::registerHandlers(QLanguageServer *,
 void QmlGoToDefinitionSupport::process(RequestPointerArgument request)
 {
     QList<QLspSpecification::Location> results;
-    QScopeGuard onExit([&results, &request]() { request->m_response.sendResponse(results); });
+    ResponseScopeGuard guard(results, request->m_response);
 
     auto itemsFound = itemsForRequest(request);
-    if (!itemsFound) {
+
+    if (guard.setErrorFrom(itemsFound))
         return;
-    }
 
-    auto location = QQmlLSUtils::findDefinitionOf(itemsFound->front().domItem);
+    QQmlLSUtilsItemLocation &front = std::get<QList<QQmlLSUtilsItemLocation>>(itemsFound).front();
+
+    auto location = QQmlLSUtils::findDefinitionOf(front.domItem);
     if (!location)
         return;
 
     QLspSpecification::Location l;
     l.uri = QUrl::fromLocalFile(location->filename).toEncoded();
 
-    QQmlJS::Dom::DomItem file = itemsFound->front().domItem.goToFile(location->filename);
+    QQmlJS::Dom::DomItem file = front.domItem.goToFile(location->filename);
     auto fileOfBasePtr = file.ownerAs<QQmlJS::Dom::QmlFile>();
     if (!fileOfBasePtr) {
-        qWarning() << u"Could not obtain the file of the base."_s;
+        qDebug() << "Could not find file" << location->filename << "in the dom!";
         return;
     }
     const QString qmlCode = fileOfBasePtr->code();
diff --git a/src/qmlls/qqmlgototypedefinitionsupport.cpp b/src/qmlls/qqmlgototypedefinitionsupport.cpp
index b6f831af2f..7e2a123af2 100644
--- a/src/qmlls/qqmlgototypedefinitionsupport.cpp
+++ b/src/qmlls/qqmlgototypedefinitionsupport.cpp
@@ -40,29 +40,30 @@ void QmlGoToTypeDefinitionSupport::registerHandlers(QLanguageServer *,
 void QmlGoToTypeDefinitionSupport::process(RequestPointerArgument request)
 {
     QList<QLspSpecification::Location> results;
-    QScopeGuard onExit([&results, &request]() { request->m_response.sendResponse(results); });
+    ResponseScopeGuard guard(results, request->m_response);
 
     auto itemsFound = itemsForRequest(request);
-    if (!itemsFound) {
+    if (guard.setErrorFrom(itemsFound))
         return;
-    }
 
-    QQmlJS::Dom::DomItem base = QQmlLSUtils::findTypeDefinitionOf(itemsFound->first().domItem);
+    QQmlLSUtilsItemLocation &front = std::get<QList<QQmlLSUtilsItemLocation>>(itemsFound).front();
+
+    QQmlJS::Dom::DomItem base = QQmlLSUtils::findTypeDefinitionOf(front.domItem);
     if (base.domKind() == QQmlJS::Dom::DomKind::Empty) {
         qWarning() << u"Could not obtain the type definition, was the type correctly resolved?"_s
                    << u"\n Obtained type was:\n"_s << base.toString()
                    << u"\nbut selected item was:\n"
-                   << itemsFound->first().domItem.toString();
+                   << front.domItem.toString();
         return;
     }
 
     if (base.domKind() == QQmlJS::Dom::DomKind::Empty) {
-        qWarning() << u"Could not obtain the base from the item"_s;
+        qDebug() << u"Could not obtain the base from the item"_s;
         return;
     }
     auto locationInfo = QQmlJS::Dom::FileLocations::fileLocationsOf(base);
     if (!locationInfo) {
-        qWarning()
+        qDebug()
                 << u"Could not obtain the text location from the base item, was it correctly resolved?\nBase was "_s
                 << base.toString();
         return;
@@ -71,7 +72,7 @@ void QmlGoToTypeDefinitionSupport::process(RequestPointerArgument request)
     QQmlJS::Dom::DomItem fileOfBase = base.containingFile();
     auto fileOfBasePtr = fileOfBase.ownerAs<QQmlJS::Dom::QmlFile>();
     if (!fileOfBasePtr) {
-        qWarning() << u"Could not obtain the file of the base."_s;
+        qDebug() << u"Could not obtain the file of the base."_s;
         return;
     }
 
diff --git a/src/qmlls/qqmlrenamesymbolsupport.cpp b/src/qmlls/qqmlrenamesymbolsupport.cpp
index 89c6617fb6..a5d3ffa495 100644
--- a/src/qmlls/qqmlrenamesymbolsupport.cpp
+++ b/src/qmlls/qqmlrenamesymbolsupport.cpp
@@ -33,46 +33,33 @@ void QQmlRenameSymbolSupport::registerHandlers(QLanguageServer *, QLanguageServe
 void QQmlRenameSymbolSupport::process(QQmlRenameSymbolSupport::RequestPointerArgument request)
 {
     QLspSpecification::WorkspaceEdit result;
-    std::optional<QQmlLSUtilsErrorMessage> error;
-
-    QScopeGuard onExit([&error, &request, &result]() {
-        if (error.has_value()) {
-            request->m_response.sendErrorResponse(error->code, error->message.toUtf8());
-        } else {
-            request->m_response.sendResponse(result);
-        }
-    });
+    ResponseScopeGuard guard(result, request->m_response);
 
     auto itemsFound = itemsForRequest(request);
-    if (!itemsFound) {
+    if (guard.setErrorFrom(itemsFound))
         return;
-    }
+
+    QQmlLSUtilsItemLocation &front = std::get<QList<QQmlLSUtilsItemLocation>>(itemsFound).front();
 
     const QString newName = QString::fromUtf8(request->m_parameters.newName);
-    auto expressionType =
-            QQmlLSUtils::resolveExpressionType(itemsFound->front().domItem, ResolveOwnerType);
+    auto expressionType = QQmlLSUtils::resolveExpressionType(front.domItem, ResolveOwnerType);
 
     if (!expressionType) {
-        error = QQmlLSUtilsErrorMessage{ 0, u"Cannot rename the requested object"_s };
+        guard.setError(QQmlLSUtilsErrorMessage{ 0, u"Cannot rename the requested object"_s });
         return;
     }
 
-    if (auto renameImpossible = QQmlLSUtils::checkNameForRename(itemsFound->front().domItem,
-                                                                newName, expressionType)) {
-        error = renameImpossible;
+    if (guard.setErrorFrom(QQmlLSUtils::checkNameForRename(front.domItem, newName, expressionType)))
         return;
-    }
 
     QList<QLspSpecification::TextDocumentEdit> editsByFileForResult;
     // The QLspSpecification::WorkspaceEdit requires the changes to be grouped by files, so
     // collect them into editsByFileUris.
     QMap<QUrl, QList<QLspSpecification::TextEdit>> editsByFileUris;
 
-    auto renames =
-            QQmlLSUtils::renameUsagesOf(itemsFound->front().domItem, newName, expressionType);
+    auto renames = QQmlLSUtils::renameUsagesOf(front.domItem, newName, expressionType);
 
-    QQmlJS::Dom::DomItem files =
-            itemsFound->front().domItem.top().field(QQmlJS::Dom::Fields::qmlFileWithPath);
+    QQmlJS::Dom::DomItem files = front.domItem.top().field(QQmlJS::Dom::Fields::qmlFileWithPath);
 
     QHash<QString, QString> codeCache;