diff --git a/.lintr_new b/.lintr_new index 4852868cf6..88b0f3ad3a 100644 --- a/.lintr_new +++ b/.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(), diff --git a/NEWS.md b/NEWS.md index ed70169a90..e525cdf743 100644 --- a/NEWS.md +++ b/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. diff --git a/R/lint.R b/R/lint.R index 86a7b41feb..1102292c47 100644 --- a/R/lint.R +++ b/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) diff --git a/R/utils.R b/R/utils.R index 505eeed59a..a0d6460c02 100644 --- a/R/utils.R +++ b/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 + ) +} diff --git a/R/zzz.R b/R/zzz.R index 5356ac7c3b..adfa7ee611 100644 --- a/R/zzz.R +++ b/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") diff --git a/tests/testthat/test-lint.R b/tests/testthat/test-lint.R index b0282e8342..9295c14ac0 100644 --- a/tests/testthat/test-lint.R +++ b/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 ") +}) diff --git a/tests/testthat/test-lint_dir.R b/tests/testthat/test-lint_dir.R index 74d19c5242..e195257617 100644 --- a/tests/testthat/test-lint_dir.R +++ b/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 ") +})