From 036e74a0ff996403ea5006027eaebea16d25c377 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Mon, 22 Jan 2024 13:14:18 -0600 Subject: [PATCH] Correctly interpolate dangerous string into cli warning (#1576) Fixes #1575 --- DESCRIPTION | 2 +- NEWS.md | 3 +++ R/namespace.R | 10 +++++++--- tests/testthat/_snaps/namespace.md | 12 ++++++++---- tests/testthat/test-namespace.R | 26 +++++++++++++++++++------- 5 files changed, 38 insertions(+), 15 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 9e847299..eb9922f3 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -53,4 +53,4 @@ Config/testthat/parallel: TRUE Encoding: UTF-8 Language: en-GB Roxygen: list(markdown = TRUE, load = "installed") -RoxygenNote: 7.2.3.9000 +RoxygenNote: 7.3.0.9000 diff --git a/NEWS.md b/NEWS.md index e382959f..3f5e80f0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,9 @@ # roxygen2 (development version) +* S3 method export warning no longer fails if class contains `{` or `}` (#1575). + * `@family` lists are now ordered more carefully, "foo1" comes after "foo" (#1563, @krlmlr). + * Re-runs of `namespace_roclet()` fixed for packages using multi-line `@rawNamespace` directives (#1572, @MichaelChirico). * `@importFrom` works again for quoted non-syntactic names, e.g. diff --git a/R/namespace.R b/R/namespace.R index 22efffc5..b4d93d01 100644 --- a/R/namespace.R +++ b/R/namespace.R @@ -401,7 +401,11 @@ warn_missing_s3_exports <- function(blocks, env) { undocumented <- methods[!methods %in% s3objects] srcrefs <- map(undocumented, attr, "srcref") - messages <- paste0("S3 method `", names(undocumented) , "` needs @export or @exportS3method tag") - map2(undocumented, messages, warn_roxy_function) -} + map2(undocumented, names(undocumented), function(fun, name) { + warn_roxy_function( + fun, + "S3 method {.arg {name}} needs @export or @exportS3method tag" + ) + }) +} diff --git a/tests/testthat/_snaps/namespace.md b/tests/testthat/_snaps/namespace.md index a9f47daf..57e080ee 100644 --- a/tests/testthat/_snaps/namespace.md +++ b/tests/testthat/_snaps/namespace.md @@ -95,11 +95,15 @@ # warns if S3 method not documented Code - roc_proc_text(namespace_roclet(), - "\n foo <- function(x) UseMethod('foo')\n foo.numeric <- function(x) 1\n\n mean.myclass <- function(x) 2\n ") + . <- roc_proc_text(namespace_roclet(), block) Message x :5: S3 method `mean.myclass` needs @export or @exportS3method tag. x :3: S3 method `foo.numeric` needs @export or @exportS3method tag. - Output - character(0) + +--- + + Code + . <- roc_proc_text(namespace_roclet(), block) + Message + x :3: S3 method `foo.{` needs @export or @exportS3method tag. diff --git a/tests/testthat/test-namespace.R b/tests/testthat/test-namespace.R index 1bea969c..1dadfa1e 100644 --- a/tests/testthat/test-namespace.R +++ b/tests/testthat/test-namespace.R @@ -472,19 +472,31 @@ test_that("non-syntactic imports can use multiple quoting forms", { # warn_missing_s3_exports ------------------------------------------------- test_that("warns if S3 method not documented", { - expect_snapshot( - roc_proc_text(namespace_roclet(), " + # Need to manually transform since the srcref is coming from the function; + # roc_proc_text() uses fake srcrefs for the blocks themselves + fix_srcref <- function(x) gsub("file[a-z0-9]+", "", x) + + block <- " foo <- function(x) UseMethod('foo') foo.numeric <- function(x) 1 mean.myclass <- function(x) 2 - "), - # Need to manually transform since the srcref is coming from the function; - # roc_proc_text() uses fake srcrefs for the blocks themselves - transform = function(x) gsub("file[a-z0-9]+", "", x) + " + expect_snapshot( + . <- roc_proc_text(namespace_roclet(), block), + transform = fix_srcref ) -}) + # Works even if method contains { + block <- " + foo <- function(x) UseMethod('foo') + `foo.{` <- function(x) 1 + " + expect_snapshot( + . <- roc_proc_text(namespace_roclet(), block), + transform = fix_srcref + ) +}) test_that("can suppress the warning", { block <- "