From 93c7f02c4e07e791cd84a82183e384427b6a3fe2 Mon Sep 17 00:00:00 2001
From: Marek Kaput <marek.kaput@swmansion.com>
Date: Fri, 22 Mar 2024 17:10:32 +0100
Subject: [PATCH] LS: Refactor `get_uri` into `LsProtoGroup::url_for_file`

commit-id:fade1eda
---
 .../src/ide/navigation/goto_definition.rs     |  4 ++--
 .../src/lang/lsp/ls_proto_group.rs            | 21 +++++++++++++++++--
 crates/cairo-lang-language-server/src/lib.rs  | 18 ++--------------
 3 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/crates/cairo-lang-language-server/src/ide/navigation/goto_definition.rs b/crates/cairo-lang-language-server/src/ide/navigation/goto_definition.rs
index fc5ead59c74..a25c51edbb6 100644
--- a/crates/cairo-lang-language-server/src/ide/navigation/goto_definition.rs
+++ b/crates/cairo-lang-language-server/src/ide/navigation/goto_definition.rs
@@ -3,7 +3,7 @@ use cairo_lang_utils::Upcast;
 use tower_lsp::lsp_types::{GotoDefinitionParams, GotoDefinitionResponse, Location, Range};
 
 use crate::lang::lsp::LsProtoGroup;
-use crate::{from_pos, get_definition_location, get_uri};
+use crate::{from_pos, get_definition_location};
 
 /// Get the definition location of a symbol at a given text document position.
 #[tracing::instrument(
@@ -18,7 +18,7 @@ pub fn goto_definition(
     let file = db.file_for_url(&params.text_document_position_params.text_document.uri);
     let position = params.text_document_position_params.position;
     let (found_file, span) = get_definition_location(db, file, position)?;
-    let found_uri = get_uri(db, found_file);
+    let found_uri = db.url_for_file(found_file);
 
     let start = from_pos(span.start.position_in_file(db.upcast(), found_file).unwrap());
     let end = from_pos(span.end.position_in_file(db.upcast(), found_file).unwrap());
diff --git a/crates/cairo-lang-language-server/src/lang/lsp/ls_proto_group.rs b/crates/cairo-lang-language-server/src/lang/lsp/ls_proto_group.rs
index e1e033a067e..2da4e897f15 100644
--- a/crates/cairo-lang-language-server/src/lang/lsp/ls_proto_group.rs
+++ b/crates/cairo-lang-language-server/src/lang/lsp/ls_proto_group.rs
@@ -1,5 +1,5 @@
 use cairo_lang_filesystem::db::FilesGroup;
-use cairo_lang_filesystem::ids::FileId;
+use cairo_lang_filesystem::ids::{FileId, FileLongId};
 use cairo_lang_utils::Upcast;
 use salsa::InternKey;
 use tower_lsp::lsp_types::Url;
@@ -27,6 +27,23 @@ pub trait LsProtoGroup: Upcast<dyn FilesGroup> {
             _ => panic!("Invalid URL: scheme is not supported by this language server."),
         }
     }
+
+    /// Get the canonical [`Url`] for a [`FileId`].
+    fn url_for_file(&self, file_id: FileId) -> Url {
+        match self.upcast().lookup_intern_file(file_id) {
+            FileLongId::OnDisk(path) => {
+                Url::from_file_path(path).expect("Salsa is expected to store absolute paths.")
+            }
+            FileLongId::Virtual(virtual_file) => {
+                let url = format!(
+                    "vfs://{}/{}.cairo",
+                    file_id.as_intern_id().as_usize(),
+                    virtual_file.name
+                );
+                Url::parse(&url).unwrap()
+            }
+        }
+    }
 }
 
-impl<T> LsProtoGroup for T where T: Upcast<dyn FilesGroup> {}
+impl<T> LsProtoGroup for T where T: Upcast<dyn FilesGroup> + ?Sized {}
diff --git a/crates/cairo-lang-language-server/src/lib.rs b/crates/cairo-lang-language-server/src/lib.rs
index d7c68a77eea..082e23abdb2 100644
--- a/crates/cairo-lang-language-server/src/lib.rs
+++ b/crates/cairo-lang-language-server/src/lib.rs
@@ -50,7 +50,6 @@ use cairo_lang_syntax::node::{ast, SyntaxNode, TypedSyntaxNode};
 use cairo_lang_test_plugin::test_plugin_suite;
 use cairo_lang_utils::ordered_hash_map::OrderedHashMap;
 use cairo_lang_utils::{try_extract_matches, OptionHelper, Upcast};
-use salsa::InternKey;
 use serde_json::Value;
 use tower_lsp::jsonrpc::{Error as LSPError, Result as LSPResult};
 use tower_lsp::lsp_types::notification::Notification;
@@ -287,7 +286,7 @@ impl Backend {
             for crate_id in db.crates() {
                 for module_id in db.crate_modules(crate_id).iter() {
                     for file_id in db.module_files(*module_id).unwrap_or_default().iter() {
-                        files_set.insert(get_uri((*db).upcast(), *file_id));
+                        files_set.insert(db.url_for_file(*file_id));
                     }
                 }
             }
@@ -1201,19 +1200,6 @@ fn is_cairo_file_path(file_path: &Url) -> bool {
     file_path.path().ends_with(".cairo")
 }
 
-/// Gets the canonical URI for a file.
-fn get_uri(db: &dyn FilesGroup, file_id: FileId) -> Url {
-    let virtual_file = match db.lookup_intern_file(file_id) {
-        FileLongId::OnDisk(path) => return Url::from_file_path(path).unwrap(),
-        FileLongId::Virtual(virtual_file) => virtual_file,
-    };
-    let uri = Url::parse(
-        format!("vfs://{}/{}.cairo", file_id.as_intern_id().as_usize(), virtual_file.name).as_str(),
-    )
-    .unwrap();
-    uri
-}
-
 /// Converts an internal diagnostic location to an LSP range.
 fn get_range(db: &dyn FilesGroup, location: &DiagnosticLocation) -> Range {
     let location = location.user_location(db);
@@ -1236,7 +1222,7 @@ fn get_diagnostics<T: DiagnosticEntry>(
             if let Some(location) = &note.location {
                 related_information.push(DiagnosticRelatedInformation {
                     location: Location {
-                        uri: get_uri(db.upcast(), location.file_id),
+                        uri: db.url_for_file(location.file_id),
                         range: get_range(db.upcast(), location),
                     },
                     message: note.text.clone(),