Accept default= in modify_defaults (#1339)

Author: MichaelChirico

NEWS.md

@@ -52,7 +52,9 @@
   It has also been renamed as the argument of the now-private functional versions of many linters, which has no direct
   effect on packages importing lintr, but is mentioned in case custom linters imitating `lintr` style have also
   adopted the `source_file` naming and want to adapt to keep in sync.
-* Deprecated `with_defaults()` in favor of `linters_with_defaults()` (#1029, @AshesITR)
+* Deprecated `with_defaults()` in favor of `linters_with_defaults()`, and add `modify_defaults()` which is intended to be used
+  more generally to modify (i.e., extend, trim, and/or update) a list of defaults. Note that the argument corresponding to
+  `with_defaults()`'s `default=` is called `defaults=` (i.e., pluralized) in both of these (#1029, #1336, @AshesITR and @michaelchirico)
 
 ## Other changes to defaults
 

R/with.R

@@ -24,22 +24,9 @@ modify_defaults <- function(defaults, ...) {
   }
   vals <- list(...)
   nms <- names2(vals)
-  missing <- !nzchar(nms, keepNA = TRUE)
-  if (any(missing)) {
-    args <- as.character(eval(substitute(alist(...)[missing])))
-    # foo_linter(x=1) => "foo"
-    # var[["foo"]]    => "foo"
-    nms[missing] <- re_substitutes(
-      re_substitutes(
-          # Very long input might have newlines which are not caught
-          #  by . in a perl regex; see #774
-        re_substitutes(args, rex("(", anything), "", options = "s"),
-        rex(start, anything, '["' %or% "::"),
-        ""
-      ),
-      rex('"]', anything, end),
-      ""
-    )
+  missing_index <- !nzchar(nms, keepNA = TRUE)
+  if (any(missing_index)) {
+    nms[missing_index] <- guess_names(..., missing_index = missing_index)
   }
 
   to_null <- vapply(vals, is.null, logical(1L))
@@ -160,6 +147,22 @@ linters_with_tags <- function(tags, ..., packages = "lintr", exclude_tags = "dep
 #'   absolute_path_linter()
 #' )
 linters_with_defaults <- function(..., defaults = default_linters) {
+  dots <- list(...)
+  if (missing(defaults) && "default" %in% names(dots)) {
+    warning(
+      "'default' is not an argument to linters_with_defaults(). Did you mean 'defaults'? ",
+      "This warning will be removed when with_defaults() is fully deprecated."
+    )
+    defaults <- dots$default
+    nms <- names2(dots)
+    missing_index <- !nzchar(nms, keepNA = TRUE)
+    if (any(missing_index)) {
+      names(dots)[missing_index] <- guess_names(..., missing_index = missing_index)
+    }
+    dots$default <- NULL
+    dots <- c(dots, list(defaults = defaults))
+    return(do.call(modify_defaults, dots))
+  }
   modify_defaults(..., defaults = defaults)
 }
 
@@ -181,3 +184,16 @@ call_linter_factory <- function(linter_factory, linter_name, package) {
   attr(linter, "name") <- linter_name
   linter
 }
+
+guess_names <- function(..., missing_index) {
+  args <- as.character(eval(substitute(alist(...)[missing_index])))
+  # foo_linter(x=1) => "foo"
+  # var[["foo"]]    => "foo"
+  # strip call: foo_linter(x=1) --> foo_linter
+  # NB: Very long input might have newlines which are not caught
+  #  by . in a perl regex; see #774
+  args <- re_substitutes(args, rex("(", anything), "", options = "s")
+  # strip extractors: pkg::foo_linter, var[["foo_linter"]] --> foo_linter
+  args <- re_substitutes(args, rex(start, anything, '["' %or% "::"), "")
+  re_substitutes(args, rex('"]', anything, end), "")
+}

tests/testthat/test-with.R

@@ -61,3 +61,17 @@ test_that("modify_defaults works", {
   # auto-sorts
   expect_equal(modify_defaults(defaults = list(b = 2L, a = 1L), c = 3L), my_default)
 })
+
+test_that("linters_with_defaults(default = .) is supported with a deprecation warning", {
+  expect_warning(linters <- linters_with_defaults(default = list(), no_tab_linter()), "'default'")
+  expect_named(linters, "no_tab_linter")
+
+  # the same warning is not triggered in modify_defaults
+  expect_silent(linters <- modify_defaults(defaults = list(), default = list(), no_tab_linter()))
+  expect_named(linters, c("default", "no_tab_linter"))
+
+  # if default= is explicitly provided alongside defaults=, assume that was intentional
+  default <- Linter(function(.) list())
+  expect_silent(linters <- linters_with_defaults(defaults = list(), default = default))
+  expect_named(linters, "default")
+})