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 0953b5e7f..63aaed7a8 100644 --- a/crates/ark/src/lsp/completions/sources/unique/file_path.rs +++ b/crates/ark/src/lsp/completions/sources/unique/file_path.rs @@ -8,11 +8,9 @@ use std::env::current_dir; use std::path::PathBuf; -use harp::object::RObject; -use harp::string::r_string_decode; +use harp::utils::r_is_string; use harp::utils::r_normalize_path; use stdext::unwrap; -use stdext::IntoResult; use tower_lsp::lsp_types::CompletionItem; use tree_sitter::Node; @@ -33,13 +31,24 @@ pub(super) fn completions_from_string_file_path( // // NOTE: This includes the quotation characters on the string, and so // also includes any internal escapes! We need to decode the R string - // before searching the path entries. + // by parsing it before searching the path entries. let token = context.document.contents.node_slice(&node)?.to_string(); - let contents = unsafe { r_string_decode(token.as_str()).into_result()? }; - log::trace!("String value (decoded): {}", contents); + + // It's entirely possible that we can fail to parse the string, `R_ParseVector()` + // can fail in various ways. We silently swallow these because they are unlikely + // to report to real file paths and just bail (posit-dev/positron#6584). + let Ok(contents) = harp::parse_expr(&token) else { + return Ok(completions); + }; + + // Double check that parsing gave a string. It should, because `node` points to + // a tree-sitter string node. + if !r_is_string(contents.sexp) { + return Ok(completions); + } // Use R to normalize the path. - let path = r_normalize_path(RObject::from(contents))?; + let path = r_normalize_path(contents)?; // parse the file path and get the directory component let mut path = PathBuf::from(path.as_str()); @@ -82,3 +91,28 @@ pub(super) fn completions_from_string_file_path( Ok(completions) } + +#[cfg(test)] +mod tests { + use crate::fixtures::point_from_cursor; + use crate::lsp::completions::sources::unique::file_path::completions_from_string_file_path; + use crate::lsp::document_context::DocumentContext; + use crate::lsp::documents::Document; + use crate::r_task; + use crate::treesitter::node_find_string; + + #[test] + fn test_unparseable_string() { + // https://github.com/posit-dev/positron/issues/6584 + r_task(|| { + // "\R" is an unrecognized escape character and `R_ParseVector()` errors on it + let (text, point) = point_from_cursor(r#" ".\R\utils.R@" "#); + let document = Document::new(text.as_str(), None); + let context = DocumentContext::new(&document, point, None); + let node = node_find_string(&context.node).unwrap(); + + let completions = completions_from_string_file_path(&node, &context).unwrap(); + assert_eq!(completions.len(), 0); + }) + } +} diff --git a/crates/harp/src/lib.rs b/crates/harp/src/lib.rs index 1b71340e0..6d34887d9 100644 --- a/crates/harp/src/lib.rs +++ b/crates/harp/src/lib.rs @@ -33,7 +33,6 @@ pub mod routines; pub mod session; pub mod size; pub mod source; -pub mod string; pub mod symbol; pub mod sys; pub mod table; diff --git a/crates/harp/src/string.rs b/crates/harp/src/string.rs deleted file mode 100644 index 5aff5d348..000000000 --- a/crates/harp/src/string.rs +++ /dev/null @@ -1,48 +0,0 @@ -// -// string.rs -// -// Copyright (C) 2022 Posit Software, PBC. All rights reserved. -// -// - -use libr::ParseStatus; -use libr::R_NaString; -use libr::R_NilValue; -use libr::R_ParseVector; -use libr::Rf_xlength; -use libr::EXPRSXP; -use libr::SEXP; -use libr::STRSXP; -use libr::VECTOR_ELT; - -use crate::object::RObject; -use crate::protect::RProtect; -use crate::r_string; -use crate::utils::r_typeof; - -// Given a quoted R string, decode it to get the string value. -pub unsafe fn r_string_decode(code: &str) -> Option { - // convert to R string - let mut protect = RProtect::new(); - let code = r_string!(code, &mut protect); - - // parse into vector - let mut ps: ParseStatus = 0; - let result = protect.add(R_ParseVector(code, -1, &mut ps, R_NilValue)); - - // check for string in result - if r_typeof(result) == EXPRSXP { - if Rf_xlength(result) != 0 { - let value = VECTOR_ELT(result, 0); - if r_typeof(value) == STRSXP { - return RObject::view(value).to::().ok(); - } - } - } - - None -} - -pub fn r_is_string(x: SEXP) -> bool { - unsafe { r_typeof(x) == STRSXP && Rf_xlength(x) == 1 && x != R_NaString } -} diff --git a/crates/harp/src/utils.rs b/crates/harp/src/utils.rs index 35662ed57..33a89419e 100644 --- a/crates/harp/src/utils.rs +++ b/crates/harp/src/utils.rs @@ -36,7 +36,6 @@ use crate::r_char; use crate::r_lang; use crate::r_null; use crate::r_symbol; -use crate::string::r_is_string; use crate::symbol::RSymbol; use crate::vector::CharacterVector; use crate::vector::IntegerVector; @@ -154,6 +153,10 @@ pub fn r_is_simple_vector(value: SEXP) -> bool { } } +pub fn r_is_string(x: SEXP) -> bool { + r_typeof(x) == STRSXP && r_length(x) == 1 && x != r_str_na() +} + /// Is `object` a matrix? /// /// Notably returns `false` for 1D arrays and >=3D arrays.