diff --git a/NEWS.md b/NEWS.md index 9490748..621b496 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,6 @@ # tidyselect (development version) + +* `eval_select()` and `eval_relocate()` gain a new `error_arg` argument that can be specified to throw a better error message when `allow_empty = FALSE` or `allow_rename = FALSE` (@olivroy, #327). * `vars_pull()` now also warns when using `.data` (#335). Please use string-quotation programmatic usage, consistently with other diff --git a/R/eval-relocate.R b/R/eval-relocate.R index 2d1360d..91b199a 100644 --- a/R/eval-relocate.R +++ b/R/eval-relocate.R @@ -68,10 +68,10 @@ eval_relocate <- function(expr, allow_rename = TRUE, allow_empty = TRUE, allow_predicates = TRUE, - error_arg = NULL, before_arg = "before", after_arg = "after", env = caller_env(), + error_arg = NULL, error_call = caller_env()) { check_dots_empty() @@ -79,6 +79,7 @@ eval_relocate <- function(expr, data <- tidyselect_data_proxy(data) expr <- as_quosure(expr, env = env) + sel <- eval_select_impl( x = data, names = names(data), @@ -89,7 +90,7 @@ eval_relocate <- function(expr, allow_empty = allow_empty, allow_predicates = allow_predicates, type = "relocate", - error_arg = error_arg, + error_arg = error_arg, error_call = error_call ) @@ -123,8 +124,7 @@ eval_relocate <- function(expr, env = env, error_call = error_call, allow_predicates = allow_predicates, - allow_rename = FALSE, - error_arg = before_arg + allow_rename = FALSE ), arg = before_arg, error_call = error_call @@ -145,8 +145,7 @@ eval_relocate <- function(expr, env = env, error_call = error_call, allow_predicates = allow_predicates, - allow_rename = FALSE, - error_arg = after_arg + allow_rename = FALSE ), arg = after_arg, error_call = error_call diff --git a/R/eval-select.R b/R/eval-select.R index f57a436..23dfeda 100644 --- a/R/eval-select.R +++ b/R/eval-select.R @@ -45,7 +45,6 @@ #' support predicates (as determined by [tidyselect_data_has_predicates()]). #' @param error_arg Argument names for `expr`. These #' are used in error messages. (You can use `"..."` if `expr = c(...)`). -#' For now, this is used when `allow_empty = FALSE`. #' @inheritParams rlang::args_dots_empty #' #' @return A named vector of numeric locations, one for each of the @@ -174,8 +173,8 @@ eval_select_impl <- function(x, allow_rename = TRUE, allow_empty = TRUE, allow_predicates = TRUE, - error_arg = NULL, type = "select", + error_arg = NULL, error_call = caller_env()) { if (!is_null(x)) { vctrs::vec_assert(x) @@ -229,7 +228,7 @@ eval_select_impl <- function(x, if (length(exclude) > 0) { if (!is.character(exclude)) { - cli::cli_abort("{.arg include} must be a character vector.", call = error_call) + cli::cli_abort("{.arg exclude} must be a character vector.", call = error_call) } to_exclude <- vctrs::vec_match(intersect(exclude, names), names) diff --git a/R/eval-walk.R b/R/eval-walk.R index 2e1e464..ee7e0be 100644 --- a/R/eval-walk.R +++ b/R/eval-walk.R @@ -110,11 +110,22 @@ ensure_named <- function(pos, check_empty(pos, allow_empty, error_arg, call = call) if (!allow_rename && any(names2(pos) != "")) { - cli::cli_abort( - "Can't rename variables in this context.", - class = "tidyselect:::error_disallowed_rename", - call = call - ) + if (is.null(error_arg)) { + cli::cli_abort( + "Can't rename variables in this context.", + class = "tidyselect:::error_disallowed_rename", + call = call + ) + } else { + cli::cli_abort( + c( + "Can't rename variables in this context.", + i = "{.arg {error_arg}} can't be renamed." + ), + class = "tidyselect:::error_disallowed_rename", + call = call + ) + } } nms <- names(pos) <- names2(pos) @@ -132,18 +143,15 @@ ensure_named <- function(pos, check_empty <- function(x, allow_empty = TRUE, error_arg = NULL, call = caller_env()) { if (!allow_empty && length(x) == 0) { if (is.null(error_arg)) { - cli::cli_abort( - "Must select at least one item.", - call = call, - class = "tidyselect_error_empty_selection" - ) + msg <- "Must select at least one item." } else { - cli::cli_abort( - "{.arg {error_arg}} must select at least one column.", - call = call, - class = "tidyselect_error_empty_selection" - ) + msg <- cli::format_inline("{.arg {error_arg}} must select at least one column.") } + cli::cli_abort( + "{msg}", + call = call, + class = "tidyselect_error_empty_selection" + ) } } diff --git a/R/utils.R b/R/utils.R index 59ec2f2..aa33865 100644 --- a/R/utils.R +++ b/R/utils.R @@ -53,9 +53,9 @@ relocate_loc <- function(x, name_spec = NULL, allow_rename = TRUE, allow_empty = TRUE, - error_arg = NULL, before_arg = "before", after_arg = "after", + error_arg = NULL, error_call = current_env()) { check_dots_empty() @@ -68,9 +68,9 @@ relocate_loc <- function(x, name_spec = name_spec, allow_rename = allow_rename, allow_empty = allow_empty, - error_arg = error_arg, before_arg = before_arg, after_arg = after_arg, + error_arg = error_arg, error_call = error_call ) } diff --git a/man/eval_relocate.Rd b/man/eval_relocate.Rd index 5c08dc0..0a8fd44 100644 --- a/man/eval_relocate.Rd +++ b/man/eval_relocate.Rd @@ -15,10 +15,10 @@ eval_relocate( allow_rename = TRUE, allow_empty = TRUE, allow_predicates = TRUE, - error_arg = NULL, before_arg = "before", after_arg = "after", env = caller_env(), + error_arg = NULL, error_call = caller_env() ) } @@ -60,16 +60,15 @@ use predicates (i.e. in \code{where()}). If \code{FALSE}, will error if \code{ex predicate. Will automatically be set to \code{FALSE} if \code{data} does not support predicates (as determined by \code{\link[=tidyselect_data_has_predicates]{tidyselect_data_has_predicates()}}).} -\item{error_arg}{Argument names for \code{expr}. These -are used in error messages. (You can use \code{"..."} if \code{expr = c(...)}). -For now, this is used when \code{allow_empty = FALSE}.} - \item{before_arg, after_arg}{Argument names for \code{before} and \code{after}. These are used in error messages.} \item{env}{The environment in which to evaluate \code{expr}. Discarded if \code{expr} is a \link[rlang:enquo]{quosure}.} +\item{error_arg}{Argument names for \code{expr}. These +are used in error messages. (You can use \code{"..."} if \code{expr = c(...)}).} + \item{error_call}{The execution environment of a currently running function, e.g. \code{caller_env()}. The function will be mentioned in error messages as the source of the error. See the diff --git a/man/eval_select.Rd b/man/eval_select.Rd index 19d85b0..b33a7ed 100644 --- a/man/eval_select.Rd +++ b/man/eval_select.Rd @@ -77,8 +77,7 @@ in an empty selection. If \code{FALSE}, will error if \code{expr} yields an empt selection.} \item{error_arg}{Argument names for \code{expr}. These -are used in error messages. (You can use \code{"..."} if \code{expr = c(...)}). -For now, this is used when \code{allow_empty = FALSE}.} +are used in error messages. (You can use \code{"..."} if \code{expr = c(...)}).} } \value{ A named vector of numeric locations, one for each of the diff --git a/tests/testthat/_snaps/eval-relocate.md b/tests/testthat/_snaps/eval-relocate.md index 27e4304..0defd0d 100644 --- a/tests/testthat/_snaps/eval-relocate.md +++ b/tests/testthat/_snaps/eval-relocate.md @@ -70,6 +70,12 @@ Condition Error in `relocate_loc()`: ! Can't rename variables in this context. + Code + relocate_loc(x, c(b, foo = b), allow_rename = FALSE, error_arg = "...") + Condition + Error in `relocate_loc()`: + ! Can't rename variables in this context. + i `...` can't be renamed. # can forbid empty selections diff --git a/tests/testthat/_snaps/eval-select.md b/tests/testthat/_snaps/eval-select.md index 745e4c1..c401f56 100644 --- a/tests/testthat/_snaps/eval-select.md +++ b/tests/testthat/_snaps/eval-select.md @@ -16,7 +16,7 @@ select_loc(x, "a", exclude = 1) Condition Error in `select_loc()`: - ! `include` must be a character vector. + ! `exclude` must be a character vector. # can forbid rename syntax (#178) @@ -40,6 +40,12 @@ Condition Error in `select_loc()`: ! Can't rename variables in this context. + Code + select_loc(mtcars, c(foo = mpg, cyl), error_arg = "x", allow_rename = FALSE) + Condition + Error in `select_loc()`: + ! Can't rename variables in this context. + i `x` can't be renamed. # can forbid empty selections diff --git a/tests/testthat/test-eval-relocate.R b/tests/testthat/test-eval-relocate.R index ddc52de..79a885e 100644 --- a/tests/testthat/test-eval-relocate.R +++ b/tests/testthat/test-eval-relocate.R @@ -168,6 +168,7 @@ test_that("can forbid rename syntax", { expect_snapshot(error = TRUE, cnd_class = TRUE, { relocate_loc(x, c(foo = b), allow_rename = FALSE) relocate_loc(x, c(b, foo = b), allow_rename = FALSE) + relocate_loc(x, c(b, foo = b), allow_rename = FALSE, error_arg = "...") }) expect_named(relocate_loc(x, c(c, b), allow_rename = FALSE), c("c", "b", "a")) diff --git a/tests/testthat/test-eval-select.R b/tests/testthat/test-eval-select.R index f48e3db..67943dd 100644 --- a/tests/testthat/test-eval-select.R +++ b/tests/testthat/test-eval-select.R @@ -92,6 +92,7 @@ test_that("can forbid rename syntax (#178)", { select_loc(mtcars, c(cyl, foo = cyl), allow_rename = FALSE) select_loc(mtcars, c(cyl, foo = mpg), allow_rename = FALSE) select_loc(mtcars, c(foo = mpg, cyl), allow_rename = FALSE) + select_loc(mtcars, c(foo = mpg, cyl), error_arg = "x", allow_rename = FALSE) }) expect_named(select_loc(mtcars, starts_with("c") | all_of("am"), allow_rename = FALSE), c("cyl", "carb", "am"))