[clangd] Support multiple cursors in selectionRange.

Summary:
One change: because there's no way to signal failure individually for
each cursor, we now "succeed" with an empty range with no parent if a
cursor doesn't point at anything.

Reviewers: usaxena95

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D76741

clang-tools-extra/clangd/ClangdLSPServer.cpp

@@ -150,21 +150,6 @@ llvm::Error validateEdits(const DraftStore &DraftMgr, const FileEdits &FE) {
           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.
@@ -1206,24 +1191,13 @@ void ClangdLSPServer::onSymbolInfo(const TextDocumentPositionParams &Params,
 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],
+      Params.textDocument.uri.file(), Params.positions,
       [Reply = std::move(Reply)](
-          llvm::Expected<std::vector<Range>> Ranges) mutable {
-        if (!Ranges) {
+          llvm::Expected<std::vector<SelectionRange>> Ranges) mutable {
+        if (!Ranges)
           return Reply(Ranges.takeError());
-        }
-        std::vector<SelectionRange> Result;
-        Result.emplace_back(render(std::move(*Ranges)));
-        return Reply(std::move(Result));
+        return Reply(std::move(*Ranges));
       });
 }
 

clang-tools-extra/clangd/ClangdServer.cpp

@@ -654,14 +654,22 @@ void ClangdServer::symbolInfo(PathRef File, Position Pos,
   WorkScheduler.runWithAST("SymbolInfo", File, std::move(Action));
 }
 
-void ClangdServer::semanticRanges(PathRef File, Position Pos,
-                                  Callback<std::vector<Range>> CB) {
-  auto Action =
-      [Pos, CB = std::move(CB)](llvm::Expected<InputsAndAST> InpAST) mutable {
-        if (!InpAST)
-          return CB(InpAST.takeError());
-        CB(clangd::getSemanticRanges(InpAST->AST, Pos));
-      };
+void ClangdServer::semanticRanges(PathRef File,
+                                  const std::vector<Position> &Positions,
+                                  Callback<std::vector<SelectionRange>> CB) {
+  auto Action = [Positions, CB = std::move(CB)](
+                    llvm::Expected<InputsAndAST> InpAST) mutable {
+    if (!InpAST)
+      return CB(InpAST.takeError());
+    std::vector<SelectionRange> Result;
+    for (const auto &Pos : Positions) {
+      if (auto Range = clangd::getSemanticRanges(InpAST->AST, Pos))
+        Result.push_back(std::move(*Range));
+      else
+        return CB(Range.takeError());
+    }
+    CB(std::move(Result));
+  };
   WorkScheduler.runWithAST("SemanticRanges", File, std::move(Action));
 }
 

clang-tools-extra/clangd/ClangdServer.h

@@ -292,8 +292,8 @@ public:
                   Callback<std::vector<SymbolDetails>> CB);
 
   /// Get semantic ranges around a specified position in a file.
-  void semanticRanges(PathRef File, Position Pos,
-                      Callback<std::vector<Range>> CB);
+  void semanticRanges(PathRef File, const std::vector<Position> &Pos,
+                      Callback<std::vector<SelectionRange>> CB);
 
   /// Get all document links in a file.
   void documentLinks(PathRef File, Callback<std::vector<DocumentLink>> CB);

clang-tools-extra/clangd/SemanticSelection.cpp

@@ -12,6 +12,7 @@
 #include "SourceCode.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Error.h"
 
 namespace clang {
@@ -26,9 +27,8 @@ void addIfDistinct(const Range &R, std::vector<Range> &Result) {
 }
 } // namespace
 
-llvm::Expected<std::vector<Range>> getSemanticRanges(ParsedAST &AST,
-                                                     Position Pos) {
-  std::vector<Range> Result;
+llvm::Expected<SelectionRange> getSemanticRanges(ParsedAST &AST, Position Pos) {
+  std::vector<Range> Ranges;
   const auto &SM = AST.getSourceManager();
   const auto &LangOpts = AST.getLangOpts();
 
@@ -56,9 +56,29 @@ llvm::Expected<std::vector<Range>> getSemanticRanges(ParsedAST &AST,
     Range R;
     R.start = sourceLocToPosition(SM, SR->getBegin());
     R.end = sourceLocToPosition(SM, SR->getEnd());
-    addIfDistinct(R, Result);
+    addIfDistinct(R, Ranges);
   }
-  return Result;
+
+  if (Ranges.empty()) {
+    // LSP provides no way to signal "the point is not within a semantic range".
+    // Return an empty range at the point.
+    SelectionRange Empty;
+    Empty.range.start = Empty.range.end = Pos;
+    return Empty;
+  }
+
+  // Convert to the LSP linked-list representation.
+  SelectionRange Head;
+  Head.range = std::move(Ranges.front());
+  SelectionRange *Tail = &Head;
+  for (auto &Range :
+       llvm::makeMutableArrayRef(Ranges.data(), Ranges.size()).drop_front()) {
+    Tail->parent = std::make_unique<SelectionRange>();
+    Tail = Tail->parent.get();
+    Tail->range = std::move(Range);
+  }
+
+  return Head;
 }
 
 } // namespace clangd

clang-tools-extra/clangd/SemanticSelection.h

@@ -22,10 +22,9 @@ namespace clangd {
 /// Returns the list of all interesting ranges around the Position \p Pos.
 /// The interesting ranges corresponds to the AST nodes in the SelectionTree
 /// containing \p Pos.
-/// Any range in the result strictly contains all the previous ranges in the
-/// result. front() is the inner most range. back() is the outermost range.
-llvm::Expected<std::vector<Range>> getSemanticRanges(ParsedAST &AST,
-                                                     Position Pos);
+/// If pos is not in any interesting range, return [Pos, Pos).
+llvm::Expected<SelectionRange> getSemanticRanges(ParsedAST &AST, Position Pos);
+
 } // namespace clangd
 } // namespace clang
 

clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp

@@ -24,8 +24,17 @@
 namespace clang {
 namespace clangd {
 namespace {
+using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
 
+// front() is SR.range, back() is outermost range.
+std::vector<Range> gatherRanges(const SelectionRange &SR) {
+  std::vector<Range> Ranges;
+  for (const SelectionRange *S = &SR; S; S = S->parent.get())
+    Ranges.push_back(S->range);
+  return Ranges;
+}
+
 TEST(SemanticSelection, All) {
   const char *Tests[] = {
       R"cpp( // Single statement in a function body.
@@ -78,7 +87,7 @@ TEST(SemanticSelection, All) {
         }]]]]
        )cpp",
       // Empty file.
-      "^",
+      "[[^]]",
       // FIXME: We should get the whole DeclStmt as a range.
       R"cpp( // Single statement in TU.
         [[int v = [[1^00]]]];
@@ -89,7 +98,7 @@ TEST(SemanticSelection, All) {
       // FIXME: No node found associated to the position.
       R"cpp( // Cursor in between spaces.
         void func() {
-          int v = 100 + ^  100;
+          int v = 100 + [[^]]  100;
         }
       )cpp",
       // Structs.
@@ -133,13 +142,13 @@ TEST(SemanticSelection, All) {
   for (const char *Test : Tests) {
     auto T = Annotations(Test);
     auto AST = TestTU::withCode(T.code()).build();
-    EXPECT_THAT(llvm::cantFail(getSemanticRanges(AST, T.point())),
+    EXPECT_THAT(gatherRanges(llvm::cantFail(getSemanticRanges(AST, T.point()))),
                 ElementsAreArray(T.ranges()))
         << Test;
   }
 }
 
-TEST(SemanticSelection, RunViaClangDServer) {
+TEST(SemanticSelection, RunViaClangdServer) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
   ClangdServer Server(CDB, FS, ClangdServer::optsForTest());
@@ -157,15 +166,20 @@ TEST(SemanticSelection, RunViaClangDServer) {
     // inp = HASH(foo(inp));
     [[inp = [[HASH([[foo([[in^p]])]])]]]];
   }]]]]
+  $empty[[^]]
   )cpp";
   Annotations SourceAnnotations(SourceContents);
   FS.Files[FooCpp] = std::string(SourceAnnotations.code());
   Server.addDocument(FooCpp, SourceAnnotations.code());
 
-  auto Ranges = runSemanticRanges(Server, FooCpp, SourceAnnotations.point());
+  auto Ranges = runSemanticRanges(Server, FooCpp, SourceAnnotations.points());
   ASSERT_TRUE(bool(Ranges))
       << "getSemanticRange returned an error: " << Ranges.takeError();
-  EXPECT_THAT(*Ranges, ElementsAreArray(SourceAnnotations.ranges()));
+  ASSERT_EQ(Ranges->size(), SourceAnnotations.points().size());
+  EXPECT_THAT(gatherRanges(Ranges->front()),
+              ElementsAreArray(SourceAnnotations.ranges()));
+  EXPECT_THAT(gatherRanges(Ranges->back()),
+              ElementsAre(SourceAnnotations.range("empty")));
 }
 } // namespace
 } // namespace clangd

clang-tools-extra/clangd/unittests/SyncAPI.cpp

@@ -146,9 +146,10 @@ RefSlab getRefs(const SymbolIndex &Index, SymbolID ID) {
   return std::move(Slab).build();
 }
 
-llvm::Expected<std::vector<Range>>
-runSemanticRanges(ClangdServer &Server, PathRef File, Position Pos) {
-  llvm::Optional<llvm::Expected<std::vector<Range>>> Result;
+llvm::Expected<std::vector<SelectionRange>>
+runSemanticRanges(ClangdServer &Server, PathRef File,
+                  const std::vector<Position> &Pos) {
+  llvm::Optional<llvm::Expected<std::vector<SelectionRange>>> Result;
   Server.semanticRanges(File, Pos, capture(Result));
   return std::move(*Result);
 }

clang-tools-extra/clangd/unittests/SyncAPI.h

@@ -56,8 +56,9 @@ SymbolSlab runFuzzyFind(const SymbolIndex &Index, StringRef Query);
 SymbolSlab runFuzzyFind(const SymbolIndex &Index, const FuzzyFindRequest &Req);
 RefSlab getRefs(const SymbolIndex &Index, SymbolID ID);
 
-llvm::Expected<std::vector<Range>>
-runSemanticRanges(ClangdServer &Server, PathRef File, Position Pos);
+llvm::Expected<std::vector<SelectionRange>>
+runSemanticRanges(ClangdServer &Server, PathRef File,
+                  const std::vector<Position> &Pos);
 
 llvm::Expected<llvm::Optional<clangd::Path>>
 runSwitchHeaderSource(ClangdServer &Server, PathRef File);

llvm/include/llvm/Testing/Support/Annotations.h

@@ -68,12 +68,14 @@ public:
   /// Crashes if there isn't exactly one.
   size_t point(llvm::StringRef Name = "") const;
   /// Returns the position of all points marked by ^ (or $name^) in the text.
+  /// Order matches the order within the text.
   std::vector<size_t> points(llvm::StringRef Name = "") const;
 
   /// Returns the location of the range marked by [[ ]] (or $name[[ ]]).
   /// Crashes if there isn't exactly one.
   Range range(llvm::StringRef Name = "") const;
   /// Returns the location of all ranges marked by [[ ]] (or $name[[ ]]).
+  /// They are ordered by start position within the text.
   std::vector<Range> ranges(llvm::StringRef Name = "") const;
 
 private: