Restore old behavior of @importFrom in some edge cases

NEWS.md

@@ -2,6 +2,10 @@
 
 * `@family` lists are now ordered more carefully, "foo1" comes after "foo" (#1563, @krlmlr).
 
+* `@importFrom magrittr "%>%"` works again (#1570, @MichaelChirico). Relatedly, `@importFrom`
+  directives not matching any known functions (e.g. `@importFrom utils plot pdf`) will not
+  produce invalid NAMESPACE files.
+
 # 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 <- !normalize_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$")
+normalize_quotes <- function(x) str_replace(x, "^(`|'|\")(.*)\\1$", "\\2")

tests/testthat/_snaps/namespace.md

@@ -83,14 +83,21 @@
     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: `InvalidUtilsFunction`.
 
 ---
 
     Code
       out <- roc_proc_text(namespace_roclet(), block)
     Message
-      x <text>:2: @importFrom Excluding unknown exports in from utils: `InvalidUtilsFunction1` and `InvalidUtilsFunction2`.
+      x <text>:2: @importFrom Excluding unknown export from utils: `InvalidUtilsFunction`.
+
+---
+
+    Code
+      out <- roc_proc_text(namespace_roclet(), block)
+    Message
+      x <text>:2: @importFrom Excluding unknown exports from utils: `InvalidUtilsFunction1` and `InvalidUtilsFunction2`.
 
 ---
 

tests/testthat/test-namespace.R

@@ -395,6 +395,15 @@ test_that("can extract non-imports from namespace preserving source", {
 })
 
 test_that("Invalid imports throw a helpful error", {
+  # No matched functions --> no output
+  block <- "
+    #' @importFrom utils InvalidUtilsFunction
+    NULL
+  "
+  expect_snapshot(out <- roc_proc_text(namespace_roclet(), block))
+  expect_equal(out, character())
+
+  # Matched functions --> only drop unmatched functions
   block <- "
     #' @importFrom utils head InvalidUtilsFunction
     NULL
@@ -418,6 +427,23 @@ test_that("Invalid imports throw a helpful error", {
   expect_snapshot(out <- roc_proc_text(namespace_roclet(), block))
   expect_equal(out, "importFrom(AnUnknownUnavailablePackage,Unchecked)")
 
+  # make sure to match several forms of non-syntactic names
+  expect_no_warning(expect_equal(
+    roc_proc_text(namespace_roclet(), "#' @importFrom stringr %>%\nNULL"),
+    'importFrom(stringr,"%>%")'
+  ))
+  expect_no_warning(expect_equal(
+    roc_proc_text(namespace_roclet(), "#' @importFrom stringr '%>%'\nNULL"),
+    "importFrom(stringr,'%>%')"
+  ))
+  expect_no_warning(expect_equal(
+    roc_proc_text(namespace_roclet(), "#' @importFrom stringr `%>%`\nNULL"),
+    "importFrom(stringr,`%>%`)"
+  ))
+  expect_no_warning(expect_equal(
+    roc_proc_text(namespace_roclet(), '#\' @importFrom stringr "%>%"\nNULL'),
+    'importFrom(stringr,"%>%")'
+  ))
 })