From 429eabf40ded67bb140df16459fbda9482ed9be4 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Wed, 15 Jan 2025 22:24:52 +0000 Subject: [PATCH 1/3] rename helper argument to match the arg name in parent --- R/expect-condition.R | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/R/expect-condition.R b/R/expect-condition.R index f032b5573..02d29728b 100644 --- a/R/expect-condition.R +++ b/R/expect-condition.R @@ -286,17 +286,17 @@ expect_condition_matching <- function(base_class, cnd_matcher <- function(base_class, class = NULL, - pattern = NULL, + regexp = NULL, ..., inherit = TRUE, ignore_deprecation = FALSE, error_call = caller_env()) { check_string(class, allow_null = TRUE, call = error_call) - check_string(pattern, allow_null = TRUE, allow_na = TRUE, call = error_call) + check_string(regexp, allow_null = TRUE, allow_na = TRUE, call = error_call) - if (is.null(pattern) && dots_n(...) > 0) { + if (is.null(regexp) && dots_n(...) > 0) { cli::cli_abort( - "Can't specify {.arg ...} without {.arg pattern}.", + "Can't specify {.arg ...} without {.arg regexp}.", call = error_call ) } @@ -317,12 +317,12 @@ cnd_matcher <- function(base_class, if (!is.null(class) && !inherits(x, class)) { return(FALSE) } - if (!is.null(pattern) && !isNA(pattern)) { + if (!is.null(regexp) && !isNA(regexp)) { withCallingHandlers( - grepl(pattern, conditionMessage(x), ...), + grepl(regexp, conditionMessage(x), ...), error = function(e) { cli::cli_abort( - "Failed to compare {base_class} to {.arg pattern}.", + "Failed to compare {base_class} to {.arg regexp}.", parent = e, call = error_call ) From b226799b71b26675e288541b0ca6315375c522f8 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Wed, 15 Jan 2025 22:42:34 +0000 Subject: [PATCH 2/3] proposed fix for ambiguous error message --- R/expect-condition.R | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/R/expect-condition.R b/R/expect-condition.R index 02d29728b..86dba40e2 100644 --- a/R/expect-condition.R +++ b/R/expect-condition.R @@ -295,8 +295,20 @@ cnd_matcher <- function(base_class, check_string(regexp, allow_null = TRUE, allow_na = TRUE, call = error_call) if (is.null(regexp) && dots_n(...) > 0) { + # pretty adversarial: expect_error(foo(), NULL, "error", 1) + # TODO(R>=4.1.0): ...names() + dots_names <- names(list(...)) + if (is.null(dots_names) { + cli::cli_abort( + "Found unnamed arguments in {.arg ...} to be passed to {.fn grepl}, but no {.arg regexp}", + call = error_call + ) + } cli::cli_abort( - "Can't specify {.arg ...} without {.arg regexp}.", + c("Found arguments in {.arg ...} to be passed to {.fn grepl}, but no {.arg regexp}", + "*" = "First problematic argument:", + "i" = dots_names[which(nzchar(dots_names))[1L]] + ), call = error_call ) } From ec92bbdbffab97769121bdd14f2167dd90459e65 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Wed, 15 Jan 2025 22:45:29 +0000 Subject: [PATCH 3/3] style: message ends with "." --- R/expect-condition.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/expect-condition.R b/R/expect-condition.R index 86dba40e2..393bf58a2 100644 --- a/R/expect-condition.R +++ b/R/expect-condition.R @@ -300,12 +300,12 @@ cnd_matcher <- function(base_class, dots_names <- names(list(...)) if (is.null(dots_names) { cli::cli_abort( - "Found unnamed arguments in {.arg ...} to be passed to {.fn grepl}, but no {.arg regexp}", + "Found unnamed arguments in {.arg ...} to be passed to {.fn grepl}, but no {.arg regexp}.", call = error_call ) } cli::cli_abort( - c("Found arguments in {.arg ...} to be passed to {.fn grepl}, but no {.arg regexp}", + c("Found arguments in {.arg ...} to be passed to {.fn grepl}, but no {.arg regexp}.", "*" = "First problematic argument:", "i" = dots_names[which(nzchar(dots_names))[1L]] ),