Merge branch 'main' into joss_paper

.github/workflows/lint.yaml

@@ -21,6 +21,7 @@ jobs:
       - uses: r-lib/actions/setup-r-dependencies@v2
         with:
           extra-packages: |
+            any::cyclocomp
             r-lib/lintr
             local::.
           needs: lint

.github/workflows/pkgdown.yaml

@@ -39,7 +39,7 @@ jobs:
 
       - name: Deploy to GitHub pages 🚀
         if: github.event_name != 'pull_request'
-        uses: JamesIves/[email protected].0
+        uses: JamesIves/[email protected].1
         with:
           clean: false
           branch: gh-pages

.github/workflows/test-coverage-examples.yaml

@@ -0,0 +1,71 @@
+on:
+  schedule:
+    # * is a special character in YAML so you have to quote this string
+    # Trigger once a month at 10:00 on the first day of every month
+    - cron: "00 10 1 * *"
+
+name: test-coverage-examples
+
+jobs:
+  test-coverage-examples:
+    runs-on: ubuntu-latest
+    env:
+      GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: r-lib/actions/setup-r@v2
+        with:
+          use-public-rspm: true
+
+      - uses: r-lib/actions/setup-r-dependencies@v2
+        with:
+          extra-packages: |
+            any::covr
+            local::.
+
+      - name: Test example coverage
+        run: |
+          options(crayon.enabled = TRUE)
+          library(covr)
+          files_to_exclude <- c(
+            # examples present but not run
+            "R/lint.R",
+            "R/use_lintr.R",
+            # mostly internal utilities
+            "R/actions.R",
+            "R/cache.R",
+            "R/deprecated.R",
+            "R/exclude.R",
+            "R/extract.R",
+            "R/ids_with_token.R",
+            "R/lintr-deprecated.R",
+            "R/make_linter_from_regex.R",
+            "R/make_linter_from_xpath.R",
+            "R/namespace.R",
+            "R/methods.R",
+            "R/settings.R",
+            "R/shared_constants.R",
+            "R/with.R",
+            "R/with_id.R",
+            "R/zzz.R"
+          )
+          coverage <- covr::package_coverage(
+            type = "examples",
+            quiet = TRUE,
+            commentDonttest = FALSE,
+            commentDontrun = FALSE,
+            line_exclusions = files_to_exclude
+          )
+          print(coverage)
+          percent_coverage <- as.integer(covr::percent_coverage(coverage))
+          threshold <- 90
+          cli::cli_rule()
+          if (percent_coverage < threshold) {
+            cli::cli_abort("Code coverage using examples ({percent_coverage}%) is below the required threshold ({threshold}%).")
+          } else {
+            cli::cli_alert_success("Code coverage using examples ({percent_coverage}%) is above the required threshold ({threshold}%).")
+          }
+          cli::cli_rule()
+        shell: Rscript {0}

DESCRIPTION

@@ -25,7 +25,6 @@ Depends:
 Imports:
     backports (>= 1.1.7),
     codetools,
-    cyclocomp,
     digest,
     glue,
     knitr,
@@ -37,6 +36,7 @@ Imports:
 Suggests:
     bookdown,
     cli,
+    cyclocomp,
     jsonlite,
     patrick (>= 0.2.0),
     rlang,
@@ -202,6 +202,7 @@ Collate:
     'with.R'
     'with_id.R'
     'xml_nodes_to_lints.R'
+    'xml_utils.R'
     'yoda_test_linter.R'
     'zzz.R'
 Language: en-US

NAMESPACE

@@ -57,6 +57,7 @@ export(expect_length_linter)
 export(expect_lint)
 export(expect_lint_free)
 export(expect_named_linter)
+export(expect_no_lint)
 export(expect_not_linter)
 export(expect_null_linter)
 export(expect_s3_class_linter)
@@ -171,7 +172,6 @@ export(with_id)
 export(xml_nodes_to_lints)
 export(xp_call_name)
 export(yoda_test_linter)
-importFrom(cyclocomp,cyclocomp)
 importFrom(glue,glue)
 importFrom(glue,glue_collapse)
 importFrom(rex,character_class)

NEWS.md

@@ -15,6 +15,8 @@
 * `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 GitHub comment bot, 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.
+* `cyclocomp_linter()` is no longer part of the default linters (#2555, @IndrajeetPatil) because the tidyverse style guide doesn't contain any guidelines on meeting certain complexity requirements. Note that users with `cyclocomp_linter()` in their configs may now need to install {cyclocomp} intentionally, in particular in CI/CD pipelines.
+* `scalar_in_linter` is now configurable to allow other `%in%` like operators to be linted. The data.table operator `%chin%` is no longer linted by default; use `in_operators = "%chin%"` to continue linting it. (@F-Noelle)
 
 ## Bug fixes
 
@@ -52,7 +54,8 @@
 * `vector_logic_linter()` is extended to recognize incorrect usage of scalar operators `&&` and `||` inside subsetting expressions like `dplyr::filter(x, A && B)` (#2166, @MichaelChirico).
 * `any_is_na_linter()` is extended to catch the unusual usage `NA %in% x` (#2113, @MichaelChirico).
 * `make_linter_from_xpath()` errors up front when `lint_message` is missing (instead of delaying this error until the linter is used, #2541, @MichaelChirico).
-* `paste_linter()`  is extended to recommend using `paste()` instead of `paste0()` for simply aggregating a character vector with `collapse=`, i.e., when `sep=` is irrelevant (#1108, @MichaelChirico).
+* `paste_linter()` is extended to recommend using `paste()` instead of `paste0()` for simply aggregating a character vector with `collapse=`, i.e., when `sep=` is irrelevant (#1108, @MichaelChirico).
+* `expect_no_lint()` was added as new function to cover the typical use case of expecting no lint message, akin to the recent {testthat} functions like `expect_no_warning()` (#2580, @F-Noelle).
 
 ### New linters
 

R/backport_linter.R

@@ -30,6 +30,11 @@
 #'   linters = backport_linter("4.0.0")
 #' )
 #'
+#' lint(
+#'   text = "str2lang(x)",
+#'   linters = backport_linter("3.2.0", except = "str2lang")
+#' )
+#'
 #' @evalRd rd_tags("backport_linter")
 #' @seealso [linters] for a complete list of linters available in lintr.
 #' @export

R/cyclocomp_linter.R

@@ -1,11 +1,11 @@
 #' Cyclomatic complexity linter
 #'
-#' Check for overly complicated expressions. See [cyclocomp::cyclocomp()].
+#' Check for overly complicated expressions. See `cyclocomp()` function from `{cyclocomp}`.
 #'
-#' @param complexity_limit Maximum cyclomatic complexity, default 15. Expressions more complex
-#' than this are linted. See [cyclocomp::cyclocomp()].
+#' @param complexity_limit Maximum cyclomatic complexity, default `15`. Expressions more complex
+#' than this are linted.
 #'
-#' @examples
+#' @examplesIf requireNamespace("cyclocomp", quietly = TRUE)
 #' # will produce lints
 #' lint(
 #'   text = "if (TRUE) 1 else 2",
@@ -23,6 +23,15 @@
 #' @export
 cyclocomp_linter <- function(complexity_limit = 15L) {
   Linter(linter_level = "expression", function(source_expression) {
+    # nocov start
+    if (!requireNamespace("cyclocomp", quietly = TRUE)) {
+      cli::cli_abort(c(
+        "Cyclocomp complexity is computed using {.fn cyclocomp::cyclocomp}.",
+        i = "Please install the needed {.pkg cyclocomp} package."
+      ))
+    }
+    # nocov end
+
     complexity <- try_silently(
       cyclocomp::cyclocomp(parse(text = source_expression$content))
     )

R/exclude.R

@@ -30,6 +30,8 @@
 #'   "   a character vector of files to exclude or a vector of lines to exclude.",
 #'   NULL
 #' )
+#'
+#' @keywords internal
 exclude <- function(lints, exclusions = settings$exclusions, linter_names = NULL, ...) {
   if (length(lints) <= 0L) {
     return(lints)
@@ -100,6 +102,7 @@ line_info <- function(line_numbers, type = c("start", "end")) {
 #' @param linter_names Names of active linters.
 #'
 #' @return A possibly named list of excluded lines, possibly for specific linters.
+#' @keywords internal
 parse_exclusions <- function(file,
                              exclude = settings$exclude,
                              exclude_next = settings$exclude_next,

R/expect_lint.R

@@ -1,6 +1,8 @@
 #' Lint expectation
 #'
-#' This is an expectation function to test that the lints produced by `lint` satisfy a number of checks.
+#' These are expectation functions to test specified linters on sample code in the `testthat` testing framework.
+#' * `expect_lint` asserts that specified lints are generated.
+#' * `expect_no_lint` asserts that no lints are generated.
 #'
 #' @param content a character vector for the file content to be linted, each vector element representing a line of
 #' text.
@@ -22,7 +24,7 @@
 #' @return `NULL`, invisibly.
 #' @examples
 #' # no expected lint
-#' expect_lint("a", NULL, trailing_blank_lines_linter())
+#' expect_no_lint("a", trailing_blank_lines_linter())
 #'
 #' # one expected lint
 #' expect_lint("a\n", "trailing blank", trailing_blank_lines_linter())
@@ -42,7 +44,8 @@
 expect_lint <- function(content, checks, ..., file = NULL, language = "en") {
   if (!requireNamespace("testthat", quietly = TRUE)) {
     stop( # nocov start
-      "'expect_lint' is designed to work within the 'testthat' testing framework, but 'testthat' is not installed.",
+      "'expect_lint' and 'expect_no_lint' are designed to work within the 'testthat' testing framework, ",
+      "but 'testthat' is not installed.",
       call. = FALSE
     ) # nocov end
   }
@@ -123,6 +126,11 @@ expect_lint <- function(content, checks, ..., file = NULL, language = "en") {
   invisible(NULL)
 }
 
+#' @rdname expect_lint
+#' @export
+expect_no_lint <- function(content, ..., file = NULL, language = "en") {
+  expect_lint(content, NULL, ..., file = file, language = language)
+}
 
 #' Test that the package is lint free
 #'

R/inner_combine_linter.R

@@ -6,6 +6,13 @@
 #'   preferred so that the most expensive part of the operation ([as.Date()])
 #'   is applied only once.
 #'
+#' Note that [strptime()] has one idiosyncrasy to be aware of, namely that
+#'   auto-detected `format=` is set by the first matching input, which means
+#'   that a case like `c(as.POSIXct("2024-01-01"), as.POSIXct("2024-01-01 01:02:03"))`
+#'   gives different results to `as.POSIXct(c("2024-01-01", "2024-01-01 01:02:03"))`.
+#'   This false positive is rare; a workaround where possible is to use
+#'   consistent formatting, i.e., `"2024-01-01 00:00:00"` in the example.
+#'
 #' @examples
 #' # will produce lints
 #' lint(

R/lintr-package.R

@@ -8,7 +8,6 @@
 "_PACKAGE"
 
 ## lintr namespace: start
-#' @importFrom cyclocomp cyclocomp
 #' @importFrom glue glue glue_collapse
 #' @importFrom rex rex regex re_matches re_substitutes character_class
 #' @importFrom stats na.omit

R/scalar_in_linter.R

@@ -1,12 +1,14 @@
 #' Block usage like x %in% "a"
 #'
 #' `vector %in% set` is appropriate for matching a vector to a set, but if
-#'   that set has size 1, `==` is more appropriate. `%chin%` from `{data.table}`
-#'   is matched as well.
+#'   that set has size 1, `==` is more appropriate.
 #'
 #' `scalar %in% vector` is OK, because the alternative (`any(vector == scalar)`)
 #'   is more circuitous & potentially less clear.
 #'
+#' @param in_operators Character vector of additional infix operators that behave like the `%in%` operator,
+#'   e.g. `{data.table}`'s `%chin%` operator.
+#'
 #' @examples
 #' # will produce lints
 #' lint(
@@ -16,7 +18,7 @@
 #'
 #' lint(
 #'   text = "x %chin% 'a'",
-#'   linters = scalar_in_linter()
+#'   linters = scalar_in_linter(in_operators = "%chin%")
 #' )
 #'
 #' # okay
@@ -28,22 +30,24 @@
 #' @evalRd rd_tags("scalar_in_linter")
 #' @seealso [linters] for a complete list of linters available in lintr.
 #' @export
-scalar_in_linter <- function() {
+scalar_in_linter <- function(in_operators = NULL) {
   # TODO(#2085): Extend to include other cases where the RHS is clearly a scalar
   # NB: all of logical, integer, double, hex, complex are parsed as NUM_CONST
-  xpath <- "
-  //SPECIAL[text() = '%in%' or text() = '%chin%']
+  xpath <- glue("
+  //SPECIAL[{xp_text_in_table(c('%in%', {in_operators}))}]
     /following-sibling::expr[NUM_CONST[not(starts-with(text(), 'NA'))] or STR_CONST]
     /parent::expr
-  "
+  ")
 
   Linter(linter_level = "expression", function(source_expression) {
     xml <- source_expression$xml_parsed_content
 
     bad_expr <- xml_find_all(xml, xpath)
     in_op <- xml_find_chr(bad_expr, "string(SPECIAL)")
-    lint_msg <-
-      paste0("Use == to match length-1 scalars, not ", in_op, ". Note that == preserves NA where ", in_op, " does not.")
+    lint_msg <- glue(
+      "Use comparison operators (e.g. ==, !=, etc.) to match length-1 scalars instead of {in_op}. ",
+      "Note that comparison operators preserve NA where {in_op} does not."
+    )
 
     xml_nodes_to_lints(
       bad_expr,

R/utils.R

@@ -86,17 +86,6 @@ names2 <- function(x) {
   names(x) %||% rep("", length(x))
 }
 
-safe_parse_to_xml <- function(parsed_content) {
-  if (is.null(parsed_content)) {
-    return(xml2::xml_missing())
-  }
-  tryCatch(
-    xml2::read_xml(xmlparsedata::xml_parse_data(parsed_content)),
-    # use xml_missing so that code doesn't always need to condition on XML existing
-    error = function(e) xml2::xml_missing()
-  )
-}
-
 get_content <- function(lines, info) {
   lines[is.na(lines)] <- ""
 
@@ -226,8 +215,8 @@ platform_independent_sort <- function(x) x[platform_independent_order(x)]
 #' writeLines("c('a', 'b')", tmp)
 #' expr_as_xml <- get_source_expressions(tmp)$expressions[[1L]]$xml_parsed_content
 #' writeLines(as.character(expr_as_xml))
-#' get_r_string(expr_as_xml, "expr[2]") # "a"
-#' get_r_string(expr_as_xml, "expr[3]") # "b"
+#' get_r_string(expr_as_xml, "expr[2]")
+#' get_r_string(expr_as_xml, "expr[3]")
 #' unlink(tmp)
 #'
 #' # more importantly, extract strings under R>=4 raw strings
@@ -236,8 +225,8 @@ platform_independent_sort <- function(x) x[platform_independent_order(x)]
 #' writeLines("c(R'(a\\b)', R'--[a\\\"\'\"\\b]--')", tmp4.0)
 #' expr_as_xml4.0 <- get_source_expressions(tmp4.0)$expressions[[1L]]$xml_parsed_content
 #' writeLines(as.character(expr_as_xml4.0))
-#' get_r_string(expr_as_xml4.0, "expr[2]") # "a\\b"
-#' get_r_string(expr_as_xml4.0, "expr[3]") # "a\\\"'\"\\b"
+#' get_r_string(expr_as_xml4.0, "expr[2]")
+#' get_r_string(expr_as_xml4.0, "expr[3]")
 #' unlink(tmp4.0)
 #'
 #' @export
@@ -257,18 +246,6 @@ get_r_string <- function(s, xpath = NULL) {
   out
 }
 
-#' str2lang, but for xml children.
-#'
-#' [xml2::xml_text()] is deceptively close to obviating this helper, but it collapses
-#'   text across lines. R is _mostly_ whitespace-agnostic, so this only matters in some edge cases,
-#'   in particular when there are comments within an expression (`<expr>` node). See #1919.
-#'
-#' @noRd
-xml2lang <- function(x) {
-  x_strip_comments <- xml_find_all(x, ".//*[not(self::COMMENT or self::expr)]")
-  str2lang(paste(xml_text(x_strip_comments), collapse = " "))
-}
-
 is_linter <- function(x) inherits(x, "linter")
 
 is_tainted <- function(lines) {

R/xml_nodes_to_lints.R

@@ -96,10 +96,3 @@ xml_nodes_to_lints <- function(xml, source_expression, lint_message,
     ranges = list(c(col1, col2))
   )
 }
-
-is_node <- function(xml) inherits(xml, "xml_node")
-is_nodeset <- function(xml) inherits(xml, "xml_nodeset")
-is_nodeset_like <- function(xml) {
-  is_nodeset(xml) ||
-    (is.list(xml) && all(vapply(xml, is_node, logical(1L))))
-}