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 HeaderInsertion; + /// Holds information about small corrections that needs to be done. Like + /// converting '->' to '.' on member access. + std::vector 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(), ASTCtx.getLangOpts())); + } } 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; } @@ -1093,8 +1099,8 @@ // Groups overloads if desired, to form CompletionCandidate::Bundles. // The bundles are scored and top results are returned, best to worst. std::vector - mergeResults(const std::vector &SemaResults, - const SymbolSlab &IndexResults) { + mergeResults(const std::vector &SemaResults, + const SymbolSlab &IndexResults) { trace::Span Tracer("Merge and score results"); std::vector Bundles; llvm::DenseMap BundleLookup; @@ -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; } 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/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/Quality.h =================================================================== --- clangd/Quality.h +++ clangd/Quality.h @@ -77,6 +77,8 @@ /// 0-1+ fuzzy-match score for unqualified name. Must be explicitly assigned. float NameMatch = 1; bool Forbidden = false; // Unavailable (e.g const) or inaccessible (private). + int FixItCount = 0; // Number of additional changes that needs to be performed + // for this change to take place. URIDistance *FileProximityMatch = nullptr; /// This is used to calculate proximity between the index symbol and the Index: clangd/Quality.cpp =================================================================== --- clangd/Quality.cpp +++ clangd/Quality.cpp @@ -292,6 +292,10 @@ // Declarations are scoped, others (like macros) are assumed global. if (SemaCCResult.Declaration) Scope = std::min(Scope, computeScope(SemaCCResult.Declaration)); + + for (const auto &FixIt : SemaCCResult.FixIts) { + FixItCount += FixIt.CodeToInsert.size(); + } } static std::pair proximityScore(llvm::StringRef SymbolURI, @@ -343,6 +347,11 @@ Score *= 0.5; } + // Penalize for FixIts. + if (FixItCount > 0) { + Score /= 1 + FixItCount; + } + return Score; } @@ -350,6 +359,7 @@ OS << formatv("=== Symbol relevance: {0}\n", S.evaluate()); OS << formatv("\tName match: {0}\n", S.NameMatch); OS << formatv("\tForbidden: {0}\n", S.Forbidden); + OS << formatv("\tFixItCount: {0}\n", S.FixItCount); OS << formatv("\tIsInstanceMember: {0}\n", S.IsInstanceMember); OS << formatv("\tContext: {0}\n", getCompletionKindString(S.Context)); OS << formatv("\tSymbol URI: {0}\n", S.SymbolURI); Index: clangd/SourceCode.h =================================================================== --- clangd/SourceCode.h +++ clangd/SourceCode.h @@ -14,6 +14,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H #include "Protocol.h" +#include "clang/Basic/Diagnostic.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 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 @@ -199,5 +199,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: unittests/clangd/CodeCompleteTests.cpp =================================================================== --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -79,7 +79,8 @@ return MemIndex::build(std::move(Slab).build()); } -CodeCompleteResult completions(ClangdServer &Server, StringRef Text, +CodeCompleteResult completions(ClangdServer &Server, StringRef TestCode, + Position point, std::vector IndexSymbols = {}, clangd::CodeCompleteOptions Opts = {}) { std::unique_ptr OverrideIndex; @@ -90,23 +91,50 @@ } auto File = testPath("foo.cpp"); - Annotations Test(Text); - runAddDocument(Server, File, Test.code()); - auto CompletionList = - cantFail(runCodeComplete(Server, File, Test.point(), Opts)); + runAddDocument(Server, File, TestCode); + auto CompletionList = cantFail(runCodeComplete(Server, File, point, Opts)); return CompletionList; } // 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(StringRef Text, Position point, std::vector 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, Text, point, std::move(IndexSymbols), + std::move(Opts)); +} + +CodeCompleteResult completions(ClangdServer &Server, Annotations Test, + std::vector IndexSymbols = {}, + clangd::CodeCompleteOptions Opts = {}) { + return completions(Server, Test.code(), Test.point(), std::move(IndexSymbols), + std::move(Opts)); +} + +CodeCompleteResult completions(ClangdServer &Server, StringRef Text, + std::vector IndexSymbols = {}, + clangd::CodeCompleteOptions Opts = {}) { + return completions(Server, Annotations(Text), std::move(IndexSymbols), + std::move(Opts)); +} + +CodeCompleteResult completions(Annotations Test, + std::vector IndexSymbols = {}, + clangd::CodeCompleteOptions Opts = {}) { + return completions(Test.code(), Test.point(), std::move(IndexSymbols), + std::move(Opts)); +} + +CodeCompleteResult completions(StringRef Text, + std::vector 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 +1366,99 @@ 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)); + } + } +} + +TEST(CompletionTest, FixItsHasLowerScore) { + CodeCompleteOptions Opts; + Opts.IncludeFixIts = true; + StringRef 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); + EXPECT_TRUE(Results.Completions.front().FixIts.empty()); +} + } // namespace } // namespace clangd } // namespace clang