From 3c6c017950db492dd71e33b0e114fda5dcdfb308 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 12 Jun 2025 18:58:30 +0200 Subject: [PATCH] Centralize client options validation (#18623) --- crates/ruff_server/src/lib.rs | 2 +- crates/ruff_server/src/server.rs | 29 +- crates/ruff_server/src/session.rs | 30 +- crates/ruff_server/src/session/index.rs | 57 +- .../src/session/index/ruff_settings.rs | 21 +- crates/ruff_server/src/session/options.rs | 1005 +++++++++++++++++ crates/ruff_server/src/session/settings.rs | 973 +--------------- crates/ruff_server/src/workspace.rs | 41 +- crates/ruff_server/tests/notebook.rs | 9 +- 9 files changed, 1164 insertions(+), 1003 deletions(-) create mode 100644 crates/ruff_server/src/session/options.rs diff --git a/crates/ruff_server/src/lib.rs b/crates/ruff_server/src/lib.rs index 44d5e1bb52..ca4ee50ab8 100644 --- a/crates/ruff_server/src/lib.rs +++ b/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] diff --git a/crates/ruff_server/src/server.rs b/crates/ruff_server/src/server.rs index 6614565f03..9e9c462f3f 100644 --- a/crates/ruff_server/src/server.rs +++ b/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, }) } diff --git a/crates/ruff_server/src/session.rs b/crates/ruff_server/src/session.rs index 00fd9d6013..79cc059b4c 100644 --- a/crates/ruff_server/src/session.rs +++ b/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, } @@ -35,7 +38,7 @@ pub struct Session { /// a specific document. pub struct DocumentSnapshot { resolved_client_capabilities: Arc, - client_settings: settings::ResolvedClientSettings, + client_settings: Arc, 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 { 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 } diff --git a/crates/ruff_server/src/session/index.rs b/crates/ruff_server/src/session/index.rs index 8f1993e12c..2e56499ecc 100644 --- a/crates/ruff_server/src/session/index.rs +++ b/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, 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 { 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 { 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> { + 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( diff --git a/crates/ruff_server/src/session/index/ruff_settings.rs b/crates/ruff_server/src/session/index/ruff_settings.rs index 3156d6e0f0..0f58ac2d94 100644 --- a/crates/ruff_server/src/session/index/ruff_settings.rs +++ b/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 { @@ -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() diff --git a/crates/ruff_server/src/session/options.rs b/crates/ruff_server/src/session/options.rs new file mode 100644 index 0000000000..47ce2fa83e --- /dev/null +++ b/crates/ruff_server/src/session/options.rs @@ -0,0 +1,1005 @@ +use std::{path::PathBuf, str::FromStr as _}; + +use lsp_types::Url; +use rustc_hash::FxHashMap; +use serde::Deserialize; +use serde_json::{Map, Value}; + +use ruff_linter::{RuleSelector, line_width::LineLength, rule_selector::ParseError}; + +use crate::session::settings::{ + ClientSettings, EditorSettings, GlobalClientSettings, ResolvedConfiguration, +}; + +pub(crate) type WorkspaceOptionsMap = FxHashMap; + +/// 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, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +pub(crate) enum ConfigurationPreference { + /// Configuration set in the editor takes priority over 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, +} + +/// A direct representation of of `configuration` schema within the client settings. +#[derive(Clone, Debug, Deserialize)] +#[cfg_attr(test, derive(PartialEq, Eq))] +#[serde(untagged)] +pub(super) enum ClientConfiguration { + /// A path to a configuration file. + String(String), + /// An object containing the configuration options. + Object(Map), +} + +#[derive(Debug, Deserialize, Default)] +#[cfg_attr(test, derive(PartialEq, Eq))] +#[serde(rename_all = "camelCase")] +pub struct GlobalOptions { + #[serde(flatten)] + client: ClientOptions, + + // These settings are only needed for tracing, and are only read from the global configuration. + // These will not be in the resolved settings. + #[serde(flatten)] + pub(crate) tracing: TracingOptions, +} + +impl GlobalOptions { + pub(crate) fn set_preview(&mut self, preview: bool) { + self.client.set_preview(preview); + } + + #[cfg(test)] + pub(crate) fn client(&self) -> &ClientOptions { + &self.client + } + + pub fn into_settings(self) -> GlobalClientSettings { + GlobalClientSettings { + options: self.client, + settings: std::cell::OnceCell::default(), + } + } +} + +/// This is a direct representation of the settings schema sent by the client. +#[derive(Clone, Debug, Deserialize, Default)] +#[cfg_attr(test, derive(PartialEq, Eq))] +#[serde(rename_all = "camelCase")] +pub struct ClientOptions { + configuration: Option, + fix_all: Option, + organize_imports: Option, + lint: Option, + format: Option, + code_action: Option, + exclude: Option>, + line_length: Option, + configuration_preference: Option, + + /// If `true` or [`None`], show syntax errors as diagnostics. + /// + /// This is useful when using Ruff with other language servers, allowing the user to refer + /// to syntax errors from only one source. + show_syntax_errors: Option, +} + +impl ClientOptions { + /// Resolves the options. + /// + /// Returns `Ok` if all options are valid. Otherwise, returns `Err` with the partially resolved settings + /// (ignoring any invalid settings). Error messages about the invalid settings are logged with tracing. + #[expect( + clippy::result_large_err, + reason = "The error is as large as the Ok variant" + )] + pub(crate) fn into_settings(self) -> Result { + let code_action = self.code_action.unwrap_or_default(); + let lint = self.lint.unwrap_or_default(); + let format = self.format.unwrap_or_default(); + let mut contains_invalid_settings = false; + + let configuration = self.configuration.and_then(|configuration| { + match ResolvedConfiguration::try_from(configuration) { + Ok(configuration) => Some(configuration), + Err(err) => { + tracing::error!("Failed to load settings from `configuration`: {err}"); + contains_invalid_settings = true; + None + } + } + }); + + let editor_settings = EditorSettings { + configuration, + lint_preview: lint.preview, + format_preview: format.preview, + select: lint.select.and_then(|select| { + Self::resolve_rules( + &select, + RuleSelectorKey::Select, + &mut contains_invalid_settings, + ) + }), + extend_select: lint.extend_select.and_then(|select| { + Self::resolve_rules( + &select, + RuleSelectorKey::ExtendSelect, + &mut contains_invalid_settings, + ) + }), + ignore: lint.ignore.and_then(|ignore| { + Self::resolve_rules( + &ignore, + RuleSelectorKey::Ignore, + &mut contains_invalid_settings, + ) + }), + exclude: self.exclude.clone(), + line_length: self.line_length, + configuration_preference: self.configuration_preference.unwrap_or_default(), + }; + + let resolved = ClientSettings { + editor_settings, + fix_all: self.fix_all.unwrap_or(true), + organize_imports: self.organize_imports.unwrap_or(true), + lint_enable: lint.enable.unwrap_or(true), + disable_rule_comment_enable: code_action + .disable_rule_comment + .and_then(|disable| disable.enable) + .unwrap_or(true), + fix_violation_enable: code_action + .fix_violation + .and_then(|fix| fix.enable) + .unwrap_or(true), + + show_syntax_errors: self.show_syntax_errors.unwrap_or(true), + }; + + if contains_invalid_settings { + Err(resolved) + } else { + Ok(resolved) + } + } + + fn resolve_rules( + rules: &[String], + key: RuleSelectorKey, + contains_invalid_settings: &mut bool, + ) -> Option> { + let (mut known, mut unknown) = (vec![], vec![]); + for rule in rules { + match RuleSelector::from_str(rule) { + Ok(selector) => known.push(selector), + Err(ParseError::Unknown(_)) => unknown.push(rule), + } + } + if !unknown.is_empty() { + *contains_invalid_settings = true; + tracing::error!("Unknown rule selectors found in `{key}`: {unknown:?}"); + } + if known.is_empty() { None } else { Some(known) } + } + + /// Update the preview flag for the linter and the formatter with the given value. + pub(crate) fn set_preview(&mut self, preview: bool) { + match self.lint.as_mut() { + None => self.lint = Some(LintOptions::default().with_preview(preview)), + Some(lint) => lint.set_preview(preview), + } + match self.format.as_mut() { + None => self.format = Some(FormatOptions::default().with_preview(preview)), + Some(format) => format.set_preview(preview), + } + } +} + +impl Combine for ClientOptions { + fn combine_with(&mut self, other: Self) { + self.configuration.combine_with(other.configuration); + self.fix_all.combine_with(other.fix_all); + self.organize_imports.combine_with(other.organize_imports); + self.lint.combine_with(other.lint); + self.format.combine_with(other.format); + self.code_action.combine_with(other.code_action); + self.exclude.combine_with(other.exclude); + self.line_length.combine_with(other.line_length); + self.configuration_preference + .combine_with(other.configuration_preference); + self.show_syntax_errors + .combine_with(other.show_syntax_errors); + } +} + +/// Settings needed to initialize tracing. These will only be +/// read from the global configuration. +#[derive(Debug, Deserialize, Default)] +#[cfg_attr(test, derive(PartialEq, Eq))] +#[serde(rename_all = "camelCase")] +pub(crate) struct TracingOptions { + pub(crate) log_level: Option, + /// Path to the log file - tildes and environment variables are supported. + pub(crate) log_file: Option, +} + +/// This is a direct representation of the workspace settings schema, +/// which inherits the schema of [`ClientOptions`] and adds extra fields +/// to describe the workspace it applies to. +#[derive(Debug, Deserialize)] +#[cfg_attr(test, derive(PartialEq, Eq))] +#[serde(rename_all = "camelCase")] +struct WorkspaceOptions { + #[serde(flatten)] + options: ClientOptions, + workspace: Url, +} + +#[derive(Clone, Debug, Default, Deserialize)] +#[cfg_attr(test, derive(PartialEq, Eq))] +#[serde(rename_all = "camelCase")] +struct LintOptions { + enable: Option, + preview: Option, + select: Option>, + extend_select: Option>, + ignore: Option>, +} + +impl LintOptions { + fn with_preview(mut self, preview: bool) -> LintOptions { + self.preview = Some(preview); + self + } + + fn set_preview(&mut self, preview: bool) { + self.preview = Some(preview); + } +} + +impl Combine for LintOptions { + fn combine_with(&mut self, other: Self) { + self.enable.combine_with(other.enable); + self.preview.combine_with(other.preview); + self.select.combine_with(other.select); + self.extend_select.combine_with(other.extend_select); + self.ignore.combine_with(other.ignore); + } +} + +#[derive(Clone, Debug, Default, Deserialize)] +#[cfg_attr(test, derive(PartialEq, Eq))] +#[serde(rename_all = "camelCase")] +struct FormatOptions { + preview: Option, +} + +impl Combine for FormatOptions { + fn combine_with(&mut self, other: Self) { + self.preview.combine_with(other.preview); + } +} + +impl FormatOptions { + fn with_preview(mut self, preview: bool) -> FormatOptions { + self.preview = Some(preview); + self + } + + fn set_preview(&mut self, preview: bool) { + self.preview = Some(preview); + } +} + +#[derive(Clone, Debug, Default, Deserialize)] +#[cfg_attr(test, derive(PartialEq, Eq))] +#[serde(rename_all = "camelCase")] +struct CodeActionOptions { + disable_rule_comment: Option, + fix_violation: Option, +} + +impl Combine for CodeActionOptions { + fn combine_with(&mut self, other: Self) { + self.disable_rule_comment + .combine_with(other.disable_rule_comment); + self.fix_violation.combine_with(other.fix_violation); + } +} + +#[derive(Clone, Debug, Deserialize)] +#[cfg_attr(test, derive(PartialEq, Eq))] +#[serde(rename_all = "camelCase")] +struct CodeActionParameters { + enable: Option, +} + +impl Combine for CodeActionParameters { + fn combine_with(&mut self, other: Self) { + self.enable.combine_with(other.enable); + } +} + +/// This is the exact schema for initialization options sent in by the client +/// during initialization. +#[derive(Debug, Deserialize)] +#[cfg_attr(test, derive(PartialEq, Eq))] +#[serde(untagged)] +enum InitializationOptions { + #[serde(rename_all = "camelCase")] + HasWorkspaces { + #[serde(rename = "globalSettings")] + global: GlobalOptions, + #[serde(rename = "settings")] + workspace: Vec, + }, + GlobalOnly { + #[serde(default)] + settings: GlobalOptions, + }, +} + +impl Default for InitializationOptions { + fn default() -> Self { + Self::GlobalOnly { + settings: GlobalOptions::default(), + } + } +} + +/// Built from the initialization options provided by the client. +#[derive(Debug)] +pub(crate) struct AllOptions { + pub(crate) global: GlobalOptions, + /// If this is `None`, the client only passed in global settings. + pub(crate) workspace: Option, +} + +impl AllOptions { + /// Initializes the controller from the serialized initialization options. + /// This fails if `options` are not valid initialization options. + pub(crate) fn from_value(options: serde_json::Value) -> Self { + Self::from_init_options( + serde_json::from_value(options) + .map_err(|err| { + tracing::error!("Failed to deserialize initialization options: {err}. Falling back to default client settings..."); + show_err_msg!("Ruff received invalid client settings - falling back to default client settings."); + }) + .unwrap_or_default(), + ) + } + + /// Update the preview flag for both the global and all workspace settings. + pub(crate) fn set_preview(&mut self, preview: bool) { + self.global.set_preview(preview); + if let Some(workspace_options) = self.workspace.as_mut() { + for options in workspace_options.values_mut() { + options.set_preview(preview); + } + } + } + + fn from_init_options(options: InitializationOptions) -> Self { + let (global_options, workspace_options) = match options { + InitializationOptions::GlobalOnly { settings: options } => (options, None), + InitializationOptions::HasWorkspaces { + global: global_options, + workspace: workspace_options, + } => (global_options, Some(workspace_options)), + }; + + Self { + global: global_options, + workspace: workspace_options.map(|workspace_options| { + workspace_options + .into_iter() + .map(|workspace_options| { + (workspace_options.workspace, workspace_options.options) + }) + .collect() + }), + } + } +} + +#[derive(Copy, Clone)] +enum RuleSelectorKey { + Select, + ExtendSelect, + Ignore, +} + +impl std::fmt::Display for RuleSelectorKey { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + RuleSelectorKey::Select => f.write_str("lint.select"), + RuleSelectorKey::ExtendSelect => f.write_str("lint.extendSelect"), + RuleSelectorKey::Ignore => f.write_str("lint.ignore"), + } + } +} + +pub(crate) 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 Combine for Option +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 Combine for Vec { + fn combine_with(&mut self, _other: Self) { + // No-op, use own elements + } +} + +/// 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 + } + } + }; +} + +// 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); + +// Custom types +impl_noop_combine!(ConfigurationPreference); +impl_noop_combine!(ClientConfiguration); +impl_noop_combine!(LineLength); + +#[cfg(test)] +mod tests { + use insta::assert_debug_snapshot; + use ruff_python_formatter::QuoteStyle; + use ruff_workspace::options::{ + FormatOptions as RuffFormatOptions, LintCommonOptions, LintOptions, Options, + }; + use serde::de::DeserializeOwned; + + #[cfg(not(windows))] + use ruff_linter::registry::Linter; + + use super::*; + + #[cfg(not(windows))] + const VS_CODE_INIT_OPTIONS_FIXTURE: &str = + include_str!("../../resources/test/fixtures/settings/vs_code_initialization_options.json"); + const GLOBAL_ONLY_INIT_OPTIONS_FIXTURE: &str = + include_str!("../../resources/test/fixtures/settings/global_only.json"); + const EMPTY_INIT_OPTIONS_FIXTURE: &str = + include_str!("../../resources/test/fixtures/settings/empty.json"); + + // This fixture contains multiple workspaces with empty initialization options. It only sets + // the `cwd` and the `workspace` value. + const EMPTY_MULTIPLE_WORKSPACE_INIT_OPTIONS_FIXTURE: &str = + include_str!("../../resources/test/fixtures/settings/empty_multiple_workspace.json"); + + const INLINE_CONFIGURATION_FIXTURE: &str = + include_str!("../../resources/test/fixtures/settings/inline_configuration.json"); + + fn deserialize_fixture(content: &str) -> T { + serde_json::from_str(content).expect("test fixture JSON should deserialize") + } + + #[cfg(not(windows))] + #[test] + fn test_vs_code_init_options_deserialize() { + let options: InitializationOptions = deserialize_fixture(VS_CODE_INIT_OPTIONS_FIXTURE); + + assert_debug_snapshot!(options, @r#" + HasWorkspaces { + global: GlobalOptions { + client: ClientOptions { + configuration: None, + fix_all: Some( + false, + ), + organize_imports: Some( + true, + ), + lint: Some( + LintOptions { + enable: Some( + true, + ), + preview: Some( + true, + ), + select: Some( + [ + "F", + "I", + ], + ), + extend_select: None, + ignore: None, + }, + ), + format: Some( + FormatOptions { + preview: None, + }, + ), + code_action: Some( + CodeActionOptions { + disable_rule_comment: Some( + CodeActionParameters { + enable: Some( + false, + ), + }, + ), + fix_violation: Some( + CodeActionParameters { + enable: Some( + false, + ), + }, + ), + }, + ), + exclude: None, + line_length: None, + configuration_preference: None, + show_syntax_errors: Some( + true, + ), + }, + tracing: TracingOptions { + log_level: None, + log_file: None, + }, + }, + workspace: [ + WorkspaceOptions { + options: ClientOptions { + configuration: None, + fix_all: Some( + true, + ), + organize_imports: Some( + true, + ), + lint: Some( + LintOptions { + enable: Some( + true, + ), + preview: None, + select: None, + extend_select: None, + ignore: None, + }, + ), + format: Some( + FormatOptions { + preview: None, + }, + ), + code_action: Some( + CodeActionOptions { + disable_rule_comment: Some( + CodeActionParameters { + enable: Some( + false, + ), + }, + ), + fix_violation: Some( + CodeActionParameters { + enable: Some( + false, + ), + }, + ), + }, + ), + exclude: None, + line_length: None, + configuration_preference: None, + show_syntax_errors: Some( + true, + ), + }, + workspace: Url { + scheme: "file", + cannot_be_a_base: false, + username: "", + password: None, + host: None, + port: None, + path: "/Users/test/projects/pandas", + query: None, + fragment: None, + }, + }, + WorkspaceOptions { + options: ClientOptions { + configuration: None, + fix_all: Some( + true, + ), + organize_imports: Some( + true, + ), + lint: Some( + LintOptions { + enable: Some( + true, + ), + preview: Some( + false, + ), + select: None, + extend_select: None, + ignore: None, + }, + ), + format: Some( + FormatOptions { + preview: None, + }, + ), + code_action: Some( + CodeActionOptions { + disable_rule_comment: Some( + CodeActionParameters { + enable: Some( + true, + ), + }, + ), + fix_violation: Some( + CodeActionParameters { + enable: Some( + false, + ), + }, + ), + }, + ), + exclude: None, + line_length: None, + configuration_preference: None, + show_syntax_errors: Some( + true, + ), + }, + workspace: Url { + scheme: "file", + cannot_be_a_base: false, + username: "", + password: None, + host: None, + port: None, + path: "/Users/test/projects/scipy", + query: None, + fragment: None, + }, + }, + ], + } + "#); + } + + #[cfg(not(windows))] + #[test] + fn test_vs_code_workspace_settings_resolve() { + let options = deserialize_fixture(VS_CODE_INIT_OPTIONS_FIXTURE); + let AllOptions { + global, + workspace: workspace_options, + } = AllOptions::from_init_options(options); + let path = + Url::from_str("file:///Users/test/projects/pandas").expect("path should be valid"); + let all_workspace_options = workspace_options.expect("workspace options should exist"); + + let workspace_options = all_workspace_options + .get(&path) + .expect("workspace options should exist") + .clone(); + let workspace_settings = workspace_options + .combine(global.client().clone()) + .into_settings() + .unwrap(); + + assert_eq!( + workspace_settings, + ClientSettings { + fix_all: true, + organize_imports: true, + lint_enable: true, + disable_rule_comment_enable: false, + fix_violation_enable: false, + show_syntax_errors: true, + editor_settings: EditorSettings { + configuration: None, + lint_preview: Some(true), + format_preview: None, + select: Some(vec![ + RuleSelector::Linter(Linter::Pyflakes), + RuleSelector::Linter(Linter::Isort) + ]), + extend_select: None, + ignore: None, + exclude: None, + line_length: None, + configuration_preference: ConfigurationPreference::default(), + }, + } + ); + let path = + Url::from_str("file:///Users/test/projects/scipy").expect("path should be valid"); + let workspace_options = all_workspace_options + .get(&path) + .expect("workspace setting should exist") + .clone(); + + let workspace_settings = workspace_options + .combine(global.client().clone()) + .into_settings() + .unwrap(); + + assert_eq!( + workspace_settings, + ClientSettings { + fix_all: true, + organize_imports: true, + lint_enable: true, + disable_rule_comment_enable: true, + fix_violation_enable: false, + show_syntax_errors: true, + editor_settings: EditorSettings { + configuration: None, + lint_preview: Some(false), + format_preview: None, + select: Some(vec![ + RuleSelector::Linter(Linter::Pyflakes), + RuleSelector::Linter(Linter::Isort) + ]), + extend_select: None, + ignore: None, + exclude: None, + line_length: None, + configuration_preference: ConfigurationPreference::EditorFirst, + }, + } + ); + } + + #[test] + fn test_global_only_init_options_deserialize() { + let options: InitializationOptions = deserialize_fixture(GLOBAL_ONLY_INIT_OPTIONS_FIXTURE); + + assert_debug_snapshot!(options, @r#" + GlobalOnly { + settings: GlobalOptions { + client: ClientOptions { + configuration: None, + fix_all: Some( + false, + ), + organize_imports: None, + lint: Some( + LintOptions { + enable: None, + preview: None, + select: None, + extend_select: None, + ignore: Some( + [ + "RUF001", + ], + ), + }, + ), + format: None, + code_action: Some( + CodeActionOptions { + disable_rule_comment: Some( + CodeActionParameters { + enable: Some( + false, + ), + }, + ), + fix_violation: None, + }, + ), + exclude: Some( + [ + "third_party", + ], + ), + line_length: Some( + LineLength( + 80, + ), + ), + configuration_preference: None, + show_syntax_errors: None, + }, + tracing: TracingOptions { + log_level: Some( + Warn, + ), + log_file: None, + }, + }, + } + "#); + } + + #[test] + fn test_global_only_resolves_correctly() { + let options = deserialize_fixture(GLOBAL_ONLY_INIT_OPTIONS_FIXTURE); + + let AllOptions { global, .. } = AllOptions::from_init_options(options); + let global = global.into_settings(); + assert_eq!( + global.to_settings(), + &ClientSettings { + fix_all: false, + organize_imports: true, + lint_enable: true, + disable_rule_comment_enable: false, + fix_violation_enable: true, + show_syntax_errors: true, + editor_settings: EditorSettings { + configuration: None, + lint_preview: None, + format_preview: None, + select: None, + 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()), + configuration_preference: ConfigurationPreference::EditorFirst, + }, + } + ); + } + + #[test] + fn test_empty_init_options_deserialize() { + let options: InitializationOptions = deserialize_fixture(EMPTY_INIT_OPTIONS_FIXTURE); + + assert_eq!(options, InitializationOptions::default()); + } + + fn assert_preview_client_options(options: &ClientOptions, preview: bool) { + assert_eq!(options.lint.as_ref().unwrap().preview.unwrap(), preview); + assert_eq!(options.format.as_ref().unwrap().preview.unwrap(), preview); + } + + fn assert_preview_all_options(all_options: &AllOptions, preview: bool) { + assert_preview_client_options(all_options.global.client(), preview); + if let Some(workspace_options) = all_options.workspace.as_ref() { + for options in workspace_options.values() { + assert_preview_client_options(options, preview); + } + } + } + + #[test] + fn test_preview_flag() { + let options = deserialize_fixture(EMPTY_MULTIPLE_WORKSPACE_INIT_OPTIONS_FIXTURE); + let mut all_options = AllOptions::from_init_options(options); + + all_options.set_preview(false); + assert_preview_all_options(&all_options, false); + + all_options.set_preview(true); + assert_preview_all_options(&all_options, true); + } + + #[test] + fn inline_configuration() { + let options: InitializationOptions = deserialize_fixture(INLINE_CONFIGURATION_FIXTURE); + + let AllOptions { + global, + workspace: None, + } = AllOptions::from_init_options(options) + else { + panic!("Expected global settings only"); + }; + + let global = global.into_settings(); + + assert_eq!( + global.to_settings(), + &ClientSettings { + fix_all: true, + organize_imports: true, + lint_enable: true, + disable_rule_comment_enable: true, + fix_violation_enable: true, + show_syntax_errors: true, + editor_settings: EditorSettings { + configuration: Some(ResolvedConfiguration::Inline(Box::new(Options { + line_length: Some(LineLength::try_from(100).unwrap()), + lint: Some(LintOptions { + common: LintCommonOptions { + extend_select: Some(vec![RuleSelector::from_str("I001").unwrap()]), + ..Default::default() + }, + ..Default::default() + }), + format: Some(RuffFormatOptions { + quote_style: Some(QuoteStyle::Single), + ..Default::default() + }), + ..Default::default() + }))), + extend_select: Some(vec![RuleSelector::from_str("RUF001").unwrap()]), + ..Default::default() + } + } + ); + } +} diff --git a/crates/ruff_server/src/session/settings.rs b/crates/ruff_server/src/session/settings.rs index f251594a0a..4015109e74 100644 --- a/crates/ruff_server/src/session/settings.rs +++ b/crates/ruff_server/src/session/settings.rs @@ -1,18 +1,58 @@ -use std::{ops::Deref, path::PathBuf, str::FromStr}; +use std::{path::PathBuf, sync::Arc}; -use lsp_types::Url; -use rustc_hash::FxHashMap; -use serde::Deserialize; -use serde_json::{Map, Value}; use thiserror::Error; use ruff_linter::RuleSelector; use ruff_linter::line_width::LineLength; -use ruff_linter::rule_selector::ParseError; use ruff_workspace::options::Options; -/// Maps a workspace URI to its associated client settings. Used during server initialization. -pub(crate) type WorkspaceSettingsMap = FxHashMap; +use crate::{ + ClientOptions, + session::options::{ClientConfiguration, ConfigurationPreference}, +}; + +pub struct GlobalClientSettings { + pub(super) options: ClientOptions, + + /// Lazily initialized client settings to avoid showing error warnings + /// when a field of the global settings has any errors but the field is overridden + /// in the workspace settings. This can avoid showing unnecessary errors + /// when the workspace settings e.g. select some rules that aren't available in a specific workspace + /// and said workspace overrides the selected rules. + pub(super) settings: std::cell::OnceCell>, +} + +impl GlobalClientSettings { + pub(super) fn options(&self) -> &ClientOptions { + &self.options + } + + fn settings_impl(&self) -> &Arc { + self.settings.get_or_init(|| { + let settings = self.options.clone().into_settings(); + let settings = match settings { + Ok(settings) => settings, + Err(settings) => { + show_err_msg!( + "Ruff received invalid settings from the editor. Refer to the logs for more information." + ); + settings + } + }; + Arc::new(settings) + }) + } + + /// Lazily resolves the client options to the settings. + pub(super) fn to_settings(&self) -> &ClientSettings { + self.settings_impl() + } + + /// Lazily resolves the client options to the settings. + pub(super) fn to_settings_arc(&self) -> Arc { + self.settings_impl().clone() + } +} /// 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 @@ -20,14 +60,14 @@ pub(crate) type WorkspaceSettingsMap = FxHashMap; #[derive(Clone, Debug)] #[cfg_attr(test, derive(PartialEq, Eq))] #[expect(clippy::struct_excessive_bools)] -pub(crate) struct ResolvedClientSettings { - fix_all: bool, - organize_imports: bool, - lint_enable: bool, - disable_rule_comment_enable: bool, - fix_violation_enable: bool, - show_syntax_errors: bool, - editor_settings: ResolvedEditorSettings, +pub(crate) struct ClientSettings { + pub(super) fix_all: bool, + pub(super) organize_imports: bool, + pub(super) lint_enable: bool, + pub(super) disable_rule_comment_enable: bool, + pub(super) fix_violation_enable: bool, + pub(super) show_syntax_errors: bool, + pub(super) editor_settings: EditorSettings, } /// Contains the resolved values of 'editor settings' - Ruff configuration for the linter/formatter that was passed in via @@ -35,7 +75,7 @@ pub(crate) struct ResolvedClientSettings { /// if these were un-set. #[derive(Clone, Debug)] #[cfg_attr(test, derive(Default, PartialEq, Eq))] -pub(crate) struct ResolvedEditorSettings { +pub(crate) struct EditorSettings { pub(super) configuration: Option, pub(super) lint_preview: Option, pub(super) format_preview: Option, @@ -55,13 +95,13 @@ pub(crate) enum ResolvedConfiguration { Inline(Box), } -impl TryFrom<&ClientConfiguration> for ResolvedConfiguration { +impl TryFrom for ResolvedConfiguration { type Error = ResolvedConfigurationError; - fn try_from(value: &ClientConfiguration) -> Result { + fn try_from(value: ClientConfiguration) -> Result { match value { ClientConfiguration::String(path) => Ok(ResolvedConfiguration::FilePath( - PathBuf::from(shellexpand::full(path)?.as_ref()), + PathBuf::from(shellexpand::full(&path)?.as_ref()), )), ClientConfiguration::Object(map) => { let options = toml::Table::try_from(map)?.try_into::()?; @@ -89,404 +129,7 @@ pub(crate) enum ResolvedConfigurationError { ExtendNotSupported, } -/// 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, PartialEq, Eq)] -#[serde(rename_all = "camelCase")] -pub(crate) enum ConfigurationPreference { - /// Configuration set in the editor takes priority over 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, -} - -/// A direct representation of of `configuration` schema within the client settings. -#[derive(Debug, Deserialize)] -#[cfg_attr(test, derive(PartialEq, Eq))] -#[serde(untagged)] -enum ClientConfiguration { - /// A path to a configuration file. - String(String), - /// An object containing the configuration options. - Object(Map), -} - -/// This is a direct representation of the settings schema sent by the client. -#[derive(Debug, Deserialize, Default)] -#[cfg_attr(test, derive(PartialEq, Eq))] -#[serde(rename_all = "camelCase")] -pub struct ClientSettings { - configuration: Option, - fix_all: Option, - organize_imports: Option, - lint: Option, - format: Option, - code_action: Option, - exclude: Option>, - line_length: Option, - configuration_preference: Option, - - /// If `true` or [`None`], show syntax errors as diagnostics. - /// - /// This is useful when using Ruff with other language servers, allowing the user to refer - /// to syntax errors from only one source. - show_syntax_errors: Option, - - // These settings are only needed for tracing, and are only read from the global configuration. - // These will not be in the resolved settings. - #[serde(flatten)] - pub(crate) tracing: TracingSettings, -} - impl ClientSettings { - /// Update the preview flag for the linter and the formatter with the given value. - pub(crate) fn set_preview(&mut self, preview: bool) { - match self.lint.as_mut() { - None => self.lint = Some(LintOptions::default().with_preview(preview)), - Some(lint) => lint.set_preview(preview), - } - match self.format.as_mut() { - None => self.format = Some(FormatOptions::default().with_preview(preview)), - Some(format) => format.set_preview(preview), - } - } -} - -/// Settings needed to initialize tracing. These will only be -/// read from the global configuration. -#[derive(Debug, Deserialize, Default)] -#[cfg_attr(test, derive(PartialEq, Eq))] -#[serde(rename_all = "camelCase")] -pub(crate) struct TracingSettings { - pub(crate) log_level: Option, - /// Path to the log file - tildes and environment variables are supported. - pub(crate) log_file: Option, -} - -/// This is a direct representation of the workspace settings schema, -/// which inherits the schema of [`ClientSettings`] and adds extra fields -/// to describe the workspace it applies to. -#[derive(Debug, Deserialize)] -#[cfg_attr(test, derive(PartialEq, Eq))] -#[serde(rename_all = "camelCase")] -struct WorkspaceSettings { - #[serde(flatten)] - settings: ClientSettings, - workspace: Url, -} - -#[derive(Debug, Default, Deserialize)] -#[cfg_attr(test, derive(PartialEq, Eq))] -#[serde(rename_all = "camelCase")] -struct LintOptions { - enable: Option, - preview: Option, - select: Option>, - extend_select: Option>, - ignore: Option>, -} - -impl LintOptions { - fn with_preview(mut self, preview: bool) -> LintOptions { - self.preview = Some(preview); - self - } - - fn set_preview(&mut self, preview: bool) { - self.preview = Some(preview); - } -} - -#[derive(Debug, Default, Deserialize)] -#[cfg_attr(test, derive(PartialEq, Eq))] -#[serde(rename_all = "camelCase")] -struct FormatOptions { - preview: Option, -} - -impl FormatOptions { - fn with_preview(mut self, preview: bool) -> FormatOptions { - self.preview = Some(preview); - self - } - - fn set_preview(&mut self, preview: bool) { - self.preview = Some(preview); - } -} - -#[derive(Debug, Default, Deserialize)] -#[cfg_attr(test, derive(PartialEq, Eq))] -#[serde(rename_all = "camelCase")] -struct CodeActionOptions { - disable_rule_comment: Option, - fix_violation: Option, -} - -#[derive(Debug, Deserialize)] -#[cfg_attr(test, derive(PartialEq, Eq))] -#[serde(rename_all = "camelCase")] -struct CodeActionParameters { - enable: Option, -} - -/// This is the exact schema for initialization options sent in by the client -/// during initialization. -#[derive(Debug, Deserialize)] -#[cfg_attr(test, derive(PartialEq, Eq))] -#[serde(untagged)] -enum InitializationOptions { - #[serde(rename_all = "camelCase")] - HasWorkspaces { - global_settings: ClientSettings, - #[serde(rename = "settings")] - workspace_settings: Vec, - }, - GlobalOnly { - #[serde(default)] - settings: ClientSettings, - }, -} - -/// Built from the initialization options provided by the client. -#[derive(Debug)] -pub(crate) struct AllSettings { - pub(crate) global_settings: ClientSettings, - /// If this is `None`, the client only passed in global settings. - pub(crate) workspace_settings: Option, -} - -impl AllSettings { - /// Initializes the controller from the serialized initialization options. - /// This fails if `options` are not valid initialization options. - pub(crate) fn from_value(options: serde_json::Value) -> Self { - Self::from_init_options( - serde_json::from_value(options) - .map_err(|err| { - tracing::error!("Failed to deserialize initialization options: {err}. Falling back to default client settings..."); - show_err_msg!("Ruff received invalid client settings - falling back to default client settings."); - }) - .unwrap_or_default(), - ) - } - - /// Update the preview flag for both the global and all workspace settings. - pub(crate) fn set_preview(&mut self, preview: bool) { - self.global_settings.set_preview(preview); - if let Some(workspace_settings) = self.workspace_settings.as_mut() { - for settings in workspace_settings.values_mut() { - settings.set_preview(preview); - } - } - } - - fn from_init_options(options: InitializationOptions) -> Self { - let (global_settings, workspace_settings) = match options { - InitializationOptions::GlobalOnly { settings } => (settings, None), - InitializationOptions::HasWorkspaces { - global_settings, - workspace_settings, - } => (global_settings, Some(workspace_settings)), - }; - - Self { - global_settings, - workspace_settings: workspace_settings.map(|workspace_settings| { - workspace_settings - .into_iter() - .map(|settings| (settings.workspace, settings.settings)) - .collect() - }), - } - } -} - -impl ResolvedClientSettings { - /// Resolves a series of client settings, prioritizing workspace settings over global settings. - /// Any fields not specified by either are set to their defaults. - pub(super) fn with_workspace( - workspace_settings: &ClientSettings, - global_settings: &ClientSettings, - ) -> Self { - Self::new_impl(&[workspace_settings, global_settings]) - } - - /// Resolves global settings only. - pub(super) fn global(global_settings: &ClientSettings) -> Self { - Self::new_impl(&[global_settings]) - } - - fn new_impl(all_settings: &[&ClientSettings]) -> Self { - let mut contains_invalid_settings = false; - - let settings = Self { - fix_all: Self::resolve_or(all_settings, |settings| settings.fix_all, true), - organize_imports: Self::resolve_or( - all_settings, - |settings| settings.organize_imports, - true, - ), - lint_enable: Self::resolve_or( - all_settings, - |settings| settings.lint.as_ref()?.enable, - true, - ), - disable_rule_comment_enable: Self::resolve_or( - all_settings, - |settings| { - settings - .code_action - .as_ref()? - .disable_rule_comment - .as_ref()? - .enable - }, - true, - ), - fix_violation_enable: Self::resolve_or( - all_settings, - |settings| { - settings - .code_action - .as_ref()? - .fix_violation - .as_ref()? - .enable - }, - true, - ), - show_syntax_errors: Self::resolve_or( - all_settings, - |settings| settings.show_syntax_errors, - true, - ), - editor_settings: ResolvedEditorSettings { - configuration: Self::resolve_optional(all_settings, |settings| { - settings.configuration.as_ref().and_then(|configuration| { - match ResolvedConfiguration::try_from(configuration) { - Ok(configuration) => Some(configuration), - Err(err) => { - tracing::error!( - "Failed to load settings from `configuration`: {err}" - ); - contains_invalid_settings = true; - None - } - } - }) - }), - lint_preview: Self::resolve_optional(all_settings, |settings| { - settings.lint.as_ref()?.preview - }), - format_preview: Self::resolve_optional(all_settings, |settings| { - settings.format.as_ref()?.preview - }), - select: Self::resolve_optional(all_settings, |settings| { - Self::resolve_rules( - settings.lint.as_ref()?.select.as_ref()?, - RuleSelectorKey::Select, - &mut contains_invalid_settings, - ) - }), - extend_select: Self::resolve_optional(all_settings, |settings| { - Self::resolve_rules( - settings.lint.as_ref()?.extend_select.as_ref()?, - RuleSelectorKey::ExtendSelect, - &mut contains_invalid_settings, - ) - }), - ignore: Self::resolve_optional(all_settings, |settings| { - Self::resolve_rules( - settings.lint.as_ref()?.ignore.as_ref()?, - RuleSelectorKey::Ignore, - &mut contains_invalid_settings, - ) - }), - exclude: Self::resolve_optional(all_settings, |settings| settings.exclude.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, - ), - }, - }; - - if contains_invalid_settings { - show_err_msg!( - "Ruff received invalid settings from the editor. Refer to the logs for more information." - ); - } - - settings - } - - fn resolve_rules( - rules: &[String], - key: RuleSelectorKey, - contains_invalid_settings: &mut bool, - ) -> Option> { - let (mut known, mut unknown) = (vec![], vec![]); - for rule in rules { - match RuleSelector::from_str(rule) { - Ok(selector) => known.push(selector), - Err(ParseError::Unknown(_)) => unknown.push(rule), - } - } - if !unknown.is_empty() { - *contains_invalid_settings = true; - tracing::error!("Unknown rule selectors found in `{key}`: {unknown:?}"); - } - if known.is_empty() { None } else { Some(known) } - } - - /// Attempts to resolve a setting using a list of available client settings as sources. - /// Client settings that come earlier in the list take priority. This function is for fields - /// that do not have a default value and should be left unset. - /// Use [`ResolvedClientSettings::resolve_or`] for settings that should have default values. - fn resolve_optional( - all_settings: &[&ClientSettings], - get: impl FnMut(&ClientSettings) -> Option, - ) -> Option { - all_settings.iter().map(Deref::deref).find_map(get) - } - - /// Attempts to resolve a setting using a list of available client settings as sources. - /// Client settings that come earlier in the list take priority. `default` will be returned - /// if none of the settings specify the requested setting. - /// Use [`ResolvedClientSettings::resolve_optional`] if the setting should be optional instead - /// of having a default value. - fn resolve_or( - all_settings: &[&ClientSettings], - get: impl Fn(&ClientSettings) -> Option, - default: T, - ) -> T { - Self::resolve_optional(all_settings, get).unwrap_or(default) - } -} - -#[derive(Copy, Clone)] -enum RuleSelectorKey { - Select, - ExtendSelect, - Ignore, -} - -impl std::fmt::Display for RuleSelectorKey { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - RuleSelectorKey::Select => f.write_str("lint.select"), - RuleSelectorKey::ExtendSelect => f.write_str("lint.extendSelect"), - RuleSelectorKey::Ignore => f.write_str("lint.ignore"), - } - } -} - -impl ResolvedClientSettings { pub(crate) fn fix_all(&self) -> bool { self.fix_all } @@ -511,501 +154,7 @@ impl ResolvedClientSettings { self.show_syntax_errors } - pub(crate) fn editor_settings(&self) -> &ResolvedEditorSettings { + pub(crate) fn editor_settings(&self) -> &EditorSettings { &self.editor_settings } } - -impl Default for InitializationOptions { - fn default() -> Self { - Self::GlobalOnly { - settings: ClientSettings::default(), - } - } -} - -#[cfg(test)] -mod tests { - use insta::assert_debug_snapshot; - use ruff_python_formatter::QuoteStyle; - use ruff_workspace::options::{ - FormatOptions as RuffFormatOptions, LintCommonOptions, LintOptions, - }; - use serde::de::DeserializeOwned; - - #[cfg(not(windows))] - use ruff_linter::registry::Linter; - - use super::*; - - #[cfg(not(windows))] - const VS_CODE_INIT_OPTIONS_FIXTURE: &str = - include_str!("../../resources/test/fixtures/settings/vs_code_initialization_options.json"); - const GLOBAL_ONLY_INIT_OPTIONS_FIXTURE: &str = - include_str!("../../resources/test/fixtures/settings/global_only.json"); - const EMPTY_INIT_OPTIONS_FIXTURE: &str = - include_str!("../../resources/test/fixtures/settings/empty.json"); - - // This fixture contains multiple workspaces with empty initialization options. It only sets - // the `cwd` and the `workspace` value. - const EMPTY_MULTIPLE_WORKSPACE_INIT_OPTIONS_FIXTURE: &str = - include_str!("../../resources/test/fixtures/settings/empty_multiple_workspace.json"); - - const INLINE_CONFIGURATION_FIXTURE: &str = - include_str!("../../resources/test/fixtures/settings/inline_configuration.json"); - - fn deserialize_fixture(content: &str) -> T { - serde_json::from_str(content).expect("test fixture JSON should deserialize") - } - - #[cfg(not(windows))] - #[test] - fn test_vs_code_init_options_deserialize() { - let options: InitializationOptions = deserialize_fixture(VS_CODE_INIT_OPTIONS_FIXTURE); - - assert_debug_snapshot!(options, @r###" - HasWorkspaces { - global_settings: ClientSettings { - configuration: None, - fix_all: Some( - false, - ), - organize_imports: Some( - true, - ), - lint: Some( - LintOptions { - enable: Some( - true, - ), - preview: Some( - true, - ), - select: Some( - [ - "F", - "I", - ], - ), - extend_select: None, - ignore: None, - }, - ), - format: Some( - FormatOptions { - preview: None, - }, - ), - code_action: Some( - CodeActionOptions { - disable_rule_comment: Some( - CodeActionParameters { - enable: Some( - false, - ), - }, - ), - fix_violation: Some( - CodeActionParameters { - enable: Some( - false, - ), - }, - ), - }, - ), - exclude: None, - line_length: None, - configuration_preference: None, - show_syntax_errors: Some( - true, - ), - tracing: TracingSettings { - log_level: None, - log_file: None, - }, - }, - workspace_settings: [ - WorkspaceSettings { - settings: ClientSettings { - configuration: None, - fix_all: Some( - true, - ), - organize_imports: Some( - true, - ), - lint: Some( - LintOptions { - enable: Some( - true, - ), - preview: None, - select: None, - extend_select: None, - ignore: None, - }, - ), - format: Some( - FormatOptions { - preview: None, - }, - ), - code_action: Some( - CodeActionOptions { - disable_rule_comment: Some( - CodeActionParameters { - enable: Some( - false, - ), - }, - ), - fix_violation: Some( - CodeActionParameters { - enable: Some( - false, - ), - }, - ), - }, - ), - exclude: None, - line_length: None, - configuration_preference: None, - show_syntax_errors: Some( - true, - ), - tracing: TracingSettings { - log_level: None, - log_file: None, - }, - }, - workspace: Url { - scheme: "file", - cannot_be_a_base: false, - username: "", - password: None, - host: None, - port: None, - path: "/Users/test/projects/pandas", - query: None, - fragment: None, - }, - }, - WorkspaceSettings { - settings: ClientSettings { - configuration: None, - fix_all: Some( - true, - ), - organize_imports: Some( - true, - ), - lint: Some( - LintOptions { - enable: Some( - true, - ), - preview: Some( - false, - ), - select: None, - extend_select: None, - ignore: None, - }, - ), - format: Some( - FormatOptions { - preview: None, - }, - ), - code_action: Some( - CodeActionOptions { - disable_rule_comment: Some( - CodeActionParameters { - enable: Some( - true, - ), - }, - ), - fix_violation: Some( - CodeActionParameters { - enable: Some( - false, - ), - }, - ), - }, - ), - exclude: None, - line_length: None, - configuration_preference: None, - show_syntax_errors: Some( - true, - ), - tracing: TracingSettings { - log_level: None, - log_file: None, - }, - }, - workspace: Url { - scheme: "file", - cannot_be_a_base: false, - username: "", - password: None, - host: None, - port: None, - path: "/Users/test/projects/scipy", - query: None, - fragment: None, - }, - }, - ], - } - "###); - } - - #[cfg(not(windows))] - #[test] - fn test_vs_code_workspace_settings_resolve() { - let options = deserialize_fixture(VS_CODE_INIT_OPTIONS_FIXTURE); - let AllSettings { - global_settings, - workspace_settings, - } = AllSettings::from_init_options(options); - let path = - Url::from_str("file:///Users/test/projects/pandas").expect("path should be valid"); - let workspace_settings = workspace_settings.expect("workspace settings should exist"); - assert_eq!( - ResolvedClientSettings::with_workspace( - workspace_settings - .get(&path) - .expect("workspace setting should exist"), - &global_settings - ), - ResolvedClientSettings { - fix_all: true, - organize_imports: true, - lint_enable: true, - disable_rule_comment_enable: false, - fix_violation_enable: false, - show_syntax_errors: true, - editor_settings: ResolvedEditorSettings { - configuration: None, - lint_preview: Some(true), - format_preview: None, - select: Some(vec![ - RuleSelector::Linter(Linter::Pyflakes), - RuleSelector::Linter(Linter::Isort) - ]), - extend_select: None, - ignore: None, - exclude: None, - line_length: None, - configuration_preference: ConfigurationPreference::default(), - }, - } - ); - let path = - Url::from_str("file:///Users/test/projects/scipy").expect("path should be valid"); - assert_eq!( - ResolvedClientSettings::with_workspace( - workspace_settings - .get(&path) - .expect("workspace setting should exist"), - &global_settings - ), - ResolvedClientSettings { - fix_all: true, - organize_imports: true, - lint_enable: true, - disable_rule_comment_enable: true, - fix_violation_enable: false, - show_syntax_errors: true, - editor_settings: ResolvedEditorSettings { - configuration: None, - lint_preview: Some(false), - format_preview: None, - select: Some(vec![ - RuleSelector::Linter(Linter::Pyflakes), - RuleSelector::Linter(Linter::Isort) - ]), - extend_select: None, - ignore: None, - exclude: None, - line_length: None, - configuration_preference: ConfigurationPreference::EditorFirst, - }, - } - ); - } - - #[test] - fn test_global_only_init_options_deserialize() { - let options: InitializationOptions = deserialize_fixture(GLOBAL_ONLY_INIT_OPTIONS_FIXTURE); - - assert_debug_snapshot!(options, @r#" - GlobalOnly { - settings: ClientSettings { - configuration: None, - fix_all: Some( - false, - ), - organize_imports: None, - lint: Some( - LintOptions { - enable: None, - preview: None, - select: None, - extend_select: None, - ignore: Some( - [ - "RUF001", - ], - ), - }, - ), - format: None, - code_action: Some( - CodeActionOptions { - disable_rule_comment: Some( - CodeActionParameters { - enable: Some( - false, - ), - }, - ), - fix_violation: None, - }, - ), - exclude: Some( - [ - "third_party", - ], - ), - line_length: Some( - LineLength( - 80, - ), - ), - configuration_preference: None, - show_syntax_errors: None, - tracing: TracingSettings { - log_level: Some( - Warn, - ), - log_file: None, - }, - }, - } - "#); - } - - #[test] - fn test_global_only_resolves_correctly() { - let options = deserialize_fixture(GLOBAL_ONLY_INIT_OPTIONS_FIXTURE); - - let AllSettings { - global_settings, .. - } = AllSettings::from_init_options(options); - assert_eq!( - ResolvedClientSettings::global(&global_settings), - ResolvedClientSettings { - fix_all: false, - organize_imports: true, - lint_enable: true, - disable_rule_comment_enable: false, - fix_violation_enable: true, - show_syntax_errors: true, - editor_settings: ResolvedEditorSettings { - configuration: None, - lint_preview: None, - format_preview: None, - select: None, - 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()), - configuration_preference: ConfigurationPreference::EditorFirst, - }, - } - ); - } - - #[test] - fn test_empty_init_options_deserialize() { - let options: InitializationOptions = deserialize_fixture(EMPTY_INIT_OPTIONS_FIXTURE); - - assert_eq!(options, InitializationOptions::default()); - } - - fn assert_preview_client_settings(settings: &ClientSettings, preview: bool) { - assert_eq!(settings.lint.as_ref().unwrap().preview.unwrap(), preview); - assert_eq!(settings.format.as_ref().unwrap().preview.unwrap(), preview); - } - - fn assert_preview_all_settings(all_settings: &AllSettings, preview: bool) { - assert_preview_client_settings(&all_settings.global_settings, preview); - if let Some(workspace_settings) = all_settings.workspace_settings.as_ref() { - for settings in workspace_settings.values() { - assert_preview_client_settings(settings, preview); - } - } - } - - #[test] - fn test_preview_flag() { - let options = deserialize_fixture(EMPTY_MULTIPLE_WORKSPACE_INIT_OPTIONS_FIXTURE); - let mut all_settings = AllSettings::from_init_options(options); - - all_settings.set_preview(false); - assert_preview_all_settings(&all_settings, false); - - all_settings.set_preview(true); - assert_preview_all_settings(&all_settings, true); - } - - #[test] - fn inline_configuration() { - let options: InitializationOptions = deserialize_fixture(INLINE_CONFIGURATION_FIXTURE); - - let AllSettings { - global_settings, - workspace_settings: None, - } = AllSettings::from_init_options(options) - else { - panic!("Expected global settings only"); - }; - - assert_eq!( - ResolvedClientSettings::global(&global_settings), - ResolvedClientSettings { - fix_all: true, - organize_imports: true, - lint_enable: true, - disable_rule_comment_enable: true, - fix_violation_enable: true, - show_syntax_errors: true, - editor_settings: ResolvedEditorSettings { - configuration: Some(ResolvedConfiguration::Inline(Box::new(Options { - line_length: Some(LineLength::try_from(100).unwrap()), - lint: Some(LintOptions { - common: LintCommonOptions { - extend_select: Some(vec![RuleSelector::from_str("I001").unwrap()]), - ..Default::default() - }, - ..Default::default() - }), - format: Some(RuffFormatOptions { - quote_style: Some(QuoteStyle::Single), - ..Default::default() - }), - ..Default::default() - }))), - extend_select: Some(vec![RuleSelector::from_str("RUF001").unwrap()]), - ..Default::default() - } - } - ); - } -} diff --git a/crates/ruff_server/src/workspace.rs b/crates/ruff_server/src/workspace.rs index d264d1e602..d8274e7669 100644 --- a/crates/ruff_server/src/workspace.rs +++ b/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); @@ -18,15 +17,15 @@ impl Workspaces { /// initialization. pub(crate) fn from_workspace_folders( workspace_folders: Option>, - mut workspace_settings: WorkspaceSettingsMap, + mut workspace_options: WorkspaceOptionsMap, ) -> std::result::Result { - 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, + /// The client options for this workspace. + options: Option, /// 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. diff --git a/crates/ruff_server/tests/notebook.rs b/crates/ruff_server/tests/notebook.rs index 64222fd04b..6d3e6be1db 100644 --- a/crates/ruff_server/tests/notebook.rs +++ b/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(¬ebook)); + 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();