[red-knot] Support files outside of any workspace (#13041)

## Summary

This PR adds basic support for files outside of any workspace in the red
knot server.

This also limits the red knot server to only work in a single workspace.
The server will not start if there are multiple workspaces.

## Test Plan

https://github.com/user-attachments/assets/de601387-0ad5-433c-9d2c-7b6ae5137654
This commit is contained in:
Dhruv Manilawala 2024-08-23 12:21:48 +05:30 committed by GitHub
parent 4f6accb5c6
commit c73a7bb929
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 56 additions and 39 deletions

View file

@ -99,6 +99,11 @@ impl Server {
anyhow::anyhow!("Failed to get the current working directory while creating a default workspace.") anyhow::anyhow!("Failed to get the current working directory while creating a default workspace.")
})?; })?;
if workspaces.len() > 1 {
// TODO(dhruvmanila): Support multi-root workspaces
anyhow::bail!("Multi-root workspaces are not supported yet");
}
Ok(Self { Ok(Self {
connection, connection,
worker_threads, worker_threads,

View file

@ -7,7 +7,6 @@ mod requests;
mod traits; mod traits;
use notifications as notification; use notifications as notification;
use red_knot_workspace::db::RootDatabase;
use requests as request; use requests as request;
use self::traits::{NotificationHandler, RequestHandler}; use self::traits::{NotificationHandler, RequestHandler};
@ -85,9 +84,10 @@ fn background_request_task<'a, R: traits::BackgroundDocumentRequestHandler>(
let Ok(path) = url_to_system_path(&url) else { let Ok(path) = url_to_system_path(&url) else {
return Box::new(|_, _| {}); return Box::new(|_, _| {});
}; };
let db = session let db = match session.workspace_db_for_path(path.as_std_path()) {
.workspace_db_for_path(path.as_std_path()) Some(db) => db.snapshot(),
.map(RootDatabase::snapshot); None => session.default_workspace_db().snapshot(),
};
let Some(snapshot) = session.take_snapshot(url) else { let Some(snapshot) = session.take_snapshot(url) else {
return Box::new(|_, _| {}); return Box::new(|_, _| {});

View file

@ -2,8 +2,6 @@ use lsp_server::ErrorCode;
use lsp_types::notification::DidCloseTextDocument; use lsp_types::notification::DidCloseTextDocument;
use lsp_types::DidCloseTextDocumentParams; use lsp_types::DidCloseTextDocumentParams;
use ruff_db::files::File;
use crate::server::api::diagnostics::clear_diagnostics; use crate::server::api::diagnostics::clear_diagnostics;
use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler}; use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler};
use crate::server::api::LSPResult; use crate::server::api::LSPResult;
@ -25,7 +23,7 @@ impl SyncNotificationHandler for DidCloseTextDocumentHandler {
_requester: &mut Requester, _requester: &mut Requester,
params: DidCloseTextDocumentParams, params: DidCloseTextDocumentParams,
) -> Result<()> { ) -> Result<()> {
let Ok(path) = url_to_system_path(&params.text_document.uri) else { let Ok(_path) = url_to_system_path(&params.text_document.uri) else {
return Ok(()); return Ok(());
}; };
@ -34,10 +32,6 @@ impl SyncNotificationHandler for DidCloseTextDocumentHandler {
.close_document(&key) .close_document(&key)
.with_failure_code(ErrorCode::InternalError)?; .with_failure_code(ErrorCode::InternalError)?;
if let Some(db) = session.workspace_db_for_path_mut(path.as_std_path()) {
File::sync_path(db, &path);
}
clear_diagnostics(key.url(), &notifier)?; clear_diagnostics(key.url(), &notifier)?;
Ok(()) Ok(())

View file

@ -1,8 +1,6 @@
use lsp_types::notification::DidCloseNotebookDocument; use lsp_types::notification::DidCloseNotebookDocument;
use lsp_types::DidCloseNotebookDocumentParams; use lsp_types::DidCloseNotebookDocumentParams;
use ruff_db::files::File;
use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler}; use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler};
use crate::server::api::LSPResult; use crate::server::api::LSPResult;
use crate::server::client::{Notifier, Requester}; use crate::server::client::{Notifier, Requester};
@ -23,7 +21,7 @@ impl SyncNotificationHandler for DidCloseNotebookHandler {
_requester: &mut Requester, _requester: &mut Requester,
params: DidCloseNotebookDocumentParams, params: DidCloseNotebookDocumentParams,
) -> Result<()> { ) -> Result<()> {
let Ok(path) = url_to_system_path(&params.notebook_document.uri) else { let Ok(_path) = url_to_system_path(&params.notebook_document.uri) else {
return Ok(()); return Ok(());
}; };
@ -32,10 +30,6 @@ impl SyncNotificationHandler for DidCloseNotebookHandler {
.close_document(&key) .close_document(&key)
.with_failure_code(lsp_server::ErrorCode::InternalError)?; .with_failure_code(lsp_server::ErrorCode::InternalError)?;
if let Some(db) = session.workspace_db_for_path_mut(path.as_std_path()) {
File::sync_path(db, &path);
}
Ok(()) Ok(())
} }
} }

View file

@ -1,7 +1,7 @@
use lsp_types::notification::DidOpenTextDocument; use lsp_types::notification::DidOpenTextDocument;
use lsp_types::DidOpenTextDocumentParams; use lsp_types::DidOpenTextDocumentParams;
use ruff_db::files::system_path_to_file; use red_knot_workspace::watch::ChangeEvent;
use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler}; use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler};
use crate::server::client::{Notifier, Requester}; use crate::server::client::{Notifier, Requester};
@ -30,11 +30,11 @@ impl SyncNotificationHandler for DidOpenTextDocumentHandler {
let document = TextDocument::new(params.text_document.text, params.text_document.version); let document = TextDocument::new(params.text_document.text, params.text_document.version);
session.open_text_document(params.text_document.uri, document); session.open_text_document(params.text_document.uri, document);
if let Some(db) = session.workspace_db_for_path_mut(path.as_std_path()) { let db = match session.workspace_db_for_path_mut(path.as_std_path()) {
// TODO(dhruvmanila): Store the `file` in `DocumentController` Some(db) => db,
let file = system_path_to_file(db, &path).unwrap(); None => session.default_workspace_db_mut(),
file.sync(db); };
} db.apply_changes(vec![ChangeEvent::Opened(path)], None);
// TODO(dhruvmanila): Publish diagnostics if the client doesn't support pull diagnostics // TODO(dhruvmanila): Publish diagnostics if the client doesn't support pull diagnostics

View file

@ -2,7 +2,7 @@ use lsp_server::ErrorCode;
use lsp_types::notification::DidOpenNotebookDocument; use lsp_types::notification::DidOpenNotebookDocument;
use lsp_types::DidOpenNotebookDocumentParams; use lsp_types::DidOpenNotebookDocumentParams;
use ruff_db::files::system_path_to_file; use red_knot_workspace::watch::ChangeEvent;
use crate::edit::NotebookDocument; use crate::edit::NotebookDocument;
use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler}; use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler};
@ -38,11 +38,11 @@ impl SyncNotificationHandler for DidOpenNotebookHandler {
.with_failure_code(ErrorCode::InternalError)?; .with_failure_code(ErrorCode::InternalError)?;
session.open_notebook_document(params.notebook_document.uri.clone(), notebook); session.open_notebook_document(params.notebook_document.uri.clone(), notebook);
if let Some(db) = session.workspace_db_for_path_mut(path.as_std_path()) { let db = match session.workspace_db_for_path_mut(path.as_std_path()) {
// TODO(dhruvmanila): Store the `file` in `DocumentController` Some(db) => db,
let file = system_path_to_file(db, &path).unwrap(); None => session.default_workspace_db_mut(),
file.sync(db); };
} db.apply_changes(vec![ChangeEvent::Opened(path)], None);
// TODO(dhruvmanila): Publish diagnostics if the client doesn't support pull diagnostics // TODO(dhruvmanila): Publish diagnostics if the client doesn't support pull diagnostics

View file

@ -26,13 +26,11 @@ impl BackgroundDocumentRequestHandler for DocumentDiagnosticRequestHandler {
fn run_with_snapshot( fn run_with_snapshot(
snapshot: DocumentSnapshot, snapshot: DocumentSnapshot,
db: Option<RootDatabase>, db: RootDatabase,
_notifier: Notifier, _notifier: Notifier,
_params: DocumentDiagnosticParams, _params: DocumentDiagnosticParams,
) -> Result<DocumentDiagnosticReportResult> { ) -> Result<DocumentDiagnosticReportResult> {
let diagnostics = db let diagnostics = compute_diagnostics(&snapshot, &db);
.map(|db| compute_diagnostics(&snapshot, &db))
.unwrap_or_default();
Ok(DocumentDiagnosticReportResult::Report( Ok(DocumentDiagnosticReportResult::Report(
DocumentDiagnosticReport::Full(RelatedFullDocumentDiagnosticReport { DocumentDiagnosticReport::Full(RelatedFullDocumentDiagnosticReport {
@ -66,11 +64,11 @@ fn to_lsp_diagnostic(message: &str) -> Diagnostic {
let (range, message) = match words.as_slice() { let (range, message) = match words.as_slice() {
[_filename, line, column, message] => { [_filename, line, column, message] => {
let line = line.parse::<u32>().unwrap_or_default(); let line = line.parse::<u32>().unwrap_or_default().saturating_sub(1);
let column = column.parse::<u32>().unwrap_or_default(); let column = column.parse::<u32>().unwrap_or_default();
( (
Range::new( Range::new(
Position::new(line.saturating_sub(1), column.saturating_sub(1)), Position::new(line, column.saturating_sub(1)),
Position::new(line, column), Position::new(line, column),
), ),
message.trim(), message.trim(),

View file

@ -34,7 +34,7 @@ pub(super) trait BackgroundDocumentRequestHandler: RequestHandler {
fn run_with_snapshot( fn run_with_snapshot(
snapshot: DocumentSnapshot, snapshot: DocumentSnapshot,
db: Option<RootDatabase>, db: RootDatabase,
notifier: Notifier, notifier: Notifier,
params: <<Self as RequestHandler>::RequestType as Request>::Params, params: <<Self as RequestHandler>::RequestType as Request>::Params,
) -> super::Result<<<Self as RequestHandler>::RequestType as Request>::Result>; ) -> super::Result<<<Self as RequestHandler>::RequestType as Request>::Result>;

View file

@ -82,6 +82,12 @@ impl Session {
}) })
} }
// TODO(dhruvmanila): Ideally, we should have a single method for `workspace_db_for_path_mut`
// and `default_workspace_db_mut` but the borrow checker doesn't allow that.
// https://github.com/astral-sh/ruff/pull/13041#discussion_r1726725437
/// Returns a reference to the workspace [`RootDatabase`] corresponding to the given path, if
/// any.
pub(crate) fn workspace_db_for_path(&self, path: impl AsRef<Path>) -> Option<&RootDatabase> { pub(crate) fn workspace_db_for_path(&self, path: impl AsRef<Path>) -> Option<&RootDatabase> {
self.workspaces self.workspaces
.range(..=path.as_ref().to_path_buf()) .range(..=path.as_ref().to_path_buf())
@ -89,6 +95,8 @@ impl Session {
.map(|(_, db)| db) .map(|(_, db)| db)
} }
/// Returns a mutable reference to the workspace [`RootDatabase`] corresponding to the given
/// path, if any.
pub(crate) fn workspace_db_for_path_mut( pub(crate) fn workspace_db_for_path_mut(
&mut self, &mut self,
path: impl AsRef<Path>, path: impl AsRef<Path>,
@ -99,6 +107,19 @@ impl Session {
.map(|(_, db)| db) .map(|(_, db)| db)
} }
/// Returns a reference to the default workspace [`RootDatabase`]. The default workspace is the
/// minimum root path in the workspace map.
pub(crate) fn default_workspace_db(&self) -> &RootDatabase {
// SAFETY: Currently, red knot only support a single workspace.
self.workspaces.values().next().unwrap()
}
/// Returns a mutable reference to the default workspace [`RootDatabase`].
pub(crate) fn default_workspace_db_mut(&mut self) -> &mut RootDatabase {
// SAFETY: Currently, red knot only support a single workspace.
self.workspaces.values_mut().next().unwrap()
}
pub fn key_from_url(&self, url: Url) -> DocumentKey { pub fn key_from_url(&self, url: Url) -> DocumentKey {
self.index().key_from_url(url) self.index().key_from_url(url)
} }

View file

@ -72,7 +72,8 @@ impl RootDatabase {
} }
match change { match change {
watch::ChangeEvent::Changed { path, kind: _ } => sync_path(self, &path), watch::ChangeEvent::Changed { path, kind: _ }
| watch::ChangeEvent::Opened(path) => sync_path(self, &path),
watch::ChangeEvent::Created { kind, path } => { watch::ChangeEvent::Created { kind, path } => {
match kind { match kind {

View file

@ -20,6 +20,9 @@ mod workspace_watcher;
/// event instead of emitting an event for each file or subdirectory in that path. /// event instead of emitting an event for each file or subdirectory in that path.
#[derive(Debug, PartialEq, Eq)] #[derive(Debug, PartialEq, Eq)]
pub enum ChangeEvent { pub enum ChangeEvent {
/// The file corresponding to the given path was opened in an editor.
Opened(SystemPathBuf),
/// A new path was created /// A new path was created
Created { Created {
path: SystemPathBuf, path: SystemPathBuf,
@ -52,7 +55,8 @@ impl ChangeEvent {
pub fn path(&self) -> Option<&SystemPath> { pub fn path(&self) -> Option<&SystemPath> {
match self { match self {
ChangeEvent::Created { path, .. } ChangeEvent::Opened(path)
| ChangeEvent::Created { path, .. }
| ChangeEvent::Changed { path, .. } | ChangeEvent::Changed { path, .. }
| ChangeEvent::Deleted { path, .. } => Some(path), | ChangeEvent::Deleted { path, .. } => Some(path),
ChangeEvent::Rescan => None, ChangeEvent::Rescan => None,