Restore old behavior of @importFrom in some edge cases (#1574)

Fixes #1570

Co-authored-by: Hadley Wickham <h.wickham@gmail.com>

NEWS.md

@@ -2,6 +2,12 @@
 
 * `@family` lists are now ordered more carefully, "foo1" comes after "foo" (#1563, @krlmlr).
 
+* `@importFrom` works again for quoted non-syntactic names, e.g.
+  `@importFrom magrittr "%>%"` or ``@importFrom rlang `:=` ``, after being broken
+  in 7.3.0 (#1570, @MichaelChirico). The unquoted form `@importFrom magrittr %>%`
+  continues to work. Relatedly, `@importFrom` directives not matching any known
+  functions (e.g. `@importFrom utils plot pdf`) produce valid NAMESPACE files again.
+
 # roxygen2 7.3.0
 
 ## New features

R/namespace.R

@@ -268,9 +268,13 @@ roxy_tag_ns.roxy_tag_importFrom <- function(x, block, env) {
   pkg <- x$val[1L]
   if (requireNamespace(pkg, quietly = TRUE)) {
     importing <- x$val[-1L]
-    unknown_idx <- !importing %in% getNamespaceExports(pkg)
+    # be sure to match '%>%', `%>%`, "%>%" all to %>% given by getNamespaceExports, #1570
+    unknown_idx <- !strip_quotes(importing) %in% getNamespaceExports(pkg)
     if (any(unknown_idx)) {
-      warn_roxy_tag(x, "Excluding unknown {cli::qty(sum(unknown_idx))} export{?s} in from {.package {pkg}}: {.code {importing[unknown_idx]}}")
+      warn_roxy_tag(x, "Excluding unknown {cli::qty(sum(unknown_idx))} export{?s} from {.package {pkg}}: {.code {importing[unknown_idx]}}")
+      if (all(unknown_idx)) {
+        return(NULL)
+      }
       x$val <- c(pkg, importing[!unknown_idx])
     }
   }

R/utils.R

@@ -179,3 +179,4 @@ auto_quote <- function(x) {
 
 is_syntactic <- function(x) make.names(x) == x
 has_quotes <- function(x) str_detect(x, "^(`|'|\").*\\1$")
+strip_quotes <- function(x) str_replace(x, "^(`|'|\")(.*)\\1$", "\\2")

tests/testthat/_snaps/namespace.md

@@ -78,24 +78,19 @@
     Message
       x <text>:2: @evalNamespace must evaluate to a character vector.
 
-# Invalid imports throw a helpful error
+# invalid imports generate helpful message
 
     Code
       out <- roc_proc_text(namespace_roclet(), block)
     Message
-      x <text>:2: @importFrom Excluding unknown export in from utils: `InvalidUtilsFunction`.
+      x <text>:2: @importFrom Excluding unknown export from utils: `InvalidUtilsFunction1`.
 
 ---
 
     Code
       out <- roc_proc_text(namespace_roclet(), block)
     Message
-      x <text>:2: @importFrom Excluding unknown exports in from utils: `InvalidUtilsFunction1` and `InvalidUtilsFunction2`.
-
----
-
-    Code
-      out <- roc_proc_text(namespace_roclet(), block)
+      x <text>:2: @importFrom Excluding unknown exports from utils: `InvalidUtilsFunction1` and `InvalidUtilsFunction2`.
 
 # warns if S3 method not documented
 

tests/testthat/test-namespace.R

@@ -394,32 +394,63 @@ test_that("can extract non-imports from namespace preserving source", {
   expect_equal(namespace_exports(path), lines[c(1:3, 5)])
 })
 
-test_that("Invalid imports throw a helpful error", {
+test_that("invalid imports generate correct declarations", {
+  # No matched functions --> no output
+  block <- "
+    #' @importFrom utils InvalidUtilsFunction
+    NULL
+  "
+  expect_message(out <- roc_proc_text(namespace_roclet(), block))
+  expect_equal(out, character())
+
+  # Matched functions --> only drop unmatched functions
   block <- "
     #' @importFrom utils head InvalidUtilsFunction
     NULL
   "
-  expect_snapshot(out <- roc_proc_text(namespace_roclet(), block))
+  expect_message(out <- roc_proc_text(namespace_roclet(), block))
   expect_equal(out, "importFrom(utils,head)")
+})
+
+test_that("invalid imports generate helpful message", {
+  block <- "
+    #' @importFrom utils head InvalidUtilsFunction1
+    NULL
+  "
+  expect_snapshot(out <- roc_proc_text(namespace_roclet(), block))
 
-  # pluralization
   block <- "
     #' @importFrom utils head InvalidUtilsFunction1 InvalidUtilsFunction2
     NULL
   "
   expect_snapshot(out <- roc_proc_text(namespace_roclet(), block))
-  expect_equal(out, "importFrom(utils,head)")
+})
 
-  # If the package is not available at roxygenize() run time, nothing we can do
+test_that("nothing we can do if package isn't installed", {
   block <- "
     #' @importFrom AnUnknownUnavailablePackage Unchecked
     NULL
   "
-  expect_snapshot(out <- roc_proc_text(namespace_roclet(), block))
+  expect_no_message(out <- roc_proc_text(namespace_roclet(), block))
   expect_equal(out, "importFrom(AnUnknownUnavailablePackage,Unchecked)")
-
 })
 
+test_that("non-syntactic imports can use multiple quoting forms", {
+  lines <- c(
+    "#' @importFrom stringr %>%",
+    "#' @importFrom stringr `%>%`",
+    "#' @importFrom stringr '%>%'",
+    "#' @importFrom stringr \"%>%\"",
+    "NULL"
+  )
+
+  import <- expect_no_warning(roc_proc_text(namespace_roclet(), lines))
+  expect_equal(import, c(
+    "importFrom(stringr,\"%>%\")",
+    "importFrom(stringr,'%>%')",
+    "importFrom(stringr,`%>%`)"
+  ))
+})
 
 # warn_missing_s3_exports -------------------------------------------------