From 037e817450d61bc5fdc55bb60eeab437f8c1180d Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 7 Aug 2024 14:56:59 +0530 Subject: [PATCH] Use struct instead of type alias for workspace settings index (#12726) ## Summary Follow-up from https://github.com/astral-sh/ruff/pull/12725, this is just a small refactor to use a wrapper struct instead of type alias for workspace settings index. This avoids the need to have the `register_workspace_settings` as a static method on `Index` and instead is a method on the new struct itself. --- crates/ruff_server/src/session/index.rs | 118 ++++++++++++++---------- 1 file changed, 69 insertions(+), 49 deletions(-) diff --git a/crates/ruff_server/src/session/index.rs b/crates/ruff_server/src/session/index.rs index cca313d817605..d648f8f251235 100644 --- a/crates/ruff_server/src/session/index.rs +++ b/crates/ruff_server/src/session/index.rs @@ -1,4 +1,5 @@ use std::borrow::Cow; +use std::ops::{Deref, DerefMut}; use std::path::PathBuf; use std::{collections::BTreeMap, path::Path, sync::Arc}; @@ -17,8 +18,6 @@ use super::{settings::ResolvedClientSettings, ClientSettings}; mod ruff_settings; -type SettingsIndex = BTreeMap; - /// Stores and tracks all open documents in a session, along with their associated settings. #[derive(Default)] pub(crate) struct Index { @@ -29,7 +28,7 @@ pub(crate) struct Index { notebook_cells: FxHashMap, /// Maps a workspace folder root to its settings. - settings: SettingsIndex, + settings: WorkspaceSettingsIndex, } /// Settings associated with a workspace. @@ -70,20 +69,15 @@ impl Index { workspace_folders: Vec<(Url, ClientSettings)>, global_settings: &ClientSettings, ) -> crate::Result { - let mut settings_index = BTreeMap::new(); + let mut settings = WorkspaceSettingsIndex::default(); for (url, workspace_settings) in workspace_folders { - Self::register_workspace_settings( - &mut settings_index, - &url, - Some(workspace_settings), - global_settings, - )?; + settings.register_workspace(&url, Some(workspace_settings), global_settings)?; } Ok(Self { documents: FxHashMap::default(), notebook_cells: FxHashMap::default(), - settings: settings_index, + settings, }) } @@ -176,7 +170,7 @@ impl Index { global_settings: &ClientSettings, ) -> crate::Result<()> { // TODO(jane): Find a way for workspace client settings to be added or changed dynamically. - Self::register_workspace_settings(&mut self.settings, url, None, global_settings) + self.settings.register_workspace(url, None, global_settings) } pub(super) fn num_documents(&self) -> usize { @@ -194,43 +188,6 @@ impl Index { .collect() } - fn register_workspace_settings( - settings_index: &mut SettingsIndex, - workspace_url: &Url, - workspace_settings: Option, - global_settings: &ClientSettings, - ) -> crate::Result<()> { - if workspace_url.scheme() != "file" { - tracing::warn!("Ignoring non-file workspace: {workspace_url}"); - show_warn_msg!("Ruff does not support non-file workspaces; Ignoring {workspace_url}"); - return Ok(()); - } - let workspace_path = workspace_url.to_file_path().map_err(|()| { - 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) - } else { - ResolvedClientSettings::global(global_settings) - }; - - let workspace_settings_index = ruff_settings::RuffSettingsIndex::new( - &workspace_path, - client_settings.editor_settings(), - ); - - settings_index.insert( - workspace_path, - WorkspaceSettings { - client_settings, - ruff_settings: workspace_settings_index, - }, - ); - - Ok(()) - } - pub(super) fn close_workspace_folder(&mut self, workspace_url: &Url) -> crate::Result<()> { let workspace_path = workspace_url.to_file_path().map_err(|()| { anyhow!("Failed to convert workspace URL to file path: {workspace_url}") @@ -427,6 +384,69 @@ impl Index { } } +/// Maps a workspace folder root to its settings. +#[derive(Default)] +struct WorkspaceSettingsIndex { + index: BTreeMap, +} + +impl WorkspaceSettingsIndex { + /// Register a workspace folder with the given settings. + /// + /// If the `workspace_settings` is [`Some`], it is preferred over the global settings for the + /// workspace. Otherwise, the global settings are used exclusively. + fn register_workspace( + &mut self, + workspace_url: &Url, + workspace_settings: Option, + global_settings: &ClientSettings, + ) -> crate::Result<()> { + if workspace_url.scheme() != "file" { + tracing::info!("Ignoring non-file workspace URL: {workspace_url}"); + show_warn_msg!("Ruff does not support non-file workspaces; Ignoring {workspace_url}"); + return Ok(()); + } + let workspace_path = workspace_url.to_file_path().map_err(|()| { + 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) + } else { + ResolvedClientSettings::global(global_settings) + }; + + let workspace_settings_index = ruff_settings::RuffSettingsIndex::new( + &workspace_path, + client_settings.editor_settings(), + ); + + self.insert( + workspace_path, + WorkspaceSettings { + client_settings, + ruff_settings: workspace_settings_index, + }, + ); + + Ok(()) + } +} + +impl Deref for WorkspaceSettingsIndex { + type Target = BTreeMap; + + fn deref(&self) -> &Self::Target { + &self.index + } +} + +impl DerefMut for WorkspaceSettingsIndex { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.index + } +} + impl DocumentController { fn new_text(document: TextDocument) -> Self { Self::Text(Arc::new(document))