Skip to content

Commit

Permalink
Do not allow for completions inside comments (#604)
Browse files Browse the repository at this point in the history
* Do not allow for completions inside comments

* Move autocompletion logic to new file/function for testing

Also move comment checking to top. It does not matter if it is within a quote or not, the result is still "no autocompletion". So move the logic to the top to avoid performing unnecessary actions.

* Add tests

* Add more tests using test local envs

* Add news entry

* Add failing test for end-of-line comment detection

* Add `detect_comments()` with additional tests

* Qualify utils::tail()

Co-authored-by: Garrick Aden-Buie <garrick@adenbuie.com>
  • Loading branch information
schloerke and gadenbuie authored Oct 26, 2021
1 parent 27f2854 commit f1d0ec9
Show file tree
Hide file tree
Showing 5 changed files with 209 additions and 61 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,4 @@ VignetteBuilder:
Encoding: UTF-8
LazyData: true
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.1.1
RoxygenNote: 7.1.2
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ learnr (development version)
* Exercise checking is now conducted in the same temporary directory where exercises are evaluated. ([#544](https://github.com/rstudio/learnr/pull/544/))
* User submissions for R code exercises are now checked for parsing errors prior to any other checks. If the submitted code is unparsable, a friendly error feedback message is returned and no further evaluation or checking is performed. ([#547](https://github.com/rstudio/learnr/pull/547))
* Parse errors from user code that fails to parse can now be inspected by the error checker, but errors in exercise setup chunks cannot. Instead, global setup and setup chunk errors are raised as internal errors with a user-facing warning. In general, internal errors are now handled more consistently. ([#596](https://github.com/rstudio/learnr/pull/596))
* Commented code within an exercise will no longer be auto completed. ([#604](https://github.com/rstudio/learnr/pull/604))

## Bug fixes

Expand Down
111 changes: 111 additions & 0 deletions R/auto-complete.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@

# Given a line buffer, return a list of possible auto completions.
# If there is a valid label, then attach the server env to allow for local overrides of functions
auto_complete_r <- function(line, label = NULL, server_env = NULL) {

# If the last line includes comments then we don't return any completions.
# It's okay to consider only the last line for comments: Comment detection
# takes into account quotes on the same line, but `quotes = FALSE` in the
# completion settings below ensures completions aren't returned if the last
# line is part of a multi-line quote.
last_line <- utils::tail(strsplit(line, "\n")[[1]], 1)
if (detect_comment(last_line)) {
# If a comment is found, return `list()` to signify no completions are found
# (Similar to the output of Map(list, list()))
return(list())
}

# set completion settings
options <- utils::rc.options()
utils::rc.options(package.suffix = "::",
funarg.suffix = " = ",
function.suffix = "(")
on.exit(do.call(utils::rc.options, as.list(options)), add = TRUE)

# If and when exercises gain access to files, then we should evaluate this
# code in the exercise dir with `quotes = TRUE` (and sanitize to keep
# filename lookup local to exercise dir)
settings <- utils::rc.settings()
utils::rc.settings(ops = TRUE, ns = TRUE, args = TRUE, func = FALSE,
ipck = TRUE, S3 = TRUE, data = TRUE, help = TRUE,
argdb = TRUE, fuzzy = FALSE, files = FALSE, quotes = FALSE)
on.exit(do.call(utils::rc.settings, as.list(settings)), add = TRUE)

# temporarily attach global setup to search path
# for R completion engine
do.call("attach", list(server_env, name = "tutorial:server_env"))
on.exit(detach("tutorial:server_env"), add = TRUE)

# temporarily attach env to search path
# for R completion engine
if (isTRUE(nzchar(label)) && is.environment(server_env[[label]])) {
do.call("attach", list(server_env[[label]], name = "tutorial:question_env"))
on.exit(detach("tutorial:question_env"), add = TRUE)
}

completions <- character()
try(silent = TRUE, {
utils <- asNamespace("utils")
utils$.assignLinebuffer(line)
utils$.assignEnd(nchar(line))
utils$.guessTokenFromLine()
utils$.completeToken()
completions <- as.character(utils$.retrieveCompletions())
})

# detect functions
splat <- strsplit(completions, ":{2,3}")
fn <- vapply(splat, function(el) {
n <- length(el)
envir <- if (n == 1) .GlobalEnv else asNamespace(el[[1]])
symbol <- if (n == 2) el[[2]] else el[[1]]
tryCatch(
is.function(get(symbol, envir = envir)),
error = function(e) FALSE
)
}, logical(1))

# remove a leading '::', ':::' from autocompletion results, as
# those won't be inserted as expected in Ace
completions <- gsub("[^:]+:{2,3}(.)", "\\1", completions)
completions <- completions[nzchar(completions)]

# zip together
result <- Map(list, completions, fn, USE.NAMES = FALSE)

# return completions
as.list(result)
}

detect_comment <- function(line = "") {
line <- strsplit(line, "")[[1]]
quote_str <- ""
in_quote <- FALSE
in_escape <- FALSE
for (char in line) {
if (identical(char, "\\")) {
in_escape <- TRUE
next
}
if (char %in% c("'", '"')) {
if (in_escape) {
in_escape <- FALSE
} else if (identical(quote_str, "")) {
in_quote <- TRUE
quote_str <- char
} else if (identical(char, quote_str)) {
in_quote <- FALSE
quote_str <- ""
} else {
# ignore a quote within a quote
}
next
}
in_escape <- FALSE
if (!identical(char, "#")) next
if (in_quote) next
return(TRUE)
}

FALSE
}
61 changes: 1 addition & 60 deletions R/http-handlers.R
Original file line number Diff line number Diff line change
Expand Up @@ -177,66 +177,7 @@ register_http_handlers <- function(session, metadata) {

Encoding(line) <- "UTF-8"

# set completion settings
options <- utils::rc.options()
utils::rc.options(package.suffix = "::",
funarg.suffix = " = ",
function.suffix = "(")
on.exit(do.call(utils::rc.options, as.list(options)), add = TRUE)

# If and when exercises gain access to files, then we should evaluate this
# code in the exercise dir with `quotes = TRUE` (and sanitize to keep
# filename lookup local to exercise dir)
settings <- utils::rc.settings()
utils::rc.settings(ops = TRUE, ns = TRUE, args = TRUE, func = FALSE,
ipck = TRUE, S3 = TRUE, data = TRUE, help = TRUE,
argdb = TRUE, fuzzy = FALSE, files = FALSE, quotes = FALSE)
on.exit(do.call(utils::rc.settings, as.list(settings)), add = TRUE)

# temporarily attach global setup to search path
# for R completion engine
do.call("attach", list(server_envir, name = "tutorial:setup"))
on.exit(detach("tutorial:setup"), add = TRUE)

# temporarily attach environment state to search path
# for R completion engine
if (nzchar(label) && is.environment(state[[label]])) {
do.call("attach", list(state[[label]], name = "tutorial:state"))
on.exit(detach("tutorial:state"), add = TRUE)
}

completions <- character()
try(silent = TRUE, {
utils <- asNamespace("utils")
utils$.assignLinebuffer(line)
utils$.assignEnd(nchar(line))
utils$.guessTokenFromLine()
utils$.completeToken()
completions <- as.character(utils$.retrieveCompletions())
})

# detect functions
splat <- strsplit(completions, ":{2,3}")
fn <- vapply(splat, function(el) {
n <- length(el)
envir <- if (n == 1) .GlobalEnv else asNamespace(el[[1]])
symbol <- if (n == 2) el[[2]] else el[[1]]
tryCatch(
is.function(get(symbol, envir = envir)),
error = function(e) FALSE
)
}, logical(1))

# remove a leading '::', ':::' from autocompletion results, as
# those won't be inserted as expected in Ace
completions <- gsub("[^:]+:{2,3}(.)", "\\1", completions)
completions <- completions[nzchar(completions)]

# zip together
result <- Map(list, completions, fn, USE.NAMES = FALSE)

# return completions
as.list(result)
auto_complete_r(line, label, state)
}))

# diagnostics handler
Expand Down
95 changes: 95 additions & 0 deletions tests/testthat/test-auto-complete.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
test_that("R auto complete finds runif vars", {

expect_equal(auto_complete_r("method not found"), list())
expect_equal(auto_complete_r("runif"), list(
list("runif", TRUE)
))
expect_equal(auto_complete_r("runif("), list(
list("n = ", FALSE),
list("min = ", FALSE),
list("max = ", FALSE)
))
})

test_that("R auto completions are not added when the line is a comment or quotes", {
runif_fn <- list(list("runif", TRUE))

# Establish expected autocomplete results
expect_equal(auto_complete_r("1 + 1\nrunif"), runif_fn)

# Completions should not be found when in a quote (even when started from a prior line)
expect_equal(auto_complete_r("1 + 1\n'runif"), list())
expect_equal(auto_complete_r("'1 + 1\nrunif"), list())
expect_equal(auto_complete_r("\" ' # # runif"), list())
expect_equal(auto_complete_r("\" ' # runif"), list())

# Comments on a prior line do not affect the auto completion
expect_equal(auto_complete_r("# 1 + 1\nrunif"), runif_fn)

# comments on a the last line do affect the auto completion
expect_equal(auto_complete_r("1 + 1 \n# runif"), list())
expect_equal(auto_complete_r("1 + 1 \nrunif #runif"), list())
expect_equal(auto_complete_r("1 + 1 \n \t # runif"), list())
})

test_that("Local env overrides global env", {
# Create a test env that contains another env nested within a label
test_env <- new.env()
test_env$test_runif <- runif
label_env <- new.env()
label_env$custom_runif <- function(a = 1, b = 2) {
a + b + c
}
label_env$runif <- function(a = 1, b = 2) {
a + b + c
}
test_env$my_label <- label_env

# Find functions defined within the test env
expect_equal(auto_complete_r("test_runif", NULL, NULL), list())
expect_equal(auto_complete_r("test_runif", NULL, test_env), list(
list("test_runif", TRUE)
))

# Find custom runif function in a label's env
expect_equal(auto_complete_r("custom_runif", NULL, NULL), list())
expect_equal(auto_complete_r("custom_runif", "my_label", test_env), list(
list("custom_runif", TRUE)
))
expect_equal(auto_complete_r("custom_runif", "other_label", test_env), list())

# # Auto complete currently (and previously) returned both the global and local runif parameters
# # TODO-future; Only return the results from the local env
# # Establish runif function is regularly found
# expect_equal(auto_complete_r("runif(", NULL, NULL), list(
# list("n = ", FALSE),
# list("min = ", FALSE),
# list("max = ", FALSE)
# ))
# # Find custom runif function in a label's env
# expect_equal(auto_complete_r("runif(", "my_label", test_env), list(
# list("a = ", FALSE),
# list("b = ", FALSE)
# ))
})

test_that("detect_comments()", {
expect_false(detect_comment(""))
expect_false(detect_comment("runif()"))
expect_true(detect_comment("#runif()"))
expect_true(detect_comment("runif() # random uniform"))
expect_true(detect_comment("#runif() # random uniform"))
expect_false(detect_comment("paste('# not a comment')"))
expect_false(detect_comment("paste('# \'still\' # not a comment')"))
expect_false(detect_comment("paste('# \"still\" # not a comment')"))
expect_true(detect_comment("paste('# \"still\" # not a comment') # is a comment"))

expect_false(detect_comment('" \' # "'))
expect_true(detect_comment('" \' # " # runif'))
expect_false(detect_comment('" \' # "'))
expect_true(detect_comment('" \' " # runif'))
expect_false(detect_comment('" \' # "'))
expect_true(detect_comment("' \" # ' # runif"))
expect_false(detect_comment('" \' # "'))
expect_true(detect_comment("' \" ' # runif"))
})

0 comments on commit f1d0ec9

Please sign in to comment.