From 15ef29e5a72c428770cd3f693e31fd493d92d4d8 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Fri, 30 Aug 2024 11:29:04 -0400 Subject: [PATCH] Implement "string subset" completions (#493) * Add tests to show that we enquote in composite subset completions * Extract out common subset helper * Add a few more treesitter `Node` utilities * Implement string subset completions as a special case of unique string completions * Custom `r_is_matrix()` just to be sure about the definition * Add support for extracting `colnames()` from a matrix * Add test for `matrix[, ""]` extracting column names * Adapt to post rebase `main` --- crates/ark/src/lsp/completions/sources.rs | 1 + .../ark/src/lsp/completions/sources/common.rs | 8 + .../lsp/completions/sources/common/subset.rs | 61 ++++ .../completions/sources/composite/subset.rs | 50 +--- .../ark/src/lsp/completions/sources/unique.rs | 1 + .../completions/sources/unique/file_path.rs | 6 +- .../lsp/completions/sources/unique/string.rs | 18 +- .../lsp/completions/sources/unique/subset.rs | 270 ++++++++++++++++++ .../ark/src/lsp/completions/sources/utils.rs | 109 ++++++- crates/ark/src/treesitter.rs | 20 ++ crates/harp/src/utils.rs | 13 +- 11 files changed, 496 insertions(+), 61 deletions(-) create mode 100644 crates/ark/src/lsp/completions/sources/common.rs create mode 100644 crates/ark/src/lsp/completions/sources/common/subset.rs create mode 100644 crates/ark/src/lsp/completions/sources/unique/subset.rs diff --git a/crates/ark/src/lsp/completions/sources.rs b/crates/ark/src/lsp/completions/sources.rs index b33752593..a4ed80c8b 100644 --- a/crates/ark/src/lsp/completions/sources.rs +++ b/crates/ark/src/lsp/completions/sources.rs @@ -5,6 +5,7 @@ // // +mod common; mod composite; mod unique; mod utils; diff --git a/crates/ark/src/lsp/completions/sources/common.rs b/crates/ark/src/lsp/completions/sources/common.rs new file mode 100644 index 000000000..378d551e9 --- /dev/null +++ b/crates/ark/src/lsp/completions/sources/common.rs @@ -0,0 +1,8 @@ +// +// common.rs +// +// Copyright (C) 2024 Posit Software, PBC. All rights reserved. +// +// + +pub(crate) mod subset; diff --git a/crates/ark/src/lsp/completions/sources/common/subset.rs b/crates/ark/src/lsp/completions/sources/common/subset.rs new file mode 100644 index 000000000..ac2aa8d97 --- /dev/null +++ b/crates/ark/src/lsp/completions/sources/common/subset.rs @@ -0,0 +1,61 @@ +// +// subset.rs +// +// Copyright (C) 2024 Posit Software, PBC. All rights reserved. +// +// + +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!(), + }; + + 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 { + return false; + }; + let Some(close_node) = arguments.child(n_children - 1) 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()); + + 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 43b02217d..6421ae77b 100644 --- a/crates/ark/src/lsp/completions/sources/composite/subset.rs +++ b/crates/ark/src/lsp/completions/sources/composite/subset.rs @@ -7,12 +7,10 @@ use anyhow::Result; use tower_lsp::lsp_types::CompletionItem; -use tree_sitter::Node; -use tree_sitter::Point; +use crate::lsp::completions::sources::common::subset::is_within_subset_delimiters; use crate::lsp::completions::sources::utils::completions_from_evaluated_object_names; use crate::lsp::document_context::DocumentContext; -use crate::lsp::traits::point::PointExt; use crate::lsp::traits::rope::RopeExt; use crate::treesitter::NodeType; use crate::treesitter::NodeTypeExt; @@ -73,50 +71,6 @@ pub(super) fn completions_from_subset( completions_from_evaluated_object_names(&text, ENQUOTE) } -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!(), - }; - - 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 { - return false; - }; - let Some(close_node) = arguments.child(n_children - 1) 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()); - - contains_start && contains_end -} - #[cfg(test)] mod tests { use harp::eval::RParseEvalOptions; @@ -148,9 +102,11 @@ mod tests { let completion = completions.get(0).unwrap(); assert_eq!(completion.label, "b".to_string()); + assert_eq!(completion.insert_text, Some("\"b\"".to_string())); let completion = completions.get(1).unwrap(); assert_eq!(completion.label, "a".to_string()); + assert_eq!(completion.insert_text, Some("\"a\"".to_string())); // Right before the `[` let point = Point { row: 0, column: 3 }; diff --git a/crates/ark/src/lsp/completions/sources/unique.rs b/crates/ark/src/lsp/completions/sources/unique.rs index 825b6637f..2e1cd7414 100644 --- a/crates/ark/src/lsp/completions/sources/unique.rs +++ b/crates/ark/src/lsp/completions/sources/unique.rs @@ -12,6 +12,7 @@ mod extractor; mod file_path; mod namespace; mod string; +mod subset; use anyhow::Result; use colon::completions_from_single_colon; diff --git a/crates/ark/src/lsp/completions/sources/unique/file_path.rs b/crates/ark/src/lsp/completions/sources/unique/file_path.rs index 10a02e320..d7a880b6a 100644 --- a/crates/ark/src/lsp/completions/sources/unique/file_path.rs +++ b/crates/ark/src/lsp/completions/sources/unique/file_path.rs @@ -21,8 +21,10 @@ use crate::lsp::completions::sources::utils::set_sort_text_by_words_first; use crate::lsp::document_context::DocumentContext; use crate::lsp::traits::rope::RopeExt; -pub(super) fn completions_from_file_path(context: &DocumentContext) -> Result> { - log::info!("completions_from_file_path()"); +pub(super) fn completions_from_string_file_path( + context: &DocumentContext, +) -> Result> { + log::info!("completions_from_string_file_path()"); let mut completions: Vec = vec![]; diff --git a/crates/ark/src/lsp/completions/sources/unique/string.rs b/crates/ark/src/lsp/completions/sources/unique/string.rs index 0522bb28a..ed462bbf7 100644 --- a/crates/ark/src/lsp/completions/sources/unique/string.rs +++ b/crates/ark/src/lsp/completions/sources/unique/string.rs @@ -8,7 +8,8 @@ use anyhow::Result; use tower_lsp::lsp_types::CompletionItem; -use super::file_path::completions_from_file_path; +use super::file_path::completions_from_string_file_path; +use crate::lsp::completions::sources::unique::subset::completions_from_string_subset; use crate::lsp::document_context::DocumentContext; use crate::treesitter::NodeTypeExt; @@ -27,9 +28,9 @@ pub fn completions_from_string(context: &DocumentContext) -> Result = vec![]; // Return empty set if we are here due to a trigger character like `$`. @@ -38,8 +39,15 @@ pub fn completions_from_string(context: &DocumentContext) -> Result"]`. This is a very unique + // case that takes priority over file path completions. + if let Some(mut candidates) = completions_from_string_subset(context)? { + completions.append(&mut candidates); + return Ok(Some(completions)); + } + + // If no special string cases are hit, we show file path completions + completions.append(&mut completions_from_string_file_path(context)?); Ok(Some(completions)) } diff --git a/crates/ark/src/lsp/completions/sources/unique/subset.rs b/crates/ark/src/lsp/completions/sources/unique/subset.rs new file mode 100644 index 000000000..64d4bb3bb --- /dev/null +++ b/crates/ark/src/lsp/completions/sources/unique/subset.rs @@ -0,0 +1,270 @@ +// +// subset.rs +// +// Copyright (C) 2024 Posit Software, PBC. All rights reserved. +// +// + +use anyhow::Result; +use ropey::Rope; +use tower_lsp::lsp_types::CompletionItem; +use tree_sitter::Node; + +use crate::lsp::completions::sources::common::subset::is_within_subset_delimiters; +use crate::lsp::completions::sources::utils::completions_from_evaluated_object_names; +use crate::lsp::document_context::DocumentContext; +use crate::lsp::traits::rope::RopeExt; +use crate::treesitter::NodeTypeExt; + +/// Checks for `[` and `[[` completions when the user is inside a `""` +/// +/// This is a _unique_ completions case where we may show the user the object's names if: +/// - We are inside a top level string, like `x[""]` +/// - We are inside a simple `c()` call, like `x[c("col", "")]` +/// +/// The latter is just a useful heuristic. For more complex function calls, we don't want +/// to populate object names because they won't make sense, like `x[match(foo, "")]`. +/// +/// Different from `composite::subset::completions_from_subset()`, which applies outside +/// of `""`, enquotes its completion items, and is composite so it meshes with other +/// generic completions. We consider this a completely different path. +pub(super) fn completions_from_string_subset( + context: &DocumentContext, +) -> Result>> { + log::info!("completions_from_string_subset()"); + + // Already inside a string + const ENQUOTE: bool = false; + + // i.e. find `x` in `x[""]` or `x[c("foo", "")]` + let Some(node) = node_find_object_for_string_subset(context.node, context) else { + return Ok(None); + }; + + // It looks like we should provide subset completions. Regardless of what happens + // when getting object names, we should at least return an empty set to stop further + // completion sources from running. + let mut completions: Vec = vec![]; + + let text = context.document.contents.node_slice(&node)?.to_string(); + + if let Some(mut candidates) = completions_from_evaluated_object_names(&text, ENQUOTE)? { + completions.append(&mut candidates); + } + + Ok(Some(completions)) +} + +fn node_find_object_for_string_subset<'tree>( + mut node: Node<'tree>, + context: &DocumentContext, +) -> Option> { + if !node.is_string() { + return None; + } + + node = match node_find_parent_call(node) { + Some(node) => node, + None => return None, + }; + + if node.is_call() { + if !node_is_c_call(&node, &context.document.contents) { + // Inside a call that isn't `c()` + return None; + } + + node = match node_find_parent_call(node) { + Some(node) => node, + None => return None, + }; + + if !node.is_subset() && !node.is_subset2() { + return None; + } + } + + 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) { + return None; + } + + // We know `node` is the subset or subset2 node of interest. Return its "function", + // i.e. likely the object name of interest to extract names for. + node = match node.child_by_field_name("function") { + Some(node) => node, + None => return None, + }; + + if !node.is_identifier() { + return None; + } + + return Some(node); +} + +fn node_find_parent_call(x: Node) -> Option { + // Find the `Argument` node + let Some(x) = x.parent() else { + return None; + }; + if !x.is_argument() { + return None; + } + + // Find the `Arguments` node + let Some(x) = x.parent() else { + return None; + }; + if !x.is_arguments() { + return None; + } + + // Find the call node - can be a generic `Call`, `Subset`, or `Subset2`. + // All 3 purposefully share the same tree structure. + let Some(x) = x.parent() else { + return None; + }; + if !x.is_call() && !x.is_subset() && !x.is_subset2() { + return None; + } + + Some(x) +} + +fn node_is_c_call(x: &Node, contents: &Rope) -> bool { + if !x.is_call() { + return false; + } + + let Some(x) = x.child_by_field_name("function") else { + return false; + }; + + if !x.is_identifier() { + return false; + } + + let Ok(text) = contents.node_slice(&x) else { + log::error!("Can't slice `contents`."); + return false; + }; + + // Is the call `c()`? + text == "c" +} + +#[cfg(test)] +mod tests { + use harp::eval::parse_eval_global; + + use crate::lsp::completions::sources::unique::subset::completions_from_string_subset; + use crate::lsp::document_context::DocumentContext; + use crate::lsp::documents::Document; + use crate::test::point_from_cursor; + use crate::test::r_test; + + #[test] + fn test_string_subset_completions() { + r_test(|| { + // Set up a list with names + parse_eval_global("foo <- list(b = 1, a = 2)").unwrap(); + + // Inside top level `""` + let (text, point) = point_from_cursor(r#"foo["@"]"#); + let document = Document::new(text.as_str(), None); + let context = DocumentContext::new(&document, point, None); + + let completions = completions_from_string_subset(&context).unwrap().unwrap(); + assert_eq!(completions.len(), 2); + + let completion = completions.get(0).unwrap(); + assert_eq!(completion.label, "b".to_string()); + // Not enquoting, so uses `label` directly + assert_eq!(completion.insert_text, None); + + let completion = completions.get(1).unwrap(); + assert_eq!(completion.label, "a".to_string()); + // Not enquoting, so uses `label` directly + assert_eq!(completion.insert_text, None); + + // Inside `""` in `[[` + let (text, point) = point_from_cursor(r#"foo[["@"]]"#); + let document = Document::new(text.as_str(), None); + let context = DocumentContext::new(&document, point, None); + let completions = completions_from_string_subset(&context).unwrap().unwrap(); + assert_eq!(completions.len(), 2); + + // Inside `""` as second argument + let (text, point) = point_from_cursor(r#"foo[, "@"]"#); + let document = Document::new(text.as_str(), None); + let context = DocumentContext::new(&document, point, None); + let completions = completions_from_string_subset(&context).unwrap().unwrap(); + assert_eq!(completions.len(), 2); + + // Inside `""` inside `c()` + let (text, point) = point_from_cursor(r#"foo[c("@")]"#); + let document = Document::new(text.as_str(), None); + let context = DocumentContext::new(&document, point, None); + let completions = completions_from_string_subset(&context).unwrap().unwrap(); + assert_eq!(completions.len(), 2); + + // Inside `""` inside `c()` with another string already specified + let (text, point) = point_from_cursor(r#"foo[c("a", "@")]"#); + let document = Document::new(text.as_str(), None); + let context = DocumentContext::new(&document, point, None); + let completions = completions_from_string_subset(&context).unwrap().unwrap(); + assert_eq!(completions.len(), 2); + + // Inside `""` inside `fn()` - no completions from string subset! + // Instead file path completions should kick in, because this is an arbitrary + // function call so subset completions don't make sense, but file ones might. + let (text, point) = point_from_cursor(r#"foo[fn("@")]"#); + let document = Document::new(text.as_str(), None); + let context = DocumentContext::new(&document, point, None); + let completions = completions_from_string_subset(&context).unwrap(); + assert!(completions.is_none()); + + // Right before the `[` + let (text, point) = point_from_cursor(r#"foo@[""]"#); + let document = Document::new(text.as_str(), None); + let context = DocumentContext::new(&document, point, None); + let completions = completions_from_string_subset(&context).unwrap(); + assert!(completions.is_none()); + + // A fake object that we can't get object names for. + // It _looks_ like we want string completions though, so we return an empty set. + let (text, point) = point_from_cursor(r#"not_real["@"]"#); + let document = Document::new(text.as_str(), None); + let context = DocumentContext::new(&document, point, None); + let completions = completions_from_string_subset(&context).unwrap().unwrap(); + assert!(completions.is_empty()); + + // Clean up + parse_eval_global("remove(foo)").unwrap(); + }) + } + + #[test] + fn test_string_subset_completions_on_matrix() { + r_test(|| { + // Set up a list with names + parse_eval_global("foo <- array(1, dim = c(2, 2))").unwrap(); + parse_eval_global("colnames(foo) <- c('a', 'b')").unwrap(); + + let (text, point) = point_from_cursor(r#"foo[, "@"]"#); + let document = Document::new(text.as_str(), None); + let context = DocumentContext::new(&document, point, None); + + let completions = completions_from_string_subset(&context).unwrap().unwrap(); + assert_eq!(completions.len(), 2); + assert_eq!(completions.get(0).unwrap().label, "a".to_string()); + assert_eq!(completions.get(1).unwrap().label, "b".to_string()); + + // Clean up + parse_eval_global("remove(foo)").unwrap(); + }) + } +} diff --git a/crates/ark/src/lsp/completions/sources/utils.rs b/crates/ark/src/lsp/completions/sources/utils.rs index e53a0c08f..562af6acd 100644 --- a/crates/ark/src/lsp/completions/sources/utils.rs +++ b/crates/ark/src/lsp/completions/sources/utils.rs @@ -201,7 +201,14 @@ pub(super) fn completions_from_evaluated_object_names( }, }; - Ok(Some(completions_from_object_names(object, name, enquote)?)) + let completions = if harp::utils::r_is_matrix(object.sexp) { + // Special case just for 2D arrays + completions_from_object_colnames(object, name, enquote)? + } else { + completions_from_object_names(object, name, enquote)? + }; + + Ok(Some(completions)) } pub(super) fn completions_from_object_names( @@ -209,18 +216,35 @@ pub(super) fn completions_from_object_names( name: &str, enquote: bool, ) -> Result> { - log::info!("completions_from_object_names({object:?})"); + completions_from_object_names_impl(object, name, enquote, "names") +} + +pub(super) fn completions_from_object_colnames( + object: RObject, + name: &str, + enquote: bool, +) -> Result> { + completions_from_object_names_impl(object, name, enquote, "colnames") +} + +fn completions_from_object_names_impl( + object: RObject, + name: &str, + enquote: bool, + function: &str, +) -> Result> { + log::info!("completions_from_object_names_impl({object:?})"); let mut completions = vec![]; unsafe { - let variable_names = RFunction::new("base", "names") + let element_names = RFunction::new("base", function) .add(object) .call()? .to::>()?; - for variable_name in variable_names { - match completion_item_from_data_variable(&variable_name, name, enquote) { + for element_name in element_names { + match completion_item_from_data_variable(&element_name, name, enquote) { Ok(item) => completions.push(item), Err(err) => log::error!("{err:?}"), } @@ -232,12 +256,15 @@ pub(super) fn completions_from_object_names( #[cfg(test)] mod tests { + use harp::eval::parse_eval_global; use tree_sitter::Point; use crate::lsp::completions::sources::utils::call_node_position_type; + use crate::lsp::completions::sources::utils::completions_from_evaluated_object_names; use crate::lsp::completions::sources::utils::CallNodePositionType; use crate::lsp::document_context::DocumentContext; use crate::lsp::documents::Document; + use crate::test::r_test; use crate::treesitter::NodeType; use crate::treesitter::NodeTypeExt; @@ -391,4 +418,76 @@ mod tests { CallNodePositionType::Ambiguous ); } + + #[test] + fn test_completions_from_evaluated_object_names() { + r_test(|| { + // Vector with names + parse_eval_global("x <- 1:2").unwrap(); + parse_eval_global("names(x) <- c('a', 'b')").unwrap(); + + let completions = completions_from_evaluated_object_names("x", false) + .unwrap() + .unwrap(); + assert_eq!(completions.len(), 2); + assert_eq!(completions.get(0).unwrap().label, String::from("a")); + assert_eq!(completions.get(1).unwrap().label, String::from("b")); + + parse_eval_global("remove(x)").unwrap(); + + // Data frame + parse_eval_global("x <- data.frame(a = 1, b = 2, c = 3)").unwrap(); + + let completions = completions_from_evaluated_object_names("x", false) + .unwrap() + .unwrap(); + assert_eq!(completions.len(), 3); + assert_eq!(completions.get(0).unwrap().label, String::from("a")); + assert_eq!(completions.get(1).unwrap().label, String::from("b")); + assert_eq!(completions.get(2).unwrap().label, String::from("c")); + + parse_eval_global("remove(x)").unwrap(); + + // 1D array with names + parse_eval_global("x <- array(1:2)").unwrap(); + parse_eval_global("names(x) <- c('a', 'b')").unwrap(); + + let completions = completions_from_evaluated_object_names("x", false) + .unwrap() + .unwrap(); + assert_eq!(completions.len(), 2); + assert_eq!(completions.get(0).unwrap().label, String::from("a")); + assert_eq!(completions.get(1).unwrap().label, String::from("b")); + + parse_eval_global("remove(x)").unwrap(); + + // Matrix with column names + parse_eval_global("x <- array(1, dim = c(1, 1))").unwrap(); + parse_eval_global("rownames(x) <- 'a'").unwrap(); + parse_eval_global("colnames(x) <- 'b'").unwrap(); + + let completions = completions_from_evaluated_object_names("x", false) + .unwrap() + .unwrap(); + assert_eq!(completions.len(), 1); + assert_eq!(completions.get(0).unwrap().label, String::from("b")); + + parse_eval_global("remove(x)").unwrap(); + + // 3D array with column names + // We currently decide not to return any names here. It is typically quite + // ambiguous which axis's names you'd want when working with >=3D arrays. + // But we did find an object, so we return an empty vector. + parse_eval_global("x <- array(1, dim = c(1, 1, 1))").unwrap(); + parse_eval_global("rownames(x) <- 'a'").unwrap(); + parse_eval_global("colnames(x) <- 'b'").unwrap(); + + let completions = completions_from_evaluated_object_names("x", false) + .unwrap() + .unwrap(); + assert!(completions.is_empty()); + + parse_eval_global("remove(x)").unwrap(); + }) + } } diff --git a/crates/ark/src/treesitter.rs b/crates/ark/src/treesitter.rs index f1409dfeb..fd05dc53a 100644 --- a/crates/ark/src/treesitter.rs +++ b/crates/ark/src/treesitter.rs @@ -302,10 +302,14 @@ pub trait NodeTypeExt: Sized { fn is_identifier_or_string(&self) -> bool; fn is_keyword(&self) -> bool; fn is_call(&self) -> bool; + fn is_subset(&self) -> bool; + fn is_subset2(&self) -> bool; fn is_comment(&self) -> bool; fn is_braced_expression(&self) -> bool; fn is_function_definition(&self) -> bool; fn is_if_statement(&self) -> bool; + fn is_argument(&self) -> bool; + fn is_arguments(&self) -> bool; fn is_namespace_operator(&self) -> bool; fn is_namespace_internal_operator(&self) -> bool; fn is_unary_operator(&self) -> bool; @@ -353,6 +357,14 @@ impl NodeTypeExt for Node<'_> { self.node_type() == NodeType::Call } + fn is_subset(&self) -> bool { + self.node_type() == NodeType::Subset + } + + fn is_subset2(&self) -> bool { + self.node_type() == NodeType::Subset2 + } + fn is_comment(&self) -> bool { self.node_type() == NodeType::Comment } @@ -369,6 +381,14 @@ impl NodeTypeExt for Node<'_> { self.node_type() == NodeType::IfStatement } + fn is_argument(&self) -> bool { + self.node_type() == NodeType::Argument + } + + fn is_arguments(&self) -> bool { + self.node_type() == NodeType::Arguments + } + fn is_namespace_operator(&self) -> bool { matches!(self.node_type(), NodeType::NamespaceOperator(_)) } diff --git a/crates/harp/src/utils.rs b/crates/harp/src/utils.rs index f5a2560ec..d68ce51b0 100644 --- a/crates/harp/src/utils.rs +++ b/crates/harp/src/utils.rs @@ -193,8 +193,17 @@ pub fn r_is_simple_vector(value: SEXP) -> bool { } } -pub fn r_is_matrix(value: SEXP) -> bool { - unsafe { Rf_isMatrix(value) == Rboolean_TRUE } +/// Is `object` a matrix? +/// +/// Notably returns `false` for 1D arrays and >=3D arrays. +pub fn r_is_matrix(object: SEXP) -> bool { + let dim = r_dim(object); + + if dim == r_null() { + return false; + } + + r_length(dim) == 2 } pub fn r_classes(value: SEXP) -> Option {