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). + bool NeedsFixIts = + false; // Whether fixits needs to be applied for that completion or not. 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,8 @@ // Declarations are scoped, others (like macros) are assumed global. if (SemaCCResult.Declaration) Scope = std::min(Scope, computeScope(SemaCCResult.Declaration)); + + NeedsFixIts = !SemaCCResult.FixIts.empty(); } static std::pair proximityScore(llvm::StringRef SymbolURI, @@ -343,6 +345,10 @@ Score *= 0.5; } + // Penalize for FixIts. + if (NeedsFixIts) + Score *= 0.5; + return Score; } @@ -350,6 +356,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("\tNeedsFixIts: {0}\n", S.NeedsFixIts); 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,6 +79,25 @@ return MemIndex::build(std::move(Slab).build()); } +CodeCompleteResult completions(ClangdServer &Server, StringRef TestCode, + Position point, + std::vector IndexSymbols = {}, + clangd::CodeCompleteOptions Opts = {}) { + std::unique_ptr OverrideIndex; + if (!IndexSymbols.empty()) { + assert(!Opts.Index && "both Index and IndexSymbols given!"); + OverrideIndex = memIndex(std::move(IndexSymbols)); + Opts.Index = OverrideIndex.get(); + } + + auto File = testPath("foo.cpp"); + 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(ClangdServer &Server, StringRef Text, std::vector IndexSymbols = {}, clangd::CodeCompleteOptions Opts = {}) { @@ -97,8 +116,6 @@ 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, std::vector IndexSymbols = {}, clangd::CodeCompleteOptions Opts = {}) { @@ -1338,6 +1355,85 @@ EXPECT_THAT(Results.Context, CodeCompletionContext::CCC_DotMemberAccess); } +TEST(CompletionTest, FixItForArrowToDot) { + MockFSProvider FS; + MockCompilationDatabase CDB; + IgnoreDiagnostics DiagConsumer; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + + 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(Server, TestCode.code(), TestCode.point(), {}, 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) { + MockFSProvider FS; + MockCompilationDatabase CDB; + IgnoreDiagnostics DiagConsumer; + ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); + + 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(Server, TestCode.code(), TestCode.point(), {}, 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: unittests/clangd/QualityTests.cpp =================================================================== --- unittests/clangd/QualityTests.cpp +++ unittests/clangd/QualityTests.cpp @@ -346,6 +346,28 @@ EXPECT_EQ(Q.Category, SymbolQualitySignals::Constructor); } +TEST(QualityTests, ItemWithFixItsRankedDown) { + CodeCompleteOptions Opts; + Opts.IncludeFixIts = true; + + auto Header = TestTU::withHeaderCode(R"cpp( + int x; + )cpp"); + auto AST = Header.build(); + + SymbolRelevanceSignals RelevanceWithFixIt; + RelevanceWithFixIt.merge( + CodeCompletionResult(&findDecl(AST, "x"), 0, nullptr, false, true, {{}})); + EXPECT_TRUE(RelevanceWithFixIt.NeedsFixIts); + + SymbolRelevanceSignals RelevanceWithoutFixIt; + RelevanceWithoutFixIt.merge( + CodeCompletionResult(&findDecl(AST, "x"), 0, nullptr, false, true, {})); + EXPECT_FALSE(RelevanceWithoutFixIt.NeedsFixIts); + + EXPECT_LT(RelevanceWithFixIt.evaluate(), RelevanceWithoutFixIt.evaluate()); +} + } // namespace } // namespace clangd } // namespace clang