Refactor indexers to collect in a vector

Author: lionel-

crates/ark/src/lsp/indexer.rs

@@ -198,127 +198,119 @@ fn index_document(document: &Document, path: &Path) {
 
     let root = ast.root_node();
     let mut cursor = root.walk();
+    let mut entries = Vec::new();
+
     for node in root.children(&mut cursor) {
-        if let Err(err) = match index_node(path, contents, &node) {
-            Ok(Some(entry)) => insert(path, entry),
-            Ok(None) => Ok(()),
-            Err(err) => Err(err),
-        } {
+        if let Err(err) = index_node(path, contents, &node, &mut entries) {
             lsp::log_error!("Can't index document: {err:?}");
         }
     }
-}
-
-fn index_node(path: &Path, contents: &Rope, node: &Node) -> anyhow::Result<Option<IndexEntry>> {
-    if let Ok(Some(entry)) = index_function(path, contents, node) {
-        return Ok(Some(entry));
-    }
-
-    // Should be after function indexing as this is a more general case
-    if let Ok(Some(entry)) = index_variable(path, contents, node) {
-        return Ok(Some(entry));
-    }
 
-    if let Ok(Some(entry)) = index_comment(path, contents, node) {
-        return Ok(Some(entry));
+    for entry in entries {
+        if let Err(err) = insert(path, entry) {
+            lsp::log_error!("Can't insert index entry: {err:?}");
+        }
     }
-
-    Ok(None)
 }
 
-fn index_function(
-    _path: &Path,
+fn index_node(
+    path: &Path,
     contents: &Rope,
     node: &Node,
-) -> anyhow::Result<Option<IndexEntry>> {
-    // Check for assignment.
-    matches!(
-        node.node_type(),
-        NodeType::BinaryOperator(BinaryOperatorType::LeftAssignment) |
-            NodeType::BinaryOperator(BinaryOperatorType::EqualsAssignment)
-    )
-    .into_result()?;
-
-    // Check for identifier on left-hand side.
-    let lhs = node.child_by_field_name("lhs").into_result()?;
-    lhs.is_identifier_or_string().into_result()?;
-
-    // Check for a function definition on the right-hand side.
-    let rhs = node.child_by_field_name("rhs").into_result()?;
-    rhs.is_function_definition().into_result()?;
-
-    let name = contents.node_slice(&lhs)?.to_string();
-    let mut arguments = Vec::new();
-
-    // Get the parameters node.
-    let parameters = rhs.child_by_field_name("parameters").into_result()?;
-
-    // Iterate through each, and get the names.
-    let mut cursor = parameters.walk();
-    for child in parameters.children(&mut cursor) {
-        let name = unwrap!(child.child_by_field_name("name"), None => continue);
-        if name.is_identifier() {
-            let name = contents.node_slice(&name)?.to_string();
-            arguments.push(name);
-        }
-    }
-
-    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::Function {
-            name: name.clone(),
-            arguments,
-        },
-    }))
+    entries: &mut Vec<IndexEntry>,
+) -> anyhow::Result<()> {
+    index_assignment(path, contents, node, entries)?;
+    index_comment(path, contents, node, entries)?;
+    Ok(())
 }
 
-fn index_variable(
+fn index_assignment(
     _path: &Path,
     contents: &Rope,
     node: &Node,
-) -> anyhow::Result<Option<IndexEntry>> {
+    entries: &mut Vec<IndexEntry>,
+) -> anyhow::Result<()> {
     if !matches!(
         node.node_type(),
         NodeType::BinaryOperator(BinaryOperatorType::LeftAssignment) |
             NodeType::BinaryOperator(BinaryOperatorType::EqualsAssignment)
     ) {
-        return Ok(None);
+        return Ok(());
     }
 
-    let Some(lhs) = node.child_by_field_name("lhs") else {
-        return Ok(None);
+    let lhs = match node.child_by_field_name("lhs") {
+        Some(lhs) => lhs,
+        None => return Ok(()),
     };
 
     let lhs_text = contents.node_slice(&lhs)?.to_string();
 
-    // Super hacky but let's wait until the typed API to do better
+    // The method matching is 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);
+        return Ok(());
     }
 
-    let start = convert_point_to_position(contents, lhs.start_position());
-    let end = convert_point_to_position(contents, lhs.end_position());
+    let Some(rhs) = node.child_by_field_name("rhs") else {
+        return Ok(());
+    };
 
-    Ok(Some(IndexEntry {
-        key: lhs_text.clone(),
-        range: Range { start, end },
-        data: IndexEntryData::Variable { name: lhs_text },
-    }))
+    if rhs.is_function_definition() {
+        // If RHS is a function definition, emit a function symbol
+        let mut arguments = Vec::new();
+        if let Some(parameters) = rhs.child_by_field_name("parameters") {
+            let mut cursor = parameters.walk();
+            for child in parameters.children(&mut cursor) {
+                let name = unwrap!(child.child_by_field_name("name"), None => continue);
+                if name.is_identifier() {
+                    let name = contents.node_slice(&name)?.to_string();
+                    arguments.push(name);
+                }
+            }
+        }
+
+        let start = convert_point_to_position(contents, lhs.start_position());
+        let end = convert_point_to_position(contents, lhs.end_position());
+
+        entries.push(IndexEntry {
+            key: lhs_text.clone(),
+            range: Range { start, end },
+            data: IndexEntryData::Function {
+                name: lhs_text,
+                arguments,
+            },
+        });
+    } else {
+        // Otherwise, emit variable
+        let start = convert_point_to_position(contents, lhs.start_position());
+        let end = convert_point_to_position(contents, lhs.end_position());
+        entries.push(IndexEntry {
+            key: lhs_text.clone(),
+            range: Range { start, end },
+            data: IndexEntryData::Variable { name: lhs_text },
+        });
+    }
+
+    Ok(())
 }
 
-fn index_comment(_path: &Path, contents: &Rope, node: &Node) -> anyhow::Result<Option<IndexEntry>> {
+fn index_comment(
+    _path: &Path,
+    contents: &Rope,
+    node: &Node,
+    entries: &mut Vec<IndexEntry>,
+) -> anyhow::Result<()> {
     // check for comment
-    node.is_comment().into_result()?;
+    if !node.is_comment() {
+        return Ok(());
+    }
 
     // see if it looks like a section
     let comment = contents.node_slice(node)?.to_string();
-    let matches = RE_COMMENT_SECTION
-        .captures(comment.as_str())
-        .into_result()?;
+    let matches = match RE_COMMENT_SECTION.captures(comment.as_str()) {
+        Some(m) => m,
+        None => return Ok(()),
+    };
 
     let level = matches.get(1).into_result()?;
     let title = matches.get(2).into_result()?;
@@ -328,18 +320,21 @@ fn index_comment(_path: &Path, contents: &Rope, node: &Node) -> anyhow::Result<O
 
     // skip things that look like knitr output
     if title.starts_with("----") {
-        return Ok(None);
+        return Ok(());
     }
 
     let start = convert_point_to_position(contents, node.start_position());
     let end = convert_point_to_position(contents, node.end_position());
 
-    Ok(Some(IndexEntry {
+    entries.push(IndexEntry {
         key: title.clone(),
         range: Range::new(start, end),
         data: IndexEntryData::Section { level, title },
-    }))
+    });
+
+    Ok(())
 }
+
 #[cfg(test)]
 mod tests {
     use std::path::PathBuf;
@@ -358,9 +353,7 @@ mod tests {
 
             let mut entries = vec![];
             for node in root.children(&mut cursor) {
-                if let Ok(Some(entry)) = index_node(&path, &doc.contents, &node) {
-                    entries.push(entry);
-                }
+                let _ = index_node(&path, &doc.contents, &node, &mut entries);
             }
             assert_debug_snapshot!(entries);
         };

crates/ark/src/lsp/snapshots/ark__lsp__indexer__tests__index_s7_methods.snap

@@ -47,8 +47,11 @@ expression: entries
                 character: 22,
             },
         },
-        data: Variable {
+        data: Function {
             name: "method(generic, Class)",
+            arguments: [
+                "arg",
+            ],
         },
     },
 ]