Disallow duplicate renaming in vars_rename()

Closes r-lib#70

NEWS.md

@@ -55,8 +55,8 @@
 * `vars_rename()` no longer ignore the names of unquoted character
   vectors of length 1 (#79).
 
-* `vars_rename()` no longer creates duplicates when a variable is
-  renamed to an existing name (#70).
+* `vars_rename()` now fails when a variable is renamed to an existing
+  name (#70).
 
 * `vars_select()`, `vars_rename()` and `vars_pull()` now detect
   missing values uniformly (#72).

R/vars-rename.R

@@ -29,13 +29,12 @@ vars_rename <- function(.vars, ..., .strict = TRUE) {
     }
   }
 
-  # Remove existing variables
-  if (length(old_vars)) {
-    dups_idx <- match(names(old_vars), .vars, nomatch = 0L)
-    if (!vec_index_is_empty(dups_idx)) {
-      .vars <- .vars[-dups_idx]
-    }
-  }
+  rename_check(
+    to = new_vars,
+    vars = setdiff(.vars, old_vars),
+    orig = .vars,
+    incl = set_names(match(old_vars, .vars), names(old_vars))
+  )
 
   select <- set_names(.vars, .vars)
   renamed_idx <- match(old_vars, .vars)

tests/testthat/outputs/vars-rename-error-rename-to-existing.txt

@@ -0,0 +1,13 @@
+> # One column
+> vars_rename(c("a", "b", "c"), b = a)
+Error: Must use unique name when renaming columns.
+* Column `a` is being renamed to existing column `b`.
+> # Multiple columns
+> vars_rename(c("a", "b", "c", "d"), c = a, d = b)
+Error: Must use unique name when renaming columns.
+* Column `a` is being renamed to existing column `c`.
+* Column `b` is being renamed to existing column `d`.
+> # Overlapping rename with one duplicate column
+> vars_rename(c("a", "b", "c"), b = a, c = b)
+Error: Must use unique name when renaming columns.
+* Column `b` is being renamed to existing column `c`.

tests/testthat/outputs/vars-rename-error-rename-to-same.txt

@@ -0,0 +1,8 @@
+> # New column
+> vars_rename(c("a", "b", "c"), foo = a, foo = b)
+Error: Must use unique names when renaming columns.
+* Columns `a` and `b` are being renamed to `foo`.
+> # Existing column
+> vars_rename(c("a", "b", "c"), c = a, c = b)
+Error: Must use unique names when renaming columns.
+* Columns `a` and `b` are being renamed to `c`.

tests/testthat/test-vars-rename.R

@@ -84,6 +84,54 @@ test_that("vars_rename() supports named character vectors of length 1 (#77)", {
   expect_identical(vars_rename(letters[1:3], !!c(B = "b")), c(a = "a", B = "b", c = "c"))
 })
 
-test_that("vars_rename() does not return duplicates (#70)", {
-  expect_identical(vars_rename(c("a", "b", "c"), b = c), c(a = "a", b = "c"))
+test_that("vars_rename() allows consecutive renames", {
+  expect_identical(vars_rename(c("a", "b", "c"), foo = a, bar = a), c(bar = "a", b = "b", c = "c"))
+})
+
+test_that("vars_rename() disallows renaming to same column", {
+  expect_error(
+    vars_rename(c("a", "b", "c"), foo = a, foo = b),
+    class = "tidyselect_error_rename_to_same"
+  )
+  expect_error(
+    vars_rename(c("a", "b", "c"), c = a, c = b),
+    class = "tidyselect_error_rename_to_same"
+  )
+
+  verify_output(test_path("outputs", "vars-rename-error-rename-to-same.txt"), {
+    "New column"
+    vars_rename(c("a", "b", "c"), foo = a, foo = b)
+
+    "Existing column"
+    vars_rename(c("a", "b", "c"), c = a, c = b)
+  })
+})
+
+test_that("vars_rename() allows overlapping renames", {
+  expect_identical(vars_rename(c("a", "b", "c"), b = c, c = b), c(a = "a", c = "b", b = "c"))
+})
+
+test_that("vars_rename() disallows renaming to existing columns (#70)", {
+  expect_error(
+    vars_rename(c("a", "b", "c"), b = a),
+    class = "tidyselect_error_rename_to_existing"
+  )
+  expect_error(
+    vars_rename(c("a", "b", "c", "d"), c = a, d = b),
+    class = "tidyselect_error_rename_to_existing"
+  )
+  expect_error(
+    vars_rename(c("a", "b", "c"), b = a, c = b),
+    class = "tidyselect_error_rename_to_existing"
+  )
+  verify_output(test_path("outputs", "vars-rename-error-rename-to-existing.txt"), {
+    "One column"
+    vars_rename(c("a", "b", "c"), b = a)
+
+    "Multiple columns"
+    vars_rename(c("a", "b", "c", "d"), c = a, d = b)
+
+    "Overlapping rename with one duplicate column"
+    vars_rename(c("a", "b", "c"), b = a, c = b)
+  })
 })