Nits

Author: MichaReiser

crates/ty_server/src/server/api/diagnostics.rs

@@ -14,8 +14,8 @@ use ruff_db::system::SystemPathBuf;
 use ty_project::{Db, ProjectDatabase};
 
 use crate::document::{FileRangeExt, ToRangeExt};
+use crate::session::DocumentSnapshot;
 use crate::session::client::Client;
-use crate::session::{DocumentHandle, DocumentSnapshot};
 use crate::system::{AnySystemPath, file_to_url};
 use crate::{DocumentRef, PositionEncoding, Session};
 
@@ -115,18 +115,18 @@ impl LspDiagnostics {
     }
 }
 
-/// Clears the diagnostics for the document identified by `key`.
+/// Clears the diagnostics for the document identified by `uri`.
 ///
 /// This is done by notifying the client with an empty list of diagnostics for the document.
 /// For notebook cells, this clears diagnostics for the specific cell.
 /// For other document types, this clears diagnostics for the main document.
-pub(super) fn clear_diagnostics(session: &Session, document: &DocumentHandle, client: &Client) {
+pub(super) fn clear_diagnostics(session: &Session, uri: &lsp_types::Url, client: &Client) {
     if session.client_capabilities().supports_pull_diagnostics() {
         return;
     }
 
     client.send_notification::<PublishDiagnostics>(PublishDiagnosticsParams {
-        uri: document.url().clone(),
+        uri: uri.clone(),
         diagnostics: vec![],
         version: None,
     });

crates/ty_server/src/server/api/notifications/did_change_watched_files.rs

@@ -1,3 +1,4 @@
+use crate::document::DocumentKey;
 use crate::server::Result;
 use crate::server::api::diagnostics::{publish_diagnostics, publish_settings_diagnostics};
 use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler};
@@ -25,7 +26,8 @@ impl SyncNotificationHandler for DidChangeWatchedFiles {
         let mut events_by_db: FxHashMap<_, Vec<ChangeEvent>> = FxHashMap::default();
 
         for change in params.changes {
-            let path = AnySystemPath::from_url(&change.uri);
+            let key = DocumentKey::from_url(&change.uri);
+            let path = key.to_file_path();
 
             let system_path = match path {
                 AnySystemPath::System(system) => system,

crates/ty_server/src/server/api/notifications/did_close.rs

@@ -56,7 +56,7 @@ impl SyncNotificationHandler for DidCloseTextDocumentHandler {
                         .diagnostic_mode()
                         .is_open_files_only()
                 {
-                    clear_diagnostics(session, &document, client);
+                    clear_diagnostics(session, document.url(), client);
                 }
             }
             AnySystemPath::SystemVirtual(virtual_path) => {
@@ -69,7 +69,7 @@ impl SyncNotificationHandler for DidCloseTextDocumentHandler {
 
                 // Always clear diagnostics for virtual files, as they don't really exist on disk
                 // which means closing them is like deleting the file.
-                clear_diagnostics(session, &document, client);
+                clear_diagnostics(session, document.url(), client);
             }
         }
 

crates/ty_server/src/server/api/requests/workspace_diagnostic.rs

@@ -9,7 +9,7 @@ use crate::server::{Action, Result};
 use crate::session::client::Client;
 use crate::session::index::Index;
 use crate::session::{SessionSnapshot, SuspendedWorkspaceDiagnosticRequest};
-use crate::system::{AnySystemPath, file_to_url};
+use crate::system::file_to_url;
 use lsp_server::RequestId;
 use lsp_types::request::WorkspaceDiagnosticRequest;
 use lsp_types::{
@@ -318,7 +318,7 @@ struct ResponseWriter<'a> {
     // It's important that we use `AnySystemPath` over `Url` here because
     // `file_to_url` isn't guaranteed to return the exact same URL as the one provided
     // by the client.
-    previous_result_ids: FxHashMap<AnySystemPath, (Url, String)>,
+    previous_result_ids: FxHashMap<DocumentKey, (Url, String)>,
 }
 
 impl<'a> ResponseWriter<'a> {
@@ -347,7 +347,7 @@ impl<'a> ResponseWriter<'a> {
 
         let previous_result_ids = previous_result_ids
             .into_iter()
-            .map(|prev| (AnySystemPath::from_url(&prev.uri), (prev.uri, prev.value)))
+            .map(|prev| (DocumentKey::from_url(&prev.uri), (prev.uri, prev.value)))
             .collect();
 
         Self {
@@ -364,18 +364,16 @@ impl<'a> ResponseWriter<'a> {
             return;
         };
 
+        let key = DocumentKey::from_url(&url);
         let version = self
             .index
-            .make_document_ref(&DocumentKey::from_url(&url))
+            .make_document_ref(&key)
             .map(|doc| i64::from(doc.version()))
             .ok();
 
         let result_id = Diagnostics::result_id_from_hash(diagnostics);
 
-        let previous_result_id = self
-            .previous_result_ids
-            .remove(&AnySystemPath::from_url(&url))
-            .map(|(_url, id)| id);
+        let previous_result_id = self.previous_result_ids.remove(&key).map(|(_url, id)| id);
 
         let report = match result_id {
             Some(new_id) if Some(&new_id) == previous_result_id.as_ref() => {
@@ -439,11 +437,11 @@ impl<'a> ResponseWriter<'a> {
 
         // Handle files that had diagnostics in previous request but no longer have any
         // Any remaining entries in previous_results are files that were fixed
-        for (previous_url, previous_result_id) in self.previous_result_ids.into_values() {
+        for (key, (previous_url, previous_result_id)) in self.previous_result_ids {
             // This file had diagnostics before but doesn't now, so we need to report it as having no diagnostics
             let version = self
                 .index
-                .make_document_ref(&DocumentKey::from_url(&previous_url))
+                .make_document_ref(&key)
                 .ok()
                 .map(|doc| i64::from(doc.version()));
 

crates/ty_server/src/session/index.rs

@@ -17,7 +17,7 @@ use rustc_hash::FxHashMap;
 #[derive(Debug)]
 pub(crate) struct Index {
     /// Maps all document file paths to the associated document controller
-    pub(super) documents: FxHashMap<DocumentKey, Document>,
+    documents: FxHashMap<DocumentKey, Document>,
 
     /// Maps opaque cell URLs to a notebook path (document)
     notebook_cells: FxHashMap<String, DocumentKey>,

crates/ty_server/src/system.rs

@@ -40,29 +40,6 @@ pub(crate) enum AnySystemPath {
 }
 
 impl AnySystemPath {
-    /// Converts the given [`Url`] to an [`AnySystemPath`].
-    ///
-    /// If the URL scheme is `file`, then the path is converted to a [`SystemPathBuf`] unless
-    /// the url isn't a valid file path.
-    ///
-    /// In all other cases, the URL is converted to a [`SystemVirtualPathBuf`].
-    pub(crate) fn from_url(url: &Url) -> Self {
-        if url.scheme() == "file" {
-            if let Ok(path) = url.to_file_path() {
-                AnySystemPath::System(
-                    SystemPathBuf::from_path_buf(path).expect("URL to be valid UTF-8"),
-                )
-            } else {
-                tracing::warn!(
-                    "Treating `file:` url `{url}` as virtual path as it isn't a valid file path"
-                );
-                AnySystemPath::SystemVirtual(SystemVirtualPath::new(url.as_str()).to_path_buf())
-            }
-        } else {
-            AnySystemPath::SystemVirtual(SystemVirtualPath::new(url.as_str()).to_path_buf())
-        }
-    }
-
     pub(crate) const fn as_system(&self) -> Option<&SystemPathBuf> {
         match self {
             AnySystemPath::System(system_path_buf) => Some(system_path_buf),