Fix checking symbol on rhs of assignment expression

R/definition.R

@@ -1,8 +1,8 @@
 definition_xpath <- paste(
     "FUNCTION/following-sibling::SYMBOL_FORMALS[text() = '{token_quote}' and @line1 <= {row}]",
-    "*[LEFT_ASSIGN/preceding-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}]]",
-    "*[RIGHT_ASSIGN/following-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}]]",
-    "*[EQ_ASSIGN/preceding-sibling::expr[count(*)=1]/SYMBOL[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}]]",
     "forcond/SYMBOL[text() = '{token_quote}' and @line1 <= {row}]",
     sep = "|")
 
@@ -11,7 +11,6 @@ definition_xpath <- paste(
 #' If the function is not found in a file but is found in a loaded package,
 #' writes the function definition to a temporary file and returns that
 #' as the location.
-
 #' @keywords internal
 definition_reply <- function(id, uri, workspace, document, point) {
 
@@ -27,15 +26,18 @@ definition_reply <- function(id, uri, workspace, document, point) {
         if (length(token)) {
             token_name <- xml_name(token)
             token_text <- xml_text(token)
+            token_start <- as.integer(xml_attr(token, "start"))
+            token_end <- as.integer(xml_attr(token, "end"))
             logger$info("definition: ", token_name, token_text)
             if (token_name %in% c("SYMBOL", "SYMBOL_FUNCTION_CALL")) {
                 # symbol
                 preceding_dollar <- xml_find_first(token, "preceding-sibling::OP-DOLLAR")
                 if (length(preceding_dollar) == 0) {
                     enclosing_scopes <- xdoc_find_enclosing_scopes(xdoc,
                         row, col, top = TRUE)
-                    token_quote <- xml_single_quote(token_text)
-                    xpath <- glue(definition_xpath, row = row, token_quote = token_quote)
+                    xpath <- glue(definition_xpath,
+                        row = row, start = token_start, end = token_end,
+                        token_quote = xml_single_quote(token_text))
                     all_defs <- xml_find_all(enclosing_scopes, xpath)
                     if (length(all_defs)) {
                         last_def <- all_defs[[length(all_defs)]]

R/hover.R

@@ -1,8 +1,8 @@
 hover_xpath <- paste(
     "FUNCTION[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}]]",
-    "*[RIGHT_ASSIGN/following-sibling::expr[count(*)=1]/SYMBOL[text() = '{token_quote}' and @line1 <= {row}]]",
-    "*[EQ_ASSIGN/preceding-sibling::expr[count(*)=1]/SYMBOL[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}]]",
     "forcond/SYMBOL[text() = '{token_quote}' and @line1 <= {row}]",
     sep = "|")
 
@@ -42,14 +42,17 @@ hover_reply <- function(id, uri, workspace, document, point) {
         if (length(token)) {
             token_name <- xml_name(token)
             token_text <- xml_text(token)
+            token_start <- as.integer(xml_attr(token, "start"))
+            token_end <- as.integer(xml_attr(token, "end"))
             logger$info(token_name, token_text)
             if (token_name %in% c("SYMBOL", "SYMBOL_FUNCTION_CALL")) {
                 # symbol
                 preceding_dollar <- xml_find_first(token, "preceding-sibling::OP-DOLLAR")
                 if (length(preceding_dollar) == 0 && (is.null(signs) || signs != WORKSPACE || is.null(sig))) {
                     enclosing_scopes <- xdoc_find_enclosing_scopes(xdoc,
                         row, col, top = TRUE)
-                    xpath <- glue(hover_xpath, row = row,
+                    xpath <- glue(hover_xpath,
+                        row = row, start = token_start, end = token_end,
                         token_quote = xml_single_quote(token_text))
                     all_defs <- xml_find_all(enclosing_scopes, xpath)
                     if (length(all_defs)) {

tests/testthat/test-definition.R

@@ -137,6 +137,69 @@ 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 on both sides of assignment", {
+    skip_on_cran()
+    client <- language_client()
+
+    withr::local_tempfile(c("single_file"), fileext = ".R")
+    writeLines(c(
+        "var1 <- 1",
+        "var1 <- var1 + 1",
+        "var2 = 2",
+        "var2 = var2 + 2",
+        "3 -> var3",
+        "var3 + 3 -> var3"
+    ), single_file)
+
+    client %>% did_save(single_file)
+
+    result <- client %>% respond_definition(single_file, c(0, 1))
+
+    expect_equal(result$range$start, list(line = 0, character = 0))
+    expect_equal(result$range$end, list(line = 0, character = 9))
+
+    result <- client %>% respond_definition(single_file, c(1, 1))
+
+    expect_equal(result$range$start, list(line = 1, character = 0))
+    expect_equal(result$range$end, list(line = 1, character = 16))
+
+    result <- client %>% respond_definition(single_file, c(1, 9))
+
+    expect_equal(result$range$start, list(line = 0, character = 0))
+    expect_equal(result$range$end, list(line = 0, character = 9))
+
+    result <- client %>% respond_definition(single_file, c(2, 1))
+
+    expect_equal(result$range$start, list(line = 2, character = 0))
+    expect_equal(result$range$end, list(line = 2, character = 8))
+
+    result <- client %>% respond_definition(single_file, c(3, 1))
+
+    expect_equal(result$range$start, list(line = 3, character = 0))
+    expect_equal(result$range$end, list(line = 3, character = 15))
+
+    result <- client %>% respond_definition(single_file, c(3, 8))
+
+    expect_equal(result$range$start, list(line = 2, character = 0))
+    expect_equal(result$range$end, list(line = 2, character = 8))
+
+    result <- client %>% respond_definition(single_file, c(4, 6))
+
+    expect_equal(result$range$start, list(line = 4, character = 0))
+    expect_equal(result$range$end, list(line = 4, character = 9))
+
+    result <- client %>% respond_definition(single_file, c(5, 1))
+
+    expect_equal(result$range$start, list(line = 4, character = 0))
+    expect_equal(result$range$end, list(line = 4, character = 9))
+
+    result <- client %>% respond_definition(single_file, c(5, 15))
+
+    expect_equal(result$range$start, list(line = 5, character = 0))
+    expect_equal(result$range$end, list(line = 5, character = 16))
+})
+
 test_that("Go to Definition works when package is specified", {
     skip_on_cran()
     # When there is a user-defined function with the same name

tests/testthat/test-hover.R

@@ -144,6 +144,68 @@ test_that("Hover works in scope with different assignment operators", {
     expect_equal(result$contents, "```r\nfor (var5 in 1:10) {\n```")
 })
 
+test_that("Hover works on both sides of assignment", {
+    skip_on_cran()
+    client <- language_client()
+
+    withr::local_tempfile(c("single_file"), fileext = ".R")
+    writeLines(c(
+        "var1 <- 1",
+        "var1 <- var1 + 1",
+        "var2 = 2",
+        "var2 = var2 + 2",
+        "3 -> var3",
+        "var3 + 3 -> var3"
+    ), single_file)
+
+    client %>% did_save(single_file)
+
+    result <- client %>% respond_hover(single_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\n```")
+
+    result <- client %>% respond_hover(single_file, c(1, 1))
+    expect_equal(result$range$start, list(line = 1, character = 0))
+    expect_equal(result$range$end, list(line = 1, character = 4))
+    expect_equal(result$contents, "```r\nvar1 <- var1 + 1\n```")
+
+    result <- client %>% respond_hover(single_file, c(1, 9))
+    expect_equal(result$range$start, list(line = 1, character = 8))
+    expect_equal(result$range$end, list(line = 1, character = 12))
+    expect_equal(result$contents, "```r\nvar1 <- 1\n```")
+
+    result <- client %>% respond_hover(single_file, c(2, 1))
+    expect_equal(result$range$start, list(line = 2, character = 0))
+    expect_equal(result$range$end, list(line = 2, character = 4))
+    expect_equal(result$contents, "```r\nvar2 = 2\n```")
+
+    result <- client %>% respond_hover(single_file, c(3, 1))
+    expect_equal(result$range$start, list(line = 3, character = 0))
+    expect_equal(result$range$end, list(line = 3, character = 4))
+    expect_equal(result$contents, "```r\nvar2 = var2 + 2\n```")
+
+    result <- client %>% respond_hover(single_file, c(3, 8))
+    expect_equal(result$range$start, list(line = 3, character = 7))
+    expect_equal(result$range$end, list(line = 3, character = 11))
+    expect_equal(result$contents, "```r\nvar2 = 2\n```")
+
+    result <- client %>% respond_hover(single_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\n3 -> var3\n```")
+
+    result <- client %>% respond_hover(single_file, c(5, 1))
+    expect_equal(result$range$start, list(line = 5, character = 0))
+    expect_equal(result$range$end, list(line = 5, character = 4))
+    expect_equal(result$contents, "```r\n3 -> var3\n```")
+
+    result <- client %>% respond_hover(single_file, c(5, 15))
+    expect_equal(result$range$start, list(line = 5, character = 12))
+    expect_equal(result$range$end, list(line = 5, character = 16))
+    expect_equal(result$contents, "```r\nvar3 + 3 -> var3\n```")
+})
+
 test_that("Hover on function argument works", {
     skip_on_cran()
     client <- language_client()