Skip to content

Commit

Permalink
Deprecate .data in pull() too (#364)
Browse files Browse the repository at this point in the history
  • Loading branch information
lionel- authored Oct 25, 2024
1 parent 441235b commit ff40c6d
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 20 deletions.
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
21 changes: 1 addition & 20 deletions R/eval-walk.R
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down
34 changes: 34 additions & 0 deletions R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
}
}
3 changes: 3 additions & 0 deletions R/vars-pull.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand Down
9 changes: 9 additions & 0 deletions tests/testthat/_snaps/eval-walk.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions tests/testthat/test-eval-walk.R
Original file line number Diff line number Diff line change
Expand Up @@ -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", {
Expand Down

0 comments on commit ff40c6d

Please sign in to comment.