[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

1135

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

1135

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

1131

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

clang-tools-extra/clangd/Protocol.h

1234

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.

1241

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

1131

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

1131

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

1241

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

138

nit: use for range.

1131

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

clang-tools-extra/clangd/Protocol.h

1243

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

1131

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

clang-tools-extra/clangd/Protocol.h

1243

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

132

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

1131

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

1148

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

clang-tools-extra/clangd/Protocol.h

421

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

1236

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

1243

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

1246

Are these constructors needed?

[usaxena95]

Addressed comments.

clang-tools-extra/clangd/ClangdLSPServer.cpp

1148

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

1246

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

llvm::Error validateEdits(const DraftStore &DraftMgr, const Tweak::Effect &E) {
   return llvm::createStringError(
       llvm::inconvertibleErrorCode(),
       "Files must be saved first: " + LastInvalidFile + " (and " +
           llvm::to_string(InvalidFileCount - 1) + " others)");
 }
 
 } // 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
 // MessageHandler ensures that initialize() is called before any other handler.
 class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
 public:
   MessageHandler(ClangdLSPServer &Server) : Server(Server) {}
 
   bool onNotify(llvm::StringRef Method, llvm::json::Value Params) override {
     WithContext HandlerContext(handlerContext());
     log("<-- {0}", Method);
     if (Method == "exit")

 }
 
 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());
+    Reply(llvm::make_error<LSPError>(
+        "More than one position provided to SelectionRange",
+        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());
+        }
+        SelectionRange Result;
+        Result.ranges = std::move(*Ranges);
+        Reply(std::vector<SelectionRange>{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

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 semantic ranges for a position. Any range must contain all the previous
+  // ranges. ranges.front() must be the inner most range. ranges.back() must be
+  // the outermost range.
+  std::vector<Range> ranges;
+};
+llvm::json::Value toJSON(const SelectionRange &);
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, 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) {
+  llvm::json::Object Result;
+  for (const auto &R : llvm::reverse(Out.ranges)) {
+    llvm::json::Object Head;
+    Head["range"] = R;
+    if (!Result.empty())
+      Head["parent"] = std::move(Result);
+    Result = std::move(Head);
+  }
+  return Result;
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const SelectionRange &R) {
+  return OS << toJSON(R);
+}
+
 } // namespace clangd
 } // namespace clang

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 int var1;\n int var2 = var1;\n}"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/selectionRange","params":{"textDocument":{"uri":"test:///main.cpp"},"positions":[{"line":2,"character":14}]}}
+#      CHECK:  "id": 1
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:    {
+# CHECK-NEXT:      "parent": {
+# CHECK-NEXT:        "parent": {
+# CHECK-NEXT:          "parent": {
+# CHECK-NEXT:            "parent": {
+# CHECK-NEXT:              "range": {
+# CHECK-NEXT:                "end": {
+# CHECK-NEXT:                  "character": 1,
+# CHECK-NEXT:                  "line": 3
+# 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": 3
+# CHECK-NEXT:              },
+# CHECK-NEXT:              "start": {
+# CHECK-NEXT:                "character": 12,
+# CHECK-NEXT:                "line": 0
+# CHECK-NEXT:              }
+# CHECK-NEXT:            }
+# CHECK-NEXT:          },
+# CHECK-NEXT:          "range": {
+# CHECK-NEXT:            "end": {
+# CHECK-NEXT:              "character": 17,
+# CHECK-NEXT:              "line": 2
+# CHECK-NEXT:            },
+# CHECK-NEXT:            "start": {
+# CHECK-NEXT:              "character": 1,
+# CHECK-NEXT:              "line": 2
+# CHECK-NEXT:            }
+# CHECK-NEXT:          }
+# CHECK-NEXT:        },
+# CHECK-NEXT:        "range": {
+# CHECK-NEXT:          "end": {
+# CHECK-NEXT:            "character": 16,
+# CHECK-NEXT:            "line": 2
+# CHECK-NEXT:          },
+# CHECK-NEXT:          "start": {
+# CHECK-NEXT:            "character": 1,
+# CHECK-NEXT:            "line": 2
+# CHECK-NEXT:          }
+# CHECK-NEXT:        }
+# CHECK-NEXT:      },
+# CHECK-NEXT:      "range": {
+# CHECK-NEXT:        "end": {
+# CHECK-NEXT:          "character": 16,
+# CHECK-NEXT:          "line": 2
+# CHECK-NEXT:        },
+# CHECK-NEXT:        "start": {
+# CHECK-NEXT:          "character": 12,
+# CHECK-NEXT:          "line": 2
+# CHECK-NEXT:        }
+# CHECK-NEXT:      }
+# CHECK-NEXT:    }
+# CHECK-NEXT:  ]
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}