From ef8727f93ff531ef556123fe76daf81506269116 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Fri, 3 May 2024 19:28:54 -0700 Subject: [PATCH] Implement support for Jupyter Notebooks in `ruff server` --- Cargo.lock | 1 + crates/ruff_notebook/src/cell.rs | 2 +- crates/ruff_notebook/src/notebook.rs | 35 +- crates/ruff_server/Cargo.toml | 1 + crates/ruff_server/src/edit.rs | 139 ++++++- crates/ruff_server/src/edit/document.rs | 4 +- crates/ruff_server/src/edit/notebook.rs | 137 +++++++ crates/ruff_server/src/edit/range.rs | 52 ++- crates/ruff_server/src/fix.rs | 110 ++++-- crates/ruff_server/src/format.rs | 6 +- crates/ruff_server/src/lib.rs | 2 +- crates/ruff_server/src/lint.rs | 147 +++++--- crates/ruff_server/src/server.rs | 25 +- crates/ruff_server/src/server/api.rs | 3 + .../ruff_server/src/server/api/diagnostics.rs | 45 ++- .../src/server/api/notifications.rs | 6 +- .../src/server/api/notifications/cancel.rs | 1 - .../server/api/notifications/did_change.rs | 16 +- .../api/notifications/did_change_notebook.rs | 16 +- .../notifications/did_change_watched_files.rs | 2 +- .../api/notifications/did_change_workspace.rs | 19 +- .../src/server/api/notifications/did_close.rs | 10 +- .../api/notifications/did_close_notebook.rs | 37 ++ .../src/server/api/notifications/did_open.rs | 19 +- .../api/notifications/did_open_notebook.rs | 27 +- .../src/server/api/requests/code_action.rs | 14 +- .../api/requests/code_action_resolve.rs | 44 +-- .../src/server/api/requests/diagnostic.rs | 6 +- .../server/api/requests/execute_command.rs | 10 +- .../src/server/api/requests/format.rs | 6 +- .../src/server/api/requests/format_range.rs | 6 +- .../src/server/api/requests/hover.rs | 6 +- crates/ruff_server/src/session.rs | 54 +-- crates/ruff_server/src/session/settings.rs | 17 +- crates/ruff_server/src/session/workspace.rs | 347 ++++++++++-------- crates/ruff_server/tests/document.rs | 4 +- crates/ruff_source_file/src/locator.rs | 7 - 37 files changed, 969 insertions(+), 414 deletions(-) create mode 100644 crates/ruff_server/src/edit/notebook.rs create mode 100644 crates/ruff_server/src/server/api/notifications/did_close_notebook.rs diff --git a/Cargo.lock b/Cargo.lock index 6b96879956769..13fc1bb20114f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2361,6 +2361,7 @@ dependencies = [ "ruff_diagnostics", "ruff_formatter", "ruff_linter", + "ruff_notebook", "ruff_python_ast", "ruff_python_codegen", "ruff_python_formatter", diff --git a/crates/ruff_notebook/src/cell.rs b/crates/ruff_notebook/src/cell.rs index 8718bd528417f..b43087b52b919 100644 --- a/crates/ruff_notebook/src/cell.rs +++ b/crates/ruff_notebook/src/cell.rs @@ -23,7 +23,7 @@ impl fmt::Display for SourceValue { impl Cell { /// Return the [`SourceValue`] of the cell. - pub(crate) fn source(&self) -> &SourceValue { + pub fn source(&self) -> &SourceValue { match self { Cell::Code(cell) => &cell.source, Cell::Markdown(cell) => &cell.source, diff --git a/crates/ruff_notebook/src/notebook.rs b/crates/ruff_notebook/src/notebook.rs index fef73cd853dfd..f2a110089dc38 100644 --- a/crates/ruff_notebook/src/notebook.rs +++ b/crates/ruff_notebook/src/notebook.rs @@ -1,5 +1,5 @@ use std::cmp::Ordering; -use std::collections::HashSet; +use std::collections::{BTreeMap, HashSet}; use std::fs::File; use std::io::{BufReader, Cursor, Read, Seek, SeekFrom, Write}; use std::path::Path; @@ -51,6 +51,15 @@ pub enum NotebookError { InvalidFormat(i64), } +pub enum NotebookSource { + File(Notebook), + Server { + source_code: String, + index: OnceCell, + cell_offsets: CellOffsets, + }, +} + #[derive(Clone, Debug, PartialEq)] pub struct Notebook { /// Python source code of the notebook. @@ -85,6 +94,24 @@ impl Notebook { Self::from_reader(Cursor::new(source_code)) } + pub fn from_cells(cells: Vec) -> Result { + let raw_notebook = RawNotebook { + cells, + metadata: crate::RawNotebookMetadata { + authors: None, + kernelspec: None, + language_info: None, + orig_nbformat: None, + title: None, + extra: BTreeMap::default(), + }, + nbformat: 4, + nbformat_minor: 5, + }; + + Self::from_raw(raw_notebook, false) + } + /// Read a Jupyter Notebook from a [`Read`] implementer. /// /// See also the black implementation @@ -113,7 +140,13 @@ impl Notebook { }); } }; + Self::from_raw(raw_notebook, trailing_newline) + } + fn from_raw( + mut raw_notebook: RawNotebook, + trailing_newline: bool, + ) -> Result { // v4 is what everybody uses if raw_notebook.nbformat != 4 { // bail because we should have already failed at the json schema stage diff --git a/crates/ruff_server/Cargo.toml b/crates/ruff_server/Cargo.toml index 3fb502dbc88e5..e019775c076d8 100644 --- a/crates/ruff_server/Cargo.toml +++ b/crates/ruff_server/Cargo.toml @@ -22,6 +22,7 @@ ruff_python_formatter = { path = "../ruff_python_formatter" } ruff_python_index = { path = "../ruff_python_index" } ruff_python_parser = { path = "../ruff_python_parser" } ruff_source_file = { path = "../ruff_source_file" } +ruff_notebook = { path = "../ruff_notebook" } ruff_text_size = { path = "../ruff_text_size" } ruff_workspace = { path = "../ruff_workspace" } diff --git a/crates/ruff_server/src/edit.rs b/crates/ruff_server/src/edit.rs index ec41b22a94479..dbb7416e0cd0c 100644 --- a/crates/ruff_server/src/edit.rs +++ b/crates/ruff_server/src/edit.rs @@ -1,18 +1,29 @@ //! Types and utilities for working with text, modifying source files, and `Ruff <-> LSP` type conversion. mod document; +mod notebook; mod range; mod replacement; -use std::collections::HashMap; +use std::{ + collections::HashMap, + path::{Display, PathBuf}, +}; -pub use document::Document; +use anyhow::anyhow; pub(crate) use document::DocumentVersion; +pub use document::TextDocument; use lsp_types::PositionEncodingKind; +pub(crate) use notebook::NotebookDocument; pub(crate) use range::{RangeExt, ToRangeExt}; pub(crate) use replacement::Replacement; +use ruff_linter::source_kind::SourceKind; +use ruff_source_file::LineIndex; -use crate::session::ResolvedClientCapabilities; +use crate::{ + fix::Fixes, + session::{DocumentQuery, ResolvedClientCapabilities}, +}; /// A convenient enumeration for supported text encodings. Can be converted to [`lsp_types::PositionEncodingKind`]. // Please maintain the order from least to greatest priority for the derived `Ord` impl. @@ -29,6 +40,108 @@ pub enum PositionEncoding { UTF8, } +/// A wrapper for a document. Can be either a text document (`.py`) +/// or a notebook document (`.ipynb`). +#[derive(Clone)] +pub(crate) enum Document { + Text(TextDocument), + Notebook(NotebookDocument), +} + +#[derive(Clone, Debug)] +pub(crate) enum DocumentKey { + File(PathBuf), + Cell(lsp_types::Url), +} + +impl Document { + pub(crate) fn version(&self) -> DocumentVersion { + match self { + Self::Notebook(notebook) => notebook.version(), + Self::Text(py) => py.version(), + } + } + + pub(crate) fn make_source_kind(&self) -> SourceKind { + match self { + Self::Notebook(notebook) => { + let notebook = SourceKind::IpyNotebook(notebook.make_ruff_notebook()); + tracing::info!("{notebook:?}"); + notebook + } + Self::Text(text) => SourceKind::Python(text.contents().to_string()), + } + } + + pub(crate) fn as_notebook(&self) -> Option<&NotebookDocument> { + match self { + Self::Notebook(notebook) => Some(¬ebook), + Self::Text(_) => None, + } + } + + pub(crate) fn as_text(&self) -> Option<&TextDocument> { + match self { + Self::Text(py) => Some(&py), + Self::Notebook(_) => None, + } + } + + pub(crate) fn as_notebook_mut(&mut self) -> Option<&mut NotebookDocument> { + match self { + Self::Notebook(ref mut notebook) => Some(notebook), + Self::Text(_) => None, + } + } + + pub(crate) fn as_text_mut(&mut self) -> Option<&mut TextDocument> { + match self { + Self::Notebook(_) => None, + Self::Text(ref mut py) => Some(py), + } + } + + pub(crate) fn as_notebook_cell(&self, uri: &lsp_types::Url) -> Option<&TextDocument> { + self.as_notebook()?.cell_document_by_uri(uri) + } + + pub(crate) fn as_notebook_cell_mut( + &mut self, + uri: &lsp_types::Url, + ) -> Option<&mut TextDocument> { + self.as_notebook_mut()?.cell_document_by_uri_mut(uri) + } +} + +impl DocumentKey { + pub(crate) fn from_url(url: &lsp_types::Url) -> Self { + if url.scheme() != "file" { + return Self::Cell(url.clone()); + } + url.to_file_path() + .map(Self::File) + .unwrap_or_else(|_| Self::Cell(url.clone())) + } + + pub(crate) fn to_url(self) -> lsp_types::Url { + match self { + Self::Cell(url) => url, + Self::File(path) => { + lsp_types::Url::from_file_path(path).expect("path should convert to file URL") + } + } + } +} + +impl std::fmt::Display for DocumentKey { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Cell(url) => url.fmt(f), + Self::File(path) => path.display().fmt(f), + } + } +} + /// Tracks multi-document edits to eventually merge into a `WorkspaceEdit`. /// Compatible with clients that don't support `workspace.workspaceEdit.documentChanges`. #[derive(Debug)] @@ -72,6 +185,26 @@ impl WorkspaceEditTracker { } } + pub(crate) fn set_fixes_for_document( + &mut self, + document: &DocumentQuery, + fixes: Fixes, + ) -> crate::Result<()> { + for (cell, edits) in fixes.into_iter() { + let uri = if let Some(notebook) = document.as_notebook() { + notebook + .cell_uri_by_index(cell) + .cloned() + .ok_or_else(|| anyhow!("cell index {cell} does not exist"))? + } else { + document.key().clone().to_url() + }; + + self.set_edits_for_document(uri, document.version(), edits)?; + } + Ok(()) + } + /// Sets the edits made to a specific document. This should only be called /// once for each document `uri`, and will fail if this is called for the same `uri` /// multiple times. diff --git a/crates/ruff_server/src/edit/document.rs b/crates/ruff_server/src/edit/document.rs index bd6c49be3eece..61d39b1cd42de 100644 --- a/crates/ruff_server/src/edit/document.rs +++ b/crates/ruff_server/src/edit/document.rs @@ -10,7 +10,7 @@ pub(crate) type DocumentVersion = i32; /// The state for an individual document in the server. Stays up-to-date /// with changes made by the user, including unsaved changes. #[derive(Debug, Clone)] -pub struct Document { +pub struct TextDocument { /// The string contents of the document. contents: String, /// A computed line index for the document. This should always reflect @@ -22,7 +22,7 @@ pub struct Document { version: DocumentVersion, } -impl Document { +impl TextDocument { pub fn new(contents: String, version: DocumentVersion) -> Self { let index = LineIndex::from_source_text(&contents); Self { diff --git a/crates/ruff_server/src/edit/notebook.rs b/crates/ruff_server/src/edit/notebook.rs new file mode 100644 index 0000000000000..3b0149df6361d --- /dev/null +++ b/crates/ruff_server/src/edit/notebook.rs @@ -0,0 +1,137 @@ +use std::{ + collections::{BTreeMap, HashMap}, + hash::BuildHasherDefault, +}; + +use lsp_types::{NotebookCellKind, Url}; +use ruff_source_file::{LineIndex, OneIndexed}; +use rustc_hash::FxHashMap; + +use crate::TextDocument; + +use super::DocumentVersion; + +#[derive(Clone, Debug)] +pub(crate) struct NotebookDocument { + cells: Vec, + version: DocumentVersion, + cell_index: FxHashMap, +} + +#[derive(Clone, Debug)] +struct NotebookCell { + url: Url, + kind: NotebookCellKind, + document: TextDocument, +} + +impl NotebookDocument { + pub(crate) fn new( + version: DocumentVersion, + cells: Vec, + cell_documents: Vec, + ) -> Self { + let mut cell_contents: FxHashMap<_, _> = cell_documents + .into_iter() + .map(|document| (document.uri, document.text)) + .collect(); + + let cells: Vec<_> = cells + .into_iter() + .map(|cell| { + let contents = cell_contents.remove(&cell.document).unwrap_or_default(); + NotebookCell::new(cell, contents, version) + }) + .collect(); + + Self { + version, + cell_index: Self::make_cell_index(cells.as_slice()), + cells, + } + } + + pub(crate) fn make_ruff_notebook(&self) -> ruff_notebook::Notebook { + let cells = self + .cells + .iter() + .map(|cell| match cell.kind { + NotebookCellKind::Code => ruff_notebook::Cell::Code(ruff_notebook::CodeCell { + execution_count: None, + id: None, + metadata: serde_json::Value::Null, + outputs: vec![], + source: ruff_notebook::SourceValue::String( + cell.document.contents().to_string(), + ), + }), + NotebookCellKind::Markup => { + ruff_notebook::Cell::Markdown(ruff_notebook::MarkdownCell { + attachments: None, + id: None, + metadata: serde_json::Value::Null, + source: ruff_notebook::SourceValue::String( + cell.document.contents().to_string(), + ), + }) + } + }) + .collect(); + + ruff_notebook::Notebook::from_cells(cells).expect("notebook should convert successfully") + } + + pub(crate) fn version(&self) -> DocumentVersion { + self.version + } + + pub(crate) fn cell_uri_by_index(&self, index: usize) -> Option<&lsp_types::Url> { + self.cells.get(index).map(|cell| &cell.url) + } + + pub(crate) fn cell_document_by_uri(&self, uri: &lsp_types::Url) -> Option<&TextDocument> { + self.cells + .get(*self.cell_index.get(uri)?) + .map(|cell| &cell.document) + } + + pub(crate) fn cell_document_by_uri_mut( + &mut self, + uri: &lsp_types::Url, + ) -> Option<&mut TextDocument> { + self.cells + .get_mut(*self.cell_index.get(uri)?) + .map(|cell| &mut cell.document) + } + + pub(crate) fn urls(&self) -> impl Iterator { + self.cells.iter().map(|cell| &cell.url) + } + + fn rebuild_cell_index(&mut self) { + self.cell_index = Self::make_cell_index(&self.cells); + } + + fn make_cell_index(cells: &[NotebookCell]) -> FxHashMap { + let mut index = + HashMap::with_capacity_and_hasher(cells.len(), BuildHasherDefault::default()); + for (i, cell) in cells.iter().enumerate() { + index.insert(cell.url.clone(), i); + } + index + } +} + +impl NotebookCell { + pub(crate) fn new( + cell: lsp_types::NotebookCell, + contents: String, + version: DocumentVersion, + ) -> Self { + Self { + url: cell.document, + kind: cell.kind, + document: TextDocument::new(contents, version), + } + } +} diff --git a/crates/ruff_server/src/edit/range.rs b/crates/ruff_server/src/edit/range.rs index c26326e2ac74b..743fb9ee0f41c 100644 --- a/crates/ruff_server/src/edit/range.rs +++ b/crates/ruff_server/src/edit/range.rs @@ -1,5 +1,6 @@ use super::PositionEncoding; use lsp_types as types; +use ruff_notebook::NotebookIndex; use ruff_source_file::OneIndexed; use ruff_source_file::{LineIndex, SourceLocation}; use ruff_text_size::{TextRange, TextSize}; @@ -11,6 +12,13 @@ pub(crate) trait RangeExt { pub(crate) trait ToRangeExt { fn to_range(&self, text: &str, index: &LineIndex, encoding: PositionEncoding) -> types::Range; + fn to_notebook_range( + &self, + text: &str, + source_index: &LineIndex, + notebook_index: &NotebookIndex, + encoding: PositionEncoding, + ) -> Option<(usize, types::Range)>; } fn u32_index_to_usize(index: u32) -> usize { @@ -83,10 +91,40 @@ impl RangeExt for lsp_types::Range { impl ToRangeExt for TextRange { fn to_range(&self, text: &str, index: &LineIndex, encoding: PositionEncoding) -> types::Range { types::Range { - start: offset_to_position(self.start(), text, index, encoding), - end: offset_to_position(self.end(), text, index, encoding), + start: source_location_to_position(offset_to_source_location( + self.start(), + text, + index, + encoding, + )), + end: source_location_to_position(offset_to_source_location( + self.end(), + text, + index, + encoding, + )), } } + + fn to_notebook_range( + &self, + text: &str, + source_index: &LineIndex, + notebook_index: &NotebookIndex, + encoding: PositionEncoding, + ) -> Option<(usize, types::Range)> { + let start = offset_to_source_location(self.start(), text, source_index, encoding); + let end = offset_to_source_location(self.end(), text, source_index, encoding); + let cell = notebook_index.cell(start.row)?; + + Some(( + cell.to_zero_indexed(), + types::Range { + start: source_location_to_position(notebook_index.translate_location(&start)), + end: source_location_to_position(notebook_index.translate_location(&end)), + }, + )) + } } /// Converts a UTF-16 code unit offset for a given line into a UTF-8 column number. @@ -111,13 +149,13 @@ fn utf8_column_offset(utf16_code_unit_offset: u32, line: &str) -> TextSize { utf8_code_unit_offset } -fn offset_to_position( +fn offset_to_source_location( offset: TextSize, text: &str, index: &LineIndex, encoding: PositionEncoding, -) -> types::Position { - let location = match encoding { +) -> SourceLocation { + match encoding { PositionEncoding::UTF8 => { let row = index.line_index(offset); let column = offset - index.line_start(row, text); @@ -143,8 +181,10 @@ fn offset_to_position( } } PositionEncoding::UTF32 => index.source_location(offset, text), - }; + } +} +fn source_location_to_position(location: SourceLocation) -> types::Position { types::Position { line: u32::try_from(location.row.to_zero_indexed()).expect("row usize fits in u32"), character: u32::try_from(location.column.to_zero_indexed()) diff --git a/crates/ruff_server/src/fix.rs b/crates/ruff_server/src/fix.rs index f94e8c138610b..a1b73d9f5769d 100644 --- a/crates/ruff_server/src/fix.rs +++ b/crates/ruff_server/src/fix.rs @@ -2,28 +2,28 @@ use ruff_linter::{ linter::{FixerResult, LinterResult}, packaging::detect_package_root, settings::{flags, types::UnsafeFixes, LinterSettings}, - source_kind::SourceKind, }; +use ruff_notebook::{Cell, SourceValue}; use ruff_python_ast::PySourceType; use ruff_source_file::LineIndex; +use rustc_hash::FxHashMap; use std::borrow::Cow; use crate::{ edit::{Replacement, ToRangeExt}, + session::DocumentQuery, PositionEncoding, }; +pub(crate) type Fixes = FxHashMap>; + pub(crate) fn fix_all( - document: &crate::edit::Document, - document_url: &lsp_types::Url, + document: &DocumentQuery, linter_settings: &LinterSettings, encoding: PositionEncoding, -) -> crate::Result> { - let source = document.contents(); - - let document_path = document_url - .to_file_path() - .expect("document URL should be a valid file path"); +) -> crate::Result { + let document_path = document.file_path(); + let source_kind = document.make_source_kind(); let package = detect_package_root( document_path @@ -34,9 +34,6 @@ pub(crate) fn fix_all( let source_type = PySourceType::default(); - // TODO(jane): Support Jupyter Notebooks - let source_kind = SourceKind::Python(source.to_string()); - // We need to iteratively apply all safe fixes onto a single file and then // create a diff between the modified file and the original source to use as a single workspace // edit. @@ -48,7 +45,7 @@ pub(crate) fn fix_all( result: LinterResult { error, .. }, .. } = ruff_linter::linter::lint_fix( - &document_path, + document_path, package, flags::Noqa::Enabled, UnsafeFixes::Disabled, @@ -66,27 +63,80 @@ pub(crate) fn fix_all( // fast path: if `transformed` is still borrowed, no changes were made and we can return early if let Cow::Borrowed(_) = transformed { - return Ok(vec![]); + return Ok(Fixes::default()); } - let modified = transformed.source_code(); + if let (Some(source_notebook), Some(modified_notebook)) = + (source_kind.as_ipy_notebook(), transformed.as_ipy_notebook()) + { + let mut fixes = Fixes::default(); + for (index, (source, modified)) in source_notebook + .cells() + .iter() + .map(|cell| match cell.source() { + SourceValue::String(string) => string.clone(), + SourceValue::StringArray(array) => array.join(""), + }) + .zip( + modified_notebook + .cells() + .iter() + .map(|cell| match cell.source() { + SourceValue::String(string) => string.clone(), + SourceValue::StringArray(array) => array.join(""), + }), + ) + .enumerate() + { + let source_index = LineIndex::from_source_text(&source); + let modified_index = LineIndex::from_source_text(&modified); - let modified_index = LineIndex::from_source_text(modified); + let Replacement { + source_range, + modified_range, + } = Replacement::between( + &source, + source_index.line_starts(), + &modified, + modified_index.line_starts(), + ); - let source_index = document.index(); + fixes.insert( + index, + vec![lsp_types::TextEdit { + range: source_range.to_range( + source_kind.source_code(), + &source_index, + encoding, + ), + new_text: modified[modified_range].to_owned(), + }], + ); + } + Ok(fixes) + } else { + let source_index = LineIndex::from_source_text(source_kind.source_code()); - let Replacement { - source_range, - modified_range, - } = Replacement::between( - source, - source_index.line_starts(), - modified, - modified_index.line_starts(), - ); + let modified = transformed.source_code(); + let modified_index = LineIndex::from_source_text(modified); - Ok(vec![lsp_types::TextEdit { - range: source_range.to_range(source, source_index, encoding), - new_text: modified[modified_range].to_owned(), - }]) + let Replacement { + source_range, + modified_range, + } = Replacement::between( + source_kind.source_code(), + source_index.line_starts(), + modified, + modified_index.line_starts(), + ); + Ok([( + usize::default(), + vec![lsp_types::TextEdit { + range: source_range.to_range(source_kind.source_code(), &source_index, encoding), + new_text: modified[modified_range].to_owned(), + }], + )] + .into_iter() + .collect()) + } } diff --git a/crates/ruff_server/src/format.rs b/crates/ruff_server/src/format.rs index d69c76dd2d931..88f919e7f97a5 100644 --- a/crates/ruff_server/src/format.rs +++ b/crates/ruff_server/src/format.rs @@ -3,10 +3,10 @@ use ruff_python_formatter::format_module_source; use ruff_text_size::TextRange; use ruff_workspace::FormatterSettings; -use crate::edit::Document; +use crate::edit::TextDocument; pub(crate) fn format( - document: &Document, + document: &TextDocument, formatter_settings: &FormatterSettings, ) -> crate::Result { // TODO(jane): support Jupyter Notebook @@ -17,7 +17,7 @@ pub(crate) fn format( } pub(crate) fn format_range( - document: &Document, + document: &TextDocument, formatter_settings: &FormatterSettings, range: TextRange, ) -> crate::Result { diff --git a/crates/ruff_server/src/lib.rs b/crates/ruff_server/src/lib.rs index 5ec26c9d399a6..a4fd8c224d490 100644 --- a/crates/ruff_server/src/lib.rs +++ b/crates/ruff_server/src/lib.rs @@ -1,6 +1,6 @@ //! ## The Ruff Language Server -pub use edit::{Document, PositionEncoding}; +pub use edit::{PositionEncoding, TextDocument}; use lsp_types::CodeActionKind; pub use server::Server; diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index fdf4c54a77302..379fd2200ea4b 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -14,17 +14,18 @@ use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; use ruff_python_parser::lexer::LexResult; use ruff_python_parser::AsMode; -use ruff_source_file::Locator; +use ruff_source_file::{LineIndex, Locator}; use ruff_text_size::Ranged; +use rustc_hash::FxHashMap; use serde::{Deserialize, Serialize}; -use crate::{edit::ToRangeExt, PositionEncoding, DIAGNOSTIC_NAME}; +use crate::{edit::ToRangeExt, session::DocumentQuery, PositionEncoding, DIAGNOSTIC_NAME}; /// This is serialized on the diagnostic `data` field. #[derive(Serialize, Deserialize, Debug, Clone)] pub(crate) struct AssociatedDiagnosticData { pub(crate) kind: DiagnosticKind, - pub(crate) fix: Fix, + pub(crate) edits: Vec, pub(crate) code: String, } @@ -37,18 +38,15 @@ pub(crate) struct DiagnosticFix { pub(crate) edits: Vec, } +pub(crate) type Diagnostics = FxHashMap>; + pub(crate) fn check( - document: &crate::edit::Document, - document_url: &lsp_types::Url, + document: &DocumentQuery, linter_settings: &LinterSettings, encoding: PositionEncoding, -) -> Vec { - let contents = document.contents(); - let index = document.index().clone(); - - let document_path = document_url - .to_file_path() - .expect("document URL should be a valid file path"); +) -> Diagnostics { + let document_path = document.file_path(); + let source_kind = document.make_source_kind(); let package = detect_package_root( document_path @@ -59,14 +57,12 @@ pub(crate) fn check( let source_type = PySourceType::default(); - // TODO(jane): Support Jupyter Notebooks - let source_kind = SourceKind::Python(contents.to_string()); - // Tokenize once. - let tokens: Vec = ruff_python_parser::tokenize(contents, source_type.as_mode()); + let tokens: Vec = + ruff_python_parser::tokenize(source_kind.source_code(), source_type.as_mode()); // Map row and column locations to byte slices (lazily). - let locator = Locator::with_index(contents, index); + let locator = Locator::new(source_kind.source_code()); // Detect the current code style (lazily). let stylist = Stylist::from_tokens(&tokens, &locator); @@ -77,10 +73,10 @@ pub(crate) fn check( // Extract the `# noqa` and `# isort: skip` directives from the source. let directives = extract_directives(&tokens, Flags::empty(), &locator, &indexer); + let index = LineIndex::from_source_text(source_kind.source_code()); + // Generate checks. - let LinterResult { - data: diagnostics, .. - } = check_path( + let LinterResult { data, .. } = check_path( &document_path, package, &locator, @@ -94,16 +90,21 @@ pub(crate) fn check( TokenSource::Tokens(tokens), ); - diagnostics + let mut diagnostics = Diagnostics::default(); + + for (cell, diagnostic) in data .into_iter() - .map(|diagnostic| to_lsp_diagnostic(diagnostic, document, encoding)) - .collect() + .map(|diagnostic| to_lsp_diagnostic(diagnostic, &source_kind, &index, encoding)) + { + diagnostics.entry(cell).or_default().push(diagnostic); + } + + diagnostics } pub(crate) fn fixes_for_diagnostics( - document: &crate::edit::Document, - encoding: PositionEncoding, diagnostics: Vec, + encoding: PositionEncoding, ) -> crate::Result> { diagnostics .into_iter() @@ -116,16 +117,6 @@ pub(crate) fn fixes_for_diagnostics( serde_json::from_value(data).map_err(|err| { anyhow::anyhow!("failed to deserialize diagnostic data: {err}") })?; - let edits = associated_data - .fix - .edits() - .iter() - .map(|edit| lsp_types::TextEdit { - range: edit - .range() - .to_range(document.contents(), document.index(), encoding), - new_text: edit.content().unwrap_or_default().to_string(), - }); Ok(Some(DiagnosticFix { fixed_diagnostic, code: associated_data.code, @@ -133,7 +124,7 @@ pub(crate) fn fixes_for_diagnostics( .kind .suggestion .unwrap_or(associated_data.kind.name), - edits: edits.collect(), + edits: associated_data.edits, })) }) .filter_map(crate::Result::transpose) @@ -142,11 +133,15 @@ pub(crate) fn fixes_for_diagnostics( fn to_lsp_diagnostic( diagnostic: Diagnostic, - document: &crate::edit::Document, + source_kind: &SourceKind, + index: &LineIndex, encoding: PositionEncoding, -) -> lsp_types::Diagnostic { +) -> (usize, lsp_types::Diagnostic) { let Diagnostic { - kind, range, fix, .. + kind, + range: diagnostic_range, + fix, + .. } = diagnostic; let rule = kind.rule(); @@ -154,9 +149,35 @@ fn to_lsp_diagnostic( let data = fix.and_then(|fix| { fix.applies(Applicability::Unsafe) .then(|| { + let edits = fix + .edits() + .iter() + .map(|edit| lsp_types::TextEdit { + range: if let Some(notebook_index) = source_kind + .as_ipy_notebook() + .map(|notebook| notebook.index()) + { + edit.range() + .to_notebook_range( + source_kind.source_code(), + index, + notebook_index, + encoding, + ) + .expect( + "diagnostic range should convert to range inside notebook cell", + ) + .1 + } else { + edit.range() + .to_range(source_kind.source_code(), index, encoding) + }, + new_text: edit.content().unwrap_or_default().to_string(), + }) + .collect(); serde_json::to_value(&AssociatedDiagnosticData { kind: kind.clone(), - fix, + edits, code: rule.noqa_code().to_string(), }) .ok() @@ -166,21 +187,39 @@ fn to_lsp_diagnostic( let code = rule.noqa_code().to_string(); - lsp_types::Diagnostic { - range: range.to_range(document.contents(), document.index(), encoding), - severity: Some(severity(&code)), - tags: tags(&code), - code: Some(lsp_types::NumberOrString::String(code)), - code_description: rule.url().and_then(|url| { - Some(lsp_types::CodeDescription { - href: lsp_types::Url::parse(&url).ok()?, - }) - }), - source: Some(DIAGNOSTIC_NAME.into()), - message: kind.body, - related_information: None, - data, + let range; + let cell; + + if let Some(notebook_index) = source_kind + .as_ipy_notebook() + .map(|notebook| notebook.index()) + { + (cell, range) = diagnostic_range + .to_notebook_range(source_kind.source_code(), index, notebook_index, encoding) + .expect("diagnostic range should convert to range inside notebook cell"); + } else { + cell = usize::default(); + range = diagnostic_range.to_range(source_kind.source_code(), index, encoding); } + + ( + cell, + lsp_types::Diagnostic { + range, + severity: Some(severity(&code)), + tags: tags(&code), + code: Some(lsp_types::NumberOrString::String(code)), + code_description: rule.url().and_then(|url| { + Some(lsp_types::CodeDescription { + href: lsp_types::Url::parse(&url).ok()?, + }) + }), + source: Some(DIAGNOSTIC_NAME.into()), + message: kind.body, + related_information: None, + data, + }, + ) } fn severity(code: &str) -> lsp_types::DiagnosticSeverity { diff --git a/crates/ruff_server/src/server.rs b/crates/ruff_server/src/server.rs index 868bfd06792ba..c93f4a927898c 100644 --- a/crates/ruff_server/src/server.rs +++ b/crates/ruff_server/src/server.rs @@ -1,6 +1,7 @@ //! Scheduling, I/O, and API endpoints. use std::num::NonZeroUsize; +use std::path::PathBuf; use lsp_server as lsp; use lsp_types as types; @@ -69,26 +70,26 @@ impl Server { mut workspace_settings, } = AllSettings::from_value(init_params.initialization_options.unwrap_or_default()); - let mut workspace_for_uri = |uri| { + let mut workspace_for_path = |path: PathBuf| { let Some(workspace_settings) = workspace_settings.as_mut() else { - return (uri, ClientSettings::default()); + return (path, ClientSettings::default()); }; - let settings = workspace_settings.remove(&uri).unwrap_or_else(|| { - tracing::warn!("No workspace settings found for {uri}"); + let settings = workspace_settings.remove(&path).unwrap_or_else(|| { + tracing::warn!("No workspace settings found for {}", path.display()); ClientSettings::default() }); - (uri, settings) + (path, settings) }; let workspaces = init_params .workspace_folders .map(|folders| folders.into_iter().map(|folder| { - workspace_for_uri(folder.uri) + workspace_for_path(folder.uri.to_file_path().unwrap()) }).collect()) .or_else(|| { tracing::debug!("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_uri(uri)]) + Some(vec![workspace_for_path(uri.to_file_path().unwrap())]) }) .ok_or_else(|| { anyhow::anyhow!("Failed to get the current working directory while creating a default workspace.") @@ -260,8 +261,14 @@ impl Server { notebook_document_sync: Some(types::OneOf::Left(NotebookDocumentSyncOptions { save: Some(false), notebook_selector: [NotebookSelector::ByNotebook { - notebook: types::Notebook::String("jupyter-notebook".to_string()), - cells: None, // Is this correct? probably not. + notebook: types::Notebook::NotebookDocumentFilter( + types::NotebookDocumentFilter::ByType { + notebook_type: "jupyter-notebook".to_string(), + scheme: Some("file".to_string()), + pattern: None, + }, + ), + cells: None, }] .to_vec(), })), diff --git a/crates/ruff_server/src/server/api.rs b/crates/ruff_server/src/server/api.rs index 4d95ea3a71bd3..28320c744bb0f 100644 --- a/crates/ruff_server/src/server/api.rs +++ b/crates/ruff_server/src/server/api.rs @@ -90,6 +90,9 @@ pub(super) fn notification<'a>(notif: server::Notification) -> Task<'a> { notification::DidChangeNotebook::METHOD => { local_notification_task::(notif) } + notification::DidCloseNotebook::METHOD => { + local_notification_task::(notif) + } method => { tracing::warn!("Received notification {method} which does not have a handler."); return Task::nothing(); diff --git a/crates/ruff_server/src/server/api/diagnostics.rs b/crates/ruff_server/src/server/api/diagnostics.rs index ba08cdf776969..9d3c6cb6f63d0 100644 --- a/crates/ruff_server/src/server/api/diagnostics.rs +++ b/crates/ruff_server/src/server/api/diagnostics.rs @@ -1,17 +1,22 @@ -use crate::{server::client::Notifier, session::DocumentSnapshot}; +use std::ops::Deref; + +use anyhow::anyhow; + +use crate::{ + edit::Document, lint::Diagnostics, server::client::Notifier, session::DocumentSnapshot, +}; use super::LSPResult; -pub(super) fn generate_diagnostics(snapshot: &DocumentSnapshot) -> Vec { +pub(super) fn generate_diagnostics(snapshot: &DocumentSnapshot) -> Diagnostics { if snapshot.client_settings().lint() { crate::lint::check( snapshot.document(), - snapshot.url(), snapshot.settings().linter(), snapshot.encoding(), ) } else { - vec![] + Diagnostics::default() } } @@ -19,17 +24,25 @@ pub(super) fn publish_diagnostics_for_document( snapshot: &DocumentSnapshot, notifier: &Notifier, ) -> crate::server::Result<()> { - let diagnostics = generate_diagnostics(snapshot); - - notifier - .notify::( - lsp_types::PublishDiagnosticsParams { - uri: snapshot.url().clone(), - diagnostics, - version: Some(snapshot.document().version()), - }, - ) - .with_failure_code(lsp_server::ErrorCode::InternalError)?; + for (cell, diagnostics) in generate_diagnostics(snapshot) { + let uri = match snapshot.document().deref() { + Document::Notebook(notebook) => notebook + .cell_uri_by_index(cell) + .cloned() + .ok_or_else(|| anyhow!("cell index {cell} does not exist")) + .with_failure_code(lsp_server::ErrorCode::InternalError)?, + Document::Text(_) => snapshot.make_url(), + }; + notifier + .notify::( + lsp_types::PublishDiagnosticsParams { + uri, + diagnostics, + version: Some(snapshot.document().version()), + }, + ) + .with_failure_code(lsp_server::ErrorCode::InternalError)?; + } Ok(()) } @@ -41,7 +54,7 @@ pub(super) fn clear_diagnostics_for_document( notifier .notify::( lsp_types::PublishDiagnosticsParams { - uri: snapshot.url().clone(), + uri: snapshot.make_url().clone(), diagnostics: vec![], version: Some(snapshot.document().version()), }, diff --git a/crates/ruff_server/src/server/api/notifications.rs b/crates/ruff_server/src/server/api/notifications.rs index 68b0240a7f376..ade0c2fbd510f 100644 --- a/crates/ruff_server/src/server/api/notifications.rs +++ b/crates/ruff_server/src/server/api/notifications.rs @@ -1,20 +1,22 @@ mod cancel; mod did_change; mod did_change_configuration; +mod did_change_notebook; mod did_change_watched_files; mod did_change_workspace; mod did_close; +mod did_close_notebook; mod did_open; mod did_open_notebook; -mod did_change_notebook; use super::traits::{NotificationHandler, SyncNotificationHandler}; pub(super) use cancel::Cancel; pub(super) use did_change::DidChange; pub(super) use did_change_configuration::DidChangeConfiguration; +pub(super) use did_change_notebook::DidChangeNotebook; pub(super) use did_change_watched_files::DidChangeWatchedFiles; pub(super) use did_change_workspace::DidChangeWorkspace; pub(super) use did_close::DidClose; +pub(super) use did_close_notebook::DidCloseNotebook; pub(super) use did_open::DidOpen; pub(super) use did_open_notebook::DidOpenNotebook; -pub(super) use did_change_notebook::DidChangeNotebook; diff --git a/crates/ruff_server/src/server/api/notifications/cancel.rs b/crates/ruff_server/src/server/api/notifications/cancel.rs index 0011733569eb7..d9d011c3f7b32 100644 --- a/crates/ruff_server/src/server/api/notifications/cancel.rs +++ b/crates/ruff_server/src/server/api/notifications/cancel.rs @@ -11,7 +11,6 @@ impl super::NotificationHandler for Cancel { } impl super::SyncNotificationHandler for Cancel { - #[tracing::instrument(skip_all)] fn run( _session: &mut Session, _notifier: 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 79c1a07f8eac6..2b68dfc72a895 100644 --- a/crates/ruff_server/src/server/api/notifications/did_change.rs +++ b/crates/ruff_server/src/server/api/notifications/did_change.rs @@ -1,3 +1,4 @@ +use crate::edit::DocumentKey; use crate::server::api::diagnostics::publish_diagnostics_for_document; use crate::server::api::LSPResult; use crate::server::client::{Notifier, Requester}; @@ -13,7 +14,6 @@ impl super::NotificationHandler for DidChange { } impl super::SyncNotificationHandler for DidChange { - #[tracing::instrument(skip_all, fields(file=%uri))] fn run( session: &mut Session, notifier: Notifier, @@ -28,18 +28,22 @@ impl super::SyncNotificationHandler for DidChange { }: types::DidChangeTextDocumentParams, ) -> Result<()> { let encoding = session.encoding(); + let key = DocumentKey::from_url(&uri); let document = session - .document_controller(&uri) + .document_controller(&key) .with_failure_code(lsp_server::ErrorCode::InvalidParams)?; + let document = document + .make_mut() + .as_text_mut() + .expect("only python documents should be passed to didChange"); + if content_changes.is_empty() { - document.make_mut().update_version(new_version); + document.update_version(new_version); return Ok(()); } - document - .make_mut() - .apply_changes(content_changes, new_version, encoding); + document.apply_changes(content_changes, new_version, encoding); // Publish diagnostics if the client doesnt support pull diagnostics if !session.resolved_client_capabilities().pull_diagnostics { 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 2a45867a676f5..e03d7280f8aa3 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 @@ -1,5 +1,3 @@ -use crate::server::api::diagnostics::publish_diagnostics_for_document; -use crate::server::api::LSPResult; use crate::server::client::{Notifier, Requester}; use crate::server::Result; use crate::session::Session; @@ -13,22 +11,16 @@ impl super::NotificationHandler for DidChangeNotebook { } impl super::SyncNotificationHandler for DidChangeNotebook { - #[tracing::instrument(skip_all, fields(file=%uri))] fn run( session: &mut Session, - notifier: Notifier, + _notifier: Notifier, _requester: &mut Requester, types::DidChangeNotebookDocumentParams { - notebook_document: - types::VersionedNotebookDocumentIdentifier { - uri, - version: new_version, - }, - change, + notebook_document: types::VersionedNotebookDocumentIdentifier { uri, version }, + change: types::NotebookDocumentChangeEvent { cells, .. }, }: types::DidChangeNotebookDocumentParams, ) -> Result<()> { - tracing::info!("Notebook Changed: {}", uri); - show_err_msg!("Notebook Changed"); + // TODO(jane): impl Ok(()) } } 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 58eebe7292558..5502e5a425e24 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 { ) -> Result<()> { for change in ¶ms.changes { session - .reload_settings(&change.uri) + .reload_settings(change.uri.to_file_path().unwrap()) .with_failure_code(lsp_server::ErrorCode::InternalError)?; } 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 e5fe4768cca36..52da57a947c3c 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,6 +2,8 @@ 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; @@ -18,14 +20,23 @@ impl super::SyncNotificationHandler for DidChangeWorkspace { _requester: &mut Requester, params: types::DidChangeWorkspaceFoldersParams, ) -> Result<()> { - for new in params.event.added { + 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(&new.uri) + .open_workspace_folder(workspace_path) .with_failure_code(lsp_server::ErrorCode::InvalidParams)?; } - for removed in params.event.removed { + 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)?; session - .close_workspace_folder(&removed.uri) + .close_workspace_folder(&workspace_path) .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 ac394597e959c..aeee3bdafd07c 100644 --- a/crates/ruff_server/src/server/api/notifications/did_close.rs +++ b/crates/ruff_server/src/server/api/notifications/did_close.rs @@ -3,6 +3,8 @@ 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; @@ -13,7 +15,6 @@ impl super::NotificationHandler for DidClose { } impl super::SyncNotificationHandler for DidClose { - #[tracing::instrument(skip_all, fields(file=%uri))] fn run( session: &mut Session, notifier: Notifier, @@ -34,8 +35,13 @@ impl super::SyncNotificationHandler for DidClose { clear_diagnostics_for_document(&snapshot, ¬ifier)?; } + let document_path = uri + .to_file_path() + .map_err(|()| anyhow!("expected document URI {uri} to be a valid file path")) + .with_failure_code(ErrorCode::InvalidParams)?; + session - .close_document(&uri) + .close_document(&document_path) .with_failure_code(lsp_server::ErrorCode::InternalError) } } 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 new file mode 100644 index 0000000000000..e54c141be5ca4 --- /dev/null +++ b/crates/ruff_server/src/server/api/notifications/did_close_notebook.rs @@ -0,0 +1,37 @@ +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; + +pub(crate) struct DidCloseNotebook; + +impl super::NotificationHandler for DidCloseNotebook { + type NotificationType = notif::DidCloseNotebookDocument; +} + +impl super::SyncNotificationHandler for DidCloseNotebook { + fn run( + session: &mut Session, + _notifier: Notifier, + _requester: &mut Requester, + types::DidCloseNotebookDocumentParams { + notebook_document: types::NotebookDocumentIdentifier { uri }, + .. + }: types::DidCloseNotebookDocumentParams, + ) -> Result<()> { + // TODO(jane): de-register any diagnostics if we're in publish diagnostics mode + + let document_path = uri + .to_file_path() + .map_err(|()| anyhow!("expected notebook URI {uri} to be a valid file path")) + .with_failure_code(ErrorCode::InvalidParams)?; + + session + .close_document(&document_path) + .with_failure_code(ErrorCode::InternalError) + } +} 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 5098e5a6d58a6..810cd029a413e 100644 --- a/crates/ruff_server/src/server/api/notifications/did_open.rs +++ b/crates/ruff_server/src/server/api/notifications/did_open.rs @@ -3,6 +3,9 @@ use crate::server::api::LSPResult; 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; @@ -13,7 +16,6 @@ impl super::NotificationHandler for DidOpen { } impl super::SyncNotificationHandler for DidOpen { - #[tracing::instrument(skip_all, fields(file=%url))] fn run( session: &mut Session, notifier: Notifier, @@ -21,21 +23,28 @@ impl super::SyncNotificationHandler for DidOpen { types::DidOpenTextDocumentParams { text_document: types::TextDocumentItem { - uri: ref url, + ref uri, text, version, .. }, }: types::DidOpenTextDocumentParams, ) -> Result<()> { - session.open_document(url, text, version); + 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); // Publish diagnostics if the client doesnt support pull diagnostics if !session.resolved_client_capabilities().pull_diagnostics { let snapshot = session - .take_snapshot(url) + .take_snapshot(uri) .ok_or_else(|| { - anyhow::anyhow!("Unable to take snapshot for document with URL {url}") + anyhow::anyhow!("Unable to take snapshot for document with URL {uri}") }) .with_failure_code(lsp_server::ErrorCode::InternalError)?; publish_diagnostics_for_document(&snapshot, ¬ifier)?; 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 219eadd3de15a..864939ecd1407 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 @@ -1,8 +1,11 @@ +use crate::edit::NotebookDocument; use crate::server::api::diagnostics::publish_diagnostics_for_document; 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; @@ -13,7 +16,6 @@ impl super::NotificationHandler for DidOpenNotebook { } impl super::SyncNotificationHandler for DidOpenNotebook { - #[tracing::instrument(skip_all, fields(file=%url))] fn run( session: &mut Session, notifier: Notifier, @@ -21,17 +23,28 @@ impl super::SyncNotificationHandler for DidOpenNotebook { types::DidOpenNotebookDocumentParams { notebook_document: types::NotebookDocument { - uri: url, - notebook_type, + uri, version, - metadata, cells, + .. }, - cell_text_documents: text_items, + cell_text_documents, }: types::DidOpenNotebookDocumentParams, ) -> Result<()> { - tracing::info!("DidOpenNotebook: {}", url); - show_err_msg!("DidOpenNotebook"); + let notebook = NotebookDocument::new(version, cells, cell_text_documents); + + 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); + + // publish diagnostics + let snapshot = session + .take_snapshot(&uri) + .expect("snapshot should be available"); + publish_diagnostics_for_document(&snapshot, ¬ifier)?; 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 7f5c0b97709ce..25a68a5693f62 100644 --- a/crates/ruff_server/src/server/api/requests/code_action.rs +++ b/crates/ruff_server/src/server/api/requests/code_action.rs @@ -60,7 +60,7 @@ fn quick_fix( ) -> crate::Result> { let document = snapshot.document(); - let fixes = fixes_for_diagnostics(document, snapshot.encoding(), diagnostics)?; + let fixes = fixes_for_diagnostics(diagnostics, snapshot.encoding())?; fixes .into_iter() @@ -68,7 +68,7 @@ fn quick_fix( let mut tracker = WorkspaceEditTracker::new(snapshot.resolved_client_capabilities()); tracker.set_edits_for_document( - snapshot.url().clone(), + snapshot.make_url().clone(), document.version(), fix.edits, )?; @@ -79,7 +79,7 @@ fn quick_fix( edit: Some(tracker.into_workspace_edit()), diagnostics: Some(vec![fix.fixed_diagnostic.clone()]), data: Some( - serde_json::to_value(snapshot.url()).expect("document url to serialize"), + serde_json::to_value(snapshot.make_url()).expect("document url to serialize"), ), ..Default::default() })) @@ -97,17 +97,15 @@ fn fix_all(snapshot: &DocumentSnapshot) -> crate::Result { // The editor will request the edit in a `CodeActionsResolve` request ( None, - Some(serde_json::to_value(snapshot.url()).expect("document url to serialize")), + Some(serde_json::to_value(snapshot.make_url()).expect("document url to serialize")), ) } else { ( Some(resolve_edit_for_fix_all( document, snapshot.resolved_client_capabilities(), - snapshot.url(), snapshot.settings().linter(), snapshot.encoding(), - document.version(), )?), None, ) @@ -132,17 +130,15 @@ fn organize_imports(snapshot: &DocumentSnapshot) -> crate::Result crate::Result { let mut tracker = WorkspaceEditTracker::new(client_capabilities); - tracker.set_edits_for_document( - url.clone(), - version, - fix_all_edit(document, url, linter_settings, encoding)?, - )?; + tracker.set_fixes_for_document(document, fix_all_edit(document, linter_settings, encoding)?)?; Ok(tracker.into_workspace_edit()) } pub(super) fn fix_all_edit( - document: &crate::edit::Document, - document_url: &types::Url, + document: &DocumentQuery, linter_settings: &LinterSettings, encoding: PositionEncoding, -) -> crate::Result> { - crate::fix::fix_all(document, document_url, linter_settings, encoding) +) -> crate::Result { + crate::fix::fix_all(document, linter_settings, encoding) } pub(super) fn resolve_edit_for_organize_imports( - document: &crate::edit::Document, + document: &DocumentQuery, client_capabilities: &ResolvedClientCapabilities, - url: &types::Url, linter_settings: &ruff_linter::settings::LinterSettings, encoding: PositionEncoding, - version: DocumentVersion, ) -> crate::Result { let mut tracker = WorkspaceEditTracker::new(client_capabilities); - tracker.set_edits_for_document( - url.clone(), - version, - organize_imports_edit(document, url, linter_settings, encoding)?, + tracker.set_fixes_for_document( + document, + organize_imports_edit(document, linter_settings, encoding)?, )?; Ok(tracker.into_workspace_edit()) } pub(super) fn organize_imports_edit( - document: &crate::edit::Document, - document_url: &types::Url, + document: &DocumentQuery, linter_settings: &LinterSettings, encoding: PositionEncoding, -) -> crate::Result> { +) -> crate::Result { let mut linter_settings = linter_settings.clone(); linter_settings.rules = [ Rule::UnsortedImports, // I001 @@ -140,5 +126,5 @@ pub(super) fn organize_imports_edit( .into_iter() .collect(); - crate::fix::fix_all(document, document_url, &linter_settings, encoding) + crate::fix::fix_all(document, &linter_settings, encoding) } diff --git a/crates/ruff_server/src/server/api/requests/diagnostic.rs b/crates/ruff_server/src/server/api/requests/diagnostic.rs index c29ed90d5fd0b..aaea03ca585b1 100644 --- a/crates/ruff_server/src/server/api/requests/diagnostic.rs +++ b/crates/ruff_server/src/server/api/requests/diagnostic.rs @@ -26,7 +26,11 @@ impl super::BackgroundDocumentRequestHandler for DocumentDiagnostic { full_document_diagnostic_report: FullDocumentDiagnosticReport { // TODO(jane): eventually this will be important for caching diagnostic information. result_id: None, - items: generate_diagnostics(&snapshot), + items: generate_diagnostics(&snapshot) + .into_iter() + .next() + .map(|(_, diagnostics)| diagnostics) + .unwrap_or_default(), }, }), )) 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 6e2933a44a119..088725ada7d76 100644 --- a/crates/ruff_server/src/server/api/requests/execute_command.rs +++ b/crates/ruff_server/src/server/api/requests/execute_command.rs @@ -62,15 +62,14 @@ impl super::SyncRequestHandler for ExecuteCommand { .with_failure_code(ErrorCode::InternalError)?; match command { Command::FixAll => { - let edits = super::code_action_resolve::fix_all_edit( + let fixes = super::code_action_resolve::fix_all_edit( snapshot.document(), - snapshot.url(), snapshot.settings().linter(), snapshot.encoding(), ) .with_failure_code(ErrorCode::InternalError)?; edit_tracker - .set_edits_for_document(uri, version, edits) + .set_fixes_for_document(snapshot.document(), fixes) .with_failure_code(ErrorCode::InternalError)?; } Command::Format => { @@ -82,15 +81,14 @@ impl super::SyncRequestHandler for ExecuteCommand { } } Command::OrganizeImports => { - let edits = super::code_action_resolve::organize_imports_edit( + let fixes = super::code_action_resolve::organize_imports_edit( snapshot.document(), - snapshot.url(), snapshot.settings().linter(), snapshot.encoding(), ) .with_failure_code(ErrorCode::InternalError)?; edit_tracker - .set_edits_for_document(uri, version, edits) + .set_fixes_for_document(snapshot.document(), fixes) .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 dbfbc1af4e098..31c45d83a8141 100644 --- a/crates/ruff_server/src/server/api/requests/format.rs +++ b/crates/ruff_server/src/server/api/requests/format.rs @@ -24,7 +24,11 @@ impl super::BackgroundDocumentRequestHandler for Format { } pub(super) fn format_document(snapshot: &DocumentSnapshot) -> Result { - let doc = snapshot.document(); + // TODO: format by cell + let doc = snapshot + .document() + .as_text_or_cell_document() + .expect("should not be called as notebook"); let source = doc.contents(); let formatted = crate::format::format(doc, snapshot.settings().formatter()) .with_failure_code(lsp_server::ErrorCode::InternalError)?; 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 e01b4b042a1eb..5d8ef86a68008 100644 --- a/crates/ruff_server/src/server/api/requests/format_range.rs +++ b/crates/ruff_server/src/server/api/requests/format_range.rs @@ -17,7 +17,11 @@ impl super::BackgroundDocumentRequestHandler for FormatRange { _notifier: Notifier, params: types::DocumentRangeFormattingParams, ) -> Result { - let document = snapshot.document(); + // TODO: format by cell + let document = snapshot + .document() + .as_text_or_cell_document() + .expect("should not be called as notebook"); let text = document.contents(); let index = document.index(); let range = params.range.to_text_range(text, index, snapshot.encoding()); diff --git a/crates/ruff_server/src/server/api/requests/hover.rs b/crates/ruff_server/src/server/api/requests/hover.rs index 23a6c09c393b0..2e9d112dcad43 100644 --- a/crates/ruff_server/src/server/api/requests/hover.rs +++ b/crates/ruff_server/src/server/api/requests/hover.rs @@ -29,7 +29,11 @@ pub(crate) fn hover( snapshot: &DocumentSnapshot, position: &types::TextDocumentPositionParams, ) -> Option { - let document = snapshot.document(); + // Hover only operates on text documents or notebook cells + let document = snapshot + .document() + .as_text_or_cell_document() + .expect("should not be called as notebook"); let line_number: usize = position .position .line diff --git a/crates/ruff_server/src/session.rs b/crates/ruff_server/src/session.rs index 51a8909c033d6..dd685ed5c89d0 100644 --- a/crates/ruff_server/src/session.rs +++ b/crates/ruff_server/src/session.rs @@ -4,16 +4,18 @@ mod capabilities; mod settings; mod workspace; +use std::path::PathBuf; use std::sync::Arc; use anyhow::anyhow; use lsp_types::{ClientCapabilities, Url}; -use crate::edit::DocumentVersion; -use crate::PositionEncoding; +use crate::edit::{DocumentKey, DocumentVersion, NotebookDocument}; +use crate::{PositionEncoding, TextDocument}; pub(crate) use self::capabilities::ResolvedClientCapabilities; pub(crate) use self::settings::{AllSettings, ClientSettings}; +pub(crate) use self::workspace::DocumentQuery; /// The global state for the LSP pub(crate) struct Session { @@ -32,9 +34,8 @@ pub(crate) struct Session { pub(crate) struct DocumentSnapshot { resolved_client_capabilities: Arc, client_settings: settings::ResolvedClientSettings, - document_ref: workspace::DocumentRef, + document_ref: workspace::DocumentQuery, position_encoding: PositionEncoding, - url: Url, } impl Session { @@ -42,7 +43,7 @@ impl Session { client_capabilities: &ClientCapabilities, position_encoding: PositionEncoding, global_settings: ClientSettings, - workspaces: Vec<(Url, ClientSettings)>, + workspaces: Vec<(PathBuf, ClientSettings)>, ) -> crate::Result { Ok(Self { position_encoding, @@ -55,45 +56,50 @@ impl Session { } pub(crate) fn take_snapshot(&self, url: &Url) -> Option { + let key = DocumentKey::from_url(url); + tracing::info!("key: {key}"); Some(DocumentSnapshot { resolved_client_capabilities: self.resolved_client_capabilities.clone(), - client_settings: self.workspaces.client_settings(url, &self.global_settings), - document_ref: self.workspaces.snapshot(url)?, + client_settings: self.workspaces.client_settings(&key, &self.global_settings), + document_ref: self.workspaces.make_document_ref(&key)?, position_encoding: self.position_encoding, - url: url.clone(), }) } - pub(crate) fn open_document(&mut self, url: &Url, contents: String, version: DocumentVersion) { - self.workspaces.open(url, contents, version); + pub(crate) fn open_notebook_document(&mut self, path: PathBuf, document: NotebookDocument) { + self.workspaces.open_notebook_document(path, document) } - pub(crate) fn close_document(&mut self, url: &Url) -> crate::Result<()> { - self.workspaces.close(url)?; + pub(crate) fn open_text_document(&mut self, path: PathBuf, document: TextDocument) { + self.workspaces.open_text_document(path, document); + } + + pub(crate) fn close_document(&mut self, path: &PathBuf) -> crate::Result<()> { + self.workspaces.close_document(path)?; Ok(()) } pub(crate) fn document_controller( &mut self, - url: &Url, + key: &DocumentKey, ) -> crate::Result<&mut workspace::DocumentController> { self.workspaces - .controller(url) - .ok_or_else(|| anyhow!("Tried to open unavailable document `{url}`")) + .controller(key) + .ok_or_else(|| anyhow!("Tried to open unavailable document `{key}`")) } - pub(crate) fn reload_settings(&mut self, url: &Url) -> crate::Result<()> { - self.workspaces.reload_settings(url) + pub(crate) fn reload_settings(&mut self, changed_path: PathBuf) -> crate::Result<()> { + self.workspaces.reload_settings(changed_path) } - pub(crate) fn open_workspace_folder(&mut self, url: &Url) -> crate::Result<()> { + pub(crate) fn open_workspace_folder(&mut self, path: PathBuf) -> crate::Result<()> { self.workspaces - .open_workspace_folder(url, &self.global_settings)?; + .open_workspace_folder(path, &self.global_settings)?; Ok(()) } - pub(crate) fn close_workspace_folder(&mut self, url: &Url) -> crate::Result<()> { - self.workspaces.close_workspace_folder(url)?; + pub(crate) fn close_workspace_folder(&mut self, path: &PathBuf) -> crate::Result<()> { + self.workspaces.close_workspace_folder(path)?; Ok(()) } @@ -119,7 +125,7 @@ impl DocumentSnapshot { &self.client_settings } - pub(crate) fn document(&self) -> &workspace::DocumentRef { + pub(crate) fn document(&self) -> &workspace::DocumentQuery { &self.document_ref } @@ -127,7 +133,7 @@ impl DocumentSnapshot { self.position_encoding } - pub(crate) fn url(&self) -> &Url { - &self.url + pub(crate) fn make_url(&self) -> lsp_types::Url { + self.document().key().clone().to_url() } } diff --git a/crates/ruff_server/src/session/settings.rs b/crates/ruff_server/src/session/settings.rs index 0786d29be2974..55eff13742d84 100644 --- a/crates/ruff_server/src/session/settings.rs +++ b/crates/ruff_server/src/session/settings.rs @@ -6,7 +6,7 @@ use rustc_hash::FxHashMap; use serde::Deserialize; /// 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,7 +170,12 @@ impl AllSettings { workspace_settings: workspace_settings.map(|workspace_settings| { workspace_settings .into_iter() - .map(|settings| (settings.workspace, settings.settings)) + .map(|settings| { + ( + settings.workspace.to_file_path().unwrap(), + settings.settings, + ) + }) .collect() }), } @@ -549,12 +554,12 @@ mod tests { global_settings, workspace_settings, } = AllSettings::from_init_options(options); - let url = Url::parse("file:///Users/test/projects/pandas").expect("url should parse"); + let path = PathBuf::from_str("/Users/test/projects/pandas").expect("path should be valid"); let workspace_settings = workspace_settings.expect("workspace settings should exist"); assert_eq!( ResolvedClientSettings::with_workspace( workspace_settings - .get(&url) + .get(&path) .expect("workspace setting should exist"), &global_settings ), @@ -580,11 +585,11 @@ mod tests { } } ); - let url = Url::parse("file:///Users/test/projects/scipy").expect("url should parse"); + let path = PathBuf::from_str("/Users/test/projects/scipy").expect("path should be valid"); assert_eq!( ResolvedClientSettings::with_workspace( workspace_settings - .get(&url) + .get(&path) .expect("workspace setting should exist"), &global_settings ), diff --git a/crates/ruff_server/src/session/workspace.rs b/crates/ruff_server/src/session/workspace.rs index cb8332c730651..fc0564a871c99 100644 --- a/crates/ruff_server/src/session/workspace.rs +++ b/crates/ruff_server/src/session/workspace.rs @@ -1,19 +1,14 @@ 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 std::{collections::BTreeMap, ops::Deref, path::PathBuf, sync::Arc}; -use self::ruff_settings::RuffSettingsIndex; +use crate::{ + edit::{Document, DocumentKey, NotebookDocument}, + TextDocument, +}; use super::{ - settings::{self, ResolvedClientSettings, ResolvedEditorSettings}, + settings::{self, ResolvedClientSettings}, ClientSettings, }; @@ -21,17 +16,24 @@ mod ruff_settings; pub(crate) use ruff_settings::RuffSettings; -#[derive(Default)] -pub(crate) struct Workspaces(BTreeMap); +type DocumentIndex = FxHashMap; +type NotebookCellIndex = FxHashMap; +type SettingsIndex = BTreeMap; -pub(crate) struct Workspace { - open_documents: OpenDocuments, - settings: ResolvedClientSettings, +#[derive(Default)] +pub(crate) struct Workspaces { + /// Maps all document file paths to the associated document controller + document_index: DocumentIndex, + /// Maps opaque cell URLs to a notebook path + notebook_cell_index: NotebookCellIndex, + /// Maps a workspace folder root to its settings. + settings_index: SettingsIndex, } -pub(crate) struct OpenDocuments { - documents: FxHashMap, - settings_index: ruff_settings::RuffSettingsIndex, +/// Settings associated with a workspace. +struct WorkspaceSettings { + client_settings: ResolvedClientSettings, + workspace_settings_index: ruff_settings::RuffSettingsIndex, } /// A mutable handler to an underlying document. @@ -43,201 +45,209 @@ pub(crate) struct DocumentController { /// A read-only reference to a document. #[derive(Clone)] -pub(crate) struct DocumentRef { +pub(crate) struct DocumentQuery { + key: DocumentKey, + file_path: PathBuf, document: Arc, settings: Arc, } impl Workspaces { pub(super) fn new( - workspaces: Vec<(Url, ClientSettings)>, + workspaces: Vec<(PathBuf, ClientSettings)>, global_settings: &ClientSettings, ) -> crate::Result { - Ok(Self( - workspaces - .into_iter() - .map(|(url, workspace_settings)| { - Workspace::new(&url, &workspace_settings, global_settings) - }) - .collect::>()?, - )) + let mut settings_index = BTreeMap::new(); + for (path, workspace_settings) in workspaces { + Self::register_workspace_settings( + &mut settings_index, + path, + Some(workspace_settings), + global_settings, + ); + } + + Ok(Self { + document_index: FxHashMap::default(), + notebook_cell_index: FxHashMap::default(), + settings_index, + }) } pub(super) fn open_workspace_folder( &mut self, - folder_url: &Url, + path: PathBuf, global_settings: &ClientSettings, ) -> 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(), global_settings)?; - self.0.insert(path, workspace); + // TODO(jane): Find a way for workspace client settings to be added or changed dynamically. + Self::register_workspace_settings(&mut self.settings_index, path, None, global_settings); 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()))?; + fn register_workspace_settings( + settings_index: &mut SettingsIndex, + workspace_path: PathBuf, + workspace_settings: Option, + global_settings: &ClientSettings, + ) { + let client_settings = if let Some(workspace_settings) = workspace_settings { + ResolvedClientSettings::with_workspace(&workspace_settings, global_settings) + } else { + ResolvedClientSettings::global(global_settings) + }; + let workspace_settings_index = ruff_settings::RuffSettingsIndex::new( + &workspace_path, + client_settings.editor_settings(), + ); + + settings_index.insert( + workspace_path, + WorkspaceSettings { + client_settings, + workspace_settings_index, + }, + ); + } + + pub(super) fn close_workspace_folder(&mut self, workspace_path: &PathBuf) -> crate::Result<()> { + self.settings_index.remove(workspace_path).ok_or_else(|| { + anyhow!( + "Tried to remove non-existent folder {}", + workspace_path.display() + ) + })?; + // O(n) complexity, which isn't ideal... but this is an uncommon operation. + self.document_index + .retain(|path, _| !path.starts_with(workspace_path)); + self.notebook_cell_index + .retain(|_, path| !path.starts_with(workspace_path)); Ok(()) } - pub(super) fn snapshot(&self, document_url: &Url) -> Option { - self.workspace_for_url(document_url)? - .open_documents - .snapshot(document_url) + pub(super) fn make_document_ref(&self, key: &DocumentKey) -> Option { + let path = self.path_for_key(key)?; + tracing::info!("Document path: {}", path.display()); + let document_settings = self + .settings_for_path(path)? + .workspace_settings_index + .get(path); + + let controller = self.document_index.get(path)?; + Some(controller.make_ref(key.clone(), path.clone(), document_settings)) } - 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 controller(&mut self, key: &DocumentKey) -> Option<&mut DocumentController> { + let path = self.path_for_key(&key)?.clone(); + self.document_index.get_mut(&path) } - 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_settings(root); + pub(super) fn reload_settings(&mut self, changed_path: PathBuf) -> crate::Result<()> { + for (root, settings) in self + .settings_index + .iter_mut() + .filter(|(path, _)| path.starts_with(&changed_path)) + { + settings.workspace_settings_index = ruff_settings::RuffSettingsIndex::new( + root, + settings.client_settings.editor_settings(), + ); + } 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 open_text_document(&mut self, path: PathBuf, document: TextDocument) { + self.document_index + .insert(path, DocumentController::new_text(document)); + } + + pub(super) fn open_notebook_document(&mut self, path: PathBuf, document: NotebookDocument) { + for url in document.urls() { + self.notebook_cell_index.insert(url.clone(), path.clone()); } + self.document_index + .insert(path, DocumentController::new_notebook(document)); } - 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 close_document(&mut self, path: &PathBuf) -> crate::Result<()> { + let controller = self.document_index.remove(path).ok_or_else(|| { + anyhow!( + "tried to close document that didn't exist at {}", + path.display() + ) + })?; + if let Some(notebook) = controller.as_notebook() { + for url in notebook.urls() { + self.notebook_cell_index.remove(url).ok_or_else(|| { + anyhow!("tried to de-register notebook cell with URL {url} that didn't exist") + })?; + } + } + Ok(()) } pub(super) fn client_settings( &self, - url: &Url, + key: &DocumentKey, 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| workspace.settings.clone(), - ) - } - - fn workspace_for_url(&self, url: &Url) -> Option<&Workspace> { - Some(self.entry_for_url(url)?.1) + let Some(path) = self.path_for_key(key) else { + return ResolvedClientSettings::global(global_settings); + }; + let Some(WorkspaceSettings { + client_settings, .. + }) = self.settings_for_path(path) + else { + return ResolvedClientSettings::global(global_settings); + }; + client_settings.clone() } - fn workspace_for_url_mut(&mut self, url: &Url) -> Option<&mut Workspace> { - Some(self.entry_for_url_mut(url)?.1) + fn path_for_key<'a>(&'a self, key: &'a DocumentKey) -> Option<&'a PathBuf> { + match key { + DocumentKey::File(path) => Some(path), + DocumentKey::Cell(uri) => self.notebook_cell_index.get(uri), + } } - fn entry_for_url(&self, url: &Url) -> Option<(&Path, &Workspace)> { - let path = url.to_file_path().ok()?; - self.0 - .range(..path) + fn settings_for_path(&self, path: &PathBuf) -> Option<&WorkspaceSettings> { + self.settings_index + .range(..path.clone()) .next_back() - .map(|(path, workspace)| (path.as_path(), workspace)) + .map(|(_, settings)| settings) } - 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) + fn settings_for_path_mut(&mut self, path: &PathBuf) -> Option<&mut WorkspaceSettings> { + self.settings_index + .range_mut(..path.clone()) .next_back() - .map(|(path, workspace)| (path.as_path(), workspace)) - } -} - -impl Workspace { - pub(crate) fn new( - root: &Url, - workspace_settings: &ClientSettings, - global_settings: &ClientSettings, - ) -> crate::Result<(PathBuf, Self)> { - let path = root - .to_file_path() - .map_err(|()| anyhow!("workspace URL was not a file path!"))?; - - let settings = ResolvedClientSettings::with_workspace(workspace_settings, global_settings); - - let workspace = Self { - open_documents: OpenDocuments::new(&path, settings.editor_settings()), - settings, - }; - - Ok((path, workspace)) - } - - fn reload_settings(&mut self, root: &Path) { - self.open_documents - .reload_settings(root, self.settings.editor_settings()); + .map(|(_, settings)| settings) } } -impl OpenDocuments { - fn new(path: &Path, editor_settings: &ResolvedEditorSettings) -> Self { +impl DocumentController { + fn new_text(document: TextDocument) -> Self { Self { - documents: FxHashMap::default(), - settings_index: RuffSettingsIndex::new(path, editor_settings), + document: Arc::new(Document::Text(document)), } } - fn snapshot(&self, url: &Url) -> Option { - let path = url - .to_file_path() - .expect("document URL should convert to file path: {url}"); - let document_settings = self.settings_index.get(&path); - Some(self.documents.get(url)?.make_ref(document_settings)) - } - - 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(()) - } - - fn reload_settings(&mut self, root: &Path, editor_settings: &ResolvedEditorSettings) { - self.settings_index = RuffSettingsIndex::new(root, editor_settings); - } -} - -impl DocumentController { - fn new(contents: String, version: DocumentVersion) -> Self { + fn new_notebook(document: NotebookDocument) -> Self { Self { - document: Arc::new(Document::new(contents, version)), + document: Arc::new(Document::Notebook(document)), } } - pub(crate) fn make_ref(&self, document_settings: Arc) -> DocumentRef { - DocumentRef { + fn make_ref( + &self, + key: DocumentKey, + // TODO(jane): I don't like that we have to specify a document key and concrete file + // path separately here... + file_path: PathBuf, + document_settings: Arc, + ) -> DocumentQuery { + DocumentQuery { + key, + file_path, document: self.document.clone(), settings: document_settings, } @@ -255,15 +265,30 @@ impl Deref for DocumentController { } } -impl Deref for DocumentRef { +impl Deref for DocumentQuery { type Target = Document; fn deref(&self) -> &Self::Target { &self.document } } -impl DocumentRef { +impl DocumentQuery { + pub(crate) fn key(&self) -> &DocumentKey { + &self.key + } + pub(crate) fn settings(&self) -> &RuffSettings { &self.settings } + + pub(crate) fn file_path(&self) -> &PathBuf { + &self.file_path + } + + pub(crate) fn as_text_or_cell_document(&self) -> Option<&TextDocument> { + match &self.key { + DocumentKey::File(_) => self.document.as_text(), + DocumentKey::Cell(url) => self.document.as_notebook_cell(url), + } + } } diff --git a/crates/ruff_server/tests/document.rs b/crates/ruff_server/tests/document.rs index 0c4da4e053aca..39791ff57e032 100644 --- a/crates/ruff_server/tests/document.rs +++ b/crates/ruff_server/tests/document.rs @@ -1,11 +1,11 @@ const PANDAS_HTML_SRC: &str = include_str!("../resources/test/fixtures/pandas_html.py"); use lsp_types::{Position, Range, TextDocumentContentChangeEvent}; -use ruff_server::{Document, PositionEncoding}; +use ruff_server::{PositionEncoding, TextDocument}; #[test] fn delete_lines_pandas_html() { - let mut document = Document::new(PANDAS_HTML_SRC.to_string(), 1); + let mut document = TextDocument::new(PANDAS_HTML_SRC.to_string(), 1); let changes = vec![ TextDocumentContentChangeEvent { diff --git a/crates/ruff_source_file/src/locator.rs b/crates/ruff_source_file/src/locator.rs index 30bd59d830fe4..792f31e247f44 100644 --- a/crates/ruff_source_file/src/locator.rs +++ b/crates/ruff_source_file/src/locator.rs @@ -23,13 +23,6 @@ impl<'a> Locator<'a> { } } - pub const fn with_index(contents: &'a str, index: LineIndex) -> Self { - Self { - contents, - index: OnceCell::with_value(index), - } - } - #[deprecated( note = "This is expensive, avoid using outside of the diagnostic phase. Prefer the other `Locator` methods instead." )]