diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -150,21 +150,6 @@ llvm::to_string(InvalidFileCount - 1) + " others)"); } -// Converts a list of Ranges to a LinkedList of SelectionRange. -SelectionRange render(const std::vector &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(); - Next->get()->range = R; - Next = &Next->get()->parent; - } - return Result; -} - } // namespace // MessageHandler dispatches incoming LSP messages. @@ -1215,15 +1200,12 @@ ErrorCode::InvalidRequest)); } Server->semanticRanges( - Params.textDocument.uri.file(), Params.positions[0], + Params.textDocument.uri.file(), Params.positions, [Reply = std::move(Reply)]( - llvm::Expected> Ranges) mutable { - if (!Ranges) { + llvm::Expected> Ranges) mutable { + if (!Ranges) return Reply(Ranges.takeError()); - } - std::vector Result; - Result.emplace_back(render(std::move(*Ranges))); - return Reply(std::move(Result)); + return Reply(std::move(*Ranges)); }); } diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -292,8 +292,8 @@ Callback> CB); /// Get semantic ranges around a specified position in a file. - void semanticRanges(PathRef File, Position Pos, - Callback> CB); + void semanticRanges(PathRef File, const std::vector &Pos, + Callback> CB); /// Get all document links in a file. void documentLinks(PathRef File, Callback> CB); diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -654,14 +654,22 @@ WorkScheduler.runWithAST("SymbolInfo", File, std::move(Action)); } -void ClangdServer::semanticRanges(PathRef File, Position Pos, - Callback> CB) { - auto Action = - [Pos, CB = std::move(CB)](llvm::Expected InpAST) mutable { - if (!InpAST) - return CB(InpAST.takeError()); - CB(clangd::getSemanticRanges(InpAST->AST, Pos)); - }; +void ClangdServer::semanticRanges(PathRef File, + const std::vector &Positions, + Callback> CB) { + auto Action = [Positions, CB = std::move(CB)]( + llvm::Expected InpAST) mutable { + if (!InpAST) + return CB(InpAST.takeError()); + std::vector 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()); + } + return CB(std::move(Result)); + }; WorkScheduler.runWithAST("SemanticRanges", File, std::move(Action)); } diff --git a/clang-tools-extra/clangd/SemanticSelection.h b/clang-tools-extra/clangd/SemanticSelection.h --- a/clang-tools-extra/clangd/SemanticSelection.h +++ b/clang-tools-extra/clangd/SemanticSelection.h @@ -22,10 +22,9 @@ /// 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> getSemanticRanges(ParsedAST &AST, - Position Pos); +/// If pos is not in any interesting range, return [Pos, Pos). +llvm::Expected getSemanticRanges(ParsedAST &AST, Position Pos); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/SemanticSelection.cpp b/clang-tools-extra/clangd/SemanticSelection.cpp --- a/clang-tools-extra/clangd/SemanticSelection.cpp +++ b/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 @@ } } // namespace -llvm::Expected> getSemanticRanges(ParsedAST &AST, - Position Pos) { - std::vector Result; +llvm::Expected getSemanticRanges(ParsedAST &AST, Position Pos) { + std::vector Ranges; const auto &SM = AST.getSourceManager(); const auto &LangOpts = AST.getLangOpts(); @@ -56,9 +56,28 @@ 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 Dummy; + Dummy.range.start = Dummy.range.end = Pos; + return Dummy; + } + + // Convert to the LSP linked-list representation. + SelectionRange Head; + Head.range = std::move(Ranges.front()); + SelectionRange *Tail = &Head; + for (auto &Range : llvm::makeArrayRef(Ranges).drop_front()) { + Tail->parent = std::make_unique(); + Tail = Tail->parent.get(); + Tail->range = Range; + } + + return Head; } } // namespace clangd diff --git a/clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp b/clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp --- a/clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp +++ b/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 gatherRanges(const SelectionRange &SR) { + std::vector 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 @@ }]]]] )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 @@ // 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 @@ 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 @@ // 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 diff --git a/clang-tools-extra/clangd/unittests/SyncAPI.h b/clang-tools-extra/clangd/unittests/SyncAPI.h --- a/clang-tools-extra/clangd/unittests/SyncAPI.h +++ b/clang-tools-extra/clangd/unittests/SyncAPI.h @@ -56,8 +56,9 @@ SymbolSlab runFuzzyFind(const SymbolIndex &Index, const FuzzyFindRequest &Req); RefSlab getRefs(const SymbolIndex &Index, SymbolID ID); -llvm::Expected> -runSemanticRanges(ClangdServer &Server, PathRef File, Position Pos); +llvm::Expected> +runSemanticRanges(ClangdServer &Server, PathRef File, + const std::vector &Pos); llvm::Expected> runSwitchHeaderSource(ClangdServer &Server, PathRef File); diff --git a/clang-tools-extra/clangd/unittests/SyncAPI.cpp b/clang-tools-extra/clangd/unittests/SyncAPI.cpp --- a/clang-tools-extra/clangd/unittests/SyncAPI.cpp +++ b/clang-tools-extra/clangd/unittests/SyncAPI.cpp @@ -146,9 +146,10 @@ return std::move(Slab).build(); } -llvm::Expected> -runSemanticRanges(ClangdServer &Server, PathRef File, Position Pos) { - llvm::Optional>> Result; +llvm::Expected> +runSemanticRanges(ClangdServer &Server, PathRef File, + const std::vector &Pos) { + llvm::Optional>> Result; Server.semanticRanges(File, Pos, capture(Result)); return std::move(*Result); }