kadircet updated this revision to Diff 159000.
kadircet marked 8 inline comments as done.
kadircet added a comment.

Fixed discussions


https://reviews.llvm.org/D50193

Files:
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Diagnostics.cpp
  clangd/Protocol.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -79,7 +79,7 @@
   return MemIndex::build(std::move(Slab).build());
 }
 
-CodeCompleteResult completions(ClangdServer &Server, StringRef Text,
+CodeCompleteResult completions(ClangdServer &Server, Annotations Test,
                                std::vector<Symbol> IndexSymbols = {},
                                clangd::CodeCompleteOptions Opts = {}) {
   std::unique_ptr<SymbolIndex> OverrideIndex;
@@ -90,23 +90,35 @@
   }
 
   auto File = testPath("foo.cpp");
-  Annotations Test(Text);
   runAddDocument(Server, File, Test.code());
   auto CompletionList =
       cantFail(runCodeComplete(Server, File, Test.point(), Opts));
   return CompletionList;
 }
 
+CodeCompleteResult completions(ClangdServer &Server, StringRef Text,
+                               std::vector<Symbol> IndexSymbols = {},
+                               clangd::CodeCompleteOptions Opts = {}) {
+  return completions(Server, Annotations(Text), IndexSymbols, Opts);
+}
+
 // Builds a server and runs code completion.
 // If IndexSymbols is non-empty, an index will be built and passed to opts.
-CodeCompleteResult completions(StringRef Text,
+CodeCompleteResult completions(Annotations Test,
                                std::vector<Symbol> IndexSymbols = {},
                                clangd::CodeCompleteOptions Opts = {}) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
   IgnoreDiagnostics DiagConsumer;
   ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
-  return completions(Server, Text, std::move(IndexSymbols), std::move(Opts));
+  return completions(Server, Test, std::move(IndexSymbols), std::move(Opts));
+}
+
+CodeCompleteResult completions(StringRef Text,
+                               std::vector<Symbol> IndexSymbols = {},
+                               clangd::CodeCompleteOptions Opts = {}) {
+  return completions(Annotations(Text), std::move(IndexSymbols),
+                     std::move(Opts));
 }
 
 std::string replace(StringRef Haystack, StringRef Needle, StringRef Repl) {
@@ -1338,6 +1350,74 @@
   EXPECT_THAT(Results.Context, CodeCompletionContext::CCC_DotMemberAccess);
 }
 
+TEST(CompletionTest, FixItForArrowToDot) {
+  CodeCompleteOptions Opts;
+  Opts.IncludeFixIts = true;
+  Annotations TestCode(
+      R"cpp(
+        class Auxilary {
+         public:
+          void AuxFunction();
+        };
+        class ClassWithPtr {
+         public:
+          void MemberFunction();
+          Auxilary* operator->() const;
+          Auxilary* Aux;
+        };
+        void f() {
+          ClassWithPtr x;
+          x[[->]]^;
+        }
+      )cpp");
+  auto Results = completions(TestCode, {}, Opts);
+  EXPECT_EQ(Results.Completions.size(), 3u);
+
+  TextEdit ReplacementEdit;
+  ReplacementEdit.range = TestCode.range();
+  ReplacementEdit.newText = ".";
+  for (const auto &C : Results.Completions) {
+    EXPECT_TRUE(C.FixIts.size() == 1u || C.Name == "AuxFunction");
+    if (!C.FixIts.empty()) {
+      EXPECT_THAT(C.FixIts, ElementsAre(ReplacementEdit));
+    }
+  }
+}
+
+TEST(CompletionTest, FixItForDotToArrow) {
+  CodeCompleteOptions Opts;
+  Opts.IncludeFixIts = true;
+  Annotations TestCode(
+      R"cpp(
+        class Auxilary {
+         public:
+          void AuxFunction();
+        };
+        class ClassWithPtr {
+         public:
+          void MemberFunction();
+          Auxilary* operator->() const;
+          Auxilary* Aux;
+        };
+        void f() {
+          ClassWithPtr x;
+          x[[.]]^;
+        }
+      )cpp");
+  auto Results = completions(TestCode, {}, Opts);
+  EXPECT_EQ(Results.Completions.size(), 3u);
+
+  TextEdit ReplacementEdit;
+  ReplacementEdit.range = TestCode.range();
+  ReplacementEdit.newText = "->";
+  for (const auto &C : Results.Completions) {
+    EXPECT_TRUE(C.FixIts.empty() || C.Name == "AuxFunction");
+    if (!C.FixIts.empty()) {
+      EXPECT_THAT(C.FixIts, ElementsAre(ReplacementEdit));
+    }
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/SourceCode.h
===================================================================
--- clangd/SourceCode.h
+++ clangd/SourceCode.h
@@ -13,6 +13,7 @@
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
+#include "Diagnostics.h"
 #include "Protocol.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Tooling/Core/Replacement.h"
@@ -64,6 +65,10 @@
 /// Get the absolute file path of a given file entry.
 llvm::Optional<std::string> getAbsoluteFilePath(const FileEntry *F,
                                                 const SourceManager &SourceMgr);
+
+TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
+                    const LangOptions &L);
+
 } // namespace clangd
 } // namespace clang
 #endif
Index: clangd/SourceCode.cpp
===================================================================
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -8,6 +8,7 @@
 //===----------------------------------------------------------------------===//
 #include "SourceCode.h"
 
+#include "Diagnostics.h"
 #include "Logger.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Basic/SourceManager.h"
@@ -199,5 +200,14 @@
   return FilePath.str().str();
 }
 
+TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
+                    const LangOptions &L) {
+  TextEdit Result;
+  Result.range =
+      halfOpenToRange(M, Lexer::makeFileCharRange(FixIt.RemoveRange, M, L));
+  Result.newText = FixIt.CodeToInsert;
+  return Result;
+}
+
 } // namespace clangd
 } // namespace clang
Index: clangd/Protocol.h
===================================================================
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -171,6 +171,10 @@
   /// The string to be inserted. For delete operations use an
   /// empty string.
   std::string newText;
+
+  bool operator==(const TextEdit &rhs) const {
+    return newText == rhs.newText && range == rhs.range;
+  }
 };
 bool fromJSON(const llvm::json::Value &, TextEdit &);
 llvm::json::Value toJSON(const TextEdit &);
Index: clangd/Diagnostics.cpp
===================================================================
--- clangd/Diagnostics.cpp
+++ clangd/Diagnostics.cpp
@@ -70,15 +70,6 @@
   return halfOpenToRange(M, R);
 }
 
-TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
-                    const LangOptions &L) {
-  TextEdit Result;
-  Result.range =
-      halfOpenToRange(M, Lexer::makeFileCharRange(FixIt.RemoveRange, M, L));
-  Result.newText = FixIt.CodeToInsert;
-  return Result;
-}
-
 bool isInsideMainFile(const SourceLocation Loc, const SourceManager &M) {
   return Loc.isValid() && M.isInMainFile(Loc);
 }
Index: clangd/CodeComplete.h
===================================================================
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -77,6 +77,10 @@
   /// FIXME(ioeric): we might want a better way to pass the index around inside
   /// clangd.
   const SymbolIndex *Index = nullptr;
+
+  /// Include completions that require small corrections, e.g. change '.' to
+  /// '->' on member access etc.
+  bool IncludeFixIts = false;
 };
 
 // Semi-structured representation of a code-complete suggestion for our C++ API.
@@ -115,6 +119,10 @@
   // Present if Header is set and should be inserted to use this item.
   llvm::Optional<TextEdit> HeaderInsertion;
 
+  /// Holds information about small corrections that needs to be done. Like
+  /// converting '->' to '.' on member access.
+  std::vector<TextEdit> FixIts;
+
   // Scores are used to rank completion items.
   struct Scores {
     // The score that items are ranked by.
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -22,6 +22,7 @@
 #include "AST.h"
 #include "CodeCompletionStrings.h"
 #include "Compiler.h"
+#include "Diagnostics.h"
 #include "FileDistance.h"
 #include "FuzzyMatch.h"
 #include "Headers.h"
@@ -280,6 +281,10 @@
       }
       Completion.Kind =
           toCompletionItemKind(C.SemaResult->Kind, C.SemaResult->Declaration);
+      for (const auto &FixIt : C.SemaResult->FixIts) {
+        Completion.FixIts.push_back(
+            toTextEdit(FixIt, ASTCtx.getSourceManager(), {}));
+      }
     }
     if (C.IndexResult) {
       Completion.Origin |= C.IndexResult->Origin;
@@ -909,6 +914,7 @@
   // the index can provide results from the preamble.
   // Tell Sema not to deserialize the preamble to look for results.
   Result.LoadExternal = !Index;
+  Result.IncludeFixIts = IncludeFixIts;
 
   return Result;
 }
@@ -1275,13 +1281,18 @@
   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;
   if (Opts.EnableSnippets)
     LSP.insertText += SnippetSuffix;
   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 = {*HeaderInsertion};
+    LSP.additionalTextEdits.push_back(*HeaderInsertion);
   return LSP;
 }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to