From 23bfceb8f9525f29b1c7d61a476c1d0b8c4fbec8 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 31 Oct 2024 15:31:15 +0100 Subject: [PATCH 1/5] Index variables --- .../src/lsp/completions/completion_item.rs | 8 ++++ .../lsp/completions/sources/composite/call.rs | 1 + .../sources/composite/workspace.rs | 11 ++++++ crates/ark/src/lsp/indexer.rs | 39 +++++++++++++++++++ crates/ark/src/lsp/symbols.rs | 13 +++++++ 5 files changed, 72 insertions(+) diff --git a/crates/ark/src/lsp/completions/completion_item.rs b/crates/ark/src/lsp/completions/completion_item.rs index c0ee1aab4..6ebc15dd2 100644 --- a/crates/ark/src/lsp/completions/completion_item.rs +++ b/crates/ark/src/lsp/completions/completion_item.rs @@ -273,6 +273,14 @@ pub(super) unsafe fn completion_item_from_object( Ok(item) } +pub(super) fn completion_item_from_variable(name: &str) -> anyhow::Result { + let mut item = completion_item(String::from(name), CompletionData::Object { + name: String::from(name), + })?; + item.kind = Some(CompletionItemKind::VALUE); + Ok(item) +} + pub(super) unsafe fn completion_item_from_promise( name: &str, object: SEXP, diff --git a/crates/ark/src/lsp/completions/sources/composite/call.rs b/crates/ark/src/lsp/completions/sources/composite/call.rs index a31f083da..cf976edbe 100644 --- a/crates/ark/src/lsp/completions/sources/composite/call.rs +++ b/crates/ark/src/lsp/completions/sources/composite/call.rs @@ -271,6 +271,7 @@ fn completions_from_workspace_arguments( // Not a function return Ok(None); }, + indexer::IndexEntryData::Variable { .. } => return Ok(None), } // Only 1 call worth of arguments are added to the completion set. diff --git a/crates/ark/src/lsp/completions/sources/composite/workspace.rs b/crates/ark/src/lsp/completions/sources/composite/workspace.rs index 29abfec75..432742d30 100644 --- a/crates/ark/src/lsp/completions/sources/composite/workspace.rs +++ b/crates/ark/src/lsp/completions/sources/composite/workspace.rs @@ -14,6 +14,7 @@ use tower_lsp::lsp_types::MarkupContent; use tower_lsp::lsp_types::MarkupKind; use crate::lsp::completions::completion_item::completion_item_from_function; +use crate::lsp::completions::completion_item::completion_item_from_variable; use crate::lsp::completions::sources::utils::filter_out_dot_prefixes; use crate::lsp::document_context::DocumentContext; use crate::lsp::indexer; @@ -97,6 +98,16 @@ pub(super) fn completions_from_workspace( }, indexer::IndexEntryData::Section { level: _, title: _ } => {}, + indexer::IndexEntryData::Variable { name } => { + let completion = match completion_item_from_variable(name) { + Ok(item) => item, + Err(err) => { + log::error!("{err:?}"); + return; + }, + }; + completions.push(completion); + }, } }); diff --git a/crates/ark/src/lsp/indexer.rs b/crates/ark/src/lsp/indexer.rs index e6b642a4e..e1e27c268 100644 --- a/crates/ark/src/lsp/indexer.rs +++ b/crates/ark/src/lsp/indexer.rs @@ -32,6 +32,9 @@ use crate::treesitter::NodeTypeExt; #[derive(Clone, Debug)] pub enum IndexEntryData { + Variable { + name: String, + }, Function { name: String, arguments: Vec, @@ -205,6 +208,11 @@ fn index_node(path: &Path, contents: &Rope, node: &Node) -> anyhow::Result anyhow::Result> { + if !matches!( + node.node_type(), + NodeType::BinaryOperator(BinaryOperatorType::LeftAssignment) | + NodeType::BinaryOperator(BinaryOperatorType::EqualsAssignment) + ) { + return Ok(None); + } + + let Some(lhs) = node.child_by_field_name("lhs") else { + return Ok(None); + }; + if !lhs.is_identifier_or_string() { + return Ok(None); + } + let name = contents.node_slice(&lhs)?.to_string(); + + let start = convert_point_to_position(contents, lhs.start_position()); + let end = convert_point_to_position(contents, lhs.end_position()); + + Ok(Some(IndexEntry { + key: name.clone(), + range: Range { start, end }, + data: IndexEntryData::Variable { name }, + })) +} + fn index_comment(_path: &Path, contents: &Rope, node: &Node) -> anyhow::Result> { // check for comment node.is_comment().into_result()?; diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index c427930dd..5be47b267 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -95,6 +95,19 @@ pub fn symbols(params: &WorkspaceSymbolParams) -> anyhow::Result { + info.push(SymbolInformation { + name: name.clone(), + kind: SymbolKind::VARIABLE, + location: Location { + uri: Url::from_file_path(path).unwrap(), + range: entry.range, + }, + tags: None, + deprecated: None, + container_name: None, + }); + }, }; }); From c5922c5712ecd227bb10b12599359c1d5ba88efc Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 1 Nov 2024 07:30:57 +0100 Subject: [PATCH 2/5] Don't reindex duplicate symbols in a document Stopgap until we are in a better position to track the scopes of objects in a document. This causes a first definition to be the symbol of reference. Jumping from a reference to a definition then always moves up. --- crates/ark/src/lsp/indexer.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/ark/src/lsp/indexer.rs b/crates/ark/src/lsp/indexer.rs index e1e27c268..019564f8e 100644 --- a/crates/ark/src/lsp/indexer.rs +++ b/crates/ark/src/lsp/indexer.rs @@ -120,7 +120,9 @@ fn insert(path: &Path, entry: IndexEntry) -> anyhow::Result<()> { let path = str_from_path(path)?; let index = index.entry(path.to_string()).or_default(); - index.insert(entry.key.clone(), entry); + if !index.contains_key(&entry.key) { + index.insert(entry.key.clone(), entry); + } Ok(()) } From f49e825141b48df02efdb659864305d44ef12964 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 1 Nov 2024 07:48:33 +0100 Subject: [PATCH 3/5] Index `method()` assignments --- crates/ark/src/lsp/indexer.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/ark/src/lsp/indexer.rs b/crates/ark/src/lsp/indexer.rs index 019564f8e..b8ff403ef 100644 --- a/crates/ark/src/lsp/indexer.rs +++ b/crates/ark/src/lsp/indexer.rs @@ -288,18 +288,21 @@ fn index_variable( let Some(lhs) = node.child_by_field_name("lhs") else { return Ok(None); }; - if !lhs.is_identifier_or_string() { + + let lhs_text = contents.node_slice(&lhs)?.to_string(); + + // Super hacky but let's wait until the typed API to do better + if !lhs_text.starts_with("method(") && !lhs.is_identifier_or_string() { return Ok(None); } - let name = contents.node_slice(&lhs)?.to_string(); let start = convert_point_to_position(contents, lhs.start_position()); let end = convert_point_to_position(contents, lhs.end_position()); Ok(Some(IndexEntry { - key: name.clone(), + key: lhs_text.clone(), range: Range { start, end }, - data: IndexEntryData::Variable { name }, + data: IndexEntryData::Variable { name: lhs_text }, })) } From 2b22a36f111ecaef90c5a59b70b1587bf3c82824 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 5 Nov 2024 11:01:32 +0100 Subject: [PATCH 4/5] Mention in changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 159b0b341..6acd4fcef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ ## 2024-10 +- Assigned objects are now indexed, in addition to assigned functions. When a name is assigned multiple times, we now only index the first occurrence. This allows you to jump to the first "declaration" of the variable. In the future we'll improve this mechanism so that you can jump to the most recent assignment. + + We also index `method(generic, class) <-` assignment to help with S7 development. This might be replaced by a "Find implementations" mechanism in the future. + - Results from completions have been improved with extra details. Package functions now display the package name (posit-dev/positron#5225) and namespace completions now display `::` to hint at what is being From ac18348723e847866096914bb678e87125672909 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 5 Nov 2024 15:19:59 +0100 Subject: [PATCH 5/5] Note indexing is only for top-level objects --- CHANGELOG.md | 2 +- crates/ark/src/lsp/indexer.rs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6acd4fcef..14f72f641 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ## 2024-10 -- Assigned objects are now indexed, in addition to assigned functions. When a name is assigned multiple times, we now only index the first occurrence. This allows you to jump to the first "declaration" of the variable. In the future we'll improve this mechanism so that you can jump to the most recent assignment. +- Objects assigned at top level are now indexed, in addition to assigned functions. When a name is assigned multiple times, we now only index the first occurrence. This allows you to jump to the first "declaration" of the variable. In the future we'll improve this mechanism so that you can jump to the most recent assignment. We also index `method(generic, class) <-` assignment to help with S7 development. This might be replaced by a "Find implementations" mechanism in the future. diff --git a/crates/ark/src/lsp/indexer.rs b/crates/ark/src/lsp/indexer.rs index b8ff403ef..34f018c7b 100644 --- a/crates/ark/src/lsp/indexer.rs +++ b/crates/ark/src/lsp/indexer.rs @@ -120,6 +120,10 @@ fn insert(path: &Path, entry: IndexEntry) -> anyhow::Result<()> { let path = str_from_path(path)?; let index = index.entry(path.to_string()).or_default(); + + // Retain the first occurrence in the index. In the future we'll track every occurrences and + // their scopes but for now we only track the first definition of an object (in a way, its + // declaration). if !index.contains_key(&entry.key) { index.insert(entry.key.clone(), entry); }