Merge branch 'main' into uni-allow

.Rbuildignore

@@ -8,7 +8,7 @@
 ^\.Rproj\.user$
 ^\.idea$
 ^\.dev$
-^\.devcontainers$
+^\.devcontainer$
 ^\.lintr$
 ^\.lintr_new$
 ^wercker\.yml$

.dev/roxygen_test.R

@@ -0,0 +1,54 @@
+# Test to ensure roxygenize() has been run on the current PR
+library(tools)
+library(roxygen2)
+
+old_dir <- file.path(tempdir(), "man")
+if (dir.exists(old_dir)) unlink(old_dir, recursive = TRUE)
+file.copy("man", tempdir(), recursive = TRUE)
+old_files <- list.files(old_dir, pattern = "\\.Rd$")
+new_dir <- "man"
+.Last <- function() unlink(old_dir, recursive = TRUE)
+
+# Rd2txt() prints to its out= argument, so we'd have to compare file contents;
+#   plain parse_Rd() keeps srcref info that encodes the file path, which as.character() strips.
+normalize_rd <- function(rd_file) as.character(parse_Rd(rd_file))
+
+rd_equal <- function(f1, f2) isTRUE(all.equal(normalize_rd(f1), normalize_rd(f2)))
+
+check_roxygenize_idempotent <- function(LOCALE) {
+  Sys.setlocale("LC_COLLATE", LOCALE)
+  roxygenize()
+
+  new_files <- list.files(new_dir, pattern = "\\.Rd$")
+
+  old_not_new <- setdiff(old_files, new_files)
+  if (length(old_not_new) > 0L) {
+    stop("Found saved .Rd files gone from a fresh run of roxygenize(): ", toString(old_not_new))
+  }
+
+  new_not_old <- setdiff(new_files, old_files)
+  if (length(new_not_old) > 0L) {
+    stop("Found new .Rd files from a fresh run of roxygenize(): ", toString(new_not_old))
+  }
+
+  for (file in new_files) {
+    old_file <- file.path(old_dir, file)
+    new_file <- file.path(new_dir, file)
+    if (rd_equal(old_file, new_file)) {
+      next
+    }
+    cat(sprintf("roxygenize() output differs from saved output for %s.\n", file))
+    cat("Here's the 'diff' comparison of the two files:\n")
+    cat("  [---]: saved output in man/ directory\n")
+    cat("  [+++]: roxygenize() output of R/ sources\n")
+    system2("diff", c("--unified", old_file, new_file))
+    stop("Failed in LOCALE=", LOCALE, ".", call. = FALSE)
+  }
+}
+
+# Run the check in a few locales to ensure there's no idempotency issues w.r.t. sorting, too
+for (LOCALE in c("C", "en_US", "hu_HU", "ja_JP")) {
+  check_roxygenize_idempotent(LOCALE)
+}
+
+unlink(old_dir, recursive = TRUE)

.devcontainer/Dockerfile

@@ -0,0 +1,14 @@
+FROM rocker/r-base
+
+RUN apt-get -qq update && \
+  apt-get install -y --no-install-recommends git libxml2-dev
+
+COPY DESCRIPTION .
+
+RUN Rscript -e '                           \
+  install.packages("remotes");             \
+  remotes::install_deps(dependencies = c(  \
+    "Imports",                             \
+    "Config/needs/development"             \
+  ))                                       \
+'

.devcontainer/devcontainer.json

@@ -0,0 +1,3 @@
+{
+    "build": { "dockerfile": "Dockerfile", "context": ".."}
+}

.github/workflows/ensure-metadata-tests.yaml → .github/workflows/repo-meta-tests.yaml

@@ -1,14 +1,14 @@
-# Ensure lint metadata is tested
+# Various repo-level tests for code quality
 on:
   push:
     branches: [main]
   pull_request:
     branches: [main]
 
-name: ensure-metadata-tests
+name: repo-meta-tests
 
 jobs:
-  ensure-metadata-tests:
+  repo-meta-tests:
     runs-on: ubuntu-latest
     env:
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
@@ -22,9 +22,17 @@ jobs:
           use-public-rspm: true
 
       - uses: r-lib/actions/setup-r-dependencies@v2
+        with:
+          extra-packages: |
+            any::roxygen2
 
       - name: Ensure lint metadata is tested
         run: |
           options(crayon.enabled = TRUE)
           callr::rscript(".dev/lint_metadata_test.R")
         shell: Rscript {0}
+
+      - name: Ensure roxygen content matches man directory
+        run: |
+          callr::rscript(".dev/roxygen_test.R")
+        shell: Rscript {0}

.lintr

@@ -5,12 +5,9 @@ linters: all_linters(
     todo_comment_linter(
       except_regex = rex::rex(
         "TODO(",
-        group(or(
-          # GitHub issue number #1234, possibly from another repo org/repo#5678
-          list(maybe(one_or_more(alnum, "-"), "/", one_or_more(alnum, ".", "-", "_")), "#", one_or_more(digit)),
-          # GitHub user. TODO(#2450): remove this temporary immunity
-          one_or_more(alnum, "-")
-        )),
+        # GitHub issue number #1234, possibly from another repo org/repo#5678
+        maybe(one_or_more(character_class("a-zA-Z0-9-")), "/", one_or_more(character_class("a-zA-Z0-9._-"))),
+        "#", one_or_more(digit),
         ")"
       )
     ),

DESCRIPTION

@@ -37,7 +37,6 @@ Imports:
 Suggests:
     bookdown,
     cli,
-    httr (>= 1.2.1),
     jsonlite,
     patrick (>= 0.2.0),
     rlang,
@@ -52,10 +51,11 @@ Enhances:
 VignetteBuilder: 
     knitr
 Config/Needs/website: tidyverse/tidytemplate
+Config/Needs/development: pkgload, cli, testthat, patrick
 Config/testthat/edition: 3
 Encoding: UTF-8
 Roxygen: list(markdown = TRUE)
-RoxygenNote: 7.2.3
+RoxygenNote: 7.3.1
 Collate:
     'make_linter_from_xpath.R'
     'xp_utils.R'
@@ -75,7 +75,6 @@ Collate:
     'class_equals_linter.R'
     'commas_linter.R'
     'commented_code_linter.R'
-    'comments.R'
     'comparison_negation_linter.R'
     'condition_call_linter.R'
     'condition_message_linter.R'

NEWS.md

@@ -14,15 +14,17 @@
 * Adjusted various lint messages for consistency in readability (#1330, @MichaelChirico). In general, we favor lint messages to be phrased like "Action, reason" to but the "what" piece of the message front-and-center. This may be a breaking change for code that tests the specific phrasing of lints.
 * `extraction_operator_linter()` is deprecated. Although switching from `$` to `[[` has some robustness benefits for package code, it can lead to non-idiomatic code in many contexts (e.g. R6 classes, Shiny applications, etc.) (#2409, @IndrajeetPatil). To enable the detection of the `$` operator for extraction through partial matching, use `options(warnPartialMatchDollar = TRUE)`.
 * `unnecessary_nested_if_linter()` is deprecated and subsumed into the new/more general `unnecessary_nesting_linter()`.
+* Drop support for posting GitHub comments from inside Travis, Wercker, and Jenkins CI tools (spurred by #2148, @MichaelChirico). We rely on GitHub Actions for linting in CI, and don't see any active users relying on these alternatives. We welcome and encourage community contributions to get support for different CI system going again.
 
 ## Bug fixes
 
 * `object_name_linter()` no longer errors when user-supplied `regexes=` have capture groups (#2188, @MichaelChirico).
 * `.lintr` config validation correctly accepts regular expressions which only compile under `perl = TRUE` (#2375, @MichaelChirico). These have always been valid (since `rex::re_matches()`, which powers the lint exclusion logic, also uses this setting), but the new up-front validation in v3.1.1 incorrectly used `perl = FALSE`.
+* `.lintr` configs set by option `lintr.linter_file` or environment variable `R_LINTR_LINTER_FILE` can point to subdirectories (#2512, @MichaelChirico).
 
 ## Changes to default linters
 
-* New default linter `return_linter()` for the style guide rule that terminal returns should be left implicit (#1100, #2354, and #2356, @MEO265 and @MichaelChirico).
+* New default linter `return_linter()` for the style guide rule that terminal returns should be left implicit (#1100, #2343, #2354, and #2356, @MEO265 and @MichaelChirico).
 
 ## New and improved features
 
@@ -62,7 +64,7 @@
 * `print_linter()` for discouraging usage of `print()` on string literals like `print("Reached here")` or `print(paste("Found", nrow(DF), "rows."))` (#1894, @MichaelChirico).
 * `unnecessary_nesting_linter()` for discouraging overly-nested code where an early return or eliminated sub-expression (inside '{') is preferable (#2317, #2334 and part of #884, @MichaelChirico).
 * `consecutive_mutate_linter()` for encouraging consecutive calls to `dplyr::mutate()` to be combined (part of #884, @MichaelChirico).
-* `if_switch_linter()` for encouraging `switch()` over repeated `if`/`else` tests (part of #884, @MichaelChirico).
+* `if_switch_linter()` for encouraging `switch()` over repeated `if`/`else` tests (#2322 and part of #884, @MichaelChirico).
 * `nested_pipe_linter()` for discouraging pipes within pipes, e.g. `df1 %>% inner_join(df2 %>% select(a, b))` (part of #884, @MichaelChirico).
 * `nrow_subset_linter()` for discouraging usage like `nrow(subset(x, conditions))` in favor of something like `with(x, sum(conditions))` which doesn't require a full subset of `x` (#2313, #2314 and part of #884, @MichaelChirico).
 * `pipe_return_linter()` for discouraging usage of `return()` inside a {magrittr} pipeline (part of #884, @MichaelChirico).

R/boolean_arithmetic_linter.R

@@ -63,7 +63,7 @@ boolean_arithmetic_linter <- function() {
     xml_nodes_to_lints(
       any_expr,
       source_expression = source_expression,
-      # TODO(michaelchirico): customize this?
+      # TODO(#2464): customize this?
       lint_message = paste(
         "Use any() to express logical aggregations.",
         "For example, replace length(which(x == y)) == 0 with !any(x == y)."

R/expect_length_linter.R

@@ -21,7 +21,7 @@
 #' @seealso [linters] for a complete list of linters available in lintr.
 #' @export
 expect_length_linter <- function() {
-  # TODO(michaelchirico): also catch expect_true(length(x) == 1)
+  # TODO(#2465): also catch expect_true(length(x) == 1)
   xpath <- sprintf("
   parent::expr
     /following-sibling::expr[

R/get_source_expressions.R

@@ -54,9 +54,11 @@
 #'   \item{lines}{The [readLines()] output for this file.}
 #' }
 #'
-#' @examplesIf requireNamespace("withr", quietly = TRUE)
-#' tmp <- withr::local_tempfile(lines = c("x <- 1", "y <- x + 1"))
+#' @examples
+#' tmp <- tempfile()
+#' writeLines(c("x <- 1", "y <- x + 1"), tmp)
 #' get_source_expressions(tmp)
+#' unlink(tmp)
 #' @export
 get_source_expressions <- function(filename, lines = NULL) {
   source_expression <- srcfile(filename, encoding = settings$encoding)

R/ids_with_token.R

@@ -17,11 +17,13 @@
 #' the `token` column of `parsed_content`. Typically `==` or `%in%`.
 #' @param source_file (DEPRECATED) Same as `source_expression`. Will be removed.
 #'
-#' @examplesIf requireNamespace("withr", quietly = TRUE)
-#' tmp <- withr::local_tempfile(lines = c("x <- 1", "y <- x + 1"))
+#' @examples
+#' tmp <- tempfile()
+#' writeLines(c("x <- 1", "y <- x + 1"), tmp)
 #' source_exprs <- get_source_expressions(tmp)
 #' ids_with_token(source_exprs$expressions[[1L]], value = "SYMBOL")
 #' with_id(source_exprs$expressions[[1L]], 2L)
+#' unlink(tmp)
 #'
 #' @return `ids_with_token`: The indices of the `parsed_content` data frame
 #' entry of the list of source expressions. Indices correspond to the