Centralize client options validation (#18623)

crates/ruff_server/src/lib.rs

@@ -3,7 +3,7 @@
 pub use edit::{DocumentKey, NotebookDocument, PositionEncoding, TextDocument};
 use lsp_types::CodeActionKind;
 pub use server::Server;
-pub use session::{ClientSettings, DocumentQuery, DocumentSnapshot, Session};
+pub use session::{ClientOptions, DocumentQuery, DocumentSnapshot, GlobalOptions, Session};
 pub use workspace::{Workspace, Workspaces};
 
 #[macro_use]

crates/ruff_server/src/server.rs

@@ -30,7 +30,7 @@ use self::schedule::Scheduler;
 use self::schedule::Task;
 use self::schedule::event_loop_thread;
 use crate::PositionEncoding;
-use crate::session::AllSettings;
+use crate::session::AllOptions;
 use crate::session::Session;
 use crate::workspace::Workspaces;
 
@@ -77,39 +77,36 @@ impl Server {
             ..
         } = init_params;
 
-        let mut all_settings = AllSettings::from_value(
+        let mut all_options = AllOptions::from_value(
             initialization_options
                 .unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::default())),
         );
         if let Some(preview) = preview {
-            all_settings.set_preview(preview);
+            all_options.set_preview(preview);
         }
-        let AllSettings {
-            global_settings,
-            workspace_settings,
-        } = all_settings;
+        let AllOptions {
+            global: global_options,
+            workspace: workspace_options,
+        } = all_options;
 
         crate::logging::init_logging(
-            global_settings.tracing.log_level.unwrap_or_default(),
-            global_settings.tracing.log_file.as_deref(),
+            global_options.tracing.log_level.unwrap_or_default(),
+            global_options.tracing.log_file.as_deref(),
         );
 
         let workspaces = Workspaces::from_workspace_folders(
             workspace_folders,
-            workspace_settings.unwrap_or_default(),
+            workspace_options.unwrap_or_default(),
         )?;
 
         tracing::debug!("Negotiated position encoding: {position_encoding:?}");
 
+        let global = global_options.into_settings();
+
         Ok(Self {
             connection,
             worker_threads,
-            session: Session::new(
-                &client_capabilities,
-                position_encoding,
-                global_settings,
-                &workspaces,
-            )?,
+            session: Session::new(&client_capabilities, position_encoding, global, &workspaces)?,
             client_capabilities,
         })
     }

crates/ruff_server/src/session.rs

@@ -4,19 +4,21 @@ use std::path::Path;
 use std::sync::Arc;
 
 use lsp_types::{ClientCapabilities, FileEvent, NotebookDocumentCellChange, Url};
-use settings::ResolvedClientSettings;
+use settings::ClientSettings;
 
 use crate::edit::{DocumentKey, DocumentVersion, NotebookDocument};
+use crate::session::settings::GlobalClientSettings;
 use crate::workspace::Workspaces;
 use crate::{PositionEncoding, TextDocument};
 
 pub(crate) use self::capabilities::ResolvedClientCapabilities;
 pub use self::index::DocumentQuery;
-pub use self::settings::ClientSettings;
-pub(crate) use self::settings::{AllSettings, WorkspaceSettingsMap};
+pub(crate) use self::options::{AllOptions, WorkspaceOptionsMap};
+pub use self::options::{ClientOptions, GlobalOptions};
 
 mod capabilities;
 mod index;
+mod options;
 mod settings;
 
 /// The global state for the LSP
@@ -26,7 +28,8 @@ pub struct Session {
     /// The global position encoding, negotiated during LSP initialization.
     position_encoding: PositionEncoding,
     /// Global settings provided by the client.
-    global_settings: ClientSettings,
+    global_settings: GlobalClientSettings,
+
     /// Tracks what LSP features the client supports and doesn't support.
     resolved_client_capabilities: Arc<ResolvedClientCapabilities>,
 }
@@ -35,7 +38,7 @@ pub struct Session {
 /// a specific document.
 pub struct DocumentSnapshot {
     resolved_client_capabilities: Arc<ResolvedClientCapabilities>,
-    client_settings: settings::ResolvedClientSettings,
+    client_settings: Arc<settings::ClientSettings>,
     document_ref: index::DocumentQuery,
     position_encoding: PositionEncoding,
 }
@@ -44,13 +47,13 @@ impl Session {
     pub fn new(
         client_capabilities: &ClientCapabilities,
         position_encoding: PositionEncoding,
-        global_settings: ClientSettings,
+        global: GlobalClientSettings,
         workspaces: &Workspaces,
     ) -> crate::Result<Self> {
         Ok(Self {
             position_encoding,
-            index: index::Index::new(workspaces, &global_settings)?,
-            global_settings,
+            index: index::Index::new(workspaces, &global)?,
+            global_settings: global,
             resolved_client_capabilities: Arc::new(ResolvedClientCapabilities::new(
                 client_capabilities,
             )),
@@ -66,7 +69,10 @@ impl Session {
         let key = self.key_from_url(url);
         Some(DocumentSnapshot {
             resolved_client_capabilities: self.resolved_client_capabilities.clone(),
-            client_settings: self.index.client_settings(&key, &self.global_settings),
+            client_settings: self
+                .index
+                .client_settings(&key)
+                .unwrap_or_else(|| self.global_settings.to_settings_arc()),
             document_ref: self.index.make_document_ref(key, &self.global_settings)?,
             position_encoding: self.position_encoding,
         })
@@ -163,8 +169,8 @@ impl Session {
     }
 
     /// Returns the resolved global client settings.
-    pub(crate) fn global_client_settings(&self) -> ResolvedClientSettings {
-        ResolvedClientSettings::global(&self.global_settings)
+    pub(crate) fn global_client_settings(&self) -> &ClientSettings {
+        self.global_settings.to_settings()
     }
 
     /// Returns the number of open documents in the session.
@@ -183,7 +189,7 @@ impl DocumentSnapshot {
         &self.resolved_client_capabilities
     }
 
-    pub(crate) fn client_settings(&self) -> &settings::ResolvedClientSettings {
+    pub(crate) fn client_settings(&self) -> &settings::ClientSettings {
         &self.client_settings
     }
 

crates/ruff_server/src/session/index.rs

@@ -11,13 +11,15 @@ use thiserror::Error;
 pub(crate) use ruff_settings::RuffSettings;
 
 use crate::edit::LanguageId;
+use crate::session::options::Combine;
+use crate::session::settings::GlobalClientSettings;
 use crate::workspace::{Workspace, Workspaces};
 use crate::{
     PositionEncoding, TextDocument,
     edit::{DocumentKey, DocumentVersion, NotebookDocument},
 };
 
-use super::{ClientSettings, settings::ResolvedClientSettings};
+use super::settings::ClientSettings;
 
 mod ruff_settings;
 
@@ -36,7 +38,7 @@ pub(crate) struct Index {
 
 /// Settings associated with a workspace.
 struct WorkspaceSettings {
-    client_settings: ResolvedClientSettings,
+    client_settings: Arc<ClientSettings>,
     ruff_settings: ruff_settings::RuffSettingsIndex,
 }
 
@@ -70,11 +72,11 @@ pub enum DocumentQuery {
 impl Index {
     pub(super) fn new(
         workspaces: &Workspaces,
-        global_settings: &ClientSettings,
+        global: &GlobalClientSettings,
     ) -> crate::Result<Self> {
         let mut settings = WorkspaceSettingsIndex::default();
         for workspace in &**workspaces {
-            settings.register_workspace(workspace, global_settings)?;
+            settings.register_workspace(workspace, global)?;
         }
 
         Ok(Self {
@@ -170,11 +172,11 @@ impl Index {
     pub(super) fn open_workspace_folder(
         &mut self,
         url: Url,
-        global_settings: &ClientSettings,
+        global: &GlobalClientSettings,
     ) -> crate::Result<()> {
         // TODO(jane): Find a way for workspace client settings to be added or changed dynamically.
         self.settings
-            .register_workspace(&Workspace::new(url), global_settings)
+            .register_workspace(&Workspace::new(url), global)
     }
 
     pub(super) fn close_workspace_folder(&mut self, workspace_url: &Url) -> crate::Result<()> {
@@ -201,7 +203,7 @@ impl Index {
     pub(super) fn make_document_ref(
         &self,
         key: DocumentKey,
-        global_settings: &ClientSettings,
+        global: &GlobalClientSettings,
     ) -> Option<DocumentQuery> {
         let url = self.url_for_key(&key)?.clone();
 
@@ -230,13 +232,12 @@ impl Index {
                     "No settings available for {} - falling back to default settings",
                     url
                 );
-                let resolved_global = ResolvedClientSettings::global(global_settings);
                 // The path here is only for completeness, it's okay to use a non-existing path
                 // in case this is an unsaved (untitled) document.
                 let path = Path::new(url.path());
                 let root = path.parent().unwrap_or(path);
                 Arc::new(RuffSettings::fallback(
-                    resolved_global.editor_settings(),
+                    global.to_settings().editor_settings(),
                     root,
                 ))
             });
@@ -330,21 +331,12 @@ impl Index {
         Ok(())
     }
 
-    pub(super) fn client_settings(
-        &self,
-        key: &DocumentKey,
-        global_settings: &ClientSettings,
-    ) -> ResolvedClientSettings {
-        let Some(url) = self.url_for_key(key) else {
-            return ResolvedClientSettings::global(global_settings);
-        };
-        let Some(WorkspaceSettings {
+    pub(super) fn client_settings(&self, key: &DocumentKey) -> Option<Arc<ClientSettings>> {
+        let url = self.url_for_key(key)?;
+        let WorkspaceSettings {
             client_settings, ..
-        }) = self.settings_for_url(url)
-        else {
-            return ResolvedClientSettings::global(global_settings);
-        };
-        client_settings.clone()
+        } = self.settings_for_url(url)?;
+        Some(client_settings.clone())
     }
 
     fn document_controller_for_key(
@@ -422,7 +414,7 @@ impl WorkspaceSettingsIndex {
     fn register_workspace(
         &mut self,
         workspace: &Workspace,
-        global_settings: &ClientSettings,
+        global: &GlobalClientSettings,
     ) -> crate::Result<()> {
         let workspace_url = workspace.url();
         if workspace_url.scheme() != "file" {
@@ -434,10 +426,21 @@ impl WorkspaceSettingsIndex {
             anyhow!("Failed to convert workspace URL to file path: {workspace_url}")
         })?;
 
-        let client_settings = if let Some(workspace_settings) = workspace.settings() {
-            ResolvedClientSettings::with_workspace(workspace_settings, global_settings)
+        let client_settings = if let Some(workspace_options) = workspace.options() {
+            let options = workspace_options.clone().combine(global.options().clone());
+            let settings = match options.into_settings() {
+                Ok(settings) => settings,
+                Err(settings) => {
+                    show_err_msg!(
+                        "The settings for the workspace {workspace_path} are invalid. Refer to the logs for more information.",
+                        workspace_path = workspace_path.display()
+                    );
+                    settings
+                }
+            };
+            Arc::new(settings)
         } else {
-            ResolvedClientSettings::global(global_settings)
+            global.to_settings_arc()
         };
 
         let workspace_settings_index = ruff_settings::RuffSettingsIndex::new(

crates/ruff_server/src/session/index/ruff_settings.rs

@@ -18,9 +18,8 @@ use ruff_workspace::{
     resolver::ConfigurationTransformer,
 };
 
-use crate::session::settings::{
-    ConfigurationPreference, ResolvedConfiguration, ResolvedEditorSettings,
-};
+use crate::session::options::ConfigurationPreference;
+use crate::session::settings::{EditorSettings, ResolvedConfiguration};
 
 #[derive(Debug)]
 pub struct RuffSettings {
@@ -64,7 +63,7 @@ impl RuffSettings {
     ///
     /// In the absence of a valid configuration file, it gracefully falls back to
     /// editor-only settings.
-    pub(crate) fn fallback(editor_settings: &ResolvedEditorSettings, root: &Path) -> RuffSettings {
+    pub(crate) fn fallback(editor_settings: &EditorSettings, root: &Path) -> RuffSettings {
         struct FallbackTransformer<'a> {
             inner: EditorConfigurationTransformer<'a>,
         }
@@ -122,14 +121,14 @@ impl RuffSettings {
 
     /// Constructs [`RuffSettings`] by merging the editor-defined settings with the
     /// default configuration.
-    fn editor_only(editor_settings: &ResolvedEditorSettings, root: &Path) -> RuffSettings {
+    fn editor_only(editor_settings: &EditorSettings, root: &Path) -> RuffSettings {
         Self::with_editor_settings(editor_settings, root, Configuration::default())
             .expect("editor configuration should merge successfully with default configuration")
     }
 
     /// Merges the `configuration` with the editor defined settings.
     fn with_editor_settings(
-        editor_settings: &ResolvedEditorSettings,
+        editor_settings: &EditorSettings,
         root: &Path,
         configuration: Configuration,
     ) -> anyhow::Result<RuffSettings> {
@@ -157,7 +156,7 @@ impl RuffSettingsIndex {
     /// skipping (3).
     pub(super) fn new(
         root: &Path,
-        editor_settings: &ResolvedEditorSettings,
+        editor_settings: &EditorSettings,
         is_default_workspace: bool,
     ) -> Self {
         if editor_settings.configuration_preference == ConfigurationPreference::EditorOnly {
@@ -392,11 +391,11 @@ impl RuffSettingsIndex {
     }
 }
 
-struct EditorConfigurationTransformer<'a>(&'a ResolvedEditorSettings, &'a Path);
+struct EditorConfigurationTransformer<'a>(&'a EditorSettings, &'a Path);
 
 impl ConfigurationTransformer for EditorConfigurationTransformer<'_> {
     fn transform(&self, filesystem_configuration: Configuration) -> Configuration {
-        let ResolvedEditorSettings {
+        let EditorSettings {
             configuration,
             format_preview,
             lint_preview,
@@ -515,7 +514,7 @@ mod tests {
     /// This test ensures that the inline configuration is correctly applied to the configuration.
     #[test]
     fn inline_settings() {
-        let editor_settings = ResolvedEditorSettings {
+        let editor_settings = EditorSettings {
             configuration: Some(ResolvedConfiguration::Inline(Box::new(Options {
                 line_length: Some(LineLength::try_from(120).unwrap()),
                 ..Default::default()
@@ -533,7 +532,7 @@ mod tests {
     /// settings is prioritized.
     #[test]
     fn inline_and_specific_settings_resolution_order() {
-        let editor_settings = ResolvedEditorSettings {
+        let editor_settings = EditorSettings {
             configuration: Some(ResolvedConfiguration::Inline(Box::new(Options {
                 line_length: Some(LineLength::try_from(120).unwrap()),
                 ..Default::default()

crates/ruff_server/src/workspace.rs

@@ -3,8 +3,7 @@ use std::ops::Deref;
 use lsp_types::{Url, WorkspaceFolder};
 use thiserror::Error;
 
-use crate::ClientSettings;
-use crate::session::WorkspaceSettingsMap;
+use crate::session::{ClientOptions, WorkspaceOptionsMap};
 
 #[derive(Debug)]
 pub struct Workspaces(Vec<Workspace>);
@@ -18,15 +17,15 @@ impl Workspaces {
     /// initialization.
     pub(crate) fn from_workspace_folders(
         workspace_folders: Option<Vec<WorkspaceFolder>>,
-        mut workspace_settings: WorkspaceSettingsMap,
+        mut workspace_options: WorkspaceOptionsMap,
     ) -> std::result::Result<Workspaces, WorkspacesError> {
-        let mut client_settings_for_url = |url: &Url| {
-            workspace_settings.remove(url).unwrap_or_else(|| {
+        let mut client_options_for_url = |url: &Url| {
+            workspace_options.remove(url).unwrap_or_else(|| {
                 tracing::info!(
-                    "No workspace settings found for {}, using default settings",
+                    "No workspace options found for {}, using default options",
                     url
                 );
-                ClientSettings::default()
+                ClientOptions::default()
             })
         };
 
@@ -35,8 +34,8 @@ impl Workspaces {
                 folders
                     .into_iter()
                     .map(|folder| {
-                        let settings = client_settings_for_url(&folder.uri);
-                        Workspace::new(folder.uri).with_settings(settings)
+                        let options = client_options_for_url(&folder.uri);
+                        Workspace::new(folder.uri).with_options(options)
                     })
                     .collect()
             } else {
@@ -48,8 +47,8 @@ impl Workspaces {
                 );
                 let uri = Url::from_file_path(current_dir)
                     .map_err(|()| WorkspacesError::InvalidCurrentDir)?;
-                let settings = client_settings_for_url(&uri);
-                vec![Workspace::default(uri).with_settings(settings)]
+                let options = client_options_for_url(&uri);
+                vec![Workspace::default(uri).with_options(options)]
             };
 
         Ok(Workspaces(workspaces))
@@ -76,8 +75,8 @@ pub(crate) enum WorkspacesError {
 pub struct Workspace {
     /// The [`Url`] pointing to the root of the workspace.
     url: Url,
-    /// The client settings for this workspace.
-    settings: Option<ClientSettings>,
+    /// The client options for this workspace.
+    options: Option<ClientOptions>,
     /// Whether this is the default workspace as created by the server. This will be the case when
     /// no workspace folders were provided during initialization.
     is_default: bool,
@@ -88,7 +87,7 @@ impl Workspace {
     pub fn new(url: Url) -> Self {
         Self {
             url,
-            settings: None,
+            options: None,
             is_default: false,
         }
     }
@@ -97,15 +96,15 @@ impl Workspace {
     pub fn default(url: Url) -> Self {
         Self {
             url,
-            settings: None,
+            options: None,
             is_default: true,
         }
     }
 
-    /// Set the client settings for this workspace.
+    /// Set the client options for this workspace.
     #[must_use]
-    pub fn with_settings(mut self, settings: ClientSettings) -> Self {
-        self.settings = Some(settings);
+    pub fn with_options(mut self, options: ClientOptions) -> Self {
+        self.options = Some(options);
         self
     }
 
@@ -114,9 +113,9 @@ impl Workspace {
         &self.url
     }
 
-    /// Returns the client settings for this workspace.
-    pub(crate) fn settings(&self) -> Option<&ClientSettings> {
-        self.settings.as_ref()
+    /// Returns the client options for this workspace.
+    pub(crate) fn options(&self) -> Option<&ClientOptions> {
+        self.options.as_ref()
     }
 
     /// Returns true if this is the default workspace.

crates/ruff_server/tests/notebook.rs

@@ -8,7 +8,7 @@ use lsp_types::{
     Position, Range, TextDocumentContentChangeEvent, VersionedTextDocumentIdentifier,
 };
 use ruff_notebook::SourceValue;
-use ruff_server::{ClientSettings, Workspace, Workspaces};
+use ruff_server::{ClientOptions, GlobalOptions, Workspace, Workspaces};
 
 const SUPER_RESOLUTION_OVERVIEW_PATH: &str =
     "./resources/test/fixtures/tensorflow_test_notebook.ipynb";
@@ -28,13 +28,16 @@ fn super_resolution_overview() {
 
     insta::assert_snapshot!("initial_notebook", notebook_source(&notebook));
 
+    let options = GlobalOptions::default();
+    let global = options.into_settings();
+
     let mut session = ruff_server::Session::new(
         &ClientCapabilities::default(),
         ruff_server::PositionEncoding::UTF16,
-        ClientSettings::default(),
+        global,
         &Workspaces::new(vec![
             Workspace::new(lsp_types::Url::from_file_path(file_path.parent().unwrap()).unwrap())
-                .with_settings(ClientSettings::default()),
+                .with_options(ClientOptions::default()),
         ]),
     )
     .unwrap();