diff --git a/Cargo.lock b/Cargo.lock index f9b9009fa5..d797a98ff0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2583,15 +2583,9 @@ version = "0.1.0" dependencies = [ "air_r_parser", "air_r_syntax", - "assert_matches", "biome_rowan", - "log", "oak_core", - "oak_db", - "oak_package_metadata", "oak_semantic", - "oak_sources", - "stdext", "url", ] @@ -2633,7 +2627,6 @@ dependencies = [ "air_r_parser", "air_r_syntax", "anyhow", - "assert_matches", "biome_rowan", "biome_text_size", "itertools 0.14.0", @@ -2642,7 +2635,6 @@ dependencies = [ "oak_index_vec", "oak_package_metadata", "oak_semantic", - "oak_sources", "rustc-hash", "salsa", "smallvec", diff --git a/crates/ark/src/lsp.rs b/crates/ark/src/lsp.rs index 0e63279696..76a6dd8555 100644 --- a/crates/ark/src/lsp.rs +++ b/crates/ark/src/lsp.rs @@ -19,7 +19,6 @@ pub mod document_context; pub mod events; pub mod folding_range; pub mod goto_definition; -pub mod goto_definition_legacy; pub mod handler; pub mod handlers; pub mod help; @@ -42,6 +41,9 @@ pub mod symbols; pub mod traits; pub mod util; +#[cfg(test)] +mod tests; + // These send LSP messages in a non-async and non-blocking way. // The LOG level is not timestamped so we're not using it. macro_rules! log_info { diff --git a/crates/ark/src/lsp/diagnostics.rs b/crates/ark/src/lsp/diagnostics.rs index a62c452d37..483ca87d5b 100644 --- a/crates/ark/src/lsp/diagnostics.rs +++ b/crates/ark/src/lsp/diagnostics.rs @@ -1654,7 +1654,7 @@ foo let package = Package::from_parts(PathBuf::from("/mock/path"), description, namespace); // Create a library with `mockpkg` installed - let library = Library::new(vec![], None).insert("mockpkg", package); + let library = Library::new(vec![]).insert("mockpkg", package); // Simulate a search path with `library` in scope let console_scopes = vec![vec!["library".to_string()]]; @@ -1769,7 +1769,7 @@ foo let package2 = Package::from_parts(PathBuf::from("/mock/path2"), description2, namespace2); - let library = Library::new(vec![], None) + let library = Library::new(vec![]) .insert("pkg1", package1) .insert("pkg2", package2); @@ -1827,7 +1827,7 @@ foo }; let package = Package::from_parts(PathBuf::from("/mock/path"), description, namespace); - let library = Library::new(vec![], None).insert("pkg", package); + let library = Library::new(vec![]).insert("pkg", package); let console_scopes = vec![vec!["require".to_string()]]; let state = WorldState { @@ -1858,7 +1858,7 @@ foo let palmerpenguins_pkg = Package::load_from_folder(palmerpenguins_dir.path()) .unwrap() .unwrap(); - let library = Library::new(vec![], None).insert("penguins", palmerpenguins_pkg); + let library = Library::new(vec![]).insert("penguins", palmerpenguins_pkg); // Simulate a world state with the penguins package installed and attached let mut state = DEFAULT_STATE.clone(); diff --git a/crates/ark/src/lsp/goto_definition.rs b/crates/ark/src/lsp/goto_definition.rs index ff65cc14ba..e1700985e6 100644 --- a/crates/ark/src/lsp/goto_definition.rs +++ b/crates/ark/src/lsp/goto_definition.rs @@ -1,19 +1,21 @@ use aether_lsp_utils::proto::from_proto; use aether_lsp_utils::proto::to_proto; -use anyhow::anyhow; use oak_ide::NavigationTarget; use stdext::result::ResultExt; use tower_lsp::lsp_types::GotoDefinitionParams; use tower_lsp::lsp_types::GotoDefinitionResponse; use tower_lsp::lsp_types::LocationLink; +use tower_lsp::lsp_types::Position; +use url::Url; use crate::lsp::document::Document; -use crate::lsp::state::WorldState; +use crate::lsp::indexer; +use crate::lsp::traits::node::NodeExt; +use crate::treesitter::NodeTypeExt; pub(crate) fn goto_definition( document: &Document, params: GotoDefinitionParams, - state: &WorldState, ) -> anyhow::Result> { let uri = params.text_document_position_params.text_document.uri; let position = params.text_document_position_params.position; @@ -24,44 +26,82 @@ pub(crate) fn goto_definition( document.position_encoding, )?; - let (index, scope) = state.file_analysis(&uri, document); + let index = document.semantic_index(); let root = document.syntax()?; - let targets = oak_ide::goto_definition(state, offset, &uri, &root, &index, &scope); - - if targets.is_empty() { - return Ok(None); - } + let pos = oak_ide::FilePosition { + file: uri.clone(), + offset, + }; + let targets = oak_ide::goto_definition(&index, &root, &pos); let links: Vec<_> = targets .into_iter() - .filter_map(|target| nav_target_to_link(target, state).log_err()) + .filter_map(|target| nav_target_to_link(target, document).log_err()) .collect(); - if links.is_empty() { + if !links.is_empty() { + return Ok(Some(GotoDefinitionResponse::Link(links))); + } + + // Within-file resolution found nothing. Fall back to the workspace + // indexer for a best-effort cross-file lookup of the identifier at the + // cursor. Non-identifier cursors (operators, parens, whitespace) return + // `None`. TODO(salsa): Replace the indexer step with a proper cross-file + // (file imports) lookup. + if let Some(link) = fallback_link_at(document, position, &uri)? { + return Ok(Some(GotoDefinitionResponse::Link(vec![link]))); + } + + Ok(None) +} + +/// Look up the identifier at `position` in the workspace indexer (current +/// file first, then other files). Returns `None` when the cursor isn't on +/// an identifier or the symbol isn't in the index. +fn fallback_link_at( + document: &Document, + position: Position, + uri: &Url, +) -> anyhow::Result> { + let point = document.tree_sitter_point_from_lsp_position(position)?; + let Some(node) = document.ast.root_node().find_closest_node_to_point(point) else { + log::warn!("Failed to find the closest node to point {point}."); + return Ok(None); + }; + + if !node.is_identifier() { return Ok(None); } - Ok(Some(GotoDefinitionResponse::Link(links))) + let symbol = node.node_as_str(&document.contents)?; + let Some((file_id, entry)) = + indexer::find_in_file(symbol, uri).or_else(|| indexer::find(symbol)) + else { + return Ok(None); + }; + + Ok(Some(LocationLink { + origin_selection_range: None, + target_uri: file_id.as_uri().clone(), + target_range: entry.range, + target_selection_range: entry.range, + })) } fn nav_target_to_link( target: NavigationTarget, - state: &WorldState, + document: &Document, ) -> anyhow::Result { - let doc = if let Some(open) = state.documents.get(&target.file) { - open - } else { - let path = target - .file - .to_file_path() - .map_err(|_| anyhow!("Can't convert URI to path: {}", target.file))?; - let contents = std::fs::read_to_string(&path)?; - &Document::new(&contents, None) - }; - - let target_range = to_proto::range(target.full_range, &doc.line_index, doc.position_encoding)?; - let target_selection_range = - to_proto::range(target.focus_range, &doc.line_index, doc.position_encoding)?; + let target_range = to_proto::range( + target.full_range, + &document.line_index, + document.position_encoding, + )?; + let target_selection_range = to_proto::range( + target.focus_range, + &document.line_index, + document.position_encoding, + )?; Ok(LocationLink { origin_selection_range: None, @@ -71,8 +111,17 @@ fn nav_target_to_link( }) } -#[cfg(test)] -mod tests { +// Parked cross-file goto-definition tests pending the Salsa Oak wiring. +// +// Snapshot of the old `LegacyDb`-based suite, trimmed to the cross-file +// cases that the within-file handler doesn't cover (collation, `source()`, +// `library()`, NAMESPACE, package scope, base/search path). Intra-file +// tests live in `crates/ark/src/lsp/tests/goto_definition.rs`. +// `#[cfg(any())]` skips the block at compile time so the references to +// deleted types (`LegacyDb`, `ExternalScope`, `ScopeLayer`, ...) stay as +// the behavioural spec to re-implement on top of `oak_db::File::resolve_at`. +#[cfg(any())] +mod parked_salsa_tests { use std::path::PathBuf; use std::process::Command; @@ -119,96 +168,6 @@ mod tests { state } - #[test] - fn test_goto_definition() { - let code = "foo <- 42\nprint(foo)\n"; - let doc = Document::new(code, None); - let uri = test_path("test.R"); - let state = make_state(&uri, &doc); - - let params = make_params(uri, 1, 6); - - assert_matches!( - goto_definition(&doc, params, &state).unwrap(), - Some(GotoDefinitionResponse::Link(ref links)) => { - assert_eq!( - links[0].target_range, - lsp_types::Range { - start: lsp_types::Position::new(0, 0), - end: lsp_types::Position::new(0, 3), - } - ); - } - ); - } - - #[test] - fn test_goto_definition_prefers_local_symbol() { - let code = "foo <- 1\nfoo\n"; - let doc = Document::new(code, None); - let uri = test_path("file.R"); - let state = make_state(&uri, &doc); - - let params = make_params(uri.clone(), 1, 0); - - assert_matches!( - goto_definition(&doc, params, &state).unwrap(), - Some(GotoDefinitionResponse::Link(ref links)) => { - assert_eq!(links[0].target_uri, uri); - assert_eq!( - links[0].target_range, - lsp_types::Range { - start: lsp_types::Position::new(0, 0), - end: lsp_types::Position::new(0, 3), - } - ); - } - ); - } - - #[test] - fn test_goto_definition_no_use_returns_none() { - let code = "x <- 1\n"; - let doc = Document::new(code, None); - let uri = test_path("test.R"); - let state = make_state(&uri, &doc); - - let params = make_params(uri, 0, 3); - let result = goto_definition(&doc, params, &state).unwrap(); - assert_eq!(result, None); - } - - #[test] - fn test_goto_definition_unresolved_returns_none() { - let code = "foo\n"; - let doc = Document::new(code, None); - let uri = test_path("test.R"); - let state = make_state(&uri, &doc); - - let params = make_params(uri, 0, 0); - let result = goto_definition(&doc, params, &state).unwrap(); - assert_eq!(result, None); - } - - #[test] - fn test_other_file_not_visible_without_scope_chain() { - // file2 uses `foo` but file1's definition is not in the scope chain, - // so it should not resolve. - let doc1 = Document::new("foo <- 1\n", None); - let uri1 = test_path("file1.R"); - - let doc2 = Document::new("foo\n", None); - let uri2 = test_path("file2.R"); - - let mut state = WorldState::default(); - state.documents.insert(uri1, doc1); - state.documents.insert(uri2.clone(), doc2.clone()); - - let params = make_params(uri2, 0, 0); - let result = goto_definition(&doc2, params, &state).unwrap(); - assert_eq!(result, None); - } - #[test] fn test_package_import_from_resolves() { // A package with `importFrom(dplyr, mutate)` should make `mutate` diff --git a/crates/ark/src/lsp/goto_definition_legacy.rs b/crates/ark/src/lsp/goto_definition_legacy.rs deleted file mode 100644 index 5dce1365ca..0000000000 --- a/crates/ark/src/lsp/goto_definition_legacy.rs +++ /dev/null @@ -1,268 +0,0 @@ -// -// definitions.rs -// -// Copyright (C) 2022 Posit Software, PBC. All rights reserved. -// -// - -use anyhow::Result; -use tower_lsp::lsp_types::GotoDefinitionParams; -use tower_lsp::lsp_types::GotoDefinitionResponse; -use tower_lsp::lsp_types::LocationLink; -use tower_lsp::lsp_types::Range; - -use crate::lsp::document::Document; -use crate::lsp::indexer; -use crate::lsp::traits::node::NodeExt; -use crate::treesitter::NodeTypeExt; - -pub fn goto_definition( - document: &Document, - params: GotoDefinitionParams, -) -> Result> { - // get reference to AST - let ast = &document.ast; - - // try to find node at position - let position = params.text_document_position_params.position; - let point = document.tree_sitter_point_from_lsp_position(position)?; - - let Some(node) = ast.root_node().find_closest_node_to_point(point) else { - log::warn!("Failed to find the closest node to point {point}."); - return Ok(None); - }; - - let start = document.lsp_position_from_tree_sitter_point(node.start_position())?; - let end = document.lsp_position_from_tree_sitter_point(node.end_position())?; - let range = Range { start, end }; - - // Search for a reference in the document index - if node.is_identifier() { - let symbol = node.node_as_str(&document.contents)?; - - // First search in current file, then in all files - let uri = ¶ms.text_document_position_params.text_document.uri; - let info = indexer::find_in_file(symbol, uri).or_else(|| indexer::find(symbol)); - - if let Some((file_id, entry)) = info { - let target_uri = file_id.as_uri().clone(); - let link = LocationLink { - origin_selection_range: None, - target_uri, - target_range: entry.range, - target_selection_range: entry.range, - }; - let response = GotoDefinitionResponse::Link(vec![link]); - return Ok(Some(response)); - } - } - - // TODO: We should see if we can find the referenced item in: - // - // 1. The document's current AST, - // 2. The public functions from other documents in the project, - // 3. A definition in the R session (which we could open in a virtual document) - // - // If we can't find a definition, then we can return the referenced item itself, - // which will tell Positron to instead try to look for references for that symbol. - let link = LocationLink { - origin_selection_range: Some(range), - target_uri: params.text_document_position_params.text_document.uri, - target_range: range, - target_selection_range: range, - }; - - let response = GotoDefinitionResponse::Link(vec![link]); - Ok(Some(response)) -} - -#[cfg(test)] -mod tests { - use assert_matches::assert_matches; - use tower_lsp::lsp_types; - - use super::*; - use crate::lsp::document::Document; - use crate::lsp::indexer; - use crate::lsp::util::test_path; - - #[test] - fn test_goto_definition() { - let _guard = indexer::ResetIndexerGuard; - - let code = r#" -foo <- 42 -print(foo) -"#; - let doc = Document::new(code, None); - let uri = test_path("test.R"); - - indexer::update(&doc, &uri).unwrap(); - - let params = GotoDefinitionParams { - text_document_position_params: lsp_types::TextDocumentPositionParams { - text_document: lsp_types::TextDocumentIdentifier { uri }, - 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_eq!( - links[0].target_range, - lsp_types::Range { - start: lsp_types::Position::new(1, 0), - end: lsp_types::Position::new(1, 3), - } - ); - } - ); - } - - #[test] - fn test_goto_definition_comment_section() { - let _guard = indexer::ResetIndexerGuard; - - let code = r#" -# foo ---- -foo <- 1 -print(foo) -"#; - let doc = Document::new(code, None); - let uri = test_path("test.R"); - - indexer::update(&doc, &uri).unwrap(); - - let params = lsp_types::GotoDefinitionParams { - text_document_position_params: lsp_types::TextDocumentPositionParams { - text_document: lsp_types::TextDocumentIdentifier { uri }, - 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)) => { - // 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), - } - ); - } - ); - } - - #[test] - fn test_goto_definition_prefers_local_symbol() { - let _guard = indexer::ResetIndexerGuard; - - // Both files define the same symbol - let code1 = r#" -foo <- 1 -foo -"#; - let code2 = r#" -foo <- 2 -foo -"#; - - let doc1 = Document::new(code1, None); - let doc2 = Document::new(code2, None); - - let uri1 = test_path("file1.R"); - let uri2 = test_path("file2.R"); - - indexer::update(&doc1, &uri1).unwrap(); - indexer::update(&doc2, &uri2).unwrap(); - - // Go to definition for foo in file1 - let params1 = GotoDefinitionParams { - text_document_position_params: lsp_types::TextDocumentPositionParams { - text_document: lsp_types::TextDocumentIdentifier { uri: uri1.clone() }, - position: lsp_types::Position::new(2, 0), - }, - work_done_progress_params: Default::default(), - partial_result_params: Default::default(), - }; - assert_matches!( - goto_definition(&doc1, params1).unwrap(), - Some(GotoDefinitionResponse::Link(ref links)) => { - // Should jump to foo in file1 - assert_eq!(links[0].target_uri, uri1); - } - ); - - // Go to definition for foo in file2 - let params2 = GotoDefinitionParams { - text_document_position_params: lsp_types::TextDocumentPositionParams { - text_document: lsp_types::TextDocumentIdentifier { uri: uri2.clone() }, - position: lsp_types::Position::new(2, 0), - }, - work_done_progress_params: Default::default(), - partial_result_params: Default::default(), - }; - assert_matches!( - goto_definition(&doc2, params2).unwrap(), - Some(GotoDefinitionResponse::Link(ref links)) => { - // Should jump to foo in file2 - assert_eq!(links[0].target_uri, uri2); - } - ); - } - - #[test] - fn test_goto_definition_falls_back_to_other_file() { - let _guard = indexer::ResetIndexerGuard; - - // file1 defines foo, file2 does not - let code1 = r#" -foo <- 1 -"#; - let code2 = r#" -foo -"#; - - let doc1 = Document::new(code1, None); - let doc2 = Document::new(code2, None); - - // Use test_path for cross-platform compatibility - let uri1 = test_path("file1.R"); - let uri2 = test_path("file2.R"); - - indexer::update(&doc1, &uri1).unwrap(); - indexer::update(&doc2, &uri2).unwrap(); - - // Go to definition for foo in file2 (should jump to file1) - let params2 = GotoDefinitionParams { - text_document_position_params: lsp_types::TextDocumentPositionParams { - text_document: lsp_types::TextDocumentIdentifier { uri: uri2.clone() }, - position: lsp_types::Position::new(1, 0), - }, - work_done_progress_params: Default::default(), - partial_result_params: Default::default(), - }; - let result2 = goto_definition(&doc2, params2).unwrap(); - assert_matches!( - result2, - Some(GotoDefinitionResponse::Link(ref links)) => { - // Should jump to foo in file1 - assert_eq!(links[0].target_uri, uri1); - 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 c86e08a057..7431ea6b98 100644 --- a/crates/ark/src/lsp/handlers.rs +++ b/crates/ark/src/lsp/handlers.rs @@ -50,7 +50,6 @@ use crate::lsp::document_context::DocumentContext; use crate::lsp::find_references::find_references; use crate::lsp::folding_range::folding_range; use crate::lsp::goto_definition::goto_definition; -use crate::lsp::goto_definition_legacy; use crate::lsp::help_topic::help_topic; use crate::lsp::help_topic::HelpTopicParams; use crate::lsp::help_topic::HelpTopicResponse; @@ -279,13 +278,7 @@ pub(crate) fn handle_goto_definition( let uri = ¶ms.text_document_position_params.text_document.uri; let document = state.get_document(uri)?; - if std::env::var("ARK_OAK_GOTO_DEF").is_ok() { - Ok(goto_definition(document, params, state).log_err().flatten()) - } else { - Ok(goto_definition_legacy::goto_definition(document, params) - .log_err() - .flatten()) - } + Ok(goto_definition(document, params).log_err().flatten()) } #[tracing::instrument(level = "info", skip_all)] diff --git a/crates/ark/src/lsp/main_loop.rs b/crates/ark/src/lsp/main_loop.rs index daf65c0667..fbc1ab3c7f 100644 --- a/crates/ark/src/lsp/main_loop.rs +++ b/crates/ark/src/lsp/main_loop.rs @@ -12,7 +12,6 @@ use std::path::PathBuf; use std::pin::Pin; use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; -use std::sync::Arc; use std::sync::LazyLock; use std::sync::RwLock; @@ -20,8 +19,6 @@ use anyhow::anyhow; use futures::stream::FuturesUnordered; use futures::StreamExt; use oak_semantic::library::Library; -use oak_sources::PackageCache; -use stdext::result::ResultExt; use tokio::sync::mpsc; use tokio::sync::mpsc::unbounded_channel as tokio_unbounded_channel; use tokio::task; @@ -189,7 +186,7 @@ impl GlobalState { /// and auxiliary loop. pub(crate) fn new( client: Client, - r_home: PathBuf, + _r_home: PathBuf, console_notification_tx: TokioUnboundedSender, ) -> Self { // Transmission channel for the main loop events. Shared with the @@ -219,12 +216,7 @@ impl GlobalState { let library_paths: Vec = library_paths.into_iter().map(PathBuf::from).collect(); - let r = harp::command::r_executable(&r_home); - let package_sources = r - .and_then(|r| PackageCache::new(r, library_paths.clone()).log_err()) - .map(|cache| Arc::new(cache) as Arc); - - let library = Library::new(library_paths, package_sources); + let library = Library::new(library_paths); Self { world: WorldState::new(library), diff --git a/crates/ark/src/lsp/state.rs b/crates/ark/src/lsp/state.rs index 7f529caedc..7a728e7f5c 100644 --- a/crates/ark/src/lsp/state.rs +++ b/crates/ark/src/lsp/state.rs @@ -1,48 +1,14 @@ -use std::borrow::Cow; use std::collections::HashMap; -use std::collections::HashSet; use std::path::Path; -use std::path::PathBuf; use anyhow::anyhow; -use oak_core::file::list_r_files; -use oak_db::semantic_index_with_source_resolver; -use oak_db::LegacyDb; -use oak_ide::ExternalScope; use oak_semantic::library::Library; -use oak_semantic::scope_layer::default_search_path; -use oak_semantic::scope_layer::file_layers; -use oak_semantic::scope_layer::package_root_layers; -use oak_semantic::semantic_index::SemanticIndex; -use oak_semantic::SourceResolution; -use stdext::result::ResultExt; use url::Url; use crate::lsp::config::LspConfig; use crate::lsp::document::Document; use crate::lsp::inputs::source_root::SourceRoot; -impl LegacyDb for WorldState { - fn semantic_index(&self, file: &Url) -> Option { - let doc = self.workspace_document(file)?; - let source_root = self.source_root(file); - - let mut stack = HashSet::new(); - stack.insert(file.clone()); - - let index = semantic_index_with_source_resolver(&doc.parse.tree(), |path| { - let dir = source_root.as_ref()?; - self.resolve_source(dir, path, &mut stack) - }); - - Some(index) - } - - fn library(&self) -> &Library { - &self.library - } -} - #[derive(Clone, Default, Debug)] /// The world state, i.e. all the inputs necessary for analysing or refactoring /// code. This is a pure value. There is no interior mutability in this data @@ -122,197 +88,6 @@ impl WorldState { Err(anyhow!("Can't find document for URI {uri}")) } } - - /// Look up a document by URL: returns an open document if available, - /// otherwise reads from disk. - /// - /// TODO(salsa): Replace with a proper VFS so non-opened workspace documents - /// are cached rather than re-read on every query. - pub(crate) fn workspace_document(&self, uri: &Url) -> Option> { - if let Some(doc) = self.documents.get(uri) { - return Some(Cow::Borrowed(doc)); - } - - let path = uri.to_file_path().log_err()?; - let contents = std::fs::read_to_string(&path).log_err()?; - Some(Cow::Owned(Document::new(&contents, None))) - } - - /// Resolve from the workspace root, falling back to the file's directory. - /// This matches RStudio behaviour. Potential improvements: support - /// `setwd()` calls and find the workspace containing the file if there are - /// multiple. - fn source_root(&self, file: &Url) -> Option { - self.workspace - .folders - .first() - .and_then(|url| url.to_file_path().ok()) - .or_else(|| { - file.to_file_path() - .ok() - .and_then(|p| p.parent().map(|d| d.to_path_buf())) - }) - } - - /// Create the semantic index and scope chain for a particular file. - /// - /// For scripts, the index is built with a source resolver so that - /// `source()` directives carry the sourced file's exports. - /// For packages, cross-file visibility comes from NAMESPACE imports and - /// collation ordering. - pub(crate) fn file_analysis( - &self, - file: &Url, - doc: &Document, - ) -> (SemanticIndex, ExternalScope) { - match self.root { - Some(SourceRoot::Package(ref pkg)) => self.package_file_analysis(file, doc, pkg), - _ => self.script_file_analysis(file, doc), - } - } - - // TODO(salsa): Align with the deferred-resolution model used for scripts. Package - // files should produce Import-kind forwarding definitions and resolve - // through `resolve_definition()`, rather than pre-resolving through the - // scope chain. This will happen naturally once Salsa provides cheap - // cross-file index access. - fn package_file_analysis( - &self, - file: &Url, - doc: &Document, - pkg: &oak_semantic::package::Package, - ) -> (SemanticIndex, ExternalScope) { - let root_layers = package_root_layers(pkg.namespace()); - - // FIXME: This only works for source packages. If we do #1168, then the - // `DESCRIPTION` of an installed package won't live alongside its cached `R/` - // sources. - let r_dir = pkg.path().join("R"); - - // If there is a collation field, we use it as an authoritative source - // for the files in the package (and the order in which they are loaded) - let ordered: Vec = pkg - .description() - .collate() - .map(|names| names.into_iter().map(|n| r_dir.join(n)).collect()) - .unwrap_or_else(|| { - // No collation field, list R files and sort in C order - // (R's default collation) - let mut paths = list_r_files(&r_dir); - paths.sort(); - paths - }); - - let Some(file_path) = file.to_file_path().ok() else { - return (doc.semantic_index(), ExternalScope::default()); - }; - - // Iterate in reverse collation order so later files (which shadow - // earlier ones) come first in the chain. Split at the current file - // to separate predecessors from the full set. - let mut top_level = Vec::new(); - let mut lazy = Vec::new(); - let mut past_current = false; - - for path in ordered.iter().rev() { - if *path == file_path { - past_current = true; - continue; - } - - let Some(uri) = Url::from_file_path(path).log_err() else { - continue; - }; - let Some(doc) = self.workspace_document(&uri) else { - continue; - }; - - let layers = file_layers(uri.clone(), &doc.semantic_index()); - lazy.extend(layers.clone()); - if past_current { - top_level.extend(layers); - } - } - - top_level.extend(root_layers.clone()); - lazy.extend(root_layers); - - ( - doc.semantic_index(), - ExternalScope::package(top_level, lazy), - ) - } - - fn script_file_analysis(&self, file: &Url, doc: &Document) -> (SemanticIndex, ExternalScope) { - let source_root = self.source_root(file); - - let mut stack = HashSet::new(); - stack.insert(file.clone()); - - let index = semantic_index_with_source_resolver(&doc.parse.tree(), |path| { - let dir = source_root.as_ref()?; - self.resolve_source(dir, path, &mut stack) - }); - - let semantic_calls = index.semantic_calls().to_vec(); - ( - index, - ExternalScope::search_path(semantic_calls, default_search_path()), - ) - } - - /// Resolve a `source()` call into a [`SourceResolution`] containing the - /// sourced file's exported names and `library()` package attachments. - /// - /// `stack` tracks files currently being resolved (grey set) to break - /// cycles. A file is added when resolution starts and removed when it - /// finishes, so shared dependencies (diamond patterns) are resolved - /// independently for each parent. - fn resolve_source( - &self, - base_dir: &Path, - path: &str, - stack: &mut HashSet, - ) -> Option { - let resolved = base_dir.join(path); - let url = Url::from_file_path(&resolved).log_err()?; - - if !stack.insert(url.clone()) { - return None; - } - - let Some(sourced_doc) = self.workspace_document(&url) else { - stack.remove(&url); - return None; - }; - - // Build the sourced file's index with a nested resolver so that - // transitive `source()` calls are also resolved. The base - // directory stays the same (workspace root) throughout the chain. - let index = semantic_index_with_source_resolver(&sourced_doc.parse.tree(), |nested_path| { - self.resolve_source(base_dir, nested_path, stack) - }); - - let names: Vec = index - .exports() - .keys() - .map(|name| name.to_string()) - .collect(); - - let packages: Vec = index - .attached_packages() - .into_iter() - .map(|s| s.to_string()) - .collect(); - - stack.remove(&url); - - Some(SourceResolution { - url, - names, - packages, - }) - } } pub(crate) fn with_document( @@ -352,75 +127,3 @@ pub(crate) fn workspace_uris(state: &WorldState) -> Vec { let uris: Vec = state.documents.iter().map(|elt| elt.0.clone()).collect(); uris } - -#[cfg(test)] -mod tests { - use biome_rowan::TextSize; - use oak_semantic::scope_layer::ScopeLayer; - use stdext::assert_not; - - use super::*; - use crate::lsp::document::Document; - use crate::lsp::util::test_path; - - fn make_state(uri: &Url, doc: &Document) -> WorldState { - let mut state = WorldState::default(); - state.documents.insert(uri.clone(), doc.clone()); - state - } - - fn has_package(layers: &[ScopeLayer], name: &str) -> bool { - layers - .iter() - .any(|l| matches!(l, ScopeLayer::PackageExports(p) if p == name)) - } - - #[test] - fn test_script_library_directive_position_sensitive() { - // At top-level, `library()` is position-sensitive: code before - // the call should not see the package, code after should. - // The lazy scope (used for completions etc.) sees all directives. - let code = "inform('hi')\nlibrary(rlang)\ninform('hello')\n"; - let doc = Document::new(code, None); - let uri = test_path("script.R"); - let state = make_state(&uri, &doc); - - let (index, scope) = state.file_analysis(&uri, &doc); - - let before = scope.at(&index, TextSize::from(0)); - assert_not!(has_package(&before, "rlang")); - - let after = scope.at(&index, TextSize::from(code.rfind("inform").unwrap() as u32)); - assert!(has_package(&after, "rlang")); - - assert!(has_package(&scope.lazy(), "rlang")); - } - - #[test] - fn test_script_library_directive_visible_in_function_before_call() { - // Function bodies see all directives regardless of position, - // because the function will typically be called after the - // script has been fully sourced. - let code = "f <- function() inform('hello')\nlibrary(rlang)\n"; - let doc = Document::new(code, None); - let uri = test_path("script.R"); - let state = make_state(&uri, &doc); - - let (index, scope) = state.file_analysis(&uri, &doc); - - let in_function = scope.at(&index, TextSize::from(code.find("inform").unwrap() as u32)); - assert!(has_package(&in_function, "rlang")); - } - - #[test] - fn test_script_without_library_no_extra_packages() { - let code = "inform('hello')\n"; - let doc = Document::new(code, None); - let uri = test_path("script.R"); - let state = make_state(&uri, &doc); - - let (_index, scope) = state.file_analysis(&uri, &doc); - let layers = scope.lazy(); - assert_not!(has_package(&layers, "rlang")); - } -} diff --git a/crates/ark/src/lsp/tests.rs b/crates/ark/src/lsp/tests.rs new file mode 100644 index 0000000000..a0e6099473 --- /dev/null +++ b/crates/ark/src/lsp/tests.rs @@ -0,0 +1 @@ +mod goto_definition; diff --git a/crates/ark/src/lsp/tests/goto_definition.rs b/crates/ark/src/lsp/tests/goto_definition.rs new file mode 100644 index 0000000000..0fe97a83f2 --- /dev/null +++ b/crates/ark/src/lsp/tests/goto_definition.rs @@ -0,0 +1,151 @@ +use assert_matches::assert_matches; +use tower_lsp::lsp_types; +use tower_lsp::lsp_types::GotoDefinitionParams; +use tower_lsp::lsp_types::GotoDefinitionResponse; + +use crate::lsp::document::Document; +use crate::lsp::goto_definition::goto_definition; +use crate::lsp::indexer; +use crate::lsp::util::test_path; + +fn make_params(uri: lsp_types::Url, line: u32, character: u32) -> GotoDefinitionParams { + GotoDefinitionParams { + text_document_position_params: lsp_types::TextDocumentPositionParams { + text_document: lsp_types::TextDocumentIdentifier { uri }, + position: lsp_types::Position::new(line, character), + }, + work_done_progress_params: Default::default(), + partial_result_params: Default::default(), + } +} + +#[test] +fn test_goto_definition() { + let code = "foo <- 42\nprint(foo)\n"; + let doc = Document::new(code, None); + let uri = test_path("test.R"); + + let params = make_params(uri, 1, 6); + + assert_matches!( + goto_definition(&doc, params).unwrap(), + Some(GotoDefinitionResponse::Link(ref links)) => { + assert_eq!( + links[0].target_range, + lsp_types::Range { + start: lsp_types::Position::new(0, 0), + end: lsp_types::Position::new(0, 3), + } + ); + } + ); +} + +#[test] +fn test_goto_definition_prefers_local_symbol() { + let code = "foo <- 1\nfoo\n"; + let doc = Document::new(code, None); + let uri = test_path("file.R"); + + let params = make_params(uri.clone(), 1, 0); + + assert_matches!( + goto_definition(&doc, params).unwrap(), + Some(GotoDefinitionResponse::Link(ref links)) => { + assert_eq!(links[0].target_uri, uri); + assert_eq!( + links[0].target_range, + lsp_types::Range { + start: lsp_types::Position::new(0, 0), + end: lsp_types::Position::new(0, 3), + } + ); + } + ); +} + +#[test] +fn test_unbound_identifier_returns_none() { + // A free identifier with no local def and no indexer entry returns + // `None`, matching how rust-analyzer and ty handle the same case. + let _guard = indexer::ResetIndexerGuard; + + let doc = Document::new("foo\n", None); + let uri = test_path("file.R"); + + let params = make_params(uri, 0, 0); + assert_eq!(goto_definition(&doc, params).unwrap(), None); +} + +#[test] +fn test_cursor_on_operator_returns_none() { + // Cursor on `<-` (a non-identifier token): even though the file has + // an identifier nearby, the operator itself doesn't resolve to + // anything, so the response is `None`. + let _guard = indexer::ResetIndexerGuard; + + let other = Document::new("foo <- function() 'other'\n", None); + let other_uri = test_path("other.R"); + indexer::update(&other, &other_uri).unwrap(); + + let doc = Document::new("foo <- 1\n", None); + let uri = test_path("file.R"); + + // Cursor on the `<` of `<-` at column 4. + let params = make_params(uri, 0, 4); + assert_eq!(goto_definition(&doc, params).unwrap(), None); +} + +#[test] +fn test_fallback_resolves_cross_file() { + // A free variable that the intra-file resolver can't bind falls back + // to the workspace indexer, which finds the definition in another + // indexed file. + let _guard = indexer::ResetIndexerGuard; + + let doc1 = Document::new("foo <- function() 1\n", None); + let uri1 = test_path("defs.R"); + indexer::update(&doc1, &uri1).unwrap(); + + let doc2 = Document::new("foo\n", None); + let uri2 = test_path("uses.R"); + + let params = make_params(uri2, 0, 0); + assert_matches!( + goto_definition(&doc2, params).unwrap(), + Some(GotoDefinitionResponse::Link(ref links)) => { + assert_eq!(links.len(), 1); + assert_eq!(links[0].target_uri, uri1); + } + ); +} + +#[test] +fn test_fallback_skipped_when_local_def_wins() { + // When the symbol has a binding within the file, the within-file + // result is returned and the indexer fallback isn't consulted. + let _guard = indexer::ResetIndexerGuard; + + let other = Document::new("foo <- function() 'other'\n", None); + let other_uri = test_path("other.R"); + indexer::update(&other, &other_uri).unwrap(); + + let code = "foo <- 1\nfoo\n"; + let doc = Document::new(code, None); + let uri = test_path("main.R"); + + let params = make_params(uri.clone(), 1, 0); + assert_matches!( + goto_definition(&doc, params).unwrap(), + Some(GotoDefinitionResponse::Link(ref links)) => { + assert_eq!(links[0].target_uri, uri); + assert_eq!( + links[0].target_range, + lsp_types::Range { + start: lsp_types::Position::new(0, 0), + end: lsp_types::Position::new(0, 3), + } + ); + } + ); +} diff --git a/crates/oak_db/src/legacy.rs b/crates/oak_db/src/legacy.rs deleted file mode 100644 index b7c7a47e0b..0000000000 --- a/crates/oak_db/src/legacy.rs +++ /dev/null @@ -1,51 +0,0 @@ -use aether_syntax::RRoot; -use oak_semantic::library::Library; -use oak_semantic::semantic_index::SemanticIndex; -use oak_semantic::ImportsResolver; -use oak_semantic::SourceResolution; -use url::Url; - -/// Legacy database trait used by the tree-sitter backed code path. -/// -/// Exists only during the Salsa migration. -pub trait LegacyDb { - /// `None` means the file disappeared between index-build and query time, - /// which is an edge case, not a normal path. With Salsa inputs this - /// becomes infallible. - fn semantic_index(&self, file: &Url) -> Option; - fn library(&self) -> &Library; -} - -/// Build a [`SemanticIndex`] with cross-file `source()` resolution -/// driven by a `FnMut` callback. -/// -/// Used by `ark::lsp::state::WorldState`'s tree-sitter backed -/// resolver, which closes over LSP state (workspace document map, -/// cycle-detection stack) to satisfy [`ImportsResolver`] without -/// defining a struct. -/// -/// TODO(salsa): retire alongside the LSP migration to oak_db's -/// `File::semantic_index` tracked query. Once `ark::lsp::state` -/// switches to consuming the tracked query (or implements its own -/// [`ImportsResolver`] directly), this function and its private -/// [`CallbackImportsResolver`] adapter can be deleted. -pub fn semantic_index_with_source_resolver( - root: &RRoot, - resolver: impl FnMut(&str) -> Option, -) -> SemanticIndex { - let resolver = CallbackImportsResolver(resolver); - oak_semantic::build_index(root, resolver) -} - -struct CallbackImportsResolver(F) -where - F: FnMut(&str) -> Option; - -impl ImportsResolver for CallbackImportsResolver -where - F: FnMut(&str) -> Option, -{ - fn resolve_source(&mut self, path: &str) -> Option { - (self.0)(path) - } -} diff --git a/crates/oak_db/src/lib.rs b/crates/oak_db/src/lib.rs index 3e0e8a306a..b8879a9f4d 100644 --- a/crates/oak_db/src/lib.rs +++ b/crates/oak_db/src/lib.rs @@ -6,7 +6,6 @@ mod file_imports; mod file_resolve; mod imports; mod inputs; -mod legacy; mod name; mod parse; mod storage; @@ -27,7 +26,5 @@ pub use inputs::Package; pub use inputs::Root; pub use inputs::RootKind; pub use inputs::WorkspaceRoots; -pub use legacy::semantic_index_with_source_resolver; -pub use legacy::LegacyDb; pub use name::Name; pub use storage::OakDatabase; diff --git a/crates/oak_ide/Cargo.toml b/crates/oak_ide/Cargo.toml index c362a4efc9..22090554a3 100644 --- a/crates/oak_ide/Cargo.toml +++ b/crates/oak_ide/Cargo.toml @@ -16,17 +16,10 @@ workspace = true [dependencies] aether_syntax.workspace = true biome_rowan.workspace = true -log.workspace = true oak_core.workspace = true -oak_db.workspace = true oak_semantic.workspace = true -oak_sources.workspace = true -stdext.workspace = true url.workspace = true [dev-dependencies] aether_parser.workspace = true -assert_matches.workspace = true -oak_package_metadata.workspace = true oak_semantic = { workspace = true, features = ["testing"] } -oak_sources = { workspace = true, features = ["testing"] } diff --git a/crates/oak_ide/src/external_scope.rs b/crates/oak_ide/src/external_scope.rs deleted file mode 100644 index f3605457ef..0000000000 --- a/crates/oak_ide/src/external_scope.rs +++ /dev/null @@ -1,136 +0,0 @@ -use std::borrow::Cow; - -use biome_rowan::TextSize; -use oak_semantic::scope_layer::ScopeLayer; -use oak_semantic::semantic_index::ScopeKind; -use oak_semantic::semantic_index::SemanticCall; -use oak_semantic::semantic_index::SemanticCallKind; -use oak_semantic::semantic_index::SemanticIndex; -use oak_semantic::ScopeId; - -/// The external scope chain for a file, determined by its project context. -#[derive(Debug)] -pub enum ExternalScope { - /// File inside an R package. The scope chain includes layers from other - /// package files (ordered by collation, later files shadow earlier ones) - /// and from NAMESPACE imports (`importFrom`, `import`), with base - /// at the bottom. Top-level code only sees predecessor files - /// whereas function bodies (lazy scopes) see all files because the - /// namespace is fully populated before any function runs. - Package { - top_level: Vec, - lazy: Vec, - }, - - /// Script or file outside a package. The scope chain is the R - /// search path: `library()` attachments from the file itself, - /// default packages (stats, graphics, etc.), and base. - /// - /// At top-level, only `Attach` semantic calls that appear before - /// the cursor are visible (R executes scripts sequentially). - /// Inside function bodies all visible attachments are exposed - /// because the function will typically be called after the full - /// script has been sourced. - SearchPath { - base: Vec, - semantic_calls: Vec, - }, -} - -impl Default for ExternalScope { - fn default() -> Self { - Self::SearchPath { - base: Vec::new(), - semantic_calls: Vec::new(), - } - } -} - -impl ExternalScope { - pub fn package(top_level: Vec, lazy: Vec) -> Self { - Self::Package { top_level, lazy } - } - - pub fn search_path(semantic_calls: Vec, base: Vec) -> Self { - Self::SearchPath { - base, - semantic_calls, - } - } - - /// Return the scope chain appropriate for the given offset. For - /// packages, top-level scope uses predecessors only while lazy - /// (function) scopes see all files. For scripts, top-level code - /// only sees `library()` calls that precede the cursor while - /// function bodies see all attachments. - pub fn at(&self, index: &SemanticIndex, offset: TextSize) -> Cow<'_, [ScopeLayer]> { - match self { - Self::Package { top_level, lazy } => { - let (_, scope) = index.scope_at(offset); - match scope.kind() { - ScopeKind::File => Cow::Borrowed(top_level), - ScopeKind::Function => Cow::Borrowed(lazy), - } - }, - Self::SearchPath { - base, - semantic_calls, - } => { - let (cursor_scope, _) = index.scope_at(offset); - let file_scope = ScopeId::from(0); - let in_function = cursor_scope != file_scope; - let layers: Vec<_> = semantic_calls - .iter() - .rev() - .filter(|c| { - let call_scope = c.scope(); - // File-scope attachments are always visible inside - // function bodies (the function is typically called - // after the full script has been sourced). - if in_function && call_scope == file_scope { - return true; - } - c.offset() < offset && - index.ancestor_scopes(cursor_scope).any(|s| s == call_scope) - }) - .filter_map(|c| match c.kind() { - SemanticCallKind::Attach { package } => { - Some(ScopeLayer::PackageExports(package.clone())) - }, - SemanticCallKind::Source { .. } => None, - }) - .chain(base.iter().cloned()) - .collect(); - Cow::Owned(layers) - }, - } - } - - /// The full scope for lazy contexts. Useful for features that don't - /// have a cursor position (e.g. completions, workspace symbols). - pub fn lazy(&self) -> Cow<'_, [ScopeLayer]> { - match self { - Self::Package { lazy, .. } => Cow::Borrowed(lazy), - Self::SearchPath { - semantic_calls, - base, - .. - } => { - let file_scope = ScopeId::from(0); - let mut layers: Vec = semantic_calls - .iter() - .rev() - .filter(|c| c.scope() == file_scope) - .filter_map(|c| match c.kind() { - SemanticCallKind::Attach { package } => { - Some(ScopeLayer::PackageExports(package.clone())) - }, - SemanticCallKind::Source { .. } => None, - }) - .collect(); - layers.extend(base.iter().cloned()); - Cow::Owned(layers) - }, - } - } -} diff --git a/crates/oak_ide/src/goto_definition.rs b/crates/oak_ide/src/goto_definition.rs index 2797beedb1..5d864e1e71 100644 --- a/crates/oak_ide/src/goto_definition.rs +++ b/crates/oak_ide/src/goto_definition.rs @@ -1,57 +1,24 @@ -use std::collections::HashSet; - use aether_syntax::RSyntaxNode; -use biome_rowan::TextSize; -use oak_db::LegacyDb; -use oak_semantic::external::resolve_external_name; -use oak_semantic::external::resolve_in_package; -use oak_semantic::package_definitions::PackageDefinitionVisibility; -use oak_semantic::scope_layer::ScopeLayer; -use oak_semantic::semantic_index::DefinitionKind; use oak_semantic::semantic_index::SemanticIndex; -use oak_semantic::semantic_index::Use; -use oak_semantic::DefinitionId; use oak_semantic::ScopeId; use oak_semantic::UseId; use url::Url; -use crate::ExternalScope; +use crate::FilePosition; use crate::Identifier; use crate::NavigationTarget; /// Resolve the symbol at `offset` in a file. /// -/// Uses `Identifier::classify` to determine what the offset points at: -/// -/// - Definition site (LHS of assignment, parameter, for variable): -/// navigates to itself. We round-trip through the index to get -/// `full_range` and `focus_range` from the `Definition`. Currently both -/// are the name range, but once we distinguish them `full_range` will -/// come from the `DefinitionKind`'s `RSyntaxNode` (the whole assignment / -/// parameter expression). -/// -/// - Use site (name reference): resolves via the use-def map, enclosing -/// scopes, and the external scope chain. -/// -/// - Namespace access (`pkg::sym` or `pkg:::sym`): resolves the symbol -/// directly in the named package. -/// -/// Returns an empty `Vec` if the offset doesn't point at a known -/// identifier, or if the symbol cannot be resolved. -// TODO(salsa): This body is the `LegacyDb`-based implementation. Gets -// rewritten on top of `oak_db::File::resolve_at`, at which point the -// helpers below (`resolve_use`, `resolve_import`, `resolve_external`, -// `resolve_namespace_access`) and the `ExternalScope` parameter all go -// away. +/// TODO(salsa) Within-file only. `pkg::sym` and any unresolved free variable +/// currently return an empty list. Cross-file resolution lives in the +/// salsa-backed path and will be wired in later. pub fn goto_definition( - db: &dyn LegacyDb, - offset: TextSize, - file: &Url, - root: &RSyntaxNode, index: &SemanticIndex, - scope: &ExternalScope, + root: &RSyntaxNode, + position: &FilePosition, ) -> Vec { - let Some(ident) = Identifier::classify(root, index, offset) else { + let Some(ident) = Identifier::classify(index, root, position.offset) else { return Vec::new(); }; @@ -61,163 +28,45 @@ pub fn goto_definition( let name = index.symbols(scope_id).symbol(def.symbol()).name(); vec![NavigationTarget { - file: file.clone(), + file: position.file.clone(), name: name.to_string(), full_range: def.range(), focus_range: def.range(), }] }, Identifier::Use { scope_id, use_id } => { - let use_site = &index.uses(scope_id)[use_id]; - resolve_use(db, file, index, scope_id, use_id, use_site, offset, scope) - }, - Identifier::NamespaceAccess { - ref package, - ref symbol, - internal, - .. - } => { - let visibility = if internal { - PackageDefinitionVisibility::Internal - } else { - PackageDefinitionVisibility::Exported - }; - resolve_namespace_access(db, symbol, package, visibility) + resolve_use(index, &position.file, scope_id, use_id) }, + Identifier::NamespaceAccess { .. } => Vec::new(), } } fn resolve_use( - db: &dyn LegacyDb, - file: &Url, index: &SemanticIndex, + file: &Url, scope_id: ScopeId, use_id: UseId, - use_site: &Use, - offset: TextSize, - scope: &ExternalScope, ) -> Vec { - let use_def_map = index.use_def_map(scope_id); - let bindings = use_def_map.bindings_at_use(use_id); - - let symbol_name = index.symbols(scope_id).symbol(use_site.symbol()).name(); - - let definitions = bindings.definitions(); - - let local_targets = |scope, defs: &[DefinitionId]| -> Vec { - defs.iter() - .filter_map(|&def_id| { - let def = &index.definitions(scope)[def_id]; - match def.kind() { - DefinitionKind::Import { file, name, .. } => resolve_import(db, file, name), - _ => Some(NavigationTarget { - file: file.clone(), - name: symbol_name.to_string(), - full_range: def.range(), - focus_range: def.range(), - }), - } - }) - .collect() - }; - - let external_targets = || { - let scope_chain = scope.at(index, offset); - resolve_external(db, symbol_name, &scope_chain) - }; - - if !definitions.is_empty() { - let mut targets = local_targets(scope_id, definitions); - if bindings.may_be_unbound() { - targets.extend(external_targets()); - } - return targets; - } - - // No local definitions. If we're in a nested scope, check enclosing - // bindings (the symbol might be defined in an outer function scope). - if let Some((enclosing_scope, enclosing_bindings)) = - index.enclosing_bindings(scope_id, use_site.symbol()) - { - let enclosing_defs = enclosing_bindings.definitions(); - if !enclosing_defs.is_empty() { - let mut targets = local_targets(enclosing_scope, enclosing_defs); - if enclosing_bindings.may_be_unbound() { - targets.extend(external_targets()); + let symbol = index.uses(scope_id)[use_id].symbol(); + let symbol_name = index.symbols(scope_id).symbol(symbol).name().to_string(); + + // `reaching_definitions` unions the local use-def map with the + // enclosing-scope snapshot when `may_be_unbound` is true. That + // covers the conditional-local-plus-outer-binding case where both + // the inner conditional def and the outer def can reach the use. + // + // TODO(salsa): when the result is empty, fall through to cross-file + // resolution (file imports / package exports). + index + .reaching_definitions(scope_id, use_id) + .map(|(scope, def_id)| { + let def = &index.definitions(scope)[def_id]; + NavigationTarget { + file: file.clone(), + name: symbol_name.clone(), + full_range: def.range(), + focus_range: def.range(), } - return targets; - } - } - - external_targets() -} - -/// Chase a `DefinitionKind::Import` forwarding binding to the actual -/// definition in the target file. Recurses through chains of Import -/// definitions (e.g., a.R sources b.R sources c.R). -/// -/// TODO(salsa): Move to `oak_semantic` once it depends on `oak_db`. -fn resolve_import(db: &dyn LegacyDb, file: &Url, name: &str) -> Option { - let mut visited = HashSet::new(); - resolve_import_inner(db, file, name, &mut visited) -} - -fn resolve_import_inner( - db: &dyn LegacyDb, - file: &Url, - name: &str, - visited: &mut HashSet<(Url, String)>, -) -> Option { - if !visited.insert((file.clone(), name.to_string())) { - return None; - } - - let target_index = db.semantic_index(file)?; - - // Imports always target top-level definitions in the sourced file. - let file_scope = ScopeId::from(0); - let symbols = &target_index.symbols(file_scope); - - let (_def_id, def) = target_index - .definitions(file_scope) - .iter() - .filter(|(_id, def)| symbols.symbol(def.symbol()).name() == name) - .last()?; - - match def.kind() { - DefinitionKind::Import { - file: next_file, - name: next_name, - .. - } => resolve_import_inner(db, next_file, next_name, visited), - _ => Some(NavigationTarget { - file: file.clone(), - name: name.to_string(), - full_range: def.range(), - focus_range: def.range(), - }), - } -} - -fn resolve_namespace_access( - db: &dyn LegacyDb, - symbol: &str, - package: &str, - visibility: PackageDefinitionVisibility, -) -> Vec { - let Some(external) = resolve_in_package(db.library(), package, symbol, visibility) else { - return Vec::new(); - }; - vec![external.into()] -} - -fn resolve_external( - db: &dyn LegacyDb, - symbol: &str, - scope_chain: &[ScopeLayer], -) -> Vec { - let Some(external) = resolve_external_name(db.library(), scope_chain, symbol) else { - return Vec::new(); - }; - vec![external.into()] + }) + .collect() } diff --git a/crates/oak_ide/src/identifier.rs b/crates/oak_ide/src/identifier.rs index 27508c74d6..6ce0d0de35 100644 --- a/crates/oak_ide/src/identifier.rs +++ b/crates/oak_ide/src/identifier.rs @@ -34,7 +34,7 @@ pub enum Identifier { } impl Identifier { - pub fn classify(root: &RSyntaxNode, index: &SemanticIndex, offset: TextSize) -> Option { + pub fn classify(index: &SemanticIndex, root: &RSyntaxNode, offset: TextSize) -> Option { let (scope_id, _) = index.scope_at(offset); // `Import` definitions have empty ranges (no physical text position, diff --git a/crates/oak_ide/src/lib.rs b/crates/oak_ide/src/lib.rs index 838af1c748..52555c00f2 100644 --- a/crates/oak_ide/src/lib.rs +++ b/crates/oak_ide/src/lib.rs @@ -1,14 +1,19 @@ -mod external_scope; mod goto_definition; mod identifier; use biome_rowan::TextRange; -pub use external_scope::ExternalScope; +use biome_rowan::TextSize; pub use goto_definition::goto_definition; pub use identifier::Identifier; -use oak_semantic::external::ExternalDefinition; use url::Url; +/// A cursor location in the workspace: a file and an offset into it. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FilePosition { + pub file: Url, + pub offset: TextSize, +} + /// A location in source code that the editor can navigate to. /// /// Shared result type for IDE features like goto-definition, find-references, @@ -20,15 +25,3 @@ pub struct NavigationTarget { pub full_range: TextRange, pub focus_range: TextRange, } - -impl From for NavigationTarget { - fn from(value: ExternalDefinition) -> Self { - let (file, name, range) = value.into_parts(); - Self { - file, - name, - full_range: range, - focus_range: range, - } - } -} diff --git a/crates/oak_ide/tests/goto_definition.rs b/crates/oak_ide/tests/goto_definition.rs index 476b7715b2..a6742f7a47 100644 --- a/crates/oak_ide/tests/goto_definition.rs +++ b/crates/oak_ide/tests/goto_definition.rs @@ -1,31 +1,14 @@ -use std::collections::HashMap; -use std::path::PathBuf; -use std::sync::Arc; - use aether_parser::parse; use aether_parser::RParserOptions; use aether_syntax::RSyntaxNode; use biome_rowan::TextRange; use biome_rowan::TextSize; -use oak_db::LegacyDb; use oak_ide::goto_definition; -use oak_ide::ExternalScope; +use oak_ide::FilePosition; use oak_ide::NavigationTarget; -use oak_package_metadata::description::Description; -use oak_package_metadata::namespace::Namespace; use oak_semantic::build_index; -use oak_semantic::library::Library; -use oak_semantic::package::Package; -use oak_semantic::scope_layer::file_layers; -use oak_semantic::scope_layer::ScopeLayer; -use oak_semantic::semantic_index::SemanticCallKind; use oak_semantic::semantic_index::SemanticIndex; -use oak_semantic::ImportsResolver; use oak_semantic::NoopImportsResolver; -use oak_semantic::ScopeId; -use oak_semantic::SourceResolution; -use oak_sources::test::TestPackageCache; -use stdext::SortedVec; use url::Url; fn parse_source(source: &str) -> (RSyntaxNode, SemanticIndex) { @@ -35,110 +18,6 @@ fn parse_source(source: &str) -> (RSyntaxNode, SemanticIndex) { (root, index) } -/// Cross-file resolver that returns the same resolution for any path. Used -/// by the source()-resolution tests below; the path argument is irrelevant -/// because each test wires a single sourced file. -struct ConstResolver(SourceResolution); - -impl ImportsResolver for ConstResolver { - fn resolve_source(&mut self, _path: &str) -> Option { - Some(self.0.clone()) - } -} - -struct TestDb { - library: Library, - sources: HashMap, -} - -impl LegacyDb for TestDb { - fn semantic_index(&self, file: &Url) -> Option { - // Rebuild from source for tests. We store the source instead. - self.sources.get(file).map(|source| { - let parsed = parse(source, RParserOptions::default()); - build_index(&parsed.tree(), NoopImportsResolver) - }) - } - fn library(&self) -> &Library { - &self.library - } -} - -fn empty_library() -> Library { - Library::new(vec![], None) -} - -struct TestPackage { - name: String, - file: PathBuf, - exports: Vec, - internals: Vec, -} - -impl TestPackage { - // Most convenient input types for test construction - fn new(name: &str, file: &str, exports: Vec<&str>, internals: Vec<&str>) -> Self { - Self { - name: String::from(name), - file: PathBuf::from(file), - exports: exports - .into_iter() - .map(|export| export.to_string()) - .collect(), - internals: internals - .into_iter() - .map(|export| export.to_string()) - .collect(), - } - } -} - -fn test_library(packages: Vec) -> Library { - let cache = TestPackageCache::new().unwrap(); - - // Both exports and internals are in the test file - for package in &packages { - let content = test_file(&package.exports, &package.internals); - cache - .add(&package.name, vec![(package.file.as_path(), &content)]) - .expect("Can write to cache"); - } - - let cache = Arc::new(cache); - - let mut library = Library::new(vec![], Some(cache)); - - // Only exports are included in `Namespace` - for package in &packages { - let ns = Namespace { - exports: SortedVec::from_vec(package.exports.clone()), - ..Default::default() - }; - let desc = Description { - name: package.name.clone(), - ..Default::default() - }; - let pkg = Package::from_parts(PathBuf::from("/fake"), desc, ns); - library = library.insert(&package.name, pkg); - } - - library -} - -// Create a file worth of function definitions -fn test_file(exports: &Vec, internals: &Vec) -> String { - let mut out = String::new(); - - for export in exports { - out.push_str(&format!("{export} <- function() {{}}\n\n")); - } - for internal in internals { - out.push_str(&format!("{internal} <- function() {{}}\n\n")); - } - - out -} - fn text_range(start: u32, end: u32) -> TextRange { TextRange::new(TextSize::from(start), TextSize::from(end)) } @@ -151,6 +30,13 @@ fn offset(n: u32) -> TextSize { TextSize::from(n) } +fn pos(file: &Url, n: u32) -> FilePosition { + FilePosition { + file: file.clone(), + offset: offset(n), + } +} + // --- Local resolution --- #[test] @@ -160,19 +46,8 @@ fn test_local_simple() { let source = "x <- 1\nx\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; - let targets = goto_definition( - &db, - offset(7), - &file, - &root, - &idx, - &ExternalScope::default(), - ); + let targets = goto_definition(&idx, &root, &pos(&file, 7)); assert_eq!(targets, vec![NavigationTarget { file, name: "x".to_string(), @@ -187,19 +62,8 @@ fn test_local_reassignment_shadows() { let source = "x <- 1\nx <- 2\nx\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; - let targets = goto_definition( - &db, - offset(14), - &file, - &root, - &idx, - &ExternalScope::default(), - ); + let targets = goto_definition(&idx, &root, &pos(&file, 14)); assert_eq!(targets, vec![NavigationTarget { file, name: "x".to_string(), @@ -213,21 +77,10 @@ fn test_local_conditional_returns_both() { let source = "if (TRUE) x <- 1 else x <- 2\nx\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; let use_offset = source.rfind('x').unwrap() as u32; - let targets = goto_definition( - &db, - offset(use_offset), - &file, - &root, - &idx, - &ExternalScope::default(), - ); + let targets = goto_definition(&idx, &root, &pos(&file, use_offset)); assert_eq!(targets, vec![ NavigationTarget { file: file.clone(), @@ -249,20 +102,9 @@ fn test_local_in_function() { let source = "f <- function() {\n x <- 1\n x\n}\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; let use_offset = source.rfind('x').unwrap() as u32; - let targets = goto_definition( - &db, - offset(use_offset), - &file, - &root, - &idx, - &ExternalScope::default(), - ); + let targets = goto_definition(&idx, &root, &pos(&file, use_offset)); assert_eq!(targets, vec![NavigationTarget { file, name: "x".to_string(), @@ -276,20 +118,9 @@ fn test_local_parameter() { let source = "f <- function(x) {\n x\n}\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; let use_offset = source.rfind('x').unwrap() as u32; - let targets = goto_definition( - &db, - offset(use_offset), - &file, - &root, - &idx, - &ExternalScope::default(), - ); + let targets = goto_definition(&idx, &root, &pos(&file, use_offset)); assert_eq!(targets, vec![NavigationTarget { file, name: "x".to_string(), @@ -305,20 +136,9 @@ fn test_enclosing_scope() { let source = "x <- 1\nf <- function() {\n x\n}\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; let use_offset = source.rfind('x').unwrap() as u32; - let targets = goto_definition( - &db, - offset(use_offset), - &file, - &root, - &idx, - &ExternalScope::default(), - ); + let targets = goto_definition(&idx, &root, &pos(&file, use_offset)); assert_eq!(targets, vec![NavigationTarget { file, name: "x".to_string(), @@ -327,125 +147,93 @@ fn test_enclosing_scope() { }]); } -// --- External resolution: project file --- - #[test] -fn test_external_project_file() { - let source = "foo\n"; - let file = file_url("current.R"); +fn test_conditional_local_falls_through_to_enclosing() { + // Inside `f`, `x` is conditionally defined: bindings = {inner def} + // with `may_be_unbound = true`. At runtime the use of `x` reaches + // either the inner def (when the branch ran) or the outer def (when + // it didn't). Goto-def should return both. + // + // "x <- 1\nf <- function(cond) {\n if (cond) x <- 2\n x\n}\n" + // 0 7 30 42 49 + let source = "x <- 1\nf <- function(cond) {\n if (cond) x <- 2\n x\n}\n"; + let file = file_url("test.R"); let (root, idx) = parse_source(source); - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; - let other_url = file_url("other.R"); - let other_source = "foo <- 42\n"; - let (_other_root, other_idx) = parse_source(other_source); - let scope_chain = file_layers(other_url.clone(), &other_idx); + let outer_def = source.find("x <- 1").unwrap() as u32; + let inner_def = source.find("x <- 2").unwrap() as u32; + let use_offset = source.rfind('x').unwrap() as u32; - let targets = goto_definition( - &db, - offset(0), - &file, - &root, - &idx, - &ExternalScope::package(scope_chain.clone(), scope_chain), - ); + let targets = goto_definition(&idx, &root, &pos(&file, use_offset)); + let ranges: Vec<_> = targets.into_iter().map(|t| t.full_range).collect(); + assert!(ranges.contains(&text_range(outer_def, outer_def + 1))); + assert!(ranges.contains(&text_range(inner_def, inner_def + 1))); + assert_eq!(ranges.len(), 2); +} + +// --- Member access (`$`) --- + +#[test] +fn test_dollar_lhs_resolves() { + // Cursor on `foo` in `foo$bar` resolves to the definition of `foo` + let source = "foo <- list()\nfoo$bar\n"; + let file = file_url("test.R"); + let (root, idx) = parse_source(source); + + // `foo` in `foo$bar` starts at offset 14 + let targets = goto_definition(&idx, &root, &pos(&file, 14)); assert_eq!(targets, vec![NavigationTarget { - file: other_url, + file, name: "foo".to_string(), full_range: text_range(0, 3), focus_range: text_range(0, 3), }]); } -// --- External resolution: package --- - #[test] -fn test_external_package() { - let source = "mutate\n"; +fn test_dollar_rhs_no_resolution() { + // Cursor on `bar` in `foo$bar`: member names are not tracked by the index + let source = "foo <- list()\nfoo$bar\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); - let library = test_library(vec![TestPackage::new( - "dplyr", - "dplyr.R", - vec!["filter", "mutate", "select"], - vec![], - )]); - let db = TestDb { - library, - sources: HashMap::new(), - }; - let scope_chain = vec![ScopeLayer::PackageExports("dplyr".to_string())]; + // `bar` starts at offset 18 + let targets = goto_definition(&idx, &root, &pos(&file, 18)); + assert!(targets.is_empty()); +} - let targets = goto_definition( - &db, - offset(0), - &file, - &root, - &idx, - &ExternalScope::package(scope_chain.clone(), scope_chain), - ); +// --- No resolution --- - assert_eq!(targets.len(), 1); +#[test] +fn test_no_use_at_offset() { + let source = "x <- 1\n"; + let file = file_url("test.R"); + let (root, idx) = parse_source(source); - let target = targets.first().unwrap(); - assert!(target.file.path().ends_with("dplyr.R")); - assert_eq!(target.name, "mutate".to_string()); + let targets = goto_definition(&idx, &root, &pos(&file, 3)); + assert!(targets.is_empty()); } -// --- External resolution: importFrom --- - #[test] -fn test_external_import_from() { - let source = "tibble\n"; +fn test_unresolved_symbol() { + let source = "foo\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; - - let mut imports = HashMap::new(); - imports.insert("tibble".to_string(), "tibble".to_string()); - let scope_chain = vec![ScopeLayer::PackageImports(imports)]; - let targets = goto_definition( - &db, - offset(0), - &file, - &root, - &idx, - &ExternalScope::package(scope_chain.clone(), scope_chain), - ); - // importFrom resolves to a package, no file/range to navigate to + let targets = goto_definition(&idx, &root, &pos(&file, 0)); assert!(targets.is_empty()); } -// --- Member access (`$`) --- +// --- Definition site navigation --- #[test] -fn test_dollar_lhs_resolves() { - // Cursor on `foo` in `foo$bar` resolves to the definition of `foo` - let source = "foo <- list()\nfoo$bar\n"; +fn test_definition_site_assignment() { + // Cursor on the `foo` in `foo <- 1` should navigate to itself + let source = "foo <- 1\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; - // `foo` in `foo$bar` starts at offset 14 - let targets = goto_definition( - &db, - offset(14), - &file, - &root, - &idx, - &ExternalScope::default(), - ); + let targets = goto_definition(&idx, &root, &pos(&file, 0)); assert_eq!(targets, vec![NavigationTarget { file, name: "foo".to_string(), @@ -455,362 +243,557 @@ fn test_dollar_lhs_resolves() { } #[test] -fn test_dollar_rhs_no_resolution() { - // Cursor on `bar` in `foo$bar`: member names are not tracked by the index - let source = "foo <- list()\nfoo$bar\n"; +fn test_definition_site_parameter() { + let source = "f <- function(x) { x }\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; - // `bar` starts at offset 18 - let targets = goto_definition( - &db, - offset(18), - &file, - &root, - &idx, - &ExternalScope::default(), - ); - assert!(targets.is_empty()); + // Cursor on the `x` parameter name (offset 14) + let targets = goto_definition(&idx, &root, &pos(&file, 14)); + assert_eq!(targets, vec![NavigationTarget { + file, + name: "x".to_string(), + full_range: text_range(14, 15), + focus_range: text_range(14, 15), + }]); } -// --- Use inside function body with cross-file definition --- - #[test] -fn test_use_in_function_body_resolves_via_external() { - // Reproduces: `is_null` used inside a function body, defined in another - // file. The use is free in the function scope, so resolution should fall - // through enclosing scopes to the external scope chain. - let source = "f <- function(x) {\n if (is_null(x)) NULL\n}\n"; - let file = file_url("R/cnd-last.R"); +fn test_definition_site_for_variable() { + let source = "for (i in 1:10) i\n"; + let file = file_url("test.R"); let (root, idx) = parse_source(source); - let other_source = "is_null <- is.null\n"; - let (_other_root, other_idx) = parse_source(other_source); - let other_url = file_url("R/types.R"); - let scope_chain = file_layers(other_url.clone(), &other_idx); + // Cursor on the `i` in `for (i in ...)` + let targets = goto_definition(&idx, &root, &pos(&file, 5)); + assert_eq!(targets, vec![NavigationTarget { + file, + name: "i".to_string(), + full_range: text_range(5, 6), + focus_range: text_range(5, 6), + }]); +} - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; +// --- Right assignment --- - // `is_null` starts at offset 24 - let is_null_offset = source.find("is_null").unwrap(); - assert_eq!(is_null_offset, 25); +#[test] +fn test_right_assignment_definition_site() { + // `1 -> x`: cursor on `x` (the definition target) + let source = "1 -> x\nx\n"; + let file = file_url("test.R"); + let (root, idx) = parse_source(source); - let scope = ExternalScope::package(Vec::new(), scope_chain); + let targets = goto_definition(&idx, &root, &pos(&file, 5)); + assert_eq!(targets, vec![NavigationTarget { + file: file.clone(), + name: "x".to_string(), + full_range: text_range(5, 6), + focus_range: text_range(5, 6), + }]); +} - let targets = goto_definition( - &db, - offset(is_null_offset as u32), - &file, - &root, - &idx, - &scope, - ); +#[test] +fn test_right_assignment_use_resolves() { + // `1 -> x` then use `x` + let source = "1 -> x\nx\n"; + let file = file_url("test.R"); + let (root, idx) = parse_source(source); + + let targets = goto_definition(&idx, &root, &pos(&file, 7)); assert_eq!(targets, vec![NavigationTarget { - file: other_url, - name: "is_null".to_string(), - full_range: text_range(0, 7), - focus_range: text_range(0, 7), + file, + name: "x".to_string(), + full_range: text_range(5, 6), + focus_range: text_range(5, 6), }]); } -// --- No resolution --- +// --- Super assignment --- #[test] -fn test_no_use_at_offset() { - let source = "x <- 1\n"; +fn test_super_assignment_resolves_in_enclosing() { + // `x <<- 1` inside a function creates a definition in the file scope. + // A use of `x` in another function should resolve to it. + let source = "f <- function() x <<- 1\ng <- function() x\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; - let targets = goto_definition( - &db, - offset(3), - &file, - &root, - &idx, - &ExternalScope::default(), - ); - assert!(targets.is_empty()); + // `x` use in `g` body + let use_offset = source.rfind('x').unwrap() as u32; + let targets = goto_definition(&idx, &root, &pos(&file, use_offset)); + assert_eq!(targets.len(), 1); + assert_eq!(targets[0].name, "x"); + assert_eq!(targets[0].file, file); } #[test] -fn test_unresolved_symbol() { - let source = "foo\n"; +fn test_super_assignment_definition_site() { + // Cursor on `x` in `x <<- 1` + let source = "f <- function() {\n x <<- 1\n}\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; - let targets = goto_definition( - &db, - offset(0), - &file, - &root, - &idx, - &ExternalScope::default(), - ); - assert!(targets.is_empty()); + // `x` at offset 20 + let def_offset = source.find("x <<-").unwrap() as u32; + let targets = goto_definition(&idx, &root, &pos(&file, def_offset)); + assert_eq!(targets.len(), 1); + assert_eq!(targets[0].name, "x"); } -// --- Local takes precedence over external --- +// --- String definitions --- #[test] -fn test_local_shadows_external() { - let source = "foo <- 1\nfoo\n"; +fn test_string_definition() { + // `"foo" <- 1` is equivalent to `foo <- 1` in R + let source = "\"foo\" <- 1\nfoo\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); - let library = test_library(vec![TestPackage::new("pkg", "pkg.R", vec!["foo"], vec![])]); - let db = TestDb { - library, - sources: HashMap::new(), - }; - - let scope_chain = vec![ScopeLayer::PackageExports("pkg".to_string())]; - let use_offset = source.rfind("foo").unwrap() as u32; - let targets = goto_definition( - &db, - offset(use_offset), - &file, - &root, - &idx, - &ExternalScope::package(scope_chain.clone(), scope_chain), - ); + // Use of `foo` at offset 11 + let targets = goto_definition(&idx, &root, &pos(&file, 11)); assert_eq!(targets, vec![NavigationTarget { file, name: "foo".to_string(), - full_range: text_range(0, 3), - focus_range: text_range(0, 3), + // The definition range covers the string literal `"foo"` + full_range: text_range(0, 5), + focus_range: text_range(0, 5), }]); } -// --- Conditional definition (may_be_unbound but has local defs) --- +// --- Nested functions --- #[test] -fn test_conditional_definition_includes_external() { - let source = "if (TRUE) x <- 1\nx\n"; +fn test_deeply_nested_function() { + // Free variable `z` resolves through two function scopes to file scope + let source = "z <- 1\nf <- function() {\n g <- function() {\n z\n }\n}\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); - let other_url = file_url("other.R"); - let other_source = "x <- 99\n"; - let (_other_root, other_idx) = parse_source(other_source); - let scope_chain = file_layers(other_url.clone(), &other_idx); + let use_offset = source.rfind('z').unwrap() as u32; + let targets = goto_definition(&idx, &root, &pos(&file, use_offset)); + assert_eq!(targets, vec![NavigationTarget { + file, + name: "z".to_string(), + full_range: text_range(0, 1), + focus_range: text_range(0, 1), + }]); +} - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; - - let use_offset = source.rfind('x').unwrap() as u32; - let targets = goto_definition( - &db, - offset(use_offset), - &file, - &root, - &idx, - &ExternalScope::package(scope_chain.clone(), scope_chain), - ); - assert_eq!(targets, vec![ - NavigationTarget { - file, - name: "x".to_string(), - full_range: text_range(10, 11), - focus_range: text_range(10, 11), - }, - NavigationTarget { - file: other_url, - name: "x".to_string(), - full_range: text_range(0, 1), - focus_range: text_range(0, 1), - }, - ]); -} - -// --- Definition site navigation --- +// --- Use on RHS of assignment --- #[test] -fn test_definition_site_assignment() { - // Cursor on the `foo` in `foo <- 1` should navigate to itself - let source = "foo <- 1\n"; +fn test_use_on_rhs_of_assignment() { + // `x <- x + 1`: the `x` on the RHS refers to the previous binding + let source = "x <- 1\nx <- x + 1\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; - let targets = goto_definition( - &db, - offset(0), - &file, - &root, - &idx, - &ExternalScope::default(), - ); + // The `x` on the RHS of the second assignment. `x <- x + 1` starts at + // offset 7, the RHS `x` is at offset 12. + let rhs_offset = 7 + "x <- ".len() as u32; + let targets = goto_definition(&idx, &root, &pos(&file, rhs_offset)); assert_eq!(targets, vec![NavigationTarget { file, - name: "foo".to_string(), - full_range: text_range(0, 3), - focus_range: text_range(0, 3), + name: "x".to_string(), + // Resolves to the first definition + full_range: text_range(0, 1), + focus_range: text_range(0, 1), }]); } +// --- Namespace access (`::`, `:::`) returns empty in the within-file handler --- + #[test] -fn test_definition_site_parameter() { - let source = "f <- function(x) { x }\n"; +fn test_namespace_access_returns_empty() { + let source = "dplyr::mutate\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; - // Cursor on the `x` parameter name (offset 14) - let targets = goto_definition( - &db, - offset(14), - &file, - &root, - &idx, - &ExternalScope::default(), + // Cursor on `mutate` (offset 7) + let targets = goto_definition(&idx, &root, &pos(&file, 7)); + assert!(targets.is_empty()); + + // Cursor on `dplyr` (offset 0) + let targets = goto_definition(&idx, &root, &pos(&file, 0)); + assert!(targets.is_empty()); +} + +// --- Identifier::classify keeps recognizing `pkg::sym` --- + +#[test] +fn test_namespace_classify() { + use oak_ide::Identifier; + + let source = "dplyr::mutate\n"; + let parsed = parse(source, RParserOptions::default()); + let root = parsed.syntax(); + let idx = build_index(&parsed.tree(), NoopImportsResolver); + + // Cursor on `mutate` (offset 7) + let ident = Identifier::classify(&idx, &root, offset(7)); + assert_eq!( + ident, + Some(Identifier::NamespaceAccess { + package: "dplyr".to_string(), + symbol: "mutate".to_string(), + internal: false, + package_range: text_range(0, 5), + symbol_range: text_range(7, 13), + }) + ); + + // Cursor on `dplyr` (offset 2) + let ident = Identifier::classify(&idx, &root, offset(2)); + assert_eq!( + ident, + Some(Identifier::NamespaceAccess { + package: "dplyr".to_string(), + symbol: "mutate".to_string(), + internal: false, + package_range: text_range(0, 5), + symbol_range: text_range(7, 13), + }) ); - assert_eq!(targets, vec![NavigationTarget { - file, - name: "x".to_string(), - full_range: text_range(14, 15), - focus_range: text_range(14, 15), - }]); } #[test] -fn test_definition_site_for_variable() { - let source = "for (i in 1:10) i\n"; - let file = file_url("test.R"); - let (root, idx) = parse_source(source); - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; +fn test_namespace_classify_triple_colon() { + use oak_ide::Identifier; - // Cursor on the `i` in `for (i in ...)` - let targets = goto_definition( - &db, - offset(5), - &file, - &root, - &idx, - &ExternalScope::default(), + let source = "pkg:::sym\n"; + let parsed = parse(source, RParserOptions::default()); + let root = parsed.syntax(); + let idx = build_index(&parsed.tree(), NoopImportsResolver); + + let ident = Identifier::classify(&idx, &root, offset(6)); + assert_eq!( + ident, + Some(Identifier::NamespaceAccess { + package: "pkg".to_string(), + symbol: "sym".to_string(), + internal: true, + package_range: text_range(0, 3), + symbol_range: text_range(6, 9), + }) ); - assert_eq!(targets, vec![NavigationTarget { - file, - name: "i".to_string(), - full_range: text_range(5, 6), - focus_range: text_range(5, 6), - }]); } -// --- Right assignment --- +#[test] +fn test_namespace_classify_in_call() { + use oak_ide::Identifier; + + // foo::bar() + // 0123456789 + let source = "foo::bar()\n"; + let parsed = parse(source, RParserOptions::default()); + let root = parsed.syntax(); + let idx = build_index(&parsed.tree(), NoopImportsResolver); + + let ident = Identifier::classify(&idx, &root, offset(5)); + assert_eq!( + ident, + Some(Identifier::NamespaceAccess { + package: "foo".to_string(), + symbol: "bar".to_string(), + internal: false, + package_range: text_range(0, 3), + symbol_range: text_range(5, 8), + }) + ); +} #[test] -fn test_right_assignment_definition_site() { - // `1 -> x`: cursor on `x` (the definition target) - let source = "1 -> x\nx\n"; - let file = file_url("test.R"); - let (root, idx) = parse_source(source); - let db = TestDb { - library: empty_library(), - sources: HashMap::new(), - }; +fn test_namespace_classify_in_extract() { + use oak_ide::Identifier; - let targets = goto_definition( - &db, - offset(5), - &file, - &root, - &idx, - &ExternalScope::default(), + // foo::bar$baz + // 0123456789... + let source = "foo::bar$baz\n"; + let parsed = parse(source, RParserOptions::default()); + let root = parsed.syntax(); + let idx = build_index(&parsed.tree(), NoopImportsResolver); + + // Cursor on `bar` (offset 5) -- inside the RNamespaceExpression + let ident = Identifier::classify(&idx, &root, offset(5)); + assert_eq!( + ident, + Some(Identifier::NamespaceAccess { + package: "foo".to_string(), + symbol: "bar".to_string(), + internal: false, + package_range: text_range(0, 3), + symbol_range: text_range(5, 8), + }) ); - assert_eq!(targets, vec![NavigationTarget { - file: file.clone(), - name: "x".to_string(), - full_range: text_range(5, 6), - focus_range: text_range(5, 6), - }]); + + // Cursor on `baz` (offset 9) -- RHS of $, not a namespace access + let ident = Identifier::classify(&idx, &root, offset(9)); + assert_eq!(ident, None); } #[test] -fn test_right_assignment_use_resolves() { - // `1 -> x` then use `x` - let source = "1 -> x\nx\n"; - let file = file_url("test.R"); +fn test_namespace_classify_string_selectors() { + use oak_ide::Identifier; + + // "foo"::"bar" + // 0123456789... + let source = "\"foo\"::\"bar\"\n"; + let parsed = parse(source, RParserOptions::default()); + let root = parsed.syntax(); + let idx = build_index(&parsed.tree(), NoopImportsResolver); + + let ident = Identifier::classify(&idx, &root, offset(7)); + assert_eq!( + ident, + Some(Identifier::NamespaceAccess { + package: "foo".to_string(), + symbol: "bar".to_string(), + internal: false, + package_range: text_range(0, 5), + symbol_range: text_range(7, 12), + }) + ); +} + +// Parked cross-file goto-definition tests pending the Salsa Oak wiring. +// +// Snapshot of the old `LegacyDb`-based suite, trimmed to the cross-file cases +// that the within-file handler doesn't cover (`source()`, package collation, +// `library()`, NAMESPACE imports, `pkg::sym` resolution). Intra-file tests live +// in the active section above. `#[cfg(any())]` skips this block at compile time +// so the references to deleted types (`LegacyDb`, `ExternalScope`, +// `ScopeLayer`, ...) stay as the behavioural spec to re-implement on top of +// `oak_db::File::resolve_at`. +#[cfg(any())] +#[rustfmt::skip] +mod parked_salsa_tests { +use std::collections::HashMap; +use std::path::PathBuf; +use std::sync::Arc; + +use aether_parser::parse; +use aether_parser::RParserOptions; +use aether_syntax::RSyntaxNode; +use biome_rowan::TextRange; +use biome_rowan::TextSize; +use oak_db::LegacyDb; +use oak_ide::goto_definition; +use oak_ide::ExternalScope; +use oak_ide::NavigationTarget; +use oak_package_metadata::description::Description; +use oak_package_metadata::namespace::Namespace; +use oak_semantic::build_index; +use oak_semantic::library::Library; +use oak_semantic::package::Package; +use oak_semantic::scope_layer::file_layers; +use oak_semantic::scope_layer::ScopeLayer; +use oak_semantic::semantic_index::SemanticCallKind; +use oak_semantic::semantic_index::SemanticIndex; +use oak_semantic::ImportsResolver; +use oak_semantic::NoopImportsResolver; +use oak_semantic::ScopeId; +use oak_semantic::SourceResolution; +use oak_sources::test::TestPackageCache; +use stdext::SortedVec; +use url::Url; + +fn parse_source(source: &str) -> (RSyntaxNode, SemanticIndex) { + let parsed = parse(source, RParserOptions::default()); + let root = parsed.syntax(); + let index = build_index(&parsed.tree(), NoopImportsResolver); + (root, index) +} + +/// Cross-file resolver that returns the same resolution for any path. Used +/// by the source()-resolution tests below; the path argument is irrelevant +/// because each test wires a single sourced file. +struct ConstResolver(SourceResolution); + +impl ImportsResolver for ConstResolver { + fn resolve_source(&mut self, _path: &str) -> Option { + Some(self.0.clone()) + } +} + +struct TestDb { + library: Library, + sources: HashMap, +} + +impl LegacyDb for TestDb { + fn semantic_index(&self, file: &Url) -> Option { + // Rebuild from source for tests. We store the source instead. + self.sources.get(file).map(|source| { + let parsed = parse(source, RParserOptions::default()); + build_index(&parsed.tree(), NoopImportsResolver) + }) + } + fn library(&self) -> &Library { + &self.library + } +} + +fn empty_library() -> Library { + Library::new(vec![], None) +} + +struct TestPackage { + name: String, + file: PathBuf, + exports: Vec, + internals: Vec, +} + +impl TestPackage { + // Most convenient input types for test construction + fn new(name: &str, file: &str, exports: Vec<&str>, internals: Vec<&str>) -> Self { + Self { + name: String::from(name), + file: PathBuf::from(file), + exports: exports + .into_iter() + .map(|export| export.to_string()) + .collect(), + internals: internals + .into_iter() + .map(|export| export.to_string()) + .collect(), + } + } +} + +fn test_library(packages: Vec) -> Library { + let cache = TestPackageCache::new().unwrap(); + + // Both exports and internals are in the test file + for package in &packages { + let content = test_file(&package.exports, &package.internals); + cache + .add(&package.name, vec![(package.file.as_path(), &content)]) + .expect("Can write to cache"); + } + + let cache = Arc::new(cache); + + let mut library = Library::new(vec![], Some(cache)); + + // Only exports are included in `Namespace` + for package in &packages { + let ns = Namespace { + exports: SortedVec::from_vec(package.exports.clone()), + ..Default::default() + }; + let desc = Description { + name: package.name.clone(), + ..Default::default() + }; + let pkg = Package::from_parts(PathBuf::from("/fake"), desc, ns); + library = library.insert(&package.name, pkg); + } + + library +} + +// Create a file worth of function definitions +fn test_file(exports: &Vec, internals: &Vec) -> String { + let mut out = String::new(); + + for export in exports { + out.push_str(&format!("{export} <- function() {{}}\n\n")); + } + for internal in internals { + out.push_str(&format!("{internal} <- function() {{}}\n\n")); + } + + out +} + +fn text_range(start: u32, end: u32) -> TextRange { + TextRange::new(TextSize::from(start), TextSize::from(end)) +} + +fn file_url(name: &str) -> Url { + Url::parse(&format!("file:///project/R/{name}")).unwrap() +} + +fn offset(n: u32) -> TextSize { + TextSize::from(n) +} + +// --- External resolution: project file --- + +#[test] +fn test_external_project_file() { + let source = "foo\n"; + let file = file_url("current.R"); let (root, idx) = parse_source(source); let db = TestDb { library: empty_library(), sources: HashMap::new(), }; + let other_url = file_url("other.R"); + let other_source = "foo <- 42\n"; + let (_other_root, other_idx) = parse_source(other_source); + let scope_chain = file_layers(other_url.clone(), &other_idx); + let targets = goto_definition( &db, - offset(7), + offset(0), &file, &root, &idx, - &ExternalScope::default(), + &ExternalScope::package(scope_chain.clone(), scope_chain), ); assert_eq!(targets, vec![NavigationTarget { - file, - name: "x".to_string(), - full_range: text_range(5, 6), - focus_range: text_range(5, 6), + file: other_url, + name: "foo".to_string(), + full_range: text_range(0, 3), + focus_range: text_range(0, 3), }]); } -// --- Super assignment --- +// --- External resolution: package --- #[test] -fn test_super_assignment_resolves_in_enclosing() { - // `x <<- 1` inside a function creates a definition in the file scope. - // A use of `x` in another function should resolve to it. - let source = "f <- function() x <<- 1\ng <- function() x\n"; +fn test_external_package() { + let source = "mutate\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); + let library = test_library(vec![TestPackage::new( + "dplyr", + "dplyr.R", + vec!["filter", "mutate", "select"], + vec![], + )]); let db = TestDb { - library: empty_library(), + library, sources: HashMap::new(), }; - // `x` use in `g` body - let use_offset = source.rfind('x').unwrap() as u32; + let scope_chain = vec![ScopeLayer::PackageExports("dplyr".to_string())]; + let targets = goto_definition( &db, - offset(use_offset), + offset(0), &file, &root, &idx, - &ExternalScope::default(), + &ExternalScope::package(scope_chain.clone(), scope_chain), ); + assert_eq!(targets.len(), 1); - assert_eq!(targets[0].name, "x"); - assert_eq!(targets[0].file, file); + + let target = targets.first().unwrap(); + assert!(target.file.path().ends_with("dplyr.R")); + assert_eq!(target.name, "mutate".to_string()); } +// --- External resolution: importFrom --- + #[test] -fn test_super_assignment_definition_site() { - // Cursor on `x` in `x <<- 1` - let source = "f <- function() {\n x <<- 1\n}\n"; +fn test_external_import_from() { + let source = "tibble\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); let db = TestDb { @@ -818,112 +801,138 @@ fn test_super_assignment_definition_site() { sources: HashMap::new(), }; - // `x` at offset 20 - let def_offset = source.find("x <<-").unwrap() as u32; + let mut imports = HashMap::new(); + imports.insert("tibble".to_string(), "tibble".to_string()); + let scope_chain = vec![ScopeLayer::PackageImports(imports)]; + let targets = goto_definition( &db, - offset(def_offset), + offset(0), &file, &root, &idx, - &ExternalScope::default(), + &ExternalScope::package(scope_chain.clone(), scope_chain), ); - assert_eq!(targets.len(), 1); - assert_eq!(targets[0].name, "x"); + // importFrom resolves to a package, no file/range to navigate to + assert!(targets.is_empty()); } -// --- String definitions --- +// --- Use inside function body with cross-file definition --- #[test] -fn test_string_definition() { - // `"foo" <- 1` is equivalent to `foo <- 1` in R - let source = "\"foo\" <- 1\nfoo\n"; - let file = file_url("test.R"); +fn test_use_in_function_body_resolves_via_external() { + // Reproduces: `is_null` used inside a function body, defined in another + // file. The use is free in the function scope, so resolution should fall + // through enclosing scopes to the external scope chain. + let source = "f <- function(x) {\n if (is_null(x)) NULL\n}\n"; + let file = file_url("R/cnd-last.R"); let (root, idx) = parse_source(source); + + let other_source = "is_null <- is.null\n"; + let (_other_root, other_idx) = parse_source(other_source); + let other_url = file_url("R/types.R"); + let scope_chain = file_layers(other_url.clone(), &other_idx); + let db = TestDb { library: empty_library(), sources: HashMap::new(), }; - // Use of `foo` at offset 11 + // `is_null` starts at offset 24 + let is_null_offset = source.find("is_null").unwrap(); + assert_eq!(is_null_offset, 25); + + let scope = ExternalScope::package(Vec::new(), scope_chain); + let targets = goto_definition( &db, - offset(11), + offset(is_null_offset as u32), &file, &root, &idx, - &ExternalScope::default(), + &scope, ); assert_eq!(targets, vec![NavigationTarget { - file, - name: "foo".to_string(), - // The definition range covers the string literal `"foo"` - full_range: text_range(0, 5), - focus_range: text_range(0, 5), + file: other_url, + name: "is_null".to_string(), + full_range: text_range(0, 7), + focus_range: text_range(0, 7), }]); } -// --- Nested functions --- +// --- Local takes precedence over external --- #[test] -fn test_deeply_nested_function() { - // Free variable `z` resolves through two function scopes to file scope - let source = "z <- 1\nf <- function() {\n g <- function() {\n z\n }\n}\n"; +fn test_local_shadows_external() { + let source = "foo <- 1\nfoo\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); + let library = test_library(vec![TestPackage::new("pkg", "pkg.R", vec!["foo"], vec![])]); let db = TestDb { - library: empty_library(), + library, sources: HashMap::new(), }; - let use_offset = source.rfind('z').unwrap() as u32; + let scope_chain = vec![ScopeLayer::PackageExports("pkg".to_string())]; + + let use_offset = source.rfind("foo").unwrap() as u32; let targets = goto_definition( &db, offset(use_offset), &file, &root, &idx, - &ExternalScope::default(), + &ExternalScope::package(scope_chain.clone(), scope_chain), ); assert_eq!(targets, vec![NavigationTarget { file, - name: "z".to_string(), - full_range: text_range(0, 1), - focus_range: text_range(0, 1), + name: "foo".to_string(), + full_range: text_range(0, 3), + focus_range: text_range(0, 3), }]); } -// --- Use on RHS of assignment --- +// --- Conditional definition (may_be_unbound but has local defs) --- #[test] -fn test_use_on_rhs_of_assignment() { - // `x <- x + 1`: the `x` on the RHS refers to the previous binding - let source = "x <- 1\nx <- x + 1\n"; +fn test_conditional_definition_includes_external() { + let source = "if (TRUE) x <- 1\nx\n"; let file = file_url("test.R"); let (root, idx) = parse_source(source); + + let other_url = file_url("other.R"); + let other_source = "x <- 99\n"; + let (_other_root, other_idx) = parse_source(other_source); + let scope_chain = file_layers(other_url.clone(), &other_idx); + let db = TestDb { library: empty_library(), sources: HashMap::new(), }; - // The `x` on the RHS of the second assignment. `x <- x + 1` starts at - // offset 7, the RHS `x` is at offset 12. - let rhs_offset = 7 + "x <- ".len() as u32; + let use_offset = source.rfind('x').unwrap() as u32; let targets = goto_definition( &db, - offset(rhs_offset), + offset(use_offset), &file, &root, &idx, - &ExternalScope::default(), + &ExternalScope::package(scope_chain.clone(), scope_chain), ); - assert_eq!(targets, vec![NavigationTarget { - file, - name: "x".to_string(), - // Resolves to the first definition - full_range: text_range(0, 1), - focus_range: text_range(0, 1), - }]); + assert_eq!(targets, vec![ + NavigationTarget { + file, + name: "x".to_string(), + full_range: text_range(10, 11), + focus_range: text_range(10, 11), + }, + NavigationTarget { + file: other_url, + name: "x".to_string(), + full_range: text_range(0, 1), + focus_range: text_range(0, 1), + }, + ]); } // --- library() directive in predecessor file --- @@ -1167,64 +1176,6 @@ fn test_fixme_namespace_access_cursor_on_operator() { assert_eq!(target.name, "mutate".to_string()); } -#[test] -fn test_namespace_classify() { - use oak_ide::Identifier; - - let source = "dplyr::mutate\n"; - let parsed = parse(source, RParserOptions::default()); - let root = parsed.syntax(); - let idx = build_index(&parsed.tree(), NoopImportsResolver); - - // Cursor on `mutate` (offset 7) - let ident = Identifier::classify(&root, &idx, offset(7)); - assert_eq!( - ident, - Some(Identifier::NamespaceAccess { - package: "dplyr".to_string(), - symbol: "mutate".to_string(), - internal: false, - package_range: text_range(0, 5), - symbol_range: text_range(7, 13), - }) - ); - - // Cursor on `dplyr` (offset 2) - let ident = Identifier::classify(&root, &idx, offset(2)); - assert_eq!( - ident, - Some(Identifier::NamespaceAccess { - package: "dplyr".to_string(), - symbol: "mutate".to_string(), - internal: false, - package_range: text_range(0, 5), - symbol_range: text_range(7, 13), - }) - ); -} - -#[test] -fn test_namespace_classify_triple_colon() { - use oak_ide::Identifier; - - let source = "pkg:::sym\n"; - let parsed = parse(source, RParserOptions::default()); - let root = parsed.syntax(); - let idx = build_index(&parsed.tree(), NoopImportsResolver); - - let ident = Identifier::classify(&root, &idx, offset(6)); - assert_eq!( - ident, - Some(Identifier::NamespaceAccess { - package: "pkg".to_string(), - symbol: "sym".to_string(), - internal: true, - package_range: text_range(0, 3), - symbol_range: text_range(6, 9), - }) - ); -} - #[test] fn test_namespace_access_in_call() { // foo::bar() — cursor on `bar` @@ -1281,83 +1232,6 @@ fn test_namespace_access_in_extract() { assert_eq!(target.name, "bar".to_string()); } -#[test] -fn test_namespace_classify_in_call() { - use oak_ide::Identifier; - - // foo::bar() - // 0123456789 - let source = "foo::bar()\n"; - let parsed = parse(source, RParserOptions::default()); - let root = parsed.syntax(); - let idx = build_index(&parsed.tree(), NoopImportsResolver); - - let ident = Identifier::classify(&root, &idx, offset(5)); - assert_eq!( - ident, - Some(Identifier::NamespaceAccess { - package: "foo".to_string(), - symbol: "bar".to_string(), - internal: false, - package_range: text_range(0, 3), - symbol_range: text_range(5, 8), - }) - ); -} - -#[test] -fn test_namespace_classify_in_extract() { - use oak_ide::Identifier; - - // foo::bar$baz - // 0123456789... - let source = "foo::bar$baz\n"; - let parsed = parse(source, RParserOptions::default()); - let root = parsed.syntax(); - let idx = build_index(&parsed.tree(), NoopImportsResolver); - - // Cursor on `bar` (offset 5) — inside the RNamespaceExpression - let ident = Identifier::classify(&root, &idx, offset(5)); - assert_eq!( - ident, - Some(Identifier::NamespaceAccess { - package: "foo".to_string(), - symbol: "bar".to_string(), - internal: false, - package_range: text_range(0, 3), - symbol_range: text_range(5, 8), - }) - ); - - // Cursor on `baz` (offset 9) — RHS of $, not a namespace access - let ident = Identifier::classify(&root, &idx, offset(9)); - assert_eq!(ident, None); -} - -#[test] -fn test_namespace_classify_string_selectors() { - use oak_ide::Identifier; - - // "foo"::"bar" - // 0123456789... - let source = "\"foo\"::\"bar\"\n"; - let parsed = parse(source, RParserOptions::default()); - let root = parsed.syntax(); - let idx = build_index(&parsed.tree(), NoopImportsResolver); - - let ident = Identifier::classify(&root, &idx, offset(7)); - assert_eq!( - ident, - Some(Identifier::NamespaceAccess { - package: "foo".to_string(), - symbol: "bar".to_string(), - internal: false, - package_range: text_range(0, 5), - symbol_range: text_range(7, 12), - }) - ); -} - // --- source() directive --- #[test] @@ -1796,3 +1670,4 @@ fn test_resolve_import_last_def_wins() { focus_range: text_range(26, 29), }]); } +} diff --git a/crates/oak_semantic/Cargo.toml b/crates/oak_semantic/Cargo.toml index 3d26667650..28bc9f26ce 100644 --- a/crates/oak_semantic/Cargo.toml +++ b/crates/oak_semantic/Cargo.toml @@ -3,7 +3,7 @@ name = "oak_semantic" version = "0.1.0" description = """ Per-file semantic index for R: scopes, symbols, bindings, and uses. -Also includes the Library cache, package definitions, and scope layers. +Also includes the Library cache. """ authors.workspace = true @@ -29,7 +29,6 @@ log.workspace = true oak_core.workspace = true oak_index_vec.workspace = true oak_package_metadata.workspace = true -oak_sources.workspace = true rustc-hash.workspace = true smallvec.workspace = true stdext.workspace = true @@ -44,7 +43,5 @@ workspace = true optional = true [dev-dependencies] -assert_matches.workspace = true oak_semantic = { workspace = true, features = ["testing"] } -oak_sources = { workspace = true, features = ["testing"] } tempfile.workspace = true diff --git a/crates/oak_semantic/src/external.rs b/crates/oak_semantic/src/external.rs deleted file mode 100644 index 7daafcaf48..0000000000 --- a/crates/oak_semantic/src/external.rs +++ /dev/null @@ -1,108 +0,0 @@ -use biome_rowan::TextRange; -use stdext::result::ResultExt; -use url::Url; - -use crate::library::Library; -use crate::package_definitions::PackageDefinitionVisibility; -use crate::scope_layer::ScopeLayer; - -/// The result of resolving a name against the external scope chain. -/// -/// TODO: Try to fold into `Definition` after switch to Salsa -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct ExternalDefinition { - file: Url, - name: String, - range: TextRange, -} - -impl ExternalDefinition { - pub fn into_parts(self) -> (Url, String, TextRange) { - (self.file, self.name, self.range) - } - - pub fn file(&self) -> &Url { - &self.file - } - - pub fn name(&self) -> &str { - &self.name - } - - pub fn range(&self) -> TextRange { - self.range - } -} - -/// Walk the scope chain front-to-back, returning the first match. -pub fn resolve_external_name( - library: &Library, - scope: &[ScopeLayer], - name: &str, -) -> Option { - for source in scope { - match source { - ScopeLayer::FileExports { file, exports } => { - if let Some(range) = exports.get(name) { - return Some(ExternalDefinition { - file: file.clone(), - name: name.to_string(), - range: *range, - }); - } - }, - - ScopeLayer::PackageImports(names) => { - if let Some(pkg) = names.get(name) { - if let Some(def) = resolve_in_package( - library, - pkg, - name, - PackageDefinitionVisibility::Exported, - ) { - return Some(def); - } - } - }, - - ScopeLayer::PackageExports(pkg) => { - if let Some(def) = - resolve_in_package(library, pkg, name, PackageDefinitionVisibility::Exported) - { - return Some(def); - } - }, - } - } - - None -} - -/// Resolve a name in a specific package's exported symbols. -pub fn resolve_in_package( - library: &Library, - package: &str, - name: &str, - visibility: PackageDefinitionVisibility, -) -> Option { - // FIXME: Slow without salsa! - let package_definitions = library.definitions(package)?; - let definition = package_definitions.get(name)?; - - if visibility == PackageDefinitionVisibility::Exported && - definition.visibility() == PackageDefinitionVisibility::Internal - { - // Requested only exports, but located definition is internal. - // Notably if we requested internal, both exports and internal are returned. - return None; - } - - let file = package_definitions.file(definition.file_id()); - let file = Url::from_file_path(file).log_err()?; - - Some(ExternalDefinition { - file, - name: name.to_string(), - range: definition.range(), - }) -} diff --git a/crates/oak_semantic/src/lib.rs b/crates/oak_semantic/src/lib.rs index 49c05a8731..eeac6409dc 100644 --- a/crates/oak_semantic/src/lib.rs +++ b/crates/oak_semantic/src/lib.rs @@ -1,10 +1,7 @@ pub mod builder; -pub mod external; pub mod library; pub mod package; -pub mod package_definitions; pub mod resolver; -pub mod scope_layer; pub mod semantic_index; pub mod use_def_map; diff --git a/crates/oak_semantic/src/library.rs b/crates/oak_semantic/src/library.rs index 30b9b05ae5..59fd147809 100644 --- a/crates/oak_semantic/src/library.rs +++ b/crates/oak_semantic/src/library.rs @@ -3,11 +3,9 @@ use std::path::PathBuf; use std::sync::Arc; use std::sync::RwLock; -use oak_sources::PackageSources; -use stdext::result::ResultExt; - use crate::package::Package; -use crate::package_definitions::PackageDefinitions; + +// TODO(salsa): Replace by oak_db inputs /// Lazily manages a list of known R packages by name #[derive(Default, Clone, Debug)] @@ -15,23 +13,13 @@ pub struct Library { /// Paths to library directories, i.e. what `base::libPaths()` returns. pub library_paths: Arc>, - /// Object for loading package sources - /// - /// Stored as `dyn PackageSources` so we can easily swap in a test cache during - /// LSP feature testing - package_sources: Option>, - packages: Arc>>>>, } impl Library { - pub fn new( - library_paths: Vec, - package_sources: Option>, - ) -> Self { + pub fn new(library_paths: Vec) -> Self { Self { library_paths: Arc::new(library_paths), - package_sources, packages: Arc::new(RwLock::new(HashMap::new())), } } @@ -63,18 +51,6 @@ impl Library { pkg } - /// Collect all top level definitions from a package's sources - /// - /// FIXME: This is currently very expensive as it reparses the package at every call. - /// We expect this to be a tracked salsa function, which should memoize it - /// efficiently. - pub fn definitions(&self, name: &str) -> Option { - let package_sources = self.package_sources.as_ref()?; - let package = self.get(name)?; - let directory = package_sources.get(&package.description().name)?; - PackageDefinitions::load_from_directory(&directory, package.namespace()).log_err() - } - /// Insert a package in the library for testing purposes. #[cfg(any(test, feature = "testing"))] pub fn insert(self, name: &str, package: Package) -> Self { @@ -144,7 +120,7 @@ importFrom(pkg, baz) let (temp_dir, _pkg_dir) = create_temp_package(pkg_name, description, namespace); // Library should point to the temp_dir as its only library path - let lib = Library::new(vec![temp_dir.path().to_path_buf()], None); + let lib = Library::new(vec![temp_dir.path().to_path_buf()]); // First access loads from disk let pkg = lib.get(pkg_name).unwrap(); diff --git a/crates/oak_semantic/src/package_definitions.rs b/crates/oak_semantic/src/package_definitions.rs deleted file mode 100644 index 7b529a0e69..0000000000 --- a/crates/oak_semantic/src/package_definitions.rs +++ /dev/null @@ -1,165 +0,0 @@ -use std::fs::DirEntry; -use std::path::Path; -use std::path::PathBuf; - -use aether_parser::RParserOptions; -use biome_text_size::TextRange; -use oak_index_vec::define_index; -use oak_index_vec::IndexVec; -use oak_package_metadata::namespace::Namespace; -use rustc_hash::FxHashMap; -use stdext::result::ResultExt; - -use crate::build_index; -use crate::resolver::NoopImportsResolver; - -define_index!(FileId); - -/// The top level definitions exposed by a single package -#[derive(Debug)] -pub struct PackageDefinitions { - /// Vector of file names indexable by `FileId` - files: IndexVec, - - /// Map from object name to its definition - definitions: FxHashMap, -} - -impl PackageDefinitions { - /// Given an `R/` directory, load all files recursively within that directory - /// - /// Most R packages have flat `R/` directories, but some base packages have OS - /// specific subdirectories, notably utils and parallel. - pub(crate) fn load_from_directory( - directory: &Path, - namespace: &Namespace, - ) -> anyhow::Result { - let mut files = IndexVec::new(); - let mut definitions = FxHashMap::default(); - - visit_paths( - directory, - namespace, - &mut files, - &mut definitions, - &append_definitions, - )?; - - Ok(Self { files, definitions }) - } - - /// Get a [PackageDefinition] by name - pub fn get(&self, name: &str) -> Option<&PackageDefinition> { - self.definitions.get(name) - } - - /// Get the [Path] for a [FileId] - pub fn file(&self, file_id: FileId) -> &Path { - self.files[file_id].as_path() - } -} - -/// For a given R file: -/// - Read from disk -/// - Parse -/// - Create a semantic index -/// - Extract top level file exports and categorize as exported/internal -fn append_definitions( - entry: &DirEntry, - namespace: &Namespace, - files: &mut IndexVec, - definitions: &mut FxHashMap, -) { - let file = entry.path(); - - if !oak_core::file::is_r_file(&file) { - return; - } - - let Some(content) = std::fs::read_to_string(&file).log_err() else { - return; - }; - - let parsed = aether_parser::parse(&content, RParserOptions::default()); - if parsed.has_error() { - return; - } - - let index = build_index(&parsed.tree(), NoopImportsResolver); - - let file_id = files.push(file); - - for (name, def) in index.exports() { - let visibility = if namespace.exports.contains_str(name) { - PackageDefinitionVisibility::Exported - } else { - PackageDefinitionVisibility::Internal - }; - - let definition = PackageDefinition { - visibility, - file_id, - range: def.range(), - }; - - definitions.insert(name.to_string(), definition); - } -} - -/// Recursively walk a directory -#[expect(clippy::type_complexity)] -fn visit_paths( - directory: &Path, - namespace: &Namespace, - files: &mut IndexVec, - definitions: &mut FxHashMap, - cb: &dyn Fn( - &DirEntry, - &Namespace, - &mut IndexVec, - &mut FxHashMap, - ), -) -> std::io::Result<()> { - if directory.is_dir() { - for entry in std::fs::read_dir(directory)? { - let entry = entry?; - let path = entry.path(); - if path.is_dir() { - visit_paths(&path, namespace, files, definitions, cb)?; - } else { - cb(&entry, namespace, files, definitions); - } - } - } - Ok(()) -} - -#[derive(Debug, Clone, Copy, PartialEq)] -pub enum PackageDefinitionVisibility { - Exported, - Internal, -} - -/// A definition within a package -/// -/// Retrieve its original file name using its [FileId] and [PackageDefinitions::file()]. -#[derive(Debug)] -pub struct PackageDefinition { - visibility: PackageDefinitionVisibility, - file_id: FileId, - range: TextRange, -} - -impl PackageDefinition { - pub fn visibility(&self) -> PackageDefinitionVisibility { - self.visibility - } - - pub fn file_id(&self) -> FileId { - self.file_id - } - - pub fn range(&self) -> TextRange { - self.range - } -} diff --git a/crates/oak_semantic/src/scope_layer.rs b/crates/oak_semantic/src/scope_layer.rs deleted file mode 100644 index 45220ed952..0000000000 --- a/crates/oak_semantic/src/scope_layer.rs +++ /dev/null @@ -1,112 +0,0 @@ -//! TODO(salsa): Pre-salsa scope-chain implementation. The salsa-tracked -//! equivalents live in `oak_db::file_imports` (`File::imports` / -//! `File::imports_at`) and `oak_db::file_resolve` (`File::resolve` / -//! `File::resolve_at`). This module is still used by the `LegacyDb`-based -//! body of `oak_ide::goto_definition`. It goes away once that body is -//! rewritten on top of `File::resolve_at(db, offset)`. At that point -//! `ExternalScope`, the `LegacyDb` helpers in `oak_ide::goto_definition`, -//! `ark::lsp::state::file_analysis`, and this module all go together. - -use std::collections::HashMap; - -use biome_rowan::TextRange; -use oak_package_metadata::namespace::Namespace; -use url::Url; - -use crate::semantic_index::SemanticCallKind; -use crate::semantic_index::SemanticIndex; - -/// A layer in the scope chain. Layers are ordered most-local-first; resolution -/// iterates front-to-back, first match wins. -#[derive(Debug, Clone)] -pub enum ScopeLayer { - /// Bindings from a project file's top-level definitions. - /// When a name is defined multiple times, the last definition wins. - FileExports { - file: Url, - exports: HashMap, - }, - - /// Imports from e.g. `importFrom`. Maps symbol name to package name. - PackageImports(HashMap), - - /// Exports of an attached package (`library()` or NAMESPACE `import()`). - PackageExports(String), -} - -/// Compute the scope layers that a single file contributes to the -/// scope chain: one `FileExports` layer from its top-level definitions, plus -/// one `PackageExports` layer per `library()`/`require()` semantic call. -pub fn file_layers(file: Url, index: &SemanticIndex) -> Vec { - let mut layers = Vec::new(); - - let exports = index - .exports() - .into_iter() - .map(|(name, def)| (name.to_string(), def.range())) - .collect(); - - layers.push(ScopeLayer::FileExports { file, exports }); - - for call in index.semantic_calls() { - match call.kind() { - SemanticCallKind::Attach { package } => { - layers.push(ScopeLayer::PackageExports(package.clone())); - }, - SemanticCallKind::Source { .. } => { - // `source()` injects into local scope, not the search path; - // not a scope-chain layer. - }, - } - } - - layers -} - -/// The default R search path for scripts: the default packages that R -/// attaches on startup, in search order (last attached = searched first). -pub fn default_search_path() -> Vec { - // R's default packages, in reverse attachment order (most recently - // attached first). These are always on the search path unless - // overridden by `R_DEFAULT_PACKAGES`. - let default_packages = [ - "utils", - "stats", - "datasets", - "methods", - "grDevices", - "graphics", - "base", - ]; - default_packages - .into_iter() - .map(|pkg| ScopeLayer::PackageExports(pkg.to_string())) - .collect() -} - -/// Build the root layers for a package from its NAMESPACE. -/// -/// These go at the back of every file's scope chain: -/// - `PackageImports` from `importFrom()` directives (name → package) -/// - `PackageExports` from `import()` directives -/// - `PackageExports` for `base` (always implicitly available) -pub fn package_root_layers(namespace: &Namespace) -> Vec { - let mut layers = Vec::new(); - - if !namespace.imports.is_empty() { - let map = namespace - .imports - .iter() - .map(|imp| (imp.name.clone(), imp.package.clone())) - .collect(); - layers.push(ScopeLayer::PackageImports(map)); - } - - for pkg in &namespace.package_imports { - layers.push(ScopeLayer::PackageExports(pkg.clone())); - } - - layers.push(ScopeLayer::PackageExports("base".to_string())); - - layers -} diff --git a/crates/oak_semantic/src/semantic_index.rs b/crates/oak_semantic/src/semantic_index.rs index 8dc362a6ae..39727264cf 100644 --- a/crates/oak_semantic/src/semantic_index.rs +++ b/crates/oak_semantic/src/semantic_index.rs @@ -247,6 +247,43 @@ impl SemanticIndex { None } + /// All definitions that reach the use at `(scope, use_id)`. + /// + /// The local use-def bindings always count. The enclosing-scope snapshot + /// also counts when `may_be_unbound` is true. That happens when the local + /// binding doesn't cover every control-flow path, so execution can fall + /// through to the outer scope. + /// + /// When `may_be_unbound` is false we deliberately skip the enclosing scope. + /// Otherwise a shadowed inner use would also bind to the outer def of the + /// same name. + pub fn reaching_definitions( + &self, + scope: ScopeId, + use_id: UseId, + ) -> impl Iterator + '_ { + let bindings = self.use_def_map(scope).bindings_at_use(use_id); + let local = bindings.definitions().iter().map(move |&d| (scope, d)); + + let enclosing = if bindings.may_be_unbound() { + let symbol = self.uses(scope)[use_id].symbol(); + self.enclosing_bindings(scope, symbol) + } else { + None + }; + let enclosing_iter = + enclosing + .into_iter() + .flat_map(|(enclosing_scope, enclosing_bindings)| { + enclosing_bindings + .definitions() + .iter() + .map(move |&def| (enclosing_scope, def)) + }); + + local.chain(enclosing_iter) + } + /// Resolve a free variable's bindings from the enclosing scope. /// /// When a use in `scope` may be unbound (`may_be_unbound: true`), some diff --git a/crates/oak_semantic/tests/external.rs b/crates/oak_semantic/tests/external.rs deleted file mode 100644 index 9d937969a7..0000000000 --- a/crates/oak_semantic/tests/external.rs +++ /dev/null @@ -1,604 +0,0 @@ -use std::path::PathBuf; -use std::sync::Arc; - -use aether_parser::parse; -use aether_parser::RParserOptions; -use assert_matches::assert_matches; -use biome_rowan::TextRange; -use biome_rowan::TextSize; -use oak_package_metadata::description::Description; -use oak_package_metadata::namespace::Import; -use oak_package_metadata::namespace::Namespace; -use oak_semantic::build_index; -use oak_semantic::external::resolve_external_name; -use oak_semantic::library::Library; -use oak_semantic::package::Package; -use oak_semantic::scope_layer::file_layers; -use oak_semantic::scope_layer::package_root_layers; -use oak_semantic::scope_layer::ScopeLayer; -use oak_semantic::NoopImportsResolver; -use oak_sources::test::TestPackageCache; -use stdext::SortedVec; -use url::Url; - -fn empty_library() -> Library { - Library::new(vec![], None) -} - -struct TestPackage { - name: String, - file: PathBuf, - exports: Vec, - internals: Vec, -} - -impl TestPackage { - // Most convenient input types for test construction - fn new(name: &str, file: &str, exports: Vec<&str>, internals: Vec<&str>) -> Self { - Self { - name: String::from(name), - file: PathBuf::from(file), - exports: exports - .into_iter() - .map(|export| export.to_string()) - .collect(), - internals: internals - .into_iter() - .map(|export| export.to_string()) - .collect(), - } - } -} - -fn test_library(packages: Vec) -> Library { - let cache = TestPackageCache::new().unwrap(); - - // Both exports and internals are in the test file - for package in &packages { - let content = test_file(&package.exports, &package.internals); - cache - .add(&package.name, vec![(package.file.as_path(), &content)]) - .expect("Can write to cache"); - } - - let cache = Arc::new(cache); - - let mut library = Library::new(vec![], Some(cache)); - - // Only exports are included in `Namespace` - for package in &packages { - let ns = Namespace { - exports: SortedVec::from_vec(package.exports.clone()), - ..Default::default() - }; - let desc = Description { - name: package.name.clone(), - ..Default::default() - }; - let pkg = Package::from_parts(PathBuf::from("/fake"), desc, ns); - library = library.insert(&package.name, pkg); - } - - library -} - -// Create a file worth of function definitions -fn test_file(exports: &Vec, internals: &Vec) -> String { - let mut out = String::new(); - - for export in exports { - out.push_str(&format!("{export} <- function() {{}}\n\n")); - } - for internal in internals { - out.push_str(&format!("{internal} <- function() {{}}\n\n")); - } - - out -} - -fn range(start: u32, end: u32) -> TextRange { - TextRange::new(TextSize::from(start), TextSize::from(end)) -} - -fn file_url(name: &str) -> Url { - Url::parse(&format!("file:///project/R/{name}")).unwrap() -} - -fn file_exports(file: &str, entries: Vec<(&str, TextRange)>) -> ScopeLayer { - ScopeLayer::FileExports { - file: file_url(file), - exports: entries - .into_iter() - .map(|(n, r)| (n.to_string(), r)) - .collect(), - } -} - -fn package_imports(entries: Vec<(&str, &str)>) -> ScopeLayer { - ScopeLayer::PackageImports( - entries - .into_iter() - .map(|(n, p)| (n.to_string(), p.to_string())) - .collect(), - ) -} - -// --- resolve_external_name --- - -#[test] -fn test_resolve_file_exports() { - let scope = vec![file_exports("utils.R", vec![("helper", range(0, 6))])]; - - let result = resolve_external_name(&empty_library(), &scope, "helper").unwrap(); - assert_eq!(result.file(), &file_url("utils.R")); - assert_eq!(result.name(), "helper"); - assert_eq!(result.range(), range(0, 6)); -} - -#[test] -fn test_resolve_file_exports_miss() { - let scope = vec![file_exports("utils.R", vec![("helper", range(0, 6))])]; - - let result = resolve_external_name(&empty_library(), &scope, "other"); - assert_eq!(result, None); -} - -#[test] -fn test_resolve_imported_names() { - let library = test_library(vec![TestPackage::new( - "stats", - "stats.R", - vec!["median"], - vec![], - )]); - let scope = vec![package_imports(vec![("median", "stats")])]; - - let result = resolve_external_name(&library, &scope, "median").unwrap(); - assert!(result.file().path().ends_with("stats.R")); - assert_eq!(result.name(), "median") -} - -#[test] -fn test_resolve_package_exports() { - let library = test_library(vec![TestPackage::new( - "dplyr", - "dplyr.R", - vec!["filter", "mutate", "select"], - vec![], - )]); - - let scope = vec![ScopeLayer::PackageExports("dplyr".to_string())]; - - let result = resolve_external_name(&library, &scope, "filter").unwrap(); - assert!(result.file().path().ends_with("dplyr.R")); - assert_eq!(result.name(), "filter"); -} - -#[test] -fn test_resolve_package_exports_miss() { - let library = test_library(vec![TestPackage::new( - "dplyr", - "dplyr.R", - vec!["filter", "mutate", "select"], - vec![], - )]); - - let scope = vec![ScopeLayer::PackageExports("dplyr".to_string())]; - - let result = resolve_external_name(&library, &scope, "summarise"); - assert_eq!(result, None); -} - -#[test] -fn test_resolve_unknown_package_skipped() { - let scope = vec![ScopeLayer::PackageExports("nonexistent".to_string())]; - - let result = resolve_external_name(&empty_library(), &scope, "foo"); - assert_eq!(result, None); -} - -#[test] -fn test_resolve_package_shadowing() { - // Both dplyr and stats export `filter`. dplyr was loaded later so it - // appears earlier in the scope and shadows stats's version. - let library = test_library(vec![ - TestPackage::new("stats", "stats.R", vec!["filter", "median"], vec![]), - TestPackage::new("dplyr", "dplyr.R", vec!["filter", "mutate"], vec![]), - ]); - - let scope = vec![ - ScopeLayer::PackageExports("dplyr".to_string()), - ScopeLayer::PackageExports("stats".to_string()), - ]; - - // dplyr's `filter` wins - let result = resolve_external_name(&library, &scope, "filter").unwrap(); - assert!(result.file().path().ends_with("dplyr.R")); - assert_eq!(result.name(), "filter"); - - // `median` only in stats, falls through - let result = resolve_external_name(&library, &scope, "median").unwrap(); - assert!(result.file().path().ends_with("stats.R")); - assert_eq!(result.name(), "median"); -} - -#[test] -fn test_resolve_first_match_wins() { - let library = test_library(vec![TestPackage::new( - "stats", - "stats.R", - vec!["filter"], - vec![], - )]); - - let scope = vec![ - file_exports("utils.R", vec![("filter", range(0, 6))]), - ScopeLayer::PackageExports("stats".to_string()), - ]; - - // File export should win over package export - let result = resolve_external_name(&library, &scope, "filter").unwrap(); - assert_eq!(result.file(), &file_url("utils.R")); - assert_eq!(result.name(), "filter"); - assert_eq!(result.range(), range(0, 6)); -} - -#[test] -fn test_resolve_falls_through_to_later_layer() { - let library = test_library(vec![TestPackage::new( - "dplyr", - "dplyr.R", - vec!["filter", "mutate"], - vec![], - )]); - - let scope = vec![ - file_exports("utils.R", vec![("helper", range(0, 6))]), - ScopeLayer::PackageExports("dplyr".to_string()), - ]; - - // "filter" is not in file exports, falls through to package - let result = resolve_external_name(&library, &scope, "filter").unwrap(); - assert!(result.file().path().ends_with("dplyr.R")); - assert_eq!(result.name(), "filter"); -} - -#[test] -fn test_resolve_imported_names_shadow_package_exports() { - let library = test_library(vec![ - TestPackage::new("dplyr", "dplyr.R", vec!["filter"], vec![]), - TestPackage::new("stats", "stats.R", vec!["filter"], vec![]), - ]); - - let scope = vec![ - package_imports(vec![("filter", "stats")]), - ScopeLayer::PackageExports("dplyr".to_string()), - ]; - - let result = resolve_external_name(&library, &scope, "filter").unwrap(); - assert!(result.file().path().ends_with("stats.R")); - assert_eq!(result.name(), "filter"); -} - -#[test] -fn test_resolve_empty_scope() { - let result = resolve_external_name(&empty_library(), &[], "anything"); - assert_eq!(result, None); -} - -#[test] -fn test_resolve_file_exports_last_definition_wins() { - // HashMap built from a vec: last insert wins - let scope = vec![file_exports("utils.R", vec![ - ("x", range(0, 1)), - ("x", range(10, 11)), - ])]; - - let result = resolve_external_name(&empty_library(), &scope, "x").unwrap(); - assert_eq!(result.file(), &file_url("utils.R")); - assert_eq!(result.name(), "x"); - assert_eq!(result.range(), range(10, 11)); -} - -// --- file_layers --- - -fn index_source(source: &str) -> oak_semantic::semantic_index::SemanticIndex { - let parsed = parse(source, RParserOptions::default()); - build_index(&parsed.tree(), NoopImportsResolver) -} - -#[test] -fn test_file_layers_exports_only() { - let index = index_source("x <- 1\ny <- 2"); - let layers = file_layers(file_url("foo.R"), &index); - - assert_eq!(layers.len(), 1); - assert_matches!(&layers[0], ScopeLayer::FileExports { file, exports } => { - assert_eq!(file, &file_url("foo.R")); - assert!(exports.contains_key("x")); - assert!(exports.contains_key("y")); - assert_eq!(exports.len(), 2); - }); -} - -#[test] -fn test_file_layers_with_library_directives() { - let index = index_source("library(dplyr)\nlibrary(tidyr)\nx <- 1"); - let layers = file_layers(file_url("script.R"), &index); - - // FileExports + 2 PackageExports - assert_eq!(layers.len(), 3); - - assert_matches!(&layers[0], ScopeLayer::FileExports { exports, .. } => { - assert_eq!(exports.len(), 1); - assert!(exports.contains_key("x")); - }); - assert_matches!(&layers[1], ScopeLayer::PackageExports(pkg) => { - assert_eq!(pkg, "dplyr"); - }); - assert_matches!(&layers[2], ScopeLayer::PackageExports(pkg) => { - assert_eq!(pkg, "tidyr"); - }); -} - -#[test] -fn test_file_layers_last_def_wins() { - // "x <- 1\nx <- 2" has two definitions of x; the map should keep the last - let index = index_source("x <- 1\nx <- 2"); - let layers = file_layers(file_url("foo.R"), &index); - - assert_matches!(&layers[0], ScopeLayer::FileExports { exports, .. } => { - assert_eq!(exports.len(), 1); - let range = exports.get("x").unwrap(); - assert_eq!(range.start(), TextSize::from(7)); - }); -} - -#[test] -fn test_file_layers_empty_file() { - let index = index_source(""); - let layers = file_layers(file_url("empty.R"), &index); - - assert_eq!(layers.len(), 1); - assert_matches!(&layers[0], ScopeLayer::FileExports { exports, .. } => { - assert!(exports.is_empty()); - }); -} - -#[test] -fn test_file_layers_source_directive_skipped() { - let index = index_source("library(dplyr)\nsource(\"helpers.R\")\nx <- 1"); - let layers = file_layers(file_url("script.R"), &index); - - // FileExports + PackageExports(dplyr), source() is not emitted as a layer - assert_eq!(layers.len(), 2); - assert_matches!(&layers[0], ScopeLayer::FileExports { exports, .. } => { - assert_eq!(exports.len(), 1); - assert!(exports.contains_key("x")); - }); - assert_matches!(&layers[1], ScopeLayer::PackageExports(pkg) => { - assert_eq!(pkg, "dplyr"); - }); -} - -// --- Integration: file_layers -> resolve_external_name --- - -#[test] -fn test_file_layers_resolve_roundtrip() { - let library = test_library(vec![TestPackage::new( - "dplyr", - "dplyr.R", - vec!["filter", "mutate"], - vec![], - )]); - - let index = index_source("library(dplyr)\nmy_helper <- function() NULL"); - let layers = file_layers(file_url("script.R"), &index); - - // Resolve a file export - let result = resolve_external_name(&library, &layers, "my_helper").unwrap(); - assert_eq!(result.file(), &file_url("script.R")); - - // Resolve a package export - let result = resolve_external_name(&library, &layers, "filter").unwrap(); - assert!(result.file().path().ends_with("dplyr.R")); - assert_eq!(result.name(), "filter"); - - // Miss - let result = resolve_external_name(&library, &layers, "unknown"); - assert_eq!(result, None); -} - -#[test] -fn test_chained_scope_predecessor_files() { - let library = test_library(vec![TestPackage::new( - "ggplot2", - "ggplot2.R", - vec!["aes", "ggplot"], - vec![], - )]); - - // Simulate two predecessor files, then root layers - let index_a = index_source("helper_a <- 1"); - let index_b = index_source("library(ggplot2)\nhelper_b <- 2"); - - let mut scope = Vec::new(); - - // Later files come first (they shadow earlier ones) - scope.extend(file_layers(file_url("b.R"), &index_b)); - scope.extend(file_layers(file_url("a.R"), &index_a)); - - // Resolve from predecessor file b - // Predecessor file export from b.R - let result = resolve_external_name(&library, &scope, "helper_b").unwrap(); - assert_eq!(result.file(), &file_url("b.R")); - - // Resolve from predecessor file a - let result = resolve_external_name(&library, &scope, "helper_a").unwrap(); - assert_eq!(result.file(), &file_url("a.R")); - - // Resolve from ggplot2 (attached by b.R) - let result = resolve_external_name(&library, &scope, "ggplot").unwrap(); - assert!(result.file().path().ends_with("ggplot2.R")); - assert_eq!(result.name(), "ggplot"); -} - -// --- root_layers --- - -#[test] -fn test_root_layers_from_namespace_imports() { - let ns = Namespace { - package_imports: vec!["rlang".to_string(), "cli".to_string()], - ..Default::default() - }; - let layers = package_root_layers(&ns); - assert_eq!(layers.len(), 3); - assert_matches!(&layers[0], ScopeLayer::PackageExports(pkg) => { - assert_eq!(pkg, "rlang"); - }); - assert_matches!(&layers[1], ScopeLayer::PackageExports(pkg) => { - assert_eq!(pkg, "cli"); - }); - assert_matches!(&layers[2], ScopeLayer::PackageExports(pkg) => { - assert_eq!(pkg, "base"); - }); -} - -#[test] -fn test_root_layers_empty_namespace() { - let ns = Namespace::default(); - let layers = package_root_layers(&ns); - assert_eq!(layers.len(), 1); - assert_matches!(&layers[0], ScopeLayer::PackageExports(pkg) => { - assert_eq!(pkg, "base"); - }); -} - -#[test] -fn test_root_layers_includes_importfrom() { - let ns = Namespace { - imports: vec![ - Import { - name: "median".to_string(), - package: "stats".to_string(), - }, - Import { - name: "head".to_string(), - package: "utils".to_string(), - }, - ], - ..Default::default() - }; - let layers = package_root_layers(&ns); - assert_eq!(layers.len(), 2); - assert_matches!(&layers[0], ScopeLayer::PackageImports(map) => { - assert_eq!(map.get("median").unwrap(), "stats"); - assert_eq!(map.get("head").unwrap(), "utils"); - }); - assert_matches!(&layers[1], ScopeLayer::PackageExports(pkg) => { - assert_eq!(pkg, "base"); - }); -} - -#[test] -fn test_root_layers_importfrom_before_package_exports() { - let ns = Namespace { - imports: vec![Import { - name: "filter".to_string(), - package: "stats".to_string(), - }], - package_imports: vec!["dplyr".to_string()], - ..Default::default() - }; - let layers = package_root_layers(&ns); - assert_eq!(layers.len(), 3); - assert_matches!(&layers[0], ScopeLayer::PackageImports(_)); - assert_matches!(&layers[1], ScopeLayer::PackageExports(pkg) => { - assert_eq!(pkg, "dplyr"); - }); - assert_matches!(&layers[2], ScopeLayer::PackageExports(pkg) => { - assert_eq!(pkg, "base"); - }); -} - -// --- scope chain assembly --- - -#[test] -fn test_scope_chain_combines_predecessors_and_root() { - let library = test_library(vec![ - TestPackage::new("rlang", "rlang.R", vec!["sym", "expr"], vec![]), - TestPackage::new("dplyr", "dplyr.R", vec!["filter", "mutate"], vec![]), - ]); - - let index_a = index_source("helper_a <- 1"); - let index_b = index_source("library(dplyr)\nhelper_b <- 2"); - - let ns = Namespace { - package_imports: vec!["rlang".to_string()], - ..Default::default() - }; - - let mut scope = Vec::new(); - scope.extend(file_layers(file_url("b.R"), &index_b)); - scope.extend(file_layers(file_url("a.R"), &index_a)); - scope.extend(package_root_layers(&ns)); - - // Predecessor file export - let result = resolve_external_name(&library, &scope, "helper_b").unwrap(); - assert_eq!(result.file(), &file_url("b.R")); - assert_eq!(result.name(), "helper_b"); - assert_eq!(result.range(), range(15, 23)); - - // Predecessor library() directive - let result = resolve_external_name(&library, &scope, "filter").unwrap(); - assert!(result.file().path().ends_with("dplyr.R")); - assert_eq!(result.name(), "filter"); - - // Root layer (NAMESPACE import) - let result = resolve_external_name(&library, &scope, "sym").unwrap(); - assert!(result.file().path().ends_with("rlang.R")); - assert_eq!(result.name(), "sym"); - - // Miss - assert_eq!(resolve_external_name(&library, &scope, "unknown"), None); -} - -#[test] -fn test_scope_chain_predecessors_shadow_root() { - let library = test_library(vec![TestPackage::new( - "rlang", - "rlang.R", - vec!["expr"], - vec![], - )]); - - let index = index_source("expr <- function() NULL"); - - let ns = Namespace { - package_imports: vec!["rlang".to_string()], - ..Default::default() - }; - - let mut scope = Vec::new(); - scope.extend(file_layers(file_url("utils.R"), &index)); - scope.extend(package_root_layers(&ns)); - - // File export shadows the rlang root layer - let result = resolve_external_name(&library, &scope, "expr").unwrap(); - assert_eq!(result.file(), &file_url("utils.R")); -} - -#[test] -fn test_scope_chain_later_predecessor_shadows_earlier() { - // b.R is loaded after a.R, so b.R's layers come first in the scope - // and its `helper` should shadow a.R's `helper`. - let index_a = index_source("helper <- 1"); - let index_b = index_source("helper <- 2"); - - let mut scope = Vec::new(); - scope.extend(file_layers(file_url("b.R"), &index_b)); - scope.extend(file_layers(file_url("a.R"), &index_a)); - - let result = resolve_external_name(&empty_library(), &scope, "helper").unwrap(); - assert_eq!(result.file(), &file_url("b.R")); -}