Emit R6Class methods as workspace symbols #861

lionel- · 2025-06-30T08:41:21Z

Add a bunch of tests for the workspace indexers which were not tested before.
Add a new indexed type Method. This is used to include R6Class methods in workspace symbols but exclusing them from completions.
Indexers now push symbols onto a list instead of returning a single symbol to their callers. This allows a single indexer to handle multiple symbols.
The assignment handler now pushes R6Class method symbols in addition to the assigned object.

QA Notes

Add the following to a file:

class <- R6::R6Class(
    'class',
    public = list(
      initialize = function() 'initialize',
      foo = function() {
        1
      }
    ),
    private = list(
      bar = function() {
        2
      },
      not_indexed1 = NULL
    ),
    other = list(
      not_indexed2 = function() {
        3
      }
    )
  )

These symbols should now be available as Workspace symbols (# prefix in command palette): initialize(), foo(), bar() and you should be able to jump straight to the definition of these methods from any file.

The symbols starting with not_indexed should not be available.

All this is tested on the backend side.

DavisVaughan · 2025-07-16T13:46:54Z

crates/ark/src/lsp/completions/sources/composite/call.rs

@@ -267,6 +267,7 @@ fn completions_from_workspace_arguments(
            return Ok(None);


# prefix in command palette

One of the best settings I've tweaked is

"search.quickOpen.includeSymbols": true

So that a simple Cmd + P brings up a search bar that merges both file names and workspace symbols

DavisVaughan · 2025-07-16T13:49:34Z

crates/ark/src/lsp/completions/sources/composite/call.rs

@@ -267,6 +267,7 @@ fn completions_from_workspace_arguments(
            return Ok(None);
        },
        indexer::IndexEntryData::Variable { .. } => return Ok(None),
+        indexer::IndexEntryData::Method { .. } => return Ok(None),
    }



Would it be worth trying to give these a unique name? Like, trying to sniff out the R6 class name to include as well?

Imagine if I implemented two of three of these similar interfaces in the same file, then I'd have no way of telling them apart

What does Rust do for something like this?

https://github.com/DavisVaughan/almanac/blob/main/R/cache-rrule.R
https://github.com/DavisVaughan/almanac/blob/main/R/cache-radjusted.R

DavisVaughan · 2025-07-16T14:04:39Z

crates/ark/src/treesitter.rs

+        for m in self.cursor.matches(&self.query, node, contents) {
+            for cap in m.captures.iter() {
+                let cap_name = &self.query.capture_names()[cap.index as usize];
+                if *cap_name == capture_name {
+                    result.push(cap.node);
+                }
+            }


Do you want cursor.matches() or the flat cursor.captures()?

If I remember right, captures() is a flat list, but matches() has some structure that it doesn't look like you use

DavisVaughan · 2025-07-16T14:09:05Z

crates/ark/src/lsp/indexer.rs

+    if crate::treesitter::node_is_call(&rhs, "R6Class", contents) ||
+        crate::treesitter::node_is_namespaced_call(&rhs, "R6", "R6Class", contents)
+    {
+        index_r6_class(path, contents, &rhs, entries)?;


It should probably early return after this right? If not, maybe a comment about why fallthrough is good?

DavisVaughan · 2025-07-16T14:12:10Z

crates/ark/src/lsp/indexer.rs

+        let start = convert_point_to_position(contents, lhs.start_position());
+        let end = convert_point_to_position(contents, lhs.end_position());


Just checking, do you use lhs.start_position() or node.start_position()?

Same question below in the // Otherwise, emit variable path

oh i see this in the test, maybe a comment here too?

// Note that unlike document symbols whose ranges cover the whole entity // they represent, the range of workspace symbols only cover the identifers

DavisVaughan · 2025-07-16T14:15:18Z

crates/ark/src/lsp/indexer.rs

+    // Tree-sitter query to match individual methods in R6Class public/private lists
+    let query_str = r#"
+    (argument
+      name: (identifier) @access
+      value: (call
+        function: (identifier) @_list_fn
+        arguments: (arguments
+          (argument
+            name: (identifier) @method_name
+            value: (function_definition) @method_fn
+          )
+        )
+      )
+      (#match? @access "public|private")
+      (#eq? @_list_fn "list")
+    )
+    "#;
+    let mut ts_query = crate::treesitter::TSQuery::new(query_str)?;


One thing I will note is that tree-sitter recommends that you "compile" your static Query objects exactly once ahead of time, because they do take a non zero amount of overhead to parse and compile.

Would it be possible to rearrange things to end up with a Lazy<Query> that is initialized once and then fed to your TSQuery helper?

DavisVaughan · 2025-07-16T14:25:45Z

crates/ark/src/lsp/indexer.rs

+    // We'll switch from Rope to String in the near future so let's not
+    // worry about this conversion now
+    let contents_str = contents.to_string();


Agreed, should be nicer all around

If you wanted to be super fancy I believe you could implement TextProvider for Rope

https://github.com/tree-sitter/tree-sitter/blob/618b9dd66e1bd529d9757202cca5a99d333ce394/lib/binding_rust/lib.rs#L3092

https://github.com/tree-sitter/tree-sitter/blob/618b9dd66e1bd529d9757202cca5a99d333ce394/lib/binding_rust/lib.rs#L349-L355

It's probably not worth it but i think the idea is that you can somehow provide some iterator when you don't have a complete contiguous buffer available

Actually it might be easier than that? It looks like they already provide an implementation of TextProvider for FnMut(Node) -> <iterator over bytes>

https://github.com/tree-sitter/tree-sitter/blob/618b9dd66e1bd529d9757202cca5a99d333ce394/lib/binding_rust/lib.rs#L3561-L3580

So you could probably provide something like

mut |node: Node| { iter::once(contents.node_slice(&node).unwrap().to_string().as_bytes()) }

which would materialize a node worth of text in to_string() but not the whole document

It's not allowed to fail so I guess you have to unwrap()

May or may not be worth it, just a thought

DavisVaughan · 2025-07-16T14:48:39Z

crates/ark/src/lsp/indexer.rs

+    private = list(
+        private_method = function() {
+            1
+        }
+    ),


Also include a private variable to ensure it isn't indexed?

Like
https://github.com/DavisVaughan/almanac/blob/7b14f6e8f1e685975231e5dadb40bb5bb8f2a9c8/R/cache-radjusted.R#L16

lionel- added 5 commits June 30, 2025 10:24

Add basic indexer tests

127737a

Refactor indexers to collect in a vector

a139fe4

Index R6 classes

21974f0

Index and emit R6 methods as methods

3d4b53e

Rewrite method matching with TS query

ee3801d

lionel- mentioned this pull request Jul 15, 2025

Static diagnostics for library() and require() calls #870

Open

DavisVaughan approved these changes Jul 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Emit R6Class methods as workspace symbols #861

Emit R6Class methods as workspace symbols #861

Uh oh!

lionel- commented Jun 30, 2025

Uh oh!

DavisVaughan Jul 16, 2025

Uh oh!

DavisVaughan Jul 16, 2025

Uh oh!

DavisVaughan Jul 16, 2025

Uh oh!

DavisVaughan Jul 16, 2025

Uh oh!

DavisVaughan Jul 16, 2025

Uh oh!

DavisVaughan Jul 16, 2025

Uh oh!

DavisVaughan Jul 16, 2025

Uh oh!

DavisVaughan Jul 16, 2025

Uh oh!

DavisVaughan Jul 16, 2025

Uh oh!

DavisVaughan Jul 16, 2025

Uh oh!

DavisVaughan Jul 16, 2025

Uh oh!

Uh oh!

		@@ -267,6 +267,7 @@ fn completions_from_workspace_arguments(
		return Ok(None);

		let start = convert_point_to_position(contents, lhs.start_position());
		let end = convert_point_to_position(contents, lhs.end_position());

Emit R6Class methods as workspace symbols #861

Are you sure you want to change the base?

Emit R6Class methods as workspace symbols #861

Uh oh!

Conversation

lionel- commented Jun 30, 2025

QA Notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!