Catch extraneous ... arguments to lint()/lint_dir() early

.lintr_new

@@ -1,7 +1,7 @@
 linters: linters_with_defaults(
     any_duplicated_linter(),
     any_is_na_linter(),
-    backport_linter("oldrel-4", except = c("R_user_dir", "str2lang", "str2expression", "deparse1")),
+    backport_linter("oldrel-4", except = c("R_user_dir", "str2lang", "str2expression", "deparse1", "...names")),
     consecutive_assertion_linter(),
     expect_comparison_linter(),
     expect_identical_linter(),

NEWS.md

@@ -14,6 +14,7 @@
 * New `xp_call_name()` helper to facilitate writing custom linters (#2023, @MichaelChirico). This helper converts a matched XPath to the R function to which it corresponds. This is useful for including the "offending" function in the lint's message.
 * New `make_linter_from_xpath()` to facilitate making simple linters directly from a single XPath (#2064, @MichaelChirico). This is especially helpful for making on-the-fly/exploratory linters, but also extends to any case where the linter can be fully defined from a static lint message and single XPath.
 * Toggle lint progress indicators with argument `show_progress` to `lint_dir()` and `lint_package()` (#972, @MichaelChirico). The default is still to show progress in `interactive()` sessions. Progress is also now shown with a "proper" progress bar (`utils::txtProgressBar()`), which in particular solves the issue of progress `.` spilling well past the width of the screen in large directories.
+* `lint()`, `lint_dir()`, and `lint_package()` fail more gracefully when the user mis-spells an argument name (#2134, @MichaelChirico).
 * `fixed_regex_linter()`
   + Is pipe-aware, in particular removing false positives arong piping into {stringr} functions like `x |> str_replace(fixed("a"), "b")` (#1811, @MichaelChirico).
   + Gains an option `allow_unescaped` (default `FALSE`) to toggle linting regexes not requiring any escapes or character classes (#1689, @MichaelChirico). Thus `fixed_regex_linter(allow_unescaped = TRUE)` would lint on `grepl("[$]", x)` but not on `grepl("a", x)` since the latter does not use any regex special characters.

R/lint.R

@@ -39,6 +39,8 @@ lint <- function(filename, linters = NULL, ..., cache = FALSE, parse_settings =
     stop("'cache' is no longer available as a positional argument; please supply 'cache' as a named argument instead.")
   }
 
+  check_dots(...names(), c("exclude", "parse_exclusions"))
+
   needs_tempfile <- missing(filename) || re_matches(filename, rex(newline))
   inline_data <- !is.null(text) || needs_tempfile
   lines <- get_lines(filename, text)
@@ -138,6 +140,8 @@ lint_dir <- function(path = ".", ...,
     )
   }
 
+  check_dots(...names(), c("lint", "exclude", "parse_exclusions"))
+
   if (isTRUE(parse_settings)) {
     read_settings(path)
     on.exit(clear_settings, add = TRUE)

R/utils.R

@@ -265,3 +265,22 @@ is_linter <- function(x) inherits(x, "linter")
 is_tainted <- function(lines) {
   inherits(tryCatch(nchar(lines), error = identity), "error")
 }
+
+#' Check that the entries in ... are valid
+#'
+#' @param dot_names Supplied names, from [...names()].
+#' @param ref_calls Functions consuming these `...` (character).
+#' @param ref_help Help page to refer users hitting an error to.
+#' @noRd
+check_dots <- function(dot_names, ref_calls, ref_help = as.character(sys.call(-1L)[[1L]])) {
+  valid_args <- unlist(lapply(ref_calls, function(f) names(formals(f))))
+  is_valid <- dot_names %in% valid_args
+  if (all(is_valid)) {
+    return(invisible())
+  }
+  stop(
+    "Found unknown arguments in ...: ", toString(dot_names[!is_valid]), ".\n",
+    "Check for typos and see ?", ref_help, " for valid arguments.",
+    call. = FALSE
+  )
+}

R/zzz.R

@@ -284,7 +284,7 @@ settings <- NULL
   toset <- !(names(op_lintr) %in% names(op))
   if (any(toset)) options(op_lintr[toset])
 
-  backports::import(pkgname, c("trimws", "lengths", "deparse1"))
+  backports::import(pkgname, c("trimws", "lengths", "deparse1", "...names"))
   # requires R>=3.6.0; see https://github.com/r-lib/backports/issues/68
   base_ns <- getNamespace("base")
   backports_ns <- getNamespace("backports")

tests/testthat/test-lint.R

@@ -219,7 +219,7 @@ test_that("compatibility warnings work", {
   )
 })
 
-test_that("Deprecated positional usage of cache= works, with warning", {
+test_that("Deprecated positional usage of cache= errors", {
   expect_error(
     lint("a = 2\n", FALSE, linters = assignment_linter()),
     "'cache' is no longer available as a positional argument",
@@ -241,3 +241,7 @@ test_that("Linters throwing an error give a helpful error", {
     rex::rex("Linter 'broken_linter' failed in ", anything, basename(tmp_file), ": a broken linter")
   )
 })
+
+test_that("typo in argument name gives helpful error", {
+  expect_error(lint("xxx", litners = identity), "Found unknown arguments in [.][.][.].*[?]lint ")
+})

tests/testthat/test-lint_dir.R

@@ -99,7 +99,7 @@ test_that("lint_dir works with specific linters without specifying other argumen
   expect_length(lint_dir(the_dir, commented_code_linter(), parse_settings = FALSE), 0L)
 })
 
-test_that("lint_dir continues to accept relative_path= in 2nd positional argument, with a warning", {
+test_that("lint_dir errors for relative_path= in 2nd positional argument", {
   the_dir <- test_path("dummy_packages", "package", "vignettes")
 
   expect_error(
@@ -108,3 +108,7 @@ test_that("lint_dir continues to accept relative_path= in 2nd positional argumen
     fixed = TRUE
   )
 })
+
+test_that("typo in argument name gives helpful error", {
+  expect_error(lint_dir(litners = identity), "Found unknown arguments in [.][.][.].*[?]lint_dir ")
+})