diff --git a/arity.toml b/arity.toml index 2eb2d1a92..0ac5b72e4 100644 --- a/arity.toml +++ b/arity.toml @@ -18,6 +18,8 @@ ignore = [ "coalesce", # Calls to unexported functions with `:::` are deliberate here. "internal-function", + # Development versions such as 3.4.9000.9000 follow the tidyverse convention. + "description-malformed-version", # Existing findings that are not addressed yet. "roxygen-param", "roxygen-return", diff --git a/sess/DESCRIPTION b/sess/DESCRIPTION index 5b933319e..0b3678cab 100644 --- a/sess/DESCRIPTION +++ b/sess/DESCRIPTION @@ -1,7 +1,7 @@ Package: sess Type: Package Title: High-Performance IPC Bridge for R Sessions -Version: 3.4.0.9006 +Version: 3.4.9000.9000 Authors@R: c( person(given = "Randy", family = "Lai", @@ -37,7 +37,9 @@ Suggests: systemfonts, httpgd, jgd, + R6, rstudioapi, + S7, svglite, tinytest NeedsCompilation: no diff --git a/sess/R/handlers.R b/sess/R/handlers.R index 2ae69706c..9c9a16891 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -704,6 +704,11 @@ handle_complete <- function(expr_str, trigger = NULL) { error = function(e) NULL ) + completion_items(obj, trigger) +} + +# Completion items for the members of `obj` after the `trigger` operator. +completion_items <- function(obj, trigger = NULL) { if (is.null(obj) || is.null(trigger)) { return(NULL) } @@ -727,6 +732,9 @@ handle_complete <- function(expr_str, trigger = NULL) { })) } + # TODO: S7 objects are not S4, so their properties are not offered after `@` + # yet. They can be recognised without S7 (class "S7_object"), but listing + # property names needs S7::prop_names() or S7 internals. if (trigger == "@" && isS4(obj)) { nms <- methods::slotNames(obj) return(lapply(nms, function(n) { diff --git a/sess/R/inspection.R b/sess/R/inspection.R new file mode 100644 index 000000000..57d1f7687 --- /dev/null +++ b/sess/R/inspection.R @@ -0,0 +1,184 @@ +# Hover and completion targets that editors may request without evaluating code. +# +# Requests carry the text in front of the cursor (for example `x`, `df$col`, or +# `obj@slot`). The text is parsed but never evaluated. It is accepted only when +# it is a single name, a single constant, or a chain of `$` and `@` accesses on +# a name, and is then resolved by looking the name up and applying fixed +# accessor operations. +# +# Why requests must not evaluate arbitrary expressions: +# - Hover and completion run without an explicit user action, so evaluating the +# request runs code just because the cursor moved or `$` was typed. Clients +# cannot be relied on to send only names: the editor's extraction allows +# spaces, so hovering `x` in `repeat x <- x + 1` sends `repeat x`, which +# parses and would loop forever. R's own completion (utils rcompgen) likewise +# does not evaluate a prefix that contains an explicit function call, and +# IPython limits completion to side-effect-free evaluation by default. +# - Validating requests here keeps the guarantee independent of the client. +# This also allows exposing these handlers to other clients later, for +# example as arf IPC handlers, which run outside arf's evaluation policy and +# approval. +# +# Constants are accepted because evaluating them runs no code, and the editor +# already requests hover for them. +# +# Not evaluating the request does not make inspection free of side effects: +# promises, active bindings, S3 `$` methods, and methods of `str()` or +# `.DollarNames()` run code that is already defined in the session. They are +# kept so that hover and completion keep working for lazy data, R6 objects and +# classes with custom accessors. + +inspection_invalid <- function(message) { + stop(structure( + class = c("sess_inspection_invalid", "error", "condition"), + list(message = message, call = NULL) + )) +} + +inspection_is_constant <- function(expr) { + is.null(expr) || (is.atomic(expr) && length(expr) == 1L && is.null(attributes(expr))) +} + +inspection_is_name <- function(expr) { + # `...` and `..1` are not bindings that can be looked up by name. + is.symbol(expr) && !grepl("^\\.\\.(\\.|[0-9]+)$", as.character(expr)) +} + +# Parse a request into list(root, path) without evaluating it. +# +# Parsing is done in R so that the accepted syntax, such as numeric literals +# and spacing around `$`, matches R exactly and is validated in one place. +inspection_target <- function(expr) { + if (!is.character(expr) || length(expr) != 1L || is.na(expr)) { + inspection_invalid("expr must be a single string") + } + exprs <- tryCatch( + parse(text = expr, keep.source = FALSE), + error = function(e) inspection_invalid("expr is not valid R syntax") + ) + # Earlier versions evaluated only the first of several expressions. + if (length(exprs) != 1L) { + inspection_invalid("expr must be a single expression") + } + node <- exprs[[1L]] + if (inspection_is_constant(node)) { + return(list(root = list(kind = "constant", value = node), path = list())) + } + path <- list() + while (is.call(node)) { + operator <- node[[1L]] + if ( + length(node) != 3L || !(identical(operator, quote(`$`)) || identical(operator, quote(`@`))) + ) { + inspection_invalid("expr must be a name or a chain of $ and @ accesses") + } + member <- node[[3L]] + if (is.symbol(member)) { + member <- as.character(member) + } else if (!is.character(member) || length(member) != 1L || is.na(member)) { + inspection_invalid("$ and @ must be followed by a name") + } + # A quoted member such as `x$""` must still be a valid symbol name. + if (!nzchar(member) || nchar(member, type = "bytes") > 10000L) { + inspection_invalid("$ and @ must be followed by a non-empty name") + } + path[[length(path) + 1L]] <- list(operator = as.character(operator), name = member) + node <- node[[2L]] + } + if (!inspection_is_name(node)) { + inspection_invalid("expr must start with a name") + } + # Accessors were collected from the outermost one. + list(root = list(kind = "name", name = as.character(node)), path = rev(path)) +} + +# Apply one accessor with the base operator, regardless of any `$` or `@` +# defined in the global environment. +# +# The operators do not evaluate their member argument, so the call is built +# with the member as a symbol. The operator is embedded as a function object so +# that masking definitions are not looked up, and the object is bound in a +# private environment so that it is never re-evaluated. The environment's +# parent is the global environment so that S3 methods defined there still +# dispatch, as they would for `x$name` typed at the console. `@` is used rather +# than methods::slot() so that its own dispatch (S4 slots, S7 properties) is +# kept. +inspection_access <- function(object, operator, name) { + if (operator == "$" && is.atomic(object) && !is.object(object)) { + # A plain vector has no members; `$` would signal an error. + return(list(found = FALSE)) + } + if (operator == "@") { + if (isS4(object) && !inspection_has_slot(object, name)) { + return(list(found = FALSE)) + } + if (!isS4(object) && !is.object(object)) { + return(list(found = FALSE)) + } + } + fun <- if (operator == "$") base::`$` else base::`@` + env <- new.env(parent = .GlobalEnv) + assign(".object", object, envir = env) + value <- eval(as.call(list(fun, as.name(".object"), as.name(name))), env) + list(found = TRUE, value = value) +} + +# Whether base `@` reads `name` from an S4 object rather than signalling that +# there is no such slot. methods::.hasSlot() covers stored slots and other +# attributes; `@` also reads the data part `.Data`, the S3 class `.S3Class`, and +# `names` of a list-based object, even when they are not stored as attributes. +inspection_has_slot <- function(object, name) { + methods::.hasSlot(object, name) || + name %in% c(".Data", ".S3Class") || + (identical(name, "names") && typeof(object) == "list") +} + +# Resolve a parsed target to list(found, value). Each step reads its binding +# once, so an active binding or promise on the path runs at most once. +# Errors and interrupts from the session's own code are not caught. +inspection_resolve <- function(target) { + root <- target$root + if (identical(root$kind, "constant")) { + return(list(found = TRUE, value = root$value)) + } + # exists() does not force promises or call active bindings. + if (!exists(root$name, envir = .GlobalEnv, inherits = TRUE)) { + return(list(found = FALSE)) + } + object <- get(root$name, envir = .GlobalEnv, inherits = TRUE) + for (step in target$path) { + result <- inspection_access(object, step$operator, step$name) + if (!isTRUE(result$found)) { + return(result) + } + object <- result$value + } + list(found = TRUE, value = object) +} + +# Hover text for a request, or NULL when nothing is found. Invalid requests +# signal a `sess_inspection_invalid` error. +inspection_hover <- function(expr) { + resolved <- inspection_resolve(inspection_target(expr)) + if (!isTRUE(resolved$found)) { + return(NULL) + } + list(str = capture_str(resolved$value)) +} + +# Completion items after `$` or `@`, or NULL when nothing is found. +inspection_completion <- function(expr, trigger = NULL) { + if (!is.null(trigger) && (!is.character(trigger) || length(trigger) != 1L || is.na(trigger))) { + inspection_invalid("trigger must be a single string") + } + target <- inspection_target(expr) + # Without a member operator there are no items, so do not read any binding. + if (is.null(trigger) || !trigger %in% c("$", "@")) { + return(NULL) + } + resolved <- inspection_resolve(target) + if (!isTRUE(resolved$found)) { + return(NULL) + } + completion_items(resolved$value, trigger) +} diff --git a/sess/inst/tinytest/test-inspection.R b/sess/inst/tinytest/test-inspection.R new file mode 100644 index 000000000..55123c377 --- /dev/null +++ b/sess/inst/tinytest/test-inspection.R @@ -0,0 +1,235 @@ +# Hover and completion requests are parsed and resolved without evaluating them. +local({ + hover <- sess:::inspection_hover + completion <- sess:::inspection_completion + capture_str <- sess:::capture_str + prefix <- "sess_inspection_" + # The test masks these names in the global environment; restore any that + # existed before so that running it in a live session is harmless. + masked <- c("$", "@", "T", "$.sess_inspection_s3", ".DollarNames.sess_inspection_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) + if ("sess_inspection_search" %in% search()) { + detach("sess_inspection_search", character.only = TRUE) + } + }) + global <- function(name, value) assign(paste0(prefix, name), value, envir = .GlobalEnv) + invalid <- function(expr, trigger = NULL) { + condition <- tryCatch( + if (is.null(trigger)) hover(expr) else completion(expr, trigger), + error = function(e) e + ) + inherits(condition, "sess_inspection_invalid") + } + + # Names: the global environment first, then the search path. + global("value", list(a = 1L)) + expect_identical(hover("sess_inspection_value"), list(str = capture_str(list(a = 1L)))) + expect_identical(hover("pi"), list(str = capture_str(pi))) + search_env <- attach(NULL, name = "sess_inspection_search") + assign("sess_inspection_shadowed", "search", envir = search_env) + expect_identical(hover("sess_inspection_shadowed")$str, capture_str("search")) + global("shadowed", "global") + expect_identical(hover("sess_inspection_shadowed")$str, capture_str("global")) + expect_null(hover("sess_inspection_missing")) + expect_null(completion("sess_inspection_missing", "$")) + global("null", NULL) + expect_identical(hover("sess_inspection_null"), list(str = capture_str(NULL))) + + # Promises and active bindings on the path are read once. + counter <- new.env() + counter$forced <- 0L + delayedAssign( + "sess_inspection_promise", + { + counter$forced <- counter$forced + 1L + list(a = list(b = 2L)) + }, + assign.env = .GlobalEnv + ) + expect_identical(hover("sess_inspection_promise$a$b")$str, capture_str(2L)) + expect_identical(counter$forced, 1L) + active <- 0L + makeActiveBinding( + "sess_inspection_active", + function() { + active <<- active + 1L + list(x = 1L, y = "a") + }, + .GlobalEnv + ) + expect_identical(hover("sess_inspection_active$x")$str, capture_str(1L)) + expect_identical(active, 1L) + expect_identical(vapply(completion("sess_inspection_active", "$"), `[[`, "", "name"), c("x", "y")) + expect_identical(active, 2L) + + # Accessors on base structures, mixed chains and quoted members. + global("nested", list(a = list(b = "leaf"))) + expect_identical(hover("sess_inspection_nested$a$b")$str, capture_str("leaf")) + expect_identical(hover("sess_inspection_nested $ a $ b")$str, capture_str("leaf")) + expect_identical(hover("sess_inspection_nested$\"a\"")$str, capture_str(list(b = "leaf"))) + expect_identical(hover("sess_inspection_nested$none")$str, capture_str(NULL)) + global("frame", data.frame(x = 1:3)) + expect_identical(hover("sess_inspection_frame$x")$str, capture_str(1:3)) + env <- new.env() + env$v <- "in env" + global("env", env) + expect_identical(hover("sess_inspection_env$v")$str, capture_str("in env")) + global("vector", 1:3) + expect_null(hover("sess_inspection_vector$a")) + + # S3 `$` methods defined in the global environment dispatch as at the console. + global("s3", structure(list(), class = "sess_inspection_s3")) + assign("$.sess_inspection_s3", function(x, name) paste("member", name), envir = .GlobalEnv) + expect_identical(hover("sess_inspection_s3$abc")$str, capture_str("member abc")) + + # S4 slots, also mixed with `$`. + methods::setClass("sess_inspection_s4", representation(items = "list"), where = .GlobalEnv) + global("s4", methods::new("sess_inspection_s4", items = list(a = "slot item"))) + expect_identical(hover("sess_inspection_s4@items$a")$str, capture_str("slot item")) + expect_null(hover("sess_inspection_s4@missing")) + # Pseudo-slots that base `@` reads but methods::.hasSlot() does not report. + expect_identical(hover("sess_inspection_s4@.S3Class")$str, capture_str("sess_inspection_s4")) + methods::setClass("sess_inspection_s4_list", contains = "list", where = .GlobalEnv) + global("s4_list", methods::new("sess_inspection_s4_list", list(1L))) + expect_identical(hover("sess_inspection_s4_list@.Data")$str, capture_str(list(1L))) + expect_identical(hover("sess_inspection_s4_list@names")$str, capture_str(NULL)) + # Errors other than a missing slot are not hidden. + methods::removeClass("sess_inspection_s4_list", where = .GlobalEnv) + expect_error(hover("sess_inspection_s4_list@.Data")) + expect_null(hover("sess_inspection_nested@a")) + expect_identical(vapply(completion("sess_inspection_s4", "@"), `[[`, "", "name"), "items") + # Masking `$` or `@` in the global environment does not change the operators. + assign("$", function(x, name) stop("masked $ was called"), envir = .GlobalEnv) + assign("@", function(object, name) stop("masked @ was called"), envir = .GlobalEnv) + # The test itself avoids `$` while it is masked. + expect_identical(hover("sess_inspection_s4@items$a")[["str"]], capture_str("slot item")) + rm(list = c("$", "@"), envir = .GlobalEnv) + methods::removeClass("sess_inspection_s4", where = .GlobalEnv) + + if (requireNamespace("R6", quietly = TRUE)) { + reads <- 0L + Counter <- R6::R6Class( + "sess_inspection_r6", + public = list(field = "public"), + active = list( + current = function() { + reads <<- reads + 1L + "active" + } + ) + ) + global("r6", Counter$new()) + expect_identical(hover("sess_inspection_r6$field")$str, capture_str("public")) + expect_identical(hover("sess_inspection_r6$current")$str, capture_str("active")) + expect_identical(reads, 1L) + } + + if (requireNamespace("S7", quietly = TRUE)) { + Point <- S7::new_class("sess_inspection_s7", properties = list(x = S7::class_double)) + global("s7", Point(x = 3)) + expect_identical(hover("sess_inspection_s7@x")$str, capture_str(3)) + } + + # Every constant the parser produces is accepted. + for (constant in c( + "TRUE", + "1.5", + "1L", + "1i", + "0x10", + "1e3", + "Inf", + "NaN", + "NULL", + "NA", + "NA_integer_", + "NA_character_", + "\"text\"" + )) { + value <- eval(parse(text = constant, keep.source = FALSE)[[1L]], baseenv()) + expect_identical(hover(constant), list(str = capture_str(value)), info = constant) + } + expect_false(identical(hover("NA"), hover("NULL"))) + # T and F are names, so they follow a rebinding. + assign("T", "rebound", envir = .GlobalEnv) + expect_identical(hover("T")$str, capture_str("rebound")) + rm(list = "T", envir = .GlobalEnv) + + # Anything else is rejected before any binding is read. + active <- 0L + for (expr in c( + "sess_inspection_active()", + "sess_inspection_active$x()", + "sess_inspection_active[[1]]", + "sess_inspection_active[\"x\"]", + "sess_inspection_active <- 1", + "-sess_inspection_active", + "base::sess_inspection_active", + "sess_inspection_active; sess_inspection_active", + "sess_inspection_active\nsess_inspection_active", + "local(sess_inspection_active)", + "(sess_inspection_active)", + "1$a", + "sess_inspection_active$1", + "...", + "..1", + "", + "sess_inspection_active$", + "sess_inspection_active$\"\"" + )) { + expect_true(invalid(expr), info = expr) + expect_true(invalid(expr, "$"), info = expr) + } + expect_identical(active, 0L) + # Completion without a member operator has no items and reads no binding. + expect_null(completion("sess_inspection_active", NULL)) + expect_null(completion("sess_inspection_active", "[")) + expect_identical(active, 0L) + expect_true(invalid(1)) + expect_true(invalid(c("a", "b"))) + expect_true(invalid(NA_character_)) + expect_true(invalid("sess_inspection_nested", 1)) + # A trailing comment is dropped by the parser and does not form a call. + expect_identical(hover("sess_inspection_nested$a$b # note")$str, capture_str("leaf")) + + # Completion results keep the existing shape. + expect_identical( + completion("sess_inspection_nested", "$"), + sess:::completion_items(list(a = list(b = "leaf")), "$") + ) + assign( + ".DollarNames.sess_inspection_s3", + function(x, pattern = "") c("one", "two"), + envir = .GlobalEnv + ) + expect_identical(vapply(completion("sess_inspection_s3", "$"), `[[`, "", "name"), c("one", "two")) + expect_null(completion("sess_inspection_vector", "$")) + global("empty", list()) + expect_null(completion("sess_inspection_empty", "$")) + expect_null(completion("sess_inspection_nested", NULL)) + + # Errors and interrupts from the session's own code are not turned into results. + makeActiveBinding("sess_inspection_failing", function() stop("binding failed"), .GlobalEnv) + failure <- tryCatch(hover("sess_inspection_failing"), error = function(e) e) + expect_true(inherits(failure, "error")) + expect_false(inherits(failure, "sess_inspection_invalid")) + assign( + ".DollarNames.sess_inspection_s3", + function(x, pattern = "") stop("names failed"), + envir = .GlobalEnv + ) + expect_error(completion("sess_inspection_s3", "$"), "names failed") + makeActiveBinding( + "sess_inspection_interrupted", + function() signalCondition(structure(class = c("interrupt", "condition"), list())), + .GlobalEnv + ) + expect_identical( + tryCatch(hover("sess_inspection_interrupted"), interrupt = function(e) "interrupted"), + "interrupted" + ) +})