diff --git a/crates/ruff_server/src/edit.rs b/crates/ruff_server/src/edit.rs index 8529146de2..e711b36953 100644 --- a/crates/ruff_server/src/edit.rs +++ b/crates/ruff_server/src/edit.rs @@ -5,9 +5,9 @@ mod range; mod replacement; mod text_document; -use std::{collections::HashMap, path::PathBuf}; +use std::collections::HashMap; -use lsp_types::PositionEncodingKind; +use lsp_types::{PositionEncodingKind, Url}; pub(crate) use notebook::NotebookDocument; pub(crate) use range::{NotebookRange, RangeExt, ToRangeExt}; pub(crate) use replacement::Replacement; @@ -35,20 +35,18 @@ pub enum PositionEncoding { /// This document ID can point to either be a standalone Python file, a full notebook, or a cell within a notebook. #[derive(Clone, Debug)] pub(crate) enum DocumentKey { - Notebook(PathBuf), - NotebookCell(lsp_types::Url), - Text(PathBuf), + Notebook(Url), + NotebookCell(Url), + Text(Url), } impl DocumentKey { /// Converts the key back into its original URL. - pub(crate) fn into_url(self) -> lsp_types::Url { + pub(crate) fn into_url(self) -> Url { match self { - DocumentKey::NotebookCell(url) => url, - DocumentKey::Notebook(path) | DocumentKey::Text(path) => { - lsp_types::Url::from_file_path(path) - .expect("file path originally from URL should convert back to URL") - } + DocumentKey::NotebookCell(url) + | DocumentKey::Notebook(url) + | DocumentKey::Text(url) => url, } } } @@ -56,8 +54,7 @@ impl DocumentKey { impl std::fmt::Display for DocumentKey { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::NotebookCell(url) => url.fmt(f), - Self::Notebook(path) | Self::Text(path) => path.display().fmt(f), + Self::NotebookCell(url) | Self::Notebook(url) | Self::Text(url) => url.fmt(f), } } } @@ -67,7 +64,7 @@ impl std::fmt::Display for DocumentKey { #[derive(Debug)] pub(crate) enum WorkspaceEditTracker { DocumentChanges(Vec), - Changes(HashMap>), + Changes(HashMap>), } impl From for lsp_types::PositionEncodingKind { @@ -122,7 +119,7 @@ impl WorkspaceEditTracker { /// multiple times. pub(crate) fn set_edits_for_document( &mut self, - uri: lsp_types::Url, + uri: Url, _version: DocumentVersion, edits: Vec, ) -> crate::Result<()> { diff --git a/crates/ruff_server/src/fix.rs b/crates/ruff_server/src/fix.rs index da7b0914f8..04762b86f8 100644 --- a/crates/ruff_server/src/fix.rs +++ b/crates/ruff_server/src/fix.rs @@ -26,33 +26,37 @@ pub(crate) fn fix_all( linter_settings: &LinterSettings, encoding: PositionEncoding, ) -> crate::Result { - let document_path = query.file_path(); let source_kind = query.make_source_kind(); let file_resolver_settings = query.settings().file_resolver(); + let document_path = query.file_path(); // If the document is excluded, return an empty list of fixes. - if let Some(exclusion) = match_any_exclusion( - document_path, - &file_resolver_settings.exclude, - &file_resolver_settings.extend_exclude, - Some(&linter_settings.exclude), - None, - ) { - tracing::debug!( - "Ignored path via `{}`: {}", - exclusion, - document_path.display() - ); - return Ok(Fixes::default()); - } + let package = if let Some(document_path) = document_path.as_ref() { + if let Some(exclusion) = match_any_exclusion( + document_path, + &file_resolver_settings.exclude, + &file_resolver_settings.extend_exclude, + Some(&linter_settings.exclude), + None, + ) { + tracing::debug!( + "Ignored path via `{}`: {}", + exclusion, + document_path.display() + ); + return Ok(Fixes::default()); + } - let package = detect_package_root( - document_path - .parent() - .expect("a path to a document should have a parent path"), - &linter_settings.namespace_packages, - ); + detect_package_root( + document_path + .parent() + .expect("a path to a document should have a parent path"), + &linter_settings.namespace_packages, + ) + } else { + None + }; let source_type = query.source_type(); @@ -67,7 +71,7 @@ pub(crate) fn fix_all( result: LinterResult { error, .. }, .. } = ruff_linter::linter::lint_fix( - document_path, + query.virtual_file_path(), package, flags::Noqa::Enabled, UnsafeFixes::Disabled, diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index 76950dee27..b984143fa2 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -1,5 +1,8 @@ //! Access to the Ruff linting API for the LSP +use rustc_hash::FxHashMap; +use serde::{Deserialize, Serialize}; + use ruff_diagnostics::{Applicability, Diagnostic, DiagnosticKind, Edit, Fix}; use ruff_linter::{ directives::{extract_directives, Flags}, @@ -17,8 +20,6 @@ use ruff_python_parser::AsMode; use ruff_source_file::{LineIndex, Locator}; use ruff_text_size::{Ranged, TextRange}; use ruff_workspace::resolver::match_any_exclusion; -use rustc_hash::FxHashMap; -use serde::{Deserialize, Serialize}; use crate::{ edit::{NotebookRange, ToRangeExt}, @@ -60,33 +61,37 @@ pub(crate) struct DiagnosticFix { pub(crate) type Diagnostics = FxHashMap>; pub(crate) fn check(query: &DocumentQuery, encoding: PositionEncoding) -> Diagnostics { - let document_path = query.file_path(); let source_kind = query.make_source_kind(); let file_resolver_settings = query.settings().file_resolver(); let linter_settings = query.settings().linter(); + let document_path = query.file_path(); // If the document is excluded, return an empty list of diagnostics. - if let Some(exclusion) = match_any_exclusion( - document_path, - &file_resolver_settings.exclude, - &file_resolver_settings.extend_exclude, - Some(&linter_settings.exclude), - None, - ) { - tracing::debug!( - "Ignored path via `{}`: {}", - exclusion, - document_path.display() - ); - return Diagnostics::default(); - } + let package = if let Some(document_path) = document_path.as_ref() { + if let Some(exclusion) = match_any_exclusion( + document_path, + &file_resolver_settings.exclude, + &file_resolver_settings.extend_exclude, + Some(&linter_settings.exclude), + None, + ) { + tracing::debug!( + "Ignored path via `{}`: {}", + exclusion, + document_path.display() + ); + return Diagnostics::default(); + } - let package = detect_package_root( - document_path - .parent() - .expect("a path to a document should have a parent path"), - &linter_settings.namespace_packages, - ); + detect_package_root( + document_path + .parent() + .expect("a path to a document should have a parent path"), + &linter_settings.namespace_packages, + ) + } else { + None + }; let source_type = query.source_type(); @@ -109,7 +114,7 @@ pub(crate) fn check(query: &DocumentQuery, encoding: PositionEncoding) -> Diagno // Generate checks. let LinterResult { data, .. } = check_path( - document_path, + query.virtual_file_path(), package, &locator, &stylist, @@ -123,7 +128,7 @@ pub(crate) fn check(query: &DocumentQuery, encoding: PositionEncoding) -> Diagno ); let noqa_edits = generate_noqa_edits( - document_path, + query.virtual_file_path(), data.as_slice(), &locator, indexer.comment_ranges(), diff --git a/crates/ruff_server/src/server.rs b/crates/ruff_server/src/server.rs index 9c995b402a..76c5f1a55f 100644 --- a/crates/ruff_server/src/server.rs +++ b/crates/ruff_server/src/server.rs @@ -1,7 +1,6 @@ //! Scheduling, I/O, and API endpoints. use std::num::NonZeroUsize; -use std::path::PathBuf; use lsp_server as lsp; use lsp_types as types; @@ -75,27 +74,27 @@ impl Server { .unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::default())), ); - let mut workspace_for_path = |path: PathBuf| { + let mut workspace_for_url = |url: lsp_types::Url| { let Some(workspace_settings) = workspace_settings.as_mut() else { - return (path, ClientSettings::default()); + return (url, ClientSettings::default()); }; - let settings = workspace_settings.remove(&path).unwrap_or_else(|| { - tracing::warn!("No workspace settings found for {}", path.display()); + let settings = workspace_settings.remove(&url).unwrap_or_else(|| { + tracing::warn!("No workspace settings found for {}", url); ClientSettings::default() }); - (path, settings) + (url, settings) }; let workspaces = init_params .workspace_folders .filter(|folders| !folders.is_empty()) .map(|folders| folders.into_iter().map(|folder| { - workspace_for_path(folder.uri.to_file_path().unwrap()) + workspace_for_url(folder.uri) }).collect()) .or_else(|| { tracing::warn!("No workspace(s) were provided during initialization. Using the current working directory as a default workspace..."); let uri = types::Url::from_file_path(std::env::current_dir().ok()?).ok()?; - Some(vec![workspace_for_path(uri.to_file_path().unwrap())]) + Some(vec![workspace_for_url(uri)]) }) .ok_or_else(|| { anyhow::anyhow!("Failed to get the current working directory while creating a default workspace.") @@ -109,7 +108,7 @@ impl Server { position_encoding, global_settings, workspaces, - ), + )?, client_capabilities, }) } diff --git a/crates/ruff_server/src/server/api.rs b/crates/ruff_server/src/server/api.rs index 28320c744b..6917d0ff6c 100644 --- a/crates/ruff_server/src/server/api.rs +++ b/crates/ruff_server/src/server/api.rs @@ -122,7 +122,7 @@ fn background_request_task<'a, R: traits::BackgroundDocumentRequestHandler>( let (id, params) = cast_request::(req)?; Ok(Task::background(schedule, move |session: &Session| { // TODO(jane): we should log an error if we can't take a snapshot. - let Some(snapshot) = session.take_snapshot(&R::document_url(¶ms)) else { + let Some(snapshot) = session.take_snapshot(R::document_url(¶ms).into_owned()) else { return Box::new(|_, _| {}); }; Box::new(move |notifier, responder| { @@ -152,7 +152,7 @@ fn background_notification_thread<'a, N: traits::BackgroundDocumentNotificationH let (id, params) = cast_notification::(req)?; Ok(Task::background(schedule, move |session: &Session| { // TODO(jane): we should log an error if we can't take a snapshot. - let Some(snapshot) = session.take_snapshot(&N::document_url(¶ms)) else { + let Some(snapshot) = session.take_snapshot(N::document_url(¶ms).into_owned()) else { return Box::new(|_, _| {}); }; Box::new(move |notifier, _| { diff --git a/crates/ruff_server/src/server/api/notifications/did_change.rs b/crates/ruff_server/src/server/api/notifications/did_change.rs index 2d4653363a..27ee65a3a1 100644 --- a/crates/ruff_server/src/server/api/notifications/did_change.rs +++ b/crates/ruff_server/src/server/api/notifications/did_change.rs @@ -27,9 +27,7 @@ impl super::SyncNotificationHandler for DidChange { content_changes, }: types::DidChangeTextDocumentParams, ) -> Result<()> { - let key = session - .key_from_url(&uri) - .with_failure_code(ErrorCode::InternalError)?; + let key = session.key_from_url(uri); session .update_text_document(&key, content_changes, new_version) @@ -37,7 +35,7 @@ impl super::SyncNotificationHandler for DidChange { // Publish diagnostics if the client doesnt support pull diagnostics if !session.resolved_client_capabilities().pull_diagnostics { - let snapshot = session.take_snapshot(&uri).unwrap(); + let snapshot = session.take_snapshot(key.into_url()).unwrap(); publish_diagnostics_for_document(&snapshot, ¬ifier)?; } diff --git a/crates/ruff_server/src/server/api/notifications/did_change_notebook.rs b/crates/ruff_server/src/server/api/notifications/did_change_notebook.rs index b0ddb4747c..97f12f4e17 100644 --- a/crates/ruff_server/src/server/api/notifications/did_change_notebook.rs +++ b/crates/ruff_server/src/server/api/notifications/did_change_notebook.rs @@ -23,16 +23,14 @@ impl super::SyncNotificationHandler for DidChangeNotebook { change: types::NotebookDocumentChangeEvent { cells, metadata }, }: types::DidChangeNotebookDocumentParams, ) -> Result<()> { - let key = session - .key_from_url(&uri) - .with_failure_code(ErrorCode::InternalError)?; + let key = session.key_from_url(uri); session .update_notebook_document(&key, cells, metadata, version) .with_failure_code(ErrorCode::InternalError)?; // publish new diagnostics let snapshot = session - .take_snapshot(&uri) + .take_snapshot(key.into_url()) .expect("snapshot should be available"); publish_diagnostics_for_document(&snapshot, ¬ifier)?; 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 15e6b60f89..8c2e232bda 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 @@ -21,7 +21,7 @@ impl super::SyncNotificationHandler for DidChangeWatchedFiles { params: types::DidChangeWatchedFilesParams, ) -> Result<()> { for change in ¶ms.changes { - session.reload_settings(&change.uri.to_file_path().unwrap()); + session.reload_settings(&change.uri); } if !params.changes.is_empty() { @@ -33,7 +33,7 @@ impl super::SyncNotificationHandler for DidChangeWatchedFiles { // publish diagnostics for text documents for url in session.text_document_urls() { let snapshot = session - .take_snapshot(&url) + .take_snapshot(url.clone()) .expect("snapshot should be available"); publish_diagnostics_for_document(&snapshot, ¬ifier)?; } @@ -42,7 +42,7 @@ impl super::SyncNotificationHandler for DidChangeWatchedFiles { // always publish diagnostics for notebook files (since they don't use pull diagnostics) for url in session.notebook_document_urls() { let snapshot = session - .take_snapshot(&url) + .take_snapshot(url.clone()) .expect("snapshot should be available"); publish_diagnostics_for_document(&snapshot, ¬ifier)?; } diff --git a/crates/ruff_server/src/server/api/notifications/did_change_workspace.rs b/crates/ruff_server/src/server/api/notifications/did_change_workspace.rs index a5868dd9e5..6b92357993 100644 --- a/crates/ruff_server/src/server/api/notifications/did_change_workspace.rs +++ b/crates/ruff_server/src/server/api/notifications/did_change_workspace.rs @@ -2,8 +2,6 @@ use crate::server::api::LSPResult; use crate::server::client::{Notifier, Requester}; use crate::server::Result; use crate::session::Session; -use anyhow::anyhow; -use lsp_server::ErrorCode; use lsp_types as types; use lsp_types::notification as notif; @@ -20,21 +18,14 @@ impl super::SyncNotificationHandler for DidChangeWorkspace { _requester: &mut Requester, params: types::DidChangeWorkspaceFoldersParams, ) -> Result<()> { - for types::WorkspaceFolder { ref uri, .. } in params.event.added { - let workspace_path = uri - .to_file_path() - .map_err(|()| anyhow!("expected document URI {uri} to be a valid file path")) - .with_failure_code(ErrorCode::InvalidParams)?; - - session.open_workspace_folder(workspace_path); - } - for types::WorkspaceFolder { ref uri, .. } in params.event.removed { - let workspace_path = uri - .to_file_path() - .map_err(|()| anyhow!("expected document URI {uri} to be a valid file path")) - .with_failure_code(ErrorCode::InvalidParams)?; + for types::WorkspaceFolder { uri, .. } in params.event.added { session - .close_workspace_folder(&workspace_path) + .open_workspace_folder(&uri) + .with_failure_code(lsp_server::ErrorCode::InvalidParams)?; + } + for types::WorkspaceFolder { uri, .. } in params.event.removed { + session + .close_workspace_folder(&uri) .with_failure_code(lsp_server::ErrorCode::InvalidParams)?; } Ok(()) diff --git a/crates/ruff_server/src/server/api/notifications/did_close.rs b/crates/ruff_server/src/server/api/notifications/did_close.rs index b78295c737..4fc4bb1c5f 100644 --- a/crates/ruff_server/src/server/api/notifications/did_close.rs +++ b/crates/ruff_server/src/server/api/notifications/did_close.rs @@ -24,7 +24,7 @@ impl super::SyncNotificationHandler for DidClose { ) -> Result<()> { // Publish an empty diagnostic report for the document. This will de-register any existing diagnostics. let snapshot = session - .take_snapshot(&uri) + .take_snapshot(uri.clone()) .ok_or_else(|| anyhow::anyhow!("Unable to take snapshot for document with URL {uri}")) .with_failure_code(lsp_server::ErrorCode::InternalError)?; clear_diagnostics_for_document(snapshot.query(), ¬ifier)?; diff --git a/crates/ruff_server/src/server/api/notifications/did_close_notebook.rs b/crates/ruff_server/src/server/api/notifications/did_close_notebook.rs index a66e3126e5..561f2d8e68 100644 --- a/crates/ruff_server/src/server/api/notifications/did_close_notebook.rs +++ b/crates/ruff_server/src/server/api/notifications/did_close_notebook.rs @@ -22,9 +22,7 @@ impl super::SyncNotificationHandler for DidCloseNotebook { .. }: types::DidCloseNotebookDocumentParams, ) -> Result<()> { - let key = session - .key_from_url(&uri) - .with_failure_code(ErrorCode::InternalError)?; + let key = session.key_from_url(uri); session .close_document(&key) diff --git a/crates/ruff_server/src/server/api/notifications/did_open.rs b/crates/ruff_server/src/server/api/notifications/did_open.rs index 810cd029a4..9c15666733 100644 --- a/crates/ruff_server/src/server/api/notifications/did_open.rs +++ b/crates/ruff_server/src/server/api/notifications/did_open.rs @@ -4,8 +4,6 @@ use crate::server::client::{Notifier, Requester}; use crate::server::Result; use crate::session::Session; use crate::TextDocument; -use anyhow::anyhow; -use lsp_server::ErrorCode; use lsp_types as types; use lsp_types::notification as notif; @@ -23,26 +21,18 @@ impl super::SyncNotificationHandler for DidOpen { types::DidOpenTextDocumentParams { text_document: types::TextDocumentItem { - ref uri, - text, - version, - .. + uri, text, version, .. }, }: types::DidOpenTextDocumentParams, ) -> Result<()> { - let document_path: std::path::PathBuf = uri - .to_file_path() - .map_err(|()| anyhow!("expected document URI {uri} to be a valid file path")) - .with_failure_code(ErrorCode::InvalidParams)?; - let document = TextDocument::new(text, version); - session.open_text_document(document_path, document); + session.open_text_document(uri.clone(), document); // Publish diagnostics if the client doesnt support pull diagnostics if !session.resolved_client_capabilities().pull_diagnostics { let snapshot = session - .take_snapshot(uri) + .take_snapshot(uri.clone()) .ok_or_else(|| { anyhow::anyhow!("Unable to take snapshot for document with URL {uri}") }) diff --git a/crates/ruff_server/src/server/api/notifications/did_open_notebook.rs b/crates/ruff_server/src/server/api/notifications/did_open_notebook.rs index 3310346282..4cd21c35fc 100644 --- a/crates/ruff_server/src/server/api/notifications/did_open_notebook.rs +++ b/crates/ruff_server/src/server/api/notifications/did_open_notebook.rs @@ -4,7 +4,6 @@ use crate::server::api::LSPResult; use crate::server::client::{Notifier, Requester}; use crate::server::Result; use crate::session::Session; -use anyhow::anyhow; use lsp_server::ErrorCode; use lsp_types as types; use lsp_types::notification as notif; @@ -40,16 +39,11 @@ impl super::SyncNotificationHandler for DidOpenNotebook { ) .with_failure_code(ErrorCode::InternalError)?; - let notebook_path = uri - .to_file_path() - .map_err(|()| anyhow!("expected notebook URI {uri} to be a valid file path")) - .with_failure_code(ErrorCode::InvalidParams)?; - - session.open_notebook_document(notebook_path, notebook); + session.open_notebook_document(uri.clone(), notebook); // publish diagnostics let snapshot = session - .take_snapshot(&uri) + .take_snapshot(uri) .expect("snapshot should be available"); publish_diagnostics_for_document(&snapshot, ¬ifier)?; diff --git a/crates/ruff_server/src/server/api/requests/execute_command.rs b/crates/ruff_server/src/server/api/requests/execute_command.rs index fcac7b9bd3..1cf06cf102 100644 --- a/crates/ruff_server/src/server/api/requests/execute_command.rs +++ b/crates/ruff_server/src/server/api/requests/execute_command.rs @@ -57,7 +57,7 @@ impl super::SyncRequestHandler for ExecuteCommand { let mut edit_tracker = WorkspaceEditTracker::new(session.resolved_client_capabilities()); for Argument { uri, version } in arguments { let snapshot = session - .take_snapshot(&uri) + .take_snapshot(uri.clone()) .ok_or(anyhow::anyhow!("Document snapshot not available for {uri}",)) .with_failure_code(ErrorCode::InternalError)?; match command { diff --git a/crates/ruff_server/src/server/api/requests/format.rs b/crates/ruff_server/src/server/api/requests/format.rs index ca3e39a5db..917b6bf45a 100644 --- a/crates/ruff_server/src/server/api/requests/format.rs +++ b/crates/ruff_server/src/server/api/requests/format.rs @@ -1,18 +1,14 @@ -use std::path::Path; - use lsp_types::{self as types, request as req}; use types::TextEdit; -use ruff_python_ast::PySourceType; use ruff_source_file::LineIndex; use ruff_workspace::resolver::match_any_exclusion; -use ruff_workspace::{FileResolverSettings, FormatterSettings}; use crate::edit::{Replacement, ToRangeExt}; use crate::fix::Fixes; use crate::server::api::LSPResult; use crate::server::{client::Notifier, Result}; -use crate::session::DocumentSnapshot; +use crate::session::{DocumentQuery, DocumentSnapshot}; use crate::{PositionEncoding, TextDocument}; pub(crate) struct Format; @@ -37,34 +33,25 @@ pub(super) fn format_full_document(snapshot: &DocumentSnapshot) -> Result let mut fixes = Fixes::default(); let query = snapshot.query(); - if let Some(notebook) = snapshot.query().as_notebook() { - for (url, text_document) in notebook - .urls() - .map(|url| (url.clone(), notebook.cell_document_by_uri(url).unwrap())) - { - if let Some(changes) = format_text_document( - text_document, - query.source_type(), - query.file_path(), - query.settings().file_resolver(), - query.settings().formatter(), - snapshot.encoding(), - true, - )? { - fixes.insert(url, changes); + match snapshot.query() { + DocumentQuery::Notebook { notebook, .. } => { + for (url, text_document) in notebook + .urls() + .map(|url| (url.clone(), notebook.cell_document_by_uri(url).unwrap())) + { + if let Some(changes) = + format_text_document(text_document, query, snapshot.encoding(), true)? + { + fixes.insert(url, changes); + } } } - } else { - if let Some(changes) = format_text_document( - query.as_single_document().unwrap(), - query.source_type(), - query.file_path(), - query.settings().file_resolver(), - query.settings().formatter(), - snapshot.encoding(), - false, - )? { - fixes.insert(snapshot.query().make_key().into_url(), changes); + DocumentQuery::Text { document, .. } => { + if let Some(changes) = + format_text_document(document, query, snapshot.encoding(), false)? + { + fixes.insert(snapshot.query().make_key().into_url(), changes); + } } } @@ -81,10 +68,7 @@ pub(super) fn format_document(snapshot: &DocumentSnapshot) -> Result Result Result { + let file_resolver_settings = query.settings().file_resolver(); + let formatter_settings = query.settings().formatter(); + // If the document is excluded, return early. - if let Some(exclusion) = match_any_exclusion( - file_path, - &file_resolver_settings.exclude, - &file_resolver_settings.extend_exclude, - None, - Some(&formatter_settings.exclude), - ) { - tracing::debug!("Ignored path via `{}`: {}", exclusion, file_path.display()); - return Ok(None); + if let Some(file_path) = query.file_path() { + if let Some(exclusion) = match_any_exclusion( + &file_path, + &file_resolver_settings.exclude, + &file_resolver_settings.extend_exclude, + None, + Some(&formatter_settings.exclude), + ) { + tracing::debug!("Ignored path via `{}`: {}", exclusion, file_path.display()); + return Ok(None); + } } let source = text_document.contents(); - let mut formatted = crate::format::format(text_document, source_type, formatter_settings) - .with_failure_code(lsp_server::ErrorCode::InternalError)?; + let mut formatted = + crate::format::format(text_document, query.source_type(), formatter_settings) + .with_failure_code(lsp_server::ErrorCode::InternalError)?; // fast path - if the code is the same, return early if formatted == source { return Ok(None); 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 ea09f1f47e..7b678a3e16 100644 --- a/crates/ruff_server/src/server/api/requests/format_range.rs +++ b/crates/ruff_server/src/server/api/requests/format_range.rs @@ -1,15 +1,11 @@ -use std::path::Path; - use lsp_types::{self as types, request as req, Range}; -use ruff_python_ast::PySourceType; use ruff_workspace::resolver::match_any_exclusion; -use ruff_workspace::{FileResolverSettings, FormatterSettings}; use crate::edit::{RangeExt, ToRangeExt}; use crate::server::api::LSPResult; use crate::server::{client::Notifier, Result}; -use crate::session::DocumentSnapshot; +use crate::session::{DocumentQuery, DocumentSnapshot}; use crate::{PositionEncoding, TextDocument}; pub(crate) struct FormatRange; @@ -39,45 +35,43 @@ fn format_document_range( .as_single_document() .expect("format should only be called on text documents or notebook cells"); let query = snapshot.query(); - format_text_document_range( - text_document, - range, - query.source_type(), - query.file_path(), - query.settings().file_resolver(), - query.settings().formatter(), - snapshot.encoding(), - ) + format_text_document_range(text_document, range, query, snapshot.encoding()) } /// Formats the specified [`Range`] in the [`TextDocument`]. fn format_text_document_range( text_document: &TextDocument, range: Range, - source_type: PySourceType, - file_path: &Path, - file_resolver_settings: &FileResolverSettings, - formatter_settings: &FormatterSettings, + query: &DocumentQuery, encoding: PositionEncoding, ) -> Result { + let file_resolver_settings = query.settings().file_resolver(); + let formatter_settings = query.settings().formatter(); + // If the document is excluded, return early. - if let Some(exclusion) = match_any_exclusion( - file_path, - &file_resolver_settings.exclude, - &file_resolver_settings.extend_exclude, - None, - Some(&formatter_settings.exclude), - ) { - tracing::debug!("Ignored path via `{}`: {}", exclusion, file_path.display()); - return Ok(None); + if let Some(file_path) = query.file_path() { + if let Some(exclusion) = match_any_exclusion( + &file_path, + &file_resolver_settings.exclude, + &file_resolver_settings.extend_exclude, + None, + Some(&formatter_settings.exclude), + ) { + tracing::debug!("Ignored path via `{}`: {}", exclusion, file_path.display()); + return Ok(None); + } } let text = text_document.contents(); let index = text_document.index(); let range = range.to_text_range(text, index, encoding); - let formatted_range = - crate::format::format_range(text_document, source_type, formatter_settings, range) - .with_failure_code(lsp_server::ErrorCode::InternalError)?; + let formatted_range = crate::format::format_range( + text_document, + query.source_type(), + formatter_settings, + 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 683b3ea82c..d1327bb8e3 100644 --- a/crates/ruff_server/src/session.rs +++ b/crates/ruff_server/src/session.rs @@ -1,13 +1,7 @@ //! Data model, state management, and configuration resolution. -mod capabilities; -mod index; -mod settings; - -use std::path::PathBuf; use std::sync::Arc; -use anyhow::anyhow; use lsp_types::{ClientCapabilities, NotebookDocumentCellChange, Url}; use crate::edit::{DocumentKey, DocumentVersion, NotebookDocument}; @@ -17,6 +11,10 @@ pub(crate) use self::capabilities::ResolvedClientCapabilities; pub(crate) use self::index::DocumentQuery; pub(crate) use self::settings::{AllSettings, ClientSettings}; +mod capabilities; +mod index; +mod settings; + /// The global state for the LSP pub(crate) struct Session { /// Used to retrieve information about open documents and settings. @@ -43,27 +41,25 @@ impl Session { client_capabilities: &ClientCapabilities, position_encoding: PositionEncoding, global_settings: ClientSettings, - workspace_folders: Vec<(PathBuf, ClientSettings)>, - ) -> Self { - Self { + workspace_folders: Vec<(Url, ClientSettings)>, + ) -> crate::Result { + Ok(Self { position_encoding, - index: index::Index::new(workspace_folders, &global_settings), + index: index::Index::new(workspace_folders, &global_settings)?, global_settings, resolved_client_capabilities: Arc::new(ResolvedClientCapabilities::new( client_capabilities, )), - } + }) } - pub(crate) fn key_from_url(&self, url: &lsp_types::Url) -> crate::Result { - self.index - .key_from_url(url) - .ok_or_else(|| anyhow!("No document found for {url}")) + pub(crate) fn key_from_url(&self, url: Url) -> DocumentKey { + self.index.key_from_url(url) } /// Creates a document snapshot with the URL referencing the document to snapshot. - pub(crate) fn take_snapshot(&self, url: &Url) -> Option { - let key = self.key_from_url(url).ok()?; + pub(crate) fn take_snapshot(&self, url: Url) -> Option { + 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), @@ -73,12 +69,12 @@ impl Session { } /// Iterates over the LSP URLs for all open text documents. These URLs are valid file paths. - pub(super) fn text_document_urls(&self) -> impl Iterator + '_ { + pub(super) fn text_document_urls(&self) -> impl Iterator + '_ { self.index.text_document_urls() } /// Iterates over the LSP URLs for all open notebook documents. These URLs are valid file paths. - pub(super) fn notebook_document_urls(&self) -> impl Iterator + '_ { + pub(super) fn notebook_document_urls(&self) -> impl Iterator + '_ { self.index.notebook_document_urls() } @@ -114,16 +110,16 @@ impl Session { .update_notebook_document(key, cells, metadata, version, encoding) } - /// Registers a notebook document at the provided `path`. + /// Registers a notebook document at the provided `url`. /// If a document is already open here, it will be overwritten. - pub(crate) fn open_notebook_document(&mut self, path: PathBuf, document: NotebookDocument) { - self.index.open_notebook_document(path, document); + pub(crate) fn open_notebook_document(&mut self, url: Url, document: NotebookDocument) { + self.index.open_notebook_document(url, document); } - /// Registers a text document at the provided `path`. + /// Registers a text document at the provided `url`. /// If a document is already open here, it will be overwritten. - pub(crate) fn open_text_document(&mut self, path: PathBuf, document: TextDocument) { - self.index.open_text_document(path, document); + pub(crate) fn open_text_document(&mut self, url: Url, document: TextDocument) { + self.index.open_text_document(url, document); } /// De-registers a document, specified by its key. @@ -134,19 +130,18 @@ impl Session { } /// Reloads the settings index - pub(crate) fn reload_settings(&mut self, changed_path: &PathBuf) { - self.index.reload_settings(changed_path); + pub(crate) fn reload_settings(&mut self, changed_url: &Url) { + self.index.reload_settings(changed_url); } - /// Open a workspace folder at the given `path`. - pub(crate) fn open_workspace_folder(&mut self, path: PathBuf) { - self.index - .open_workspace_folder(path, &self.global_settings); + /// Open a workspace folder at the given `url`. + pub(crate) fn open_workspace_folder(&mut self, url: &Url) -> crate::Result<()> { + self.index.open_workspace_folder(url, &self.global_settings) } - /// Close a workspace folder at the given `path`. - pub(crate) fn close_workspace_folder(&mut self, path: &PathBuf) -> crate::Result<()> { - self.index.close_workspace_folder(path)?; + /// Close a workspace folder at the given `url`. + pub(crate) fn close_workspace_folder(&mut self, url: &Url) -> crate::Result<()> { + self.index.close_workspace_folder(url)?; Ok(()) } diff --git a/crates/ruff_server/src/session/index.rs b/crates/ruff_server/src/session/index.rs index ee9965e668..27a7bedc25 100644 --- a/crates/ruff_server/src/session/index.rs +++ b/crates/ruff_server/src/session/index.rs @@ -1,36 +1,31 @@ use anyhow::anyhow; +use lsp_types::Url; use rustc_hash::FxHashMap; -use std::{ - collections::BTreeMap, - path::{Path, PathBuf}, - sync::Arc, -}; +use std::path::PathBuf; +use std::{collections::BTreeMap, path::Path, sync::Arc}; use crate::{ edit::{DocumentKey, DocumentVersion, NotebookDocument}, PositionEncoding, TextDocument, }; -use super::{ - settings::{self, ResolvedClientSettings}, - ClientSettings, -}; +use super::{settings::ResolvedClientSettings, ClientSettings}; mod ruff_settings; pub(crate) use ruff_settings::RuffSettings; -type DocumentIndex = FxHashMap; -type NotebookCellIndex = FxHashMap; type SettingsIndex = BTreeMap; /// Stores and tracks all open documents in a session, along with their associated settings. #[derive(Default)] pub(crate) struct Index { - /// Maps all document file paths to the associated document controller - documents: DocumentIndex, - /// Maps opaque cell URLs to a notebook path - notebook_cells: NotebookCellIndex, + /// Maps all document file URLs to the associated document controller + documents: FxHashMap, + + /// Maps opaque cell URLs to a notebook URL (document) + notebook_cells: FxHashMap, + /// Maps a workspace folder root to its settings. settings: SettingsIndex, } @@ -38,10 +33,11 @@ pub(crate) struct Index { /// Settings associated with a workspace. struct WorkspaceSettings { client_settings: ResolvedClientSettings, - workspace_settings_index: ruff_settings::RuffSettingsIndex, + ruff_settings: ruff_settings::RuffSettingsIndex, } /// A mutable handler to an underlying document. +#[derive(Debug)] enum DocumentController { Text(Arc), Notebook(Arc), @@ -53,14 +49,15 @@ enum DocumentController { #[derive(Clone)] pub(crate) enum DocumentQuery { Text { - file_path: PathBuf, + file_url: Url, document: Arc, settings: Arc, }, Notebook { /// The selected notebook cell, if it exists. - cell_uri: Option, - file_path: PathBuf, + cell_url: Option, + /// The URL of the notebook. + file_url: Url, notebook: Arc, settings: Arc, }, @@ -68,42 +65,38 @@ pub(crate) enum DocumentQuery { impl Index { pub(super) fn new( - workspace_folders: Vec<(PathBuf, ClientSettings)>, + workspace_folders: Vec<(Url, ClientSettings)>, global_settings: &ClientSettings, - ) -> Self { + ) -> crate::Result { let mut settings_index = BTreeMap::new(); - for (path, workspace_settings) in workspace_folders { + for (url, workspace_settings) in workspace_folders { Self::register_workspace_settings( &mut settings_index, - path, + &url, Some(workspace_settings), global_settings, - ); + )?; } - Self { + Ok(Self { documents: FxHashMap::default(), notebook_cells: FxHashMap::default(), settings: settings_index, - } + }) } - pub(super) fn text_document_urls(&self) -> impl Iterator + '_ { + pub(super) fn text_document_urls(&self) -> impl Iterator + '_ { self.documents .iter() .filter(|(_, doc)| doc.as_text().is_some()) - .map(|(path, _)| { - lsp_types::Url::from_file_path(path).expect("valid file path should convert to URL") - }) + .map(|(url, _)| url) } - pub(super) fn notebook_document_urls(&self) -> impl Iterator + '_ { + pub(super) fn notebook_document_urls(&self) -> impl Iterator + '_ { self.documents .iter() .filter(|(_, doc)| doc.as_notebook().is_some()) - .map(|(path, _)| { - lsp_types::Url::from_file_path(path).expect("valid file path should convert to URL") - }) + .map(|(url, _)| url) } pub(super) fn update_text_document( @@ -128,22 +121,17 @@ impl Index { Ok(()) } - pub(super) fn key_from_url(&self, url: &lsp_types::Url) -> Option { - if self.notebook_cells.contains_key(url) { - return Some(DocumentKey::NotebookCell(url.clone())); + pub(super) fn key_from_url(&self, url: Url) -> DocumentKey { + if self.notebook_cells.contains_key(&url) { + DocumentKey::NotebookCell(url) + } else if Path::new(url.path()) + .extension() + .map_or(false, |ext| ext.eq_ignore_ascii_case("ipynb")) + { + DocumentKey::Notebook(url) + } else { + DocumentKey::Text(url) } - let path = url.to_file_path().ok()?; - Some( - match path - .extension() - .unwrap_or_default() - .to_str() - .unwrap_or_default() - { - "ipynb" => DocumentKey::Notebook(path), - _ => DocumentKey::Text(path), - }, - ) } pub(super) fn update_notebook_document( @@ -161,7 +149,7 @@ impl Index { .. }) = cells.as_ref().and_then(|cells| cells.structure.as_ref()) { - let Some(path) = self.path_for_key(key).cloned() else { + let Some(path) = self.url_for_key(key).cloned() else { anyhow::bail!("Tried to open unavailable document `{key}`"); }; @@ -190,24 +178,29 @@ impl Index { pub(super) fn open_workspace_folder( &mut self, - path: PathBuf, + url: &Url, 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, path, None, global_settings); + Self::register_workspace_settings(&mut self.settings, url, None, global_settings) } fn register_workspace_settings( settings_index: &mut SettingsIndex, - workspace_path: PathBuf, + workspace_url: &Url, workspace_settings: Option, global_settings: &ClientSettings, - ) { + ) -> crate::Result<()> { let client_settings = if let Some(workspace_settings) = workspace_settings { ResolvedClientSettings::with_workspace(&workspace_settings, global_settings) } else { ResolvedClientSettings::global(global_settings) }; + + let workspace_path = workspace_url + .to_file_path() + .map_err(|()| anyhow!("workspace URL was not a file path!"))?; + let workspace_settings_index = ruff_settings::RuffSettingsIndex::new( &workspace_path, client_settings.editor_settings(), @@ -217,23 +210,31 @@ impl Index { workspace_path, WorkspaceSettings { client_settings, - workspace_settings_index, + ruff_settings: workspace_settings_index, }, ); + + Ok(()) } - pub(super) fn close_workspace_folder(&mut self, workspace_path: &PathBuf) -> crate::Result<()> { - self.settings.remove(workspace_path).ok_or_else(|| { + pub(super) fn close_workspace_folder(&mut self, workspace_url: &Url) -> crate::Result<()> { + let workspace_path = workspace_url + .to_file_path() + .map_err(|()| anyhow!("workspace URL was not a file path!"))?; + + self.settings.remove(&workspace_path).ok_or_else(|| { anyhow!( - "Tried to remove non-existent folder {}", - workspace_path.display() + "Tried to remove non-existent workspace URI {}", + workspace_url ) })?; + // O(n) complexity, which isn't ideal... but this is an uncommon operation. self.documents - .retain(|path, _| !path.starts_with(workspace_path)); + .retain(|url, _| !Path::new(url.path()).starts_with(&workspace_path)); self.notebook_cells - .retain(|_, path| !path.starts_with(workspace_path)); + .retain(|_, url| !Path::new(url.path()).starts_with(&workspace_path)); + Ok(()) } @@ -242,70 +243,99 @@ impl Index { key: DocumentKey, global_settings: &ClientSettings, ) -> Option { - let path = self.path_for_key(&key)?.clone(); + let url = self.url_for_key(&key)?.clone(); + let document_settings = self - .settings_for_path(&path) - .map(|settings| settings.workspace_settings_index.get(&path)) + .settings_for_url(&url) + .map(|settings| { + if let Ok(file_path) = url.to_file_path() { + settings.ruff_settings.get(&file_path) + } else { + // For a new unsaved and untitled document, use the ruff settings from the top of the workspace + // but only IF: + // * It is the only workspace + // * The ruff setting is at the top of the workspace (in the root folder) + // Otherwise, use the fallback settings. + if self.settings.len() == 1 { + let workspace_path = self.settings.keys().next().unwrap(); + settings.ruff_settings.get(&workspace_path.join("untitled")) + } else { + tracing::debug!("Use the fallback settings for the new document '{url}'."); + settings.ruff_settings.fallback() + } + } + }) .unwrap_or_else(|| { tracing::warn!( "No settings available for {} - falling back to default settings", - path.display() + url ); let resolved_global = ResolvedClientSettings::global(global_settings); - let root = path.parent().unwrap_or(&path); + // 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(), root, )) }); - let controller = self.documents.get(&path)?; - let cell_uri = match key { - DocumentKey::NotebookCell(uri) => Some(uri), + let controller = self.documents.get(&url)?; + let cell_url = match key { + DocumentKey::NotebookCell(cell_url) => Some(cell_url), _ => None, }; - Some(controller.make_ref(cell_uri, path, document_settings)) + Some(controller.make_ref(cell_url, url, document_settings)) } /// Reloads relevant existing settings files based on a changed settings file path. /// This does not currently register new settings files. - pub(super) fn reload_settings(&mut self, changed_path: &PathBuf) { - let search_path = changed_path.parent().unwrap_or(changed_path); - for (root, settings) in self - .settings - .iter_mut() - .filter(|(path, _)| path.starts_with(search_path)) - { - settings.workspace_settings_index = ruff_settings::RuffSettingsIndex::new( + pub(super) fn reload_settings(&mut self, changed_url: &Url) { + let Ok(changed_path) = changed_url.to_file_path() else { + // Files that don't map to a path can't be a workspace configuration file. + return; + }; + + let Some(enclosing_folder) = changed_path.parent() else { + return; + }; + + // TODO: I think this does not correctly reload settings when using `extend` and the extended + // setting isn't in a parent folder. + for (root, settings) in self.settings.range_mut(enclosing_folder.to_path_buf()..) { + if !root.starts_with(enclosing_folder) { + break; + } + + settings.ruff_settings = ruff_settings::RuffSettingsIndex::new( root, settings.client_settings.editor_settings(), ); } } - pub(super) fn open_text_document(&mut self, path: PathBuf, document: TextDocument) { + pub(super) fn open_text_document(&mut self, url: Url, document: TextDocument) { self.documents - .insert(path, DocumentController::new_text(document)); + .insert(url, DocumentController::new_text(document)); } - pub(super) fn open_notebook_document(&mut self, path: PathBuf, document: NotebookDocument) { - for url in document.urls() { - self.notebook_cells.insert(url.clone(), path.clone()); + pub(super) fn open_notebook_document(&mut self, notebook_url: Url, document: NotebookDocument) { + for cell_url in document.urls() { + self.notebook_cells + .insert(cell_url.clone(), notebook_url.clone()); } self.documents - .insert(path, DocumentController::new_notebook(document)); + .insert(notebook_url, DocumentController::new_notebook(document)); } pub(super) fn close_document(&mut self, key: &DocumentKey) -> crate::Result<()> { - let Some(path) = self.path_for_key(key).cloned() else { - anyhow::bail!("Tried to open unavailable document `{key}`"); + let Some(url) = self.url_for_key(key).cloned() else { + anyhow::bail!("Tried to close unavailable document `{key}`"); }; - let Some(controller) = self.documents.remove(&path) else { - anyhow::bail!( - "tried to close document that didn't exist at {}", - path.display() - ) + let Some(controller) = self.documents.remove(&url) else { + anyhow::bail!("tried to close document that didn't exist at {}", url) }; if let Some(notebook) = controller.as_notebook() { for url in notebook.urls() { @@ -321,13 +351,13 @@ impl Index { &self, key: &DocumentKey, global_settings: &ClientSettings, - ) -> settings::ResolvedClientSettings { - let Some(path) = self.path_for_key(key) else { + ) -> ResolvedClientSettings { + let Some(url) = self.url_for_key(key) else { return ResolvedClientSettings::global(global_settings); }; let Some(WorkspaceSettings { client_settings, .. - }) = self.settings_for_path(path) + }) = self.settings_for_url(url) else { return ResolvedClientSettings::global(global_settings); }; @@ -338,22 +368,38 @@ impl Index { &mut self, key: &DocumentKey, ) -> crate::Result<&mut DocumentController> { - let Some(path) = self.path_for_key(key).cloned() else { + let Some(url) = self.url_for_key(key).cloned() else { anyhow::bail!("Tried to open unavailable document `{key}`"); }; - let Some(controller) = self.documents.get_mut(&path) else { - anyhow::bail!("Document controller not available at `{}`", path.display()); + let Some(controller) = self.documents.get_mut(&url) else { + anyhow::bail!("Document controller not available at `{}`", url); }; Ok(controller) } - fn path_for_key<'a>(&'a self, key: &'a DocumentKey) -> Option<&'a PathBuf> { + fn url_for_key<'a>(&'a self, key: &'a DocumentKey) -> Option<&'a Url> { match key { DocumentKey::Notebook(path) | DocumentKey::Text(path) => Some(path), DocumentKey::NotebookCell(uri) => self.notebook_cells.get(uri), } } + fn settings_for_url(&self, url: &Url) -> Option<&WorkspaceSettings> { + if let Ok(path) = url.to_file_path() { + self.settings_for_path(&path) + } else { + // If there's only a single workspace, use that configuration for an untitled document. + if self.settings.len() == 1 { + tracing::debug!( + "Falling back to configuration of the only active workspace for the new document '{url}'." + ); + self.settings.values().next() + } else { + None + } + } + } + fn settings_for_path(&self, path: &Path) -> Option<&WorkspaceSettings> { self.settings .range(..path.to_path_buf()) @@ -373,19 +419,19 @@ impl DocumentController { fn make_ref( &self, - cell_uri: Option, - file_path: PathBuf, + cell_url: Option, + file_url: Url, settings: Arc, ) -> DocumentQuery { match &self { Self::Notebook(notebook) => DocumentQuery::Notebook { - cell_uri, - file_path, + cell_url, + file_url, notebook: notebook.clone(), settings, }, Self::Text(document) => DocumentQuery::Text { - file_path, + file_url, document: document.clone(), settings, }, @@ -426,12 +472,12 @@ impl DocumentQuery { /// Retrieve the original key that describes this document query. pub(crate) fn make_key(&self) -> DocumentKey { match self { - Self::Text { file_path, .. } => DocumentKey::Text(file_path.clone()), + Self::Text { file_url, .. } => DocumentKey::Text(file_url.clone()), Self::Notebook { - cell_uri: Some(cell_uri), + cell_url: Some(cell_uri), .. } => DocumentKey::NotebookCell(cell_uri.clone()), - Self::Notebook { file_path, .. } => DocumentKey::Notebook(file_path.clone()), + Self::Notebook { file_url, .. } => DocumentKey::Notebook(file_url.clone()), } } @@ -465,7 +511,7 @@ impl DocumentQuery { /// Get the source type of the document associated with this query. pub(crate) fn source_type(&self) -> ruff_python_ast::PySourceType { match self { - Self::Text { .. } => ruff_python_ast::PySourceType::from(self.file_path()), + Self::Text { .. } => ruff_python_ast::PySourceType::from(self.virtual_file_path()), Self::Notebook { .. } => ruff_python_ast::PySourceType::Ipynb, } } @@ -478,20 +524,39 @@ impl DocumentQuery { } } - /// Get the underlying file path for the document selected by this query. - pub(crate) fn file_path(&self) -> &Path { + /// Get the URL for the document selected by this query. + pub(crate) fn file_url(&self) -> &Url { match self { - Self::Text { file_path, .. } | Self::Notebook { file_path, .. } => file_path, + Self::Text { file_url, .. } | Self::Notebook { file_url, .. } => file_url, } } + /// Get the path for the document selected by this query. + /// + /// Returns `None` if this is an unsaved (untitled) document. + /// + /// The path isn't guaranteed to point to a real path on the filesystem. This is the case + /// for unsaved (untitled) documents. + pub(crate) fn file_path(&self) -> Option { + self.file_url().to_file_path().ok() + } + + /// Get the path for the document selected by this query, ignoring whether the file exists on disk. + /// + /// Returns the URL's path if this is an unsaved (untitled) document. + pub(crate) fn virtual_file_path(&self) -> &Path { + Path::new(self.file_url().path()) + } + /// Attempt to access the single inner text document selected by the query. /// If this query is selecting an entire notebook document, this will return `None`. pub(crate) fn as_single_document(&self) -> Option<&TextDocument> { match self { Self::Text { document, .. } => Some(document), Self::Notebook { - notebook, cell_uri, .. + notebook, + cell_url: cell_uri, + .. } => cell_uri .as_ref() .and_then(|cell_uri| notebook.cell_document_by_uri(cell_uri)), diff --git a/crates/ruff_server/src/session/index/ruff_settings.rs b/crates/ruff_server/src/session/index/ruff_settings.rs index eed37b1f95..1939a91aca 100644 --- a/crates/ruff_server/src/session/index/ruff_settings.rs +++ b/crates/ruff_server/src/session/index/ruff_settings.rs @@ -28,6 +28,7 @@ pub(crate) struct RuffSettings { } pub(super) struct RuffSettingsIndex { + /// Index from folder to the resoled ruff settings. index: BTreeMap>, fallback: Arc, } @@ -189,14 +190,15 @@ impl RuffSettingsIndex { } pub(super) fn get(&self, document_path: &Path) -> Arc { - if let Some((_, settings)) = self - .index + self.index .range(..document_path.to_path_buf()) .rfind(|(path, _)| document_path.starts_with(path)) - { - return settings.clone(); - } + .map(|(_, settings)| settings) + .unwrap_or_else(|| &self.fallback) + .clone() + } + pub(super) fn fallback(&self) -> Arc { self.fallback.clone() } } diff --git a/crates/ruff_server/src/session/settings.rs b/crates/ruff_server/src/session/settings.rs index a7faa5e3d0..8fc98572bf 100644 --- a/crates/ruff_server/src/session/settings.rs +++ b/crates/ruff_server/src/session/settings.rs @@ -7,7 +7,7 @@ use serde::Deserialize; use ruff_linter::{line_width::LineLength, RuleSelector}; /// Maps a workspace URI to its associated client settings. Used during server initialization. -pub(crate) type WorkspaceSettingsMap = FxHashMap; +pub(crate) type WorkspaceSettingsMap = FxHashMap; /// 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 @@ -170,12 +170,7 @@ impl AllSettings { workspace_settings: workspace_settings.map(|workspace_settings| { workspace_settings .into_iter() - .map(|settings| { - ( - settings.workspace.to_file_path().unwrap(), - settings.settings, - ) - }) + .map(|settings| (settings.workspace, settings.settings)) .collect() }), } @@ -564,7 +559,8 @@ mod tests { global_settings, workspace_settings, } = AllSettings::from_init_options(options); - let path = PathBuf::from_str("/Users/test/projects/pandas").expect("path should be valid"); + 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( @@ -595,7 +591,8 @@ mod tests { } } ); - let path = PathBuf::from_str("/Users/test/projects/scipy").expect("path should be valid"); + let path = + Url::from_str("file:///Users/test/projects/scipy").expect("path should be valid"); assert_eq!( ResolvedClientSettings::with_workspace( workspace_settings