ruff server: Ruff configuration from client settings overrides proj…

…ect configuration (#11062)

## Summary

This is a follow-up to #10984 that
implements configuration resolution for editor configuration. By 'editor
configuration', I'm referring to the client settings that correspond to
Ruff configuration/options, like `preview`, `select`, and so on. These
will be combined with 'project configuration' (configuration taken from
project files such as `pyproject.toml`) to generate the final linter and
formatter settings used by `RuffSettings`. Editor configuration takes
priority over project configuration.

In a follow-up pull request, I'll implement a new client setting that
allows project configuration to override editor configuration, as per
[this issue](astral-sh/ruff-vscode#425).

## Review guide

The first commit, e38966d, is just
doing re-arrangement so that we can pass the right things to
`RuffSettings::resolve`. The actual resolution logic is in the second
commit, 0eec9ee. It might help to look
at these comments individually since the diff is rather messy.

## Test Plan

For the settings to show up in VS Code, you'll need to checkout this
branch: astral-sh/ruff-vscode#456.

To test that the resolution for a specific setting works as expected,
run through the following scenarios, setting it in project and editor
configuration as needed:

| Set in project configuration? | Set in editor configuration? |
Expected Outcome |

|-------------------------------|--------------------------------------------------|------------------------------------------------------------------------------------------|
| No | No | The editor should behave as if the setting was set to its
default value. |
| Yes | No | The editor should behave as if the setting was set to the
value in project configuration. |
| No | Yes | The editor should behave as if the setting was set to the
value in editor configuration. |
| Yes | Yes (but distinctive from project configuration) | The editor
should behave as if the setting was set to the value in editor
configuration. |

An exception to this is `extendSelect`, which does not have an analog in
TOML configuration. Instead, you should verify that `extendSelect`
amends the `select` setting. If `select` is set in both editor and
project configuration, `extendSelect` will only append to the `select`
value in editor configuration, so make sure to un-set it there if you're
testing `extendSelect` with `select` in project configuration.

crates/ruff_server/src/server/api/diagnostics.rs

@@ -6,7 +6,7 @@ pub(super) fn generate_diagnostics(snapshot: &DocumentSnapshot) -> Vec<lsp_types
     if snapshot.client_settings().lint() {
         crate::lint::check(
             snapshot.document(),
-            &snapshot.settings().linter,
+            snapshot.settings().linter(),
             snapshot.encoding(),
         )
     } else {

crates/ruff_server/src/server/api/requests/code_action.rs

@@ -105,7 +105,7 @@ fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result<CodeActionOrCommand> {
                 document,
                 snapshot.resolved_client_capabilities(),
                 snapshot.url(),
-                &snapshot.settings().linter,
+                snapshot.settings().linter(),
                 snapshot.encoding(),
                 document.version(),
             )?),
@@ -140,7 +140,7 @@ fn organize_imports(snapshot: &DocumentSnapshot) -> crate::Result<CodeActionOrCo
                 document,
                 snapshot.resolved_client_capabilities(),
                 snapshot.url(),
-                &snapshot.settings().linter,
+                snapshot.settings().linter(),
                 snapshot.encoding(),
                 document.version(),
             )?),

crates/ruff_server/src/server/api/requests/code_action_resolve.rs

@@ -54,7 +54,7 @@ impl super::BackgroundDocumentRequestHandler for CodeActionResolve {
                     document,
                     snapshot.resolved_client_capabilities(),
                     snapshot.url(),
-                    &snapshot.settings().linter,
+                    snapshot.settings().linter(),
                     snapshot.encoding(),
                     document.version(),
                 )
@@ -65,7 +65,7 @@ impl super::BackgroundDocumentRequestHandler for CodeActionResolve {
                     document,
                     snapshot.resolved_client_capabilities(),
                     snapshot.url(),
-                    &snapshot.settings().linter,
+                    snapshot.settings().linter(),
                     snapshot.encoding(),
                     document.version(),
                 )

crates/ruff_server/src/server/api/requests/execute_command.rs

@@ -64,7 +64,7 @@ impl super::SyncRequestHandler for ExecuteCommand {
                 Command::FixAll => {
                     let edits = super::code_action_resolve::fix_all_edit(
                         snapshot.document(),
-                        &snapshot.settings().linter,
+                        snapshot.settings().linter(),
                         snapshot.encoding(),
                     )
                     .with_failure_code(ErrorCode::InternalError)?;
@@ -83,7 +83,7 @@ impl super::SyncRequestHandler for ExecuteCommand {
                 Command::OrganizeImports => {
                     let edits = super::code_action_resolve::organize_imports_edit(
                         snapshot.document(),
-                        &snapshot.settings().linter,
+                        snapshot.settings().linter(),
                         snapshot.encoding(),
                     )
                     .with_failure_code(ErrorCode::InternalError)?;

crates/ruff_server/src/server/api/requests/format.rs

@@ -26,7 +26,7 @@ impl super::BackgroundDocumentRequestHandler for Format {
 pub(super) fn format_document(snapshot: &DocumentSnapshot) -> Result<super::FormatResponse> {
     let doc = snapshot.document();
     let source = doc.contents();
-    let formatted = crate::format::format(doc, &snapshot.settings().formatter)
+    let formatted = crate::format::format(doc, snapshot.settings().formatter())
         .with_failure_code(lsp_server::ErrorCode::InternalError)?;
     // fast path - if the code is the same, return early
     if formatted == source {

crates/ruff_server/src/server/api/requests/format_range.rs

@@ -22,7 +22,7 @@ impl super::BackgroundDocumentRequestHandler for FormatRange {
         let index = document.index();
         let range = params.range.to_text_range(text, index, snapshot.encoding());
         let formatted_range =
-            crate::format::format_range(document, &snapshot.settings().formatter, range)
+            crate::format::format_range(document, snapshot.settings().formatter(), range)
                 .with_failure_code(lsp_server::ErrorCode::InternalError)?;
         Ok(Some(vec![types::TextEdit {
             range: formatted_range

crates/ruff_server/src/session.rs

@@ -46,11 +46,11 @@ impl Session {
     ) -> crate::Result<Self> {
         Ok(Self {
             position_encoding,
+            workspaces: workspace::Workspaces::new(workspaces, &global_settings)?,
             global_settings,
             resolved_client_capabilities: Arc::new(ResolvedClientCapabilities::new(
                 client_capabilities,
             )),
-            workspaces: workspace::Workspaces::new(workspaces)?,
         })
     }
 
@@ -87,7 +87,8 @@ impl Session {
     }
 
     pub(crate) fn open_workspace_folder(&mut self, url: &Url) -> crate::Result<()> {
-        self.workspaces.open_workspace_folder(url)?;
+        self.workspaces
+            .open_workspace_folder(url, &self.global_settings)?;
         Ok(())
     }
 

crates/ruff_server/src/session/settings.rs

@@ -1,4 +1,4 @@
-use std::{ffi::OsString, ops::Deref, path::PathBuf, str::FromStr};
+use std::{ops::Deref, str::FromStr};
 
 use lsp_types::Url;
 use ruff_linter::{line_width::LineLength, RuleSelector};
@@ -11,7 +11,7 @@ pub(crate) type WorkspaceSettingsMap = FxHashMap<Url, ClientSettings>;
 /// Resolved client settings for a specific document. These settings are meant to be
 /// used directly by the server, and are *not* a 1:1 representation with how the client
 /// sends them.
-#[derive(Debug)]
+#[derive(Clone, Debug)]
 #[cfg_attr(test, derive(PartialEq, Eq))]
 #[allow(clippy::struct_excessive_bools)]
 pub(crate) struct ResolvedClientSettings {
@@ -22,25 +22,22 @@ pub(crate) struct ResolvedClientSettings {
     #[allow(dead_code)]
     disable_rule_comment_enable: bool,
     fix_violation_enable: bool,
-    // TODO(jane): Remove once editor settings resolution is implemented
-    #[allow(dead_code)]
     editor_settings: ResolvedEditorSettings,
 }
 
 /// 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(Debug, Default)]
+#[derive(Clone, Debug, Default)]
 #[cfg_attr(test, derive(PartialEq, Eq))]
-#[allow(dead_code)] // TODO(jane): Remove once editor settings resolution is implemented
 pub(crate) struct ResolvedEditorSettings {
-    lint_preview: Option<bool>,
-    format_preview: Option<bool>,
-    select: Option<Vec<RuleSelector>>,
-    extend_select: Option<Vec<RuleSelector>>,
-    ignore: Option<Vec<RuleSelector>>,
-    exclude: Option<Vec<PathBuf>>,
-    line_length: Option<LineLength>,
+    pub(super) lint_preview: Option<bool>,
+    pub(super) format_preview: Option<bool>,
+    pub(super) select: Option<Vec<RuleSelector>>,
+    pub(super) extend_select: Option<Vec<RuleSelector>>,
+    pub(super) ignore: Option<Vec<RuleSelector>>,
+    pub(super) exclude: Option<Vec<String>>,
+    pub(super) line_length: Option<LineLength>,
 }
 
 /// This is a direct representation of the settings schema sent by the client.
@@ -251,14 +248,7 @@ impl ResolvedClientSettings {
                         .collect()
                 }),
                 exclude: Self::resolve_optional(all_settings, |settings| {
-                    Some(
-                        settings
-                            .exclude
-                            .as_ref()?
-                            .iter()
-                            .map(|path| PathBuf::from(OsString::from(path)))
-                            .collect(),
-                    )
+                    Some(settings.exclude.as_ref()?.clone())
                 }),
                 line_length: Self::resolve_optional(all_settings, |settings| settings.line_length),
             },
@@ -306,6 +296,10 @@ impl ResolvedClientSettings {
     pub(crate) fn fix_violation(&self) -> bool {
         self.fix_violation_enable
     }
+
+    pub(crate) fn editor_settings(&self) -> &ResolvedEditorSettings {
+        &self.editor_settings
+    }
 }
 
 impl Default for InitializationOptions {
@@ -653,7 +647,7 @@ mod tests {
                     select: None,
                     extend_select: None,
                     ignore: Some(vec![RuleSelector::from_str("RUF001").unwrap()]),
-                    exclude: Some(vec![PathBuf::from_str("third_party").unwrap()]),
+                    exclude: Some(vec!["third_party".into()]),
                     line_length: Some(LineLength::try_from(80).unwrap())
                 }
             }

crates/ruff_server/src/session/workspace.rs

@@ -12,7 +12,10 @@ use crate::{edit::DocumentVersion, Document};
 
 use self::ruff_settings::RuffSettingsIndex;
 
-use super::{settings, ClientSettings};
+use super::{
+    settings::{self, ResolvedClientSettings, ResolvedEditorSettings},
+    ClientSettings,
+};
 
 mod ruff_settings;
 
@@ -23,7 +26,7 @@ pub(crate) struct Workspaces(BTreeMap<PathBuf, Workspace>);
 
 pub(crate) struct Workspace {
     open_documents: OpenDocuments,
-    settings: ClientSettings,
+    settings: ResolvedClientSettings,
 }
 
 pub(crate) struct OpenDocuments {
@@ -46,18 +49,28 @@ pub(crate) struct DocumentRef {
 }
 
 impl Workspaces {
-    pub(super) fn new(workspaces: Vec<(Url, ClientSettings)>) -> crate::Result<Self> {
+    pub(super) fn new(
+        workspaces: Vec<(Url, ClientSettings)>,
+        global_settings: &ClientSettings,
+    ) -> crate::Result<Self> {
         Ok(Self(
             workspaces
                 .into_iter()
-                .map(|(url, settings)| Workspace::new(&url, settings))
+                .map(|(url, workspace_settings)| {
+                    Workspace::new(&url, &workspace_settings, global_settings)
+                })
                 .collect::<crate::Result<_>>()?,
         ))
     }
 
-    pub(super) fn open_workspace_folder(&mut self, folder_url: &Url) -> crate::Result<()> {
+    pub(super) fn open_workspace_folder(
+        &mut self,
+        folder_url: &Url,
+        global_settings: &ClientSettings,
+    ) -> crate::Result<()> {
         // TODO(jane): find a way to allow for workspace settings to be updated dynamically
-        let (path, workspace) = Workspace::new(folder_url, ClientSettings::default())?;
+        let (path, workspace) =
+            Workspace::new(folder_url, &ClientSettings::default(), global_settings)?;
         self.0.insert(path, workspace);
         Ok(())
     }
@@ -117,12 +130,7 @@ impl Workspaces {
                 );
                 settings::ResolvedClientSettings::global(global_settings)
             },
-            |workspace| {
-                settings::ResolvedClientSettings::with_workspace(
-                    &workspace.settings,
-                    global_settings,
-                )
-            },
+            |workspace| workspace.settings.clone(),
         )
     }
 
@@ -152,29 +160,36 @@ impl Workspaces {
 }
 
 impl Workspace {
-    pub(crate) fn new(root: &Url, settings: ClientSettings) -> crate::Result<(PathBuf, Self)> {
+    pub(crate) fn new(
+        root: &Url,
+        workspace_settings: &ClientSettings,
+        global_settings: &ClientSettings,
+    ) -> crate::Result<(PathBuf, Self)> {
         let path = root
             .to_file_path()
             .map_err(|()| anyhow!("workspace URL was not a file path!"))?;
 
+        let settings = ResolvedClientSettings::with_workspace(workspace_settings, global_settings);
+
         let workspace = Self {
-            open_documents: OpenDocuments::new(&path),
+            open_documents: OpenDocuments::new(&path, settings.editor_settings()),
             settings,
         };
 
         Ok((path, workspace))
     }
 
     fn reload_settings(&mut self, root: &Path) {
-        self.open_documents.reload_settings(root);
+        self.open_documents
+            .reload_settings(root, self.settings.editor_settings());
     }
 }
 
 impl OpenDocuments {
-    fn new(path: &Path) -> Self {
+    fn new(path: &Path, editor_settings: &ResolvedEditorSettings) -> Self {
         Self {
             documents: FxHashMap::default(),
-            settings_index: RuffSettingsIndex::new(path),
+            settings_index: RuffSettingsIndex::new(path, editor_settings),
         }
     }
 
@@ -209,8 +224,8 @@ impl OpenDocuments {
         Ok(())
     }
 
-    fn reload_settings(&mut self, root: &Path) {
-        self.settings_index = RuffSettingsIndex::new(root);
+    fn reload_settings(&mut self, root: &Path, editor_settings: &ResolvedEditorSettings) {
+        self.settings_index = RuffSettingsIndex::new(root, editor_settings);
     }
 }