From 416cf46d7ffa4a0009acff2dc54e700f4924a08a Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 11 Jun 2026 13:56:56 +0200 Subject: [PATCH 1/4] Add `wire_url()` method on `WorldState` --- crates/ark/src/lsp/find_references.rs | 2 +- crates/ark/src/lsp/goto_definition.rs | 7 ++--- crates/ark/src/lsp/rename.rs | 2 +- crates/ark/src/lsp/state.rs | 12 +++++++++ crates/ark/src/lsp/tests.rs | 1 + crates/ark/src/lsp/tests/find_references.rs | 19 ++++++++++++++ crates/ark/src/lsp/tests/goto_definition.rs | 19 ++++++++++++++ crates/ark/src/lsp/tests/rename.rs | 16 ++++++++++++ crates/ark/src/lsp/tests/state.rs | 29 +++++++++++++++++++++ 9 files changed, 102 insertions(+), 5 deletions(-) create mode 100644 crates/ark/src/lsp/tests/state.rs diff --git a/crates/ark/src/lsp/find_references.rs b/crates/ark/src/lsp/find_references.rs index af2050c13d..28c57b368a 100644 --- a/crates/ark/src/lsp/find_references.rs +++ b/crates/ark/src/lsp/find_references.rs @@ -30,7 +30,7 @@ pub(crate) fn find_references( .iter() .filter_map(|fr| { let range = to_proto::range(fr.range, fr.file.line_index(db), encoding).log_err()?; - Some(Location::new(fr.file.path(db).to_url(), range)) + Some(Location::new(state.wire_url(fr.file), range)) }) .collect(); diff --git a/crates/ark/src/lsp/goto_definition.rs b/crates/ark/src/lsp/goto_definition.rs index 8baaf62ed0..c5a2cdd83c 100644 --- a/crates/ark/src/lsp/goto_definition.rs +++ b/crates/ark/src/lsp/goto_definition.rs @@ -35,7 +35,7 @@ pub(crate) fn goto_definition( // to several bindings; the client offers all of them. let links = targets .iter() - .map(|target| nav_target_to_link(db, encoding, target)) + .map(|target| nav_target_to_link(state, encoding, target)) .collect::>>()?; Ok(Some(GotoDefinitionResponse::Link(links))) @@ -45,17 +45,18 @@ pub(crate) fn goto_definition( /// offsets in the target file, so we translate them through that file's own /// line index, not the file the request came from. fn nav_target_to_link( - db: &dyn Db, + state: &WorldState, encoding: PositionEncoding, target: &NavigationTarget, ) -> anyhow::Result { + let db = &state.db; let line_index = target.file.line_index(db); let target_range = to_proto::range(target.full_range, line_index, encoding)?; let target_selection_range = to_proto::range(target.focus_range, line_index, encoding)?; Ok(LocationLink { origin_selection_range: None, - target_uri: target.file.path(db).to_url(), + target_uri: state.wire_url(target.file), target_range, target_selection_range, }) diff --git a/crates/ark/src/lsp/rename.rs b/crates/ark/src/lsp/rename.rs index 71fea04d01..8a5b28f6fe 100644 --- a/crates/ark/src/lsp/rename.rs +++ b/crates/ark/src/lsp/rename.rs @@ -66,7 +66,7 @@ pub(crate) fn rename( let mut changes: HashMap> = HashMap::new(); for range in ranges { let line_index = range.file.line_index(db); - let target_url = range.file.path(db).to_url(); + let target_url = state.wire_url(range.file); let range = to_proto::range(range.range, line_index, encoding)?; changes.entry(target_url).or_default().push(TextEdit { range, diff --git a/crates/ark/src/lsp/state.rs b/crates/ark/src/lsp/state.rs index 5cc32f585f..42ee509550 100644 --- a/crates/ark/src/lsp/state.rs +++ b/crates/ark/src/lsp/state.rs @@ -120,6 +120,18 @@ impl WorldState { Ok(ark_file.clone()) } + /// URL to put on the wire for `file`. Open buffers keep the editor's + /// verbatim URL so the frontend sees the URI it sent us. Files that were + /// never opened in the editor (disk-scanned files, resolution targets) have + /// no verbatim URL, so synthesise one from the normalised path. + pub(crate) fn wire_url(&self, file: File) -> Url { + let path = file.path(&self.db); + self.open_files + .get(path) + .map(|ark_file| ark_file.url.clone()) + .unwrap_or_else(|| path.to_url()) + } + /// Register an editor buffer in `open_files`, keying on the normalised /// [`FilePath`] and stashing the verbatim editor URL on [`ArkFile::url`] for /// wire output. diff --git a/crates/ark/src/lsp/tests.rs b/crates/ark/src/lsp/tests.rs index e34c6c5087..c07cd3dcb3 100644 --- a/crates/ark/src/lsp/tests.rs +++ b/crates/ark/src/lsp/tests.rs @@ -4,5 +4,6 @@ mod find_references; mod goto_definition; mod main_loop; mod rename; +mod state; mod state_handlers; mod utils; diff --git a/crates/ark/src/lsp/tests/find_references.rs b/crates/ark/src/lsp/tests/find_references.rs index db04c35f4b..afb130b489 100644 --- a/crates/ark/src/lsp/tests/find_references.rs +++ b/crates/ark/src/lsp/tests/find_references.rs @@ -1,3 +1,4 @@ +use aether_path::FilePath; use tower_lsp::lsp_types; use tower_lsp::lsp_types::Location; use tower_lsp::lsp_types::ReferenceContext; @@ -170,6 +171,24 @@ fn test_cross_file_dollar_kind() { .any(|l| l.uri == uri2 && l.range == range((2, 0), (2, 3)))); } +#[test] +fn test_locations_use_verbatim_url() { + // The doubled slash survives in the editor's URL but `FilePath` normalises + // it away, so a path round-trip would change it. Locations must carry the + // exact URI the editor opened the buffer with. + let code = "foo <- 1\nfoo\n"; + let uri = lsp_types::Url::parse("file:///C:/proj//foo.R").unwrap(); + let state = make_state(&uri, code); + + let params = make_params(uri.clone(), 1, 0, true); + let locs = find_references(params, &state).unwrap(); + + assert!(!locs.is_empty()); + assert!(locs.iter().all(|loc| loc.uri == uri)); + // Confirm the round-trip really would have differed, so the check above bites. + assert_ne!(FilePath::from_url(&uri).to_url(), uri); +} + #[test] fn test_cross_file_namespace_access() { // Cursor on `mutate` in `dplyr::mutate` -- structural scan matches every diff --git a/crates/ark/src/lsp/tests/goto_definition.rs b/crates/ark/src/lsp/tests/goto_definition.rs index c54b1d9c91..387111bff2 100644 --- a/crates/ark/src/lsp/tests/goto_definition.rs +++ b/crates/ark/src/lsp/tests/goto_definition.rs @@ -1,3 +1,4 @@ +use aether_path::FilePath; use assert_matches::assert_matches; use tower_lsp::lsp_types; use tower_lsp::lsp_types::GotoDefinitionParams; @@ -63,6 +64,24 @@ fn test_goto_definition_prefers_local_symbol() { ); } +#[test] +fn test_target_uri_is_verbatim() { + // The doubled slash normalises away in `FilePath`, so the target URI is + // only correct if it comes from the buffer's verbatim URL. + let uri = lsp_types::Url::parse("file:///C:/proj//file.R").unwrap(); + let state = make_state(&uri, "foo <- 1\nfoo\n"); + + let params = make_params(uri.clone(), 1, 0); + + assert_matches!( + goto_definition(params, &state).unwrap(), + Some(GotoDefinitionResponse::Link(ref links)) => { + assert_eq!(links[0].target_uri, uri); + } + ); + assert_ne!(FilePath::from_url(&uri).to_url(), uri); +} + #[test] fn test_unbound_identifier_returns_none() { // A free identifier with no reachable binding returns `None`, matching how diff --git a/crates/ark/src/lsp/tests/rename.rs b/crates/ark/src/lsp/tests/rename.rs index 8b746995f5..46bb940955 100644 --- a/crates/ark/src/lsp/tests/rename.rs +++ b/crates/ark/src/lsp/tests/rename.rs @@ -74,6 +74,22 @@ fn test_prepare_rename_on_namespace_access_returns_none() { assert!(prepare_rename(params, &state).unwrap().is_none()); } +#[test] +fn test_edits_keyed_by_verbatim_url() { + // The `WorkspaceEdit` must key its changes on the buffer's verbatim URL, + // not a normalised round-trip, or the editor won't match them to the file. + let code = "foo <- 1\nfoo\n"; + let uri = lsp_types::Url::parse("file:///C:/proj//file.R").unwrap(); + let state = make_state(&uri, code); + + let params = make_rename_params(uri.clone(), 0, 0, "bar"); + let edit = rename(params, &state).unwrap().unwrap(); + + let changes = edit.changes.expect("changes map"); + assert!(changes.contains_key(&uri)); + assert_ne!(FilePath::from_url(&uri).to_url(), uri); +} + #[test] fn test_rename_emits_edits_for_def_and_uses() { let code = "foo <- 1\nfoo + foo\n"; diff --git a/crates/ark/src/lsp/tests/state.rs b/crates/ark/src/lsp/tests/state.rs new file mode 100644 index 0000000000..e42ef9b159 --- /dev/null +++ b/crates/ark/src/lsp/tests/state.rs @@ -0,0 +1,29 @@ +use aether_path::FilePath; +use oak_scan::DbScan; +use url::Url; + +use crate::lsp::state::WorldState; + +#[test] +fn test_wire_url_open_buffer_keeps_verbatim_url() { + let mut state = WorldState::default(); + let url = Url::parse("file:///C:/proj//foo.R").unwrap(); + let file = state + .db + .upsert_editor(FilePath::from_url(&url), "x <- 1\n".to_string()); + state.insert_ark_file(url.clone(), file, None); + assert_eq!(state.wire_url(file), url); +} + +#[test] +fn test_wire_url_non_open_file_synthesises_url() { + let mut state = WorldState::default(); + let url = Url::parse("file:///C:/proj//bar.R").unwrap(); + let file = state + .db + .upsert_editor(FilePath::from_url(&url), "y <- 2\n".to_string()); + // Not inserted into open_files, so wire_url synthesises from path + let wire = state.wire_url(file); + assert_eq!(wire, file.path(&state.db).to_url()); + assert_ne!(wire, url); +} From 6efda726a5cf8269f207c2628277e14ec40bff25 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 12 Jun 2026 10:09:47 +0200 Subject: [PATCH 2/4] Use `wire_url()` with indexer-based handlers --- .../sources/composite/workspace.rs | 4 +++- crates/ark/src/lsp/diagnostics.rs | 2 +- crates/ark/src/lsp/indexer.rs | 20 +++++++++---------- crates/ark/src/lsp/symbols.rs | 6 ++++-- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/crates/ark/src/lsp/completions/sources/composite/workspace.rs b/crates/ark/src/lsp/completions/sources/composite/workspace.rs index 3a4f6968dd..18c3d3cfc5 100644 --- a/crates/ark/src/lsp/completions/sources/composite/workspace.rs +++ b/crates/ark/src/lsp/completions/sources/composite/workspace.rs @@ -69,13 +69,15 @@ fn completions_from_workspace( let token = token.as_str(); // get entries from the index - indexer::map(&state.db, |uri, symbol, entry| { + indexer::map(&state.db, |file, symbol, entry| { if !symbol.fuzzy_matches(token) { return; } match &entry.data { indexer::IndexEntryData::Function { name, .. } => { + let uri = state.wire_url(file); + let fun_context = match completion_context.function_context() { Ok(fun_context) => fun_context, Err(err) => { diff --git a/crates/ark/src/lsp/diagnostics.rs b/crates/ark/src/lsp/diagnostics.rs index cce381bbde..bc4ec750a0 100644 --- a/crates/ark/src/lsp/diagnostics.rs +++ b/crates/ark/src/lsp/diagnostics.rs @@ -171,7 +171,7 @@ pub(crate) fn generate_diagnostics( context.document_symbols.push(HashMap::new()); // Add the current workspace symbols. - indexer::map(db, |_uri, _symbol, entry| match &entry.data { + indexer::map(db, |_file, _symbol, entry| match &entry.data { indexer::IndexEntryData::Function { name, arguments: _ } => { context.workspace_symbols.insert(name.to_string()); }, diff --git a/crates/ark/src/lsp/indexer.rs b/crates/ark/src/lsp/indexer.rs index 046a4669f4..725448734c 100644 --- a/crates/ark/src/lsp/indexer.rs +++ b/crates/ark/src/lsp/indexer.rs @@ -10,7 +10,6 @@ use std::sync::LazyLock; use aether_lsp_utils::proto::to_proto; use aether_lsp_utils::proto::PositionEncoding; -use aether_path::FilePath; use oak_db::File; use regex::Regex; use stdext::result::ResultExt; @@ -19,7 +18,6 @@ use stdext::unwrap::IntoResult; use tower_lsp::lsp_types::Range; use tree_sitter::Node; use tree_sitter::Query; -use url::Url; use crate::lsp; use crate::lsp::db::ArkDb; @@ -82,16 +80,14 @@ pub struct IndexEntry { } /// Convert an index entry's tree-sitter point range to an LSP range, resolving -/// the file's line index from the db. Returns `None` if the file is no longer -/// in the db or the points fall outside it, in which case the symbol is -/// dropped from the results. +/// the file's line index from the db. Returns `None` if the points fall outside +/// the line index, in which case the symbol is dropped from the results. pub(crate) fn index_range_to_lsp_range( db: &dyn ArkDb, - uri: &Url, + file: File, range: IndexRange, encoding: PositionEncoding, ) -> Option { - let file = db.file_by_path(&FilePath::from_url(uri))?; let line_index = file.line_index(db); let to_position = |point: IndexPoint| { @@ -160,15 +156,16 @@ fn file_index(db: &dyn ArkDb, file: File) -> FileIndex { FileIndex { symbols } } -/// Visit every workspace symbol across all indexable files. -pub(crate) fn map(db: &dyn ArkDb, mut callback: impl FnMut(&Url, &str, &IndexEntry)) { +/// Visit every workspace symbol across all indexable files. Callers that need a +/// URL for messages to the frontend can resolve it from `File` via +/// `WorldState::wire_url`. +pub(crate) fn map(db: &dyn ArkDb, mut callback: impl FnMut(File, &str, &IndexEntry)) { for &file in oak_db::workspace_files(db) { if !is_indexable(db, file) { continue; } - let url = file.path(db).to_url(); for (symbol, entry) in file_index(db, file).symbols.iter() { - callback(&url, symbol, entry); + callback(file, symbol, entry); } } } @@ -380,6 +377,7 @@ mod tests { use assert_matches::assert_matches; use insta::assert_debug_snapshot; use oak_scan::DbScan; + use url::Url; use super::*; use crate::lsp::ark_file::test_ark_file; diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index 844432c435..6d4ec0c3fb 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -64,15 +64,17 @@ pub(crate) fn symbols( let encoding = state.config.position_encoding; let mut info: Vec = Vec::new(); - indexer::map(db, |uri, symbol, entry| { + indexer::map(db, |file, symbol, entry| { if !symbol.fuzzy_matches(query) { return; } - let Some(range) = indexer::index_range_to_lsp_range(db, uri, entry.range, encoding) else { + let Some(range) = indexer::index_range_to_lsp_range(db, file, entry.range, encoding) else { return; }; + let uri = state.wire_url(file); + match &entry.data { IndexEntryData::Function { name, arguments: _ } => { info.push(SymbolInformation { From 6658ad0dbf5acda9422fc91969af7c785726c510 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 16 Jun 2026 22:47:56 +0200 Subject: [PATCH 3/4] Remove unnecessary clones --- crates/ark/src/lsp/symbols.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index 6d4ec0c3fb..4b1c42f633 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -81,7 +81,7 @@ pub(crate) fn symbols( name: name.to_string(), kind: SymbolKind::FUNCTION, location: Location { - uri: uri.clone(), + uri, range, }, tags: None, @@ -96,7 +96,7 @@ pub(crate) fn symbols( name: title.to_string(), kind: SymbolKind::STRING, location: Location { - uri: uri.clone(), + uri, range, }, tags: None, @@ -111,7 +111,7 @@ pub(crate) fn symbols( name: name.clone(), kind: SymbolKind::VARIABLE, location: Location { - uri: uri.clone(), + uri, range, }, tags: None, @@ -125,7 +125,7 @@ pub(crate) fn symbols( name: name.clone(), kind: SymbolKind::METHOD, location: Location { - uri: uri.clone(), + uri, range, }, tags: None, From 5cee1001e8c92631cd7ec234aff6096b3c66c5c0 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 16 Jun 2026 22:58:02 +0200 Subject: [PATCH 4/4] Reformat --- crates/ark/src/lsp/symbols.rs | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index 4b1c42f633..eccb05f144 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -80,10 +80,7 @@ pub(crate) fn symbols( info.push(SymbolInformation { name: name.to_string(), kind: SymbolKind::FUNCTION, - location: Location { - uri, - range, - }, + location: Location { uri, range }, tags: None, deprecated: None, container_name: None, @@ -95,10 +92,7 @@ pub(crate) fn symbols( info.push(SymbolInformation { name: title.to_string(), kind: SymbolKind::STRING, - location: Location { - uri, - range, - }, + location: Location { uri, range }, tags: None, deprecated: None, container_name: None, @@ -110,10 +104,7 @@ pub(crate) fn symbols( info.push(SymbolInformation { name: name.clone(), kind: SymbolKind::VARIABLE, - location: Location { - uri, - range, - }, + location: Location { uri, range }, tags: None, deprecated: None, container_name: None, @@ -124,10 +115,7 @@ pub(crate) fn symbols( info.push(SymbolInformation { name: name.clone(), kind: SymbolKind::METHOD, - location: Location { - uri, - range, - }, + location: Location { uri, range }, tags: None, deprecated: None, container_name: None,