From bc90461818e252ef6a4c276d54c24b2b4f67f27e Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Thu, 25 Oct 2018 04:22:52 +0000 Subject: [PATCH] [clangd] Clean up LSP structs around configuration. NFC, no protocol changes. - align struct names/comments with LSP, remove redundant "clangd" prefixes. - don't map config structs as Optional<> when their presence/absence doesn't signal anything and all fields must have sensible "absent" values - be more lax around parsing of 'any'-typed messages llvm-svn: 345235 --- clang-tools-extra/clangd/ClangdLSPServer.cpp | 38 +++++++++++++--------------- clang-tools-extra/clangd/ClangdLSPServer.h | 2 +- clang-tools-extra/clangd/Protocol.cpp | 22 ++++++++-------- clang-tools-extra/clangd/Protocol.h | 32 +++++++++++------------ 4 files changed, 45 insertions(+), 49 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 6769d38..f08d7b0 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -303,15 +303,14 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params, if (Server) return Reply(make_error("server already initialized", ErrorCode::InvalidRequest)); - if (Params.initializationOptions) - CompileCommandsDir = Params.initializationOptions->compilationDatabasePath; + if (const auto &Dir = Params.initializationOptions.compilationDatabasePath) + CompileCommandsDir = Dir; CDB.emplace(UseInMemoryCDB ? CompilationDB::makeInMemory() : CompilationDB::makeDirectoryBased(CompileCommandsDir)); Server.emplace(CDB->getCDB(), FSProvider, static_cast(*this), ClangdServerOpts); - if (Params.initializationOptions) - applyConfiguration(Params.initializationOptions->ParamsChange); + applyConfiguration(Params.initializationOptions.ConfigSettings); CCOpts.EnableSnippets = Params.capabilities.CompletionSnippets; DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.DiagnosticFixes; @@ -654,25 +653,22 @@ void ClangdLSPServer::onHover(const TextDocumentPositionParams &Params, } void ClangdLSPServer::applyConfiguration( - const ClangdConfigurationParamsChange &Params) { + const ConfigurationSettings &Settings) { // Per-file update to the compilation database. - if (Params.compilationDatabaseChanges) { - const auto &CompileCommandUpdates = *Params.compilationDatabaseChanges; - bool ShouldReparseOpenFiles = false; - for (auto &Entry : CompileCommandUpdates) { - /// The opened files need to be reparsed only when some existing - /// entries are changed. - PathRef File = Entry.first; - if (!CDB->setCompilationCommandForFile( - File, tooling::CompileCommand( - std::move(Entry.second.workingDirectory), File, - std::move(Entry.second.compilationCommand), - /*Output=*/""))) - ShouldReparseOpenFiles = true; - } - if (ShouldReparseOpenFiles) - reparseOpenedFiles(); + bool ShouldReparseOpenFiles = false; + for (auto &Entry : Settings.compilationDatabaseChanges) { + /// The opened files need to be reparsed only when some existing + /// entries are changed. + PathRef File = Entry.first; + if (!CDB->setCompilationCommandForFile( + File, tooling::CompileCommand( + std::move(Entry.second.workingDirectory), File, + std::move(Entry.second.compilationCommand), + /*Output=*/""))) + ShouldReparseOpenFiles = true; } + if (ShouldReparseOpenFiles) + reparseOpenedFiles(); } // FIXME: This function needs to be properly tested. diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h index c207ebc..089c218 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -93,7 +93,7 @@ private: /// may be very expensive. This method is normally called when the /// compilation database is changed. void reparseOpenedFiles(); - void applyConfiguration(const ClangdConfigurationParamsChange &Settings); + void applyConfiguration(const ConfigurationSettings &Settings); /// Used to indicate that the 'shutdown' request was received from the /// Language Server client. diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp index d4892682..1bb5961 100644 --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -663,20 +663,22 @@ bool fromJSON(const json::Value &Params, ClangdCompileCommand &CDbUpdate) { O.map("compilationCommand", CDbUpdate.compilationCommand); } -bool fromJSON(const json::Value &Params, - ClangdConfigurationParamsChange &CCPC) { +bool fromJSON(const json::Value &Params, ConfigurationSettings &S) { json::ObjectMapper O(Params); - return O && - O.map("compilationDatabaseChanges", CCPC.compilationDatabaseChanges); + if (!O) + return true; // 'any' type in LSP. + O.map("compilationDatabaseChanges", S.compilationDatabaseChanges); + return true; } -bool fromJSON(const json::Value &Params, ClangdInitializationOptions &Opts) { - if (!fromJSON(Params, Opts.ParamsChange)) { - return false; - } - +bool fromJSON(const json::Value &Params, InitializationOptions &Opts) { json::ObjectMapper O(Params); - return O && O.map("compilationDatabasePath", Opts.compilationDatabasePath); + if (!O) + return true; // 'any' type in LSP. + + fromJSON(Params, Opts.ConfigSettings); + O.map("compilationDatabasePath", Opts.compilationDatabasePath); + return true; } bool fromJSON(const json::Value &Params, ReferenceParams &R) { diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 079873e..3035249 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -355,25 +355,26 @@ struct ClangdCompileCommand { }; bool fromJSON(const llvm::json::Value &, ClangdCompileCommand &); -/// Clangd extension to set clangd-specific "initializationOptions" in the -/// "initialize" request and for the "workspace/didChangeConfiguration" -/// notification since the data received is described as 'any' type in LSP. -struct ClangdConfigurationParamsChange { - // The changes that happened to the compilation database. +/// Clangd extension: parameters configurable at any time, via the +/// `workspace/didChangeConfiguration` notification. +/// LSP defines this type as `any`. +struct ConfigurationSettings { + // Changes to the in-memory compilation database. // The key of the map is a file name. - llvm::Optional> - compilationDatabaseChanges; + std::map compilationDatabaseChanges; }; -bool fromJSON(const llvm::json::Value &, ClangdConfigurationParamsChange &); +bool fromJSON(const llvm::json::Value &, ConfigurationSettings &); -struct ClangdInitializationOptions { +/// Clangd extension: parameters configurable at `initialize` time. +/// LSP defines this type as `any`. +struct InitializationOptions { // What we can change throught the didChangeConfiguration request, we can // also set through the initialize request (initializationOptions field). - ClangdConfigurationParamsChange ParamsChange; + ConfigurationSettings ConfigSettings; llvm::Optional compilationDatabasePath; }; -bool fromJSON(const llvm::json::Value &, ClangdInitializationOptions &); +bool fromJSON(const llvm::json::Value &, InitializationOptions &); struct InitializeParams { /// The process Id of the parent process that started @@ -402,9 +403,8 @@ struct InitializeParams { /// The initial trace setting. If omitted trace is disabled ('off'). llvm::Optional trace; - // We use this predefined struct because it is easier to use - // than the protocol specified type of 'any'. - llvm::Optional initializationOptions; + /// User-provided initialization options. + InitializationOptions initializationOptions; }; bool fromJSON(const llvm::json::Value &, InitializeParams &); @@ -477,9 +477,7 @@ struct DidChangeWatchedFilesParams { bool fromJSON(const llvm::json::Value &, DidChangeWatchedFilesParams &); struct DidChangeConfigurationParams { - // We use this predefined struct because it is easier to use - // than the protocol specified type of 'any'. - ClangdConfigurationParamsChange settings; + ConfigurationSettings settings; }; bool fromJSON(const llvm::json::Value &, DidChangeConfigurationParams &); -- 2.7.4