Skip to content

Commit 1f29a04

Browse files
authored
[ty] Support LSP client settings (#19614)
## Summary This PR implements support for providing LSP client settings. The complementary PR in the ty VS Code extension: astral-sh/ty-vscode#106. Notes for the previous iteration of this PR is in #19614 (comment) (click on "Details"). Specifically, this PR splits the client settings into 3 distinct groups. Keep in mind that these groups are not visible to the user, they're merely an implementation detail. The groups are: 1. `GlobalOptions` - these are the options that are global to the language server and will be the same for all the workspaces that are handled by the server 2. `WorkspaceOptions` - these are the options that are specific to a workspace and will be applied only when running any logic for that workspace 3. `InitializationOptions` - these are the options that can be specified during initialization The initialization options are a superset that contains both the global and workspace options flattened into a 1-dimensional structure. This means that the user can specify any and all fields present in `GlobalOptions` and `WorkspaceOptions` in the initialization options in addition to the fields that are _specific_ to initialization options. From the current set of available settings, following are only available during initialization because they are required at that time, are static during the runtime of the server and changing their values require a restart to take effect: - `logLevel` - `logFile` And, following are available under `GlobalOptions`: - `diagnosticMode` And, following under `WorkspaceOptions`: - `disableLanguageServices` - `pythonExtension` (Python environment information that is populated by the ty VS Code extension) ### `workspace/configuration` This request allows server to ask the client for configuration to a specific workspace. But, this is only supported by the client that has the `workspace.configuration` client capability set to `true`. What to do for clients that don't support pulling configurations? In that case, the settings needs to be provided in the initialization options and updating the values of those settings can only be done by restarting the server. With the way this is implemented, this means that if the client does not support pulling workspace configuration then there's no way to specify settings specific to a workspace. Earlier, this would've been possible by providing an array of client options with an additional field which specifies which workspace the options belong to but that adds complexity and clients that actually do not support `workspace/configuration` would usually not support multiple workspaces either. Now, for the clients that do support this, the server will initiate the request to get the configuration for all the workspaces at the start of the server. Once the server receives these options, it will resolve them for each workspace as follows: 1. Combine the client options sent during initialization with the options specific to the workspace creating the final client options that's specific to this workspace 2. Create a global options by combining the global options from (1) for all workspaces which in turn will also combine the global options sent during initialization The global options are resolved into the global settings and are available on the `Session` which is initialized with the default global settings. The workspace options are resolved into the workspace settings and are available on the respective `Workspace`. The `SessionSnapshot` contains the global settings while the document snapshot contains the workspace settings. We could add the global settings to the document snapshot but that's currently not needed. ### Document diagnostic dynamic registration Currently, the document diagnostic server capability is created based on the `diagnosticMode` sent during initialization. But, that wouldn't provide us with the complete picture. This means the server needs to defer registering the document diagnostic capability at a later point once the settings have been resolved. This is done using dynamic registration for clients that support it. For clients that do not support dynamic registration for document diagnostic capability, the server advertises itself as always supporting workspace diagnostics and work done progress token. This dynamic registration now allows us to change the server capability for workspace diagnostics based on the resolved `diagnosticMode` value. In the future, once `workspace/didChangeConfiguration` is supported, we can avoid the server restart when users have changed any client settings. ## Test Plan Add integration tests and recorded videos on the user experience in various editors: ### VS Code For VS Code users, the settings experience is unchanged because the extension defines it's own interface on how the user can specify the server setting. This means everything is under the `ty.*` namespace as usual. https://github.com/user-attachments/assets/c2e5ba5c-7617-406e-a09d-e397ce9c3b93 ### Zed For Zed, the settings experience has changed. Users can specify settings during initialization: ```json { "lsp": { "ty": { "initialization_options": { "logLevel": "debug", "logFile": "~/.cache/ty.log", "diagnosticMode": "workspace", "disableLanguageServices": true } }, } } ``` Or, can specify the options under the `settings` key: ```json { "lsp": { "ty": { "settings": { "ty": { "diagnosticMode": "openFilesOnly", "disableLanguageServices": true } }, "initialization_options": { "logLevel": "debug", "logFile": "~/.cache/ty.log" } }, } } ``` The `logLevel` and `logFile` setting still needs to go under the initialization options because they're required by the server during initialization. We can remove the nesting of the settings under the "ty" namespace by updating the return type of https://github.com/zed-extensions/ty/blob/db9ea0cdfd7352529748ef5bf729a152c0219805/src/tychecker.rs#L45-L49 to be wrapped inside `ty` directly so that users can avoid doing the double nesting. There's one issue here which is that if the `diagnosticMode` is specified in both the initialization option and settings key, then the resolution is a bit different - if either of them is set to be `workspace`, then it wins which means that in the following configuration, the diagnostic mode is `workspace`: ```json { "lsp": { "ty": { "settings": { "ty": { "diagnosticMode": "openFilesOnly" } }, "initialization_options": { "diagnosticMode": "workspace" } }, } } ``` This behavior is mainly a result of combining global options from various workspace configuration results. Users should not be able to provide global options in multiple workspaces but that restriction cannot be done on the server side. The ty VS Code extension restricts these global settings to only be set in the user settings and not in workspace settings but we do not control extensions in other editors. https://github.com/user-attachments/assets/8e2d6c09-18e6-49e5-ab78-6cf942fe1255 ### Neovim Same as in Zed. ### Other Other editors that do not support `workspace/configuration`, the users would need to provide the server settings during initialization.
1 parent 529d81d commit 1f29a04

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+1220
-543
lines changed

Cargo.lock

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ ruff_text_size = { path = "crates/ruff_text_size" }
4040
ruff_workspace = { path = "crates/ruff_workspace" }
4141

4242
ty = { path = "crates/ty" }
43+
ty_combine = { path = "crates/ty_combine" }
4344
ty_ide = { path = "crates/ty_ide" }
4445
ty_project = { path = "crates/ty_project", default-features = false }
4546
ty_python_semantic = { path = "crates/ty_python_semantic" }

crates/ruff_macros/src/combine.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,14 @@ pub(crate) fn derive_impl(input: DeriveInput) -> syn::Result<proc_macro2::TokenS
1212
.map(|member| {
1313

1414
quote_spanned!(
15-
member.span() => crate::combine::Combine::combine_with(&mut self.#member, other.#member)
15+
member.span() => ty_combine::Combine::combine_with(&mut self.#member, other.#member)
1616
)
1717
})
1818
.collect();
1919

2020
Ok(quote! {
2121
#[automatically_derived]
22-
impl crate::combine::Combine for #ident {
22+
impl ty_combine::Combine for #ident {
2323
#[allow(deprecated)]
2424
fn combine_with(&mut self, other: Self) {
2525
#(

crates/ruff_macros/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ pub fn derive_combine_options(input: TokenStream) -> TokenStream {
4747
.into()
4848
}
4949

50-
/// Automatically derives a `ty_project::project::Combine` implementation for the attributed type
51-
/// that calls `ty_project::project::Combine::combine` for each field.
50+
/// Automatically derives a `ty_combine::Combine` implementation for the attributed type
51+
/// that calls `ty_combine::Combine::combine` for each field.
5252
///
5353
/// The derive macro can only be used on structs. Enums aren't yet supported.
5454
#[proc_macro_derive(Combine)]

crates/ty/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ license.workspace = true
1616
[dependencies]
1717
ruff_db = { workspace = true, features = ["os", "cache"] }
1818
ruff_python_ast = { workspace = true }
19+
ty_combine = { workspace = true }
1920
ty_python_semantic = { workspace = true }
2021
ty_project = { workspace = true, features = ["zstd"] }
2122
ty_server = { workspace = true }

crates/ty/src/args.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::python_version::PythonVersion;
33
use clap::error::ErrorKind;
44
use clap::{ArgAction, ArgMatches, Error, Parser};
55
use ruff_db::system::SystemPathBuf;
6-
use ty_project::combine::Combine;
6+
use ty_combine::Combine;
77
use ty_project::metadata::options::{EnvironmentOptions, Options, SrcOptions, TerminalOptions};
88
use ty_project::metadata::value::{RangedValue, RelativeGlobPattern, RelativePathBuf, ValueSource};
99
use ty_python_semantic::lint;

crates/ty_combine/Cargo.toml

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
[package]
2+
name = "ty_combine"
3+
version = "0.0.0"
4+
edition.workspace = true
5+
rust-version.workspace = true
6+
homepage.workspace = true
7+
documentation.workspace = true
8+
repository.workspace = true
9+
authors.workspace = true
10+
license.workspace = true
11+
12+
[dependencies]
13+
ruff_db = { workspace = true }
14+
ruff_python_ast = { workspace = true }
15+
ty_python_semantic = { workspace = true }
16+
17+
ordermap = { workspace = true }
18+
19+
[lints]
20+
workspace = true

crates/ty_project/src/combine.rs renamed to crates/ty_combine/src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,10 +161,11 @@ impl_noop_combine!(String);
161161

162162
#[cfg(test)]
163163
mod tests {
164-
use crate::combine::Combine;
165164
use ordermap::OrderMap;
166165
use std::collections::HashMap;
167166

167+
use super::Combine;
168+
168169
#[test]
169170
fn combine_option() {
170171
assert_eq!(Some(1).combine(Some(2)), Some(1));

crates/ty_project/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ ruff_options_metadata = { workspace = true }
1919
ruff_python_ast = { workspace = true, features = ["serde"] }
2020
ruff_python_formatter = { workspace = true, optional = true }
2121
ruff_text_size = { workspace = true }
22+
ty_combine = { workspace = true }
2223
ty_python_semantic = { workspace = true, features = ["serde"] }
2324
ty_vendored = { workspace = true }
2425

crates/ty_project/src/db.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,10 @@ impl ProjectDatabase {
107107

108108
/// Set the check mode for the project.
109109
pub fn set_check_mode(&mut self, mode: CheckMode) {
110-
tracing::debug!("Updating project to check {mode}");
111-
self.project().set_check_mode(self).to(mode);
110+
if self.project().check_mode(self) != mode {
111+
tracing::debug!("Updating project to check {mode}");
112+
self.project().set_check_mode(self).to(mode);
113+
}
112114
}
113115

114116
/// Returns a mutable reference to the system.

0 commit comments

Comments
 (0)