diff --git a/R/completion.R b/R/completion.R index b2ba3c75..1015538c 100644 --- a/R/completion.R +++ b/R/completion.R @@ -313,17 +313,17 @@ workspace_completion <- function(workspace, token, } scope_completion_symbols_xpath <- paste( - "*[self::FUNCTION or self::OP-LAMBDA]/following-sibling::SYMBOL_FORMALS", + "(*|descendant-or-self::exprlist/*)[self::FUNCTION or self::OP-LAMBDA]/following-sibling::SYMBOL_FORMALS", + "(*|descendant-or-self::exprlist/*)/LEFT_ASSIGN[not(following-sibling::expr/*[self::FUNCTION or self::OP-LAMBDA])]/preceding-sibling::expr[count(*)=1]/SYMBOL", + "(*|descendant-or-self::exprlist/*)/RIGHT_ASSIGN[not(preceding-sibling::expr/*[self::FUNCTION or self::OP-LAMBDA])]/following-sibling::expr[count(*)=1]/SYMBOL", + "(*|descendant-or-self::exprlist/*)/EQ_ASSIGN[not(following-sibling::expr/*[self::FUNCTION or self::OP-LAMBDA])]/preceding-sibling::expr[count(*)=1]/SYMBOL", "forcond/SYMBOL", - "*/LEFT_ASSIGN[not(following-sibling::expr/*[self::FUNCTION or self::OP-LAMBDA])]/preceding-sibling::expr[count(*)=1]/SYMBOL", - "*/RIGHT_ASSIGN[not(preceding-sibling::expr/*[self::FUNCTION or self::OP-LAMBDA])]/following-sibling::expr[count(*)=1]/SYMBOL", - "*/EQ_ASSIGN[not(following-sibling::expr/*[self::FUNCTION or self::OP-LAMBDA])]/preceding-sibling::expr[count(*)=1]/SYMBOL", sep = "|") scope_completion_functs_xpath <- paste( - "*/LEFT_ASSIGN[following-sibling::expr/*[self::FUNCTION or self::OP-LAMBDA]]/preceding-sibling::expr[count(*)=1]/SYMBOL", - "*/RIGHT_ASSIGN[preceding-sibling::expr/*[self::FUNCTION or self::OP-LAMBDA]]/following-sibling::expr[count(*)=1]/SYMBOL", - "*/EQ_ASSIGN[following-sibling::expr/*[self::FUNCTION or self::OP-LAMBDA]]/preceding-sibling::expr[count(*)=1]/SYMBOL", + "(*|descendant-or-self::exprlist/*)/LEFT_ASSIGN[following-sibling::expr/*[self::FUNCTION or self::OP-LAMBDA]]/preceding-sibling::expr[count(*)=1]/SYMBOL", + "(*|descendant-or-self::exprlist/*)/RIGHT_ASSIGN[preceding-sibling::expr/*[self::FUNCTION or self::OP-LAMBDA]]/following-sibling::expr[count(*)=1]/SYMBOL", + "(*|descendant-or-self::exprlist/*)/EQ_ASSIGN[following-sibling::expr/*[self::FUNCTION or self::OP-LAMBDA]]/preceding-sibling::expr[count(*)=1]/SYMBOL", sep = "|") scope_completion <- function(uri, workspace, token, point, snippet_support = NULL) { diff --git a/R/definition.R b/R/definition.R index 108c63a8..61a4a282 100644 --- a/R/definition.R +++ b/R/definition.R @@ -1,8 +1,8 @@ definition_xpath <- paste( - "*[self::FUNCTION or self::OP-LAMBDA]/following-sibling::SYMBOL_FORMALS[text() = '{token_quote}' and @line1 <= {row}]", - "*[LEFT_ASSIGN/preceding-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}] and LEFT_ASSIGN/following-sibling::expr[@start > {start} or @end < {end}]]", - "*[RIGHT_ASSIGN/following-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}] and RIGHT_ASSIGN/preceding-sibling::expr[@start > {start} or @end < {end}]]", - "*[EQ_ASSIGN/preceding-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}] and EQ_ASSIGN/following-sibling::expr[@start > {start} or @end < {end}]]", + "(*|descendant-or-self::exprlist/*)[self::FUNCTION or self::OP-LAMBDA]/following-sibling::SYMBOL_FORMALS[text() = '{token_quote}' and @line1 <= {row}]", + "(*|descendant-or-self::exprlist/*)[LEFT_ASSIGN[preceding-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}] and following-sibling::expr[@start > {start} or @end < {end}]]]", + "(*|descendant-or-self::exprlist/*)[RIGHT_ASSIGN[following-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}] and preceding-sibling::expr[@start > {start} or @end < {end}]]]", + "(*|descendant-or-self::exprlist/*)[EQ_ASSIGN[preceding-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}] and following-sibling::expr[@start > {start} or @end < {end}]]]", "forcond/SYMBOL[text() = '{token_quote}' and @line1 <= {row}]", sep = "|") diff --git a/R/document.R b/R/document.R index 48a29580..45b1e084 100644 --- a/R/document.R +++ b/R/document.R @@ -402,6 +402,8 @@ parse_document <- function(uri, content) { if (is_rmarkdown(uri)) { content <- purl(content) } + # replace tab with a space since the width of a tab is 1 in LSP but 8 in getParseData(). + content <- gsub("\t", " ", content, fixed = TRUE) expr <- tryCatch(parse(text = content, keep.source = TRUE), error = function(e) NULL) if (!is.null(expr)) { parse_env <- function() { diff --git a/R/hover.R b/R/hover.R index a134fa84..91a70179 100644 --- a/R/hover.R +++ b/R/hover.R @@ -1,8 +1,8 @@ hover_xpath <- paste( - "*[self::FUNCTION or self::OP-LAMBDA][following-sibling::SYMBOL_FORMALS[text() = '{token_quote}' and @line1 <= {row}]]/parent::expr", - "*[LEFT_ASSIGN/preceding-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}] and LEFT_ASSIGN/following-sibling::expr[@start > {start} or @end < {end}]]", - "*[RIGHT_ASSIGN/following-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}] and RIGHT_ASSIGN/preceding-sibling::expr[@start > {start} or @end < {end}]]", - "*[EQ_ASSIGN/preceding-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}] and EQ_ASSIGN/following-sibling::expr[@start > {start} or @end < {end}]]", + "(*|descendant-or-self::exprlist/*)[self::FUNCTION or self::OP-LAMBDA][following-sibling::SYMBOL_FORMALS[text() = '{token_quote}' and @line1 <= {row}]]/parent::expr", + "(*|descendant-or-self::exprlist/*)[LEFT_ASSIGN[preceding-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}] and following-sibling::expr[@start > {start} or @end < {end}]]]", + "(*|descendant-or-self::exprlist/*)[RIGHT_ASSIGN[following-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}] and preceding-sibling::expr[@start > {start} or @end < {end}]]]", + "(*|descendant-or-self::exprlist/*)[EQ_ASSIGN[preceding-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}] and following-sibling::expr[@start > {start} or @end < {end}]]]", "forcond/SYMBOL[text() = '{token_quote}' and @line1 <= {row}]", sep = "|") @@ -23,7 +23,7 @@ hover_reply <- function(id, uri, workspace, document, point) { resolved <- FALSE version <- workspace$get_parse_data(uri)$version - logger$info("hover:", list(uri = uri, version = version)) + logger$info("hover:", list(uri = uri, version = version, token = token_result)) xdoc <- workspace$get_parse_data(uri)$xml_doc @@ -206,6 +206,9 @@ hover_reply <- function(id, uri, workspace, document, point) { } else if (token_name == "COMMENT") { # comment resolved <- TRUE + } else if (startsWith(token_name, "OP-")) { + # operators + resolved <- TRUE } } } diff --git a/R/signature.R b/R/signature.R index cb9db0fd..50deb6b7 100644 --- a/R/signature.R +++ b/R/signature.R @@ -1,6 +1,6 @@ signature_xpath <- paste( - "*[LEFT_ASSIGN/preceding-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}]]/expr[FUNCTION | OP-LAMBDA]", - "*[EQ_ASSIGN/preceding-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}]]/expr[FUNCTION | OP-LAMBDA]", + "(*|descendant-or-self::exprlist/*)[LEFT_ASSIGN/preceding-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}]]/expr[FUNCTION|OP-LAMBDA]", + "(*|descendant-or-self::exprlist/*)[EQ_ASSIGN/preceding-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}]]/expr[FUNCTION|OP-LAMBDA]", sep = "|") #' the response to a textDocument/signatureHelp Request diff --git a/tests/testthat/test-completion.R b/tests/testthat/test-completion.R index b612976b..45f08358 100644 --- a/tests/testthat/test-completion.R +++ b/tests/testthat/test-completion.R @@ -524,6 +524,50 @@ test_that("Completion of symbols in scope works", { discard(~ .$kind == CompletionItemKind$Text), 1) }) +test_that("Completion of symbols in scope works with semi-colons", { + skip_on_cran() + client <- language_client() + + temp_file <- withr::local_tempfile(fileext = ".R") + writeLines( + c( + "xvar0 <- rnorm(10);", + "my_fun <- function(xvar1) {", + " xvar2 = 1;", + " 2 -> xvar3;", + " for (xvar4 in 1:10) {", + " xvar", + " }", + "}" + ), + temp_file + ) + + client %>% did_save(temp_file) + + result <- client %>% respond_completion( + temp_file, c(5, 12), + retry_when = function(result) length(result) == 0 || length(result$items) == 0 + ) + + expect_length(result$items %>% discard(~ .$kind == CompletionItemKind$Text), 5) + expect_length(result$items %>% + keep(~ .$label == "xvar0") %>% + discard(~ .$kind == CompletionItemKind$Text), 1) + expect_length(result$items %>% + keep(~ .$label == "xvar1") %>% + discard(~ .$kind == CompletionItemKind$Text), 1) + expect_length(result$items %>% + keep(~ .$label == "xvar2") %>% + discard(~ .$kind == CompletionItemKind$Text), 1) + expect_length(result$items %>% + keep(~ .$label == "xvar3") %>% + discard(~ .$kind == CompletionItemKind$Text), 1) + expect_length(result$items %>% + keep(~ .$label == "xvar4") %>% + discard(~ .$kind == CompletionItemKind$Text), 1) +}) + test_that("Completion inside a package works", { skip_on_cran() wd <- path_real(path_package("languageserver", "projects", "mypackage")) diff --git a/tests/testthat/test-definition.R b/tests/testthat/test-definition.R index 7c054a45..cefb492e 100644 --- a/tests/testthat/test-definition.R +++ b/tests/testthat/test-definition.R @@ -161,6 +161,56 @@ test_that("Go to Definition works in scope with different assignment operators", expect_equal(result$range$end, list(line = 4, character = 11)) }) +test_that("Go to Definition works in scope with semi-colons", { + skip_on_cran() + client <- language_client() + + single_file <- withr::local_tempfile(fileext = ".R") + writeLines(c( + "my_fn <- function(var1) {", + " var2 <- 1;", + " var3 = 2;", + " 3 -> var4;", + " for (var5 in 1:10) {", + " var1 + var2 + var3 + var4 + var5;", + " }", + "}", + "my_fn(1)" + ), single_file) + + client %>% did_save(single_file) + + # first query a known function to make sure the file is processed + result <- client %>% respond_definition(single_file, c(8, 0)) + + expect_equal(result$range$start, list(line = 0, character = 0)) + expect_equal(result$range$end, list(line = 7, character = 1)) + + result <- client %>% respond_definition(single_file, c(5, 5)) + + expect_equal(result$range$start, list(line = 0, character = 18)) + expect_equal(result$range$end, list(line = 0, character = 22)) + + result <- client %>% respond_definition(single_file, c(5, 12)) + + expect_equal(result$range$start, list(line = 1, character = 2)) + expect_equal(result$range$end, list(line = 1, character = 11)) + + result <- client %>% respond_definition(single_file, c(5, 20)) + + expect_equal(result$range$start, list(line = 2, character = 2)) + expect_equal(result$range$end, list(line = 2, character = 10)) + + result <- client %>% respond_definition(single_file, c(5, 26)) + + expect_equal(result$range$start, list(line = 3, character = 2)) + expect_equal(result$range$end, list(line = 3, character = 11)) + + result <- client %>% respond_definition(single_file, c(5, 34)) + + expect_equal(result$range$start, list(line = 4, character = 7)) + expect_equal(result$range$end, list(line = 4, character = 11)) +}) test_that("Go to Definition works on both sides of assignment", { skip_on_cran() diff --git a/tests/testthat/test-hover.R b/tests/testthat/test-hover.R index bcdfd9d3..fee268c1 100644 --- a/tests/testthat/test-hover.R +++ b/tests/testthat/test-hover.R @@ -92,6 +92,85 @@ test_that("Hover on variable works", { expect_equal(result$contents, "```r\nvar1 <- 0\n```") }) +test_that("Hover on variable with leading tabs works", { + skip_on_cran() + client <- language_client() + + temp_file <- withr::local_tempfile(fileext = ".R") + writeLines( + c( + "index1 <- 1:10", + "\tindex1 + 1", + "\t\tindex1 + 2" + ), + temp_file + ) + + client %>% did_save(temp_file) + + result <- client %>% respond_hover(temp_file, c(0, 1)) + expect_equal(result$range$start, list(line = 0, character = 0)) + expect_equal(result$range$end, list(line = 0, character = 6)) + expect_equal(result$contents, "```r\nindex1 <- 1:10\n```") + + result <- client %>% respond_hover(temp_file, c(1, 4)) + expect_equal(result$range$start, list(line = 1, character = 1)) + expect_equal(result$range$end, list(line = 1, character = 7)) + expect_equal(result$contents, "```r\nindex1 <- 1:10\n```") + + result <- client %>% respond_hover(temp_file, c(2, 7)) + expect_equal(result$range$start, list(line = 2, character = 2)) + expect_equal(result$range$end, list(line = 2, character = 8)) + expect_equal(result$contents, "```r\nindex1 <- 1:10\n```") +}) + +test_that("Hover on variable works with semi-colons", { + skip_on_cran() + client <- language_client() + + temp_file <- withr::local_tempfile(fileext = ".R") + writeLines( + c( + "var1 <- 1:10;", + "f(var1)", + "local({", + " var2 <- 2:10;", + " f(var1, var2)", + " var1 <- 0;", + " f(var1, var2)", + "})" + ), + temp_file + ) + + client %>% did_save(temp_file) + + result <- client %>% respond_hover(temp_file, c(0, 1)) + expect_equal(result$range$start, list(line = 0, character = 0)) + expect_equal(result$range$end, list(line = 0, character = 4)) + expect_equal(result$contents, "```r\nvar1 <- 1:10;\n```") + + result <- client %>% respond_hover(temp_file, c(1, 3)) + expect_equal(result$range$start, list(line = 1, character = 2)) + expect_equal(result$range$end, list(line = 1, character = 6)) + expect_equal(result$contents, "```r\nvar1 <- 1:10;\n```") + + result <- client %>% respond_hover(temp_file, c(4, 6)) + expect_equal(result$range$start, list(line = 4, character = 5)) + expect_equal(result$range$end, list(line = 4, character = 9)) + expect_equal(result$contents, "```r\nvar1 <- 1:10;\n```") + + result <- client %>% respond_hover(temp_file, c(4, 13)) + expect_equal(result$range$start, list(line = 4, character = 11)) + expect_equal(result$range$end, list(line = 4, character = 15)) + expect_equal(result$contents, "```r\nvar2 <- 2:10;\n```") + + result <- client %>% respond_hover(temp_file, c(6, 6)) + expect_equal(result$range$start, list(line = 6, character = 5)) + expect_equal(result$range$end, list(line = 6, character = 9)) + expect_equal(result$contents, "```r\nvar1 <- 0;\n```") +}) + test_that("Hover works in scope with different assignment operators", { skip_on_cran() client <- language_client() @@ -308,6 +387,33 @@ test_that("Hover works with local function", { )) }) +test_that("Hover on operator is ignored", { + skip_on_cran() + client <- language_client() + + temp_file <- withr::local_tempfile(fileext = ".R") + writeLines( + c( + "for (ll in 1:3) {", + " p = ll", + " I = array(0:0, dim=c(p,p))", + "}" + ), + temp_file + ) + + client %>% did_save(temp_file) + + result <- client %>% respond_hover(temp_file, c(2, 22), + retry_when = function(result) length(result) > 0) + expect_equal(result, NULL) + + result <- client %>% respond_hover(temp_file, c(2, 23)) + expect_equal(result$range$start, list(line = 2, character = 22)) + expect_equal(result$range$end, list(line = 2, character = 23)) + expect_equal(result$contents, "```r\np = ll\n```") +}) + test_that("Hover works across multiple files", { skip_on_cran() client <- language_client() diff --git a/tests/testthat/test-signature.R b/tests/testthat/test-signature.R index 672d1a00..21fb9e25 100644 --- a/tests/testthat/test-signature.R +++ b/tests/testthat/test-signature.R @@ -46,6 +46,25 @@ test_that("Signature of user function works", { expect_equal(result$signatures[[1]]$label, "foo(x, y = 3)") }) +test_that("Signature of user function works with semi-colon", { + skip_on_cran() + client <- language_client() + + defn_file <- withr::local_tempfile(fileext = ".R") + temp_file <- withr::local_tempfile(fileext = ".R") + + writeLines(c("foo <- function(x, y = 3) { x + y };"), defn_file) + writeLines(c("foo(3, "), temp_file) + + client %>% did_save(defn_file) %>% did_save(temp_file) + + result <- client %>% respond_signature( + temp_file, c(0, 7), + retry_when = function(result) length(result) == 0 || length(result$signatures) == 0) + expect_length(result$signatures, 1) + expect_equal(result$signatures[[1]]$label, "foo(x, y = 3)") +}) + test_that("Signature in Rmarkdown works", { skip_on_cran() client <- language_client()