Use use_import_from() in other importing functions (r-lib#1396)

* add `use_import_from()` to `use_pipe()`

* remove test re: advice since use_import_from requires pkg doc

* add `use_import_from()` to `use_tibble()` and update test

* add `use_import_from()` to `use_data_table()`, remove template, and update tests

* update `use_data_table()` docs

* add a more informative message when no pkg doc

* simplify sentence in data table doc

* tell user about data table dependency action

* check installed before checking fun exists

* update tests + test `use_data_table()` dependency action

* nvm, this is done in `check_has_package_doc()`

* just don't test when data.table not installed

* there are so many places where the package needs to be checked, better to do so immediately

* pacify tibble test

* simplify data.table tests

* mock use_data_table() tests to get around ns loading

* mock tibble tests, too, just in case

* remove dead code RIP

R/data-table.R

@@ -2,16 +2,15 @@
 #'
 #' @description
 #'
-#' `use_data_table` facilitates importing the data.table package by handling
-#' up-front some common set-up tasks for using it in your package.
-#'
-#' This function does two main things:
-#'
-#' 1. Import the entire data.table namespace (with `@import`).
-#'
-#' 2. Block the usage of data.table as a dependency (`DESCRIPTION` field
-#' `Depends`); `data.table` should be used as an _imported_ or _suggested_
-#' package only. See this
+#' `use_data_table()` imports the `data.table()` function from the data.table
+#' package, as well as several important symbols: `:=`, `.SD`, `.BY`, `.N`,
+#' `.I`, `.GRP`, `.NGRP`, `.EACHI`. This is a minimal setup to get `data.table`s
+#' working with your package. See the [importing
+#' data.table](https://rdatatable.gitlab.io/data.table/articles/datatable-importing.html)
+#' vignette for other strategies. In addition to importing these function,
+#' `use_data_table()` also blocks the usage of data.table in the `Depends` field
+#' of the `DESCRIPTION` file; `data.table` should be used as an _imported_ or
+#' _suggested_ package only. See this
 #' [discussion](https://github.com/Rdatatable/data.table/issues/3076).
 #' @export
 use_data_table <- function() {
@@ -21,10 +20,12 @@ use_data_table <- function() {
   deps <- desc::desc_get_deps(".")
   if (any(deps$type == "Depends" & deps$package == "data.table")) {
     ui_warn("data.table should be in Imports or Suggests, not Depends")
+    ui_done("Deleting data.table from {ui_field('Depends')}")
     desc::desc_del_dep("data.table", "Depends", file = proj_get())
   }
-  use_dependency("data.table", "Imports")
-  new <- use_template("data-table.R", "R/utils-data-table.R")
-  ui_todo("Run {ui_code('devtools::document()')}")
-  invisible(new)
+
+  use_import_from(
+    "data.table",
+    c("data.table", ":=", ".SD", ".BY", ".N", ".I", ".GRP", ".NGRP", ".EACHI")
+  )
 }

R/pipe.R

@@ -22,26 +22,11 @@ use_pipe <- function(export = TRUE) {
   check_is_package("use_pipe()")
   check_uses_roxygen("use_pipe()")
 
-  use_dependency("magrittr", "Imports")
-
   if (export) {
+    use_dependency("magrittr", "Imports")
     use_template("pipe.R", "R/utils-pipe.R") && roxygen_remind()
     return(invisible(TRUE))
   }
 
-  if (has_package_doc()) {
-    roxygen_ns_append("@importFrom magrittr %>%") && roxygen_remind()
-    return(invisible(TRUE))
-  }
-
-  ui_todo(
-    "Copy and paste this line into some roxygen header, then run \\
-    {ui_code('devtools::document()')}:"
-  )
-  ui_code_block("#' @importFrom magrittr %>%", copy = FALSE)
-  ui_todo(
-    "Alternative recommendation: call {ui_code('use_package_doc()')}, then \\
-    call {ui_code('use_pipe()')} again."
-  )
-  invisible(FALSE)
+  use_import_from("magrittr", "%>%")
 }

R/tibble.R

@@ -31,9 +31,10 @@ use_tibble <- function() {
   check_is_package("use_tibble()")
   check_uses_roxygen("use_tibble()")
 
-  use_dependency("tibble", "Imports")
-  roxygen_ns_append("@importFrom tibble tibble") && roxygen_remind()
+  created <- use_import_from("tibble", "tibble")
 
   ui_todo("Document a returned tibble like so:")
   ui_code_block("#' @return a [tibble][tibble::tibble-package]", copy = FALSE)
+
+  invisible(created)
 }

R/use_import_from.R

@@ -27,9 +27,9 @@ use_import_from <- function(package, fun, load = is_interactive()) {
   }
   check_is_package("use_import_from()")
   check_uses_roxygen("use_import_from()")
+  check_installed(package)
   check_has_package_doc("use_import_from()")
-
-  purrr::walk2(package, fun, check_fun_exists)
+  check_functions_exist(package, fun)
 
   use_dependency(package, "Imports")
   changed <- roxygen_ns_append(glue("@importFrom {package} {fun}"))
@@ -41,6 +41,10 @@ use_import_from <- function(package, fun, load = is_interactive()) {
   invisible(changed)
 }
 
+check_functions_exist <- function(package, fun) {
+  purrr::walk2(package, fun, check_fun_exists)
+}
+
 check_fun_exists <- function(package, fun) {
   if (exists(fun, envir = asNamespace(package))) {
     return()

tests/testthat/_snaps/data-table.md

@@ -0,0 +1,30 @@
+# use_data_table() Imports data.table
+
+    Code
+      roxygen_ns_show()
+    Output
+      [1] "#' @importFrom data.table .BY"       
+      [2] "#' @importFrom data.table .EACHI"    
+      [3] "#' @importFrom data.table .GRP"      
+      [4] "#' @importFrom data.table .I"        
+      [5] "#' @importFrom data.table .N"        
+      [6] "#' @importFrom data.table .NGRP"     
+      [7] "#' @importFrom data.table .SD"       
+      [8] "#' @importFrom data.table :="        
+      [9] "#' @importFrom data.table data.table"
+
+# use_data_table() blocks use of Depends
+
+    Code
+      roxygen_ns_show()
+    Output
+      [1] "#' @importFrom data.table .BY"       
+      [2] "#' @importFrom data.table .EACHI"    
+      [3] "#' @importFrom data.table .GRP"      
+      [4] "#' @importFrom data.table .I"        
+      [5] "#' @importFrom data.table .N"        
+      [6] "#' @importFrom data.table .NGRP"     
+      [7] "#' @importFrom data.table .SD"       
+      [8] "#' @importFrom data.table :="        
+      [9] "#' @importFrom data.table data.table"
+

tests/testthat/_snaps/tibble.md

@@ -4,11 +4,7 @@
       use_tibble()
     Message <message>
       v Adding 'tibble' to Imports field in DESCRIPTION
-      * Copy and paste the following lines into 'R/mypackage-package.R':
-        ## usethis namespace: start
-        #' @importFrom tibble tibble
-        ## usethis namespace: end
-        NULL
+      v Adding '@importFrom tibble tibble' to 'R/mypackage-package.R'
       * Document a returned tibble like so:
         #' @return a [tibble][tibble::tibble-package]
 

tests/testthat/test-data-table.R

@@ -5,11 +5,32 @@ test_that("use_data_table() requires a package", {
 
 test_that("use_data_table() Imports data.table", {
   create_local_package()
+  use_package_doc()
   with_mock(
     check_installed = function(pkg) TRUE,
+    roxygen_update_ns = function(...) NULL,
+    check_functions_exist = function(...) TRUE,
     use_data_table()
   )
+
+  expect_match(desc::desc_get("Imports"), "data.table")
+  expect_snapshot(roxygen_ns_show())
+})
+
+test_that("use_data_table() blocks use of Depends", {
+  create_local_package()
+  use_package_doc()
+  desc::desc_set_dep("data.table", "Depends")
+  with_mock(
+    check_installed = function(pkg) TRUE,
+    roxygen_update_ns = function(...) NULL,
+    check_functions_exist = function(...) TRUE,
+    expect_warning(
+      use_data_table(),
+      "data.table should be in Imports or Suggests, not Depends"
+    )
+  )
+
   expect_match(desc::desc_get("Imports"), "data.table")
-  datatable_doc <- read_utf8(proj_path("R", "utils-data-table.R"))
-  expect_match(datatable_doc, "#' @import data.table", all = FALSE)
+  expect_snapshot(roxygen_ns_show())
 })

tests/testthat/test-pipe.R

@@ -19,9 +19,3 @@ test_that("use_pipe(export = FALSE) adds roxygen to package doc", {
   expect_match(package_doc, "#' @importFrom magrittr %>%", all = FALSE)
 })
 
-test_that("use_pipe(export = FALSE) gives advice if no package doc", {
-  create_local_package()
-  withr::local_options(list(usethis.quiet = FALSE))
-  expect_snapshot(use_pipe(export = FALSE))
-  expect_match(desc::desc_get("Imports", proj_get()), "magrittr")
-})

tests/testthat/test-tibble.R

@@ -6,6 +6,14 @@ test_that("use_tibble() requires a package", {
 test_that("use_tibble() Imports tibble", {
   create_local_package(path_temp("mypackage"))
   withr::local_options(list(usethis.quiet = FALSE))
-  expect_snapshot(use_tibble())
+  ui_silence(use_package_doc())
+
+  with_mock(
+    check_installed = function(pkg) TRUE,
+    roxygen_update_ns = function(...) NULL,
+    check_functions_exist = function(...) TRUE,
+    expect_snapshot(use_tibble())
+  )
+
   expect_match(desc::desc_get("Imports", proj_get()), "tibble")
 })