This revision was automatically updated to reflect the committed changes.
Closed by commit rL339543: [clangd] Support textEdit in addition to insertText.
(authored by kadircet, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D50449
Files:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeComplete.h
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/SourceCode.h
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
===================================================================
--- clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
@@ -37,6 +37,13 @@
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 @@
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
Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -1439,6 +1439,82 @@
}
}
+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
Index: clang-tools-extra/trunk/clangd/CodeComplete.h
===================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.h
+++ clang-tools-extra/trunk/clangd/CodeComplete.h
@@ -123,6 +123,9 @@
/// 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.
Index: clang-tools-extra/trunk/clangd/SourceCode.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/SourceCode.cpp
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp
@@ -224,5 +224,10 @@
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
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/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 @@
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 @@
// 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 @@
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 @@
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;
Index: clang-tools-extra/trunk/clangd/SourceCode.h
===================================================================
--- clang-tools-extra/trunk/clangd/SourceCode.h
+++ clang-tools-extra/trunk/clangd/SourceCode.h
@@ -76,6 +76,8 @@
/// 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
