diff --git a/NEWS.md b/NEWS.md index d9e0aabae..3a713b0df 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # rlang (development version) +* `arg_match()` now throws better error messages if `multiple = TRUE` or if a single value is allowed (@olivroy, #1635, #1682). + * `env_browse()` and `env_is_browsed()` are now defunct as they require an API that is no longer available to packages (#1727). diff --git a/R/arg.R b/R/arg.R index 42ae5535c..5221d54fa 100644 --- a/R/arg.R +++ b/R/arg.R @@ -60,7 +60,7 @@ arg_match <- function(arg, abort(msg, call = error_call, arg = error_arg) } if (length(arg) > 1 && !setequal(arg, values)) { - msg <- arg_match_invalid_msg(arg, values, error_arg) + msg <- arg_match_invalid_msg(arg, values, multiple, error_arg) abort(msg, call = error_call, arg = error_arg) } @@ -74,6 +74,8 @@ arg_match <- function(arg, } arg_match_multi <- function(arg, values, error_arg, error_call) { + # Set an option temporarily to make sure the multiple message shows. + local_options("rlang_multiple_error" = TRUE) map_chr(arg, ~ arg_match0(.x, values, error_arg, error_call = error_call)) } @@ -128,7 +130,8 @@ stop_arg_match <- function(arg, values, error_arg, error_call) { check_string(arg, arg = error_arg, call = error_call) } - msg <- arg_match_invalid_msg(arg, values, error_arg) + multiple <- getOption("rlang_multiple_error", FALSE) + msg <- arg_match_invalid_msg(arg, values, multiple, error_arg) # Try suggest the most probable and helpful candidate value candidate <- NULL @@ -160,14 +163,26 @@ stop_arg_match <- function(arg, values, error_arg, error_call) { abort(msg, call = error_call, arg = error_arg) } -arg_match_invalid_msg <- function(val, values, error_arg) { - msg <- paste0(format_arg(error_arg), " must be one of ") +arg_match_invalid_msg <- function(val, values, multiple, error_arg) { + if (length(values) == 1) { + #1635 + word <- " must be " + } else if (multiple) { + #1682 + word <- " must be in " + } else { + word <- " must be one of " + } + msg <- paste0(format_arg(error_arg), word) msg <- paste0(msg, oxford_comma(chr_quoted(values, "\""))) if (is_null(val)) { msg <- paste0(msg, ".") } else { - msg <- paste0(msg, sprintf(', not "%s\".', val[[1]])) + # only show incorrect values. + incorrect_val <- setdiff(val, values) + incorrect_val <- oxford_comma(incorrect_val) + msg <- paste0(msg, sprintf(', not "%s\".', incorrect_val)) } msg diff --git a/tests/testthat/_snaps/arg.md b/tests/testthat/_snaps/arg.md index a27951d92..a0fb721ec 100644 --- a/tests/testthat/_snaps/arg.md +++ b/tests/testthat/_snaps/arg.md @@ -19,6 +19,19 @@ `arg` must be length 1 or a permutation of `c("foo", "bar")`. +# arg_match() works well when multiple = TRUE (#1682) + + Code + f("x", values = c("y", "z")) + Condition + Error in `f()`: + ! `x` must be in "y" or "z", not "x". + Code + f(c("x", "y"), values = c("y", "z")) + Condition + Error in `f()`: + ! `x` must be in "y" or "z", not "x". + # `arg_match()` has informative error messages Code @@ -138,14 +151,14 @@ Output Error in `my_wrapper()`: - ! `my_arg` must be one of "foo", "bar", or "baz", not "ba". + ! `my_arg` must be in "foo", "bar", or "baz", not "ba". i Did you mean "bar"? Code (expect_error(my_wrapper(c("foo", "ba")))) Output Error in `my_wrapper()`: - ! `my_arg` must be one of "foo", "bar", or "baz", not "ba". + ! `my_arg` must be in "foo", "bar", or "baz", not "ba". i Did you mean "bar"? # arg_match0() defuses argument diff --git a/tests/testthat/_snaps/cnd-abort.md b/tests/testthat/_snaps/cnd-abort.md index ba16518c8..58941b879 100644 --- a/tests/testthat/_snaps/cnd-abort.md +++ b/tests/testthat/_snaps/cnd-abort.md @@ -1397,21 +1397,21 @@ Output Error: - ! `"f"` must be one of "foo", not "f". + ! `"f"` must be "foo", not "f". i Did you mean "foo"? Code (expect_error(eval_bare(quote(arg_match0("f", "foo"))))) Output Error: - ! `"f"` must be one of "foo", not "f". + ! `"f"` must be "foo", not "f". i Did you mean "foo"? Code (expect_error(eval_bare(quote(arg_match0("f", "foo")), env()))) Output Error: - ! `"f"` must be one of "foo", not "f". + ! `"f"` must be "foo", not "f". i Did you mean "foo"? # error_call() and format_error_call() preserve special syntax ops diff --git a/tests/testthat/test-arg.R b/tests/testthat/test-arg.R index 0b0a2add3..9374ad89f 100644 --- a/tests/testthat/test-arg.R +++ b/tests/testthat/test-arg.R @@ -50,6 +50,17 @@ test_that("informative error message on partial match", { ) }) +test_that("arg_match() works well when multiple = TRUE (#1682)", { + f <- function(x, ...) { + arg_match(x, ..., multiple = TRUE) + } + expect_snapshot(error = TRUE, { + f("x", values = c("y", "z")) + f(c("x", "y"), values = c("y", "z")) + + }) +}) + test_that("`arg_match()` has informative error messages", { arg_match_wrapper <- function(...) { arg_match0_wrapper(...)