[clangd] Support textEdit in addition to insertText.

Summary:
Completion replies contains textEdits as well. Note that this change
relies on https://reviews.llvm.org/D50443.

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: mgrang, ioeric, MaskRay, jkorous, arphaman, cfe-commits

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

llvm-svn: 339543

clang-tools-extra/clangd/CodeComplete.cpp

@@ -34,6 +34,7 @@
 #include "index/Index.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
@@ -285,6 +286,11 @@ struct CodeCompletionBuilder {
         Completion.FixIts.push_back(
             toTextEdit(FixIt, ASTCtx.getSourceManager(), ASTCtx.getLangOpts()));
       }
+      std::sort(Completion.FixIts.begin(), Completion.FixIts.end(),
+                [](const TextEdit &X, const TextEdit &Y) {
+                  return std::tie(X.range.start.line, X.range.start.character) <
+                         std::tie(Y.range.start.line, Y.range.start.character);
+                });
     }
     if (C.IndexResult) {
       Completion.Origin |= C.IndexResult->Origin;
@@ -1044,6 +1050,23 @@ class CodeCompleteFlow {
   // This is called by run() once Sema code completion is done, but before the
   // Sema data structures are torn down. It does all the real work.
   CodeCompleteResult runWithSema() {
+    const auto &CodeCompletionRange = CharSourceRange::getCharRange(
+        Recorder->CCSema->getPreprocessor().getCodeCompletionTokenRange());
+    Range TextEditRange;
+    // When we are getting completions with an empty identifier, for example
+    //    std::vector<int> asdf;
+    //    asdf.^;
+    // Then the range will be invalid and we will be doing insertion, use
+    // current cursor position in such cases as range.
+    if (CodeCompletionRange.isValid()) {
+      TextEditRange = halfOpenToRange(Recorder->CCSema->getSourceManager(),
+                                      CodeCompletionRange);
+    } else {
+      const auto &Pos = sourceLocToPosition(
+          Recorder->CCSema->getSourceManager(),
+          Recorder->CCSema->getPreprocessor().getCodeCompletionLoc());
+      TextEditRange.start = TextEditRange.end = Pos;
+    }
     Filter = FuzzyMatcher(
         Recorder->CCSema->getPreprocessor().getCodeCompletionFilter());
     QueryScopes = getQueryScopes(Recorder->CCContext,
@@ -1063,6 +1086,7 @@ class CodeCompleteFlow {
     for (auto &C : Top) {
       Output.Completions.push_back(toCodeCompletion(C.first));
       Output.Completions.back().Score = C.second;
+      Output.Completions.back().CompletionTokenRange = TextEditRange;
     }
     Output.HasMore = Incomplete;
     Output.Context = Recorder->CCContext.getKind();
@@ -1278,16 +1302,29 @@ CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const {
   LSP.documentation = Documentation;
   LSP.sortText = sortText(Score.Total, Name);
   LSP.filterText = Name;
-  // FIXME(kadircet): Use LSP.textEdit instead of insertText, because it causes
-  // undesired behaviours. Like completing "this.^" into "this-push_back".
-  LSP.insertText = RequiredQualifier + Name;
+  LSP.textEdit = {CompletionTokenRange, RequiredQualifier + Name};
+  // Merge continious additionalTextEdits into main edit. The main motivation
+  // behind this is to help LSP clients, it seems most of them are confused when
+  // they are provided with additionalTextEdits that are consecutive to main
+  // edit.
+  // Note that we store additional text edits from back to front in a line. That
+  // is mainly to help LSP clients again, so that changes do not effect each
+  // other.
+  for (const auto &FixIt : FixIts) {
+    if (IsRangeConsecutive(FixIt.range, LSP.textEdit->range)) {
+      LSP.textEdit->newText = FixIt.newText + LSP.textEdit->newText;
+      LSP.textEdit->range.start = FixIt.range.start;
+    } else {
+      LSP.additionalTextEdits.push_back(FixIt);
+    }
+  }
   if (Opts.EnableSnippets)
-    LSP.insertText += SnippetSuffix;
+    LSP.textEdit->newText += SnippetSuffix;
+  // FIXME(kadircet): Do not even fill insertText after making sure textEdit is
+  // compatible with most of the editors.
+  LSP.insertText = LSP.textEdit->newText;
   LSP.insertTextFormat = Opts.EnableSnippets ? InsertTextFormat::Snippet
                                              : InsertTextFormat::PlainText;
-  LSP.additionalTextEdits.reserve(FixIts.size() + (HeaderInsertion ? 1 : 0));
-  for (const auto &FixIt : FixIts)
-    LSP.additionalTextEdits.push_back(FixIt);
   if (HeaderInsertion)
     LSP.additionalTextEdits.push_back(*HeaderInsertion);
   return LSP;

clang-tools-extra/clangd/CodeComplete.h

@@ -123,6 +123,9 @@ struct CodeCompletion {
   /// converting '->' to '.' on member access.
   std::vector<TextEdit> FixIts;
 
+  /// Holds the range of the token we are going to replace with this completion.
+  Range CompletionTokenRange;
+
   // Scores are used to rank completion items.
   struct Scores {
     // The score that items are ranked by.

clang-tools-extra/clangd/SourceCode.cpp

@@ -224,5 +224,10 @@ TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
   return Result;
 }
 
+bool IsRangeConsecutive(const Range &Left, const Range &Right) {
+  return Left.end.line == Right.start.line &&
+         Left.end.character == Right.start.character;
+}
+
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/SourceCode.h

@@ -76,6 +76,8 @@ TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
 /// are normalized as much as possible.
 llvm::Optional<std::string> getRealPath(const FileEntry *F,
                                         const SourceManager &SourceMgr);
+
+bool IsRangeConsecutive(const Range &Left, const Range &Right);
 } // namespace clangd
 } // namespace clang
 #endif

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

@@ -1439,6 +1439,82 @@ TEST(CompletionTest, FixItForDotToArrow) {
   }
 }
 
+TEST(CompletionTest, RenderWithFixItMerged) {
+  TextEdit FixIt;
+  FixIt.range.end.character = 5;
+  FixIt.newText = "->";
+
+  CodeCompletion C;
+  C.Name = "x";
+  C.RequiredQualifier = "Foo::";
+  C.FixIts = {FixIt};
+  C.CompletionTokenRange.start.character = 5;
+
+  CodeCompleteOptions Opts;
+  Opts.IncludeFixIts = true;
+
+  auto R = C.render(Opts);
+  EXPECT_TRUE(R.textEdit);
+  EXPECT_EQ(R.textEdit->newText, "->Foo::x");
+  EXPECT_TRUE(R.additionalTextEdits.empty());
+}
+
+TEST(CompletionTest, RenderWithFixItNonMerged) {
+  TextEdit FixIt;
+  FixIt.range.end.character = 4;
+  FixIt.newText = "->";
+
+  CodeCompletion C;
+  C.Name = "x";
+  C.RequiredQualifier = "Foo::";
+  C.FixIts = {FixIt};
+  C.CompletionTokenRange.start.character = 5;
+
+  CodeCompleteOptions Opts;
+  Opts.IncludeFixIts = true;
+
+  auto R = C.render(Opts);
+  EXPECT_TRUE(R.textEdit);
+  EXPECT_EQ(R.textEdit->newText, "Foo::x");
+  EXPECT_THAT(R.additionalTextEdits, UnorderedElementsAre(FixIt));
+}
+
+TEST(CompletionTest, CompletionTokenRange) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  constexpr const char *TestCodes[] = {
+      R"cpp(
+        class Auxilary {
+         public:
+          void AuxFunction();
+        };
+        void f() {
+          Auxilary x;
+          x.[[Aux]]^;
+        }
+      )cpp",
+      R"cpp(
+        class Auxilary {
+         public:
+          void AuxFunction();
+        };
+        void f() {
+          Auxilary x;
+          x.[[]]^;
+        }
+      )cpp"};
+  for (const auto &Text : TestCodes) {
+    Annotations TestCode(Text);
+    auto Results = completions(Server, TestCode.code(), TestCode.point());
+
+    EXPECT_EQ(Results.Completions.size(), 1u);
+    EXPECT_THAT(Results.Completions.front().CompletionTokenRange, TestCode.range());
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

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

@@ -37,6 +37,13 @@ Position position(int line, int character) {
   return Pos;
 }
 
+Range range(const std::pair<int, int> p1, const std::pair<int, int> p2) {
+  Range range;
+  range.start = position(p1.first, p1.second);
+  range.end = position(p2.first, p2.second);
+  return range;
+}
+
 TEST(SourceCodeTests, PositionToOffset) {
   // line out of bounds
   EXPECT_THAT_EXPECTED(positionToOffset(File, position(-1, 2)), Failed());
@@ -119,6 +126,14 @@ TEST(SourceCodeTests, OffsetToPosition) {
   EXPECT_THAT(offsetToPosition(File, 30), Pos(2, 9)) << "out of bounds";
 }
 
+TEST(SourceCodeTests, IsRangeConsecutive) {
+  EXPECT_TRUE(IsRangeConsecutive(range({2, 2}, {2, 3}), range({2, 3}, {2, 4})));
+  EXPECT_FALSE(
+      IsRangeConsecutive(range({0, 2}, {0, 3}), range({2, 3}, {2, 4})));
+  EXPECT_FALSE(
+      IsRangeConsecutive(range({2, 2}, {2, 3}), range({2, 4}, {2, 5})));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang