Only consult LHS inputs to determine case_when() output size

Author: DavisVaughan

NEWS.md

@@ -1,5 +1,7 @@
 # dplyr (development version)
 
+* `case_when()` now determines the output size from only the LHS inputs, rather than from both the LHS and RHS inputs. This allows the legacy behavior of `TRUE ~` to continue to work, while improving `case_when()`'s safety by guarding against some silent confusing failure cases (#7082).
+
 * `bind_rows()` now replaces empty (or `NA`) element names in a list with its numeric index while preserving existing names (#7719, @Meghansaha).
 
 * New `slice_sample()` example showing how to use it to shuffle rows (#7707, @Hzanib).

R/case-when.R

@@ -16,18 +16,18 @@
 #'
 #'   The RHS inputs will be coerced to their common type.
 #'
-#'   All inputs will be recycled to their common size. That said, we encourage
-#'   all LHS inputs to be the same size. Recycling is mainly useful for RHS
-#'   inputs, where you might supply a size 1 input that will be recycled to the
-#'   size of the LHS inputs.
+#'   For historical reasons, all LHS inputs will be recycled to their common
+#'   size. That said, we encourage all LHS inputs to be the same size, which you
+#'   can optionally enforce with `.size`. All RHS inputs will be recycled to the
+#'   common size of the LHS inputs.
 #'
 #'   `NULL` inputs are ignored.
 #'
 #' @param .default The value used when all of the LHS inputs return either
 #'   `FALSE` or `NA`.
 #'
 #'   `.default` must be size 1 or the same size as the common size computed
-#'   from `...`.
+#'   from the LHS inputs.
 #'
 #'   `.default` participates in the computation of the common type with the RHS
 #'   inputs.
@@ -45,11 +45,12 @@
 #'   supplied, this overrides the common type of the RHS inputs.
 #'
 #' @param .size An optional size declaring the desired output size. If supplied,
-#'   this overrides the common size computed from `...`.
+#'   this overrides the common size computed from the LHS inputs.
 #'
-#' @return A vector with the same size as the common size computed from the
-#'   inputs in `...` and the same type as the common type of the RHS inputs
-#'   in `...`.
+#' @return A vector
+#'
+#'   - The size of the vector is the common size of the LHS inputs, or `.size`.
+#'   - The type of the vector is the common type of the RHS inputs, or `.ptype`.
 #'
 #' @seealso [case_match()]
 #'
@@ -162,12 +163,18 @@ case_when <- function(..., .default = NULL, .ptype = NULL, .size = NULL) {
   conditions <- args$lhs
   values <- args$rhs
 
-  # `case_when()`'s formula interface finds the common size of ALL of its inputs.
-  # This is what allows `TRUE ~` to work.
-  .size <- vec_size_common(!!!conditions, !!!values, .size = .size)
-
+  # Unfortunate historical behavior patch:
+  #
+  # `case_when()`'s formula interface finds the common size of all LHS inputs.
+  # This is what allows `TRUE ~` to work. Unfortunately, this has caused a lot
+  # of confusion over the years when `TRUE ~` isn't involved (#7082) and we wish
+  # that we could enforce that all LHS inputs must be the same size (without
+  # recycling).
+  #
+  # Additionally, for `case_when()`, the LHS common size is the size that each
+  # RHS value must recycle to.
+  .size <- vec_size_common(!!!conditions, .size = .size)
   conditions <- vec_recycle_common(!!!conditions, .size = .size)
-  values <- vec_recycle_common(!!!values, .size = .size)
 
   vec_case_when(
     conditions = conditions,

man/case_when.Rd

@@ -20,7 +20,7 @@
       case_when(TRUE ~ 1:2, .size = 3)
     Condition
       Error in `case_when()`:
-      ! Can't recycle `..1 (right)` (size 2) to size 3.
+      ! `..1 (right)` must have size 3, not size 2.
 
 # invalid type errors are correct (#6261) (#6206)
 
@@ -66,14 +66,57 @@
       Caused by error:
       ! oh no!
 
+# only LHS sizes are consulted to compute the output size (#7082)
+
+    Code
+      x <- 1
+      case_when(x == 1 ~ "a", x == 2 ~ character(), .default = "other")
+    Condition
+      Error in `case_when()`:
+      ! `..2 (right)` must have size 1, not size 0.
+
+---
+
+    Code
+      case_when(TRUE ~ 1:3, FALSE ~ 4:6)
+    Condition
+      Error in `case_when()`:
+      ! `..1 (right)` must have size 1, not size 3.
+
+---
+
+    Code
+      case_when(NA ~ 1:3, TRUE ~ 4:6)
+    Condition
+      Error in `case_when()`:
+      ! `..1 (right)` must have size 1, not size 3.
+
+---
+
+    Code
+      x <- 1:3
+      my_scalar_condition <- is.double(x)
+      case_when(my_scalar_condition ~ x, TRUE ~ 4:6)
+    Condition
+      Error in `case_when()`:
+      ! `..1 (right)` must have size 1, not size 3.
+
+---
+
+    Code
+      case_when(TRUE ~ integer(), FALSE ~ integer())
+    Condition
+      Error in `case_when()`:
+      ! `..1 (right)` must have size 1, not size 0.
+
 # case_when() give meaningful errors
 
     Code
       (expect_error(case_when(c(TRUE, FALSE) ~ 1:3, c(FALSE, TRUE) ~ 1:2)))
     Output
-      <error/vctrs_error_incompatible_size>
+      <error/vctrs_error_assert_size>
       Error in `case_when()`:
-      ! Can't recycle `..1 (left)` (size 2) to match `..1 (right)` (size 3).
+      ! `..1 (right)` must have size 2, not size 3.
     Code
       (expect_error(case_when(c(TRUE, FALSE) ~ 1, c(FALSE, TRUE, FALSE) ~ 2, c(FALSE,
         TRUE, FALSE, NA) ~ 3)))
@@ -86,7 +129,7 @@
     Output
       <error/rlang_error>
       Error in `case_when()`:
-      ! `..1 (left)` must be a logical vector, not a double vector.
+      ! `..1 (left)` must be a logical vector, not the number 50.
     Code
       (expect_error(case_when(paste(50))))
     Output

tests/testthat/_snaps/case-when.md

@@ -68,40 +68,6 @@ test_that("any `TRUE` overrides an `NA`", {
   )
 })
 
-test_that("atomic conditions (#2909)", {
-  expect_equal(
-    case_when(
-      TRUE ~ 1:3,
-      FALSE ~ 4:6
-    ),
-    1:3
-  )
-  expect_equal(
-    case_when(
-      NA ~ 1:3,
-      TRUE ~ 4:6
-    ),
-    4:6
-  )
-})
-
-test_that("zero-length conditions and values (#3041)", {
-  expect_equal(
-    case_when(
-      TRUE ~ integer(),
-      FALSE ~ integer()
-    ),
-    integer()
-  )
-  expect_equal(
-    case_when(
-      logical() ~ 1,
-      logical() ~ 2
-    ),
-    numeric()
-  )
-})
-
 test_that("case_when can be used in anonymous functions (#3422)", {
   res <- tibble(a = 1:3) |>
     mutate(b = (function(x) case_when(x < 2 ~ TRUE, .default = FALSE))(a)) |>
@@ -210,8 +176,10 @@ test_that("NULL inputs are compacted", {
 
 test_that("passes through `.default` correctly", {
   expect_identical(case_when(FALSE ~ 1, .default = 2), 2)
-  expect_identical(case_when(FALSE ~ 1:5, .default = 2), rep(2, 5))
-  expect_identical(case_when(FALSE ~ 1:5, .default = 2:6), 2:6)
+  expect_identical(
+    case_when(c(TRUE, FALSE, TRUE, FALSE, TRUE) ~ 1:5, .default = 2),
+    c(1, 2, 3, 2, 5)
+  )
 })
 
 test_that("`.default` isn't part of recycling", {
@@ -269,6 +237,51 @@ test_that("throws chained errors when formula evaluation fails", {
   })
 })
 
+test_that("only LHS sizes are consulted to compute the output size (#7082)", {
+  # Motivating example from #7082. We want this case to fail. LHS common size is
+  # 1. RHS inputs must recycle to this size. If both the LHS and RHS inputs were
+  # consulted to compute a common size of 0, this would incorrectly return
+  # `character()`, which is a silent confusing failure.
+  expect_snapshot(error = TRUE, {
+    x <- 1
+    case_when(
+      x == 1 ~ "a",
+      x == 2 ~ character(),
+      .default = "other"
+    )
+  })
+
+  # The following all used to work but we believe they are not good
+  # examples of how to use `case_when()`, and are not worth supporting if it
+  # means that the actual bug captured above silently slips through
+
+  # Used to return `1:3` and `4:6` respectively (#2909)
+  # This could only practically affect you if you are using `case_when()` like
+  # a very weird if/else, which we do not encourage.
+  expect_snapshot(error = TRUE, {
+    case_when(TRUE ~ 1:3, FALSE ~ 4:6)
+  })
+  expect_snapshot(error = TRUE, {
+    case_when(NA ~ 1:3, TRUE ~ 4:6)
+  })
+  expect_snapshot(error = TRUE, {
+    # In theory a user could have done something very weird like this
+    x <- 1:3
+    my_scalar_condition <- is.double(x)
+    case_when(my_scalar_condition ~ x, TRUE ~ 4:6)
+  })
+
+  # Used to return `integer()` (#3041).
+  # If you go back and look at #3041, it was an issue about the case where all
+  # LHS inputs have size 0 and all RHS inputs have size 1. That still works as
+  # expected. This extra test of LHS inputs having size 1 and RHS inputs having
+  # size 0 was not something the user wanted, and does not make sense to us.
+  expect_snapshot(error = TRUE, {
+    case_when(TRUE ~ integer(), FALSE ~ integer())
+  })
+  expect_identical(case_when(logical() ~ 1, logical() ~ 2), numeric())
+})
+
 test_that("case_when() give meaningful errors", {
   expect_snapshot({
     (expect_error(