fully deprecate positional arguments in lint()+friends (#1992)

NEWS.md

@@ -8,6 +8,7 @@
 * `consecutive_stopifnot_linter()` is deprecated in favor of the more general (see below) `consecutive_assertion_linter()` (#1604, @MichaelChirico).
 * `no_tab_linter()` is deprecated in favor of `whitespace_linter()` for naming consistency and future generalization (#1954, @MichaelChirico).
 * `available_linters()` prioritizes `tags` over `exclude_tags` in the case of overlap, i.e., tags listed in both arguments are included, not excluded. We don't expect many people to be affected by this, and the old behavior was not made explicit in the documentation, but make note of it here since it required changing a test in lintr's own suite where `linters_with_tags()` implicitly assumed this behavior.
+* `lint()`, `lint_dir()`, and `lint_package()` no longer accept certain arguments (`cache=` for `lint()`, `relative_path=` for the latter two) positionally. The `warning()` since 3.0.0 has been upgraded to an error.
 
 ## Bug fixes
 

R/lint.R

@@ -35,15 +35,8 @@
 #'
 #' @export
 lint <- function(filename, linters = NULL, ..., cache = FALSE, parse_settings = TRUE, text = NULL) {
-  # TODO(next release after 3.0.0): remove this deprecated workaround
-  dots <- list(...)
-  if (has_positional_logical(dots)) {
-    warning(
-      "'cache' is no longer available as a positional argument; please supply 'cache' as a named argument instead. ",
-      "This warning will be upgraded to an error in the next release."
-    )
-    cache <- dots[[1L]]
-    dots <- dots[-1L]
+  if (has_positional_logical(list(...))) {
+    stop("'cache' is no longer available as a positional argument; please supply 'cache' as a named argument instead.")
   }
 
   needs_tempfile <- missing(filename) || rex::re_matches(filename, rex::rex(newline))
@@ -75,9 +68,7 @@ lint <- function(filename, linters = NULL, ..., cache = FALSE, parse_settings =
   lint_obj <- define_cache_key(filename, inline_data, lines)
   lints <- retrieve_file(lint_cache, lint_obj, linters)
   if (!is.null(lints)) {
-    # TODO: once cache= is fully deprecated as 3rd positional argument (see top of body), we can restore the cleaner:
-    # > exclude(lints, lines = lines, ...)
-    return(do.call(exclude, c(list(lints, lines = lines, linter_names = names(linters)), dots)))
+    return(exclude(lints, lines = lines, linter_names = names(linters), ...))
   }
 
   lints <- list()
@@ -102,9 +93,7 @@ lint <- function(filename, linters = NULL, ..., cache = FALSE, parse_settings =
   cache_file(lint_cache, filename, linters, lints)
   save_cache(lint_cache, filename, cache_path)
 
-  # TODO: once cache= is fully deprecated as 3rd positional argument (see top of body), we can restore the cleaner:
-  # > exclude(lints, lines = lines, ...)
-  res <- do.call(exclude, c(list(lints, lines = lines, linter_names = names(linters)), dots))
+  res <- exclude(lints, lines = lines, linter_names = names(linters), ...)
 
   # simplify filename if inline
   zap_temp_filename(res, needs_tempfile)
@@ -138,16 +127,11 @@ lint_dir <- function(path = ".", ...,
                      exclusions = list("renv", "packrat"),
                      pattern = rex::rex(".", one_of("Rr"), or("", "html", "md", "nw", "rst", "tex", "txt"), end),
                      parse_settings = TRUE) {
-  # TODO(next release after 3.0.0): remove this deprecated workaround
-  dots <- list(...)
-  if (has_positional_logical(dots)) {
-    warning(
+  if (has_positional_logical(list(...))) {
+    stop(
       "'relative_path' is no longer available as a positional argument; ",
-      "please supply 'relative_path' as a named argument instead. ",
-      "This warning will be upgraded to an error in the next release."
+      "please supply 'relative_path' as a named argument instead. "
     )
-    relative_path <- dots[[1L]]
-    dots <- dots[-1L]
   }
 
   if (isTRUE(parse_settings)) {
@@ -179,9 +163,7 @@ lint_dir <- function(path = ".", ...,
     files,
     function(file) {
       maybe_report_progress()
-      # TODO: once relative_path= is fully deprecated as 2nd positional argument (see top of body), restore the cleaner:
-      # > lint(file, ..., parse_settings = FALSE, exclusions = exclusions)
-      do.call(lint, c(list(file, parse_settings = FALSE, exclusions = exclusions), dots))
+      lint(file, ..., parse_settings = FALSE, exclusions = exclusions)
     }
   ))
 
@@ -230,16 +212,11 @@ lint_package <- function(path = ".", ...,
                          relative_path = TRUE,
                          exclusions = list("R/RcppExports.R"),
                          parse_settings = TRUE) {
-  # TODO(next release after 3.0.0): remove this deprecated workaround
-  dots <- list(...)
-  if (has_positional_logical(dots)) {
-    warning(
+  if (has_positional_logical(list(...))) {
+    stop(
       "'relative_path' is no longer available as a positional argument; ",
-      "please supply 'relative_path' as a named argument instead. ",
-      "This warning will be upgraded to an error in the next release."
+      "please supply 'relative_path' as a named argument instead. "
     )
-    relative_path <- dots[[1L]]
-    dots <- dots[-1L]
   }
 
   if (length(path) > 1L) {
@@ -263,12 +240,7 @@ lint_package <- function(path = ".", ...,
   )
 
   r_directories <- file.path(pkg_path, c("R", "tests", "inst", "vignettes", "data-raw", "demo", "exec"))
-  # TODO: once relative_path= is fully deprecated as 2nd positional argument (see top of body), restore the cleaner:
-  # > lints <- lint_dir(r_directories, relative_path = FALSE, exclusions = exclusions, parse_settings = FALSE, ...)
-  lints <- do.call(
-    lint_dir,
-    c(list(r_directories, relative_path = FALSE, exclusions = exclusions, parse_settings = FALSE), dots)
-  )
+  lints <- lint_dir(r_directories, relative_path = FALSE, exclusions = exclusions, parse_settings = FALSE, ...)
 
   if (isTRUE(relative_path)) {
     path <- normalizePath(pkg_path, mustWork = FALSE)

tests/testthat/test-dir_linters.R

@@ -96,12 +96,9 @@ test_that("lint_dir works with specific linters without specifying other argumen
 test_that("lint_dir continues to accept relative_path= in 2nd positional argument, with a warning", {
   the_dir <- test_path("dummy_packages", "package", "vignettes")
 
-  expect_warning(
-    {
-      positional_lints <- lint_dir(the_dir, FALSE)
-    },
+  expect_error(
+    lint_dir(the_dir, FALSE),
     "'relative_path' is no longer available as a positional argument",
     fixed = TRUE
   )
-  expect_identical(positional_lints, lint_dir(the_dir, relative_path = FALSE))
 })

tests/testthat/test-lint_file.R

@@ -220,14 +220,11 @@ test_that("compatibility warnings work", {
 })
 
 test_that("Deprecated positional usage of cache= works, with warning", {
-  expect_warning(
-    {
-      l <- lint("a = 2\n", FALSE, linters = assignment_linter())
-    },
+  expect_error(
+    lint("a = 2\n", FALSE, linters = assignment_linter()),
     "'cache' is no longer available as a positional argument",
     fixed = TRUE
   )
-  expect_identical(l, lint("a = 2\n", assignment_linter(), cache = FALSE))
 })
 
 test_that("Linters throwing an error give a helpful error", {