From 324b3bc9a6f26d3f23b8746b7e3d35f4f30b2fbb Mon Sep 17 00:00:00 2001
From: Lionel Henry <lionel.hry@gmail.com>
Date: Wed, 11 Sep 2019 19:00:37 +0200
Subject: [PATCH] Support S3 vectors in `one_of()`

Part of #109
---
 NEWS.md                              | 3 ++-
 R/select-helpers.R                   | 8 +++++---
 tests/testthat/test-select-helpers.R | 9 +++++++--
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index 9c00bac0..7bc4b71b 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,7 +1,8 @@
 
 # tidyselect 0.2.5.9000
 
-* `vars_select()` now supports unquoting factors and other S3 vectors.
+* `one_of()` now supports factors and other S3 vectors. Similary,
+  `vars_select()` now supports unquoting S3 vectors.
 
 * `vars_rename()` no longer ignore the names of unquoted character
   vectors of length 1 (#79).
diff --git a/R/select-helpers.R b/R/select-helpers.R
index 26add82b..942e7cdd 100644
--- a/R/select-helpers.R
+++ b/R/select-helpers.R
@@ -113,11 +113,13 @@ num_range <- function(prefix, range, width = NULL, vars = peek_vars()) {
 one_of <- function(..., .vars = peek_vars()) {
   keep <- list(...)
 
-  bad_input <- detect_index(keep, negate(is.character))
+  bad_input <- detect_index(keep, ~ !vec_is_coercible(., chr()))
   if (bad_input) {
-    type <- friendly_type_of(keep)
-    abort(glue::glue("Input { bad_input } must be a character vector, not { type }"))
+    type <- friendly_type_of(keep[[bad_input]])
+    msg <- glue::glue("Input { bad_input } must be a vector of column names, not {type}.")
+    abort(msg, "tidyselect_error_incompatible_index_type")
   }
+
   keep <- vctrs::vec_c(!!!keep, .ptype = character())
 
   if (!all(keep %in% .vars)) {
diff --git a/tests/testthat/test-select-helpers.R b/tests/testthat/test-select-helpers.R
index 08ed790f..61ce2faf 100644
--- a/tests/testthat/test-select-helpers.R
+++ b/tests/testthat/test-select-helpers.R
@@ -123,8 +123,9 @@ test_that("order is determined from inputs (#53)", {
 test_that("one_of gives useful errors", {
   expect_error(
     one_of(1L, .vars = c("x", "y")),
-    "Input 1 must be a character vector, not a list",
-    fixed = TRUE
+    "Input 1 must be a vector of column names, not an integer vector",
+    fixed = TRUE,
+    class = "tidyselect_error_incompatible_index_type"
   )
 })
 
@@ -273,3 +274,7 @@ test_that("last_col() selects last argument with offset", {
   expect_error(last_col(3, vars), "`offset` must be smaller than the number of columns")
   expect_error(last_col(vars = chr()), "Can't select last column when input is empty")
 })
+
+test_that("one_of() supports S3 vectors", {
+  expect_identical(vars_select(letters, one_of(factor(c("a", "c")))), c(a = "a", c = "c"))
+})