From 2d30be20d6a582b56051cee3062682347691d422 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Sun, 14 Apr 2024 15:24:02 -0700 Subject: [PATCH 01/11] Restructure session code to be more modular --- crates/ruff_server/src/session.rs | 285 +----------------- crates/ruff_server/src/session/workspace.rs | 246 +++++++++++++++ .../src/session/workspace/configuration.rs | 36 +++ 3 files changed, 293 insertions(+), 274 deletions(-) create mode 100644 crates/ruff_server/src/session/workspace.rs create mode 100644 crates/ruff_server/src/session/workspace/configuration.rs diff --git a/crates/ruff_server/src/session.rs b/crates/ruff_server/src/session.rs index 72580c3a3022b..c23a199d85e08 100644 --- a/crates/ruff_server/src/session.rs +++ b/crates/ruff_server/src/session.rs @@ -2,27 +2,23 @@ mod capabilities; mod settings; +mod workspace; -use std::collections::BTreeMap; -use std::path::{Path, PathBuf}; -use std::{ops::Deref, sync::Arc}; +use std::sync::Arc; use anyhow::anyhow; use lsp_types::{ClientCapabilities, Url}; -use ruff_workspace::resolver::{ConfigurationTransformer, Relativity}; -use rustc_hash::FxHashMap; -use crate::edit::{Document, DocumentVersion}; +use crate::edit::DocumentVersion; use crate::PositionEncoding; pub(crate) use self::capabilities::ResolvedClientCapabilities; -use self::settings::ResolvedClientSettings; pub(crate) use self::settings::{AllSettings, ClientSettings}; /// The global state for the LSP pub(crate) struct Session { /// Workspace folders in the current session, which contain the state of all open files. - workspaces: Workspaces, + workspaces: workspace::Workspaces, /// The global position encoding, negotiated during LSP initialization. position_encoding: PositionEncoding, /// Global settings provided by the client. @@ -34,49 +30,14 @@ pub(crate) struct Session { /// An immutable snapshot of `Session` that references /// a specific document. pub(crate) struct DocumentSnapshot { - configuration: Arc, + configuration: Arc, resolved_client_capabilities: Arc, client_settings: settings::ResolvedClientSettings, - document_ref: DocumentRef, + document_ref: workspace::DocumentRef, position_encoding: PositionEncoding, url: Url, } -#[derive(Default)] -pub(crate) struct RuffConfiguration { - // settings to pass into the ruff linter - pub(crate) linter: ruff_linter::settings::LinterSettings, - // settings to pass into the ruff formatter - pub(crate) formatter: ruff_workspace::FormatterSettings, -} - -#[derive(Default)] -pub(crate) struct Workspaces(BTreeMap); - -pub(crate) struct Workspace { - open_documents: OpenDocuments, - configuration: Arc, - settings: ClientSettings, -} - -#[derive(Default)] -pub(crate) struct OpenDocuments { - documents: FxHashMap, -} - -/// A mutable handler to an underlying document. -/// Handles copy-on-write mutation automatically when -/// calling `deref_mut`. -pub(crate) struct DocumentController { - document: Arc, -} - -/// A read-only reference to a document. -#[derive(Clone)] -pub(crate) struct DocumentRef { - document: Arc, -} - impl Session { pub(crate) fn new( client_capabilities: &ClientCapabilities, @@ -90,7 +51,7 @@ impl Session { resolved_client_capabilities: Arc::new(ResolvedClientCapabilities::new( client_capabilities, )), - workspaces: Workspaces::new(workspaces)?, + workspaces: workspace::Workspaces::new(workspaces)?, }) } @@ -117,7 +78,7 @@ impl Session { pub(crate) fn document_controller( &mut self, url: &Url, - ) -> crate::Result<&mut DocumentController> { + ) -> crate::Result<&mut workspace::DocumentController> { self.workspaces .controller(url) .ok_or_else(|| anyhow!("Tried to open unavailable document `{url}`")) @@ -146,69 +107,8 @@ impl Session { } } -impl OpenDocuments { - fn snapshot(&self, url: &Url) -> Option { - Some(self.documents.get(url)?.make_ref()) - } - - fn controller(&mut self, url: &Url) -> Option<&mut DocumentController> { - self.documents.get_mut(url) - } - - fn open(&mut self, url: &Url, contents: String, version: DocumentVersion) { - if self - .documents - .insert(url.clone(), DocumentController::new(contents, version)) - .is_some() - { - tracing::warn!("Opening document `{url}` that is already open!"); - } - } - - fn close(&mut self, url: &Url) -> crate::Result<()> { - let Some(_) = self.documents.remove(url) else { - return Err(anyhow!( - "Tried to close document `{url}`, which was not open" - )); - }; - Ok(()) - } -} - -impl DocumentController { - fn new(contents: String, version: DocumentVersion) -> Self { - Self { - document: Arc::new(Document::new(contents, version)), - } - } - - pub(crate) fn make_ref(&self) -> DocumentRef { - DocumentRef { - document: self.document.clone(), - } - } - - pub(crate) fn make_mut(&mut self) -> &mut Document { - Arc::make_mut(&mut self.document) - } -} - -impl Deref for DocumentController { - type Target = Document; - fn deref(&self) -> &Self::Target { - &self.document - } -} - -impl Deref for DocumentRef { - type Target = Document; - fn deref(&self) -> &Self::Target { - &self.document - } -} - impl DocumentSnapshot { - pub(crate) fn configuration(&self) -> &RuffConfiguration { + pub(crate) fn configuration(&self) -> &workspace::RuffConfiguration { &self.configuration } @@ -216,11 +116,11 @@ impl DocumentSnapshot { &self.resolved_client_capabilities } - pub(crate) fn client_settings(&self) -> &ResolvedClientSettings { + pub(crate) fn client_settings(&self) -> &settings::ResolvedClientSettings { &self.client_settings } - pub(crate) fn document(&self) -> &DocumentRef { + pub(crate) fn document(&self) -> &workspace::DocumentRef { &self.document_ref } @@ -232,166 +132,3 @@ impl DocumentSnapshot { &self.url } } - -impl Workspaces { - fn new(workspaces: Vec<(Url, ClientSettings)>) -> crate::Result { - Ok(Self( - workspaces - .into_iter() - .map(|(url, settings)| Workspace::new(&url, settings)) - .collect::>()?, - )) - } - - fn open_workspace_folder(&mut self, folder_url: &Url) -> crate::Result<()> { - // TODO(jane): find a way to allow for workspace settings to be updated dynamically - let (path, workspace) = Workspace::new(folder_url, ClientSettings::default())?; - self.0.insert(path, workspace); - Ok(()) - } - - fn close_workspace_folder(&mut self, folder_url: &Url) -> crate::Result<()> { - let path = folder_url - .to_file_path() - .map_err(|()| anyhow!("Folder URI was not a proper file path"))?; - self.0 - .remove(&path) - .ok_or_else(|| anyhow!("Tried to remove non-existent folder {}", path.display()))?; - Ok(()) - } - - fn snapshot(&self, document_url: &Url) -> Option { - self.workspace_for_url(document_url)? - .open_documents - .snapshot(document_url) - } - - fn controller(&mut self, document_url: &Url) -> Option<&mut DocumentController> { - self.workspace_for_url_mut(document_url)? - .open_documents - .controller(document_url) - } - - fn configuration(&self, document_url: &Url) -> Option<&Arc> { - Some(&self.workspace_for_url(document_url)?.configuration) - } - - fn reload_configuration(&mut self, changed_url: &Url) -> crate::Result<()> { - let (path, workspace) = self - .entry_for_url_mut(changed_url) - .ok_or_else(|| anyhow!("Workspace not found for {changed_url}"))?; - workspace.reload_configuration(path); - Ok(()) - } - - fn open(&mut self, url: &Url, contents: String, version: DocumentVersion) { - if let Some(workspace) = self.workspace_for_url_mut(url) { - workspace.open_documents.open(url, contents, version); - } - } - - fn close(&mut self, url: &Url) -> crate::Result<()> { - self.workspace_for_url_mut(url) - .ok_or_else(|| anyhow!("Workspace not found for {url}"))? - .open_documents - .close(url) - } - - fn client_settings( - &self, - url: &Url, - global_settings: &ClientSettings, - ) -> ResolvedClientSettings { - self.workspace_for_url(url).map_or_else( - || { - tracing::warn!( - "Workspace not found for {url}. Global settings will be used for this document" - ); - ResolvedClientSettings::global(global_settings) - }, - |workspace| { - ResolvedClientSettings::with_workspace(&workspace.settings, global_settings) - }, - ) - } - - fn workspace_for_url(&self, url: &Url) -> Option<&Workspace> { - Some(self.entry_for_url(url)?.1) - } - - fn workspace_for_url_mut(&mut self, url: &Url) -> Option<&mut Workspace> { - Some(self.entry_for_url_mut(url)?.1) - } - - fn entry_for_url(&self, url: &Url) -> Option<(&Path, &Workspace)> { - let path = url.to_file_path().ok()?; - self.0 - .range(..path) - .next_back() - .map(|(path, workspace)| (path.as_path(), workspace)) - } - - fn entry_for_url_mut(&mut self, url: &Url) -> Option<(&Path, &mut Workspace)> { - let path = url.to_file_path().ok()?; - self.0 - .range_mut(..path) - .next_back() - .map(|(path, workspace)| (path.as_path(), workspace)) - } -} - -impl Workspace { - pub(crate) fn new(root: &Url, settings: ClientSettings) -> crate::Result<(PathBuf, Self)> { - let path = root - .to_file_path() - .map_err(|()| anyhow!("workspace URL was not a file path!"))?; - // Fall-back to default configuration - let configuration = Self::find_configuration_or_fallback(&path); - - Ok(( - path, - Self { - open_documents: OpenDocuments::default(), - configuration: Arc::new(configuration), - settings, - }, - )) - } - - fn reload_configuration(&mut self, path: &Path) { - self.configuration = Arc::new(Self::find_configuration_or_fallback(path)); - } - - fn find_configuration_or_fallback(root: &Path) -> RuffConfiguration { - find_configuration_from_root(root).unwrap_or_else(|err| { - tracing::error!("The following error occurred when trying to find a configuration file at `{}`:\n{err}", root.display()); - tracing::error!("Falling back to default configuration for `{}`", root.display()); - RuffConfiguration::default() - }) - } -} - -pub(crate) fn find_configuration_from_root(root: &Path) -> crate::Result { - let pyproject = ruff_workspace::pyproject::find_settings_toml(root)? - .ok_or_else(|| anyhow!("No pyproject.toml/ruff.toml/.ruff.toml file was found"))?; - let settings = ruff_workspace::resolver::resolve_root_settings( - &pyproject, - Relativity::Parent, - &LSPConfigTransformer, - )?; - Ok(RuffConfiguration { - linter: settings.linter, - formatter: settings.formatter, - }) -} - -struct LSPConfigTransformer; - -impl ConfigurationTransformer for LSPConfigTransformer { - fn transform( - &self, - config: ruff_workspace::configuration::Configuration, - ) -> ruff_workspace::configuration::Configuration { - config - } -} diff --git a/crates/ruff_server/src/session/workspace.rs b/crates/ruff_server/src/session/workspace.rs new file mode 100644 index 0000000000000..ee15dfaf22e93 --- /dev/null +++ b/crates/ruff_server/src/session/workspace.rs @@ -0,0 +1,246 @@ +use anyhow::anyhow; +use lsp_types::Url; +use rustc_hash::FxHashMap; +use std::{ + collections::BTreeMap, + ops::Deref, + path::{Path, PathBuf}, + sync::Arc, +}; + +use crate::{edit::DocumentVersion, Document}; + +use super::{settings, ClientSettings}; + +mod configuration; + +pub(crate) use configuration::RuffConfiguration; + +#[derive(Default)] +pub(crate) struct Workspaces(BTreeMap); + +pub(crate) struct Workspace { + open_documents: OpenDocuments, + configuration: Arc, + settings: ClientSettings, +} + +#[derive(Default)] +pub(crate) struct OpenDocuments { + documents: FxHashMap, +} + +/// A mutable handler to an underlying document. +/// Handles copy-on-write mutation automatically when +/// calling `deref_mut`. +pub(crate) struct DocumentController { + document: Arc, +} + +/// A read-only reference to a document. +#[derive(Clone)] +pub(crate) struct DocumentRef { + document: Arc, +} + +impl Workspaces { + pub(super) fn new(workspaces: Vec<(Url, ClientSettings)>) -> crate::Result { + Ok(Self( + workspaces + .into_iter() + .map(|(url, settings)| Workspace::new(&url, settings)) + .collect::>()?, + )) + } + + pub(super) fn open_workspace_folder(&mut self, folder_url: &Url) -> crate::Result<()> { + // TODO(jane): find a way to allow for workspace settings to be updated dynamically + let (path, workspace) = Workspace::new(folder_url, ClientSettings::default())?; + self.0.insert(path, workspace); + Ok(()) + } + + pub(super) fn close_workspace_folder(&mut self, folder_url: &Url) -> crate::Result<()> { + let path = folder_url + .to_file_path() + .map_err(|()| anyhow!("Folder URI was not a proper file path"))?; + self.0 + .remove(&path) + .ok_or_else(|| anyhow!("Tried to remove non-existent folder {}", path.display()))?; + Ok(()) + } + + pub(super) fn snapshot(&self, document_url: &Url) -> Option { + self.workspace_for_url(document_url)? + .open_documents + .snapshot(document_url) + } + + pub(super) fn controller(&mut self, document_url: &Url) -> Option<&mut DocumentController> { + self.workspace_for_url_mut(document_url)? + .open_documents + .controller(document_url) + } + + pub(super) fn configuration(&self, document_url: &Url) -> Option<&Arc> { + Some(&self.workspace_for_url(document_url)?.configuration) + } + + pub(super) fn reload_configuration(&mut self, changed_url: &Url) -> crate::Result<()> { + let (path, workspace) = self + .entry_for_url_mut(changed_url) + .ok_or_else(|| anyhow!("Workspace not found for {changed_url}"))?; + workspace.reload_configuration(path); + Ok(()) + } + + pub(super) fn open(&mut self, url: &Url, contents: String, version: DocumentVersion) { + if let Some(workspace) = self.workspace_for_url_mut(url) { + workspace.open_documents.open(url, contents, version); + } + } + + pub(super) fn close(&mut self, url: &Url) -> crate::Result<()> { + self.workspace_for_url_mut(url) + .ok_or_else(|| anyhow!("Workspace not found for {url}"))? + .open_documents + .close(url) + } + + pub(super) fn client_settings( + &self, + url: &Url, + global_settings: &ClientSettings, + ) -> settings::ResolvedClientSettings { + self.workspace_for_url(url).map_or_else( + || { + tracing::warn!( + "Workspace not found for {url}. Global settings will be used for this document" + ); + settings::ResolvedClientSettings::global(global_settings) + }, + |workspace| { + settings::ResolvedClientSettings::with_workspace( + &workspace.settings, + global_settings, + ) + }, + ) + } + + fn workspace_for_url(&self, url: &Url) -> Option<&Workspace> { + Some(self.entry_for_url(url)?.1) + } + + fn workspace_for_url_mut(&mut self, url: &Url) -> Option<&mut Workspace> { + Some(self.entry_for_url_mut(url)?.1) + } + + fn entry_for_url(&self, url: &Url) -> Option<(&Path, &Workspace)> { + let path = url.to_file_path().ok()?; + self.0 + .range(..path) + .next_back() + .map(|(path, workspace)| (path.as_path(), workspace)) + } + + fn entry_for_url_mut(&mut self, url: &Url) -> Option<(&Path, &mut Workspace)> { + let path = url.to_file_path().ok()?; + self.0 + .range_mut(..path) + .next_back() + .map(|(path, workspace)| (path.as_path(), workspace)) + } +} + +impl Workspace { + pub(crate) fn new(root: &Url, settings: ClientSettings) -> crate::Result<(PathBuf, Self)> { + let path = root + .to_file_path() + .map_err(|()| anyhow!("workspace URL was not a file path!"))?; + // Fall-back to default configuration + let configuration = Self::find_configuration_or_fallback(&path); + + Ok(( + path, + Self { + open_documents: OpenDocuments::default(), + configuration: Arc::new(configuration), + settings, + }, + )) + } + + fn reload_configuration(&mut self, path: &Path) { + self.configuration = Arc::new(Self::find_configuration_or_fallback(path)); + } + + fn find_configuration_or_fallback(root: &Path) -> RuffConfiguration { + configuration::find_configuration_from_root(root).unwrap_or_else(|err| { + tracing::error!("The following error occurred when trying to find a configuration file at `{}`:\n{err}", root.display()); + tracing::error!("Falling back to default configuration for `{}`", root.display()); + RuffConfiguration::default() + }) + } +} + +impl OpenDocuments { + fn snapshot(&self, url: &Url) -> Option { + Some(self.documents.get(url)?.make_ref()) + } + + fn controller(&mut self, url: &Url) -> Option<&mut DocumentController> { + self.documents.get_mut(url) + } + + fn open(&mut self, url: &Url, contents: String, version: DocumentVersion) { + if self + .documents + .insert(url.clone(), DocumentController::new(contents, version)) + .is_some() + { + tracing::warn!("Opening document `{url}` that is already open!"); + } + } + + fn close(&mut self, url: &Url) -> crate::Result<()> { + let Some(_) = self.documents.remove(url) else { + return Err(anyhow!( + "Tried to close document `{url}`, which was not open" + )); + }; + Ok(()) + } +} + +impl DocumentController { + fn new(contents: String, version: DocumentVersion) -> Self { + Self { + document: Arc::new(Document::new(contents, version)), + } + } + + pub(crate) fn make_ref(&self) -> DocumentRef { + DocumentRef { + document: self.document.clone(), + } + } + + pub(crate) fn make_mut(&mut self) -> &mut Document { + Arc::make_mut(&mut self.document) + } +} + +impl Deref for DocumentController { + type Target = Document; + fn deref(&self) -> &Self::Target { + &self.document + } +} + +impl Deref for DocumentRef { + type Target = Document; + fn deref(&self) -> &Self::Target { + &self.document + } +} diff --git a/crates/ruff_server/src/session/workspace/configuration.rs b/crates/ruff_server/src/session/workspace/configuration.rs new file mode 100644 index 0000000000000..b8ba39275723c --- /dev/null +++ b/crates/ruff_server/src/session/workspace/configuration.rs @@ -0,0 +1,36 @@ +use anyhow::anyhow; +use ruff_workspace::resolver::{ConfigurationTransformer, Relativity}; +use std::path::Path; + +#[derive(Default)] +pub(crate) struct RuffConfiguration { + // settings to pass into the ruff linter + pub(crate) linter: ruff_linter::settings::LinterSettings, + // settings to pass into the ruff formatter + pub(crate) formatter: ruff_workspace::FormatterSettings, +} + +pub(crate) fn find_configuration_from_root(root: &Path) -> crate::Result { + let pyproject = ruff_workspace::pyproject::find_settings_toml(root)? + .ok_or_else(|| anyhow!("No pyproject.toml/ruff.toml/.ruff.toml file was found"))?; + let settings = ruff_workspace::resolver::resolve_root_settings( + &pyproject, + Relativity::Parent, + &LSPConfigTransformer, + )?; + Ok(RuffConfiguration { + linter: settings.linter, + formatter: settings.formatter, + }) +} + +struct LSPConfigTransformer; + +impl ConfigurationTransformer for LSPConfigTransformer { + fn transform( + &self, + config: ruff_workspace::configuration::Configuration, + ) -> ruff_workspace::configuration::Configuration { + config + } +} From f7c073d4412242dfe09fd620371432735ffa0c18 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Sun, 14 Apr 2024 15:47:22 -0700 Subject: [PATCH 02/11] Make configuration a property of individual documents instead of per-workspace --- crates/ruff_server/src/session.rs | 4 +- crates/ruff_server/src/session/workspace.rs | 62 ++++++++++++------- .../src/session/workspace/configuration.rs | 31 +++++----- 3 files changed, 56 insertions(+), 41 deletions(-) diff --git a/crates/ruff_server/src/session.rs b/crates/ruff_server/src/session.rs index c23a199d85e08..9ee0dbd5c3e79 100644 --- a/crates/ruff_server/src/session.rs +++ b/crates/ruff_server/src/session.rs @@ -30,7 +30,6 @@ pub(crate) struct Session { /// An immutable snapshot of `Session` that references /// a specific document. pub(crate) struct DocumentSnapshot { - configuration: Arc, resolved_client_capabilities: Arc, client_settings: settings::ResolvedClientSettings, document_ref: workspace::DocumentRef, @@ -57,7 +56,6 @@ impl Session { pub(crate) fn take_snapshot(&self, url: &Url) -> Option { Some(DocumentSnapshot { - configuration: self.workspaces.configuration(url)?.clone(), resolved_client_capabilities: self.resolved_client_capabilities.clone(), client_settings: self.workspaces.client_settings(url, &self.global_settings), document_ref: self.workspaces.snapshot(url)?, @@ -109,7 +107,7 @@ impl Session { impl DocumentSnapshot { pub(crate) fn configuration(&self) -> &workspace::RuffConfiguration { - &self.configuration + self.document().configuration() } pub(crate) fn resolved_client_capabilities(&self) -> &ResolvedClientCapabilities { diff --git a/crates/ruff_server/src/session/workspace.rs b/crates/ruff_server/src/session/workspace.rs index ee15dfaf22e93..92219e06ae560 100644 --- a/crates/ruff_server/src/session/workspace.rs +++ b/crates/ruff_server/src/session/workspace.rs @@ -21,13 +21,13 @@ pub(crate) struct Workspaces(BTreeMap); pub(crate) struct Workspace { open_documents: OpenDocuments, - configuration: Arc, settings: ClientSettings, } #[derive(Default)] pub(crate) struct OpenDocuments { documents: FxHashMap, + configuration_index: configuration::ConfigurationIndex, } /// A mutable handler to an underlying document. @@ -35,12 +35,14 @@ pub(crate) struct OpenDocuments { /// calling `deref_mut`. pub(crate) struct DocumentController { document: Arc, + configuration: Arc, } /// A read-only reference to a document. #[derive(Clone)] pub(crate) struct DocumentRef { document: Arc, + configuration: Arc, } impl Workspaces { @@ -82,15 +84,11 @@ impl Workspaces { .controller(document_url) } - pub(super) fn configuration(&self, document_url: &Url) -> Option<&Arc> { - Some(&self.workspace_for_url(document_url)?.configuration) - } - pub(super) fn reload_configuration(&mut self, changed_url: &Url) -> crate::Result<()> { - let (path, workspace) = self - .entry_for_url_mut(changed_url) + let workspace = self + .workspace_for_url_mut(changed_url) .ok_or_else(|| anyhow!("Workspace not found for {changed_url}"))?; - workspace.reload_configuration(path); + workspace.reload_configuration(); Ok(()) } @@ -158,29 +156,18 @@ impl Workspace { let path = root .to_file_path() .map_err(|()| anyhow!("workspace URL was not a file path!"))?; - // Fall-back to default configuration - let configuration = Self::find_configuration_or_fallback(&path); Ok(( path, Self { open_documents: OpenDocuments::default(), - configuration: Arc::new(configuration), settings, }, )) } - fn reload_configuration(&mut self, path: &Path) { - self.configuration = Arc::new(Self::find_configuration_or_fallback(path)); - } - - fn find_configuration_or_fallback(root: &Path) -> RuffConfiguration { - configuration::find_configuration_from_root(root).unwrap_or_else(|err| { - tracing::error!("The following error occurred when trying to find a configuration file at `{}`:\n{err}", root.display()); - tracing::error!("Falling back to default configuration for `{}`", root.display()); - RuffConfiguration::default() - }) + fn reload_configuration(&mut self) { + self.open_documents.reload_configuration(); } } @@ -194,9 +181,13 @@ impl OpenDocuments { } fn open(&mut self, url: &Url, contents: String, version: DocumentVersion) { + let configuration = self.configuration_index.get_or_insert(url); if self .documents - .insert(url.clone(), DocumentController::new(contents, version)) + .insert( + url.clone(), + DocumentController::new(contents, version, configuration), + ) .is_some() { tracing::warn!("Opening document `{url}` that is already open!"); @@ -211,18 +202,37 @@ impl OpenDocuments { }; Ok(()) } + + fn reload_configuration(&mut self) { + self.configuration_index.clear(); + + for (path, document) in &mut self.documents { + let new_configuration = self.configuration_index.get_or_insert(path); + document.update_configuration(new_configuration); + } + } } impl DocumentController { - fn new(contents: String, version: DocumentVersion) -> Self { + fn new( + contents: String, + version: DocumentVersion, + configuration: Arc, + ) -> Self { Self { document: Arc::new(Document::new(contents, version)), + configuration, } } + pub(crate) fn update_configuration(&mut self, new_configuration: Arc) { + self.configuration = new_configuration; + } + pub(crate) fn make_ref(&self) -> DocumentRef { DocumentRef { document: self.document.clone(), + configuration: self.configuration.clone(), } } @@ -244,3 +254,9 @@ impl Deref for DocumentRef { &self.document } } + +impl DocumentRef { + pub(crate) fn configuration(&self) -> &RuffConfiguration { + &self.configuration + } +} diff --git a/crates/ruff_server/src/session/workspace/configuration.rs b/crates/ruff_server/src/session/workspace/configuration.rs index b8ba39275723c..8818a1e61559d 100644 --- a/crates/ruff_server/src/session/workspace/configuration.rs +++ b/crates/ruff_server/src/session/workspace/configuration.rs @@ -1,6 +1,6 @@ -use anyhow::anyhow; -use ruff_workspace::resolver::{ConfigurationTransformer, Relativity}; -use std::path::Path; +use lsp_types::Url; +use ruff_workspace::resolver::ConfigurationTransformer; +use std::{collections::BTreeMap, path::PathBuf, sync::Arc}; #[derive(Default)] pub(crate) struct RuffConfiguration { @@ -10,18 +10,19 @@ pub(crate) struct RuffConfiguration { pub(crate) formatter: ruff_workspace::FormatterSettings, } -pub(crate) fn find_configuration_from_root(root: &Path) -> crate::Result { - let pyproject = ruff_workspace::pyproject::find_settings_toml(root)? - .ok_or_else(|| anyhow!("No pyproject.toml/ruff.toml/.ruff.toml file was found"))?; - let settings = ruff_workspace::resolver::resolve_root_settings( - &pyproject, - Relativity::Parent, - &LSPConfigTransformer, - )?; - Ok(RuffConfiguration { - linter: settings.linter, - formatter: settings.formatter, - }) +#[derive(Default)] +pub(super) struct ConfigurationIndex { + index: BTreeMap>, +} + +impl ConfigurationIndex { + pub(super) fn get_or_insert(&mut self, path: &Url) -> Arc { + todo!("impl"); + } + + pub(super) fn clear(&mut self) { + self.index.clear(); + } } struct LSPConfigTransformer; From fc35618f17f761de170125f089ce7fc054df356b Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Mon, 15 Apr 2024 01:23:57 -0700 Subject: [PATCH 03/11] Implement `get_or_insert` for configuration index --- .../src/session/workspace/configuration.rs | 45 +++++++++++++++++-- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/crates/ruff_server/src/session/workspace/configuration.rs b/crates/ruff_server/src/session/workspace/configuration.rs index 8818a1e61559d..46eb8491a30b2 100644 --- a/crates/ruff_server/src/session/workspace/configuration.rs +++ b/crates/ruff_server/src/session/workspace/configuration.rs @@ -1,6 +1,11 @@ +use anyhow::anyhow; use lsp_types::Url; -use ruff_workspace::resolver::ConfigurationTransformer; -use std::{collections::BTreeMap, path::PathBuf, sync::Arc}; +use ruff_workspace::resolver::{ConfigurationTransformer, Relativity}; +use std::{ + collections::BTreeMap, + path::{Path, PathBuf}, + sync::Arc, +}; #[derive(Default)] pub(crate) struct RuffConfiguration { @@ -16,13 +21,45 @@ pub(super) struct ConfigurationIndex { } impl ConfigurationIndex { - pub(super) fn get_or_insert(&mut self, path: &Url) -> Arc { - todo!("impl"); + pub(super) fn get_or_insert(&mut self, document_url: &Url) -> Arc { + let document_path = document_url + .to_file_path() + .expect("document URL should be a valid path"); + let folder = document_path + .parent() + .expect("document URL should be a file path and have a parent"); + if let Some(config) = self.index.get(folder) { + return config.clone(); + } + + let config = Arc::new(Self::find_configuration_at_path(folder).unwrap_or_else(|err| { + tracing::error!("The following error occurred when trying to find a configuration file at `{}`:\n{err}", document_path.display()); + tracing::error!("Falling back to default configuration for `{}`", document_path.display()); + RuffConfiguration::default() + })); + + self.index.insert(folder.to_path_buf(), config.clone()); + + config } pub(super) fn clear(&mut self) { self.index.clear(); } + + fn find_configuration_at_path(folder: &Path) -> crate::Result { + let pyproject = ruff_workspace::pyproject::find_settings_toml(folder)? + .ok_or_else(|| anyhow!("No pyproject.toml/ruff.toml/.ruff.toml file was found"))?; + let settings = ruff_workspace::resolver::resolve_root_settings( + &pyproject, + Relativity::Parent, + &LSPConfigTransformer, + )?; + Ok(RuffConfiguration { + linter: settings.linter, + formatter: settings.formatter, + }) + } } struct LSPConfigTransformer; From 9db096a5687c5b76f6d552f1c1211d012d17511f Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Mon, 15 Apr 2024 11:56:44 -0700 Subject: [PATCH 04/11] Rename 'configuration' (a shorthand for project configuration) to 'ruff settings' --- .../notifications/did_change_watched_files.rs | 2 +- .../src/server/api/requests/code_action.rs | 4 +- .../api/requests/code_action_resolve.rs | 4 +- .../src/server/api/requests/diagnostic.rs | 2 +- .../server/api/requests/execute_command.rs | 4 +- .../src/server/api/requests/format.rs | 2 +- .../src/server/api/requests/format_range.rs | 2 +- crates/ruff_server/src/session.rs | 8 ++-- crates/ruff_server/src/session/workspace.rs | 38 +++++++++---------- .../{configuration.rs => ruff_settings.rs} | 16 ++++---- 10 files changed, 41 insertions(+), 41 deletions(-) rename crates/ruff_server/src/session/workspace/{configuration.rs => ruff_settings.rs} (88%) diff --git a/crates/ruff_server/src/server/api/notifications/did_change_watched_files.rs b/crates/ruff_server/src/server/api/notifications/did_change_watched_files.rs index 75547ff73a50b..f1a65b2af4a1a 100644 --- a/crates/ruff_server/src/server/api/notifications/did_change_watched_files.rs +++ b/crates/ruff_server/src/server/api/notifications/did_change_watched_files.rs @@ -19,7 +19,7 @@ impl super::SyncNotificationHandler for DidChangeWatchedFiles { ) -> Result<()> { for change in params.changes { session - .reload_configuration(&change.uri) + .reload_ruff_settings(&change.uri) .with_failure_code(lsp_server::ErrorCode::InternalError)?; } Ok(()) diff --git a/crates/ruff_server/src/server/api/requests/code_action.rs b/crates/ruff_server/src/server/api/requests/code_action.rs index 0d697b0e59196..3b87940d1e1eb 100644 --- a/crates/ruff_server/src/server/api/requests/code_action.rs +++ b/crates/ruff_server/src/server/api/requests/code_action.rs @@ -105,7 +105,7 @@ fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result { document, snapshot.resolved_client_capabilities(), snapshot.url(), - &snapshot.configuration().linter, + &snapshot.ruff_settings().linter, snapshot.encoding(), document.version(), )?), @@ -140,7 +140,7 @@ fn organize_imports(snapshot: &DocumentSnapshot) -> crate::Result { let edits = super::code_action_resolve::fix_all_edit( snapshot.document(), - &snapshot.configuration().linter, + &snapshot.ruff_settings().linter, snapshot.encoding(), ) .with_failure_code(ErrorCode::InternalError)?; @@ -83,7 +83,7 @@ impl super::SyncRequestHandler for ExecuteCommand { Command::OrganizeImports => { let edits = super::code_action_resolve::organize_imports_edit( snapshot.document(), - &snapshot.configuration().linter, + &snapshot.ruff_settings().linter, snapshot.encoding(), ) .with_failure_code(ErrorCode::InternalError)?; diff --git a/crates/ruff_server/src/server/api/requests/format.rs b/crates/ruff_server/src/server/api/requests/format.rs index 05a4485f594e7..8bb3c6a1eb21b 100644 --- a/crates/ruff_server/src/server/api/requests/format.rs +++ b/crates/ruff_server/src/server/api/requests/format.rs @@ -26,7 +26,7 @@ impl super::BackgroundDocumentRequestHandler for Format { pub(super) fn format_document(snapshot: &DocumentSnapshot) -> Result { let doc = snapshot.document(); let source = doc.contents(); - let formatted = crate::format::format(doc, &snapshot.configuration().formatter) + let formatted = crate::format::format(doc, &snapshot.ruff_settings().formatter) .with_failure_code(lsp_server::ErrorCode::InternalError)?; // fast path - if the code is the same, return early if formatted == source { diff --git a/crates/ruff_server/src/server/api/requests/format_range.rs b/crates/ruff_server/src/server/api/requests/format_range.rs index aef39d971be44..2572729a6df1d 100644 --- a/crates/ruff_server/src/server/api/requests/format_range.rs +++ b/crates/ruff_server/src/server/api/requests/format_range.rs @@ -22,7 +22,7 @@ impl super::BackgroundDocumentRequestHandler for FormatRange { let index = document.index(); let range = params.range.to_text_range(text, index, snapshot.encoding()); let formatted_range = - crate::format::format_range(document, &snapshot.configuration().formatter, range) + crate::format::format_range(document, &snapshot.ruff_settings().formatter, range) .with_failure_code(lsp_server::ErrorCode::InternalError)?; Ok(Some(vec![types::TextEdit { range: formatted_range diff --git a/crates/ruff_server/src/session.rs b/crates/ruff_server/src/session.rs index 9ee0dbd5c3e79..3724e33641064 100644 --- a/crates/ruff_server/src/session.rs +++ b/crates/ruff_server/src/session.rs @@ -82,8 +82,8 @@ impl Session { .ok_or_else(|| anyhow!("Tried to open unavailable document `{url}`")) } - pub(crate) fn reload_configuration(&mut self, url: &Url) -> crate::Result<()> { - self.workspaces.reload_configuration(url) + pub(crate) fn reload_ruff_settings(&mut self, url: &Url) -> crate::Result<()> { + self.workspaces.reload_ruff_settings(url) } pub(crate) fn open_workspace_folder(&mut self, url: &Url) -> crate::Result<()> { @@ -106,8 +106,8 @@ impl Session { } impl DocumentSnapshot { - pub(crate) fn configuration(&self) -> &workspace::RuffConfiguration { - self.document().configuration() + pub(crate) fn ruff_settings(&self) -> &workspace::RuffSettings { + self.document().ruff_settings() } pub(crate) fn resolved_client_capabilities(&self) -> &ResolvedClientCapabilities { diff --git a/crates/ruff_server/src/session/workspace.rs b/crates/ruff_server/src/session/workspace.rs index 92219e06ae560..049d089e49403 100644 --- a/crates/ruff_server/src/session/workspace.rs +++ b/crates/ruff_server/src/session/workspace.rs @@ -12,9 +12,9 @@ use crate::{edit::DocumentVersion, Document}; use super::{settings, ClientSettings}; -mod configuration; +mod ruff_settings; -pub(crate) use configuration::RuffConfiguration; +pub(crate) use ruff_settings::RuffSettings; #[derive(Default)] pub(crate) struct Workspaces(BTreeMap); @@ -27,7 +27,7 @@ pub(crate) struct Workspace { #[derive(Default)] pub(crate) struct OpenDocuments { documents: FxHashMap, - configuration_index: configuration::ConfigurationIndex, + ruff_settings_index: ruff_settings::RuffSettingsIndex, } /// A mutable handler to an underlying document. @@ -35,14 +35,14 @@ pub(crate) struct OpenDocuments { /// calling `deref_mut`. pub(crate) struct DocumentController { document: Arc, - configuration: Arc, + configuration: Arc, } /// A read-only reference to a document. #[derive(Clone)] pub(crate) struct DocumentRef { document: Arc, - configuration: Arc, + ruff_settings: Arc, } impl Workspaces { @@ -84,11 +84,11 @@ impl Workspaces { .controller(document_url) } - pub(super) fn reload_configuration(&mut self, changed_url: &Url) -> crate::Result<()> { + pub(super) fn reload_ruff_settings(&mut self, changed_url: &Url) -> crate::Result<()> { let workspace = self .workspace_for_url_mut(changed_url) .ok_or_else(|| anyhow!("Workspace not found for {changed_url}"))?; - workspace.reload_configuration(); + workspace.reload_ruff_settings(); Ok(()) } @@ -166,8 +166,8 @@ impl Workspace { )) } - fn reload_configuration(&mut self) { - self.open_documents.reload_configuration(); + fn reload_ruff_settings(&mut self) { + self.open_documents.reload_ruff_settings(); } } @@ -181,7 +181,7 @@ impl OpenDocuments { } fn open(&mut self, url: &Url, contents: String, version: DocumentVersion) { - let configuration = self.configuration_index.get_or_insert(url); + let configuration = self.ruff_settings_index.get_or_insert(url); if self .documents .insert( @@ -203,12 +203,12 @@ impl OpenDocuments { Ok(()) } - fn reload_configuration(&mut self) { - self.configuration_index.clear(); + fn reload_ruff_settings(&mut self) { + self.ruff_settings_index.clear(); for (path, document) in &mut self.documents { - let new_configuration = self.configuration_index.get_or_insert(path); - document.update_configuration(new_configuration); + let new_settings = self.ruff_settings_index.get_or_insert(path); + document.update_ruff_settings(new_settings); } } } @@ -217,7 +217,7 @@ impl DocumentController { fn new( contents: String, version: DocumentVersion, - configuration: Arc, + configuration: Arc, ) -> Self { Self { document: Arc::new(Document::new(contents, version)), @@ -225,14 +225,14 @@ impl DocumentController { } } - pub(crate) fn update_configuration(&mut self, new_configuration: Arc) { + pub(crate) fn update_ruff_settings(&mut self, new_configuration: Arc) { self.configuration = new_configuration; } pub(crate) fn make_ref(&self) -> DocumentRef { DocumentRef { document: self.document.clone(), - configuration: self.configuration.clone(), + ruff_settings: self.configuration.clone(), } } @@ -256,7 +256,7 @@ impl Deref for DocumentRef { } impl DocumentRef { - pub(crate) fn configuration(&self) -> &RuffConfiguration { - &self.configuration + pub(crate) fn ruff_settings(&self) -> &RuffSettings { + &self.ruff_settings } } diff --git a/crates/ruff_server/src/session/workspace/configuration.rs b/crates/ruff_server/src/session/workspace/ruff_settings.rs similarity index 88% rename from crates/ruff_server/src/session/workspace/configuration.rs rename to crates/ruff_server/src/session/workspace/ruff_settings.rs index 46eb8491a30b2..06b18ebec143c 100644 --- a/crates/ruff_server/src/session/workspace/configuration.rs +++ b/crates/ruff_server/src/session/workspace/ruff_settings.rs @@ -8,7 +8,7 @@ use std::{ }; #[derive(Default)] -pub(crate) struct RuffConfiguration { +pub(crate) struct RuffSettings { // settings to pass into the ruff linter pub(crate) linter: ruff_linter::settings::LinterSettings, // settings to pass into the ruff formatter @@ -16,12 +16,12 @@ pub(crate) struct RuffConfiguration { } #[derive(Default)] -pub(super) struct ConfigurationIndex { - index: BTreeMap>, +pub(super) struct RuffSettingsIndex { + index: BTreeMap>, } -impl ConfigurationIndex { - pub(super) fn get_or_insert(&mut self, document_url: &Url) -> Arc { +impl RuffSettingsIndex { + pub(super) fn get_or_insert(&mut self, document_url: &Url) -> Arc { let document_path = document_url .to_file_path() .expect("document URL should be a valid path"); @@ -35,7 +35,7 @@ impl ConfigurationIndex { let config = Arc::new(Self::find_configuration_at_path(folder).unwrap_or_else(|err| { tracing::error!("The following error occurred when trying to find a configuration file at `{}`:\n{err}", document_path.display()); tracing::error!("Falling back to default configuration for `{}`", document_path.display()); - RuffConfiguration::default() + RuffSettings::default() })); self.index.insert(folder.to_path_buf(), config.clone()); @@ -47,7 +47,7 @@ impl ConfigurationIndex { self.index.clear(); } - fn find_configuration_at_path(folder: &Path) -> crate::Result { + fn find_configuration_at_path(folder: &Path) -> crate::Result { let pyproject = ruff_workspace::pyproject::find_settings_toml(folder)? .ok_or_else(|| anyhow!("No pyproject.toml/ruff.toml/.ruff.toml file was found"))?; let settings = ruff_workspace::resolver::resolve_root_settings( @@ -55,7 +55,7 @@ impl ConfigurationIndex { Relativity::Parent, &LSPConfigTransformer, )?; - Ok(RuffConfiguration { + Ok(RuffSettings { linter: settings.linter, formatter: settings.formatter, }) From f63e3d2a05f28f5677fc498fc064ccff1073c6d1 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Mon, 15 Apr 2024 12:39:59 -0700 Subject: [PATCH 05/11] RuffSettingsIndex now loads eagerly instead of lazily, and only creates entries for the folder where the config files reside --- crates/ruff_server/src/session/workspace.rs | 47 ++++-------- .../src/session/workspace/ruff_settings.rs | 74 ++++++++++--------- 2 files changed, 56 insertions(+), 65 deletions(-) diff --git a/crates/ruff_server/src/session/workspace.rs b/crates/ruff_server/src/session/workspace.rs index 049d089e49403..ac662906369ad 100644 --- a/crates/ruff_server/src/session/workspace.rs +++ b/crates/ruff_server/src/session/workspace.rs @@ -35,7 +35,6 @@ pub(crate) struct OpenDocuments { /// calling `deref_mut`. pub(crate) struct DocumentController { document: Arc, - configuration: Arc, } /// A read-only reference to a document. @@ -85,10 +84,10 @@ impl Workspaces { } pub(super) fn reload_ruff_settings(&mut self, changed_url: &Url) -> crate::Result<()> { - let workspace = self - .workspace_for_url_mut(changed_url) + let (root, workspace) = self + .entry_for_url_mut(changed_url) .ok_or_else(|| anyhow!("Workspace not found for {changed_url}"))?; - workspace.reload_ruff_settings(); + workspace.reload_ruff_settings(root); Ok(()) } @@ -166,14 +165,18 @@ impl Workspace { )) } - fn reload_ruff_settings(&mut self) { - self.open_documents.reload_ruff_settings(); + fn reload_ruff_settings(&mut self, root: &Path) { + self.open_documents.reload_ruff_settings(root); } } impl OpenDocuments { fn snapshot(&self, url: &Url) -> Option { - Some(self.documents.get(url)?.make_ref()) + let path = url + .to_file_path() + .expect("document URL should convert to file path: {url}"); + let document_settings = self.ruff_settings_index.get(&path).clone(); + Some(self.documents.get(url)?.make_ref(document_settings)) } fn controller(&mut self, url: &Url) -> Option<&mut DocumentController> { @@ -181,13 +184,9 @@ impl OpenDocuments { } fn open(&mut self, url: &Url, contents: String, version: DocumentVersion) { - let configuration = self.ruff_settings_index.get_or_insert(url); if self .documents - .insert( - url.clone(), - DocumentController::new(contents, version, configuration), - ) + .insert(url.clone(), DocumentController::new(contents, version)) .is_some() { tracing::warn!("Opening document `{url}` that is already open!"); @@ -203,36 +202,22 @@ impl OpenDocuments { Ok(()) } - fn reload_ruff_settings(&mut self) { - self.ruff_settings_index.clear(); - - for (path, document) in &mut self.documents { - let new_settings = self.ruff_settings_index.get_or_insert(path); - document.update_ruff_settings(new_settings); - } + fn reload_ruff_settings(&mut self, root: &Path) { + self.ruff_settings_index.reload(root); } } impl DocumentController { - fn new( - contents: String, - version: DocumentVersion, - configuration: Arc, - ) -> Self { + fn new(contents: String, version: DocumentVersion) -> Self { Self { document: Arc::new(Document::new(contents, version)), - configuration, } } - pub(crate) fn update_ruff_settings(&mut self, new_configuration: Arc) { - self.configuration = new_configuration; - } - - pub(crate) fn make_ref(&self) -> DocumentRef { + pub(crate) fn make_ref(&self, document_settings: Arc) -> DocumentRef { DocumentRef { document: self.document.clone(), - ruff_settings: self.configuration.clone(), + ruff_settings: document_settings, } } diff --git a/crates/ruff_server/src/session/workspace/ruff_settings.rs b/crates/ruff_server/src/session/workspace/ruff_settings.rs index 06b18ebec143c..547666e3e3cde 100644 --- a/crates/ruff_server/src/session/workspace/ruff_settings.rs +++ b/crates/ruff_server/src/session/workspace/ruff_settings.rs @@ -1,6 +1,7 @@ -use anyhow::anyhow; -use lsp_types::Url; -use ruff_workspace::resolver::{ConfigurationTransformer, Relativity}; +use ruff_workspace::{ + pyproject::settings_toml, + resolver::{ConfigurationTransformer, Relativity}, +}; use std::{ collections::BTreeMap, path::{Path, PathBuf}, @@ -18,48 +19,53 @@ pub(crate) struct RuffSettings { #[derive(Default)] pub(super) struct RuffSettingsIndex { index: BTreeMap>, + fallback: Arc, } impl RuffSettingsIndex { - pub(super) fn get_or_insert(&mut self, document_url: &Url) -> Arc { - let document_path = document_url - .to_file_path() - .expect("document URL should be a valid path"); - let folder = document_path - .parent() - .expect("document URL should be a file path and have a parent"); - if let Some(config) = self.index.get(folder) { - return config.clone(); + pub(super) fn reload(&mut self, root: &Path) { + self.clear(); + + for directory in std::fs::read_dir(root).unwrap().filter_map(|entry| { + entry + .ok() + .and_then(|entry| entry.file_type().ok()?.is_dir().then_some(entry)) + }) { + let path = directory.path(); + if let Some(pyproject) = settings_toml(&path).ok().flatten() { + let Ok(settings) = ruff_workspace::resolver::resolve_root_settings( + &pyproject, + Relativity::Parent, + &LSPConfigTransformer, + ) else { + continue; + }; + self.index.insert( + path, + Arc::new(RuffSettings { + linter: settings.linter, + formatter: settings.formatter, + }), + ); + } } + } - let config = Arc::new(Self::find_configuration_at_path(folder).unwrap_or_else(|err| { - tracing::error!("The following error occurred when trying to find a configuration file at `{}`:\n{err}", document_path.display()); - tracing::error!("Falling back to default configuration for `{}`", document_path.display()); - RuffSettings::default() - })); + pub(super) fn get(&self, document_path: &Path) -> &Arc { + if let Some((_, ruff_settings)) = + self.index.range(..document_path.to_path_buf()).next_back() + { + return ruff_settings; + } - self.index.insert(folder.to_path_buf(), config.clone()); + tracing::warn!("No ruff settings file (pyproject.toml/ruff.toml/.ruff.toml) found for {} - falling back to default configuration", document_path.display()); - config + &self.fallback } - pub(super) fn clear(&mut self) { + fn clear(&mut self) { self.index.clear(); } - - fn find_configuration_at_path(folder: &Path) -> crate::Result { - let pyproject = ruff_workspace::pyproject::find_settings_toml(folder)? - .ok_or_else(|| anyhow!("No pyproject.toml/ruff.toml/.ruff.toml file was found"))?; - let settings = ruff_workspace::resolver::resolve_root_settings( - &pyproject, - Relativity::Parent, - &LSPConfigTransformer, - )?; - Ok(RuffSettings { - linter: settings.linter, - formatter: settings.formatter, - }) - } } struct LSPConfigTransformer; From c113c722ee212fd33cabbf4ab37e0aee9ed31383 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Mon, 15 Apr 2024 18:58:56 -0700 Subject: [PATCH 06/11] Load index properly on startup --- crates/ruff_server/src/session/workspace.rs | 23 ++++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/crates/ruff_server/src/session/workspace.rs b/crates/ruff_server/src/session/workspace.rs index ac662906369ad..10d2b8a56a545 100644 --- a/crates/ruff_server/src/session/workspace.rs +++ b/crates/ruff_server/src/session/workspace.rs @@ -10,6 +10,8 @@ use std::{ use crate::{edit::DocumentVersion, Document}; +use self::ruff_settings::RuffSettingsIndex; + use super::{settings, ClientSettings}; mod ruff_settings; @@ -24,7 +26,6 @@ pub(crate) struct Workspace { settings: ClientSettings, } -#[derive(Default)] pub(crate) struct OpenDocuments { documents: FxHashMap, ruff_settings_index: ruff_settings::RuffSettingsIndex, @@ -156,13 +157,12 @@ impl Workspace { .to_file_path() .map_err(|()| anyhow!("workspace URL was not a file path!"))?; - Ok(( - path, - Self { - open_documents: OpenDocuments::default(), - settings, - }, - )) + let workspace = Self { + open_documents: OpenDocuments::new(&path), + settings, + }; + + Ok((path, workspace)) } fn reload_ruff_settings(&mut self, root: &Path) { @@ -171,6 +171,13 @@ impl Workspace { } impl OpenDocuments { + fn new(path: &Path) -> Self { + Self { + documents: FxHashMap::default(), + ruff_settings_index: RuffSettingsIndex::new(path), + } + } + fn snapshot(&self, url: &Url) -> Option { let path = url .to_file_path() From a746423e4090797006815920a6260d52a39d7b86 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Mon, 15 Apr 2024 18:59:54 -0700 Subject: [PATCH 07/11] Fix reload/get functions --- Cargo.lock | 1 + crates/ruff_server/Cargo.toml | 1 + .../src/session/workspace/ruff_settings.rs | 50 +++++++++++++++---- 3 files changed, 41 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dc45dd1f2e6e8..fc0e62980afa6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2425,6 +2425,7 @@ dependencies = [ "serde", "serde_json", "tracing", + "walkdir", ] [[package]] diff --git a/crates/ruff_server/Cargo.toml b/crates/ruff_server/Cargo.toml index 0b6056dcbf0cc..24297c970717f 100644 --- a/crates/ruff_server/Cargo.toml +++ b/crates/ruff_server/Cargo.toml @@ -35,6 +35,7 @@ rustc-hash = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } tracing = { workspace = true } +walkdir = { workspace = true } [dev-dependencies] insta = { workspace = true } diff --git a/crates/ruff_server/src/session/workspace/ruff_settings.rs b/crates/ruff_server/src/session/workspace/ruff_settings.rs index 547666e3e3cde..6bff2a975f90b 100644 --- a/crates/ruff_server/src/session/workspace/ruff_settings.rs +++ b/crates/ruff_server/src/session/workspace/ruff_settings.rs @@ -1,3 +1,4 @@ +use ruff_linter::display_settings; use ruff_workspace::{ pyproject::settings_toml, resolver::{ConfigurationTransformer, Relativity}, @@ -7,6 +8,7 @@ use std::{ path::{Path, PathBuf}, sync::Arc, }; +use walkdir::{DirEntry, WalkDir}; #[derive(Default)] pub(crate) struct RuffSettings { @@ -16,23 +18,46 @@ pub(crate) struct RuffSettings { pub(crate) formatter: ruff_workspace::FormatterSettings, } -#[derive(Default)] pub(super) struct RuffSettingsIndex { index: BTreeMap>, fallback: Arc, } +impl std::fmt::Display for RuffSettings { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + display_settings! { + formatter = f, + fields = [ + self.linter, + self.formatter + ] + } + Ok(()) + } +} + impl RuffSettingsIndex { + pub(super) fn new(path: &Path) -> Self { + let mut index = Self { + index: BTreeMap::default(), + fallback: Arc::default(), + }; + + index.reload(path); + + index + } + pub(super) fn reload(&mut self, root: &Path) { self.clear(); - for directory in std::fs::read_dir(root).unwrap().filter_map(|entry| { - entry - .ok() - .and_then(|entry| entry.file_type().ok()?.is_dir().then_some(entry)) - }) { - let path = directory.path(); - if let Some(pyproject) = settings_toml(&path).ok().flatten() { + for directory in WalkDir::new(root) + .into_iter() + .filter_map(Result::ok) + .filter(|entry| entry.file_type().is_dir()) + .map(DirEntry::into_path) + { + if let Some(pyproject) = settings_toml(&directory).ok().flatten() { let Ok(settings) = ruff_workspace::resolver::resolve_root_settings( &pyproject, Relativity::Parent, @@ -41,7 +66,7 @@ impl RuffSettingsIndex { continue; }; self.index.insert( - path, + directory, Arc::new(RuffSettings { linter: settings.linter, formatter: settings.formatter, @@ -52,8 +77,11 @@ impl RuffSettingsIndex { } pub(super) fn get(&self, document_path: &Path) -> &Arc { - if let Some((_, ruff_settings)) = - self.index.range(..document_path.to_path_buf()).next_back() + if let Some((_, ruff_settings)) = self + .index + .range(..document_path.to_path_buf()) + .rev() + .find(|(path, _)| document_path.starts_with(path)) { return ruff_settings; } From 0cebae4395a140a57babe1792078865b073abe88 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Tue, 16 Apr 2024 10:59:34 -0700 Subject: [PATCH 08/11] ruff_settings -> settings --- .../notifications/did_change_watched_files.rs | 2 +- .../src/server/api/requests/code_action.rs | 4 +-- .../api/requests/code_action_resolve.rs | 4 +-- .../src/server/api/requests/diagnostic.rs | 2 +- .../server/api/requests/execute_command.rs | 4 +-- .../src/server/api/requests/format.rs | 2 +- .../src/server/api/requests/format_range.rs | 2 +- crates/ruff_server/src/session.rs | 8 +++--- crates/ruff_server/src/session/workspace.rs | 26 +++++++++---------- .../src/session/workspace/ruff_settings.rs | 4 +-- 10 files changed, 29 insertions(+), 29 deletions(-) diff --git a/crates/ruff_server/src/server/api/notifications/did_change_watched_files.rs b/crates/ruff_server/src/server/api/notifications/did_change_watched_files.rs index f1a65b2af4a1a..3c2f2e09ef844 100644 --- a/crates/ruff_server/src/server/api/notifications/did_change_watched_files.rs +++ b/crates/ruff_server/src/server/api/notifications/did_change_watched_files.rs @@ -19,7 +19,7 @@ impl super::SyncNotificationHandler for DidChangeWatchedFiles { ) -> Result<()> { for change in params.changes { session - .reload_ruff_settings(&change.uri) + .reload_settings(&change.uri) .with_failure_code(lsp_server::ErrorCode::InternalError)?; } Ok(()) diff --git a/crates/ruff_server/src/server/api/requests/code_action.rs b/crates/ruff_server/src/server/api/requests/code_action.rs index 3b87940d1e1eb..59a651d764a8b 100644 --- a/crates/ruff_server/src/server/api/requests/code_action.rs +++ b/crates/ruff_server/src/server/api/requests/code_action.rs @@ -105,7 +105,7 @@ fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result { document, snapshot.resolved_client_capabilities(), snapshot.url(), - &snapshot.ruff_settings().linter, + &snapshot.settings().linter, snapshot.encoding(), document.version(), )?), @@ -140,7 +140,7 @@ fn organize_imports(snapshot: &DocumentSnapshot) -> crate::Result { let edits = super::code_action_resolve::fix_all_edit( snapshot.document(), - &snapshot.ruff_settings().linter, + &snapshot.settings().linter, snapshot.encoding(), ) .with_failure_code(ErrorCode::InternalError)?; @@ -83,7 +83,7 @@ impl super::SyncRequestHandler for ExecuteCommand { Command::OrganizeImports => { let edits = super::code_action_resolve::organize_imports_edit( snapshot.document(), - &snapshot.ruff_settings().linter, + &snapshot.settings().linter, snapshot.encoding(), ) .with_failure_code(ErrorCode::InternalError)?; diff --git a/crates/ruff_server/src/server/api/requests/format.rs b/crates/ruff_server/src/server/api/requests/format.rs index 8bb3c6a1eb21b..c8a9260324f82 100644 --- a/crates/ruff_server/src/server/api/requests/format.rs +++ b/crates/ruff_server/src/server/api/requests/format.rs @@ -26,7 +26,7 @@ impl super::BackgroundDocumentRequestHandler for Format { pub(super) fn format_document(snapshot: &DocumentSnapshot) -> Result { let doc = snapshot.document(); let source = doc.contents(); - let formatted = crate::format::format(doc, &snapshot.ruff_settings().formatter) + let formatted = crate::format::format(doc, &snapshot.settings().formatter) .with_failure_code(lsp_server::ErrorCode::InternalError)?; // fast path - if the code is the same, return early if formatted == source { diff --git a/crates/ruff_server/src/server/api/requests/format_range.rs b/crates/ruff_server/src/server/api/requests/format_range.rs index 2572729a6df1d..904be4914e8b5 100644 --- a/crates/ruff_server/src/server/api/requests/format_range.rs +++ b/crates/ruff_server/src/server/api/requests/format_range.rs @@ -22,7 +22,7 @@ impl super::BackgroundDocumentRequestHandler for FormatRange { let index = document.index(); let range = params.range.to_text_range(text, index, snapshot.encoding()); let formatted_range = - crate::format::format_range(document, &snapshot.ruff_settings().formatter, range) + crate::format::format_range(document, &snapshot.settings().formatter, range) .with_failure_code(lsp_server::ErrorCode::InternalError)?; Ok(Some(vec![types::TextEdit { range: formatted_range diff --git a/crates/ruff_server/src/session.rs b/crates/ruff_server/src/session.rs index 3724e33641064..d1d7b80f2e474 100644 --- a/crates/ruff_server/src/session.rs +++ b/crates/ruff_server/src/session.rs @@ -82,8 +82,8 @@ impl Session { .ok_or_else(|| anyhow!("Tried to open unavailable document `{url}`")) } - pub(crate) fn reload_ruff_settings(&mut self, url: &Url) -> crate::Result<()> { - self.workspaces.reload_ruff_settings(url) + pub(crate) fn reload_settings(&mut self, url: &Url) -> crate::Result<()> { + self.workspaces.reload_settings(url) } pub(crate) fn open_workspace_folder(&mut self, url: &Url) -> crate::Result<()> { @@ -106,8 +106,8 @@ impl Session { } impl DocumentSnapshot { - pub(crate) fn ruff_settings(&self) -> &workspace::RuffSettings { - self.document().ruff_settings() + pub(crate) fn settings(&self) -> &workspace::RuffSettings { + self.document().settings() } pub(crate) fn resolved_client_capabilities(&self) -> &ResolvedClientCapabilities { diff --git a/crates/ruff_server/src/session/workspace.rs b/crates/ruff_server/src/session/workspace.rs index 10d2b8a56a545..ac429a09e3d1e 100644 --- a/crates/ruff_server/src/session/workspace.rs +++ b/crates/ruff_server/src/session/workspace.rs @@ -28,7 +28,7 @@ pub(crate) struct Workspace { pub(crate) struct OpenDocuments { documents: FxHashMap, - ruff_settings_index: ruff_settings::RuffSettingsIndex, + settings_index: ruff_settings::RuffSettingsIndex, } /// A mutable handler to an underlying document. @@ -42,7 +42,7 @@ pub(crate) struct DocumentController { #[derive(Clone)] pub(crate) struct DocumentRef { document: Arc, - ruff_settings: Arc, + settings: Arc, } impl Workspaces { @@ -84,11 +84,11 @@ impl Workspaces { .controller(document_url) } - pub(super) fn reload_ruff_settings(&mut self, changed_url: &Url) -> crate::Result<()> { + pub(super) fn reload_settings(&mut self, changed_url: &Url) -> crate::Result<()> { let (root, workspace) = self .entry_for_url_mut(changed_url) .ok_or_else(|| anyhow!("Workspace not found for {changed_url}"))?; - workspace.reload_ruff_settings(root); + workspace.reload_settings(root); Ok(()) } @@ -165,8 +165,8 @@ impl Workspace { Ok((path, workspace)) } - fn reload_ruff_settings(&mut self, root: &Path) { - self.open_documents.reload_ruff_settings(root); + fn reload_settings(&mut self, root: &Path) { + self.open_documents.reload_settings(root); } } @@ -174,7 +174,7 @@ impl OpenDocuments { fn new(path: &Path) -> Self { Self { documents: FxHashMap::default(), - ruff_settings_index: RuffSettingsIndex::new(path), + settings_index: RuffSettingsIndex::new(path), } } @@ -182,7 +182,7 @@ impl OpenDocuments { let path = url .to_file_path() .expect("document URL should convert to file path: {url}"); - let document_settings = self.ruff_settings_index.get(&path).clone(); + let document_settings = self.settings_index.get(&path).clone(); Some(self.documents.get(url)?.make_ref(document_settings)) } @@ -209,8 +209,8 @@ impl OpenDocuments { Ok(()) } - fn reload_ruff_settings(&mut self, root: &Path) { - self.ruff_settings_index.reload(root); + fn reload_settings(&mut self, root: &Path) { + self.settings_index.reload(root); } } @@ -224,7 +224,7 @@ impl DocumentController { pub(crate) fn make_ref(&self, document_settings: Arc) -> DocumentRef { DocumentRef { document: self.document.clone(), - ruff_settings: document_settings, + settings: document_settings, } } @@ -248,7 +248,7 @@ impl Deref for DocumentRef { } impl DocumentRef { - pub(crate) fn ruff_settings(&self) -> &RuffSettings { - &self.ruff_settings + pub(crate) fn settings(&self) -> &RuffSettings { + &self.settings } } diff --git a/crates/ruff_server/src/session/workspace/ruff_settings.rs b/crates/ruff_server/src/session/workspace/ruff_settings.rs index 6bff2a975f90b..b0529eb23dde2 100644 --- a/crates/ruff_server/src/session/workspace/ruff_settings.rs +++ b/crates/ruff_server/src/session/workspace/ruff_settings.rs @@ -77,13 +77,13 @@ impl RuffSettingsIndex { } pub(super) fn get(&self, document_path: &Path) -> &Arc { - if let Some((_, ruff_settings)) = self + if let Some((_, settings)) = self .index .range(..document_path.to_path_buf()) .rev() .find(|(path, _)| document_path.starts_with(path)) { - return ruff_settings; + return settings; } tracing::warn!("No ruff settings file (pyproject.toml/ruff.toml/.ruff.toml) found for {} - falling back to default configuration", document_path.display()); From 46964071f5107ca49f90de0fc8a2cd0fd5fb3b54 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Tue, 16 Apr 2024 11:02:30 -0700 Subject: [PATCH 09/11] Remove index reload method in favor of recreating the index from scratch --- crates/ruff_server/src/session/workspace.rs | 2 +- .../src/session/workspace/ruff_settings.rs | 26 ++++++------------- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/crates/ruff_server/src/session/workspace.rs b/crates/ruff_server/src/session/workspace.rs index ac429a09e3d1e..800f769ea327e 100644 --- a/crates/ruff_server/src/session/workspace.rs +++ b/crates/ruff_server/src/session/workspace.rs @@ -210,7 +210,7 @@ impl OpenDocuments { } fn reload_settings(&mut self, root: &Path) { - self.settings_index.reload(root); + self.settings_index = RuffSettingsIndex::new(root); } } diff --git a/crates/ruff_server/src/session/workspace/ruff_settings.rs b/crates/ruff_server/src/session/workspace/ruff_settings.rs index b0529eb23dde2..bca58204ed901 100644 --- a/crates/ruff_server/src/session/workspace/ruff_settings.rs +++ b/crates/ruff_server/src/session/workspace/ruff_settings.rs @@ -37,19 +37,8 @@ impl std::fmt::Display for RuffSettings { } impl RuffSettingsIndex { - pub(super) fn new(path: &Path) -> Self { - let mut index = Self { - index: BTreeMap::default(), - fallback: Arc::default(), - }; - - index.reload(path); - - index - } - - pub(super) fn reload(&mut self, root: &Path) { - self.clear(); + pub(super) fn new(root: &Path) -> Self { + let mut index = BTreeMap::default(); for directory in WalkDir::new(root) .into_iter() @@ -65,7 +54,7 @@ impl RuffSettingsIndex { ) else { continue; }; - self.index.insert( + index.insert( directory, Arc::new(RuffSettings { linter: settings.linter, @@ -74,6 +63,11 @@ impl RuffSettingsIndex { ); } } + + Self { + index, + fallback: Arc::default(), + } } pub(super) fn get(&self, document_path: &Path) -> &Arc { @@ -90,10 +84,6 @@ impl RuffSettingsIndex { &self.fallback } - - fn clear(&mut self) { - self.index.clear(); - } } struct LSPConfigTransformer; From 5190c9043f9b62c3b52e92b408e27b46793fe555 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Tue, 16 Apr 2024 11:07:01 -0700 Subject: [PATCH 10/11] RuffSettingsIndex::get returns an owned Arc --- crates/ruff_server/src/session/workspace.rs | 2 +- crates/ruff_server/src/session/workspace/ruff_settings.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ruff_server/src/session/workspace.rs b/crates/ruff_server/src/session/workspace.rs index 800f769ea327e..a508a360c8cd2 100644 --- a/crates/ruff_server/src/session/workspace.rs +++ b/crates/ruff_server/src/session/workspace.rs @@ -182,7 +182,7 @@ impl OpenDocuments { let path = url .to_file_path() .expect("document URL should convert to file path: {url}"); - let document_settings = self.settings_index.get(&path).clone(); + let document_settings = self.settings_index.get(&path); Some(self.documents.get(url)?.make_ref(document_settings)) } diff --git a/crates/ruff_server/src/session/workspace/ruff_settings.rs b/crates/ruff_server/src/session/workspace/ruff_settings.rs index bca58204ed901..ee6a7960d58ee 100644 --- a/crates/ruff_server/src/session/workspace/ruff_settings.rs +++ b/crates/ruff_server/src/session/workspace/ruff_settings.rs @@ -70,19 +70,19 @@ impl RuffSettingsIndex { } } - pub(super) fn get(&self, document_path: &Path) -> &Arc { + pub(super) fn get(&self, document_path: &Path) -> Arc { if let Some((_, settings)) = self .index .range(..document_path.to_path_buf()) .rev() .find(|(path, _)| document_path.starts_with(path)) { - return settings; + return settings.clone(); } tracing::warn!("No ruff settings file (pyproject.toml/ruff.toml/.ruff.toml) found for {} - falling back to default configuration", document_path.display()); - &self.fallback + self.fallback.clone() } } From 34d442f215ddebdca60c783847a47dde3c61e200 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Tue, 16 Apr 2024 11:07:37 -0700 Subject: [PATCH 11/11] Make fallback log `info` instead of `warn` --- crates/ruff_server/src/session/workspace/ruff_settings.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_server/src/session/workspace/ruff_settings.rs b/crates/ruff_server/src/session/workspace/ruff_settings.rs index ee6a7960d58ee..d1e13d6370f2f 100644 --- a/crates/ruff_server/src/session/workspace/ruff_settings.rs +++ b/crates/ruff_server/src/session/workspace/ruff_settings.rs @@ -80,7 +80,7 @@ impl RuffSettingsIndex { return settings.clone(); } - tracing::warn!("No ruff settings file (pyproject.toml/ruff.toml/.ruff.toml) found for {} - falling back to default configuration", document_path.display()); + tracing::info!("No ruff settings file (pyproject.toml/ruff.toml/.ruff.toml) found for {} - falling back to default configuration", document_path.display()); self.fallback.clone() }