Index: clangd/CodeComplete.h =================================================================== --- clangd/CodeComplete.h +++ clangd/CodeComplete.h @@ -123,6 +123,9 @@ /// converting '->' to '.' on member access. std::vector 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: clangd/CodeComplete.cpp =================================================================== --- clangd/CodeComplete.cpp +++ 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 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: clangd/SourceCode.h =================================================================== --- clangd/SourceCode.h +++ clangd/SourceCode.h @@ -76,6 +76,8 @@ /// are normalized as much as possible. llvm::Optional getRealPath(const FileEntry *F, const SourceManager &SourceMgr); + +bool IsRangeConsecutive(const Range &Left, const Range &Right); } // namespace clangd } // namespace clang #endif Index: clangd/SourceCode.cpp =================================================================== --- clangd/SourceCode.cpp +++ 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: unittests/clangd/CodeCompleteTests.cpp =================================================================== --- unittests/clangd/CodeCompleteTests.cpp +++ 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: unittests/clangd/SourceCodeTests.cpp =================================================================== --- unittests/clangd/SourceCodeTests.cpp +++ unittests/clangd/SourceCodeTests.cpp @@ -37,6 +37,13 @@ return Pos; } +Range range(const std::pair p1, const std::pair 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