Add more error checking to rename_with() (#6574)

* Add more error checking to `rename_with()`

* NEWS bullet

* Use `length()` for simplicity and clarity

NEWS.md

@@ -1,5 +1,10 @@
 # dplyr (development version)
 
+* `rename_with()` now disallows renaming in the `.cols` tidy-selection (#6561).
+
+* `rename_with()` now checks that the result of `.fn` is the right type and size
+  (#6561).
+
 * `reframe()` is a new experimental verb that creates a new data frame by
   applying functions to columns of an existing data frame. It is very similar to
   `summarise()`, with two big differences:

R/rename.R

@@ -63,10 +63,25 @@ rename_with <- function(.data, .fn, .cols = everything(), ...) {
 #' @export
 rename_with.data.frame <- function(.data, .fn, .cols = everything(), ...) {
   .fn <- as_function(.fn)
-  cols <- tidyselect::eval_select(enquo(.cols), .data)
+  cols <- tidyselect::eval_select(enquo(.cols), .data, allow_rename = FALSE)
 
   names <- names(.data)
-  names[cols] <- .fn(names[cols], ...)
+
+  sel <- vec_slice(names, cols)
+  new <- .fn(sel, ...)
+
+  if (!is_character(new)) {
+    cli::cli_abort(
+      "{.arg .fn} must return a character vector, not {.obj_type_friendly {new}}."
+    )
+  }
+  if (length(new) != length(sel)) {
+    cli::cli_abort(
+      "{.arg .fn} must return a vector of length {length(sel)}, not {length(new)}."
+    )
+  }
+
+  names <- vec_assign(names, cols, new)
   names <- vec_as_names(names, repair = "check_unique")
 
   set_names(.data, names)

tests/testthat/_snaps/rename.md

@@ -0,0 +1,24 @@
+# `.fn` result type is checked (#6561)
+
+    Code
+      rename_with(df, fn)
+    Condition
+      Error in `rename_with()`:
+      ! `.fn` must return a character vector, not an integer.
+
+# `.fn` result size is checked (#6561)
+
+    Code
+      rename_with(df, fn)
+    Condition
+      Error in `rename_with()`:
+      ! `.fn` must return a vector of length 2, not 3.
+
+# can't rename in `.cols`
+
+    Code
+      rename_with(df, toupper, .cols = c(y = x))
+    Condition
+      Error in `rename_with()`:
+      ! Can't rename variables in this context.
+

tests/testthat/test-rename.R

@@ -55,5 +55,31 @@ test_that("passes ... along", {
 
 test_that("can't create duplicated names", {
   df <- tibble(x = 1, y = 2)
-  expect_error(df %>% rename_with(~ "X"), class = "vctrs_error_names")
+  expect_error(df %>% rename_with(~ rep_along(.x, "X")), class = "vctrs_error_names")
+})
+
+test_that("`.fn` result type is checked (#6561)", {
+  df <- tibble(x = 1)
+  fn <- function(x) 1L
+
+  expect_snapshot(error = TRUE, {
+    rename_with(df, fn)
+  })
+})
+
+test_that("`.fn` result size is checked (#6561)", {
+  df <- tibble(x = 1, y = 2)
+  fn <- function(x) c("a", "b", "c")
+
+  expect_snapshot(error = TRUE, {
+    rename_with(df, fn)
+  })
+})
+
+test_that("can't rename in `.cols`", {
+  df <- tibble(x = 1)
+
+  expect_snapshot(error = TRUE, {
+    rename_with(df, toupper, .cols = c(y = x))
+  })
 })