diff --git a/crates/ruff_server/src/session/index/ruff_settings.rs b/crates/ruff_server/src/session/index/ruff_settings.rs index 08bb79e85e..b71cbdeec9 100644 --- a/crates/ruff_server/src/session/index/ruff_settings.rs +++ b/crates/ruff_server/src/session/index/ruff_settings.rs @@ -336,7 +336,6 @@ struct EditorConfigurationTransformer<'a>(&'a ResolvedEditorSettings, &'a Path); impl ConfigurationTransformer for EditorConfigurationTransformer<'_> { fn transform(&self, filesystem_configuration: Configuration) -> Configuration { let ResolvedEditorSettings { - configuration, format_preview, lint_preview, select, @@ -345,17 +344,18 @@ impl ConfigurationTransformer for EditorConfigurationTransformer<'_> { exclude, line_length, configuration_preference, - } = self.0.clone(); - - let project_root = self.1; + .. + } = self.0; let editor_configuration = Configuration { lint: LintConfiguration { preview: lint_preview.map(PreviewMode::from), rule_selections: vec![RuleSelection { - select, - extend_select: extend_select.unwrap_or_default(), - ignore: ignore.unwrap_or_default(), + select: select.clone(), + extend_select: extend_select + .as_ref() + .map_or_else(Default::default, Clone::clone), + ignore: ignore.as_ref().map_or_else(Default::default, Clone::clone), ..RuleSelection::default() }], ..LintConfiguration::default() @@ -364,36 +364,24 @@ impl ConfigurationTransformer for EditorConfigurationTransformer<'_> { preview: format_preview.map(PreviewMode::from), ..FormatConfiguration::default() }, - exclude: exclude.map(|exclude| { + exclude: exclude.as_ref().map(|exclude| { exclude - .into_iter() + .iter() .map(|pattern| { - let absolute = normalize_path_to(&pattern, project_root); - FilePattern::User(pattern, absolute) + let absolute = normalize_path_to(pattern, self.1); + FilePattern::User(pattern.clone(), absolute) }) .collect() }), - line_length, + line_length: *line_length, ..Configuration::default() }; // Merge in the editor-specified configuration file, if it exists. - let editor_configuration = if let Some(config_file_path) = configuration { - tracing::debug!( - "Combining settings from editor-specified configuration file at: {}", - config_file_path.display() - ); - match open_configuration_file(&config_file_path) { - Ok(config_from_file) => editor_configuration.combine(config_from_file), - err => { - tracing::error!( - "{:?}", - err.context("Unable to load editor-specified configuration file") - .unwrap_err() - ); - editor_configuration - } - } + let editor_configuration = if let Some(config) = self.0.configuration() { + // TODO(dhruvmanila): This log doesn't make sense without the surrounding context + tracing::debug!("Combining settings from editor-specified configuration file"); + editor_configuration.combine(config) } else { editor_configuration }; @@ -409,19 +397,3 @@ impl ConfigurationTransformer for EditorConfigurationTransformer<'_> { } } } - -fn open_configuration_file(config_path: &Path) -> crate::Result { - ruff_workspace::resolver::resolve_configuration( - config_path, - Relativity::Cwd, - &IdentityTransformer, - ) -} - -struct IdentityTransformer; - -impl ConfigurationTransformer for IdentityTransformer { - fn transform(&self, config: Configuration) -> Configuration { - config - } -} diff --git a/crates/ruff_server/src/session/settings.rs b/crates/ruff_server/src/session/settings.rs index 50bbd41307..559e80c9bd 100644 --- a/crates/ruff_server/src/session/settings.rs +++ b/crates/ruff_server/src/session/settings.rs @@ -5,6 +5,8 @@ use rustc_hash::FxHashMap; use serde::Deserialize; use ruff_linter::{line_width::LineLength, RuleSelector}; +use ruff_workspace::configuration::Configuration; +use ruff_workspace::resolver::{resolve_configuration, ConfigurationTransformer, Relativity}; /// Maps a workspace URI to its associated client settings. Used during server initialization. pub(crate) type WorkspaceSettingsMap = FxHashMap; @@ -13,7 +15,7 @@ pub(crate) type WorkspaceSettingsMap = FxHashMap; /// used directly by the server, and are *not* a 1:1 representation with how the client /// sends them. #[derive(Clone, Debug)] -#[cfg_attr(test, derive(PartialEq, Eq))] +#[cfg_attr(test, derive(PartialEq))] #[allow(clippy::struct_excessive_bools)] pub(crate) struct ResolvedClientSettings { fix_all: bool, @@ -29,9 +31,7 @@ pub(crate) struct ResolvedClientSettings { /// LSP client settings. These fields are optional because we don't want to override file-based linter/formatting settings /// if these were un-set. #[derive(Clone, Debug)] -#[cfg_attr(test, derive(PartialEq, Eq))] pub(crate) struct ResolvedEditorSettings { - pub(super) configuration: Option, pub(super) lint_preview: Option, pub(super) format_preview: Option, pub(super) select: Option>, @@ -40,6 +40,58 @@ pub(crate) struct ResolvedEditorSettings { pub(super) exclude: Option>, pub(super) line_length: Option, pub(super) configuration_preference: ConfigurationPreference, + + /// The resolved configuration from the editor settings, cached on first access. + resolved_configuration: Option, +} + +impl ResolvedEditorSettings { + /// Returns the resolved configuration from the editor settings, if it exists. + pub(super) fn configuration(&self) -> Option { + self.resolved_configuration + .as_ref()? + .configuration + .clone() + .map(|config| *config) + } +} + +#[derive(Clone, Debug)] +pub(crate) struct ResolvedConfiguration { + /// The path to the configuration file as specified in the editor settings. + #[allow(dead_code)] + path: PathBuf, + /// The resolved configuration from the editor settings. It will be `None` if the configuration + /// failed to load e.g., due to a file not existing or invalid configuration. + configuration: Option>, +} + +impl ResolvedConfiguration { + fn new(path: PathBuf) -> ResolvedConfiguration { + tracing::info!( + "Loading configuration from editor-specified file: {}", + path.display() + ); + let configuration = match resolve_configuration( + &path, + Relativity::Cwd, + &IdentityTransformer, + ) { + Ok(config) => Some(Box::new(config)), + Err(err) => { + tracing::error!("{:?}", err); + show_err_msg!( + "Failed to load editor-specified configuration file at {}. Refer to the logs for more details.", + path.display() + ); + None + } + }; + ResolvedConfiguration { + path, + configuration, + } + } } /// Determines how multiple conflicting configurations should be resolved - in this @@ -305,13 +357,14 @@ impl ResolvedClientSettings { true, ), editor_settings: ResolvedEditorSettings { - configuration: Self::resolve_optional(all_settings, |settings| { + resolved_configuration: Self::resolve_optional(all_settings, |settings| { settings .configuration .as_ref() .and_then(|config_path| shellexpand::full(config_path).ok()) .map(|config_path| PathBuf::from(config_path.as_ref())) - }), + }) + .map(ResolvedConfiguration::new), lint_preview: Self::resolve_optional(all_settings, |settings| { settings.lint.as_ref()?.preview }), @@ -422,6 +475,14 @@ impl Default for InitializationOptions { } } +struct IdentityTransformer; + +impl ConfigurationTransformer for IdentityTransformer { + fn transform(&self, config: Configuration) -> Configuration { + config + } +} + #[cfg(test)] mod tests { use insta::assert_debug_snapshot; @@ -449,6 +510,26 @@ mod tests { serde_json::from_str(content).expect("test fixture JSON should deserialize") } + impl PartialEq for ResolvedConfiguration { + fn eq(&self, other: &Self) -> bool { + self.path == other.path + } + } + + impl PartialEq for ResolvedEditorSettings { + fn eq(&self, other: &Self) -> bool { + self.resolved_configuration == other.resolved_configuration + && self.lint_preview == other.lint_preview + && self.format_preview == other.format_preview + && self.select == other.select + && self.extend_select == other.extend_select + && self.ignore == other.ignore + && self.exclude == other.exclude + && self.line_length == other.line_length + && self.configuration_preference == other.configuration_preference + } + } + #[cfg(not(windows))] #[test] fn test_vs_code_init_options_deserialize() { @@ -682,7 +763,7 @@ mod tests { fix_violation_enable: false, show_syntax_errors: true, editor_settings: ResolvedEditorSettings { - configuration: None, + resolved_configuration: None, lint_preview: Some(true), format_preview: None, select: Some(vec![ @@ -714,7 +795,7 @@ mod tests { fix_violation_enable: false, show_syntax_errors: true, editor_settings: ResolvedEditorSettings { - configuration: None, + resolved_configuration: None, lint_preview: Some(false), format_preview: None, select: Some(vec![ @@ -809,7 +890,7 @@ mod tests { fix_violation_enable: true, show_syntax_errors: true, editor_settings: ResolvedEditorSettings { - configuration: None, + resolved_configuration: None, lint_preview: None, format_preview: None, select: None,