From fd161441dd6162e8cd65ce648cdbc509ecf04372 Mon Sep 17 00:00:00 2001 From: eitsupi Date: Sun, 15 Nov 2026 11:47:58 +0000 Subject: [PATCH 1/2] feat(sess): resolve socket hover and completion without evaluating requests Switch the socket session's hover and completion handlers to the shared inspection parser and resolver, so the request text is no longer passed to eval(). Requests that resolve to a name, a constant, or a chain of $ and @ accesses give the same results as before; other requests give no result. Errors are reported as no result and interrupts still propagate. The Interactive interrupt test now makes inspection slow with an active binding, since hover no longer evaluates arbitrary expressions. --- sess/R/handlers.R | 28 ++--- sess/inst/tinytest/test-hover-completion.R | 114 +++++++++++++++++++++ src/test/node/interactiveRuntime.test.ts | 10 +- 3 files changed, 130 insertions(+), 22 deletions(-) create mode 100644 sess/inst/tinytest/test-hover-completion.R diff --git a/sess/R/handlers.R b/sess/R/handlers.R index 9c9a16891..3ce4f6183 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -684,27 +684,17 @@ handle_listview_view <- function(view_id, index, path = list()) { workspace_show_view(location$data, location$title, state$owner %||% state$title) } -handle_hover <- function(expr_str) { - tryCatch( - { - expr <- parse(text = expr_str, keep.source = FALSE)[[1]] - obj <- eval(expr, .GlobalEnv) - list(str = capture_str(obj)) - }, - error = function(e) NULL - ) -} +# Hover and completion for the socket session. Requests are resolved with the +# shared inspection parser and resolver (inspection.R) without evaluating the +# request text. Hover and completion are shown without an explicit user +# action, so any failure, including invalid requests and errors from methods +# such as `.DollarNames()`, is reported to the editor as no result rather than +# as an error with a console warning. Interrupts are not caught, so that the +# dispatcher can reply that the request was interrupted. +handle_hover <- function(expr_str) tryCatch(inspection_hover(expr_str), error = function(e) NULL) handle_complete <- function(expr_str, trigger = NULL) { - obj <- tryCatch( - { - expr <- parse(text = expr_str, keep.source = FALSE)[[1]] - eval(expr, .GlobalEnv) - }, - error = function(e) NULL - ) - - completion_items(obj, trigger) + tryCatch(inspection_completion(expr_str, trigger), error = function(e) NULL) } # Completion items for the members of `obj` after the `trigger` operator. diff --git a/sess/inst/tinytest/test-hover-completion.R b/sess/inst/tinytest/test-hover-completion.R new file mode 100644 index 000000000..210a5c5c2 --- /dev/null +++ b/sess/inst/tinytest/test-hover-completion.R @@ -0,0 +1,114 @@ +# The socket session's hover and completion handlers use the inspection +# resolver: requests they used to answer give the same results, and other +# requests give no result without being evaluated. +local({ + hover <- sess:::handle_hover + complete <- sess:::handle_complete + capture_str <- sess:::capture_str + completion_items <- sess:::completion_items + prefix <- "sess_hover_" + masked <- c("$.sess_hover_s3", ".DollarNames.sess_hover_s3") + saved <- mget(intersect(masked, ls(.GlobalEnv, all.names = TRUE)), envir = .GlobalEnv) + on.exit({ + rm(list = ls(.GlobalEnv, all.names = TRUE, pattern = paste0("^", prefix)), envir = .GlobalEnv) + rm(list = intersect(masked, ls(.GlobalEnv, all.names = TRUE)), envir = .GlobalEnv) + list2env(saved, envir = .GlobalEnv) + }) + global <- function(name, value) assign(paste0(prefix, name), value, envir = .GlobalEnv) + + global("df", data.frame(x = 1:3, y = letters[1:3])) + global("list", list(a = list(b = "leaf"), c = 1)) + global("env", list2env(list(one = 1, two = "2"))) + global("s3", structure(list(), class = "sess_hover_s3")) + assign("$.sess_hover_s3", function(x, name) paste("member", name), envir = .GlobalEnv) + assign(".DollarNames.sess_hover_s3", function(x, pattern = "") c("p", "q"), envir = .GlobalEnv) + methods::setClass( + "sess_hover_s4", + representation(items = "list", label = "character"), + where = .GlobalEnv + ) + on.exit(methods::removeClass("sess_hover_s4", where = .GlobalEnv), add = TRUE) + global("s4", methods::new("sess_hover_s4", items = list(a = 1), label = "s4")) + + # These requests previously went through eval(); the results are unchanged. + previous <- function(expr) eval(parse(text = expr, keep.source = FALSE)[[1L]], .GlobalEnv) + same <- function(expr, trigger = NULL) { + expect_identical(hover(expr), list(str = capture_str(previous(expr))), info = expr) + if (!is.null(trigger)) { + expect_identical( + complete(expr, trigger), + completion_items(previous(expr), trigger), + info = expr + ) + } + } + same("sess_hover_df", "$") + same("sess_hover_df$x") + same("sess_hover_list", "$") + same("sess_hover_list $ a", "$") + same("sess_hover_list$a$b") + same("sess_hover_env", "$") + same("sess_hover_env$two") + same("sess_hover_s3", "$") + same("sess_hover_s3$p") + same("sess_hover_s4", "@") + same("sess_hover_s4@items", "$") + same("pi") + for (constant in c("1", "1.5", "1L", "TRUE", "NULL", "NA", "0x10", "\"text\"")) { + same(constant) + } + if (requireNamespace("data.table", quietly = TRUE)) { + global("dt", data.table::data.table(id = 1:2, value = c("a", "b"))) + same("sess_hover_dt", "$") + same("sess_hover_dt$value") + } + if (requireNamespace("R6", quietly = TRUE)) { + Counter <- R6::R6Class( + "SessHoverCounter", + public = list(count = 1, add = function() invisible(self)), + active = list(double = function() self$count * 2) + ) + global("r6", Counter$new()) + same("sess_hover_r6", "$") + same("sess_hover_r6$double") + } + + # Missing names and members give no result, as before. + expect_null(hover("sess_hover_missing")) + expect_null(complete("sess_hover_missing", "$")) + expect_null(complete("sess_hover_df", NULL)) + + # Other requests give no result, and the request text is not evaluated. + rejected <- c( + "sess_hover_called <- TRUE", + "assign(\"sess_hover_called\", TRUE)", + "sess_hover_list; sess_hover_called <- TRUE", + "(sess_hover_called <- TRUE)$a", + "sess_hover_list$a(" + ) + for (expr in rejected) { + expect_null(hover(expr), info = expr) + expect_null(complete(expr, "$"), info = expr) + } + expect_false(exists("sess_hover_called", envir = .GlobalEnv)) + expect_null(complete("get(\"sess_hover_list\")", "$")) + + # Errors from the session's own code give no result; interrupts propagate. + makeActiveBinding("sess_hover_failing", function() stop("binding failed"), .GlobalEnv) + expect_null(hover("sess_hover_failing")) + assign(".DollarNames.sess_hover_s3", function(x, pattern = "") stop("names failed"), .GlobalEnv) + expect_null(complete("sess_hover_s3", "$")) + makeActiveBinding( + "sess_hover_interrupted", + function() signalCondition(structure(class = c("interrupt", "condition"), list())), + .GlobalEnv + ) + expect_identical( + tryCatch(hover("sess_hover_interrupted"), interrupt = function(e) "interrupted"), + "interrupted" + ) + expect_identical( + tryCatch(complete("sess_hover_interrupted", "$"), interrupt = function(e) "interrupted"), + "interrupted" + ) +}) diff --git a/src/test/node/interactiveRuntime.test.ts b/src/test/node/interactiveRuntime.test.ts index a7543c33d..20d1cfa95 100644 --- a/src/test/node/interactiveRuntime.test.ts +++ b/src/test/node/interactiveRuntime.test.ts @@ -411,12 +411,16 @@ sess::display("display-${cell}", "text/plain"); cat("stderr-${cell}\\n", file=st test('interrupts a slow inspection and continues serving R requests', async () => { for (const afterTimeout of [false, true]) { const marker = `inspection-started-${String(afterTimeout)}`; + // Hover does not evaluate its request, so make reading the binding slow. + const binding = `slow_inspection_${String(afterTimeout)}`; + const setup = await submit( + `makeActiveBinding("${binding}", function() { cat("${marker}"); Sys.sleep(30); 42 }, .GlobalEnv)`, + ); + assert.strictEqual((await finished(setup)).state, 'success'); const inspection = client .request('inspect', { method: 'hover', - params: { - expr: `local({ cat("${marker}"); Sys.sleep(30); 42 })`, - }, + params: { expr: binding }, }) .then( () => '', From 8dceb33bb325ce139360b6b3ea3fe5e7efbbfe1c Mon Sep 17 00:00:00 2001 From: eitsupi Date: Sun, 15 Nov 2026 12:03:18 +0000 Subject: [PATCH 2/2] test(sess): cover rejected control flow in socket hover and completion --- sess/inst/tinytest/test-hover-completion.R | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sess/inst/tinytest/test-hover-completion.R b/sess/inst/tinytest/test-hover-completion.R index 210a5c5c2..c824662a1 100644 --- a/sess/inst/tinytest/test-hover-completion.R +++ b/sess/inst/tinytest/test-hover-completion.R @@ -79,7 +79,11 @@ local({ expect_null(complete("sess_hover_df", NULL)) # Other requests give no result, and the request text is not evaluated. + # Control flow is rejected: hovering `x` in `repeat x <- x + 1` sends + # `repeat x`, which used to loop forever. `repeat break` fails instead of + # hanging if it is evaluated again. rejected <- c( + "repeat break", "sess_hover_called <- TRUE", "assign(\"sess_hover_called\", TRUE)", "sess_hover_list; sess_hover_called <- TRUE",