project: Handle `textDocument/didSave` and `textDocument/didChange` (un)registration and usage correctly (#36441)

Smit Barmase created

Follow-up of https://github.com/zed-industries/zed/pull/35306

This PR contains two changes:

Both changes are inspired from:
https://github.com/microsoft/vscode-languageserver-node/blob/d90a87f9557a0df9142cfb33e251cfa6fe27d970/client/src/common/textSynchronization.ts

1. Handling `textDocument/didSave` and `textDocument/didChange`
registration and unregistration correctly:

```rs
#[derive(Debug, Eq, PartialEq, Clone, Deserialize, Serialize)]
#[serde(untagged)]
pub enum TextDocumentSyncCapability {
    Kind(TextDocumentSyncKind),
    Options(TextDocumentSyncOptions),
}
```

- `textDocument/didSave` dynamic registration contains "includeText"
- `textDocument/didChange` dynamic registration contains "syncKind"

While storing this to Language Server, we use
`TextDocumentSyncCapability::Options` instead of
`TextDocumentSyncCapability::Kind` since it also include
[change](https://github.com/gluon-lang/lsp-types/blob/be7336e92a6ad23f214df19bcdceab17f39531a9/src/lib.rs#L1714-L1717)
field as `TextDocumentSyncCapability::Kind` as well as
[save](https://github.com/gluon-lang/lsp-types/blob/be7336e92a6ad23f214df19bcdceab17f39531a9/src/lib.rs#L1727-L1729)
field as `TextDocumentSyncSaveOptions`. This way while registering or
unregistering both of them, we don't accidentaly mess with other data.

So, if at intialization we end up getting
`TextDocumentSyncCapability::Kind` and we receive any above kind of
dynamic registration, we change `TextDocumentSyncCapability::Kind` to
`TextDocumentSyncCapability::Options` so we can store more data anyway.

2. Modify `include_text` method to only depend on
`TextDocumentSyncSaveOptions`, instead of depending on
`TextDocumentSyncKind`. Idea behind this is,
`TextDocumentSyncSaveOptions` should be responsible for
"textDocument/didSave" notification, and `TextDocumentSyncKind` should
be responsible for "textDocument/didChange", which it already is:
https://github.com/zed-industries/zed/blob/4b79eade1da2f5f7dfa18208cf882c8e6ca8a97f/crates/project/src/lsp_store.rs#L7324-L7331

Release Notes:

- N/A

Change summary

crates/project/src/lsp_store.rs | 72 +++++++++++++++++++++++++++-------
1 file changed, 56 insertions(+), 16 deletions(-)

Detailed changes

crates/project/src/lsp_store.rs 🔗

@@ -11820,8 +11820,28 @@ impl LspStore {
                         .transpose()?
                     {
                         server.update_capabilities(|capabilities| {
+                            let mut sync_options =
+                                Self::take_text_document_sync_options(capabilities);
+                            sync_options.change = Some(sync_kind);
                             capabilities.text_document_sync =
-                                Some(lsp::TextDocumentSyncCapability::Kind(sync_kind));
+                                Some(lsp::TextDocumentSyncCapability::Options(sync_options));
+                        });
+                        notify_server_capabilities_updated(&server, cx);
+                    }
+                }
+                "textDocument/didSave" => {
+                    if let Some(save_options) = reg
+                        .register_options
+                        .and_then(|opts| opts.get("includeText").cloned())
+                        .map(serde_json::from_value::<lsp::TextDocumentSyncSaveOptions>)
+                        .transpose()?
+                    {
+                        server.update_capabilities(|capabilities| {
+                            let mut sync_options =
+                                Self::take_text_document_sync_options(capabilities);
+                            sync_options.save = Some(save_options);
+                            capabilities.text_document_sync =
+                                Some(lsp::TextDocumentSyncCapability::Options(sync_options));
                         });
                         notify_server_capabilities_updated(&server, cx);
                     }
@@ -11973,7 +11993,19 @@ impl LspStore {
                 }
                 "textDocument/didChange" => {
                     server.update_capabilities(|capabilities| {
-                        capabilities.text_document_sync = None;
+                        let mut sync_options = Self::take_text_document_sync_options(capabilities);
+                        sync_options.change = None;
+                        capabilities.text_document_sync =
+                            Some(lsp::TextDocumentSyncCapability::Options(sync_options));
+                    });
+                    notify_server_capabilities_updated(&server, cx);
+                }
+                "textDocument/didSave" => {
+                    server.update_capabilities(|capabilities| {
+                        let mut sync_options = Self::take_text_document_sync_options(capabilities);
+                        sync_options.save = None;
+                        capabilities.text_document_sync =
+                            Some(lsp::TextDocumentSyncCapability::Options(sync_options));
                     });
                     notify_server_capabilities_updated(&server, cx);
                 }
@@ -12001,6 +12033,20 @@ impl LspStore {
 
         Ok(())
     }
+
+    fn take_text_document_sync_options(
+        capabilities: &mut lsp::ServerCapabilities,
+    ) -> lsp::TextDocumentSyncOptions {
+        match capabilities.text_document_sync.take() {
+            Some(lsp::TextDocumentSyncCapability::Options(sync_options)) => sync_options,
+            Some(lsp::TextDocumentSyncCapability::Kind(sync_kind)) => {
+                let mut sync_options = lsp::TextDocumentSyncOptions::default();
+                sync_options.change = Some(sync_kind);
+                sync_options
+            }
+            None => lsp::TextDocumentSyncOptions::default(),
+        }
+    }
 }
 
 // Registration with empty capabilities should be ignored.
@@ -13103,24 +13149,18 @@ async fn populate_labels_for_symbols(
 
 fn include_text(server: &lsp::LanguageServer) -> Option<bool> {
     match server.capabilities().text_document_sync.as_ref()? {
-        lsp::TextDocumentSyncCapability::Kind(kind) => match *kind {
-            lsp::TextDocumentSyncKind::NONE => None,
-            lsp::TextDocumentSyncKind::FULL => Some(true),
-            lsp::TextDocumentSyncKind::INCREMENTAL => Some(false),
-            _ => None,
-        },
-        lsp::TextDocumentSyncCapability::Options(options) => match options.save.as_ref()? {
-            lsp::TextDocumentSyncSaveOptions::Supported(supported) => {
-                if *supported {
-                    Some(true)
-                } else {
-                    None
-                }
-            }
+        lsp::TextDocumentSyncCapability::Options(opts) => match opts.save.as_ref()? {
+            // Server wants didSave but didn't specify includeText.
+            lsp::TextDocumentSyncSaveOptions::Supported(true) => Some(false),
+            // Server doesn't want didSave at all.
+            lsp::TextDocumentSyncSaveOptions::Supported(false) => None,
+            // Server provided SaveOptions.
             lsp::TextDocumentSyncSaveOptions::SaveOptions(save_options) => {
                 Some(save_options.include_text.unwrap_or(false))
             }
         },
+        // We do not have any save info. Kind affects didChange only.
+        lsp::TextDocumentSyncCapability::Kind(_) => None,
     }
 }