Skip to content

Commit

Permalink
ruff server: Support setting to prioritize project configuration ov…
Browse files Browse the repository at this point in the history
…er editor configuration (#11086)

## Summary

This is intended to address
astral-sh/ruff-vscode#425, and is a follow-up
to #11062.

A new client setting is now supported by the server,
`prioritizeFileConfiguration`. This is a boolean setting (default:
`false`) that, if set to `true`, will instruct the configuration
resolver to prioritize file configuration (aka discovered TOML files)
over configuration passed in by the editor.

A corresponding extension PR has been opened, which makes this setting
available for VS Code:
astral-sh/ruff-vscode#457.

## Test Plan

To test this with VS Code, you'll need to check out [the VS Code
PR](astral-sh/ruff-vscode#457) that adds this
setting.

The test process is similar to
#11062, but in scenarios where the
editor configuration would take priority over file configuration, file
configuration should take priority.
  • Loading branch information
snowsignal authored Apr 26, 2024
1 parent cd3e319 commit 16a1f3c
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 7 deletions.
38 changes: 34 additions & 4 deletions crates/ruff_server/src/session/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub(crate) struct ResolvedClientSettings {
/// Contains the resolved values of 'editor settings' - Ruff configuration for the linter/formatter that was passed in via
/// LSP client settings. These fields are optional because we don't want to override file-based linter/formatting settings
/// if these were un-set.
#[derive(Clone, Debug, Default)]
#[derive(Clone, Debug)]
#[cfg_attr(test, derive(PartialEq, Eq))]
pub(crate) struct ResolvedEditorSettings {
pub(super) lint_preview: Option<bool>,
Expand All @@ -38,6 +38,23 @@ pub(crate) struct ResolvedEditorSettings {
pub(super) ignore: Option<Vec<RuleSelector>>,
pub(super) exclude: Option<Vec<String>>,
pub(super) line_length: Option<LineLength>,
pub(super) configuration_preference: ConfigurationPreference,
}

/// Determines how multiple conflicting configurations should be resolved - in this
/// case, the configuration from the client settings and configuration from local
/// `.toml` files (aka 'workspace' configuration).
#[derive(Clone, Copy, Debug, Deserialize, Default)]
#[cfg_attr(test, derive(PartialEq, Eq))]
#[serde(rename_all = "camelCase")]
pub(crate) enum ConfigurationPreference {
/// Configuration set in the editor takes priority over workspace configuration set in `.toml` files.
#[default]
EditorFirst,
/// Configuration set in `.toml` files takes priority over configuration set in the editor.
FilesystemFirst,
/// `.toml` files are ignored completely, and only the editor configuration is used.
EditorOnly,
}

/// This is a direct representation of the settings schema sent by the client.
Expand All @@ -52,6 +69,7 @@ pub(crate) struct ClientSettings {
code_action: Option<CodeActionOptions>,
exclude: Option<Vec<String>>,
line_length: Option<LineLength>,
configuration_preference: Option<ConfigurationPreference>,
}

/// This is a direct representation of the workspace settings schema,
Expand Down Expand Up @@ -251,6 +269,11 @@ impl ResolvedClientSettings {
Some(settings.exclude.as_ref()?.clone())
}),
line_length: Self::resolve_optional(all_settings, |settings| settings.line_length),
configuration_preference: Self::resolve_or(
all_settings,
|settings| settings.configuration_preference,
ConfigurationPreference::EditorFirst,
),
},
}
}
Expand Down Expand Up @@ -383,6 +406,7 @@ mod tests {
),
exclude: None,
line_length: None,
configuration_preference: None,
},
workspace_settings: [
WorkspaceSettings {
Expand Down Expand Up @@ -429,6 +453,7 @@ mod tests {
),
exclude: None,
line_length: None,
configuration_preference: None,
},
workspace: Url {
scheme: "file",
Expand Down Expand Up @@ -488,6 +513,7 @@ mod tests {
),
exclude: None,
line_length: None,
configuration_preference: None,
},
workspace: Url {
scheme: "file",
Expand Down Expand Up @@ -538,7 +564,8 @@ mod tests {
extend_select: None,
ignore: None,
exclude: None,
line_length: None
line_length: None,
configuration_preference: ConfigurationPreference::default(),
}
}
);
Expand Down Expand Up @@ -566,7 +593,8 @@ mod tests {
extend_select: None,
ignore: None,
exclude: None,
line_length: None
line_length: None,
configuration_preference: ConfigurationPreference::EditorFirst,
}
}
);
Expand Down Expand Up @@ -620,6 +648,7 @@ mod tests {
80,
),
),
configuration_preference: None,
},
),
}
Expand Down Expand Up @@ -648,7 +677,8 @@ mod tests {
extend_select: None,
ignore: Some(vec![RuleSelector::from_str("RUF001").unwrap()]),
exclude: Some(vec!["third_party".into()]),
line_length: Some(LineLength::try_from(80).unwrap())
line_length: Some(LineLength::try_from(80).unwrap()),
configuration_preference: ConfigurationPreference::EditorFirst,
}
}
);
Expand Down
15 changes: 12 additions & 3 deletions crates/ruff_server/src/session/workspace/ruff_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use std::{
};
use walkdir::{DirEntry, WalkDir};

use crate::session::settings::ResolvedEditorSettings;
use crate::session::settings::{ConfigurationPreference, ResolvedEditorSettings};

#[derive(Default)]
pub(crate) struct RuffSettings {
Expand Down Expand Up @@ -107,7 +107,7 @@ struct EditorConfigurationTransformer<'a>(&'a ResolvedEditorSettings, &'a Path);
impl<'a> ConfigurationTransformer for EditorConfigurationTransformer<'a> {
fn transform(
&self,
project_configuration: ruff_workspace::configuration::Configuration,
filesystem_configuration: ruff_workspace::configuration::Configuration,
) -> ruff_workspace::configuration::Configuration {
let ResolvedEditorSettings {
format_preview,
Expand All @@ -117,6 +117,7 @@ impl<'a> ConfigurationTransformer for EditorConfigurationTransformer<'a> {
ignore,
exclude,
line_length,
configuration_preference,
} = self.0.clone();

let project_root = self.1;
Expand Down Expand Up @@ -149,6 +150,14 @@ impl<'a> ConfigurationTransformer for EditorConfigurationTransformer<'a> {
..Default::default()
};

editor_configuration.combine(project_configuration)
match configuration_preference {
ConfigurationPreference::EditorFirst => {
editor_configuration.combine(filesystem_configuration)
}
ConfigurationPreference::FilesystemFirst => {
filesystem_configuration.combine(editor_configuration)
}
ConfigurationPreference::EditorOnly => editor_configuration,
}
}
}

0 comments on commit 16a1f3c

Please sign in to comment.