From 2d3e02e2f4e96197289e27aeadcebe11d88f45a4 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 7 Jul 2025 13:26:28 +0200 Subject: [PATCH 1/8] Don't include comment sections in workspace symbols by default --- crates/ark/src/lsp/config.rs | 23 +++++++++++++++++++++++ crates/ark/src/lsp/handlers.rs | 3 ++- crates/ark/src/lsp/main_loop.rs | 2 +- crates/ark/src/lsp/symbols.rs | 29 +++++++++++++++++------------ 4 files changed, 43 insertions(+), 14 deletions(-) diff --git a/crates/ark/src/lsp/config.rs b/crates/ark/src/lsp/config.rs index 0628ec8c2..f4da4ff00 100644 --- a/crates/ark/src/lsp/config.rs +++ b/crates/ark/src/lsp/config.rs @@ -33,6 +33,14 @@ pub static GLOBAL_SETTINGS: &[Setting] = &[ .unwrap_or_else(|| SymbolsConfig::default().include_assignments_in_blocks) }, }, + Setting { + key: "positron.r.workspaceSymbols.includeCommentSections", + set: |cfg, v| { + cfg.workspace_symbols.include_comment_sections = v + .as_bool() + .unwrap_or_else(|| WorkspaceSymbolsConfig::default().include_comment_sections) + }, + }, ]; /// These document settings are updated on a URI basis. Each document has its @@ -77,6 +85,7 @@ pub static DOCUMENT_SETTINGS: &[Setting] = &[ pub(crate) struct LspConfig { pub(crate) diagnostics: DiagnosticsConfig, pub(crate) symbols: SymbolsConfig, + pub(crate) workspace_symbols: WorkspaceSymbolsConfig, } #[derive(Serialize, Deserialize, Clone, Debug)] @@ -85,6 +94,12 @@ pub struct SymbolsConfig { pub include_assignments_in_blocks: bool, } +#[derive(Serialize, Deserialize, Clone, Debug)] +pub struct WorkspaceSymbolsConfig { + /// Whether to include sections like `# My section ---` in workspace symbols. + pub include_comment_sections: bool, +} + /// Configuration of a document. /// /// The naming follows where possible. @@ -120,6 +135,14 @@ impl Default for SymbolsConfig { } } +impl Default for WorkspaceSymbolsConfig { + fn default() -> Self { + Self { + include_comment_sections: false, + } + } +} + impl Default for IndentationConfig { fn default() -> Self { Self { diff --git a/crates/ark/src/lsp/handlers.rs b/crates/ark/src/lsp/handlers.rs index 64bca86f3..4d54a91ad 100644 --- a/crates/ark/src/lsp/handlers.rs +++ b/crates/ark/src/lsp/handlers.rs @@ -129,8 +129,9 @@ pub(crate) async fn handle_initialized( #[tracing::instrument(level = "info", skip_all)] pub(crate) fn handle_symbol( params: WorkspaceSymbolParams, + state: &WorldState, ) -> anyhow::Result>> { - symbols::symbols(¶ms) + symbols::symbols(¶ms, state) .map(|res| Some(res)) .or_else(|err| { // Missing doc: Why are we not propagating errors to the frontend? diff --git a/crates/ark/src/lsp/main_loop.rs b/crates/ark/src/lsp/main_loop.rs index ac5588cea..0ee4be1a3 100644 --- a/crates/ark/src/lsp/main_loop.rs +++ b/crates/ark/src/lsp/main_loop.rs @@ -281,7 +281,7 @@ impl GlobalState { respond(tx, || state_handlers::initialize(params, &mut self.lsp_state, &mut self.world), LspResponse::Initialize)?; }, LspRequest::WorkspaceSymbol(params) => { - respond(tx, || handlers::handle_symbol(params), LspResponse::WorkspaceSymbol)?; + respond(tx, || handlers::handle_symbol(params, &self.world), LspResponse::WorkspaceSymbol)?; }, LspRequest::DocumentSymbol(params) => { respond(tx, || handlers::handle_document_symbol(params, &self.world), LspResponse::DocumentSymbol)?; diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index df6d8b84d..d07e91700 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -56,7 +56,10 @@ fn new_symbol_node( symbol } -pub fn symbols(params: &WorkspaceSymbolParams) -> anyhow::Result> { +pub(crate) fn symbols( + params: &WorkspaceSymbolParams, + state: &WorldState, +) -> anyhow::Result> { let query = ¶ms.query; let mut info: Vec = Vec::new(); @@ -81,17 +84,19 @@ pub fn symbols(params: &WorkspaceSymbolParams) -> anyhow::Result { - info.push(SymbolInformation { - name: title.to_string(), - kind: SymbolKind::STRING, - location: Location { - uri: Url::from_file_path(path).unwrap(), - range: entry.range, - }, - tags: None, - deprecated: None, - container_name: None, - }); + if state.config.workspace_symbols.include_comment_sections { + info.push(SymbolInformation { + name: title.to_string(), + kind: SymbolKind::STRING, + location: Location { + uri: Url::from_file_path(path).unwrap(), + range: entry.range, + }, + tags: None, + deprecated: None, + container_name: None, + }); + } }, IndexEntryData::Variable { name } => { From 18bbff5e9090f4173f45a680aa4f7a6178f325cc Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 7 Jul 2025 13:55:17 +0200 Subject: [PATCH 2/8] Give priority to non-section entries --- crates/ark/src/lsp/indexer.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/ark/src/lsp/indexer.rs b/crates/ark/src/lsp/indexer.rs index 70176ca7c..6880f8bb7 100644 --- a/crates/ark/src/lsp/indexer.rs +++ b/crates/ark/src/lsp/indexer.rs @@ -126,10 +126,17 @@ fn insert(path: &Path, entry: IndexEntry) -> anyhow::Result<()> { let index = index.entry(path.to_string()).or_default(); - // Retain the first occurrence in the index. In the future we'll track every occurrences and - // their scopes but for now we only track the first definition of an object (in a way, its + // We generally retain only the first occurrence in the index. In the + // future we'll track every occurrences and their scopes but for now we + // only track the first definition of an object (in a way, its // declaration). - if !index.contains_key(&entry.key) { + if let Some(existing_entry) = index.get(&entry.key) { + // Give priority to non-section entries. + if matches!(existing_entry.data, IndexEntryData::Section { .. }) { + index.insert(entry.key.clone(), entry); + } + // Else, ignore. + } else { index.insert(entry.key.clone(), entry); } From f87b292018b4c6a03d662680f410c917b961f724 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 22 Jul 2025 15:15:55 +0200 Subject: [PATCH 3/8] Add tests for `insert()` --- Cargo.lock | 1 + crates/ark/Cargo.toml | 1 + crates/ark/src/lsp/indexer.rs | 72 ++++++++++++++++++++++++++++++++++- 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0f6dcd098..e926f3dd2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -283,6 +283,7 @@ dependencies = [ "actix-web", "amalthea", "anyhow", + "assert_matches", "async-trait", "base64 0.21.0", "bus", diff --git a/crates/ark/Cargo.toml b/crates/ark/Cargo.toml index 2a8b1b6e3..333a20e39 100644 --- a/crates/ark/Cargo.toml +++ b/crates/ark/Cargo.toml @@ -66,6 +66,7 @@ tracing-error = "0.2.0" insta = { version = "1.39.0" } stdext = { path = "../stdext", features = ["testing"] } tempfile = "3.13.0" +assert_matches = "1.5.0" [build-dependencies] cc = "1.1.22" diff --git a/crates/ark/src/lsp/indexer.rs b/crates/ark/src/lsp/indexer.rs index 6880f8bb7..5d0694f36 100644 --- a/crates/ark/src/lsp/indexer.rs +++ b/crates/ark/src/lsp/indexer.rs @@ -125,7 +125,12 @@ fn insert(path: &Path, entry: IndexEntry) -> anyhow::Result<()> { let path = str_from_path(path)?; let index = index.entry(path.to_string()).or_default(); + index_insert(index, entry); + Ok(()) +} + +fn index_insert(index: &mut HashMap, entry: IndexEntry) { // We generally retain only the first occurrence in the index. In the // future we'll track every occurrences and their scopes but for now we // only track the first definition of an object (in a way, its @@ -139,8 +144,6 @@ fn insert(path: &Path, entry: IndexEntry) -> anyhow::Result<()> { } else { index.insert(entry.key.clone(), entry); } - - Ok(()) } fn clear(path: &Path) -> anyhow::Result<()> { @@ -408,7 +411,9 @@ fn index_comment( mod tests { use std::path::PathBuf; + use assert_matches::assert_matches; use insta::assert_debug_snapshot; + use tower_lsp::lsp_types; use super::*; use crate::lsp::documents::Document; @@ -539,4 +544,67 @@ class <- R6::R6Class( "# ); } + + #[test] + fn test_index_insert_priority() { + let mut index = HashMap::new(); + + let section_entry = IndexEntry { + key: "foo".to_string(), + range: Range::new( + lsp_types::Position::new(0, 0), + lsp_types::Position::new(0, 3), + ), + data: IndexEntryData::Section { + level: 1, + title: "foo".to_string(), + }, + }; + + let variable_entry = IndexEntry { + key: "foo".to_string(), + range: Range::new( + lsp_types::Position::new(1, 0), + lsp_types::Position::new(1, 3), + ), + data: IndexEntryData::Variable { + name: "foo".to_string(), + }, + }; + + // The Variable has priority and should replace the Section + index_insert(&mut index, section_entry.clone()); + index_insert(&mut index, variable_entry.clone()); + assert_matches!( + &index.get("foo").unwrap().data, + IndexEntryData::Variable { name } => assert_eq!(name, "foo") + ); + + // Inserting a Section again with the same key does not override the Variable + index_insert(&mut index, section_entry.clone()); + assert_matches!( + &index.get("foo").unwrap().data, + IndexEntryData::Variable { name } => assert_eq!(name, "foo") + ); + + let function_entry = IndexEntry { + key: "foo".to_string(), + range: Range::new( + lsp_types::Position::new(2, 0), + lsp_types::Position::new(2, 3), + ), + data: IndexEntryData::Function { + name: "foo".to_string(), + arguments: vec!["a".to_string()], + }, + }; + + // Inserting another kind of variable (e.g., Function) with the same key + // does not override it either. The first occurrence is generally retained. + index_insert(&mut index, function_entry.clone()); + assert_matches!( + &index.get("foo").unwrap().data, + IndexEntryData::Variable { name } => assert_eq!(name, "foo") + ); + } } From 800dc7088ff6dfaf1223df4dcfcda738a412ec1a Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 22 Jul 2025 15:32:15 +0200 Subject: [PATCH 4/8] Add test for comment sections in workspace symbols --- crates/ark/src/lsp/indexer.rs | 6 ++++++ crates/ark/src/lsp/symbols.rs | 39 +++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/crates/ark/src/lsp/indexer.rs b/crates/ark/src/lsp/indexer.rs index 5d0694f36..b997e982c 100644 --- a/crates/ark/src/lsp/indexer.rs +++ b/crates/ark/src/lsp/indexer.rs @@ -158,6 +158,12 @@ fn clear(path: &Path) -> anyhow::Result<()> { Ok(()) } +#[cfg(test)] +pub(crate) fn indexer_clear() { + let mut index = WORKSPACE_INDEX.lock().unwrap(); + index.clear(); +} + fn str_from_path(path: &Path) -> anyhow::Result<&str> { path.to_str().ok_or(anyhow!( "Couldn't convert path {} to string", diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index d07e91700..146b1b80f 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -583,6 +583,8 @@ mod tests { use tower_lsp::lsp_types::Position; use super::*; + use crate::lsp::config::LspConfig; + use crate::lsp::config::WorkspaceSymbolsConfig; use crate::lsp::documents::Document; fn test_symbol(code: &str) -> Vec { @@ -894,4 +896,41 @@ a <- function() { insta::assert_debug_snapshot!(symbols); } + + #[test] + fn test_workspace_symbols_include_comment_sections() { + fn run(include_comment_sections: bool) -> Vec { + let code = "# Section ----\nfoo <- 1"; + + let mut config = LspConfig::default(); + config.workspace_symbols = WorkspaceSymbolsConfig { + include_comment_sections, + }; + let mut state = WorldState::default(); + state.config = config; + + // Index the document + let doc = Document::new(code, None); + indexer::update(&doc, std::path::Path::new("/test.R")).unwrap(); + + // Query for all symbols + let params = WorkspaceSymbolParams { + query: "Section".to_string(), + ..Default::default() + }; + let result = super::symbols(¶ms, &state).unwrap(); + let out = result.into_iter().map(|s| s.name).collect(); + + indexer::indexer_clear(); + out + } + + // Should include section when true + let with_sections = run(true); + assert!(with_sections.contains(&"Section".to_string())); + + // Should not include section when false + let without_sections = run(false); + assert!(!without_sections.contains(&"Section".to_string())); + } } From 22adbae4fdbf80a0001d3cd55951d48c2e918908 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 22 Jul 2025 15:55:25 +0200 Subject: [PATCH 5/8] Add simple test for `goto_definition()` --- crates/ark/src/lsp/definitions.rs | 56 ++++++++++++++++++++++++++++++- crates/ark/src/lsp/handlers.rs | 2 +- crates/ark/src/lsp/indexer.rs | 12 +++++++ crates/ark/src/lsp/symbols.rs | 4 ++- 4 files changed, 71 insertions(+), 3 deletions(-) diff --git a/crates/ark/src/lsp/definitions.rs b/crates/ark/src/lsp/definitions.rs index eb26e3dc7..f4eba3470 100644 --- a/crates/ark/src/lsp/definitions.rs +++ b/crates/ark/src/lsp/definitions.rs @@ -20,7 +20,7 @@ use crate::lsp::traits::node::NodeExt; use crate::lsp::traits::rope::RopeExt; use crate::treesitter::NodeTypeExt; -pub unsafe fn goto_definition<'a>( +pub fn goto_definition<'a>( document: &'a Document, params: GotoDefinitionParams, ) -> Result> { @@ -75,3 +75,57 @@ pub unsafe fn goto_definition<'a>( let response = GotoDefinitionResponse::Link(vec![link]); Ok(Some(response)) } + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use assert_matches::assert_matches; + use tower_lsp::lsp_types; + + use super::*; + use crate::lsp::documents::Document; + use crate::lsp::indexer; + + #[test] + fn test_goto_definition() { + // Reset the indexer on exit + let _guard = indexer::IndexerGuard; + + let code = r#" +foo <- 42 +print(foo) +"#; + let doc = Document::new(code, None); + let path = PathBuf::from("/foo/test.R"); + + indexer::update(&doc, &path).unwrap(); + + let params = GotoDefinitionParams { + text_document_position_params: lsp_types::TextDocumentPositionParams { + text_document: lsp_types::TextDocumentIdentifier { + uri: Url::from_file_path(&path).unwrap(), + }, + position: lsp_types::Position::new(2, 7), + }, + work_done_progress_params: Default::default(), + partial_result_params: Default::default(), + }; + + assert_matches!( + goto_definition(&doc, params).unwrap(), + Some(GotoDefinitionResponse::Link(ref links)) => { + assert!(!links.is_empty()); + assert_eq!(links[0].target_uri, Url::from_file_path(&path).unwrap()); + + assert_eq!( + links[0].target_range, + lsp_types::Range { + start: lsp_types::Position::new(1, 0), + end: lsp_types::Position::new(1, 3), + } + ); + } + ); + } +} diff --git a/crates/ark/src/lsp/handlers.rs b/crates/ark/src/lsp/handlers.rs index 4d54a91ad..9cc2f90e2 100644 --- a/crates/ark/src/lsp/handlers.rs +++ b/crates/ark/src/lsp/handlers.rs @@ -289,7 +289,7 @@ pub(crate) fn handle_goto_definition( let document = state.get_document(uri)?; // build goto definition context - let result = unwrap!(unsafe { goto_definition(&document, params) }, Err(err) => { + let result = unwrap!(goto_definition(&document, params), Err(err) => { lsp::log_error!("{err:?}"); return Ok(None); }); diff --git a/crates/ark/src/lsp/indexer.rs b/crates/ark/src/lsp/indexer.rs index b997e982c..074e2807c 100644 --- a/crates/ark/src/lsp/indexer.rs +++ b/crates/ark/src/lsp/indexer.rs @@ -164,6 +164,18 @@ pub(crate) fn indexer_clear() { index.clear(); } +/// RAII guard that clears `WORKSPACE_INDEX` when dropped. +/// Useful for ensuring a clean index state in tests. +#[cfg(test)] +pub(crate) struct IndexerGuard; + +#[cfg(test)] +impl Drop for IndexerGuard { + fn drop(&mut self) { + indexer_clear(); + } +} + fn str_from_path(path: &Path) -> anyhow::Result<&str> { path.to_str().ok_or(anyhow!( "Couldn't convert path {} to string", diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index 146b1b80f..2efa62a2a 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -586,6 +586,7 @@ mod tests { use crate::lsp::config::LspConfig; use crate::lsp::config::WorkspaceSymbolsConfig; use crate::lsp::documents::Document; + use crate::lsp::indexer::IndexerGuard; fn test_symbol(code: &str) -> Vec { let doc = Document::new(code, None); @@ -900,6 +901,8 @@ a <- function() { #[test] fn test_workspace_symbols_include_comment_sections() { fn run(include_comment_sections: bool) -> Vec { + let _guard = IndexerGuard; + let code = "# Section ----\nfoo <- 1"; let mut config = LspConfig::default(); @@ -921,7 +924,6 @@ a <- function() { let result = super::symbols(¶ms, &state).unwrap(); let out = result.into_iter().map(|s| s.name).collect(); - indexer::indexer_clear(); out } From 0eb735e068100f5e3428214c8023bd54c739fd52 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 22 Jul 2025 16:41:12 +0200 Subject: [PATCH 6/8] Add goto-definition test for section vs variable --- crates/ark/src/lsp/definitions.rs | 48 +++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/crates/ark/src/lsp/definitions.rs b/crates/ark/src/lsp/definitions.rs index f4eba3470..0b9939fe2 100644 --- a/crates/ark/src/lsp/definitions.rs +++ b/crates/ark/src/lsp/definitions.rs @@ -128,4 +128,52 @@ print(foo) } ); } + + #[test] + fn test_goto_definition_comment_section() { + // Reset the indexer on exit + let _guard = indexer::IndexerGuard; + + let code = r#" +# foo ---- +foo <- 1 +print(foo) +"#; + let doc = Document::new(code, None); + let path = PathBuf::from("/foo/section_test.R"); + indexer::update(&doc, &path).unwrap(); + + let params = lsp_types::GotoDefinitionParams { + text_document_position_params: lsp_types::TextDocumentPositionParams { + text_document: lsp_types::TextDocumentIdentifier { + uri: lsp_types::Url::from_file_path(&path).unwrap(), + }, + position: lsp_types::Position::new(3, 7), + }, + work_done_progress_params: Default::default(), + partial_result_params: Default::default(), + }; + + assert_matches!( + goto_definition(&doc, params).unwrap(), + Some(lsp_types::GotoDefinitionResponse::Link(ref links)) => { + assert!(!links.is_empty()); + + let link = &links[0]; + assert_eq!( + link.target_uri, + lsp_types::Url::from_file_path(&path).unwrap() + ); + + // The section should is not the target, the variable has priority + assert_eq!( + links[0].target_range, + lsp_types::Range { + start: lsp_types::Position::new(2, 0), + end: lsp_types::Position::new(2, 3), + } + ); + } + ); + } } From d05ea05c616430e74bf2880c4fff50dfe84418a0 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 22 Jul 2025 17:09:18 +0200 Subject: [PATCH 7/8] Make guard name more explicit --- crates/ark/src/lsp/definitions.rs | 6 ++---- crates/ark/src/lsp/indexer.rs | 4 ++-- crates/ark/src/lsp/symbols.rs | 4 ++-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/crates/ark/src/lsp/definitions.rs b/crates/ark/src/lsp/definitions.rs index 0b9939fe2..587cf4ea6 100644 --- a/crates/ark/src/lsp/definitions.rs +++ b/crates/ark/src/lsp/definitions.rs @@ -89,8 +89,7 @@ mod tests { #[test] fn test_goto_definition() { - // Reset the indexer on exit - let _guard = indexer::IndexerGuard; + let _guard = indexer::ResetIndexerGuard; let code = r#" foo <- 42 @@ -131,8 +130,7 @@ print(foo) #[test] fn test_goto_definition_comment_section() { - // Reset the indexer on exit - let _guard = indexer::IndexerGuard; + let _guard = indexer::ResetIndexerGuard; let code = r#" # foo ---- diff --git a/crates/ark/src/lsp/indexer.rs b/crates/ark/src/lsp/indexer.rs index 074e2807c..5954c5b23 100644 --- a/crates/ark/src/lsp/indexer.rs +++ b/crates/ark/src/lsp/indexer.rs @@ -167,10 +167,10 @@ pub(crate) fn indexer_clear() { /// RAII guard that clears `WORKSPACE_INDEX` when dropped. /// Useful for ensuring a clean index state in tests. #[cfg(test)] -pub(crate) struct IndexerGuard; +pub(crate) struct ResetIndexerGuard; #[cfg(test)] -impl Drop for IndexerGuard { +impl Drop for ResetIndexerGuard { fn drop(&mut self) { indexer_clear(); } diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index 2efa62a2a..000a8b021 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -586,7 +586,7 @@ mod tests { use crate::lsp::config::LspConfig; use crate::lsp::config::WorkspaceSymbolsConfig; use crate::lsp::documents::Document; - use crate::lsp::indexer::IndexerGuard; + use crate::lsp::indexer::ResetIndexerGuard; fn test_symbol(code: &str) -> Vec { let doc = Document::new(code, None); @@ -901,7 +901,7 @@ a <- function() { #[test] fn test_workspace_symbols_include_comment_sections() { fn run(include_comment_sections: bool) -> Vec { - let _guard = IndexerGuard; + let _guard = ResetIndexerGuard; let code = "# Section ----\nfoo <- 1"; From 40e9c0f9ea1058d80702cd8039e2e0088227f5f1 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 22 Jul 2025 17:20:38 +0200 Subject: [PATCH 8/8] Fix tests on Windows `lsp_types::Url::from_file_path()` returns `None` if not a valid path. Path should start with e.g. `C:\` on Windows. --- crates/ark/src/lsp/definitions.rs | 27 ++++++--------------------- crates/ark/src/lsp/symbols.rs | 4 +++- crates/ark/src/lsp/util.rs | 14 ++++++++++++++ 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/crates/ark/src/lsp/definitions.rs b/crates/ark/src/lsp/definitions.rs index 587cf4ea6..479676c57 100644 --- a/crates/ark/src/lsp/definitions.rs +++ b/crates/ark/src/lsp/definitions.rs @@ -78,14 +78,13 @@ pub fn goto_definition<'a>( #[cfg(test)] mod tests { - use std::path::PathBuf; - use assert_matches::assert_matches; use tower_lsp::lsp_types; use super::*; use crate::lsp::documents::Document; use crate::lsp::indexer; + use crate::lsp::util::test_path; #[test] fn test_goto_definition() { @@ -96,15 +95,13 @@ foo <- 42 print(foo) "#; let doc = Document::new(code, None); - let path = PathBuf::from("/foo/test.R"); + let (path, uri) = test_path(); indexer::update(&doc, &path).unwrap(); let params = GotoDefinitionParams { text_document_position_params: lsp_types::TextDocumentPositionParams { - text_document: lsp_types::TextDocumentIdentifier { - uri: Url::from_file_path(&path).unwrap(), - }, + text_document: lsp_types::TextDocumentIdentifier { uri }, position: lsp_types::Position::new(2, 7), }, work_done_progress_params: Default::default(), @@ -114,9 +111,6 @@ print(foo) assert_matches!( goto_definition(&doc, params).unwrap(), Some(GotoDefinitionResponse::Link(ref links)) => { - assert!(!links.is_empty()); - assert_eq!(links[0].target_uri, Url::from_file_path(&path).unwrap()); - assert_eq!( links[0].target_range, lsp_types::Range { @@ -138,14 +132,13 @@ foo <- 1 print(foo) "#; let doc = Document::new(code, None); - let path = PathBuf::from("/foo/section_test.R"); + let (path, uri) = test_path(); + indexer::update(&doc, &path).unwrap(); let params = lsp_types::GotoDefinitionParams { text_document_position_params: lsp_types::TextDocumentPositionParams { - text_document: lsp_types::TextDocumentIdentifier { - uri: lsp_types::Url::from_file_path(&path).unwrap(), - }, + text_document: lsp_types::TextDocumentIdentifier { uri }, position: lsp_types::Position::new(3, 7), }, work_done_progress_params: Default::default(), @@ -155,14 +148,6 @@ print(foo) assert_matches!( goto_definition(&doc, params).unwrap(), Some(lsp_types::GotoDefinitionResponse::Link(ref links)) => { - assert!(!links.is_empty()); - - let link = &links[0]; - assert_eq!( - link.target_uri, - lsp_types::Url::from_file_path(&path).unwrap() - ); - // The section should is not the target, the variable has priority assert_eq!( links[0].target_range, diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index 000a8b021..894f990fd 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -587,6 +587,7 @@ mod tests { use crate::lsp::config::WorkspaceSymbolsConfig; use crate::lsp::documents::Document; use crate::lsp::indexer::ResetIndexerGuard; + use crate::lsp::util::test_path; fn test_symbol(code: &str) -> Vec { let doc = Document::new(code, None); @@ -914,7 +915,8 @@ a <- function() { // Index the document let doc = Document::new(code, None); - indexer::update(&doc, std::path::Path::new("/test.R")).unwrap(); + let (path, _) = test_path(); + indexer::update(&doc, &path).unwrap(); // Query for all symbols let params = WorkspaceSymbolParams { diff --git a/crates/ark/src/lsp/util.rs b/crates/ark/src/lsp/util.rs index 5140e4bd1..2fa50896d 100644 --- a/crates/ark/src/lsp/util.rs +++ b/crates/ark/src/lsp/util.rs @@ -28,3 +28,17 @@ pub unsafe extern "C-unwind" fn ps_object_id(object: SEXP) -> anyhow::Result (std::path::PathBuf, url::Url) { + use std::path::PathBuf; + + let path = if cfg!(windows) { + PathBuf::from(r"C:\test.R") + } else { + PathBuf::from("/test.R") + }; + let uri = url::Url::from_file_path(&path).unwrap(); + + (path, uri) +}