Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp @@ -193,7 +193,9 @@ llvm::Optional CompileCommandsDir) : Out(Out), CDB(/*Logger=*/Out, std::move(CompileCommandsDir)), Server(CDB, /*DiagConsumer=*/*this, FSProvider, AsyncThreadsCount, - SnippetCompletions, /*Logger=*/Out, ResourceDir) {} + clangd::CodeCompleteOptions( + /*EnableSnippetsAndCodePatterns=*/SnippetCompletions), + /*Logger=*/Out, ResourceDir) {} void ClangdLSPServer::run(std::istream &In) { assert(!IsDone && "Run was called before"); Index: clang-tools-extra/trunk/clangd/ClangdServer.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.h +++ clang-tools-extra/trunk/clangd/ClangdServer.h @@ -209,7 +209,8 @@ ClangdServer(GlobalCompilationDatabase &CDB, DiagnosticsConsumer &DiagConsumer, FileSystemProvider &FSProvider, unsigned AsyncThreadsCount, - bool SnippetCompletions, clangd::Logger &Logger, + clangd::CodeCompleteOptions CodeCompleteOpts, + clangd::Logger &Logger, llvm::Optional ResourceDir = llvm::None); /// Set the root path of the workspace. @@ -309,7 +310,7 @@ // If set, this represents the workspace path. llvm::Optional RootPath; std::shared_ptr PCHs; - bool SnippetCompletions; + clangd::CodeCompleteOptions CodeCompleteOpts; /// Used to serialize diagnostic callbacks. /// FIXME(ibiryukov): get rid of an extra map and put all version counters /// into CppFile. Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp @@ -144,15 +144,15 @@ ClangdServer::ClangdServer(GlobalCompilationDatabase &CDB, DiagnosticsConsumer &DiagConsumer, FileSystemProvider &FSProvider, - unsigned AsyncThreadsCount, bool SnippetCompletions, + unsigned AsyncThreadsCount, + clangd::CodeCompleteOptions CodeCompleteOpts, clangd::Logger &Logger, llvm::Optional ResourceDir) : Logger(Logger), CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider), ResourceDir(ResourceDir ? ResourceDir->str() : getStandardResourceDir()), PCHs(std::make_shared()), - SnippetCompletions(SnippetCompletions), WorkScheduler(AsyncThreadsCount) { -} + CodeCompleteOpts(CodeCompleteOpts), WorkScheduler(AsyncThreadsCount) {} void ClangdServer::setRootPath(PathRef RootPath) { std::string NewRootPath = llvm::sys::path::convert_to_slash( @@ -237,7 +237,7 @@ std::vector Result = clangd::codeComplete( File, Resources->getCompileCommand(), Preamble ? &Preamble->Preamble : nullptr, Contents, Pos, TaggedFS.Value, - PCHs, SnippetCompletions, Logger); + PCHs, CodeCompleteOpts, Logger); return make_tagged(std::move(Result), std::move(TaggedFS.Tag)); }); Index: clang-tools-extra/trunk/clangd/ClangdUnit.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.h +++ clang-tools-extra/trunk/clangd/ClangdUnit.h @@ -251,13 +251,49 @@ clangd::Logger &Logger; }; +struct CodeCompleteOptions { + CodeCompleteOptions() = default; + + /// Uses default values for all flags, but sets EnableSnippets and + /// IncludeCodePatterns to the value of EnableSnippetsAndCodePatterns. + explicit CodeCompleteOptions(bool EnableSnippetsAndCodePatterns); + + CodeCompleteOptions(bool EnableSnippets, bool IncludeCodePatterns, + bool IncludeMacros, bool IncludeGlobals, + bool IncludeBriefComments); + + /// Returns options that can be passed to clang's completion engine. + clang::CodeCompleteOptions getClangCompleteOpts(); + + /// When true, completion items will contain expandable code snippets in + /// completion (e.g. `return ${1:expression}` or `foo(${1:int a}, ${2:int + /// b})). + bool EnableSnippets = false; + + /// Add code patterns to completion results. + /// If EnableSnippets is false, this options is ignored and code patterns will + /// always be omitted. + bool IncludeCodePatterns = false; + + /// Add macros to code completion results. + bool IncludeMacros = true; + + /// Add globals to code completion results. + bool IncludeGlobals = true; + + /// Add brief comments to completion items, if available. + /// FIXME(ibiryukov): it looks like turning this option on significantly slows + /// down completion, investigate if it can be made faster. + bool IncludeBriefComments = true; +}; + /// Get code completions at a specified \p Pos in \p FileName. std::vector codeComplete(PathRef FileName, tooling::CompileCommand Command, PrecompiledPreamble const *Preamble, StringRef Contents, Position Pos, IntrusiveRefCntPtr VFS, std::shared_ptr PCHs, - bool SnippetCompletions, clangd::Logger &Logger); + clangd::CodeCompleteOptions Opts, clangd::Logger &Logger); /// Get signature help at a specified \p Pos in \p FileName. SignatureHelp signatureHelp(PathRef FileName, tooling::CompileCommand Command, Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp @@ -369,7 +369,7 @@ class CompletionItemsCollector : public CodeCompleteConsumer { public: - CompletionItemsCollector(const CodeCompleteOptions &CodeCompleteOpts, + CompletionItemsCollector(const clang::CodeCompleteOptions &CodeCompleteOpts, std::vector &Items) : CodeCompleteConsumer(CodeCompleteOpts, /*OutputIsBinary=*/false), Items(Items), @@ -471,8 +471,9 @@ : public CompletionItemsCollector { public: - PlainTextCompletionItemsCollector(const CodeCompleteOptions &CodeCompleteOpts, - std::vector &Items) + PlainTextCompletionItemsCollector( + const clang::CodeCompleteOptions &CodeCompleteOpts, + std::vector &Items) : CompletionItemsCollector(CodeCompleteOpts, Items) {} private: @@ -507,8 +508,9 @@ class SnippetCompletionItemsCollector final : public CompletionItemsCollector { public: - SnippetCompletionItemsCollector(const CodeCompleteOptions &CodeCompleteOpts, - std::vector &Items) + SnippetCompletionItemsCollector( + const clang::CodeCompleteOptions &CodeCompleteOpts, + std::vector &Items) : CompletionItemsCollector(CodeCompleteOpts, Items) {} private: @@ -617,7 +619,7 @@ class SignatureHelpCollector final : public CodeCompleteConsumer { public: - SignatureHelpCollector(const CodeCompleteOptions &CodeCompleteOpts, + SignatureHelpCollector(const clang::CodeCompleteOptions &CodeCompleteOpts, SignatureHelp &SigHelp) : CodeCompleteConsumer(CodeCompleteOpts, /*OutputIsBinary=*/false), SigHelp(SigHelp), @@ -708,7 +710,8 @@ }; // SignatureHelpCollector bool invokeCodeComplete(std::unique_ptr Consumer, - const CodeCompleteOptions &Options, PathRef FileName, + const clang::CodeCompleteOptions &Options, + PathRef FileName, const tooling::CompileCommand &Command, PrecompiledPreamble const *Preamble, StringRef Contents, Position Pos, IntrusiveRefCntPtr VFS, @@ -774,29 +777,51 @@ } // namespace +clangd::CodeCompleteOptions::CodeCompleteOptions( + bool EnableSnippetsAndCodePatterns) + : CodeCompleteOptions() { + EnableSnippets = EnableSnippetsAndCodePatterns; + IncludeCodePatterns = EnableSnippetsAndCodePatterns; +} + +clangd::CodeCompleteOptions::CodeCompleteOptions(bool EnableSnippets, + bool IncludeCodePatterns, + bool IncludeMacros, + bool IncludeGlobals, + bool IncludeBriefComments) + : EnableSnippets(EnableSnippets), IncludeCodePatterns(IncludeCodePatterns), + IncludeMacros(IncludeMacros), IncludeGlobals(IncludeGlobals), + IncludeBriefComments(IncludeBriefComments) {} + +clang::CodeCompleteOptions clangd::CodeCompleteOptions::getClangCompleteOpts() { + clang::CodeCompleteOptions Result; + Result.IncludeCodePatterns = EnableSnippets && IncludeCodePatterns; + Result.IncludeMacros = IncludeMacros; + Result.IncludeGlobals = IncludeGlobals; + Result.IncludeBriefComments = IncludeBriefComments; + + return Result; +} + std::vector clangd::codeComplete(PathRef FileName, tooling::CompileCommand Command, PrecompiledPreamble const *Preamble, StringRef Contents, Position Pos, IntrusiveRefCntPtr VFS, std::shared_ptr PCHs, - bool SnippetCompletions, clangd::Logger &Logger) { + clangd::CodeCompleteOptions Opts, clangd::Logger &Logger) { std::vector Results; - CodeCompleteOptions Options; std::unique_ptr Consumer; - Options.IncludeGlobals = true; - Options.IncludeMacros = true; - Options.IncludeBriefComments = true; - if (SnippetCompletions) { - Options.IncludeCodePatterns = true; - Consumer = - llvm::make_unique(Options, Results); + clang::CodeCompleteOptions ClangCompleteOpts = Opts.getClangCompleteOpts(); + if (Opts.EnableSnippets) { + Consumer = llvm::make_unique( + ClangCompleteOpts, Results); } else { - Options.IncludeCodePatterns = false; - Consumer = - llvm::make_unique(Options, Results); + Consumer = llvm::make_unique( + ClangCompleteOpts, Results); } - invokeCodeComplete(std::move(Consumer), Options, FileName, Command, Preamble, - Contents, Pos, std::move(VFS), std::move(PCHs), Logger); + invokeCodeComplete(std::move(Consumer), ClangCompleteOpts, FileName, Command, + Preamble, Contents, Pos, std::move(VFS), std::move(PCHs), + Logger); return Results; } @@ -807,7 +832,7 @@ std::shared_ptr PCHs, clangd::Logger &Logger) { SignatureHelp Result; - CodeCompleteOptions Options; + clang::CodeCompleteOptions Options; Options.IncludeGlobals = false; Options.IncludeMacros = false; Options.IncludeCodePatterns = false; Index: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp @@ -135,6 +135,33 @@ namespace clangd { namespace { +struct StringWithPos { + std::string Text; + clangd::Position MarkerPos; +}; + +/// Returns location of "{mark}" substring in \p Text and removes it from \p +/// Text. Note that \p Text must contain exactly one occurence of "{mark}". +/// +/// Marker name can be configured using \p MarkerName parameter. +StringWithPos parseTextMarker(StringRef Text, StringRef MarkerName = "mark") { + SmallString<16> Marker; + Twine("{" + MarkerName + "}").toVector(/*ref*/ Marker); + + std::size_t MarkerOffset = Text.find(Marker); + assert(MarkerOffset != StringRef::npos && "{mark} wasn't found in Text."); + + std::string WithoutMarker; + WithoutMarker += Text.take_front(MarkerOffset); + WithoutMarker += Text.drop_front(MarkerOffset + Marker.size()); + assert(StringRef(WithoutMarker).find(Marker) == StringRef::npos && + "There were multiple occurences of {mark} inside Text"); + + clangd::Position MarkerPos = + clangd::offsetToPosition(WithoutMarker, MarkerOffset); + return {std::move(WithoutMarker), MarkerPos}; +} + // Don't wait for async ops in clangd test more than that to avoid blocking // indefinitely in case of bugs. static const std::chrono::seconds DefaultFutureTimeout = @@ -305,7 +332,7 @@ ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*SnippetCompletions=*/false, + clangd::CodeCompleteOptions(), EmptyLogger::getInstance()); for (const auto &FileWithContents : ExtraFiles) FS.Files[getVirtualTestFilePath(FileWithContents.first)] = @@ -369,7 +396,8 @@ ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*SnippetCompletions=*/false, EmptyLogger::getInstance()); + clangd::CodeCompleteOptions(), + EmptyLogger::getInstance()); const auto SourceContents = R"cpp( #include "foo.h" @@ -414,7 +442,8 @@ MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*SnippetCompletions=*/false, EmptyLogger::getInstance()); + clangd::CodeCompleteOptions(), + EmptyLogger::getInstance()); const auto SourceContents = R"cpp( #include "foo.h" @@ -461,7 +490,7 @@ MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); // Run ClangdServer synchronously. ClangdServer Server(CDB, DiagConsumer, FS, - /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false, + /*AsyncThreadsCount=*/0, clangd::CodeCompleteOptions(), EmptyLogger::getInstance()); auto FooCpp = getVirtualTestFilePath("foo.cpp"); @@ -495,7 +524,7 @@ "-stdlib=libstdc++"}); // Run ClangdServer synchronously. ClangdServer Server(CDB, DiagConsumer, FS, - /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false, + /*AsyncThreadsCount=*/0, clangd::CodeCompleteOptions(), EmptyLogger::getInstance()); // Just a random gcc version string @@ -544,7 +573,7 @@ ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, - /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false, + /*AsyncThreadsCount=*/0, clangd::CodeCompleteOptions(), EmptyLogger::getInstance()); // No need to sync reparses, because reparses are performed on the calling // thread to true. @@ -589,13 +618,22 @@ class ClangdCompletionTest : public ClangdVFSTest { protected: - bool ContainsItem(std::vector const &Items, StringRef Name) { + template + bool ContainsItemPred(std::vector const &Items, + Predicate Pred) { for (const auto &Item : Items) { - if (Item.insertText == Name) + if (Pred(Item)) return true; } return false; } + + bool ContainsItem(std::vector const &Items, StringRef Name) { + return ContainsItemPred(Items, [Name](clangd::CompletionItem Item) { + return Item.insertText == Name; + }); + return false; + } }; TEST_F(ClangdCompletionTest, CheckContentsOverride) { @@ -604,7 +642,8 @@ MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*SnippetCompletions=*/false, EmptyLogger::getInstance()); + clangd::CodeCompleteOptions(), + EmptyLogger::getInstance()); auto FooCpp = getVirtualTestFilePath("foo.cpp"); const auto SourceContents = R"cpp( @@ -655,6 +694,166 @@ } } +TEST_F(ClangdCompletionTest, CompletionOptions) { + MockFSProvider FS; + ErrorCheckingDiagConsumer DiagConsumer; + MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); + CDB.ExtraClangFlags.push_back("-xc++"); + + auto FooCpp = getVirtualTestFilePath("foo.cpp"); + FS.Files[FooCpp] = ""; + FS.ExpectedFile = FooCpp; + + const auto GlobalCompletionSourceTemplate = R"cpp( +#define MACRO X + +int global_var; +int global_func(); + +struct GlobalClass {}; + +struct ClassWithMembers { + /// Doc for method. + int method(); +}; + +int test() { + struct LocalClass {}; + + /// Doc for local_var. + int local_var; + + {complete} +} +)cpp"; + const auto MemberCompletionSourceTemplate = R"cpp( +#define MACRO X + +int global_var; + +int global_func(); + +struct GlobalClass {}; + +struct ClassWithMembers { + /// Doc for method. + int method(); + + int field; +}; + +int test() { + struct LocalClass {}; + + /// Doc for local_var. + int local_var; + + ClassWithMembers().{complete} +} +)cpp"; + + StringWithPos GlobalCompletion = + parseTextMarker(GlobalCompletionSourceTemplate, "complete"); + StringWithPos MemberCompletion = + parseTextMarker(MemberCompletionSourceTemplate, "complete"); + + auto TestWithOpts = [&](clangd::CodeCompleteOptions Opts) { + ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), + Opts, EmptyLogger::getInstance()); + // No need to sync reparses here as there are no asserts on diagnostics (or + // other async operations). + Server.addDocument(FooCpp, GlobalCompletion.Text); + + StringRef MethodItemText = Opts.EnableSnippets ? "method()" : "method"; + StringRef GlobalFuncItemText = + Opts.EnableSnippets ? "global_func()" : "global_func"; + + /// For after-dot completion we must always get consistent results. + { + auto Results = Server + .codeComplete(FooCpp, MemberCompletion.MarkerPos, + StringRef(MemberCompletion.Text)) + .get() + .Value; + + // Class members. The only items that must be present in after-dor + // completion. + EXPECT_TRUE(ContainsItem(Results, MethodItemText)); + EXPECT_TRUE(ContainsItem(Results, "field")); + // Global items. + EXPECT_FALSE(ContainsItem(Results, "global_var")); + EXPECT_FALSE(ContainsItem(Results, GlobalFuncItemText)); + EXPECT_FALSE(ContainsItem(Results, "GlobalClass")); + // A macro. + EXPECT_FALSE(ContainsItem(Results, "MACRO")); + // Local items. + EXPECT_FALSE(ContainsItem(Results, "LocalClass")); + // There should be no code patterns (aka snippets) in after-dot + // completion. At least there aren't any we're aware of. + EXPECT_FALSE( + ContainsItemPred(Results, [](clangd::CompletionItem const &Item) { + return Item.kind == clangd::CompletionItemKind::Snippet; + })); + // Check documentation. + EXPECT_EQ( + Opts.IncludeBriefComments, + ContainsItemPred(Results, [](clangd::CompletionItem const &Item) { + return !Item.documentation.empty(); + })); + } + // Global completion differs based on the Opts that were passed. + { + auto Results = Server + .codeComplete(FooCpp, GlobalCompletion.MarkerPos, + StringRef(GlobalCompletion.Text)) + .get() + .Value; + + // Class members. Should never be present in global completions. + EXPECT_FALSE(ContainsItem(Results, MethodItemText)); + EXPECT_FALSE(ContainsItem(Results, "field")); + // Global items. + EXPECT_EQ(ContainsItem(Results, "global_var"), Opts.IncludeGlobals); + EXPECT_EQ(ContainsItem(Results, GlobalFuncItemText), Opts.IncludeGlobals); + EXPECT_EQ(ContainsItem(Results, "GlobalClass"), Opts.IncludeGlobals); + // A macro. + EXPECT_EQ(ContainsItem(Results, "MACRO"), Opts.IncludeMacros); + // Local items. Must be present always. + EXPECT_TRUE(ContainsItem(Results, "local_var")); + EXPECT_TRUE(ContainsItem(Results, "LocalClass")); + // FIXME(ibiryukov): snippets have wrong Item.kind now. Reenable this + // check after https://reviews.llvm.org/D38720 makes it in. + // + // Code patterns (aka snippets). + // EXPECT_EQ( + // Opts.IncludeCodePatterns && Opts.EnableSnippets, + // ContainsItemPred(Results, [](clangd::CompletionItem const &Item) { + // return Item.kind == clangd::CompletionItemKind::Snippet; + // })); + + // Check documentation. + EXPECT_EQ( + Opts.IncludeBriefComments, + ContainsItemPred(Results, [](clangd::CompletionItem const &Item) { + return !Item.documentation.empty(); + })); + } + }; + + for (bool IncludeMacros : {true, false}) + for (bool IncludeGlobals : {true, false}) + for (bool IncludeBriefComments : {true, false}) + for (bool EnableSnippets : {true, false}) + for (bool IncludeCodePatterns : {true, false}) { + TestWithOpts(clangd::CodeCompleteOptions( + /*EnableSnippets=*/EnableSnippets, + /*IncludeCodePatterns=*/IncludeCodePatterns, + /*IncludeMacros=*/IncludeMacros, + /*IncludeGlobals=*/IncludeGlobals, + /*IncludeBriefComments=*/IncludeBriefComments)); + } +} + class ClangdThreadingTest : public ClangdVFSTest {}; TEST_F(ClangdThreadingTest, StressTest) { @@ -753,7 +952,7 @@ { MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*SnippetCompletions=*/false, + clangd::CodeCompleteOptions(), EmptyLogger::getInstance()); // Prepare some random distributions for the test. @@ -913,7 +1112,8 @@ MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(), - /*SnippetCompletions=*/false, EmptyLogger::getInstance()); + clangd::CodeCompleteOptions(), + EmptyLogger::getInstance()); auto SourceContents = R"cpp( #include "foo.h" @@ -1038,7 +1238,7 @@ std::move(StartSecondReparsePromise)); MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); - ClangdServer Server(CDB, DiagConsumer, FS, 4, /*SnippetCompletions=*/false, + ClangdServer Server(CDB, DiagConsumer, FS, 4, clangd::CodeCompleteOptions(), EmptyLogger::getInstance()); Server.addDocument(FooCpp, SourceContentsWithErrors); StartSecondReparse.wait();