refactor: Move symbol features into separate module

felix-andreas · felix-andreas · commit 6dce17eec48b · 2025-07-17T02:06:17.000+02:00
diff --git a/crates/roughly/src/index.rs b/crates/roughly/src/index.rs
@@ -1,8 +1,5 @@
 use {
-    crate::{
-        lsp_types::{Url as Uri, *},
-        tree, utils,
-    },
+    crate::{lsp_types::Range, tree, utils},
     ropey::Rope,
     std::{
         collections::HashMap,
@@ -42,23 +39,6 @@ pub enum ItemInfo {
     R6Field,
 }
 
-fn to_symbol_kind(info: &ItemInfo) -> SymbolKind {
-    match info {
-        ItemInfo::Unknown => SymbolKind::VARIABLE,
-        ItemInfo::Integer | ItemInfo::Float | ItemInfo::Complex => SymbolKind::NUMBER,
-        ItemInfo::Bool => SymbolKind::BOOLEAN,
-        ItemInfo::String => SymbolKind::STRING,
-        ItemInfo::Null => SymbolKind::NULL,
-        ItemInfo::Function => SymbolKind::FUNCTION,
-        ItemInfo::S4Class => SymbolKind::CLASS,
-        ItemInfo::S4Generic => SymbolKind::INTERFACE,
-        ItemInfo::S4Method { .. } => SymbolKind::METHOD,
-        ItemInfo::R6Class => SymbolKind::CLASS,
-        ItemInfo::R6Method => SymbolKind::METHOD,
-        ItemInfo::R6Field => SymbolKind::FIELD,
-    }
-}
-
 impl Item {
     pub fn new(
         name: String,
@@ -84,25 +64,6 @@ impl Item {
             _ => self.name.clone(),
         }
     }
-
-    pub fn to_document_symbol(&self) -> DocumentSymbol {
-        #[allow(deprecated)]
-        DocumentSymbol {
-            name: self.display_name(),
-            kind: to_symbol_kind(&self.info),
-            detail: self.detail.clone(),
-            tags: None,
-            range: self.range,
-            selection_range: self.selection_range,
-            children: self.children.as_ref().map(|children| {
-                children
-                    .iter()
-                    .map(|child| child.to_document_symbol())
-                    .collect()
-            }),
-            deprecated: None,
-        }
-    }
 }
 
 pub trait SymbolsMap {
@@ -131,42 +92,6 @@ impl SymbolsMap for HashMap<PathBuf, Vec<Item>> {
     }
 }
 
-pub fn get_workspace_symbols(
-    query: &str,
-    workspace_symbols: &impl SymbolsMap,
-) -> Vec<WorkspaceSymbol> {
-    workspace_symbols.filter_map(
-        |path, symbols| {
-            let uri = Uri::from_file_path(path).unwrap();
-            symbols
-                .iter()
-                .flat_map(|symbol| {
-                    std::iter::once((symbol, None)).chain(
-                        symbol
-                            .children
-                            .as_ref()
-                            .into_iter()
-                            .flatten()
-                            .map(|child| (child, Some(symbol.name.as_ref()))),
-                    )
-                })
-                .filter(|(symbol, _)| utils::starts_with_lowercase(&symbol.display_name(), query))
-                .map(move |(symbol, container_name)| WorkspaceSymbol {
-                    name: symbol.display_name(),
-                    kind: to_symbol_kind(&symbol.info),
-                    tags: None,
-                    container_name: container_name.map(str::to_string),
-                    location: OneOf::Left(Location {
-                        uri: uri.clone(),
-                        range: symbol.range,
-                    }),
-                    data: None,
-                })
-        },
-        128,
-    )
-}
-
 #[derive(Debug)]
 pub struct IndexError;
 
@@ -554,27 +479,6 @@ fn index_call(call: Node, rope: &Rope, nested: bool) -> Option<Item> {
     }
 }
 
-pub fn document_symbol(
-    name: String,
-    kind: SymbolKind,
-    detail: Option<String>,
-    range: Range,
-    selection_range: Range,
-    children: Option<Vec<DocumentSymbol>>,
-) -> DocumentSymbol {
-    #[allow(deprecated)]
-    DocumentSymbol {
-        name,
-        kind,
-        detail,
-        tags: None,
-        range,
-        selection_range,
-        children,
-        deprecated: None, // required for struct, but deprecated; allow deprecated field
-    }
-}
-
 // note: this function shouldn't be used for keyword-only arguments (arguments after ...)
 pub fn get_argument<'a>(
     arguments: Node<'a>,
diff --git a/crates/roughly/src/lib.rs b/crates/roughly/src/lib.rs
@@ -8,6 +8,7 @@ pub mod index;
 pub mod references;
 pub mod rename;
 pub mod server;
+pub mod symbols;
 pub mod tree;
 pub mod utils;
 
diff --git a/crates/roughly/src/server.rs b/crates/roughly/src/server.rs
@@ -18,7 +18,7 @@ use {
             Url, WorkspaceEdit, WorkspaceSymbolParams, WorkspaceSymbolResponse,
             notification::{DidChangeWatchedFiles, Notification},
         },
-        references, rename, tree, utils,
+        references, rename, symbols, tree, utils,
     },
     async_lsp::{
         ClientSocket, ErrorCode, LanguageClient, LanguageServer, ResponseError,
@@ -91,9 +91,9 @@ struct ServerState {
     base_path: PathBuf,
     document_map: HashMap<PathBuf, Document>,
     /// stores symbolds for all other files
-    document_symbols: HashMap<PathBuf, Vec<Item>>,
+    document_items: HashMap<PathBuf, Vec<Item>>,
     /// stores index for all files in R/ folder
-    workspace_symbols: HashMap<PathBuf, Vec<Item>>,
+    workspace_items: HashMap<PathBuf, Vec<Item>>,
     parser: Parser,
 }
 
@@ -114,8 +114,8 @@ impl ServerState {
             config,
             experimental_features,
             base_path: std::env::current_dir().unwrap().join("R"),
-            workspace_symbols: HashMap::new(),
-            document_symbols: HashMap::new(),
+            workspace_items: HashMap::new(),
+            document_items: HashMap::new(),
             document_map: HashMap::new(),
             parser: tree::new_parser(),
         })
@@ -133,7 +133,7 @@ impl LanguageServer for ServerState {
         tracing::info!(?self.experimental_features, "initialize");
 
         match index::index_dir(&self.base_path, &mut self.parser) {
-            Ok(symbols) => self.workspace_symbols.extend(symbols),
+            Ok(items) => self.workspace_items.extend(items),
             Err(IndexError) => self
                 .client
                 .show_message(ShowMessageParams {
@@ -236,12 +236,12 @@ impl LanguageServer for ServerState {
 
         let diagnostics = diagnostics::analyze_full(tree.root_node(), &rope, self.config.lint);
 
-        let symbols = index::index(tree.root_node(), &rope, false, false);
+        let items = index::index(tree.root_node(), &rope, false, false);
         if path.starts_with(&self.base_path) {
             // note: we need to insert into workspace in case a new file is created
-            self.workspace_symbols.insert(path.clone(), symbols);
+            self.workspace_items.insert(path.clone(), items);
         } else {
-            self.document_symbols.insert(path.clone(), symbols);
+            self.document_items.insert(path.clone(), items);
         }
 
         self.document_map.insert(path, Document { rope, tree });
@@ -330,14 +330,14 @@ impl LanguageServer for ServerState {
         // UPDATE DIAGNOSTICS
         let diagnostics = diagnostics::analyze_fast(tree.root_node(), rope, self.config.lint);
 
-        // UPDATE SYMBOLS
+        // UPDATE ITEMS
         // note: We must re-index on every change (not just on save)
         // because textDocument/documentSymbol is triggered before textDocument/didSave.
-        let symbols = index::index(tree.root_node(), rope, false, false);
+        let items = index::index(tree.root_node(), rope, false, false);
         if path.starts_with(&self.base_path) {
-            self.workspace_symbols.insert(path, symbols);
+            self.workspace_items.insert(path, items);
         } else {
-            self.document_symbols.insert(path, symbols);
+            self.document_items.insert(path, items);
         }
 
         if let Err(error) = self
@@ -403,11 +403,11 @@ impl LanguageServer for ServerState {
                 match change.typ {
                     FileChangeType::CREATED | FileChangeType::CHANGED => {
                         // note: potential race condition if the user already has the file open and begins editing immediately.
-                        let symbols = index::index_file(&path, &mut self.parser);
-                        self.workspace_symbols.insert(path.clone(), symbols);
+                        let items = index::index_file(&path, &mut self.parser);
+                        self.workspace_items.insert(path.clone(), items);
                     }
                     FileChangeType::DELETED => {
-                        self.workspace_symbols.remove(&path);
+                        self.workspace_items.remove(&path);
                     }
                     _ => unreachable!(),
                 }
@@ -440,7 +440,7 @@ impl LanguageServer for ServerState {
             position,
             &document.rope,
             &document.tree,
-            &self.workspace_symbols,
+            &self.workspace_items,
         );
 
         box_future(Ok(completions))
@@ -471,7 +471,7 @@ impl LanguageServer for ServerState {
             position.character as usize,
             &document.rope,
             &document.tree,
-            &self.workspace_symbols,
+            &self.workspace_items,
         );
 
         box_future(Ok(definitions))
@@ -585,7 +585,7 @@ impl LanguageServer for ServerState {
             include_declaration,
             &document.rope,
             &document.tree,
-            &self.workspace_symbols,
+            &self.workspace_items,
         );
 
         box_future(Ok(references))
@@ -634,17 +634,17 @@ impl LanguageServer for ServerState {
         let uri = params.text_document.uri;
         let path = uri.to_file_path().unwrap();
 
-        let symbols_map = if path.starts_with(&self.base_path) {
-            &self.workspace_symbols
+        let items_map = if path.starts_with(&self.base_path) {
+            &self.workspace_items
         } else {
-            &self.document_symbols
+            &self.document_items
         };
 
-        let Some(symbols) = symbols_map.get(&path) else {
+        let Some(items) = items_map.get(&path) else {
             tracing::error!(?path, "symbols not found");
             return box_future(Err(path_not_found_error(&path)));
         };
-        let symbols: Vec<DocumentSymbol> = symbols.iter().map(|item| item.to_document_symbol()).collect();
+        let symbols: Vec<DocumentSymbol> = symbols::document(items);
 
         box_future(Ok(Some(DocumentSymbolResponse::Nested(symbols))))
     }
@@ -657,7 +657,7 @@ impl LanguageServer for ServerState {
 
         tracing::debug!(?query);
 
-        let symbols = index::get_workspace_symbols(&query, &self.workspace_symbols);
+        let symbols = symbols::workspace(&query, &self.workspace_items);
 
         box_future(Ok(Some(WorkspaceSymbolResponse::Nested(symbols))))
     }
diff --git a/crates/roughly/src/symbols.rs b/crates/roughly/src/symbols.rs
@@ -0,0 +1,76 @@
+use crate::{
+    index::{Item, ItemInfo, SymbolsMap},
+    lsp_types::{DocumentSymbol, Location, OneOf, SymbolKind, Url as Uri, WorkspaceSymbol},
+    utils,
+};
+
+pub fn document(items: &[Item]) -> Vec<DocumentSymbol> {
+    items.iter().map(to_document_symbol).collect()
+}
+
+pub fn workspace(query: &str, workspace_symbols: &impl SymbolsMap) -> Vec<WorkspaceSymbol> {
+    workspace_symbols.filter_map(
+        |path, symbols| {
+            let uri = Uri::from_file_path(path).unwrap();
+            symbols
+                .iter()
+                .flat_map(|symbol| {
+                    std::iter::once((symbol, None)).chain(
+                        symbol
+                            .children
+                            .as_ref()
+                            .into_iter()
+                            .flatten()
+                            .map(|child| (child, Some(symbol.name.as_ref()))),
+                    )
+                })
+                .filter(|(symbol, _)| utils::starts_with_lowercase(&symbol.display_name(), query))
+                .map(move |(symbol, container_name)| WorkspaceSymbol {
+                    name: symbol.display_name(),
+                    kind: to_symbol_kind(&symbol.info),
+                    tags: None,
+                    container_name: container_name.map(str::to_string),
+                    location: OneOf::Left(Location {
+                        uri: uri.clone(),
+                        range: symbol.range,
+                    }),
+                    data: None,
+                })
+        },
+        128,
+    )
+}
+
+pub fn to_document_symbol(item: &Item) -> DocumentSymbol {
+    #[allow(deprecated)]
+    DocumentSymbol {
+        name: item.display_name(),
+        kind: to_symbol_kind(&item.info),
+        detail: item.detail.clone(),
+        tags: None,
+        range: item.range,
+        selection_range: item.selection_range,
+        children: item
+            .children
+            .as_ref()
+            .map(|children| children.iter().map(to_document_symbol).collect()),
+        deprecated: None,
+    }
+}
+
+fn to_symbol_kind(info: &ItemInfo) -> SymbolKind {
+    match info {
+        ItemInfo::Unknown => SymbolKind::VARIABLE,
+        ItemInfo::Integer | ItemInfo::Float | ItemInfo::Complex => SymbolKind::NUMBER,
+        ItemInfo::Bool => SymbolKind::BOOLEAN,
+        ItemInfo::String => SymbolKind::STRING,
+        ItemInfo::Null => SymbolKind::NULL,
+        ItemInfo::Function => SymbolKind::FUNCTION,
+        ItemInfo::S4Class => SymbolKind::CLASS,
+        ItemInfo::S4Generic => SymbolKind::INTERFACE,
+        ItemInfo::S4Method { .. } => SymbolKind::METHOD,
+        ItemInfo::R6Class => SymbolKind::CLASS,
+        ItemInfo::R6Method => SymbolKind::METHOD,
+        ItemInfo::R6Field => SymbolKind::FIELD,
+    }
+}
