diff --git a/NEWS.md b/NEWS.md index 090f6865..098fe1bb 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,8 @@ list/environment factories, declarative R6 classes and package method registries. Package metadata is prepared in a background worker; completion never calls document expressions, constructors, methods or active getters. +- Preserve member completion for assigned and reassigned Polars query chains, + including DataFrame results after `collect()`, within the inference budget. - Use the same static member inference for signature help and hover, including chained methods and named argument documentation. Member requests wait for the current parse after edits, fixing stale completions on the first `$` trigger. diff --git a/R/member-completion.R b/R/member-completion.R index c939b47e..84fb40e5 100644 --- a/R/member-completion.R +++ b/R/member-completion.R @@ -260,6 +260,9 @@ member_context_index <- function(workspace, uri, document, at, parsed = NULL) { seen <- c(seen, pending) for (name in pending) { history <- document$parse_data$member_data$bindings[[name]] + # A dangling $ can make the following assignment parse as a + # member write. Later bindings must not hide the receiver's package. + history <- Filter(function(item) member_before(item$end, at), history) for (item in utils::tail(history, 1L)) { member_walk(item$expr, function(node) { if (is.symbol(node)) referenced <<- c(referenced, as.character(node)) diff --git a/R/member-extraction.R b/R/member-extraction.R index f7e5820e..07aec8a4 100644 --- a/R/member-extraction.R +++ b/R/member-extraction.R @@ -557,6 +557,53 @@ member_package_index <- function(input) { } index$roots <- index$package_roots[intersect(names(index$package_roots), index$exports)] for (name in names(index$roots)) index$namespace_roots[paste(input$package, name, sep = "::")] <- list(index$roots[[name]]) + member_prepare_returns(index) index$cache <- new.env(parent = emptyenv()) index } + +# Binding many defaults can dominate a request even when the result class does +# not depend on any argument. Prepare those guarantees in the package worker. +member_prepare_returns <- function(index) { + index$method_results <- list() + backed <- names(index$properties)[vapply(index$properties, function(fields) { + any(vapply(fields, function(value) { + length(value$type) == 1L && + any(index$members[[value$type]] %in% index$native_factories, na.rm = TRUE) + }, logical(1L))) + }, logical(1L))] + backed <- intersect(backed, names(index$classes)) + if (!length(backed)) return(invisible(NULL)) + prepare <- function(key, type = NULL) { + fn <- member_definition(index$definitions, key) + # This is a preparation-cost heuristic, not a return-type rule. Small + # functions keep using ordinary argument-sensitive request inference. + if (!member_head(fn, "function") || length(fn[[2L]]) < 10L) return() + env <- index$package_roots + for (name in names(fn[[2L]])) env[name] <- list(member_value()) + if (!is.null(type)) { + receiver <- member_lookup(index$method_receivers, paste(type, key, sep = "|")) + if (is.null(receiver)) receiver <- "self" + # Use only class guarantees, never a representative instance's fields. + env[receiver] <- list(member_value(type = type)) + } + budget <- new.env(parent = emptyenv()) + budget$remaining <- 10000L + budget$exhausted <- budget$transient <- FALSE + result <- member_infer(fn[[3L]], index, env, budget = budget, + context = list(key = key, formals = names(fn[[2L]]), actuals = list())) + if (!budget$exhausted && length(result$type) == 1L && result$type %in% backed) { + id <- if (is.null(type)) key else paste(type, key, sep = "|") + # Native-backed declarative classes expose their members in the + # index. Argument-dependent fields must not enter a universal summary. + index$method_results[id] <- list(member_value(type = result$type, classes = result$classes)) + } + } + for (key in names(index$definitions)) prepare(key) + for (type in intersect(names(index$members), names(index$classes))) { + if (any(index$members[[type]] %in% index$native_factories, na.rm = TRUE)) next + for (key in unique(unname(index$members[[type]]))) if (!is.na(key)) prepare(key, type) + } + index$return_definitions <- digest::digest(index$definitions, algo = "xxhash64") + invisible(NULL) +} diff --git a/R/member-inference.R b/R/member-inference.R index 1cea92a6..905061aa 100644 --- a/R/member-inference.R +++ b/R/member-inference.R @@ -60,9 +60,13 @@ member_strings <- function(x) { } member_lookup <- function(x, key) { - if (is.null(x) || is.null(key) || length(key) != 1L || is.na(key) || !key %in% names(x)) { + if (is.null(x) || !is.character(key) || length(key) != 1L || is.na(key) || !nzchar(key)) { return(NULL) } + # Lists and environments return NULL for absent names. Avoid scanning + # large namespace maps twice for each syntax node. + if (is.list(x) || is.environment(x)) return(x[[key]]) + if (!key %in% names(x)) return(NULL) x[[key]] } @@ -255,9 +259,18 @@ member_infer <- function( # start timing after R's first-call JIT compilation, before traversing ASTs. if (!is.null(budget$time_limit) && is.null(budget$deadline)) { budget$deadline <- proc.time()[[3L]] + budget$time_limit - } - if (depth > 64L || budget$remaining < 0L || - (!is.null(budget$deadline) && proc.time()[[3L]] > budget$deadline)) { + budget$next_time_check <- budget$remaining - 128L + } + # Reading the clock for every AST node can consume most of the request's + # time limit itself. Check periodically; node and depth bounds still apply + # on every visit, and an externally supplied deadline is checked first. + timed_out <- FALSE + if (!is.null(budget$deadline) && (is.null(budget$next_time_check) || + budget$remaining <= budget$next_time_check)) { + budget$next_time_check <- budget$remaining - 128L + timed_out <- proc.time()[[3L]] > budget$deadline + } + if (isTRUE(budget$exhausted) || depth > 64L || budget$remaining < 0L || timed_out) { budget$exhausted <- TRUE return(member_value(reason = "budget")) } @@ -273,8 +286,8 @@ member_infer <- function( ))) !name %in% names(bindings) && !shadowed && - ((!name %in% names(index$definitions) && name %in% member_base_intrinsics) || - (name %in% index$intrinsics && (is.null(index$document_bindings) || !is.null(context$key)))) + ((is.null(member_lookup(index$definitions, name)) && name %in% member_base_intrinsics) || + (name %in% index$intrinsics && (is.null(index$document_bindings) || !is.null(context$key)))) } package_env <- if (!is.null(index$package_roots)) index$package_roots else index$roots join_env <- function(a, b) { @@ -404,7 +417,7 @@ member_infer <- function( state <- flow(body, env) if (state$falls) member_join(state$returns, state$value) else state$returns } - bind_arguments <- function(fn, actuals, env) { + bind_arguments <- function(fn, actuals, env, defaults = TRUE) { formals <- as.list(fn[[2L]]) keys <- names(formals) dots <- match("...", keys) @@ -441,7 +454,9 @@ member_infer <- function( } if (!is.na(dots)) env["..."] <- list(member_value(type = "list", elements = actuals[!matched])) for (name in setdiff(keys, c(used, "..."))) { - env[name] <- list(if (identical(formals[[name]], quote(expr = ))) { + env[name] <- list(if (!defaults) { + unknown + } else if (identical(formals[[name]], quote(expr = ))) { member_value(type = ".missing") } else { infer(formals[[name]], env) @@ -641,8 +656,8 @@ member_infer <- function( if (identical(key, "self") && !is.null(receiver)) { return(member_value(type = receiver)) } - if (key %in% names(index$definitions) && (is.null(index$document_bindings) || - !is.null(context$key))) { + if (!is.null(member_lookup(index$definitions, key)) && (is.null(index$document_bindings) || + !is.null(context$key))) { return(member_value(function_key = key)) } # Imported roots must be supplied by the document/package lexical @@ -809,6 +824,26 @@ member_infer <- function( return(member_value(type = ".never")) } args <- as.list(expr)[-1L] + callee <- NULL + if (!intrinsic(head)) { + callee <- infer(expr[[1L]]) + if (!is.null(callee$function_key) && is.null(callee$metadata)) { + self <- callee$receiver_value + key <- if (is.null(self)) callee$function_key else paste(self$type, callee$function_key, sep = "|") + summary <- member_lookup(index$method_results, key) + if (is.null(summary)) summary <- member_lookup(index$method_results, callee$function_key) + if (!is.null(summary) && !any(vapply(args, identical, logical(1L), as.name("...")))) { + fn <- member_definition(index$definitions, callee$function_key) + if (member_head(fn, "function")) { + actuals <- lapply(args, function(arg) unknown) + if (is.null(bind_arguments(fn, actuals, list(), defaults = FALSE))) { + return(member_value(reason = "argument_matching")) + } + return(summary) + } + } + } + } if (head == "missing" && intrinsic(head) && length(args) == 1L && identical(args[[1L]], as.name("..."))) { dots <- member_lookup(bindings, "...") @@ -1066,7 +1101,7 @@ member_infer <- function( } return(unknown) } - callee <- infer(expr[[1L]]) + if (is.null(callee)) callee <- infer(expr[[1L]]) if (identical(callee$metadata, "S7") && callee$function_key %in% member_s7_intrinsics) { return(member_s7_call(expr, index, bindings, budget, callee$function_key)) } diff --git a/R/member-metadata.R b/R/member-metadata.R index c74722e7..7f879300 100644 --- a/R/member-metadata.R +++ b/R/member-metadata.R @@ -363,6 +363,10 @@ member_index_thaw <- function(snapshot) { return(NULL) } index <- list2env(snapshot, parent = emptyenv()) + if (!is.null(index$return_definitions) && + !identical(index$return_definitions, digest::digest(index$definitions, algo = "xxhash64"))) { + index$method_results <- list() + } index$cache <- new.env(parent = emptyenv()) index } diff --git a/tests/testthat/test-completion.R b/tests/testthat/test-completion.R index 0db2af21..88b48ab9 100644 --- a/tests/testthat/test-completion.R +++ b/tests/testthat/test-completion.R @@ -1784,3 +1784,59 @@ test_that("The first dollar trigger after an edit uses current Polars members", expect_false(any(c("fileext", "infer_schema_files", "row.names") %in% labels)) expect_true(all(vapply(result$items, function(item) identical(item$data$type, "member"), logical(1L)))) }) + +test_that("The first dollar trigger completes assigned and collected Polars queries", { + skip_on_cran() + skip_if_not_installed("polars") + client <- language_client() + temp_file <- withr::local_tempfile(fileext = ".R") + uri <- path_to_uri(temp_file) + lines <- c( + "library(polars)", "", + "csv_file <- tempfile(fileext = \".csv\")", + "write.csv(iris, csv_file, row.names = FALSE)", "", + "q <- pl$scan_csv(csv_file, infer_schema_files = 10)", "", + "q1 <- q$filter(pl$col(\"Sepal.Length\") > 5)", "q1", "", + "q1 <- q$filter(pl$col(\"Sepal.Length\") > 5)$group_by(\"Species\")$agg(pl$all()$median())$collect()", + "q1 # collected", "", + "q2 <- q1$group_by(\"Species\")$agg(pl$all()$sum())", "q2 # collected" + ) + did_open(client, temp_file, text = paste(lines, collapse = "\n")) + # Prepare package metadata using only q; q1 and q2 must complete on their + # first request, without warming their method summaries or retrying. + deadline <- Sys.time() + 15 + repeat { + ready <- respond_completion(client, temp_file, c(7L, 8L), retry = FALSE) + if (any(vapply(ready$items, function(item) identical(item$data$type, "member"), logical(1L)))) break + if (Sys.time() > deadline) break + Sys.sleep(0.1) + } + expect_true(any(vapply(ready$items, function(item) identical(item$data$type, "member"), logical(1L)))) + notify(client, "workspace/didChangeConfiguration", list(settings = list(parse_delay = 0.5))) + for (row in c(8L, 11L, 14L)) { + end <- list(line = row, character = 2L) + notify(client, "textDocument/didChange", list( + textDocument = list(uri = uri, version = row), + contentChanges = list(list(range = list(start = end, end = end), text = "$")) + )) + result <- respond(client, "textDocument/completion", list( + textDocument = list(uri = uri), + position = list(line = row, character = end$character + 1L), + context = list(triggerKind = 2L, triggerCharacter = "$") + ), retry = FALSE) + labels <- vapply(result$items, `[[`, character(1L), "label") + expect_true(all(c("filter", "group_by") %in% labels)) + expect_identical("collect" %in% labels, row == 8L) + expect_identical("lazy" %in% labels, row != 8L) + expect_true(all(vapply(result$items, function(item) identical(item$data$type, "member"), logical(1L)))) + expect_false(any(c("fileext", "infer_schema_files", "row.names") %in% labels)) + # Restore a complete statement before editing the next receiver. + notify(client, "textDocument/didChange", list( + textDocument = list(uri = uri, version = row + 1L), + contentChanges = list(list( + range = list(start = end, end = list(line = row, character = end$character + 1L)), + text = "" + )) + )) + } +}) diff --git a/tests/testthat/test-member-completion.R b/tests/testthat/test-member-completion.R index 4aa026d5..03cada92 100644 --- a/tests/testthat/test-member-completion.R +++ b/tests/testthat/test-member-completion.R @@ -39,6 +39,23 @@ test_that("Static inference stops at node, depth and time limits", { expect_true(budget$exhausted) }) +test_that("Static inference notices deadlines that expire during traversal", { + calls <- 0L + clock <- function() { + calls <<- calls + 1L + c(0, 0, if (calls == 1L) 0 else 2) + } + stub(member_infer, "proc.time", clock, depth = 2L) + budget <- new.env(parent = emptyenv()) + budget$remaining <- 20000L + budget$exhausted <- FALSE + budget$deadline <- 1 + expr <- as.call(c(list(as.name("list")), rep(list(1L), 256L))) + value <- member_infer(expr, member_generic_index(""), budget = budget) + expect_true(budget$exhausted) + expect_identical(tail(value$elements, 1L)[[1L]]$reason, "budget") +}) + test_that("Static members propagate through source factories and aliases", { expect_identical(member_labels("x <- list(alpha=1,beta=2)\ny <- x\ny$"), c("alpha", "beta")) expect_identical(member_labels(paste0( @@ -94,6 +111,20 @@ test_that("Lexical shadowing and source position prevent invented members", { expect_length(member_labels("x <- list(a=1)\nx$a <- opaque()\nx$"), 0L) }) +test_that("Package selection ignores bindings after an incomplete member access", { + index <- member_generic_index("factory <- function() opaque()") + index$package <- "fixture" + index$exports <- "factory" + index$roots$factory <- member_value(function_key = "factory") + index$constructor_types$factory <- "box" + index$members$box <- c(collect = NA_character_) + snapshot <- member_index_freeze(index) + expect_identical(member_labels( + "library(fixture)\nx <- factory()\nx$\ny <- NULL", + list(fixture = snapshot), point = list(row = 2L, col = 2L) + ), "collect") +}) + test_that("R6 fluent APIs expose public inheritance without initialization", { code <- paste0( "Parent <- R6::R6Class(\"Parent\", public=list(base=function() self, ", @@ -184,6 +215,73 @@ test_that("Installed Polars metadata resolves all original query positions witho expect_identical(unserialize(serialize(snapshot, NULL)), snapshot) }) +test_that("Polars query assignments retain LazyFrame members within the request budget", { + skip_if_not_installed("polars") + snapshot <- member_prepare_package("polars") + lines <- c( + "library(polars)", "", + "csv_file <- tempfile(fileext = \".csv\")", + "write.csv(iris, csv_file, row.names = FALSE)", "", + "q <- pl$scan_csv(csv_file, infer_schema_files = 10)", "", + "q1 <- q$filter(pl$col(\"Sepal.Length\") > 5)", "q1", "", + "q2 <- q1$group_by(\"Species\")$agg(pl$all()$sum())", "q2" + ) + for (row in c(8L, 11L)) { + edited <- lines + name <- edited[[row + 1L]] + edited[[row + 1L]] <- paste0(name, "$") + fixture <- member_fixture( + paste(edited, collapse = "\n"), list(polars = snapshot), + point = list(row = row, col = 3L) + ) + resolved <- member_resolve_cursor( + fixture$document$uri, fixture$workspace, fixture$document, fixture$point, + member_cursor(fixture$document, fixture$point) + ) + expect_identical(resolved$value$type, "polars_lazy_frame", info = name) + expect_false(resolved$budget$exhausted, info = name) + items <- member_completion( + fixture$document$uri, fixture$workspace, fixture$document, fixture$point, + TRUE, 200L + ) + labels <- vapply(items, `[[`, character(1L), "label") + expect_true(all(c("collect", "filter", "group_by") %in% labels), info = name) + expect_true(all(vapply(items, function(item) identical(item$data$type, "member"), logical(1L)))) + expect_false(any(c("fileext", "infer_schema_files", "row.names") %in% labels)) + } +}) + +test_that("Reassigned collected Polars queries retain their current DataFrame members", { + skip_if_not_installed("polars") + snapshot <- member_prepare_package("polars") + lines <- c( + "library(polars)", "", + "csv_file <- tempfile(fileext = \".csv\")", + "write.csv(iris, csv_file, row.names = FALSE)", "", + "q <- pl$scan_csv(csv_file, infer_schema_files = 10)", "", + "q1 <- q$filter(pl$col(\"Sepal.Length\") > 5)", "q1 # working here", "", + "q1 <- q$filter(pl$col(\"Sepal.Length\") > 5)$group_by(\"Species\")$agg(pl$all()$median())$collect()", + "q1 # collected", "", + "q2 <- q1$group_by(\"Species\")$agg(pl$all()$sum())", "q2 # collected" + ) + for (row in c(8L, 11L, 14L)) { + edited <- lines + edited[[row + 1L]] <- paste0(substr(edited[[row + 1L]], 1L, 2L), "$", substring(edited[[row + 1L]], 3L)) + fixture <- member_fixture(paste(edited, collapse = "\n"), list(polars = snapshot), + point = list(row = row, col = 3L)) + resolved <- member_resolve_cursor( + fixture$document$uri, fixture$workspace, fixture$document, fixture$point, + member_cursor(fixture$document, fixture$point) + ) + expect_identical(resolved$value$type, if (row == 8L) "polars_lazy_frame" else "polars_data_frame") + expect_false(resolved$budget$exhausted) + labels <- names(member_members(resolved$value, resolved$index)) + expect_true(all(c("filter", "group_by") %in% labels)) + expect_identical("collect" %in% labels, row == 8L) + expect_identical("lazy" %in% labels, row != 8L) + } +}) + test_that("LSP completion returns member identity and preserves calls", { fixture <- member_fixture("factory <- function() list(run=function(arg=1) list(done=1))\nfactory()$ru()") fixture$point <- list(row = 1L, col = 12L) @@ -351,6 +449,40 @@ test_that("Package extraction follows registries and factories with unrelated na expect_false(file.exists(marker)) }) +test_that("Prepared returns preserve argument-dependent results and metadata changes", { + marker <- tempfile() + on.exit(unlink(marker)) + code <- paste(c( + "api <- new.env(parent=emptyenv())", + "native_box <- function(pointer) {box <- new.env(); box$step <- make_step(pointer); class(box) <- \"raw_box\"; box}", + "make_step <- function(pointer) function() native_box(.Call(\"never\",pointer))", + "adapt <- function(x) UseMethod(\"adapt\")", + "adapt.raw_box <- function(x) {box <- new.env(); box$raw <- x; class(box) <- \"public_box\"; box}", + sprintf("fixed <- function(first={writeLines(\"ran\",%s)}, second=1, third=1, fourth=1, fifth=1, sixth=1, seventh=1, eighth=1, ninth=1, tenth=1) adapt(native_box(.Call(\"never\")))", deparse(marker)), + "varying <- function(first, second=1, third=1, fourth=1, fifth=1, sixth=1, seventh=1, eighth=1, ninth=1, tenth=1) if(first) adapt(native_box(NULL)) else list(other=1)", + "api$fixed <- fixed", "api$varying <- varying" + ), collapse = "\n") + scope <- new.env(parent = baseenv()) + eval(parse(text = code), scope) + index <- member_package_index(member_namespace_input(scope, + package = "returnfixture", exports = "api")) + snapshot <- member_index_freeze(index) + expect_identical(member_infer(quote(api$fixed()), index, index$roots)$type, "public_box") + expect_identical(member_infer(quote(api$fixed(stop("unused input"))), index, index$roots)$type, "public_box") + expect_identical(member_infer(quote(api$fixed(fir = 1)), index, index$roots)$type, "public_box") + expect_identical(member_infer(quote(api$fixed(first = 1, first = 2)), index, index$roots)$reason, "argument_matching") + expect_identical(member_infer(quote(api$fixed(unknown = 1)), index, index$roots)$reason, "argument_matching") + expect_identical(member_infer(quote(api$varying(FALSE)), index, index$roots)$type, "list") + expect_identical(member_infer(quote(api$varying(TRUE)), index, index$roots)$type, "public_box") + expect_identical(sort(member_infer(quote(api$varying(flag)), index, index$roots)$type), c("list", "public_box")) + # A snapshot edited before loading cannot retain a prepared result derived + # from an old method or one of its helpers. + snapshot$definitions$fixed[[3L]] <- quote(list(updated = TRUE)) + changed <- member_index_thaw(snapshot) + expect_identical(member_infer(quote(api$fixed()), changed, changed$roots)$type, "list") + expect_false(file.exists(marker)) +}) + test_that("Delegation extraction derives captures and result bodies", { code <- paste(c( "api <- new.env(parent=emptyenv()); commands <- new.env(parent=emptyenv())", diff --git a/tests/testthat/test-member-s7.R b/tests/testthat/test-member-s7.R index bc666469..6427f1da 100644 --- a/tests/testthat/test-member-s7.R +++ b/tests/testthat/test-member-s7.R @@ -159,8 +159,11 @@ test_that("Installed S7 metadata supplies current properties and constructor for test_that("S7 constructor and property providers work on first requests after edits", { skip_on_cran() skip_if_not_installed("S7") - client <- language_client() - path <- withr::local_tempfile(fileext = ".R") + # This regression needs only its open document. Scanning the test checkout + # queues unrelated package preparation and can delay S7 under parallel covr. + root <- withr::local_tempdir() + client <- language_client(working_dir = root) + path <- file.path(root, "s7-members.R") uri <- path_to_uri(path) code <- c("library(S7)", "Dog <- new_class(\"Dog\", properties=list(name=class_character, age=class_numeric))", "lola <- Dog(name=\"Lola\", age=11)") did_open(client, path, text = c(code, "lola"))