diff --git a/NEWS.md b/NEWS.md index 20927d2b..d4775f67 100644 --- a/NEWS.md +++ b/NEWS.md @@ -5,6 +5,7 @@ * `@section` titles can now contain code that includes a colon (#1878). * The automatic usage for a data object that is conditional on the `LazyData` option in the `DESCRIPTION` (see below) now correctly detects all ways to specify a true value, e.g. also `yes`, `Yes` or `True` (@jranke, #1881). * `@import` now inserts the directive as is into `NAMESPACE` when it contains a comma, making it possible to use other forms like `@import rlang, except = ":="`. +* `@importFrom`, `@importClassesFrom`, and `@importMethodsFrom` now accept multi-line input, restoring the ability to spread imports across multiple lines for readability; continuation lines must use a hanging indent, so the first flush or blank line ends the tag and content after it (e.g. from a forgotten `@examples`) is no longer silently absorbed into the namespace (#1890). # roxygen2 8.0.0 diff --git a/R/namespace.R b/R/namespace.R index 4b43ea25..c2fd5f43 100644 --- a/R/namespace.R +++ b/R/namespace.R @@ -267,7 +267,7 @@ roxy_tag_ns.roxy_tag_import <- function(x, block, env) { #' @export roxy_tag_parse.roxy_tag_importClassesFrom <- function(x) { - tag_words(x, min = 2) + tag_words(x, min = 2, multiline = "indent") } #' @export roxy_tag_ns.roxy_tag_importClassesFrom <- function(x, block, env) { @@ -276,7 +276,7 @@ roxy_tag_ns.roxy_tag_importClassesFrom <- function(x, block, env) { #' @export roxy_tag_parse.roxy_tag_importFrom <- function(x) { - tag_words(x, min = 2) + tag_words(x, min = 2, multiline = "indent") } #' @export roxy_tag_ns.roxy_tag_importFrom <- function(x, block, env) { @@ -301,7 +301,7 @@ roxy_tag_ns.roxy_tag_importFrom <- function(x, block, env) { #' @export roxy_tag_parse.roxy_tag_importMethodsFrom <- function(x) { - tag_words(x, min = 2) + tag_words(x, min = 2, multiline = "indent") } #' @export roxy_tag_ns.roxy_tag_importMethodsFrom <- function(x, block, env) { diff --git a/R/rd-describe-in.R b/R/rd-describe-in.R index 72c5a664..8d285508 100644 --- a/R/rd-describe-in.R +++ b/R/rd-describe-in.R @@ -10,7 +10,7 @@ roxy_tag_parse.roxy_tag_describeIn <- function(x) { ) NULL } else { - tag_two_part(x, "a topic name", "a description", multiline = TRUE) + tag_two_part(x, "a topic name", "a description", multiline = "always") } } diff --git a/R/rd-examples.R b/R/rd-examples.R index c0c68de9..09786b12 100644 --- a/R/rd-examples.R +++ b/R/rd-examples.R @@ -38,7 +38,7 @@ roxy_tag_parse.roxy_tag_examplesIf <- function(x) { } #' @export roxy_tag_parse.roxy_tag_example <- function(x) { - x <- tag_value(x, multiline = TRUE) + x <- tag_value(x, multiline = "always") nl <- re_count(x$val, "\n") if (any(nl) > 0) { diff --git a/R/rd-params.R b/R/rd-params.R index 4edb6777..7bcc9a05 100644 --- a/R/rd-params.R +++ b/R/rd-params.R @@ -1,6 +1,6 @@ #' @export roxy_tag_parse.roxy_tag_param <- function(x) { - tag_two_part(x, "an argument name", "a description", multiline = TRUE) + tag_two_part(x, "an argument name", "a description", multiline = "always") } #' @export diff --git a/R/rd-r6-external.R b/R/rd-r6-external.R index 123a7e5b..6e960b76 100644 --- a/R/rd-r6-external.R +++ b/R/rd-r6-external.R @@ -5,7 +5,7 @@ roxy_tag_parse.roxy_tag_R6method <- function(x) { warn_roxy_tag(x, "requires a value like {.code Class$method}") return(NULL) } - warn_if_multiline(x, raw, multiline = FALSE) + raw <- check_multiline(x, raw, multiline = "never") pieces <- strsplit(raw, "\\$")[[1]] if (length(pieces) != 2 || pieces[1] == "" || pieces[2] == "") { diff --git a/R/rd-raw.R b/R/rd-raw.R index d828f4a6..8dd1579a 100644 --- a/R/rd-raw.R +++ b/R/rd-raw.R @@ -6,7 +6,7 @@ roxy_tag_rd.roxy_tag_evalRd <- function(x, base_path, env) { } #' @export -roxy_tag_parse.roxy_tag_rawRd <- function(x) tag_value(x, multiline = TRUE) +roxy_tag_parse.roxy_tag_rawRd <- function(x) tag_value(x, multiline = "always") #' @export roxy_tag_rd.roxy_tag_rawRd <- function(x, base_path, env) { rd_section(x$tag, x$val) diff --git a/R/rd-s4.R b/R/rd-s4.R index e22c3d2f..ba9d0915 100644 --- a/R/rd-s4.R +++ b/R/rd-s4.R @@ -1,6 +1,6 @@ #' @export roxy_tag_parse.roxy_tag_field <- function(x) { - tag_two_part(x, "a field name", "a description", multiline = TRUE) + tag_two_part(x, "a field name", "a description", multiline = "always") } #' @export roxy_tag_rd.roxy_tag_field <- function(x, base_path, env) { @@ -14,7 +14,7 @@ format.rd_section_field <- function(x, ...) { #' @export roxy_tag_parse.roxy_tag_slot <- function(x) { - tag_two_part(x, "a slot name", "a description", multiline = TRUE) + tag_two_part(x, "a slot name", "a description", multiline = "always") } #' @export roxy_tag_rd.roxy_tag_slot <- function(x, base_path, env) { diff --git a/R/rd-s7.R b/R/rd-s7.R index d9d8864e..004cdad7 100644 --- a/R/rd-s7.R +++ b/R/rd-s7.R @@ -1,6 +1,6 @@ #' @export roxy_tag_parse.roxy_tag_prop <- function(x) { - x <- tag_two_part(x, "a property name", "a description", multiline = TRUE) + x <- tag_two_part(x, "a property name", "a description", multiline = "always") if (is.null(x)) { return() } diff --git a/R/rd-template.R b/R/rd-template.R index 9bca3cfe..78316d88 100644 --- a/R/rd-template.R +++ b/R/rd-template.R @@ -5,7 +5,7 @@ roxy_tag_parse.roxy_tag_template <- function(x) { #' @export roxy_tag_parse.roxy_tag_templateVar <- function(x) { - tag_two_part(x, "a variable name", "a value", multiline = TRUE) + tag_two_part(x, "a variable name", "a value", multiline = "always") } process_templates <- function(block, base_path) { diff --git a/R/rd-usage.R b/R/rd-usage.R index c6b11d20..d86749c6 100644 --- a/R/rd-usage.R +++ b/R/rd-usage.R @@ -1,6 +1,6 @@ #' @export roxy_tag_parse.roxy_tag_usage <- function(x) { - x <- tag_value(x, multiline = TRUE) + x <- tag_value(x, multiline = "always") x$val <- rd(x$val) x } diff --git a/R/tag-parser.R b/R/tag-parser.R index 32877c96..f955a4a7 100644 --- a/R/tag-parser.R +++ b/R/tag-parser.R @@ -16,10 +16,21 @@ NULL #' @export #' @rdname tag_parsers -#' @param multiline If `FALSE` (the default), tags that span multiple lines -#' will generate a warning. Set to `TRUE` for tags where multiline content -#' is expected (e.g., `@usage`, `@rawRd`). -tag_value <- function(x, multiline = FALSE) { +#' @param multiline Controls how the tag may span multiple lines: +#' * `"never"` (the default): the tag must be a single line, and spanning +#' multiple lines generates a warning. +#' * `"indent"`: the tag may span multiple lines, but continuation lines must +#' use a hanging indent (i.e. be indented more than the first line). The +#' first line that is not indented (including a blank line) ends the tag, +#' and anything after it is ignored, with a warning. Use this for tags where +#' multiline input is convenient but a flush line almost always signals a +#' missing tag (e.g., `@importFrom`). +#' * `"always"`: the tag may span any number of lines and paragraphs. Use this +#' for tags where multiline content is expected (e.g., `@usage`, `@rawRd`). +#' +#' For backward compatibility, `FALSE` and `TRUE` are accepted as synonyms for +#' `"never"` and `"always"` respectively. +tag_value <- function(x, multiline = "never") { x$val <- trimws(x$raw) if (x$val == "") { @@ -27,7 +38,7 @@ tag_value <- function(x, multiline = FALSE) { return(NULL) } - warn_if_multiline(x, x$val, multiline) + x$val <- check_multiline(x, x$val, multiline) if (!rdComplete(x$raw, is_code = FALSE)) { warn_roxy_tag(x, "has mismatched braces or quotes") @@ -123,7 +134,7 @@ tag_two_part <- function( second, required = TRUE, markdown = TRUE, - multiline = FALSE + multiline = "never" ) { if (trimws(x$raw) == "") { if (!required) { @@ -136,8 +147,8 @@ tag_two_part <- function( warn_roxy_tag(x, "has mismatched braces or quotes") NULL } else { - warn_if_multiline(x, trimws(x$raw), multiline) - pieces <- split_two_part(trimws(x$raw)) + val <- check_multiline(x, trimws(x$raw), multiline) + pieces <- split_two_part(val) if (required && pieces[[2]] == "") { warn_roxy_tag(x, "requires two parts: {first} and {second}") @@ -186,12 +197,12 @@ tag_name_description <- function(x) { #' @export #' @rdname tag_parsers #' @param min,max Minimum and maximum number of words -tag_words <- function(x, min = 0, max = Inf, multiline = FALSE) { +tag_words <- function(x, min = 0, max = Inf, multiline = "never") { val <- trimws(x$raw) - warn_if_multiline(x, val, multiline) + val <- check_multiline(x, val, multiline) - if (!rdComplete(x$raw, is_code = FALSE)) { + if (!rdComplete(val, is_code = FALSE)) { warn_roxy_tag(x, "has mismatched braces or quotes") return(NULL) } @@ -216,11 +227,35 @@ tag_words_line <- function(x) { tag_words(x) } -# Warns if multiline is FALSE and val contains multiple lines. -warn_if_multiline <- function(x, val, multiline) { - if (multiline) { - return(invisible()) +# Normalises the `multiline` argument to one of "never", "indent", or "always", +# silently translating the legacy `FALSE`/`TRUE` values for backward +# compatibility. +as_multiline <- function(multiline, error_call = caller_env()) { + if (isTRUE(multiline)) { + return("always") + } + if (isFALSE(multiline)) { + return("never") + } + + arg_match0(multiline, c("never", "indent", "always"), error_call = error_call) +} + +# Applies the multiline policy for a tag's value, warning when it is violated +# and returning the value to use (possibly truncated to its hanging-indented +# continuation). See the `multiline` parameter of `tag_value()` for the meaning +# of each mode. +check_multiline <- function(x, val, multiline) { + multiline <- as_multiline(multiline) + + if (multiline == "always") { + return(val) + } + + if (multiline == "indent") { + return(check_indent(x, val)) } + n_lines <- re_count(val, "\n") if (n_lines >= 1) { first_line <- re_split_half(val, "\n")[[1]] @@ -232,7 +267,35 @@ warn_if_multiline <- function(x, val, multiline) { ) ) } - invisible() + + val +} + +# Keeps the first line of `val` plus any immediately following lines that use a +# hanging indent (indented more than the first line). The first flush or blank +# line ends the tag; anything after it is dropped with a warning, since a flush +# line usually signals a forgotten tag (e.g. a missing `@examples`). +check_indent <- function(x, val) { + lines <- strsplit(val, "\n", fixed = TRUE)[[1]] + if (length(lines) <= 1) { + return(val) + } + + indent <- leadingSpaces(lines) + continues <- nzchar(trimws(lines[-1])) & indent[-1] > indent[[1]] + + ends_at <- if (all(continues)) length(lines) else which(!continues)[[1]] + if (ends_at < length(lines)) { + warn_roxy_tag( + x, + c( + "must use a hanging indent to span multiple lines", + i = "Continuation lines must be indented; did you forget a tag like {.code @examples}?" + ) + ) + } + + paste(lines[seq_len(ends_at)], collapse = "\n") } #' @export diff --git a/man/tag_parsers.Rd b/man/tag_parsers.Rd index 820a25be..39bae640 100644 --- a/man/tag_parsers.Rd +++ b/man/tag_parsers.Rd @@ -16,7 +16,7 @@ \alias{tag_markdown_with_sections} \title{Parse tags} \usage{ -tag_value(x, multiline = FALSE) +tag_value(x, multiline = "never") tag_inherit(x) @@ -28,12 +28,12 @@ tag_two_part( second, required = TRUE, markdown = TRUE, - multiline = FALSE + multiline = "never" ) tag_name_description(x) -tag_words(x, min = 0, max = Inf, multiline = FALSE) +tag_words(x, min = 0, max = Inf, multiline = "never") tag_words_line(x) @@ -50,9 +50,22 @@ tag_markdown_with_sections(x) \arguments{ \item{x}{A \link{roxy_tag} object to parse} -\item{multiline}{If \code{FALSE} (the default), tags that span multiple lines -will generate a warning. Set to \code{TRUE} for tags where multiline content -is expected (e.g., \verb{@usage}, \verb{@rawRd}).} +\item{multiline}{Controls how the tag may span multiple lines: +\itemize{ +\item \code{"never"} (the default): the tag must be a single line, and spanning +multiple lines generates a warning. +\item \code{"indent"}: the tag may span multiple lines, but continuation lines must +use a hanging indent (i.e. be indented more than the first line). The +first line that is not indented (including a blank line) ends the tag, +and anything after it is ignored, with a warning. Use this for tags where +multiline input is convenient but a flush line almost always signals a +missing tag (e.g., \verb{@importFrom}). +\item \code{"always"}: the tag may span any number of lines and paragraphs. Use this +for tags where multiline content is expected (e.g., \verb{@usage}, \verb{@rawRd}). +} + +For backward compatibility, \code{FALSE} and \code{TRUE} are accepted as synonyms for +\code{"never"} and \code{"always"} respectively.} \item{first, second}{Name of first and second parts of two part tags} diff --git a/tests/testthat/_snaps/namespace.md b/tests/testthat/_snaps/namespace.md index a6336eab..d649bfd1 100644 --- a/tests/testthat/_snaps/namespace.md +++ b/tests/testthat/_snaps/namespace.md @@ -5,8 +5,8 @@ Message Writing 'NAMESPACE' i Loading testNamespace - x multiline.R:1: @importFrom must be only 1 line long, not 2. - i The first line is "stats median" + x multiline.R:1: @import must be only 1 line long, not 2. + i The first line is "stats" # @exportS3Method generates fully automatically @@ -43,13 +43,21 @@ Message x :2: @importFrom must have at least 2 words, not 1. -# multiline importFrom generates warning +# blank line ends a multiline importFrom Code - . <- roc_proc_text(namespace_roclet(), block) + out <- roc_proc_text(namespace_roclet(), block) + Message + x :2: @importFrom must use a hanging indent to span multiple lines. + i Continuation lines must be indented; did you forget a tag like `@examples`? + +# flush line ends a multiline importFrom + + Code + out <- roc_proc_text(namespace_roclet(), block) Message - x :2: @importFrom must be only 1 line long, not 2. - i The first line is "test test1" + x :2: @importFrom must use a hanging indent to span multiple lines. + i Continuation lines must be indented; did you forget a tag like `@examples`? # can regenerate NAMESPACE even if its broken diff --git a/tests/testthat/test-namespace.R b/tests/testthat/test-namespace.R index 224033df..e8aea3bc 100644 --- a/tests/testthat/test-namespace.R +++ b/tests/testthat/test-namespace.R @@ -19,7 +19,7 @@ test_that("end-to-end NAMESPACE generation works", { test_that("parsing warnings only fire once", { path <- local_package_copy(test_path("testNamespace")) write_lines( - c("#' @importFrom stats median", "#' ave", "NULL"), + c("#' @import stats", "#' utils", "NULL"), file.path(path, "R", "multiline.R") ) withr::defer(pkgload::unload("testNamespace")) @@ -298,13 +298,76 @@ test_that("poorly formed importFrom throws error", { expect_snapshot(. <- roc_proc_text(namespace_roclet(), block)) }) -test_that("multiline importFrom generates warning", { - block <- " +test_that("multiline importFrom is accepted", { + out <- roc_proc_text( + namespace_roclet(), + " #' @importFrom test test1 #' test2 NULL " - expect_snapshot(. <- roc_proc_text(namespace_roclet(), block)) + ) + expect_equal( + sort(out), + sort(c("importFrom(test,test1)", "importFrom(test,test2)")) + ) +}) + +test_that("multiline importClassesFrom is accepted", { + out <- roc_proc_text( + namespace_roclet(), + " + #' @importClassesFrom test test1 + #' test2 + NULL + " + ) + expect_equal( + sort(out), + sort(c("importClassesFrom(test,test1)", "importClassesFrom(test,test2)")) + ) +}) + +test_that("multiline importMethodsFrom is accepted", { + out <- roc_proc_text( + namespace_roclet(), + " + #' @importMethodsFrom test test1 + #' test2 + NULL + " + ) + expect_equal( + sort(out), + sort(c("importMethodsFrom(test,test1)", "importMethodsFrom(test,test2)")) + ) +}) + +test_that("blank line ends a multiline importFrom", { + block <- " + #' @importFrom test test1 test2 + #' + #' a <- 1 + NULL + " + expect_snapshot(out <- roc_proc_text(namespace_roclet(), block)) + expect_equal( + sort(out), + sort(c("importFrom(test,test1)", "importFrom(test,test2)")) + ) +}) + +test_that("flush line ends a multiline importFrom", { + block <- " + #' @importFrom test test1 test2 + #' a <- 1 + NULL + " + expect_snapshot(out <- roc_proc_text(namespace_roclet(), block)) + expect_equal( + sort(out), + sort(c("importFrom(test,test1)", "importFrom(test,test2)")) + ) }) test_that("import doesn't quote if comma present", { diff --git a/tests/testthat/test-tag-parser.R b/tests/testthat/test-tag-parser.R index 35ab2dd9..21e2358a 100644 --- a/tests/testthat/test-tag-parser.R +++ b/tests/testthat/test-tag-parser.R @@ -97,6 +97,15 @@ test_that("tag_two_part() warns on multi-line content and preserves value", { expect_equal(out$val, list(name = "foo", description = "bar\nbaz")) }) +test_that("multiline accepts string strategies and legacy booleans", { + expect_equal(as_multiline("never"), "never") + expect_equal(as_multiline("indent"), "indent") + expect_equal(as_multiline("always"), "always") + expect_equal(as_multiline(FALSE), "never") + expect_equal(as_multiline(TRUE), "always") + expect_error(as_multiline("nope")) +}) + test_that("tag_value() warns on multi-line content and preserves value", { expect_snapshot({ tag <- roxy_test_tag("a\nb")