Author: kadircet Date: Wed Aug 8 01:59:29 2018 New Revision: 339224 URL: http://llvm.org/viewvc/llvm-project?rev=339224&view=rev Log: Added functionality to suggest FixIts for conversion of '->' to '.' and vice versa.
Summary: Added functionality to suggest FixIts for conversion of '->' to '.' and vice versa. Reviewers: ilya-biryukov Reviewed By: ilya-biryukov Subscribers: yvvan, ioeric, jkorous, arphaman, cfe-commits, kadircet Differential Revision: https://reviews.llvm.org/D50193 Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp clang-tools-extra/trunk/clangd/CodeComplete.h clang-tools-extra/trunk/clangd/Diagnostics.cpp clang-tools-extra/trunk/clangd/Protocol.h clang-tools-extra/trunk/clangd/Quality.cpp clang-tools-extra/trunk/clangd/Quality.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/QualityTests.cpp Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=339224&r1=339223&r2=339224&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original) +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Aug 8 01:59:29 2018 @@ -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 @@ struct CodeCompletionBuilder { } 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; @@ -906,6 +911,7 @@ clang::CodeCompleteOptions CodeCompleteO // 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; } @@ -1090,8 +1096,8 @@ private: // Groups overloads if desired, to form CompletionCandidate::Bundles. // The bundles are scored and top results are returned, best to worst. std::vector<ScoredBundle> - mergeResults(const std::vector<CodeCompletionResult> &SemaResults, - const SymbolSlab &IndexResults) { + mergeResults(const std::vector<CodeCompletionResult> &SemaResults, + const SymbolSlab &IndexResults) { trace::Span Tracer("Merge and score results"); std::vector<CompletionCandidate::Bundle> Bundles; llvm::DenseMap<size_t, size_t> BundleLookup; @@ -1272,13 +1278,18 @@ CompletionItem CodeCompletion::render(co 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; } Modified: clang-tools-extra/trunk/clangd/CodeComplete.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.h?rev=339224&r1=339223&r2=339224&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/CodeComplete.h (original) +++ clang-tools-extra/trunk/clangd/CodeComplete.h Wed Aug 8 01:59:29 2018 @@ -77,6 +77,10 @@ struct CodeCompleteOptions { /// 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 @@ struct CodeCompletion { // 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. Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.cpp?rev=339224&r1=339223&r2=339224&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Diagnostics.cpp (original) +++ clang-tools-extra/trunk/clangd/Diagnostics.cpp Wed Aug 8 01:59:29 2018 @@ -70,15 +70,6 @@ Range diagnosticRange(const clang::Diagn 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); } Modified: clang-tools-extra/trunk/clangd/Protocol.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=339224&r1=339223&r2=339224&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Protocol.h (original) +++ clang-tools-extra/trunk/clangd/Protocol.h Wed Aug 8 01:59:29 2018 @@ -171,6 +171,10 @@ struct TextEdit { /// 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 &); Modified: clang-tools-extra/trunk/clangd/Quality.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.cpp?rev=339224&r1=339223&r2=339224&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Quality.cpp (original) +++ clang-tools-extra/trunk/clangd/Quality.cpp Wed Aug 8 01:59:29 2018 @@ -292,6 +292,8 @@ void SymbolRelevanceSignals::merge(const // 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<float, unsigned> proximityScore(llvm::StringRef SymbolURI, @@ -343,6 +345,10 @@ float SymbolRelevanceSignals::evaluate() Score *= 0.5; } + // Penalize for FixIts. + if (NeedsFixIts) + Score *= 0.5; + return Score; } @@ -350,6 +356,7 @@ raw_ostream &operator<<(raw_ostream &OS, 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); Modified: clang-tools-extra/trunk/clangd/Quality.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.h?rev=339224&r1=339223&r2=339224&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Quality.h (original) +++ clang-tools-extra/trunk/clangd/Quality.h Wed Aug 8 01:59:29 2018 @@ -77,6 +77,8 @@ struct SymbolRelevanceSignals { /// 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). + /// Whether fixits needs to be applied for that completion or not. + bool NeedsFixIts = false; URIDistance *FileProximityMatch = nullptr; /// This is used to calculate proximity between the index symbol and the Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=339224&r1=339223&r2=339224&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/SourceCode.cpp (original) +++ clang-tools-extra/trunk/clangd/SourceCode.cpp Wed Aug 8 01:59:29 2018 @@ -199,5 +199,14 @@ getAbsoluteFilePath(const FileEntry *F, 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 Modified: clang-tools-extra/trunk/clangd/SourceCode.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.h?rev=339224&r1=339223&r2=339224&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/SourceCode.h (original) +++ clang-tools-extra/trunk/clangd/SourceCode.h Wed Aug 8 01:59:29 2018 @@ -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 @@ std::vector<TextEdit> replacementsToEdit /// 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 Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=339224&r1=339223&r2=339224&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Wed Aug 8 01:59:29 2018 @@ -79,6 +79,23 @@ std::unique_ptr<SymbolIndex> memIndex(st return MemIndex::build(std::move(Slab).build()); } +CodeCompleteResult completions(ClangdServer &Server, StringRef TestCode, + Position point, + std::vector<Symbol> IndexSymbols = {}, + clangd::CodeCompleteOptions Opts = {}) { + std::unique_ptr<SymbolIndex> 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; +} + CodeCompleteResult completions(ClangdServer &Server, StringRef Text, std::vector<Symbol> IndexSymbols = {}, clangd::CodeCompleteOptions Opts = {}) { @@ -1342,6 +1359,85 @@ TEST(CompletionTest, CodeCompletionConte 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 Modified: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp?rev=339224&r1=339223&r2=339224&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp Wed Aug 8 01:59:29 2018 @@ -346,6 +346,28 @@ TEST(QualityTests, ConstructorQuality) { 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, {FixItHint{}})); + 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
