Support error_arg for allow_rename = FALSE as well (#359)

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

R/eval-relocate.R

@@ -68,17 +68,18 @@ 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()
 
   allow_predicates <- allow_predicates && tidyselect_data_has_predicates(data)
   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

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)

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"
+    )
   }
 }
 

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
   )
 }

tests/testthat/_snaps/eval-relocate.md

@@ -70,6 +70,12 @@
     Condition <tidyselect:::error_disallowed_rename>
       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 <tidyselect:::error_disallowed_rename>
+      Error in `relocate_loc()`:
+      ! Can't rename variables in this context.
+      i `...` can't be renamed.
 
 # can forbid empty selections
 

tests/testthat/_snaps/eval-select.md

@@ -16,7 +16,7 @@
       select_loc(x, "a", exclude = 1)
     Condition <rlang_error>
       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 <tidyselect:::error_disallowed_rename>
       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 <tidyselect:::error_disallowed_rename>
+      Error in `select_loc()`:
+      ! Can't rename variables in this context.
+      i `x` can't be renamed.
 
 # can forbid empty selections
 

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"))

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"))