Unify and improve on ... warning with group functions (#7753)

Author: DavisVaughan

R/group-nest.R

@@ -68,9 +68,18 @@ group_nest.data.frame <- function(.tbl, ..., .key = "data", keep = FALSE) {
 #' @export
 group_nest.grouped_df <- function(.tbl, ..., .key = "data", keep = FALSE) {
   if (dots_n(...)) {
-    warn(
-      "... is ignored in group_nest(<grouped_df>), please use group_by(..., .add = TRUE) |> group_nest()"
+    warn_ignores_dots(
+      "group_nest",
+      "grouped_df",
+      "group_by(..., .add = TRUE) |> group_nest()"
     )
   }
   group_nest_impl(.tbl, .key = .key, keep = keep)
 }
+
+# This is not a deprecation warning, just giving advice
+warn_ignores_dots <- function(fn, class, with) {
+  cli::cli_warn(
+    "Calling {.fn {fn}} on a {.cls {class}} ignores `...`. Please use {.code {with}}."
+  )
+}

R/group-split.R

@@ -74,8 +74,10 @@ group_split.rowwise_df <- function(
   keep = deprecated()
 ) {
   if (dots_n(...)) {
-    warn(
-      "... is ignored in group_split(<rowwise_df>), please use as_tibble() |> group_split(...)"
+    warn_ignores_dots(
+      "group_split",
+      "rowwise_df",
+      "as_tibble() |> group_split(...)"
     )
   }
   if (!missing(keep)) {
@@ -111,8 +113,10 @@ group_split.grouped_df <- function(
     .keep <- keep
   }
   if (dots_n(...)) {
-    warn(
-      "... is ignored in group_split(<grouped_df>), please use group_by(..., .add = TRUE) |> group_split()"
+    warn_ignores_dots(
+      "group_split",
+      "grouped_df",
+      "group_by(..., .add = TRUE) |> group_split()"
     )
   }
 

tests/testthat/_snaps/group-nest.md

@@ -0,0 +1,15 @@
+# group_nest.grouped_df() warns about `...`
+
+    Code
+      group_nest(group_by(mtcars, cyl), cyl)
+    Condition
+      Warning:
+      Calling `group_nest()` on a <grouped_df> ignores `...`. Please use `group_by(..., .add = TRUE) |> group_nest()`.
+    Output
+      # A tibble: 3 x 2
+          cyl                data
+        <dbl> <list<tibble[,10]>>
+      1     4           [11 x 10]
+      2     6            [7 x 10]
+      3     8           [14 x 10]
+

tests/testthat/_snaps/group-split.md

@@ -0,0 +1,16 @@
+# group_split.grouped_df() warns about `...`
+
+    Code
+      out <- group_split(group_by(mtcars, cyl), cyl)
+    Condition
+      Warning:
+      Calling `group_split()` on a <grouped_df> ignores `...`. Please use `group_by(..., .add = TRUE) |> group_split()`.
+
+# group_split.rowwise_df() warns about `...`
+
+    Code
+      out <- group_split(rowwise(mtcars), cyl)
+    Condition
+      Warning:
+      Calling `group_split()` on a <rowwise_df> ignores `...`. Please use `as_tibble() |> group_split(...)`.
+

tests/testthat/test-group-nest.R

@@ -47,8 +47,10 @@ test_that("group_nest() works on grouped data frames", {
   expect_equal(names(bind_rows(!!!res$data)), names(starwars))
 })
 
-test_that("group_nest.grouped_df() warns about ...", {
-  expect_warning(group_nest(group_by(mtcars, cyl), cyl))
+test_that("group_nest.grouped_df() warns about `...`", {
+  expect_snapshot({
+    group_nest(group_by(mtcars, cyl), cyl)
+  })
 })
 
 test_that("group_nest() works if no grouping column", {

tests/testthat/test-group-split.R

@@ -41,12 +41,16 @@ test_that("group_split() respects empty groups", {
   expect_identical(res, list_of(tbl[1:2, ], tbl[3:4, ], tbl[integer(), ]))
 })
 
-test_that("group_split.grouped_df() warns about ...", {
-  expect_warning(group_split(group_by(mtcars, cyl), cyl))
+test_that("group_split.grouped_df() warns about `...`", {
+  expect_snapshot({
+    out <- group_split(group_by(mtcars, cyl), cyl)
+  })
 })
 
-test_that("group_split.rowwise_df() warns about ...", {
-  expect_warning(group_split(rowwise(mtcars), cyl))
+test_that("group_split.rowwise_df() warns about `...`", {
+  expect_snapshot({
+    out <- group_split(rowwise(mtcars), cyl)
+  })
 })
 
 test_that("group_split.grouped_df() works", {