Deprecate optional .cols and .fns cases (#6523)

* Remove defaults for `.cols` and `.fns`

* Internally always supply `.cols` and `.fns` where applicable

* Test that no arguments cases still work

* NEWS bullet

* Deprecate missing `.cols` everywhere

With some complex changes to ensure that warnings thrown outside of any group context still bubble up correctly

* Revise NEWS bullet

Author: DavisVaughan

NEWS.md

@@ -1,5 +1,19 @@
 # dplyr (development version)
 
+* `.cols` and `.fns` are now required arguments in `across()`, `c_across()`,
+  `if_any()`, and `if_all()`. In general, we now recommend that you use `pick()`
+  instead of empty calls to `across()` (i.e. with no arguments) or
+  `across(c(x, y))` (i.e. with no `.fns`) to select a subset of columns without
+  applying any transformation (#6523).
+  
+  * Relying on the previous default of `.cols = everything()` is deprecated.
+    We have skipped the soft-deprecation stage in this case, because indirect
+    usage of `across()` and friends in this way is rare.
+  
+  * Relying on the previous default of `.fns = NULL` is not yet formally
+    soft-deprecated, because there was no good alternative until now, but is
+    discouraged and will be soft-deprecated in the next minor release.
+
 * `cur_data()` and `cur_data_all()` are now soft-deprecated in favor of
   `pick()` (#6204).
 

R/across.R

@@ -12,6 +12,9 @@
 #' the predicate is `TRUE` for *any* of the selected columns, `if_all()`
 #' is `TRUE` when the predicate is `TRUE` for *all* selected columns.
 #'
+#' If you just need to select columns without applying a transformation to each
+#' of them, then you probably want to use [pick()] instead.
+#'
 #' `across()` supersedes the family of "scoped variants" like
 #' `summarise_at()`, `summarise_if()`, and `summarise_all()`.
 #'
@@ -27,9 +30,6 @@
 #'     `list(mean = mean, n_miss = ~ sum(is.na(.x))`. Each function is applied
 #'     to each column, and the output is named by combining the function name
 #'     and the column name using the glue specification in `.names`.
-#'   - `NULL`: the default, returns the selected columns in a data frame
-#'     without applying a transformation. This is useful for when you want to
-#'     use a function that takes a data frame.
 #'
 #'   Within these functions you can use [cur_column()] and [cur_group()]
 #'   to access the current column and grouping keys respectively.
@@ -168,14 +168,26 @@
 #'
 #' @export
 #' @seealso [c_across()] for a function that returns a vector
-across <- function(.cols = everything(),
-                   .fns = NULL,
+across <- function(.cols,
+                   .fns,
                    ...,
                    .names = NULL,
                    .unpack = FALSE) {
   mask <- peek_mask()
   caller_env <- caller_env()
 
+  .cols <- enquo(.cols)
+
+  if (quo_is_missing(.cols)) {
+    across_missing_cols_deprecate_warn()
+    .cols <- quo_set_expr(.cols, expr(everything()))
+  }
+  if (is_missing(.fns)) {
+    # Silent restoration to old defaults of `.fns` for now.
+    # TODO: Escalate this to formal deprecation.
+    .fns <- NULL
+  }
+
   if (!is_bool(.unpack) && !is_string(.unpack)) {
     stop_input_type(.unpack, "`TRUE`, `FALSE`, or a single string")
   }
@@ -188,7 +200,7 @@ across <- function(.cols = everything(),
   }
 
   setup <- across_setup(
-    {{ .cols }},
+    cols = !!.cols,
     fns = .fns,
     names = .names,
     .caller_env = caller_env,
@@ -221,6 +233,7 @@ across <- function(.cols = everything(),
   names <- setup$names
 
   if (is.null(fns)) {
+    # TODO: Deprecate and remove the `.fns = NULL` path in favor of `pick()`
     data <- mask$pick_current(vars)
 
     if (is.null(names)) {
@@ -281,13 +294,13 @@ across <- function(.cols = everything(),
 
 #' @rdname across
 #' @export
-if_any <- function(.cols = everything(), .fns = NULL, ..., .names = NULL) {
+if_any <- function(.cols, .fns, ..., .names = NULL) {
   context_local("across_if_fn", "if_any")
   if_across(`|`, across({{ .cols }}, .fns, ..., .names = .names))
 }
 #' @rdname across
 #' @export
-if_all <- function(.cols = everything(), .fns = NULL, ..., .names = NULL) {
+if_all <- function(.cols, .fns, ..., .names = NULL) {
   context_local("across_if_fn", "if_all")
   if_across(`&`, across({{ .cols }}, .fns, ..., .names = .names))
 }
@@ -330,13 +343,17 @@ if_across <- function(op, df) {
 #'   mutate(
 #'     sum = sum(c_across(w:z)),
 #'     sd = sd(c_across(w:z))
-#'  )
-c_across <- function(cols = everything()) {
+#'   )
+c_across <- function(cols) {
+  mask <- peek_mask()
   cols <- enquo(cols)
 
-  mask <- peek_mask()
-  vars <- c_across_setup(!!cols, mask = mask)
+  if (quo_is_missing(cols)) {
+    c_across_missing_cols_deprecate_warn()
+    cols <- quo_set_expr(cols, expr(everything()))
+  }
 
+  vars <- c_across_setup(!!cols, mask = mask)
 
   cols <- mask$current_cols(vars)
   vec_c(!!!cols, .name_spec = zap())
@@ -387,6 +404,7 @@ across_setup <- function(cols,
   vars <- names(data)[vars]
 
   if (is.null(fns)) {
+    # TODO: Eventually deprecate and remove the `.fns = NULL` path in favor of `pick()`
     if (!is.null(names)) {
       glue_mask <- across_glue_mask(.caller_env, .col = names_vars, .fn = "1")
       names <- vec_as_names(
@@ -411,7 +429,7 @@ across_setup <- function(cols,
   }
 
   if (!is.list(fns)) {
-    msg <- c("`.fns` must be NULL, a function, a formula, or a list of functions/formulas.")
+    msg <- c("`.fns` must be a function, a formula, or a list of functions/formulas.")
     abort(msg, call = call(across_if_fn))
   }
 
@@ -614,13 +632,22 @@ expand_across <- function(quo) {
   if (".cols" %in% names(expr)) {
     cols <- expr$.cols
   } else {
-    cols <- quote(everything())
+    across_missing_cols_deprecate_warn()
+    cols <- expr(everything())
   }
   cols <- as_quosure(cols, env)
 
+  if (".fns" %in% names(expr)) {
+    fns <- eval_tidy(expr$.fns, mask, env = env)
+  } else {
+    # In the missing case, silently restore the old default of `NULL`.
+    # TODO: Escalate this to formal deprecation.
+    fns <- NULL
+  }
+
   setup <- across_setup(
     !!cols,
-    fns = eval_tidy(expr$.fns, mask, env = env),
+    fns = fns,
     names = eval_tidy(expr$.names, mask, env = env),
     .caller_env = env,
     mask = dplyr_mask,
@@ -639,6 +666,7 @@ expand_across <- function(quo) {
 
   # No functions, so just return a list of symbols
   if (is.null(fns)) {
+    # TODO: Deprecate and remove the `.fns = NULL` path in favor of `pick()`
     expressions <- pmap(list(vars, names, seq_along(vars)), function(var, name, k) {
       quo <- new_quosure(sym(var), empty_env())
       quo <- new_dplyr_quosure(
@@ -738,6 +766,45 @@ is_inlinable_formula <- function(x, mask) {
   }
 }
 
+across_missing_cols_deprecate_warn <- function() {
+  across_if_fn <- context_peek_bare("across_if_fn") %||% "across"
+
+  # Passing the correct `user_env` through `expand_across()` to here is
+  # complicated, so instead we force the global environment. This means users
+  # won't ever see the "deprecated feature was likely used in the {pkg}"
+  # message, but the warning will still fire and that is more important.
+  user_env <- global_env()
+
+  cnd <- catch_cnd(classes = "lifecycle_warning_deprecated", {
+    lifecycle::deprecate_warn(
+      when = "1.1.0",
+      what = I(glue("Using `{across_if_fn}()` without supplying `.cols`")),
+      details = "Please supply `.cols` instead.",
+      user_env = user_env
+    )
+  })
+
+  if (is_null(cnd)) {
+    # Condition wasn't signaled
+    return(NULL)
+  }
+
+  # Subclassed so we can skip computing group context info when the warning
+  # is thrown from `expand_across()` outside of any group
+  class(cnd) <- c("dplyr:::warning_across_missing_cols_deprecated", class(cnd))
+
+  cnd_signal(cnd)
+}
+
+c_across_missing_cols_deprecate_warn <- function(user_env = caller_env(2)) {
+  lifecycle::deprecate_warn(
+    when = "1.1.0",
+    what = I("Using `c_across()` without supplying `cols`"),
+    details = "Please supply `cols` instead.",
+    user_env = user_env
+  )
+}
+
 df_unpack <- function(x, spec, caller_env, error_call = caller_env()) {
   size <- vec_size(x)
 

R/conditions.R

@@ -66,7 +66,8 @@ needs_group_context <- function(cnd) {
     "dplyr:::mutate_mixed_null",
     "dplyr:::mutate_constant_recycle_error",
     "dplyr:::summarise_mixed_null",
-    "dplyr:::error_pick_tidyselect"
+    "dplyr:::error_pick_tidyselect",
+    "dplyr:::warning_across_missing_cols_deprecated"
   ))
 }
 
@@ -368,11 +369,15 @@ signal_warnings <- function(state, error_call) {
 }
 
 new_dplyr_warning <- function(data) {
-  label <- cur_group_label(
-    data$type,
-    data$group_data$id,
-    data$group_data$group
-  )
+  if (data$needs_group_context) {
+    label <- cur_group_label(
+      data$type,
+      data$group_data$id,
+      data$group_data$group
+    )
+  } else {
+    label <- ""
+  }
 
   msg <- c(
     "i" = glue::glue("In argument `{data$name} = {data$expr}`."),

R/context.R

@@ -109,12 +109,21 @@ stop_mask_type <- function(type) {
 }
 
 cnd_data <- function(cnd, ctxt, mask_type, call) {
+  needs_group_context <- needs_group_context(cnd)
+
+  if (needs_group_context) {
+    group_data <- cur_group_data(mask_type)
+  } else {
+    group_data <- NULL
+  }
+
   list(
     cnd = cnd,
     name = ctxt$error_name,
     expr = ctxt$error_expression,
     type = mask_type,
-    group_data = cur_group_data(mask_type),
+    needs_group_context = needs_group_context,
+    group_data = group_data,
     call = call
   )
 }