ruff server: Add support for documents not exist on disk (#11588)

Co-authored-by: T-256 <[email protected]>
Co-authored-by: Micha Reiser <[email protected]>

Author: 3 people

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,29 +35,26 @@ 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,
         }
     }
 }
 
 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<lsp_types::TextDocumentEdit>),
-    Changes(HashMap<lsp_types::Url, Vec<lsp_types::TextEdit>>),
+    Changes(HashMap<Url, Vec<lsp_types::TextEdit>>),
 }
 
 impl From<PositionEncoding> 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<lsp_types::TextEdit>,
     ) -> crate::Result<()> {

crates/ruff_server/src/fix.rs

@@ -26,33 +26,37 @@ pub(crate) fn fix_all(
     linter_settings: &LinterSettings,
     encoding: PositionEncoding,
 ) -> crate::Result<Fixes> {
-    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,

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<lsp_types::Url, Vec<lsp_types::Diagnostic>>;
 
 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(),

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,
         })
     }

crates/ruff_server/src/server/api.rs

@@ -122,7 +122,7 @@ fn background_request_task<'a, R: traits::BackgroundDocumentRequestHandler>(
     let (id, params) = cast_request::<R>(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(&params)) else {
+        let Some(snapshot) = session.take_snapshot(R::document_url(&params).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::<N>(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(&params)) else {
+        let Some(snapshot) = session.take_snapshot(N::document_url(&params).into_owned()) else {
             return Box::new(|_, _| {});
         };
         Box::new(move |notifier, _| {

crates/ruff_server/src/server/api/notifications/did_change.rs

@@ -27,17 +27,15 @@ 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)
             .with_failure_code(ErrorCode::InternalError)?;
 
         // 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, &notifier)?;
         }
 

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, &notifier)?;
 

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 &params.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, &notifier)?;
                 }
@@ -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, &notifier)?;
             }