[clangd] Add semantic selection to ClangdLSPServer.

[usaxena95]

Summary

This adds semantic selection to the LSP Server.
Adds support for serialization of input request and the output reply.
Also adds regression tests for the feature.

Currently we do not support multi cursor.The LSP Server only accepts single position in the request as opposed to many position in the spec.

Spec:
https://github.com/microsoft/language-server-protocol/blob/dbaeumer/3.15/specification.md#textDocument_selectionRange

[ilya-biryukov]

A drive-by comment...

clang-tools-extra/clangd/ClangdLSPServer.cpp

1155

This should be return Reply(...), right?
Especially bad if positions is empty as we attempt to read the first element right away.

[usaxena95]

Fixed error message.

[usaxena95]

Addressed comments.

[usaxena95]

clang-tools-extra/clangd/ClangdLSPServer.cpp

1155

Aah. Yeah. Thanks.

[hokein]

I think we're missing the client/server capability implementation in this patch, we should include them.

clang-tools-extra/clangd/ClangdLSPServer.cpp

1151

maybe add an assert(!Params.positions.empty()). I think we should not run into this case.

clang-tools-extra/clangd/Protocol.h

1235

The data structures defined in Protocol are mirrored to LSP's. I think we should follow it for semantic selection as well (even though LSP use a wired structure, linked list).

so here, the structure should be like

struct SelectionRange {
    /**
     * The [range](#Range) of this selection range.
     */
    Range range;
    /**
     * The parent selection range containing this range. Therefore `parent.range` must contain `this.range`.
     */
    llvm::Optional<SelectionRange> parent;
}

And we'd need to define a render function in ClangLSPServer to covert the vector<Range> to the LSP's SelectionRange.

1242

does this operator get used in this patch?

clang-tools-extra/clangd/test/selection-range.test

5

could we create a smaller test? just void func() {} is enough, I'd keep the lit test as small as possible (since this feature has been test in the unittest)

[ilya-biryukov]

clang-tools-extra/clangd/ClangdLSPServer.cpp

1151

But Params comes to clangd over LSP, right?
That means assert can fire in case of bad inputs over LSP to clangd.
Bad inputs over LSP should never crash clangd.

[usaxena95]

Addressed comments:
Added client/server capabilities.
Made lit test smaller.

[usaxena95]

I am not sure adding client capability is useful here. I have not used the client capability for selectionRange anywhere and I think we can remove it.
WDYT ?

clang-tools-extra/clangd/ClangdLSPServer.cpp

1151

Yes this comes from the client and can be a bad input. We should just return error and not crash in such case.

clang-tools-extra/clangd/Protocol.h

1242

No. Removed it.

[hokein]

In D67720#1680460, @usaxena95 wrote:

I am not sure adding client capability is useful here. I have not used the client capability for selectionRange anywhere and I think we can remove it.
WDYT ?

The SelectionRangeClientCapabilities determines what should the LSP server send the client, if it is true, clangd should send SelectionRangeRegistrationOptions. But looking at the current specification, it doesn't seem to add too much value. I think we can just simplify return a bool for now (as you did in this patch).

clang-tools-extra/clangd/ClangdLSPServer.cpp

142

nit: use for range.

1151

but the code still doesn't handle the empty case?

clang-tools-extra/clangd/Protocol.h

1244

I think we can simplify the code further, using llvm::Optional<SelectionRange> should be enough, the parent is null for the outer-most range.

[usaxena95]

Addressed comments.

[usaxena95]

The SelectionRangeClientCapabilities determines what should the LSP server send the client, if it is true, clangd should send SelectionRangeRegistrationOptions.
But looking at the current specification, it doesn't seem to add too much value. I think we can just simplify return a bool for now (as you did in this patch).

Yeah. So should I remove the client capability since we do not use it and just return bool as now?

clang-tools-extra/clangd/ClangdLSPServer.cpp

1151

We do right ?
If the size != 1 then we just return an error.

clang-tools-extra/clangd/Protocol.h

1244

I don't think it is possible to do that since the type (SelectionRange) would be incomplete at that point. For example size of this class cannot be computed.

[hokein]

mostly good, a few nits.

clang-tools-extra/clangd/ClangdLSPServer.cpp

136

nit: remove the {}, in LLVM we prefer no {} for a single-statement body.

1151

yes, it is correct now. maybe I read the wrong snapshot before.

1168

does Reply({render(std::move(*Ranges))}); work?

clang-tools-extra/clangd/Protocol.h

422

I think we could drop this field now, it is not used, as we always return selectionRangeProvider:true.

1237

nit: please remove the markdown elements around the range in the comment.

1244

ah, good point, I see. We could just use std::unique_ptr<SelectionRange> parent;.

1247

Are these constructors needed?

[usaxena95]

Addressed comments.

clang-tools-extra/clangd/ClangdLSPServer.cpp

1168

List initialization of vector of move only types is painful 😃
Even this does not work: Reply(std::vector<SelectionRange>{render(std::move(*Ranges))}); because I think it vector tries to copy construct it because of initializer_lists

clang-tools-extra/clangd/Protocol.h

1247

We don't need them if we use just unique_ptr. They were needed before in Optional<unique_ptr>. Somehow it was not able to deduce this is a trivially-constructible + move only class.

[hokein]

thanks, looks good.

clang-tools-extra/clangd/Protocol.cpp

18

the header seems not used?

[usaxena95]

Removed ununsed header.

clang-tools-extra/clangd/ClangdLSPServer.h

void onHover(const TextDocumentPositionParams &,
                Callback<llvm::Optional<Hover>>);
   void onTypeHierarchy(const TypeHierarchyParams &,
                        Callback<llvm::Optional<TypeHierarchyItem>>);
   void onResolveTypeHierarchy(const ResolveTypeHierarchyItemParams &,
                               Callback<llvm::Optional<TypeHierarchyItem>>);
   void onChangeConfiguration(const DidChangeConfigurationParams &);
   void onSymbolInfo(const TextDocumentPositionParams &,
                     Callback<std::vector<SymbolDetails>>);
+  void onSelectionRange(const SelectionRangeParams &,
+                        Callback<std::vector<SelectionRange>>);
 
   std::vector<Fix> getFixes(StringRef File, const clangd::Diagnostic &D);
 
   /// Checks if completion request should be ignored. We need this due to the
   /// limitation of the LSP. Per LSP, a client sends requests for all "trigger
   /// character" we specify, but for '>' and ':' we need to check they actually
   /// produce '->' and '::', respectively.
   bool shouldRunCompletion(const CompletionParams &Params) const;

clang-tools-extra/clangd/ClangdLSPServer.cpp

 #include "Trace.h"
 #include "URI.h"
 #include "refactor/Tweak.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include <cstddef>
+#include <memory>
 #include <string>
+#include <vector>
 
 namespace clang {
 namespace clangd {
 namespace {
 /// Transforms a tweak into a code action that would apply it if executed.
 /// EXPECTS: T.prepare() was called and returned true.
 CodeAction toCodeAction(const ClangdServer::TweakRef &T, const URIForFile &File,
                         Range Selection) {
return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                    "File must be saved first: " +
                                        LastInvalidFile);
   return llvm::createStringError(
       llvm::inconvertibleErrorCode(),
       "Files must be saved first: " + LastInvalidFile + " (and " +
           llvm::to_string(InvalidFileCount - 1) + " others)");
 }
 
+// Converts a list of Ranges to a LinkedList of SelectionRange.
+SelectionRange render(const std::vector<Range> &Ranges) {
+  if (Ranges.empty())
+    return {};
+  SelectionRange Result;
+  Result.range = Ranges[0];
+  auto *Next = &Result.parent;
+  for (const auto &R : llvm::make_range(Ranges.begin() + 1, Ranges.end())) {
+    *Next = std::make_unique<SelectionRange>();
+    Next->get()->range = R;
+    Next = &Next->get()->parent;
+  }
+  return Result;
+}
+
 } // namespace
 
 // MessageHandler dispatches incoming LSP messages.
 // It handles cross-cutting concerns:
 //  - serializes/deserializes protocol objects to JSON
 //  - logging of inbound messages
 //  - cancellation handling
 //  - basic call tracing
llvm::json::Object Result{
              llvm::json::Object{
                  {"triggerCharacters", {"(", ","}},
              }},
             {"declarationProvider", true},
             {"definitionProvider", true},
             {"documentHighlightProvider", true},
             {"hoverProvider", true},
             {"renameProvider", std::move(RenameProvider)},
+            {"selectionRangeProvider", true},
             {"documentSymbolProvider", true},
             {"workspaceSymbolProvider", true},
             {"referencesProvider", true},
             {"executeCommandProvider",
              llvm::json::Object{
                  {"commands",
                   {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND,
                    ExecuteCommandParams::CLANGD_APPLY_TWEAK}},

 }
 
 void ClangdLSPServer::onSymbolInfo(const TextDocumentPositionParams &Params,
                                    Callback<std::vector<SymbolDetails>> Reply) {
   Server->symbolInfo(Params.textDocument.uri.file(), Params.position,
                      std::move(Reply));
 }
 
+void ClangdLSPServer::onSelectionRange(
+    const SelectionRangeParams &Params,
+    Callback<std::vector<SelectionRange>> Reply) {
+  if (Params.positions.size() != 1) {
+    elog("{0} positions provided to SelectionRange. Supports exactly one "
+         "position.",
+         Params.positions.size());
+    return Reply(llvm::make_error<LSPError>(
+        "SelectionRange supports exactly one position",
+        ErrorCode::InvalidRequest));
+  }
+  Server->semanticRanges(
+      Params.textDocument.uri.file(), Params.positions[0],
+      [Reply = std::move(Reply)](
+          llvm::Expected<std::vector<Range>> Ranges) mutable {
+        if (!Ranges) {
+          return Reply(Ranges.takeError());
+        }
+        std::vector<SelectionRange> Result;
+        Result.emplace_back(render(std::move(*Ranges)));
+        return Reply(std::move(Result));
+      });
+}
+
 ClangdLSPServer::ClangdLSPServer(
     class Transport &Transp, const FileSystemProvider &FSProvider,
     const clangd::CodeCompleteOptions &CCOpts,
     llvm::Optional<Path> CompileCommandsDir, bool UseDirBasedCDB,
     llvm::Optional<OffsetEncoding> ForcedOffsetEncoding,
     const ClangdServer::Options &Opts)
     : Transp(Transp), MsgHandler(new MessageHandler(*this)),
       FSProvider(FSProvider), CCOpts(CCOpts),
ClangdLSPServer::ClangdLSPServer(
   MsgHandler->bind("textDocument/didOpen", &ClangdLSPServer::onDocumentDidOpen);
   MsgHandler->bind("textDocument/didClose", &ClangdLSPServer::onDocumentDidClose);
   MsgHandler->bind("textDocument/didChange", &ClangdLSPServer::onDocumentDidChange);
   MsgHandler->bind("workspace/didChangeWatchedFiles", &ClangdLSPServer::onFileEvent);
   MsgHandler->bind("workspace/didChangeConfiguration", &ClangdLSPServer::onChangeConfiguration);
   MsgHandler->bind("textDocument/symbolInfo", &ClangdLSPServer::onSymbolInfo);
   MsgHandler->bind("textDocument/typeHierarchy", &ClangdLSPServer::onTypeHierarchy);
   MsgHandler->bind("typeHierarchy/resolve", &ClangdLSPServer::onResolveTypeHierarchy);
+  MsgHandler->bind("textDocument/selectionRange", &ClangdLSPServer::onSelectionRange);
   // clang-format on
 }
 
 ClangdLSPServer::~ClangdLSPServer() { IsBeingDestroyed = true; }
 
 bool ClangdLSPServer::run() {
   // Run the Language Server loop.
   bool CleanExit = true;

clang-tools-extra/clangd/Protocol.h

 
 #include "URI.h"
 #include "index/SymbolID.h"
 #include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/raw_ostream.h"
 #include <bitset>
+#include <memory>
 #include <string>
 #include <vector>
 
 namespace clang {
 namespace clangd {
 
 enum class ErrorCode {
   // Defined by JSON RPC.
struct ClientCapabilities {
   bool CodeActionStructure = false;
 
   /// Client supports semantic highlighting.
   /// textDocument.semanticHighlightingCapabilities.semanticHighlighting
   bool SemanticHighlighting = false;
 
   /// Supported encodings for LSP character offsets. (clangd extension).
   llvm::Optional<std::vector<OffsetEncoding>> offsetEncoding;
 
   /// The content format that should be used for Hover requests.
   /// textDocument.hover.contentEncoding
   MarkupKind HoverContentFormat = MarkupKind::PlainText;
 
   /// The client supports testing for validity of rename operations
   /// before execution.
   bool RenamePrepareSupport = false;
 };

 struct SemanticHighlightingParams {
   /// The textdocument these highlightings belong to.
   TextDocumentIdentifier TextDocument;
   /// The lines of highlightings that should be sent.
   std::vector<SemanticHighlightingInformation> Lines;
 };
 llvm::json::Value toJSON(const SemanticHighlightingParams &Highlighting);
 
+struct SelectionRangeParams {
+  /// The text document.
+  TextDocumentIdentifier textDocument;
+
+  /// The positions inside the text document.
+  std::vector<Position> positions;
+};
+bool fromJSON(const llvm::json::Value &, SelectionRangeParams &);
+
+struct SelectionRange {
+  /**
+   * The range of this selection range.
+   */
+  Range range;
+  /**
+   * The parent selection range containing this range. Therefore `parent.range`
+   * must contain `this.range`.
+   */
+  std::unique_ptr<SelectionRange> parent;
+};
+llvm::json::Value toJSON(const SelectionRange &);
+
 } // namespace clangd
 } // namespace clang
 
 namespace llvm {
 template <> struct format_provider<clang::clangd::Position> {
   static void format(const clang::clangd::Position &Pos, raw_ostream &OS,
                      StringRef Style) {
     assert(Style.empty() && "style modifiers for this type are not supported");
     OS << Pos;
   }
 };
 } // namespace llvm
 
 #endif

clang-tools-extra/clangd/Protocol.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "Protocol.h"
 #include "Logger.h"
 #include "URI.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/Hashing.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"

 
 llvm::json::Value toJSON(const SemanticHighlightingParams &Highlighting) {
   return llvm::json::Object{
       {"textDocument", Highlighting.TextDocument},
       {"lines", std::move(Highlighting.Lines)},
   };
 }
 
+bool fromJSON(const llvm::json::Value &Params, SelectionRangeParams &P) {
+  llvm::json::ObjectMapper O(Params);
+  return O && O.map("textDocument", P.textDocument) &&
+         O.map("positions", P.positions);
+}
+
+llvm::json::Value toJSON(const SelectionRange &Out) {
+  if (Out.parent) {
+    return llvm::json::Object{{"range", Out.range},
+                              {"parent", toJSON(*Out.parent)}};
+  }
+  return llvm::json::Object{{"range", Out.range}};
+}
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/test/initialize-params.test

 # CHECK-NEXT:        "commands": [
 # CHECK-NEXT:          "clangd.applyFix",
 # CHECK-NEXT:          "clangd.applyTweak"
 # CHECK-NEXT:        ]
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "hoverProvider": true,
 # CHECK-NEXT:      "referencesProvider": true,
 # CHECK-NEXT:      "renameProvider": true,
+# CHECK-NEXT:      "selectionRangeProvider": true,
 # CHECK-NEXT:      "signatureHelpProvider": {
 # CHECK-NEXT:        "triggerCharacters": [
 # CHECK-NEXT:          "(",
 # CHECK-NEXT:          ","
 # CHECK-NEXT:        ]
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "textDocumentSync": 2,
 # CHECK-NEXT:      "typeHierarchyProvider": true

clang-tools-extra/clangd/test/selection-range.test

+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void func() {\n}"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/selectionRange","params":{"textDocument":{"uri":"test:///main.cpp"},"positions":[{"line":1,"character":0}]}}
+#      CHECK:  "id": 1
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:    {
+# CHECK-NEXT:      "parent": {
+# CHECK-NEXT:        "range": {
+# CHECK-NEXT:          "end": {
+# CHECK-NEXT:            "character": 1,
+# CHECK-NEXT:            "line": 1
+# CHECK-NEXT:          },
+# CHECK-NEXT:          "start": {
+# CHECK-NEXT:            "character": 0,
+# CHECK-NEXT:            "line": 0
+# CHECK-NEXT:          }
+# CHECK-NEXT:        }
+# CHECK-NEXT:      },
+# CHECK-NEXT:      "range": {
+# CHECK-NEXT:        "end": {
+# CHECK-NEXT:          "character": 1,
+# CHECK-NEXT:          "line": 1
+# CHECK-NEXT:        },
+# CHECK-NEXT:        "start": {
+# CHECK-NEXT:          "character": 12,
+# CHECK-NEXT:          "line": 0
+# CHECK-NEXT:        }
+# CHECK-NEXT:      }
+# CHECK-NEXT:    }
+# CHECK-NEXT:  ]
+# CHECK-NEXT:}
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}