[clangd] Isolate logic for setting LSPServer options

[kadircet]

Summary

Moves clangdserveropt gathering into a separate function to make sure
it's done before we take any actions based on options.

This also moves some state that controls how we handle certain requests from
LSPServer to options.

[kadircet]

rename helper & add comment

[sammccall]

I like the change to initialize() a lot.

clang-tools-extra/clangd/ClangdLSPServer.h

64

I don't really like making these options part of this struct.

Currently this struct is largely used to send data from main->ClangdLSPServer.
After this patch, half of the fields are for that and half of them are scratch space for ClangdLSPServer to use itself. Worse, they *look* like things that can/should be set beforehand, but will in fact the passed values are ignored.
(Admittedly, some of the *inherited* fields are used in the same way, sigh).

Additionally, these options are publicly exposed when they don't need to be - these are internals leaking into API without a use case.

---

Having updateServerOptions be a hidden helper rather than a member is nice, but I think not as important as the actual public interface.

Grouping the options is also nice. Having the members be consecutive seems enough to me (they're almost grouped: background queue state is in the wrong place). Adding a second struct also seems like an option though?

[kadircet]

I agree this is creating confusing state for constructors of ClangdLSPServer. It
would be interesting to create LSPServer after the initialize request one day.

• Make client-capability related options private again.

clang-tools-extra/clangd/ClangdLSPServer.h

struct Options : ClangdServer::Options {
     /// Returns true if the tweak should be enabled.
     std::function<bool(const Tweak &)> TweakFilter = [](const Tweak &T) {
       return !T.hidden(); // only enable non-hidden tweaks.
     };
 
     /// Limit the number of references returned (0 means no limit).
     size_t ReferencesLimit = 0;
   };
 
   ClangdLSPServer(Transport &Transp, const ThreadsafeFS &TFS,
                   const ClangdLSPServer::Options &Opts);
   /// The destructor blocks on any outstanding background tasks.
   ~ClangdLSPServer();
 
   /// Run LSP server loop, communicating with the Transport provided in the
   /// constructor. This method must not be executed more than once.
   ///
   /// \return Whether we shut down cleanly with a 'shutdown' -> 'exit' sequence.
   bool run();
 
   /// Profiles resource-usage.
   void profile(MemoryTree &MT) const;
 
 private:
+  // Configure server using client capabilities to adjust response types and
+  // request handling.
+  void alignWithClient(const InitializeParams &Params);
   // Implement ClangdServer::Callbacks.
   void onDiagnosticsReady(PathRef File, llvm::StringRef Version,
                           std::vector<Diag> Diagnostics) override;
   void onFileUpdated(PathRef File, const TUStatus &Status) override;
   void onBackgroundIndexProgress(const BackgroundQueue::Stats &Stats) override;
   void onSemanticsMaybeChanged(PathRef File) override;
 
   // LSP methods. Notifications have signature void(const Params&).
private:
 
   void callMethod(StringRef Method, llvm::json::Value Params,
                   Callback<llvm::json::Value> CB) override;
   void notify(StringRef Method, llvm::json::Value Params) override;
 
   LSPBinder::RawHandlers Handlers;
 
   const ThreadsafeFS &TFS;
-  /// Options used for diagnostics.
-  ClangdDiagnosticOptions DiagOpts;
-  /// The supported kinds of the client.
-  SymbolKindBitset SupportedSymbolKinds;
-  /// The supported completion item kinds of the client.
-  CompletionItemKindBitset SupportedCompletionItemKinds;
-  /// Whether the client supports CodeAction response objects.
-  bool SupportsCodeAction = false;
-  /// From capabilities of textDocument/documentSymbol.
-  bool SupportsHierarchicalDocumentSymbol = false;
-  /// Whether the client supports showing file status.
-  bool SupportFileStatus = false;
-  /// Which kind of markup should we use in textDocument/hover responses.
-  MarkupKind HoverContentFormat = MarkupKind::PlainText;
-  /// Whether the client supports offsets for parameter info labels.
-  bool SupportsOffsetsInSignatureHelp = false;
   std::mutex BackgroundIndexProgressMutex;
   enum class BackgroundIndexProgress {
     // Client doesn't support reporting progress. No transitions possible.
     Unsupported,
     // The queue is idle, and the client has no progress bar.
     // Can transition to Creating when we have some activity.
     Empty,
     // We've requested the client to create a progress bar.
     // Meanwhile, the state is buffered in PendingBackgroundIndexProgress.
     Creating,
     // The client has a progress bar, and we can send it updates immediately.
     Live,
   } BackgroundIndexProgressState = BackgroundIndexProgress::Unsupported;
   // The progress to send when the progress bar is created.
   // Only valid in state Creating.
   BackgroundQueue::Stats PendingBackgroundIndexProgress;
-  /// LSP extension: skip WorkDoneProgressCreate, just send progress streams.
-  bool BackgroundIndexSkipCreate = false;
 
   Options Opts;
+  struct {
+    // Options used for diagnostics.
+    ClangdDiagnosticOptions DiagOpts;
+    // Set of symbol kinds to use.
+    SymbolKindBitset SymbolKinds;
+    // Set of completion item kinds to use.
+    CompletionItemKindBitset CompletionItemKinds;
+    // Whether to emit code actions or commands as code action responses.
+    bool EmitCodeAction = false;
+    // Whether to emit hierarchical or flat document symbol responses.
+    bool HierarchicalDocumentSymbol = false;
+    // Whether to send file status updates.
+    bool EmitFileStatus = false;
+    // Which kind of markup should we use in textDocument/hover responses.
+    MarkupKind HoverContentFormat = MarkupKind::PlainText;
+    // Whether to have offsets for parameter info labels.
+    bool OffsetsInSignatureHelp = false;
+    // LSP extension: skip WorkDoneProgressCreate, just send progress streams.
+    bool BackgroundIndexSkipCreate = false;
+  } ClientCapabilities;
   // The CDB is created by the "initialize" LSP method.
   std::unique_ptr<GlobalCompilationDatabase> BaseCDB;
   // CDB is BaseCDB plus any commands overridden via LSP extensions.
   llvm::Optional<OverlayCDB> CDB;
   // The ClangdServer is created by the "initialize" LSP method.
   llvm::Optional<ClangdServer> Server;
 };
 } // namespace clangd
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_CLANGDLSPSERVER_H

clang-tools-extra/clangd/ClangdLSPServer.cpp

static std::vector<llvm::StringRef> semanticTokenModifiers() {
   std::vector<llvm::StringRef> Modifiers;
   for (unsigned I = 0;
        I <= static_cast<unsigned>(HighlightingModifier::LastModifier); ++I)
     Modifiers.push_back(
         toSemanticTokenModifier(static_cast<HighlightingModifier>(I)));
   return Modifiers;
 }
 
-void ClangdLSPServer::onInitialize(const InitializeParams &Params,
-                                   Callback<llvm::json::Value> Reply) {
+void ClangdLSPServer::alignWithClient(const InitializeParams &Params) {
   // Determine character encoding first as it affects constructed ClangdServer.
   if (Params.capabilities.offsetEncoding && !Opts.Encoding) {
     Opts.Encoding = OffsetEncoding::UTF16; // fallback
     for (OffsetEncoding Supported : *Params.capabilities.offsetEncoding)
       if (Supported != OffsetEncoding::UnsupportedEncoding) {
         Opts.Encoding = Supported;
         break;
       }
   }
 
   if (Params.capabilities.TheiaSemanticHighlighting &&
       !Params.capabilities.SemanticTokens) {
     elog("Client requested legacy semanticHighlights notification, which is "
          "no longer supported. Migrate to standard semanticTokens request");
   }
 
   if (Params.rootUri && *Params.rootUri)
     Opts.WorkspaceRoot = std::string(Params.rootUri->file());
   else if (Params.rootPath && !Params.rootPath->empty())
     Opts.WorkspaceRoot = *Params.rootPath;
-  if (Server)
-    return Reply(llvm::make_error<LSPError>("server already initialized",
-                                            ErrorCode::InvalidRequest));
 
   Opts.CodeComplete.EnableSnippets = Params.capabilities.CompletionSnippets;
   Opts.CodeComplete.IncludeFixIts = Params.capabilities.CompletionFixes;
   if (!Opts.CodeComplete.BundleOverloads)
     Opts.CodeComplete.BundleOverloads = Params.capabilities.HasSignatureHelp;
   Opts.CodeComplete.DocumentationFormat =
       Params.capabilities.CompletionDocumentationFormat;
   Opts.SignatureHelpDocumentationFormat =
       Params.capabilities.SignatureHelpDocumentationFormat;
-  DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.DiagnosticFixes;
-  DiagOpts.SendDiagnosticCategory = Params.capabilities.DiagnosticCategory;
-  DiagOpts.EmitRelatedLocations =
+  Opts.LineFoldingOnly = Params.capabilities.LineFoldingOnly;
+  Opts.ImplicitCancellation = !Params.capabilities.CancelsStaleRequests;
+
+  ClientCapabilities.DiagOpts.EmbedFixesInDiagnostics =
+      Params.capabilities.DiagnosticFixes;
+  ClientCapabilities.DiagOpts.SendDiagnosticCategory =
+      Params.capabilities.DiagnosticCategory;
+  ClientCapabilities.DiagOpts.EmitRelatedLocations =
       Params.capabilities.DiagnosticRelatedInformation;
   if (Params.capabilities.WorkspaceSymbolKinds)
-    SupportedSymbolKinds |= *Params.capabilities.WorkspaceSymbolKinds;
+    ClientCapabilities.SymbolKinds |= *Params.capabilities.WorkspaceSymbolKinds;
   if (Params.capabilities.CompletionItemKinds)
-    SupportedCompletionItemKinds |= *Params.capabilities.CompletionItemKinds;
-  SupportsCodeAction = Params.capabilities.CodeActionStructure;
-  SupportsHierarchicalDocumentSymbol =
+    ClientCapabilities.CompletionItemKinds |=
+        *Params.capabilities.CompletionItemKinds;
+  ClientCapabilities.EmitCodeAction = Params.capabilities.CodeActionStructure;
+  ClientCapabilities.HierarchicalDocumentSymbol =
       Params.capabilities.HierarchicalDocumentSymbol;
-  SupportFileStatus = Params.initializationOptions.FileStatus;
-  HoverContentFormat = Params.capabilities.HoverContentFormat;
-  Opts.LineFoldingOnly = Params.capabilities.LineFoldingOnly;
-  SupportsOffsetsInSignatureHelp = Params.capabilities.OffsetsInSignatureHelp;
+  ClientCapabilities.EmitFileStatus = Params.initializationOptions.FileStatus;
+  ClientCapabilities.HoverContentFormat =
+      Params.capabilities.HoverContentFormat;
+  ClientCapabilities.OffsetsInSignatureHelp =
+      Params.capabilities.OffsetsInSignatureHelp;
+  ClientCapabilities.BackgroundIndexSkipCreate =
+      Params.capabilities.ImplicitProgressCreation;
+}
+
+void ClangdLSPServer::onInitialize(const InitializeParams &Params,
+                                   Callback<llvm::json::Value> Reply) {
+  if (Server)
+    return Reply(llvm::make_error<LSPError>("server already initialized",
+                                            ErrorCode::InvalidRequest));
+  alignWithClient(Params);
   if (Params.capabilities.WorkDoneProgress)
     BackgroundIndexProgressState = BackgroundIndexProgress::Empty;
-  BackgroundIndexSkipCreate = Params.capabilities.ImplicitProgressCreation;
-  Opts.ImplicitCancellation = !Params.capabilities.CancelsStaleRequests;
 
   if (Opts.UseDirBasedCDB) {
     DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS);
     if (const auto &Dir = Params.initializationOptions.compilationDatabasePath)
       CDBOpts.CompileCommandsDir = Dir;
     CDBOpts.ContextProvider = Opts.ContextProvider;
     BaseCDB =
         std::make_unique<DirectoryBasedGlobalCompilationDatabase>(CDBOpts);
void ClangdLSPServer::onInitialize(const InitializeParams &Params,
   ServerCaps["codeActionProvider"] =
       Params.capabilities.CodeActionStructure
           ? llvm::json::Object{{"codeActionKinds",
                                 {CodeAction::QUICKFIX_KIND,
                                  CodeAction::REFACTOR_KIND,
                                  CodeAction::INFO_KIND}}}
           : llvm::json::Value(true);
 
-
   std::vector<llvm::StringRef> Commands;
   for (llvm::StringRef Command : Handlers.CommandHandlers.keys())
     Commands.push_back(Command);
   llvm::sort(Commands);
   ServerCaps["executeCommandProvider"] =
       llvm::json::Object{{"commands", Commands}};
 
   llvm::json::Object Result{
void ClangdLSPServer::onWorkspaceSymbol(
     Callback<std::vector<SymbolInformation>> Reply) {
   Server->workspaceSymbols(
       Params.query, Params.limit.value_or(Opts.CodeComplete.Limit),
       [Reply = std::move(Reply),
        this](llvm::Expected<std::vector<SymbolInformation>> Items) mutable {
         if (!Items)
           return Reply(Items.takeError());
         for (auto &Sym : *Items)
-          Sym.kind = adjustKindToCapability(Sym.kind, SupportedSymbolKinds);
+          Sym.kind = adjustKindToCapability(Sym.kind, Opts.SymbolKinds);
 
         Reply(std::move(*Items));
       });
 }
 
 void ClangdLSPServer::onPrepareRename(const TextDocumentPositionParams &Params,
                                       Callback<llvm::Optional<Range>> Reply) {
   Server->prepareRename(
void ClangdLSPServer::onDocumentSymbol(const DocumentSymbolParams &Params,
                                        Callback<llvm::json::Value> Reply) {
   URIForFile FileURI = Params.textDocument.uri;
   Server->documentSymbols(
       Params.textDocument.uri.file(),
       [this, FileURI, Reply = std::move(Reply)](
           llvm::Expected<std::vector<DocumentSymbol>> Items) mutable {
         if (!Items)
           return Reply(Items.takeError());
-        adjustSymbolKinds(*Items, SupportedSymbolKinds);
-        if (SupportsHierarchicalDocumentSymbol)
+        adjustSymbolKinds(*Items, Opts.SymbolKinds);
+        if (Opts.HierarchicalDocumentSymbol)
           return Reply(std::move(*Items));
         return Reply(flattenSymbolHierarchy(*Items, FileURI));
       });
 }
 
 void ClangdLSPServer::onFoldingRange(
     const FoldingRangeParams &Params,
     Callback<std::vector<FoldingRange>> Reply) {
auto ConsumeActions =
           }
         }
         if (OnlyFix) {
           OnlyFix->isPreferred = true;
           if (Diags.size() == 1 && Diags.front().range == Selection)
             OnlyFix->diagnostics = {Diags.front()};
         }
 
-        if (SupportsCodeAction)
+        if (Opts.EmitCodeAction)
           return Reply(llvm::json::Array(Actions));
         std::vector<Command> Commands;
         for (const auto &Action : Actions) {
           if (auto Command = asCommand(Action))
             Commands.push_back(std::move(*Command));
         }
         return Reply(llvm::json::Array(Commands));
       };
Server->codeComplete(Params.textDocument.uri.file(), Params.position, Opts,
                         this](llvm::Expected<CodeCompleteResult> List) mutable {
                          if (!List)
                            return Reply(List.takeError());
                          CompletionList LSPList;
                          LSPList.isIncomplete = List->HasMore;
                          for (const auto &R : List->Completions) {
                            CompletionItem C = R.render(Opts);
                            C.kind = adjustKindToCapability(
-                               C.kind, SupportedCompletionItemKinds);
+                               C.kind, this->Opts.CompletionItemKinds);
                            LSPList.items.push_back(std::move(C));
                          }
                          return Reply(std::move(LSPList));
                        });
 }
 
 void ClangdLSPServer::onSignatureHelp(const TextDocumentPositionParams &Params,
                                       Callback<SignatureHelp> Reply) {
   Server->signatureHelp(Params.textDocument.uri.file(), Params.position,
                         Opts.SignatureHelpDocumentationFormat,
                         [Reply = std::move(Reply), this](
                             llvm::Expected<SignatureHelp> Signature) mutable {
                           if (!Signature)
                             return Reply(Signature.takeError());
-                          if (SupportsOffsetsInSignatureHelp)
+                          if (Opts.OffsetsInSignatureHelp)
                             return Reply(std::move(*Signature));
                           // Strip out the offsets from signature help for
                           // clients that only support string labels.
                           for (auto &SigInfo : Signature->signatures) {
                             for (auto &Param : SigInfo.parameters)
                               Param.labelOffsets.reset();
                           }
                           return Reply(std::move(*Signature));
Server->findHover(Params.textDocument.uri.file(), Params.position,
                     [Reply = std::move(Reply), this](
                         llvm::Expected<llvm::Optional<HoverInfo>> H) mutable {
                       if (!H)
                         return Reply(H.takeError());
                       if (!*H)
                         return Reply(llvm::None);
 
                       Hover R;
-                      R.contents.kind = HoverContentFormat;
+                      R.contents.kind = Opts.HoverContentFormat;
                       R.range = (*H)->SymRange;
-                      switch (HoverContentFormat) {
+                      switch (Opts.HoverContentFormat) {
                       case MarkupKind::PlainText:
                         R.contents.value = (*H)->present().asPlainText();
                         return Reply(std::move(R));
                       case MarkupKind::Markdown:
                         R.contents.value = (*H)->present().asMarkdown();
                         return Reply(std::move(R));
                       };
                       llvm_unreachable("unhandled MarkupKind");

 
 ClangdLSPServer::ClangdLSPServer(Transport &Transp, const ThreadsafeFS &TFS,
                                  const ClangdLSPServer::Options &Opts)
     : ShouldProfile(/*Period=*/std::chrono::minutes(5),
                     /*Delay=*/std::chrono::minutes(1)),
       ShouldCleanupMemory(/*Period=*/std::chrono::minutes(1),
                           /*Delay=*/std::chrono::minutes(1)),
       BackgroundContext(Context::current().clone()), Transp(Transp),
-      MsgHandler(new MessageHandler(*this)), TFS(TFS),
-      SupportedSymbolKinds(defaultSymbolKinds()),
-      SupportedCompletionItemKinds(defaultCompletionItemKinds()), Opts(Opts) {
+      MsgHandler(new MessageHandler(*this)), TFS(TFS), Opts(Opts) {
+  this->Opts.SymbolKinds = defaultSymbolKinds();
+  this->Opts.CompletionItemKinds = defaultCompletionItemKinds();
   if (Opts.ConfigProvider) {
     assert(!Opts.ContextProvider &&
            "Only one of ConfigProvider and ContextProvider allowed!");
     this->Opts.ContextProvider = ClangdServer::createConfiguredContextProvider(
         Opts.ConfigProvider, this);
   }
   LSPBinder Bind(this->Handlers, *this);
   Bind.method("initialize", this, &ClangdLSPServer::onInitialize);

 
 void ClangdLSPServer::onDiagnosticsReady(PathRef File, llvm::StringRef Version,
                                          std::vector<Diag> Diagnostics) {
   PublishDiagnosticsParams Notification;
   Notification.version = decodeVersion(Version);
   Notification.uri = URIForFile::canonicalize(File, /*TUPath=*/File);
   DiagnosticToReplacementMap LocalFixIts; // Temporary storage
   for (auto &Diag : Diagnostics) {
-    toLSPDiags(Diag, Notification.uri, DiagOpts,
+    toLSPDiags(Diag, Notification.uri, Opts.DiagOpts,
                [&](clangd::Diagnostic Diag, llvm::ArrayRef<Fix> Fixes) {
                  auto &FixItsForDiagnostic = LocalFixIts[Diag];
                  llvm::copy(Fixes, std::back_inserter(FixItsForDiagnostic));
                  Notification.diagnostics.push_back(std::move(Diag));
                });
   }
 
   // Cache FixIts
void ClangdLSPServer::onBackgroundIndexProgress(
   switch (BackgroundIndexProgressState) {
   case BackgroundIndexProgress::Unsupported:
     return;
   case BackgroundIndexProgress::Creating:
     // Cache this update for when the progress bar is available.
     PendingBackgroundIndexProgress = Stats;
     return;
   case BackgroundIndexProgress::Empty: {
-    if (BackgroundIndexSkipCreate) {
+    if (Opts.BackgroundIndexSkipCreate) {
       NotifyProgress(Stats);
       break;
     }
     // Cache this update for when the progress bar is available.
     PendingBackgroundIndexProgress = Stats;
     BackgroundIndexProgressState = BackgroundIndexProgress::Creating;
     WorkDoneProgressCreateParams CreateRequest;
     CreateRequest.token = ProgressToken;
void ClangdLSPServer::onBackgroundIndexProgress(
   }
   case BackgroundIndexProgress::Live:
     NotifyProgress(Stats);
     break;
   }
 }
 
 void ClangdLSPServer::onFileUpdated(PathRef File, const TUStatus &Status) {
-  if (!SupportFileStatus)
+  if (!Opts.EmitFileStatus)
     return;
   // FIXME: we don't emit "BuildingFile" and `RunningAction`, as these
   // two statuses are running faster in practice, which leads the UI constantly
   // changing, and doesn't provide much value. We may want to emit status at a
   // reasonable time interval (e.g. 0.5s).
   if (Status.PreambleActivity == PreambleAction::Idle &&
       (Status.ASTActivity.K == ASTAction::Building ||
        Status.ASTActivity.K == ASTAction::RunningAction))