diff --git a/Cargo.lock b/Cargo.lock index 52fc743ab..ef6ab41b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3201,7 +3201,7 @@ dependencies = [ [[package]] name = "tree-sitter-r" version = "0.20.1" -source = "git+https://github.com/r-lib/tree-sitter-r?rev=9d1a68f8f239bc3749a481ac85e2163e24f6362c#9d1a68f8f239bc3749a481ac85e2163e24f6362c" +source = "git+https://github.com/r-lib/tree-sitter-r?rev=63ee9b10de3b1e4dfaf40e36b45e9ae3c9ed8a4f#63ee9b10de3b1e4dfaf40e36b45e9ae3c9ed8a4f" dependencies = [ "cc", "tree-sitter", diff --git a/crates/ark/Cargo.toml b/crates/ark/Cargo.toml index 1be86d321..1fddcf8e0 100644 --- a/crates/ark/Cargo.toml +++ b/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" diff --git a/crates/ark/src/lsp/completions/sources/common/subset.rs b/crates/ark/src/lsp/completions/sources/common/subset.rs index ac2aa8d97..cee1dc195 100644 --- a/crates/ark/src/lsp/completions/sources/common/subset.rs +++ b/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 } diff --git a/crates/ark/src/lsp/completions/sources/composite/subset.rs b/crates/ark/src/lsp/completions/sources/composite/subset.rs index 6421ae77b..7bba1e302 100644 --- a/crates/ark/src/lsp/completions/sources/composite/subset.rs +++ b/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[]` or `x[[]]` - if !is_within_subset_delimiters(&context.point, &node, &subset_type) { + if !is_within_subset_delimiters(&context.point, &node) { return Ok(None); } diff --git a/crates/ark/src/lsp/completions/sources/unique/subset.rs b/crates/ark/src/lsp/completions/sources/unique/subset.rs index e0b5ef092..4cb61c353 100644 --- a/crates/ark/src/lsp/completions/sources/unique/subset.rs +++ b/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[]` or `x[[]]` - if !is_within_subset_delimiters(&context.point, &node, &node_type) { + if !is_within_subset_delimiters(&context.point, &node) { return None; } diff --git a/crates/ark/src/lsp/diagnostics.rs b/crates/ark/src/lsp/diagnostics.rs index 1c3cea8f3..71e31a3aa 100644 --- a/crates/ark/src/lsp/diagnostics.rs +++ b/crates/ark/src/lsp/diagnostics.rs @@ -868,38 +868,52 @@ fn check_unclosed_arguments( context: &mut DiagnosticContext, diagnostics: &mut Vec, ) -> Result { - 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 { + 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 = 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(|| { diff --git a/crates/ark/src/lsp/selection_range.rs b/crates/ark/src/lsp/selection_range.rs index 339efd6fc..356fc6010 100644 --- a/crates/ark/src/lsp/selection_range.rs +++ b/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(); diff --git a/crates/ark/src/lsp/signature_help.rs b/crates/ark/src/lsp/signature_help.rs index a30f83eb7..55323f710 100644 --- a/crates/ark/src/lsp/signature_help.rs +++ b/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()) }