Use ellipsis for all functions taking dots

Fixes #2016
r-lib · Aug 29, 2019 · 0047282 · jennybc · Aug 30, 2019 · 0047282
1 parent 1d18d89
commit 0047282
Show file tree

Hide file tree

Showing 19 changed files with 229 additions and 30 deletions.
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -20,6 +20,7 @@ Imports:
     callr,
     cli,
     digest,
+    ellipsis,
     git2r (>= 0.23.0),
     httr (>= 0.4),
     jsonlite,

diff --git a/NAMESPACE b/NAMESPACE
@@ -24,8 +24,8 @@ export(clean_dll)
 export(clean_vignettes)
 export(create)
 export(dev_mode)
-export(dev_package_deps)
 export(dev_packages)
+export(dev_packge_deps)
 export(devtest)
 export(document)
 export(dr_devtools)
@@ -85,6 +85,7 @@ export(with_debug)
 importFrom(cli,cat_bullet)
 importFrom(cli,cat_line)
 importFrom(cli,cat_rule)
+importFrom(ellipsis,check_dots_used)
 importFrom(memoise,memoise)
 importFrom(pkgbuild,clean_dll)
 importFrom(pkgbuild,find_rtools)
@@ -112,6 +113,7 @@ importFrom(remotes,install_version)
 importFrom(remotes,update_packages)
 importFrom(sessioninfo,package_info)
 importFrom(sessioninfo,session_info)
+importFrom(stats,update)
 importFrom(usethis,create_package)
 importFrom(usethis,use_test)
 importFrom(utils,available.packages)

diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,8 @@
 # devtools (development version)
 
+* All functions taking `...` now use the ellipsis package. This catches errors
+  when arguments are misspelled or incorrectly specified (#2016)
+
 * `build_vignettes()` now correctly installs the vignette builder if it is not
   already installed (#2089).
 

diff --git a/R/build-readme.R b/R/build-readme.R
@@ -36,6 +36,8 @@ build_readme <- function(path = ".", quiet = TRUE, ...) {
 
   message("Building ", pkg$package, " readme")
 
+  check_dots_used()
+
   output <- callr::r(build,
     args = list(pkg_path = pkg$path, readme_path = readme_path, ... = ..., quiet = quiet),
     show = TRUE, spinner = FALSE

diff --git a/R/build-site.R b/R/build-site.R
@@ -16,6 +16,8 @@ build_site <- function(path = ".", quiet = TRUE, ...) {
 
   pkg <- as.package(path)
 
+  check_dots_used()
+
   withr::with_temp_libpaths(action = "prefix", code = {
     install(pkg = pkg$path, upgrade = "never", reload = FALSE, quiet = quiet)
     if (isTRUE(quiet)) {

diff --git a/R/check-win.R b/R/check-win.R
@@ -19,6 +19,8 @@ NULL
 #' @describeIn check_win Check package on the development version of R.
 #' @export
 check_win_devel <- function(pkg = ".", args = NULL, manual = TRUE, quiet = FALSE, ...) {
+  check_dots_used()
+
   check_win(
     pkg = pkg, version = "R-devel", args = args, manual = manual,
     quiet = quiet, ...
@@ -28,6 +30,8 @@ check_win_devel <- function(pkg = ".", args = NULL, manual = TRUE, quiet = FALSE
 #' @describeIn check_win Check package on the release version of R.
 #' @export
 check_win_release <- function(pkg = ".", args = NULL, manual = TRUE, quiet = FALSE, ...) {
+  check_dots_used()
+
   check_win(
     pkg = pkg, version = "R-release", args = args, manual = manual,
     quiet = quiet, ...
@@ -37,6 +41,8 @@ check_win_release <- function(pkg = ".", args = NULL, manual = TRUE, quiet = FAL
 #' @describeIn check_win Check package on the previous major release version of R.
 #' @export
 check_win_oldrelease <- function(pkg = ".", args = NULL, manual = TRUE, quiet = FALSE, ...) {
+  check_dots_used()
+
   check_win(
     pkg = pkg, version = "R-oldrelease", args = args, manual = manual,
     quiet = quiet, ...

diff --git a/R/check.R b/R/check.R
@@ -99,6 +99,8 @@ check <- function(pkg = ".",
     show_env_vars(pkgbuild::compiler_flags(FALSE))
   }
 
+  check_dots_used()
+
   withr::with_envvar(pkgbuild::compiler_flags(FALSE), action = "prefix", {
     built_path <- pkgbuild::build(
       pkg$path,

diff --git a/R/install.R b/R/install.R
@@ -76,6 +76,8 @@ install <-
     }
     opts <- c(opts, args)
 
+    check_dots_used()
+
     remotes::install_deps(pkg$path,
       build = build, build_opts = build_opts,
       INSTALL_opts = opts, dependencies = dependencies, quiet = quiet,
@@ -117,6 +119,8 @@ install_deps <- function(pkg = ".",
                          ...) {
   pkg <- as.package(pkg)
 
+  check_dots_used()
+
   remotes::install_deps(pkg$path,
     dependencies = dependencies,
     repos = repos,
@@ -144,6 +148,8 @@ install_dev_deps <- function(pkg = ".",
 
   pkg <- as.package(pkg)
 
+  check_dots_used()
+
   remotes::install_deps(pkg$path, ...,
     dependencies = TRUE, upgrade = upgrade
   )

diff --git a/R/lint.R b/R/lint.R
@@ -19,5 +19,8 @@ lint <- function(pkg = ".", cache = TRUE, ...) {
   pkg <- as.package(pkg)
 
   message("Linting ", pkg$package, appendLF = FALSE)
+
+  check_dots_used()
+
   lintr::lint_package(pkg$path, cache = cache, ...)
 }
diff --git a/R/pkgbuild.R b/R/pkgbuild.R
@@ -7,6 +7,9 @@
 build <- function(pkg = ".", path = NULL, binary = FALSE, vignettes = TRUE,
                   manual = FALSE, args = NULL, quiet = FALSE, ...) {
   save_all()
+
+  check_dots_used()
+
   pkgbuild::build(
     path = pkg, dest_path = path, binary = binary,
     vignettes = vignettes, manual = manual, args = args, quiet = quiet, ...

diff --git a/R/pkgload.R b/R/pkgload.R
@@ -3,13 +3,14 @@
 #' @export
 load_all <- function(path = ".", reset = TRUE, recompile = FALSE,
                      export_all = TRUE, helpers = TRUE, quiet = FALSE, ...) {
-
   if (inherits(path, "package")) {
     path <- path$path
   }
 
   save_all()
 
+  check_dots_used()
+
   pkgload::load_all(
     path = path, reset = reset, recompile = recompile,
     export_all = export_all, helpers = helpers, quiet = quiet, ...

diff --git a/R/r-hub.R b/R/r-hub.R
@@ -43,6 +43,8 @@ check_rhub <- function(pkg = ".",
                       args = build_args)
   on.exit(unlink(built_path), add = TRUE)
 
+  check_dots_used()
+
   status <- rhub::check_for_cran(
     path = built_path,
     email = email,

diff --git a/R/remotes.R b/R/remotes.R
@@ -1,59 +1,98 @@
+#' @importFrom ellipsis check_dots_used
+with_ellipsis <- function(fun) {
+  b <- body(fun)
+  ellipsis_call <- as.call(c(call("::", as.symbol("ellipsis"), as.symbol("check_dots_used")), list()))
+
+  b <- as.call(c(as.symbol("{"), ellipsis_call, as.list(b[-1])))
+  body(fun) <- b
+  fun
+}
+
+#' Functions re-exported from the remotes package
+#'
+
+#' These functions are re-exported from the remotes package. They differ only
+#' that the ones in devtools use the [ellipsis] package to ensure all dotted
+#' arguments are used.
+#'
+#' Follow the links below to see the documentation.
+#' [install_bioc()], [install_bitbucket()], [install_cran()], [install_dev()],
+#' [install_git()], [install_github()], [install_gitlab()], [install_local()],
+#' [install_svn()], [install_url()], [install_version()], [update_packages()],
+#' [dev_package_deps()].
+#'
 #' @importFrom remotes install_bioc
+#' @name remote-reexports
+#' @keywords internal
 #' @export
-remotes::install_bioc
+install_bioc <- with_ellipsis(remotes::install_bioc)
 
 #' @importFrom remotes install_bitbucket
+#' @rdname remote-reexports
 #' @export
-remotes::install_bitbucket
+install_bitbucket <- with_ellipsis(remotes::install_bitbucket)
 
 #' @importFrom remotes install_cran
+#' @rdname remote-reexports
 #' @export
-remotes::install_cran
+install_cran <- with_ellipsis(remotes::install_cran)
 
 #' @importFrom remotes install_dev
+#' @rdname remote-reexports
 #' @export
-remotes::install_dev
+install_dev <- with_ellipsis(remotes::install_dev)
 
 #' @importFrom remotes install_git
+#' @rdname remote-reexports
 #' @export
-remotes::install_git
+install_git <- with_ellipsis(remotes::install_git)
 
 #' @importFrom remotes install_github
+#' @rdname remote-reexports
 #' @export
-remotes::install_github
+install_github <- with_ellipsis(remotes::install_github)
 
 #' @importFrom remotes github_pull
+#' @rdname reexports
 #' @export
 remotes::github_pull
 
 #' @importFrom remotes github_release
+#' @rdname reexports
 #' @export
 remotes::github_release
 
 #' @importFrom remotes install_gitlab
+#' @rdname remote-reexports
 #' @export
-remotes::install_gitlab
+install_gitlab <- with_ellipsis(remotes::install_gitlab)
 
 #' @importFrom remotes install_local
+#' @rdname remote-reexports
 #' @export
-remotes::install_local
+install_local <- with_ellipsis(remotes::install_local)
 
 #' @importFrom remotes install_svn
+#' @rdname remote-reexports
 #' @export
-remotes::install_svn
+install_svn <- with_ellipsis(remotes::install_svn)
 
 #' @importFrom remotes install_url
+#' @rdname remote-reexports
 #' @export
-remotes::install_url
+install_url <- with_ellipsis(remotes::install_url)
 
 #' @importFrom remotes install_version
+#' @rdname remote-reexports
 #' @export
-remotes::install_version
+install_version <- with_ellipsis(remotes::install_version)
 
 #' @importFrom remotes update_packages
+#' @rdname remote-reexports
 #' @export
-remotes::update_packages
+update_packages <- with_ellipsis(remotes::update_packages)
 
 #' @importFrom remotes dev_package_deps
+#' @rdname remote-reexports
 #' @export
-remotes::dev_package_deps
+dev_packge_deps <- with_ellipsis(remotes::dev_package_deps)
diff --git a/R/run-source.R b/R/run-source.R
@@ -58,6 +58,8 @@ source_url <- function(url, ..., sha1 = NULL) {
     }
   }
 
+  check_dots_used()
+
   source(temp_file, ...)
 }
 
@@ -119,6 +121,9 @@ source_gist <- function(id, ..., filename = NULL, sha1 = NULL, quiet = FALSE) {
   }
 
   if (!quiet) message("Sourcing ", url)
+
+  check_dots_used()
+
   source_url(url, ..., sha1 = sha1)
 }
 

diff --git a/R/show-news.R b/R/show-news.R
@@ -14,6 +14,8 @@ show_news <- function(pkg = ".", latest = TRUE, ...) {
     stop("No NEWS found", call. = FALSE)
   }
 
+  check_dots_used()
+
   out <- utils::news(..., db = ("tools" %:::% ".news_reader_default")(news_path))
   if (latest) {
     ver <- numeric_version(out$Version)

diff --git a/R/test.R b/R/test.R
@@ -66,6 +66,8 @@ test <- function(pkg = ".", filter = NULL, ...) {
     testthat_args <- c(testthat_args, load_helpers = FALSE)
   }
 
+  check_dots_used()
+
   withr::with_options(
     c(useFancyQuotes = FALSE),
     withr::with_envvar(
@@ -87,6 +89,8 @@ test_coverage <- function(pkg = ".", show_report = interactive(), ...) {
 
   save_all()
 
+  check_dots_used()
+
   withr::with_envvar(
     c(r_env_vars(),
       "TESTTHAT_PKG" = pkg$package
@@ -151,7 +155,6 @@ src_ext <- c("c", "cc", "cpp", "cxx", "h", "hpp", "hxx")
 #' @export
 #' @rdname test
 test_file <- function(file = find_active_file(), ...) {
-
   ext <- tolower(tools::file_ext(file))
   valid_files <- ext %in% c("r", src_ext)
   if (any(!valid_files)) {
@@ -174,6 +177,8 @@ test_file <- function(file = find_active_file(), ...) {
 
   regex <- paste0("^", escape_special_regex(test_files), "$", collapse = "|")
 
+  check_dots_used()
+
   test(filter = regex, ...)
 }
 
@@ -247,6 +252,8 @@ test_coverage_file <- function(file = find_active_file(), filter = TRUE, show_re
 
   env <- load_all(pkg$path, quiet = TRUE)$env
 
+  check_dots_used()
+
   withr::with_envvar(
     c(r_env_vars(),
       "TESTTHAT_PKG" = pkg$package

diff --git a/R/vignettes.R b/R/vignettes.R
@@ -22,6 +22,7 @@
 #'   <https://bookdown.org/yihui/rmarkdown/html-document.html#keeping-markdown>.
 #' @inheritParams tools::buildVignettes
 #' @inheritParams remotes::install_deps
+#' @importFrom stats update
 #' @keywords programming
 #' @seealso [clean_vignettes()] to remove the pdfs in
 #'   \file{doc} created from vignettes

diff --git a/man/reexports.Rd b/man/reexports.Rd