warn if some NULLs are not in default (#1049)

* warn if some NULLs are not in default * add PR number to NEWS * commas_linter Co-authored-by: Michael Chirico <[email protected]>
r-lib · Apr 4, 2022 · 05d5bd4 · 05d5bd4
1 parent 9af16af
commit 05d5bd4
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 0 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -46,6 +46,7 @@
 * `T_and_F_symbol_linter` and `semicolon_terminator_linter` are now part of the default linters
   (#517, #612, #683, #684, @AshesITR)
 * `with_defaults()` no longer duplicates the `lintr_function` class when it is already present (#511, #612, @AshesITR)
+* `with_defaults()` now warns if a named argument is `NULL` but its name is not in `default` (#1049, @AshesITR)
 * New `backport_linter()` for detecting mismatched R version dependencies (#506, @MichaelChirico)
 * `paren_brace_linter` and `no_tab_linter` also use more reliable matching (e.g.,
   excluding matches found in comments; #441 and #545, @russHyde)

diff --git a/R/with.R b/R/with.R
@@ -57,6 +57,16 @@ with_defaults <- function(..., default = default_linters) {
     )
   }
 
+  to_null <- vapply(vals, is.null, logical(1L))
+  if (!all(nms[to_null] %in% names(default))) {
+    bad_nms <- setdiff(nms[to_null], names(default))
+    is_are <- if (length(bad_nms) > 1L) "are" else "is"
+    warning(
+      "Trying to remove ", glue::glue_collapse(sQuote(bad_nms), sep = ", ", last = " and "),
+      ", which ", is_are, " not in `default`."
+    )
+  }
+
   is.na(vals) <- nms == vals
   default[nms] <- vals
 

diff --git a/tests/testthat/test-with.R b/tests/testthat/test-with.R
@@ -3,6 +3,11 @@ test_that("with_defaults works as expected with unnamed args", {
   expect_named(with_defaults(assignment_linter), names(with_defaults()))
 })
 
+test_that("with_defaults warns on unused NULLs", {
+  expect_warning(with_defaults(not_a_default = NULL), rex::rex("which is not in `default`."))
+  expect_warning(with_defaults(not_a_default = NULL, also_not_default = NULL), rex::rex("which are not in `default`."))
+})
+
 test_that("all default linters are tagged default", {
   expect_named(with_defaults(), available_linters(tags = "default")$linter)