diff --git a/NEWS.md b/NEWS.md index b260bf5..9490748 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,9 @@ # tidyselect (development version) +* `vars_pull()` now also warns when using `.data` (#335). Please + use string-quotation programmatic usage, consistently with other + tidyselect contexts. + * `num_range()` now recycles its arguments using tidyverse rules (#355). In addition, it gains a `cross` argument that allows you to take the cartesian product of these arguments instead. diff --git a/R/eval-walk.R b/R/eval-walk.R index 5923a83..2e1e464 100644 --- a/R/eval-walk.R +++ b/R/eval-walk.R @@ -313,29 +313,10 @@ call_kind <- function(expr, context_mask, error_call) { } env <- context_mask$.__current__. - fn <- as_string(head) if (fn %in% c("$", "[[") && identical(expr[[2]], quote(.data))) { - validate_dot_data(expr, error_call) - - what <- I("Use of .data in tidyselect expressions") - if (fn == "$") { - var <- as_string(expr[[3]]) - str <- encodeString(var, quote = '"') - - lifecycle::deprecate_soft("1.2.0", what, - details = cli::format_inline("Please use {.code {str}} instead of `.data${var}`"), - user_env = env - ) - } else if (fn == "[[") { - # .data[[ is an injection operator so can't give specific advice - lifecycle::deprecate_soft("1.2.0", what, - details = cli::format_inline("Please use {.code all_of(var)} (or {.code any_of(var)}) instead of {.code .data[[var]]}"), - user_env = env - ) - } - + check_dot_data(expr, env = env, error_call = error_call) return(".data") } diff --git a/R/utils.R b/R/utils.R index c5b2895..59ec2f2 100644 --- a/R/utils.R +++ b/R/utils.R @@ -229,3 +229,37 @@ mask_error_call <- function(data_mask) { } paste_lines <- function(...) paste(c(...), collapse = "\n") + +check_dot_data <- function(expr, env, error_call) { + if (is_quosure(expr)) { + expr <- quo_get_expr(expr) + } + + if (!is_call(expr, c("$", "[[")) || !identical(expr[[2]], quote(.data))) { + return() + } + + fn <- as_string(expr[[1]]) + validate_dot_data(expr, error_call) + + what <- I("Use of .data in tidyselect expressions") + if (fn == "$") { + var <- as_string(expr[[3]]) + str <- encodeString(var, quote = '"') + + lifecycle::deprecate_soft( + "1.2.0", + what, + details = cli::format_inline("Please use {.code {str}} instead of `.data${var}`"), + user_env = env + ) + } else if (fn == "[[") { + # .data[[ is an injection operator so can't give specific advice + lifecycle::deprecate_soft( + "1.2.0", + what, + details = cli::format_inline("Please use {.code all_of(var)} (or {.code any_of(var)}) instead of {.code .data[[var]]}"), + user_env = env + ) + } +} diff --git a/R/vars-pull.R b/R/vars-pull.R index a3c7315..b607e5b 100644 --- a/R/vars-pull.R +++ b/R/vars-pull.R @@ -34,6 +34,7 @@ #' vars_pull(letters, !!var) vars_pull <- function(vars, var = -1, error_call = caller_env(), error_arg = caller_arg(var)) { expr <- enquo(var) + if (quo_is_missing(expr)) { # No easy way to determine what var is in parent because it's likely # to be embraced; so don't try and use error_arg here @@ -43,6 +44,8 @@ vars_pull <- function(vars, var = -1, error_call = caller_env(), error_arg = cal ) } + check_dot_data(expr, env = quo_get_env(expr), error_call = error_call) + local_vars(vars) n <- length(vars) diff --git a/tests/testthat/_snaps/eval-walk.md b/tests/testthat/_snaps/eval-walk.md index 09e6a40..7eecc7b 100644 --- a/tests/testthat/_snaps/eval-walk.md +++ b/tests/testthat/_snaps/eval-walk.md @@ -83,6 +83,15 @@ Use of .data in tidyselect expressions was deprecated in tidyselect 1.2.0. i Please use `all_of(var)` (or `any_of(var)`) instead of `.data[[var]]` +--- + + Code + x <- vars_pull("a", .data[[var]]) + Condition + Warning: + Use of .data in tidyselect expressions was deprecated in tidyselect 1.2.0. + i Please use `all_of(var)` (or `any_of(var)`) instead of `.data[[var]]` + # eval_walk() warns when using a predicate without where() Code diff --git a/tests/testthat/test-eval-walk.R b/tests/testthat/test-eval-walk.R index c2f6292..9a79242 100644 --- a/tests/testthat/test-eval-walk.R +++ b/tests/testthat/test-eval-walk.R @@ -242,6 +242,7 @@ test_that("use of .data is deprecated", { var <- "a" expect_snapshot(x <- select_loc(x, .data$a)) expect_snapshot(x <- select_loc(x, .data[[var]])) + expect_snapshot(x <- vars_pull("a", .data[[var]])) }) test_that(".data in env-expression has the lexical definition", {