mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-26 18:06:43 +00:00
[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
https://github.com/astral-sh/ruff/pull/19614#issuecomment-3136477864
(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
db9ea0cdfd/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.
This commit is contained in:
parent
529d81daca
commit
1f29a04e9a
48 changed files with 1231 additions and 554 deletions
225
crates/ty_combine/src/lib.rs
Normal file
225
crates/ty_combine/src/lib.rs
Normal file
|
|
@ -0,0 +1,225 @@
|
|||
use std::{collections::HashMap, hash::BuildHasher};
|
||||
|
||||
use ordermap::OrderMap;
|
||||
use ruff_db::system::SystemPathBuf;
|
||||
use ruff_python_ast::PythonVersion;
|
||||
use ty_python_semantic::PythonPlatform;
|
||||
|
||||
/// Combine two values, preferring the values in `self`.
|
||||
///
|
||||
/// The logic should follow that of Cargo's `config.toml`:
|
||||
///
|
||||
/// > If a key is specified in multiple config files, the values will get merged together.
|
||||
/// > Numbers, strings, and booleans will use the value in the deeper config directory taking
|
||||
/// > precedence over ancestor directories, where the home directory is the lowest priority.
|
||||
/// > Arrays will be joined together with higher precedence items being placed later in the
|
||||
/// > merged array.
|
||||
///
|
||||
/// ## uv Compatibility
|
||||
///
|
||||
/// The merging behavior differs from uv in that values with higher precedence in arrays
|
||||
/// are placed later in the merged array. This is because we want to support overriding
|
||||
/// earlier values and values from other configurations, including unsetting them.
|
||||
/// For example: patterns coming last in file inclusion and exclusion patterns
|
||||
/// allow overriding earlier patterns, matching the `gitignore` behavior.
|
||||
/// Generally speaking, it feels more intuitive if later values override earlier values
|
||||
/// than the other way around: `ty --exclude png --exclude "!important.png"`.
|
||||
///
|
||||
/// The main downside of this approach is that the ordering can be surprising in cases
|
||||
/// where the option has a "first match" semantic and not a "last match" wins.
|
||||
/// One such example is `extra-paths` where the semantics is given by Python:
|
||||
/// the module on the first matching search path wins.
|
||||
///
|
||||
/// ```toml
|
||||
/// [environment]
|
||||
/// extra-paths = ["b", "c"]
|
||||
/// ```
|
||||
///
|
||||
/// ```bash
|
||||
/// ty --extra-paths a
|
||||
/// ```
|
||||
///
|
||||
/// That's why a user might expect that this configuration results in `["a", "b", "c"]`,
|
||||
/// because the CLI has higher precedence. However, the current implementation results in a
|
||||
/// resolved extra search path of `["b", "c", "a"]`, which means `a` will be tried last.
|
||||
///
|
||||
/// There's an argument here that the user should be able to specify the order of the paths,
|
||||
/// because only then is the user in full control of where to insert the path when specifying `extra-paths`
|
||||
/// in multiple sources.
|
||||
///
|
||||
/// ## Macro
|
||||
/// You can automatically derive `Combine` for structs with named fields by using `derive(ruff_macros::Combine)`.
|
||||
pub trait Combine {
|
||||
#[must_use]
|
||||
fn combine(mut self, other: Self) -> Self
|
||||
where
|
||||
Self: Sized,
|
||||
{
|
||||
self.combine_with(other);
|
||||
self
|
||||
}
|
||||
|
||||
fn combine_with(&mut self, other: Self);
|
||||
}
|
||||
|
||||
impl<T> Combine for Option<T>
|
||||
where
|
||||
T: Combine,
|
||||
{
|
||||
fn combine(self, other: Self) -> Self
|
||||
where
|
||||
Self: Sized,
|
||||
{
|
||||
match (self, other) {
|
||||
(Some(a), Some(b)) => Some(a.combine(b)),
|
||||
(None, Some(b)) => Some(b),
|
||||
(a, _) => a,
|
||||
}
|
||||
}
|
||||
|
||||
fn combine_with(&mut self, other: Self) {
|
||||
match (self, other) {
|
||||
(Some(a), Some(b)) => {
|
||||
a.combine_with(b);
|
||||
}
|
||||
(a @ None, Some(b)) => {
|
||||
*a = Some(b);
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl<T> Combine for Vec<T> {
|
||||
fn combine_with(&mut self, mut other: Self) {
|
||||
// `self` takes precedence over `other` but values with higher precedence must be placed after.
|
||||
// Swap the vectors so that `other` is the one that gets extended, so that the values of `self` come after.
|
||||
std::mem::swap(self, &mut other);
|
||||
self.extend(other);
|
||||
}
|
||||
}
|
||||
|
||||
impl<K, V, S> Combine for HashMap<K, V, S>
|
||||
where
|
||||
K: Eq + std::hash::Hash,
|
||||
S: BuildHasher,
|
||||
{
|
||||
fn combine_with(&mut self, mut other: Self) {
|
||||
// `self` takes precedence over `other` but `extend` overrides existing values.
|
||||
// Swap the hash maps so that `self` is the one that gets extended.
|
||||
std::mem::swap(self, &mut other);
|
||||
self.extend(other);
|
||||
}
|
||||
}
|
||||
|
||||
impl<K, V, S> Combine for OrderMap<K, V, S>
|
||||
where
|
||||
K: Eq + std::hash::Hash,
|
||||
S: BuildHasher,
|
||||
{
|
||||
fn combine_with(&mut self, other: Self) {
|
||||
for (k, v) in other {
|
||||
self.entry(k).or_insert(v);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Implements [`Combine`] for a value that always returns `self` when combined with another value.
|
||||
macro_rules! impl_noop_combine {
|
||||
($name:ident) => {
|
||||
impl Combine for $name {
|
||||
#[inline(always)]
|
||||
fn combine_with(&mut self, _other: Self) {}
|
||||
|
||||
#[inline(always)]
|
||||
fn combine(self, _other: Self) -> Self {
|
||||
self
|
||||
}
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
impl_noop_combine!(SystemPathBuf);
|
||||
impl_noop_combine!(PythonPlatform);
|
||||
impl_noop_combine!(PythonVersion);
|
||||
|
||||
// std types
|
||||
impl_noop_combine!(bool);
|
||||
impl_noop_combine!(usize);
|
||||
impl_noop_combine!(u8);
|
||||
impl_noop_combine!(u16);
|
||||
impl_noop_combine!(u32);
|
||||
impl_noop_combine!(u64);
|
||||
impl_noop_combine!(u128);
|
||||
impl_noop_combine!(isize);
|
||||
impl_noop_combine!(i8);
|
||||
impl_noop_combine!(i16);
|
||||
impl_noop_combine!(i32);
|
||||
impl_noop_combine!(i64);
|
||||
impl_noop_combine!(i128);
|
||||
impl_noop_combine!(String);
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use ordermap::OrderMap;
|
||||
use std::collections::HashMap;
|
||||
|
||||
use super::Combine;
|
||||
|
||||
#[test]
|
||||
fn combine_option() {
|
||||
assert_eq!(Some(1).combine(Some(2)), Some(1));
|
||||
assert_eq!(None.combine(Some(2)), Some(2));
|
||||
assert_eq!(Some(1).combine(None), Some(1));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn combine_vec() {
|
||||
assert_eq!(None.combine(Some(vec![1, 2, 3])), Some(vec![1, 2, 3]));
|
||||
assert_eq!(Some(vec![1, 2, 3]).combine(None), Some(vec![1, 2, 3]));
|
||||
assert_eq!(
|
||||
Some(vec![1, 2, 3]).combine(Some(vec![4, 5, 6])),
|
||||
Some(vec![4, 5, 6, 1, 2, 3])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn combine_map() {
|
||||
let a: HashMap<u32, _> = HashMap::from_iter([(1, "a"), (2, "a"), (3, "a")]);
|
||||
let b: HashMap<u32, _> = HashMap::from_iter([(0, "b"), (2, "b"), (5, "b")]);
|
||||
|
||||
assert_eq!(None.combine(Some(b.clone())), Some(b.clone()));
|
||||
assert_eq!(Some(a.clone()).combine(None), Some(a.clone()));
|
||||
assert_eq!(
|
||||
Some(a).combine(Some(b)),
|
||||
Some(HashMap::from_iter([
|
||||
(0, "b"),
|
||||
// The value from `a` takes precedence
|
||||
(1, "a"),
|
||||
(2, "a"),
|
||||
(3, "a"),
|
||||
(5, "b")
|
||||
]))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn combine_order_map() {
|
||||
let a: OrderMap<u32, _> = OrderMap::from_iter([(1, "a"), (2, "a"), (3, "a")]);
|
||||
let b: OrderMap<u32, _> = OrderMap::from_iter([(0, "b"), (2, "b"), (5, "b")]);
|
||||
|
||||
assert_eq!(None.combine(Some(b.clone())), Some(b.clone()));
|
||||
assert_eq!(Some(a.clone()).combine(None), Some(a.clone()));
|
||||
assert_eq!(
|
||||
Some(a).combine(Some(b)),
|
||||
// The value from `a` takes precedence
|
||||
Some(OrderMap::from_iter([
|
||||
(1, "a"),
|
||||
(2, "a"),
|
||||
(3, "a"),
|
||||
(0, "b"),
|
||||
(5, "b")
|
||||
]))
|
||||
);
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue