Fix idempotency of roxygenize() for packages using multi-line @rawNam…

…espace (#1573)

Fixes #1572

.gitignore

@@ -2,6 +2,8 @@
 .Rproj.user
 /.Rhistory
 
+tests/testthat/testthat-problems.rds
+
 ## Qt Creator
 *.pro
 *.pro.user

NEWS.md

@@ -1,6 +1,7 @@
 # roxygen2 (development version)
 
 * `@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.
   `@importFrom magrittr "%>%"` or ``@importFrom rlang `:=` ``, after being broken

R/namespace.R

@@ -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()
@@ -109,12 +109,15 @@ 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))
+  # Each multiline directives are a single element so they're sorted correctly
+  unlist(lapply(export_lines, function(x) paste(as.character(x), collapse = "\n")))
 }
 
 # NAMESPACE generation ----------------------------------------------------

tests/testthat/test-namespace.R

@@ -305,6 +305,17 @@ test_that("rawNamespace inserted unchanged", {
   expect_equal(out, "xyz\n  abc")
 })
 
+test_that("rawNamespace does not break idempotency", {
+  test_pkg <- test_path("testRawNamespace")
+  NAMESPACE <- file.path(test_pkg, "NAMESPACE")
+
+  lines_orig <- read_lines(NAMESPACE)
+
+  expect_no_error(roxygenize(test_pkg, namespace_roclet()))
+
+  # contents unchanged
+  expect_equal(read_lines(NAMESPACE), lines_orig)
+})
 
 # @evalNamespace ----------------------------------------------------------
 
@@ -391,7 +402,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 generate correct declarations", {

tests/testthat/testRawNamespace/DESCRIPTION

@@ -0,0 +1,9 @@
+Package: testRawNamespace
+Title: Check that re-running on multi-line @rawNamespace directive is OK
+License: GPL-2
+Description: testRawNamespace.
+Author: Hadley <[email protected]>
+Maintainer: Hadley <[email protected]>
+Encoding: UTF-8
+Version: 0.1
+RoxygenNote: 7.3.0.9000

tests/testthat/testRawNamespace/NAMESPACE

@@ -0,0 +1,10 @@
+# Generated by roxygen2: do not edit by hand
+
+
+if (TRUE) {
+  import(grDevices)
+} else {
+  import(methods)
+}
+import(graphics)
+import(utils)

tests/testthat/testRawNamespace/R/a.R

@@ -0,0 +1,9 @@
+#' @import graphics
+#' @rawNamespace
+#' if (TRUE) {
+#'   import(grDevices)
+#' } else {
+#'   import(methods)
+#' }
+#' @import utils
+NULL