Bump tree-sitter-r 3 - New "open"/"close" fields, can use `node.i…

…s_missing()`, no more `..i` diagnostic (#517)

* Bump tree-sitter-r

* Use new `"open"` and `"close"` fields on calls

Greatly simplifies things to be able to access these by name

* Add new test for `..i` no longer producing a diagnostic!

crates/ark/Cargo.toml

@@ -48,7 +48,7 @@ stdext = { path = "../stdext" }
 tokio = { version = "1.26.0", features = ["full"] }
 tower-lsp = "0.19.0"
 tree-sitter = "0.22.6"
-tree-sitter-r = { git = "https://github.com/r-lib/tree-sitter-r", rev = "9d1a68f8f239bc3749a481ac85e2163e24f6362c" }
+tree-sitter-r = { git = "https://github.com/r-lib/tree-sitter-r", rev = "63ee9b10de3b1e4dfaf40e36b45e9ae3c9ed8a4f" }
 uuid = "1.3.0"
 url = "2.4.1"
 walkdir = "2"

crates/ark/src/lsp/completions/sources/common/subset.rs

@@ -9,53 +9,21 @@ use tree_sitter::Node;
 use tree_sitter::Point;
 
 use crate::lsp::traits::point::PointExt;
-use crate::treesitter::NodeType;
-use crate::treesitter::NodeTypeExt;
-
-pub(crate) fn is_within_subset_delimiters(
-    x: &Point,
-    subset_node: &Node,
-    subset_type: &NodeType,
-) -> bool {
-    let (open, close) = match subset_type {
-        NodeType::Subset => ("[", "]"),
-        NodeType::Subset2 => ("[[", "]]"),
-        _ => std::unreachable!(),
-    };
 
+pub(crate) fn is_within_subset_delimiters(x: &Point, subset_node: &Node) -> bool {
     let Some(arguments) = subset_node.child_by_field_name("arguments") else {
         return false;
     };
 
-    let n_children = arguments.child_count();
-
-    if n_children < 2 {
-        return false;
-    }
-
-    let Some(open_node) = arguments.child(1 - 1) else {
+    let Some(open) = arguments.child_by_field_name("open") else {
         return false;
     };
-    let Some(close_node) = arguments.child(n_children - 1) else {
+    let Some(close) = arguments.child_by_field_name("close") else {
         return false;
     };
 
-    // Ensure open and closing nodes are the right type
-    if !matches!(
-        open_node.node_type(),
-        NodeType::Anonymous(kind) if kind == open
-    ) {
-        return false;
-    }
-    if !matches!(
-        close_node.node_type(),
-        NodeType::Anonymous(kind) if kind == close
-    ) {
-        return false;
-    }
-
-    let contains_start = x.is_after_or_equal(open_node.end_position());
-    let contains_end = x.is_before_or_equal(close_node.start_position());
+    let contains_start = x.is_after_or_equal(open.end_position());
+    let contains_end = x.is_before_or_equal(close.start_position());
 
     contains_start && contains_end
 }

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

@@ -27,13 +27,13 @@ pub(super) fn completions_from_subset(
     const ENQUOTE: bool = true;
 
     let mut node = context.node;
-    let mut subset_type = None;
+    let mut needs_completions = false;
 
     loop {
         let node_type = node.node_type();
 
         if matches!(node_type, NodeType::Subset | NodeType::Subset2) {
-            subset_type = Some(node_type);
+            needs_completions = true;
             break;
         }
 
@@ -49,14 +49,14 @@ pub(super) fn completions_from_subset(
         };
     }
 
-    let Some(subset_type) = subset_type else {
+    if !needs_completions {
         // Didn't detect anything worth completing in this context,
         // let other sources add their own candidates instead
         return Ok(None);
     };
 
     // Only provide subset completions if you are actually within `x[<here>]` or `x[[<here>]]`
-    if !is_within_subset_delimiters(&context.point, &node, &subset_type) {
+    if !is_within_subset_delimiters(&context.point, &node) {
         return Ok(None);
     }
 

crates/ark/src/lsp/completions/sources/unique/subset.rs

@@ -85,10 +85,8 @@ fn node_find_object_for_string_subset<'tree>(
         }
     }
 
-    let node_type = node.node_type();
-
     // Only provide subset completions if you are actually within `x[<here>]` or `x[[<here>]]`
-    if !is_within_subset_delimiters(&context.point, &node, &node_type) {
+    if !is_within_subset_delimiters(&context.point, &node) {
         return None;
     }
 

crates/ark/src/lsp/diagnostics.rs

@@ -868,38 +868,52 @@ fn check_unclosed_arguments(
     context: &mut DiagnosticContext,
     diagnostics: &mut Vec<Diagnostic>,
 ) -> Result<bool> {
-    let (open, close) = match node.node_type() {
-        NodeType::Call => ("(", ")"),
-        NodeType::Subset => ("[", "]"),
-        NodeType::Subset2 => ("[[", "]]"),
-        _ => return false.ok(),
+    let Some(open) = find_unclosed_argument_delimiter(node) else {
+        return Ok(false);
     };
 
-    let arguments = unwrap!(node.child_by_field_name("arguments"), None => {
-        return false.ok();
-    });
+    let token = match node.node_type() {
+        NodeType::Call => "(",
+        NodeType::Subset => "[",
+        NodeType::Subset2 => "[[",
+        _ => return Ok(false),
+    };
 
-    let n = arguments.child_count();
-    if n == 0 {
-        return false.ok();
+    let range = open.range();
+    let range = convert_tree_sitter_range_to_lsp_range(context.contents, range);
+    let message = format!("unmatched opening token '{token}'");
+    let diagnostic = Diagnostic::new_simple(range, message);
+    diagnostics.push(diagnostic);
+
+    true.ok()
+}
+
+fn find_unclosed_argument_delimiter(node: Node) -> Option<Node> {
+    if !matches!(
+        node.node_type(),
+        NodeType::Call | NodeType::Subset | NodeType::Subset2
+    ) {
+        return None;
     }
 
-    let lhs = arguments.child(1 - 1).unwrap();
-    let rhs = arguments.child(n - 1).unwrap();
+    let Some(arguments) = node.child_by_field_name("arguments") else {
+        return None;
+    };
 
-    if lhs.node_type() == NodeType::Anonymous(String::from(open)) &&
-        rhs.node_type() == NodeType::Anonymous(String::from(close))
-    {
-        return false.ok();
+    let Some(close) = arguments.child_by_field_name("close") else {
+        return None;
+    };
+
+    // If `close` is `MISSING`, it was error-recovered and this is an unclosed delimiter case
+    if !close.is_missing() {
+        return None;
     }
 
-    let range = lhs.range();
-    let range = convert_tree_sitter_range_to_lsp_range(context.contents, range);
-    let message = format!("unmatched opening bracket '{}'", open);
-    let diagnostic = Diagnostic::new_simple(range, message);
-    diagnostics.push(diagnostic);
+    let Some(open) = arguments.child_by_field_name("open") else {
+        return None;
+    };
 
-    true.ok()
+    Some(open)
 }
 
 fn check_unexpected_assignment_in_if_conditional(
@@ -989,11 +1003,14 @@ mod tests {
     use tower_lsp::lsp_types::Position;
 
     use crate::interface::console_inputs;
+    use crate::lsp::diagnostics::find_unclosed_argument_delimiter;
     use crate::lsp::diagnostics::generate_diagnostics;
     use crate::lsp::diagnostics::is_unmatched_block;
     use crate::lsp::documents::Document;
     use crate::lsp::state::WorldState;
     use crate::test::r_test;
+    use crate::treesitter::NodeType;
+    use crate::treesitter::NodeTypeExt;
 
     // Default state that includes installed packages and default scopes.
     static DEFAULT_STATE: Lazy<WorldState> = Lazy::new(|| current_state());
@@ -1008,6 +1025,30 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_unmatched_call_delimiter() {
+        let document = Document::new("match(a, b", None);
+        let node = document.ast.root_node().named_child(0).unwrap();
+        let open = find_unclosed_argument_delimiter(node).unwrap();
+        assert_eq!(open.node_type(), NodeType::Anonymous(String::from("(")));
+        assert_eq!(open.start_byte(), 5);
+        assert_eq!(open.end_byte(), 6);
+
+        let document = Document::new("foo[a, b", None);
+        let node = document.ast.root_node().named_child(0).unwrap();
+        let open = find_unclosed_argument_delimiter(node).unwrap();
+        assert_eq!(open.node_type(), NodeType::Anonymous(String::from("[")));
+        assert_eq!(open.start_byte(), 3);
+        assert_eq!(open.end_byte(), 4);
+
+        let document = Document::new("foo[[a, b", None);
+        let node = document.ast.root_node().named_child(0).unwrap();
+        let open = find_unclosed_argument_delimiter(node).unwrap();
+        assert_eq!(open.node_type(), NodeType::Anonymous(String::from("[[")));
+        assert_eq!(open.start_byte(), 3);
+        assert_eq!(open.end_byte(), 5);
+    }
+
     #[test]
     fn test_unmatched_braces() {
         let document = Document::new("{", None);
@@ -1079,6 +1120,18 @@ mod tests {
         })
     }
 
+    #[test]
+    fn test_no_diagnostic_for_dot_dot_i() {
+        r_test(|| {
+            let text = "..1 + ..2 + 3";
+            let document = Document::new(text, None);
+
+            let diagnostics = generate_diagnostics(document, DEFAULT_STATE.clone());
+
+            assert!(diagnostics.is_empty());
+        })
+    }
+
     #[test]
     fn test_no_diagnostic_for_rhs_of_extractor() {
         r_test(|| {

crates/ark/src/lsp/selection_range.rs

@@ -77,41 +77,13 @@ fn range_for_node(node: Node) -> Range {
 // next selection after that be the entire call
 //
 // This also applies to subset and subset2, i.e. `[a, b, c]` and `[[a, b, c]]`.
-//
-// This is another place where it would be great to be able to access the delimiters
-// by field name, as it would simplify the logic significantly and eventually allow a
-// rowan based cast to a structured `Arguments` type followed by an `Arguments` specific
-// method like `node.opening_delimiter()`.
-// https://github.com/r-lib/tree-sitter-r/issues/91
 fn range_for_arguments(node: Node) -> Range {
-    let Some(parent) = node.parent() else {
+    let Some(open) = node.child_by_field_name("open") else {
         return node.range();
     };
-
-    let (open_delimiter, close_delimiter) = match parent.node_type() {
-        NodeType::Call => (String::from("("), String::from(")")),
-        NodeType::Subset => (String::from("["), String::from("]")),
-        NodeType::Subset2 => (String::from("[["), String::from("]]")),
-        _ => return node.range(),
-    };
-    let open_delimiter = NodeType::Anonymous(open_delimiter);
-    let close_delimiter = NodeType::Anonymous(close_delimiter);
-
-    let n = node.child_count();
-
-    if n < 2 {
+    let Some(close) = node.child_by_field_name("close") else {
         return node.range();
-    }
-
-    let open = node.child(1 - 1).unwrap();
-    let close = node.child(n - 1).unwrap();
-
-    if open.node_type() != open_delimiter {
-        return node.range();
-    }
-    if close.node_type() != close_delimiter {
-        return node.range();
-    }
+    };
 
     let start_byte = open.end_byte();
     let start_point = open.end_position();

crates/ark/src/lsp/signature_help.rs

@@ -327,21 +327,12 @@ fn is_within_call_parentheses(x: &Point, node: &Node) -> bool {
         return false;
     };
 
-    let n_children = arguments.child_count();
-    if n_children < 2 {
-        log::error!("`arguments` node only has {n_children} children.");
+    let Some(open) = arguments.child_by_field_name("open") else {
         return false;
-    }
-
-    let open = arguments.child(1 - 1).unwrap();
-    let close = arguments.child(n_children - 1).unwrap();
-
-    if open.node_type() != NodeType::Anonymous(String::from("(")) {
-        return false;
-    }
-    if close.node_type() != NodeType::Anonymous(String::from(")")) {
+    };
+    let Some(close) = arguments.child_by_field_name("close") else {
         return false;
-    }
+    };
 
     x.is_after_or_equal(open.end_position()) && x.is_before_or_equal(close.start_position())
 }