Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix idempotency of roxygenize() for packages using multi-line @rawNamespace #1573

Merged
merged 10 commits into from
Jan 22, 2024
1 change: 1 addition & 0 deletions .Rbuildignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
^\.github$
^pkgdown$
^\.covrignore$
^\.devcontainer$
^azure-pipelines\.yml$
^\.Rprofile$
^r-packages$
Expand Down
14 changes: 14 additions & 0 deletions .devcontainer/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
FROM rocker/r-base

RUN apt-get -qq update && \
apt-get install -y --no-install-recommends git libxml2-dev

COPY DESCRIPTION .

RUN Rscript -e ' \
install.packages("remotes"); \
remotes::install_deps(dependencies = c( \
"Imports", \
"Config/Needs/development" \
)) \
'
3 changes: 3 additions & 0 deletions .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"build": { "dockerfile": "Dockerfile", "context": ".."}
}
1 change: 1 addition & 0 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ LinkingTo:
cpp11
VignetteBuilder:
knitr
Config/Needs/development: testthat
Config/Needs/website: tidyverse/tidytemplate
Config/testthat/edition: 3
Config/testthat/parallel: TRUE
Expand Down
6 changes: 4 additions & 2 deletions R/namespace.R
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ update_namespace_imports <- function(base_path) {
}

lines <- c(namespace_imports(base_path), namespace_exports(NAMESPACE))
results <- c(made_by("#"), sort_c(unique(lines)))
results <- c(made_by("#"), sort_c(unique(trimws(lines))))
write_if_different(NAMESPACE, results, check = TRUE)

invisible()
Expand Down Expand Up @@ -109,12 +109,14 @@ namespace_imports_blocks <- function(srcref) {
}))
}

# NB: this is designed as the conjugate of namespace_imports(), so also
# includes @rawNamespace entries which may/may not also include import directives.
namespace_exports <- function(path) {
parsed <- as.list(parse(path, keep.source = TRUE))

is_import_directive <- function(x) is_call(x, import_directives)
export_lines <- attr(parsed, "srcref")[!map_lgl(parsed, is_import_directive)]
unlist(lapply(export_lines, as.character))
unlist(lapply(export_lines, function(x) paste(as.character(x), collapse = "\n")))
}

# NAMESPACE generation ----------------------------------------------------
Expand Down
26 changes: 25 additions & 1 deletion tests/testthat/test-namespace.R
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,24 @@ test_that("rawNamespace inserted unchanged", {
expect_equal(out, "xyz\n abc")
})

test_that("rawNamespace does not break idempotency", {
initial_namespace <- roc_proc_text(namespace_roclet(), "
#' @import methods
#' @rawNamespace
#' if (TRUE) {
#' 1
#' } else {
#' 2
#' }
NULL")

tmp_namespace <- withr::local_tempfile(lines = initial_namespace)

expect_no_error(update_namespace_imports(dirname(tmp_namespace)))

# contents unchanged
expect_equal(read_lines(tmp_namespace), unlist(strsplit(initial_namespace, "\n")))
})

# @evalNamespace ----------------------------------------------------------

Expand Down Expand Up @@ -391,7 +409,13 @@ test_that("can extract non-imports from namespace preserving source", {
"export(b)"
)
path <- withr::local_tempfile(lines = lines)
expect_equal(namespace_exports(path), lines[c(1:3, 5)])
expect_equal(
namespace_exports(path),
c(
paste(lines[1:3], collapse = "\n"),
lines[5L]
)
)
})

test_that("Invalid imports throw a helpful error", {
Expand Down
Loading