Add a GHA for unexported, unused objects

.dev/unused_helpers_test.R

@@ -0,0 +1,55 @@
+# Look for unexported objects which are not used internally
+# TODO(#1513): delete this after we have a dedicated linter for it
+
+ns_contents <- parseNamespaceFile("lintr", "..")
+
+export_names <- ns_contents$exports
+registered_s3_methods <- apply(ns_contents$S3methods[, 1L:2L], 1L, paste, collapse = ".")
+
+pkgload::load_all()
+
+internal_names <- setdiff(
+  ls(asNamespace("lintr"), all.names = TRUE),
+  c(export_names, registered_s3_methods)
+)
+
+# .__global__, etc. 
+internal_names <- grep("^[.]__", internal_names, invert = TRUE, value = TRUE)
+# other generic R names
+internal_names <- setdiff(internal_names, c(".onLoad", ".onAttach", ".packageName"))
+# known false positives
+#   - %||% is a mask for certain versions of R
+#   - addin_lint* are used by RStudio even though they're not exported
+#   - regexes_rd is used in ?object_name_linter by an \Sexpr execution
+internal_names <- setdiff(internal_names, c("%||%", "addin_lint", "addin_lint_package", "regexes_rd"))
+
+is_operator <- make.names(internal_names) != internal_names
+
+operator_linter <- undesirable_operator_linter(internal_names[is_operator])
+function_linter <- undesirable_function_linter(internal_names[!is_operator])
+
+# suppressWarnings: #nolint for linters not present here
+usages <- as.data.frame(suppressWarnings(lint_dir("R", linters = list(operator_linter, function_linter))))
+usages$used_call <- gsub('.*["`]([^"`]*)["`].*', "\\1", usages$message)
+# TODO(#2815): these should be excluded by the linter itself
+usages <- subset(usages, !mapply(\(nm, l) grepl(sprintf("\\b%s <-", nm), l), rex::rex(used_call), line))
+
+# TODO(#2004): can this just come from get_source_expressions(), and/or will
+#   the above linters "just work" once we're roxygen2-aware?
+options(keep.source = TRUE)
+roxy_usage <- character()
+for (robj in roxygen2::parse_package()) {
+  for (tag in robj$tags) {
+    if (tag$tag == "evalRd") {
+      roxy_usage <- c(roxy_usage,
+        subset(getParseData(tag$val), token == "SYMBOL_FUNCTION_CALL", select = "text", drop = TRUE)
+      )
+    }
+  }
+}
+
+unused_names <- setdiff(internal_names, c(usages$used_call, roxy_usage))
+
+if (length(unused_names) > 0L) {
+  stop("These objects are in the {lintr} namespace but don't have any usage: ", toString(unused_names))
+}

.github/workflows/repo-meta-tests.yaml

@@ -45,3 +45,8 @@ jobs:
         run: |
           callr::rscript(".dev/defunct_linters_test.R")
         shell: Rscript {0}
+
+      - name: Ensure any objects in the namespace are actually needed
+        run: |
+          callr::rscript(".dev/unused_helpers_test.R")
+        shell: Rscript {0}

NAMESPACE

@@ -167,6 +167,7 @@ export(yoda_test_linter)
 importFrom(cli,cli_abort)
 importFrom(cli,cli_inform)
 importFrom(cli,cli_warn)
+importFrom(cli,qty)
 importFrom(glue,glue)
 importFrom(glue,glue_collapse)
 importFrom(rex,character_class)

NEWS.md

@@ -23,12 +23,15 @@
 * `seq_linter()`:
    + recommends using `seq_along(x)` instead of `seq_len(length(x))` (#2577, @MichaelChirico).
    + recommends using `sequence()` instead of `unlist(lapply(ints, seq))` (#2618, @Bisaloo)
-* `undesirable_operator_linter()` lints operators in prefix form, e.g. `` `%%`(x, 2)`` (#1910, @MichaelChirico). Disable this by setting `call_is_undesirable=FALSE`.
+* `undesirable_operator_linter()`:
+   + Lints operators in prefix form, e.g. `` `%%`(x, 2)`` (#1910, @MichaelChirico). Disable this by setting `call_is_undesirable=FALSE`.
+   + Accepts unnamed entries, treating them as undesirable operators, e.g. `undesirable_operator_linter("%%")` (#2536, @MichaelChirico).
 * `indentation_linter()` handles `for` un-braced for loops correctly (#2564, @MichaelChirico).
 * Setting `exclusions` supports globs like `knitr*` to exclude files/directories with a pattern (#1554, @MichaelChirico).
 * `get_source_expression()` captures warnings emitted by the R parser (currently always for mis-specified literal integers like `1.1L`) and `lint()` returns them as lints (#2065, @MichaelChirico).
 * `object_name_linter()` and `object_length_linter()` apply to objects assigned with `assign()` or generics created with `setGeneric()` (#1665, @MichaelChirico).
 * `object_usage_linter()` gains argument `interpret_extensions` to govern which false positive-prone common syntaxes should be checked for used objects (#1472, @MichaelChirico). Currently `"glue"` (renamed from earlier argument `interpret_glue`) and `"rlang"` are supported. The latter newly covers usage of the `.env` pronoun like `.env$key`, where `key` was previously missed as being a used variable.
+* `undesirable_function_linter()` accepts unnamed entries, treating them as undesirable functions, e.g. `undesirable_function_linter("sum")` (#2536, @MichaelChirico).
 
 ### New linters
 

R/lintr-package.R

@@ -8,7 +8,7 @@
 "_PACKAGE"
 
 ## lintr namespace: start
-#' @importFrom cli cli_inform cli_abort cli_warn
+#' @importFrom cli cli_inform cli_abort cli_warn qty
 #' @importFrom glue glue glue_collapse
 #' @importFrom rex rex regex re_matches re_substitutes character_class
 #' @importFrom stats complete.cases na.omit

R/undesirable_function_linter.R

@@ -2,10 +2,17 @@
 #'
 #' Report the use of undesirable functions and suggest an alternative.
 #'
-#' @param fun Named character vector. `names(fun)` correspond to undesirable functions,
-#'   while the values give a description of why the function is undesirable.
-#'   If `NA`, no additional information is given in the lint message. Defaults to
-#'   [default_undesirable_functions]. To make small customizations to this list,
+#' @param fun Character vector of undesirable function names. Input can be any of three types:
+#'   - Unnamed entries must be a character string specifying an undesirable function.
+#'   - For named entries, the name specifies the undesirable function.
+#'     + If the entry is a character string, it is used as a description of
+#'       why a given function is undesirable
+#'     + Otherwise, entries should be missing (`NA`)
+#'   A generic message that the named function is undesirable is used if no
+#'     specific description is provided.
+#'   Input can also be a list of character strings for convenience.
+#'
+#'   Defaults to [default_undesirable_functions]. To make small customizations to this list,
 #'   use [modify_defaults()].
 #' @param symbol_is_undesirable Whether to consider the use of an undesirable function
 #'   name as a symbol undesirable or not.
@@ -35,6 +42,12 @@
 #'   linters = undesirable_function_linter(fun = c("dir" = NA))
 #' )
 #'
+#'
+#' lint(
+#'   text = 'dir <- "path/to/a/directory"',
+#'   linters = undesirable_function_linter(fun = "dir")
+#' )
+#'
 #' # okay
 #' lint(
 #'   text = "vapply(x, mean, FUN.VALUE = numeric(1))",
@@ -51,16 +64,35 @@
 #'   linters = undesirable_function_linter(fun = c("dir" = NA), symbol_is_undesirable = FALSE)
 #' )
 #'
+#' lint(
+#'   text = 'dir <- "path/to/a/directory"',
+#'   linters = undesirable_function_linter(fun = "dir", symbol_is_undesirable = FALSE)
+#' )
+#'
 #' @evalRd rd_tags("undesirable_function_linter")
 #' @seealso [linters] for a complete list of linters available in lintr.
 #' @export
 undesirable_function_linter <- function(fun = default_undesirable_functions,
                                         symbol_is_undesirable = TRUE) {
-  stopifnot(is.logical(symbol_is_undesirable))
-  if (is.null(names(fun)) || !all(nzchar(names(fun))) || length(fun) == 0L) {
-    cli_abort(c(
-      x = "{.arg fun} should be a non-empty named character vector.",
-      i = "Use missing elements to indicate default messages."
+  if (is.list(fun)) fun <- unlist(fun)
+  stopifnot(
+    is.logical(symbol_is_undesirable),
+    # allow (uncoerced->implicitly logical) 'NA'
+    `\`fun\` should be a non-empty character vector` =
+      length(fun) > 0L && (is.character(fun) || all(is.na(fun)))
+  )
+
+  nm <- names2(fun)
+  implicit_idx <- !nzchar(nm)
+  if (any(implicit_idx)) {
+    names(fun)[implicit_idx] <- fun[implicit_idx]
+    is.na(fun) <- implicit_idx
+  }
+  if (anyNA(names(fun))) {
+    missing_idx <- which(is.na(names(fun))) # nolint: object_usage_linter. False positive.
+    cli_abort(paste(
+      "Unnamed elements of {.arg fun} must not be missing,",
+      "but {.val {missing_idx}} {qty(length(missing_idx))} {?is/are}."
     ))
   }
 

R/undesirable_operator_linter.R

@@ -3,10 +3,17 @@
 #' Report the use of undesirable operators, e.g. \code{\link[base:ns-dblcolon]{:::}} or
 #' [`<<-`][base::assignOps] and suggest an alternative.
 #'
-#' @param op Named character vector. `names(op)` correspond to undesirable operators,
-#'   while the values give a description of why the operator is undesirable.
-#'   If `NA`, no additional information is given in the lint message. Defaults to
-#'   [default_undesirable_operators]. To make small customizations to this list,
+#' @param op Character vector of undesirable operators. Input can be any of three types:
+#'   - Unnamed entries must be a character string specifying an undesirable operator.
+#'   - For named entries, the name specifies the undesirable operator.
+#'     + If the entry is a character string, it is used as a description of
+#'       why a given operator is undesirable
+#'     + Otherwise, entries should be missing (`NA`)
+#'   A generic message that the named operator is undesirable is used if no
+#'     specific description is provided.
+#'   Input can also be a list of character strings for convenience.
+#'
+#'   Defaults to [default_undesirable_operators]. To make small customizations to this list,
 #'   use [modify_defaults()].
 #' @param call_is_undesirable Logical, default `TRUE`. Should lints also be produced
 #'   for prefix-style usage of the operators provided in `op`?
@@ -31,6 +38,11 @@
 #'   linters = undesirable_operator_linter()
 #' )
 #'
+#' lint(
+#'   text = "mtcars$wt",
+#'   linters = undesirable_operator_linter("$")
+#' )
+#'
 #' # okay
 #' lint(
 #'   text = "a <- log(10)",
@@ -51,17 +63,38 @@
 #'   linters = undesirable_operator_linter(call_is_undesirable = FALSE)
 #' )
 #'
+#' lint(
+#'   text = 'mtcars[["wt"]]',
+#'   linters = undesirable_operator_linter("$")
+#' )
+#'
 #' @evalRd rd_tags("undesirable_operator_linter")
 #' @seealso [linters] for a complete list of linters available in lintr.
 #' @export
 undesirable_operator_linter <- function(op = default_undesirable_operators,
                                         call_is_undesirable = TRUE) {
-  if (is.null(names(op)) || !all(nzchar(names(op))) || length(op) == 0L) {
-    cli_abort(c(
-      x = "{.arg op} should be a non-empty named character vector.",
-      i = "Use missing elements to indicate default messages."
+  if (is.list(op)) op <- unlist(op)
+  stopifnot(
+    is.logical(call_is_undesirable),
+    # allow (uncoerced->implicitly logical) 'NA'
+    `\`op\` should be a non-empty character vector` =
+      length(op) > 0L && (is.character(op) || all(is.na(op)))
+  )
+
+  nm <- names2(op)
+  implicit_idx <- !nzchar(nm)
+  if (any(implicit_idx)) {
+    names(op)[implicit_idx] <- op[implicit_idx]
+    is.na(op) <- implicit_idx
+  }
+  if (anyNA(names(op))) {
+    missing_idx <- which(is.na(names(op))) # nolint: object_usage_linter. False positive.
+    cli_abort(paste(
+      "Unnamed elements of {.arg op} must not be missing,",
+      "but {.val {missing_idx}} {qty(length(missing_idx))} {?is/are}."
     ))
   }
+
   # infix must be handled individually below; non-assignment `=` are always OK
   operator_nodes <- infix_metadata$xml_tag_exact[
     infix_metadata$string_value %in% setdiff(names(op), "%%") &

R/utils.R

@@ -169,11 +169,6 @@ read_lines <- function(file, encoding = settings$encoding, ...) {
   lines
 }
 
-# nocov start
-# support for usethis::use_release_issue(). Make sure to use devtools::load_all() beforehand!
-release_bullets <- function() {}
-# nocov end
-
 # see issue #923, PR #2455 -- some locales ignore _ when running sort(), others don't.
 #   We want to consistently treat "_" < "n" = "N"; C locale does this, which 'radix' uses.
 platform_independent_order <- function(x) order(tolower(x), method = "radix")