Address additional review comments

Author: dhruvmanila

crates/red_knot_server/src/server/api/notifications/did_close_notebook.rs

@@ -1,11 +1,14 @@
 use lsp_types::notification::DidCloseNotebookDocument;
 use lsp_types::DidCloseNotebookDocumentParams;
 
+use ruff_db::files::File;
+
 use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler};
 use crate::server::api::LSPResult;
 use crate::server::client::{Notifier, Requester};
 use crate::server::Result;
 use crate::session::Session;
+use crate::system::url_to_system_path;
 
 pub(crate) struct DidCloseNotebookHandler;
 
@@ -20,12 +23,18 @@ impl SyncNotificationHandler for DidCloseNotebookHandler {
         _requester: &mut Requester,
         params: DidCloseNotebookDocumentParams,
     ) -> Result<()> {
+        let Ok(path) = url_to_system_path(&params.notebook_document.uri) else {
+            return Ok(());
+        };
+
         let key = session.key_from_url(params.notebook_document.uri);
         session
             .close_document(&key)
             .with_failure_code(lsp_server::ErrorCode::InternalError)?;
 
-        // TODO(dhruvmanila): Close the file in the `RootDatabase`
+        if let Some(db) = session.workspace_db_for_path_mut(path.as_std_path()) {
+            File::sync_path(db.get_mut(), &path);
+        }
 
         Ok(())
     }

crates/red_knot_server/src/server/api/notifications/did_open.rs

@@ -1,6 +1,7 @@
 use lsp_types::notification::DidOpenTextDocument;
 use lsp_types::DidOpenTextDocumentParams;
-use ruff_db::files::{system_path_to_file, File};
+
+use ruff_db::files::system_path_to_file;
 
 use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler};
 use crate::server::client::{Notifier, Requester};
@@ -30,12 +31,12 @@ impl SyncNotificationHandler for DidOpenTextDocumentHandler {
         session.open_text_document(params.text_document.uri, document);
 
         if let Some(db) = session.workspace_db_for_path_mut(path.as_std_path()) {
-            // TODO(dhruvmanila): Store the `file` in `TextDocument`
-            let _file = system_path_to_file(&**db, &path).unwrap();
-            File::sync_path(db.get_mut(), &path);
+            // TODO(dhruvmanila): Store the `file` in `DocumentController`
+            let file = system_path_to_file(&**db, &path).unwrap();
+            file.sync(db.get_mut());
         }
 
-        // Publish diagnostics if the client doesn't support pull diagnostics
+        // TODO(dhruvmanila): Publish diagnostics if the client doesn't support pull diagnostics
 
         Ok(())
     }

crates/red_knot_server/src/server/api/notifications/did_open_notebook.rs

@@ -2,12 +2,15 @@ use lsp_server::ErrorCode;
 use lsp_types::notification::DidOpenNotebookDocument;
 use lsp_types::DidOpenNotebookDocumentParams;
 
+use ruff_db::files::system_path_to_file;
+
 use crate::edit::NotebookDocument;
 use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler};
 use crate::server::api::LSPResult;
 use crate::server::client::{Notifier, Requester};
 use crate::server::Result;
 use crate::session::Session;
+use crate::system::url_to_system_path;
 
 pub(crate) struct DidOpenNotebookHandler;
 
@@ -22,19 +25,26 @@ impl SyncNotificationHandler for DidOpenNotebookHandler {
         _requester: &mut Requester,
         params: DidOpenNotebookDocumentParams,
     ) -> Result<()> {
+        let Ok(path) = url_to_system_path(&params.notebook_document.uri) else {
+            return Ok(());
+        };
+
         let notebook = NotebookDocument::new(
             params.notebook_document.version,
             params.notebook_document.cells,
             params.notebook_document.metadata.unwrap_or_default(),
             params.cell_text_documents,
         )
         .with_failure_code(ErrorCode::InternalError)?;
-
         session.open_notebook_document(params.notebook_document.uri.clone(), notebook);
 
-        // TODO(dhruvmanila): Open the file in the `RootDatabase`
+        if let Some(db) = session.workspace_db_for_path_mut(path.as_std_path()) {
+            // TODO(dhruvmanila): Store the `file` in `DocumentController`
+            let file = system_path_to_file(&**db, &path).unwrap();
+            file.sync(db.get_mut());
+        }
 
-        // Publish diagnostics if the client doesn't support pull diagnostics
+        // TODO(dhruvmanila): Publish diagnostics if the client doesn't support pull diagnostics
 
         Ok(())
     }

crates/red_knot_server/src/session.rs

@@ -28,6 +28,9 @@ mod capabilities;
 pub(crate) mod index;
 mod settings;
 
+// TODO(dhruvmanila): In general, the server shouldn't use any salsa queries directly and instead
+// should use methods on `RootDatabase`.
+
 /// The global state for the LSP
 pub struct Session {
     /// Used to retrieve information about open documents and settings.