From 35c6a2ccae6ad78be186c0091f1f26efe62c46b6 Mon Sep 17 00:00:00 2001 From: olivroy Date: Mon, 20 Oct 2025 09:19:03 -0400 Subject: [PATCH 1/4] add argument to fix message --- R/deprecate.R | 2 +- R/spec.R | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/R/deprecate.R b/R/deprecate.R index bfe4e91..f17cf33 100644 --- a/R/deprecate.R +++ b/R/deprecate.R @@ -346,7 +346,7 @@ lifecycle_message <- function( msg <- lifecycle_message_what(what, when) if (!is_null(with)) { - with <- spec(with, NULL, signaller = signaller) + with <- spec(with, NULL, signaller = signaller, type = "with") msg <- c(msg, "i" = lifecycle_message_with(with, what)) } diff --git a/R/spec.R b/R/spec.R index 2661824..0549eab 100644 --- a/R/spec.R +++ b/R/spec.R @@ -2,6 +2,7 @@ spec <- function( spec, env = caller_env(), signaller = "signal_lifecycle", + type = c("what", "with"), error_call = caller_env() ) { ctxt <- list( @@ -18,7 +19,7 @@ spec <- function( from = signaller ) } else { - what <- parse_what(spec, ctxt = ctxt) + what <- parse_what(spec, ctxt = ctxt, type = type) list( fn = spec_fn(what$call, ctxt = ctxt), @@ -30,16 +31,16 @@ spec <- function( } } -parse_what <- function(what, ctxt) { +parse_what <- function(what, ctxt, type = c("what", "with")) { check_string(what, call = ctxt$call) - + type <- arg_match(type) call <- parse_expr(what) if (!is_call(call)) { what <- as_string(what) cli::cli_abort( c( - "{.arg what} must have function call syntax.", + "{.arg {type}} must have function call syntax.", "", " " = "# Good:", " " = "{ ctxt$signaller }(\"{what}()\")", From cda0f349e230725812bee895c14fc97e7163ae03 Mon Sep 17 00:00:00 2001 From: olivroy Date: Mon, 20 Oct 2025 09:19:12 -0400 Subject: [PATCH 2/4] Add test --- tests/testthat/_snaps/deprecate.md | 15 +++++++++++++++ tests/testthat/test-deprecate.R | 7 +++++++ 2 files changed, 22 insertions(+) diff --git a/tests/testthat/_snaps/deprecate.md b/tests/testthat/_snaps/deprecate.md index 500f99e..610f858 100644 --- a/tests/testthat/_snaps/deprecate.md +++ b/tests/testthat/_snaps/deprecate.md @@ -206,3 +206,18 @@ Error: ! `id` must be a single string, not the number 1. +# deprecate_soft() mentions the correct argument (#152) + + Code + deprecate_soft(when = "1.0.0", what = "foo()", with = "bar") + deprecate_warn(when = "1.0.0", what = "foo()", with = "bar") + Condition + Error in `lifecycle_message()`: + ! `with` must have function call syntax. + + # Good: + deprecate_warn("bar()") + + # Bad: + deprecate_warn("bar") + diff --git a/tests/testthat/test-deprecate.R b/tests/testthat/test-deprecate.R index aa9f6f1..444bd19 100644 --- a/tests/testthat/test-deprecate.R +++ b/tests/testthat/test-deprecate.R @@ -266,3 +266,10 @@ test_that("needs_warning works as expected", { env_poke(deprecation_env, "test", TRUE) expect_false(needs_warning("test")) }) + +test_that("deprecate_soft() mentions the correct argument (#152)", { + expect_snapshot(error = TRUE, { + deprecate_soft(when = "1.0.0", what = "foo()", with = "bar") + deprecate_warn(when = "1.0.0", what = "foo()", with = "bar") + }) +}) From a4d26ecff4c05cfe8d7dd7630ded4c0307303759 Mon Sep 17 00:00:00 2001 From: olivroy Date: Mon, 20 Oct 2025 09:23:21 -0400 Subject: [PATCH 3/4] simple lints --- DESCRIPTION | 3 +-- tests/testthat/helper-lifecycle.R | 4 ---- tests/testthat/test-deprecate.R | 10 +++++----- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index b3f8dd8..4794193 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -19,11 +19,10 @@ Imports: cli (>= 3.4.0), rlang (>= 1.1.0) Suggests: - covr, knitr, lintr (>= 3.1.0), rmarkdown, - testthat (>= 3.0.1), + testthat (>= 3.2.1), tibble, tidyverse, tools, diff --git a/tests/testthat/helper-lifecycle.R b/tests/testthat/helper-lifecycle.R index a62c8c7..24ced16 100644 --- a/tests/testthat/helper-lifecycle.R +++ b/tests/testthat/helper-lifecycle.R @@ -2,10 +2,6 @@ expect_lifecycle_defunct <- function(object, ...) { expect_error(object, class = "defunctError") } -expect_no_warning <- function(...) { - expect_warning(regexp = NA, ...) -} - try2 <- function(expr) { cat(paste0("\n", as_label2(substitute(expr)), ":\n\n")) cat(catch_cnd(expr, classes = "error")$message, "\n\n") diff --git a/tests/testthat/test-deprecate.R b/tests/testthat/test-deprecate.R index 444bd19..87b8f50 100644 --- a/tests/testthat/test-deprecate.R +++ b/tests/testthat/test-deprecate.R @@ -12,8 +12,8 @@ test_that("default deprecations behave as expected", { expect_snapshot({ (expect_warning(direct(), class = "lifecycle_warning_deprecated")) }) - expect_warning(indirect(), NA) - expect_warning(indirect(), NA) + expect_no_warning(indirect()) + expect_no_warning(indirect()) expect_snapshot({ (expect_defunct(deprecate_stop("1.0.0", "foo()"))) @@ -66,8 +66,8 @@ test_that("indirect usage recommends contacting authors", { test_that("quiet suppresses _soft and _warn", { local_options(lifecycle_verbosity = "quiet") - expect_warning(deprecate_soft("1.0.0", "foo()"), NA) - expect_warning(deprecate_warn("1.0.0", "foo()"), NA) + expect_no_warning(deprecate_soft("1.0.0", "foo()")) + expect_no_warning(deprecate_warn("1.0.0", "foo()")) expect_defunct(deprecate_stop("1.0.0", "foo()")) }) @@ -108,7 +108,7 @@ test_that("soft deprecation uses correct calling envs", { # Calling package function indirectly from global env shouldn't cnd <- catch_cnd(evalq(softly_softly(), global_env()), "warning") - expect_equal(cnd, NULL) + expect_null(cnd) }) test_that("warning conditions are signaled only once if warnings are suppressed", { From ff8e9fad2705adc5ace21551c412c4cb7e790eb2 Mon Sep 17 00:00:00 2001 From: olivroy Date: Mon, 20 Oct 2025 10:00:23 -0400 Subject: [PATCH 4/4] only test deprecate_warn() (deprecate_soft() doesn't produce output --- tests/testthat/_snaps/deprecate.md | 1 - tests/testthat/test-deprecate.R | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/testthat/_snaps/deprecate.md b/tests/testthat/_snaps/deprecate.md index 610f858..e597d98 100644 --- a/tests/testthat/_snaps/deprecate.md +++ b/tests/testthat/_snaps/deprecate.md @@ -209,7 +209,6 @@ # deprecate_soft() mentions the correct argument (#152) Code - deprecate_soft(when = "1.0.0", what = "foo()", with = "bar") deprecate_warn(when = "1.0.0", what = "foo()", with = "bar") Condition Error in `lifecycle_message()`: diff --git a/tests/testthat/test-deprecate.R b/tests/testthat/test-deprecate.R index 87b8f50..3f852bf 100644 --- a/tests/testthat/test-deprecate.R +++ b/tests/testthat/test-deprecate.R @@ -269,7 +269,6 @@ test_that("needs_warning works as expected", { test_that("deprecate_soft() mentions the correct argument (#152)", { expect_snapshot(error = TRUE, { - deprecate_soft(when = "1.0.0", what = "foo()", with = "bar") deprecate_warn(when = "1.0.0", what = "foo()", with = "bar") }) })